Skip to content
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

Closed
wants to merge 33 commits into from

Conversation

raghavaggarwal2308
Copy link
Contributor

Summary

  1. Slash command to open the schedule meeting modal.
  2. Posting meeting announcements in the channel once the meeting is created.
  3. Posting meeting reminders in the channel when the meeting is started.

Screenshots:

Modal:
image

Announcement:
image

Reminder:
image

What to test:

  1. Schedule meeting modal is working properly
  2. Announcements are properly posted in channel
  3. Meeting reminders are properly sent.
  4. The user is getting a connect link on running the command if he/she is not connected.
  5. Meeting time is set to a 30 min increment from current time by default.
  6. Error handling on different fields.

Steps to reproduce

  1. Run the /zoom schedule command.

Ticket Link

Fixes #82

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 raghavaggarwal2308 self-assigned this Mar 15, 2024
@raghavaggarwal2308 raghavaggarwal2308 marked this pull request as draft March 15, 2024 13:22
@matthewbirtch
Copy link

matthewbirtch commented Mar 16, 2024

@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.

@raghavaggarwal2308 raghavaggarwal2308 marked this pull request as ready for review April 23, 2024 11:27
@raghavaggarwal2308
Copy link
Contributor Author

@mickmister @matthewbirtch This PR is ready for review

@raghavaggarwal2308 raghavaggarwal2308 changed the title WIP - [GH-82] Added slash command to schedule a meeting on Zoom [GH-82] Added slash command to schedule a meeting on Zoom Apr 29, 2024
@raghavaggarwal2308 raghavaggarwal2308 added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester 1: UX Review Requires review by a UX Designer and removed Work In Progress Not yet ready for review labels Apr 29, 2024
@mickmister
Copy link
Contributor

@raghavaggarwal2308 Are you able to have this on a cloud server to be reviewed by UX?

@matthewbirtch
Copy link

@raghavaggarwal2308 Are you able to have this on a cloud server to be reviewed by UX?

This would be helpful. Thanks!

@Kshitij-Katiyar
Copy link
Contributor

@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

Copy link

@matthewbirtch matthewbirtch left a 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.

  1. 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.

  2. 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)

    image image
  3. For the date field, can we update the calendar icon to be the same one used in the do not disturb custom time modal?
    image

  4. 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.

  5. Can you adjust the cancel button in the footer to change the class btn-link to btn-tertiary?

  6. 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.

  7. 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:
    image

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.

@Kshitij-Katiyar
Copy link
Contributor

Kshitij-Katiyar commented Jun 7, 2024

@raghavaggarwal2308 thanks for setting up the test server. I spent some time reviewing and have some changes that I'd like to request.

  1. 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.
  2. 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)
    image
    image
  3. For the date field, can we update the calendar icon to be the same one used in the do not disturb custom time modal?
    image
  4. 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.
  5. Can you adjust the cancel button in the footer to change the class btn-link to btn-tertiary?
  6. 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.
  7. 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:
    image

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
Screenshot from 2024-06-07 12-03-19

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
Copy link

@matthewbirtch Screenshot from 2024-06-07 12-03-19

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:
image

For this reason and to be consistent with google calendar, I'd say we go with separate date and time picker

@Kshitij-Katiyar
Copy link
Contributor

Kshitij-Katiyar commented Jun 11, 2024

@matthewbirtch Screenshot from 2024-06-07 12-03-19
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: image

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
Screenshot from 2024-06-11 15-28-23

  • 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.
  • For using a UI similar to mattermost DND modal, we will need to use 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.

@matthewbirtch
Copy link

  • 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.

@Kshitij-Katiyar
Copy link
Contributor

  • 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

Screenshot from 2024-06-12 13-41-53

@matthewbirtch
Copy link

@Kshitij-Katiyar would you be able to update your test server that we previously used with this branch?

@Kshitij-Katiyar
Copy link
Contributor

@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.

@matthewbirtch
Copy link

@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
image

@Kshitij-Katiyar
Copy link
Contributor

  • 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

Screenshot from 2024-06-12 13-41-53

@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
Copy link

@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?

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.

@Kshitij-Katiyar
Copy link
Contributor

@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?

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?

@raghavaggarwal2308
Copy link
Contributor Author

raghavaggarwal2308 commented Jun 20, 2024

Closing this as not planned
cc: @mickmister

@raghavaggarwal2308 raghavaggarwal2308 modified the milestone: v1.9.0 Sep 2, 2024
@raghavaggarwal2308 raghavaggarwal2308 added this to the v1.9.0 milestone Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1: UX Review Requires review by a UX Designer 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schedule meetings
5 participants