feat(gcal): add Google Meet #8818
Conversation
|
Let's use this style to keep it consistent with the other fields and to make sure it has a label |
|
@nickoferrall I'm getting the following error when trying to start a meeting What could be wrong? |
|
@nickoferrall looks like it does not work with one-on-one only, I probably need to check |
packages/server/graphql/public/typeDefs/startTeamPrompt.graphql
Outdated
Show resolved
Hide resolved
packages/client/modules/userDashboard/components/GcalModal/VideoConferencing.tsx
Outdated
Show resolved
Hide resolved
|
@nickoferrall oh I know what is the issue with one on one
I'm not sure there is a quick solution for these, so I will disable schedule button for one-on-one page and will create an issue regarding this, if no objections Issue created #8820 |
I haven't been able to reproduce that. Do you see that error when you're creating the meeting? A Loom would be great
Thanks for creating an issue! |
nickoferrall
requested review from
Dschoordsch
and removed request for
Dschoordsch
@acressall this is tricky as I'm using our Menu component that we use throughout the app. I would probably need to use the MaterialUI dropdown menu instead to get the label there when opening it, which would be inconsistent with other menus in the app. The reason the datetime is like that is because I'm using the MaterialUI datepicker as it'd be too time-consuming to build it from scratch. The meeting name and email invite inputs don't have the label at the top. Do you think the current implementation would work or do you feel strongly that we need the label when it opens? |
|
Ok, I do think we need a label for accessibility. Here's a simpler version we're using for ad-hoc teams https://www.figma.com/file/Vc09AD3hxef2ypriihfK0T/Flow---ad-hoc-teams?type=design&node-id=61-3385&mode=design&t=CAK5lsq0Uvc2jl4S-4 |
@acressall could you share designs with how that would look in the gcal modal, please? If we add a label with "Add Video Conferencing", what would the default value in the dropdown menu be? If it's "Google Meet", my concern is users might think that it's already selected - see here:
The menu in the current PR has an
And in Gcal:
|
|
Tagging in Georg for a review to get eyes on the code while going through design review |
|
Ok, for now let's keep it as what you had, and I'll make a task to review the field styles in the UX backlog |
nickoferrall
requested review from
mattkrick
and removed request for
acressall and
Dschoordsch
packages/client/modules/userDashboard/components/GcalModal/GcalModal.tsx
Outdated
Show resolved
Hide resolved
packages/client/modules/userDashboard/components/GcalModal/VideoConferencing.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| const VideoConferencing = (props: Props) => { | ||
| const {videoType, handleChangeVideoType} = props | ||
| const {togglePortal, originRef, menuPortal, menuProps} = useMenu(MenuPosition.UPPER_CENTER) |
There was a problem hiding this comment.
-1 could you use @radix-ui/react-dropdown-menu or another radix component here? They offer SO MUCH bug free great little accessibility features that this old one doesn't. Soon I wanna replace all my crummy menu/modal hacks with radix!
There was a problem hiding this comment.
I started using Radix but refactored back to the old pattern because it wasn't working well with MUI - see: #8711. I'm running into a similar issue here with Radix where the parent closes when the menu closes, and the menu items aren't clickable.
Once #8711 is resolved, I'd like to refactor the gcal modal away from the old pattern to Radix.
There was a problem hiding this comment.
Hey @mattkrick, just following up to see if this is good to merge?
There was a problem hiding this comment.
Hey @mattkrick, bumping this
There was a problem hiding this comment.
merging now, it'll make the ship this week!
This was one-on-one meeting issue |
Fix #8764
Demo: https://www.loom.com/share/b50eda8bde6e43168b64c6763d2e2a9d
To test