Closed raghavaggarwal2308 closed 2 months ago
@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.
I think we could start simple here and only show 'join meeting' if it's 5 minutes or less before the meeting starts. Otherwise hide the button.
@mickmister is there a way to get this up and running on a test server for me to test?
@mickmister @matthewbirtch This PR is still a work in progress. I need to fix the test cases and handle an edge case of updating the post when the meeting ends. Also, need to make the change mentioned by @matthewbirtch here https://github.com/mattermost/mattermost-plugin-zoom/pull/350#issuecomment-2002008909
@mickmister @matthewbirtch This PR is still a work in progress. I need to fix the test cases and handle an edge case of updating the post when the meeting ends. Also, need to make the change mentioned by @matthewbirtch here #350 (comment)
Ok, thank you for clarifying. I'll remove myself as a reviewer for now. You can add me back in when it's ready for review
@mickmister @matthewbirtch This PR is ready for review
@raghavaggarwal2308 Are you able to have this on a cloud server to be reviewed by UX?
@raghavaggarwal2308 Are you able to have this on a cloud server to be reviewed by UX?
This would be helpful. Thanks!
@raghavaggarwal2308 Are you able to have this on a cloud server to be reviewed by UX?
@mickmister @matthewbirtch Messaged you the creds on https://hub.mattermost.com. Please check
@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.
@matthewbirtch
We will seperate the date and time picker. The duration UI is similar to the zoom UI, If you suggest, then we can add start and end times.
@matthewbirtch
We will seperate the date and time picker. The duration UI is similar to the zoom UI, If you suggest, then we can add start and end times.
Interesting, even Zoom is not consistent with how they show these fields. In the Zoom desktop app, this is what I see for scheduling a meeting:
For this reason and to be consistent with google calendar, I'd say we go with separate date and time picker
@matthewbirtch
We will seperate the date and time picker. The duration UI is similar to the zoom UI, If you suggest, then we can add start and end times.
Interesting, even Zoom is not consistent with how they show these fields. In the Zoom desktop app, this is what I see for scheduling a meeting:
For this reason and to be consistent with google calendar, I'd say we go with separate date and time picker
Here is a screenshot from google calender
MaterialUI
i.e. identical to the DND input (mattermost itself uses a custom component).@matthewbirtch Please let me know which one of the above will you prefer.
- Only concern in this time component is that it only allows you to choose a time from the pre-defined times in dropdown, which are multiple of 15 minutes.
I think that's ok for this. Going with what we have for Google Calendar works.
- Only concern in this time component is that it only allows you to choose a time from the pre-defined times in dropdown, which are multiple of 15 minutes.
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?
@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 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
- Only concern in this time component is that it only allows you to choose a time from the pre-defined times in dropdown, which are multiple of 15 minutes.
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
@matthewbirtch
I haven't started the dev on this task yet, the comment I have quoted is the blocker.
As seen in the screenshot, zoom allows you to schedule meetings with custom time but the google calendar plugin
allows to choose the time with the difference of 15 mins.
Should we continue with google calendar plugin's
UI or we should explore and use a UI that allows us to choose custom time?
@matthewbirtch I haven't started the dev on this task yet, the comment I have quoted is the blocker. As seen in the screenshot, zoom allows you to schedule meetings with custom time but the
google calendar plugin
allows to choose the time with the difference of 15 mins. Should we continue withgoogle calendar plugin's
UI or we should explore and use a UI that allows us to choose custom time?
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.
@matthewbirtch I haven't started the dev on this task yet, the comment I have quoted is the blocker. As seen in the screenshot, zoom allows you to schedule meetings with custom time but the
google calendar plugin
allows to choose the time with the difference of 15 mins. Should we continue withgoogle calendar plugin's
UI or we should explore and use a UI that allows us to choose custom time?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 google-calendar-plugin
. The date picker and other components of google-calendar-plugin
are custom made so using UI the same UI will lead to copy-pasting a lot of code from google-calendar-plugin
to zoom-plugin
. Should we proceed with the same?
Closing this as not planned cc: @mickmister
Summary
Screenshots:
Modal:![image](https://github.com/mattermost/mattermost-plugin-zoom/assets/55234496/d9f0f764-d70a-4cbf-8473-0acfbcaa5b6a)
Announcement:![image](https://github.com/mattermost/mattermost-plugin-zoom/assets/55234496/dcc574b6-1957-4ccf-9ea3-5e06f08f491e)
Reminder:![image](https://github.com/mattermost/mattermost-plugin-zoom/assets/55234496/b7dd399b-7e22-4321-a525-2778a56c1d24)
What to test:
Steps to reproduce
/zoom schedule
command.Ticket Link
Fixes #82