-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GH-82] Added slash command to schedule a meeting on Zoom #350
Conversation
1. Updated and integrated API with frontend. 2. Added slash command to open the meeting modal.
1. Added logic to post meeting reminder in channle. 2. Added logic to check if the user is connected before running the schedule command 3. Removed the user of p.API from the schedule meeting feature code
@raghavaggarwal2308 can you confirm that the 'Join meeting' button will only show for Announcement post if the current time is 5 minutes within the scheduled meeting? This was discussed in this comment.
|
@mickmister @matthewbirtch This PR is ready for review |
@raghavaggarwal2308 Are you able to have this on a cloud server to be reviewed by UX? |
This would be helpful. Thanks! |
@mickmister @matthewbirtch Messaged you the creds on https://hub.mattermost.com. Please check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raghavaggarwal2308 thanks for setting up the test server. I spent some time reviewing and have some changes that I'd like to request.
-
Can we separate the date and time fields? So you would have a start time and end time? Then duration wouldn't be needed. I believe this would also better align with the calendar plugins.
-
The date picker UI style doesn't really align with any of the UI in the rest of the app. Are we able to adjust this at all? Can we style it so it is more like the other date pickers we're using in the app elsewhere? (e.g. custom do not disturb time, search on date picker - see screenshot below)
-
For the date field, can we update the calendar icon to be the same one used in the do not disturb custom time modal?
-
When I scheduled a meeting for 30 minutes in the future, it immediately opened the meeting in zoom. It shouldn't do this. It should just post the scheduled meeting post in the channel.
-
Can you adjust the cancel button in the footer to change the class
btn-link
tobtn-tertiary
? -
Is there anything that can be done to update the modal style to the latest modal style we use? This style is very dated looking.
-
For the radio buttons and checkbox labels, can we make them not bold? And also add 4-5px of space between the last two checkboxes (they are sitting too close). Something like this:
I'm sure some of this feedback will require a bit of back and forth so let me know if there is anything that is unclear.
We will seperate the date and time picker. |
Here is a screenshot from google calender
@matthewbirtch Please let me know which one of the above will you prefer. |
I think that's ok for this. Going with what we have for Google Calendar works. |
Zoom allows you to schedule meetings with custom time, using Google Calender UI will restrict it from creating meetings with custom times. @matthewbirtch |
@Kshitij-Katiyar would you be able to update your test server that we previously used with this branch? |
@matthewbirtch Updated the server with the build, Please test and let me know if you face any problems. |
@Kshitij-Katiyar I'm not seeing any changes on the test server |
@matthewbirtch |
Oh, sorry, I thought my comment "Going with what we have for Google Calendar works" answered that already. For consistency, let's use that approach of having 15 minute increments for scheduling. If we can find a solution for a time input that allows for 15 minutes selections and the ability to type a specific time, I'd be open to it, but let's make these two plugin interactions consistent for now. |
@mickmister Matthew has suggested using the UI of |
Closing this as not planned |
Summary
Screenshots:
Modal:
Announcement:
Reminder:
What to test:
Steps to reproduce
/zoom schedule
command.Ticket Link
Fixes #82