cal.com
cal.com copied to clipboard
feat: Adapt Dialog to use Drawers on mobile
What does this PR do?
Fixes the below issue. [CAL-2978] Adapt Dialog to use Drawers on modals #13338
fixes #13338
Please refer the below video and let me know if the solution is as expected:
Screen recordings of Production build -> https://www.loom.com/share/50f6857055ec4bb0848f7d5d287fc52c?sid=2015e908-3942-4a0c-b469-9f64358d0bbd
https://www.loom.com/share/51e98373a2424d90b09cdf528af8eea3?t=51&sid=295c5ed6-5d99-408f-9c60-e7c6b0ac54ed
Type of change
- New feature (non-breaking change which adds functionality)
How should this be tested?
- Open any dialog.
- Adjust the screen width to observe the changes.
Someone is attempting to deploy a commit to the cal Team on Vercel.
A member of the Team first needs to authorize it.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.
:white_check_mark: keithwillcode
:white_check_mark: gramo37
:x: prasanna
prasanna seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.
📦 Next.js Bundle Analysis for @calcom/web
This analysis was generated by the Next.js Bundle Analysis action. 🤖
Eighty-three Pages Changed Size
The following pages changed size from the code in this PR compared to its base branch:
| Page | Size (compressed) | First Load | % of Budget (350 KB) |
|---|---|---|---|
/[user]/[type] |
272.48 KB |
460.81 KB | 131.66% (🟡 +1.85%) |
/[user]/[type]/embed |
272.48 KB |
460.82 KB | 131.66% (🟡 +1.85%) |
/apps |
287.63 KB |
475.97 KB | 135.99% (🟡 +1.89%) |
/apps/[slug] |
305.66 KB |
494 KB | 141.14% (🟡 +1.89%) |
/apps/[slug]/[...pages] |
563.92 KB |
752.25 KB | 214.93% (🟡 +1.88%) |
/apps/categories |
265.53 KB |
453.86 KB | 129.67% (🟡 +1.89%) |
/apps/categories/[category] |
269.77 KB |
458.1 KB | 130.89% (🟡 +1.88%) |
/apps/installed/[category] |
289.57 KB |
477.91 KB | 136.54% (🟡 +1.90%) |
/auth/setup |
157.59 KB |
345.92 KB | 98.83% (🟡 +1.88%) |
/availability |
443.09 KB |
631.43 KB | 180.41% (🟡 +1.88%) |
/availability/[schedule] |
369.57 KB |
557.9 KB | 159.40% (🟡 +1.89%) |
/bookings/[status] |
333.73 KB |
522.06 KB | 149.16% (🟡 +1.89%) |
/d/[link]/[slug] |
272.31 KB |
460.64 KB | 131.61% (🟡 +1.85%) |
/enterprise |
265.91 KB |
454.24 KB | 129.78% (🟡 +1.89%) |
/event-types |
564.81 KB |
753.14 KB | 215.18% (🟡 +1.86%) |
/event-types/[type] |
440.86 KB |
629.19 KB | 179.77% (🟡 +1.89%) |
/getting-started/[[...step]] |
411.03 KB |
599.36 KB | 171.25% (🟡 +1.88%) |
/insights |
484.41 KB |
672.74 KB | 192.21% (🟡 +1.89%) |
/more |
265.11 KB |
453.45 KB | 129.56% (🟡 +1.88%) |
/org/[orgSlug]/[user]/[type] |
272.67 KB |
461 KB | 131.72% (🟡 +1.85%) |
/org/[orgSlug]/[user]/[type]/embed |
272.69 KB |
461.03 KB | 131.72% (🟡 +1.85%) |
/org/[orgSlug]/instant-meeting/team/[slug]/[type] |
272.32 KB |
460.66 KB | 131.62% (🟡 +1.85%) |
/org/[orgSlug]/team/[slug]/[type] |
272.5 KB |
460.83 KB | 131.67% (🟡 +1.85%) |
/settings/admin |
271.89 KB |
460.23 KB | 131.49% (🟡 +1.89%) |
/settings/admin/apps |
285.64 KB |
473.97 KB | 135.42% (🟡 +1.89%) |
/settings/admin/apps/[category] |
285.63 KB |
473.96 KB | 135.42% (🟡 +1.88%) |
/settings/admin/flags |
275.82 KB |
464.15 KB | 132.61% (🟡 +1.89%) |
/settings/admin/impersonation |
272.19 KB |
460.52 KB | 131.58% (🟡 +1.88%) |
/settings/admin/oAuth |
283.86 KB |
472.19 KB | 134.91% (🟡 +1.89%) |
/settings/admin/oAuth/oAuthView |
98.43 KB |
286.77 KB | 81.93% (🟡 +1.88%) |
/settings/admin/orgMigrations/_OrgMigrationLayout |
266.11 KB |
454.44 KB | 129.84% (🟡 +1.89%) |
/settings/admin/orgMigrations/moveTeamToOrg |
281.47 KB |
469.81 KB | 134.23% (🟡 +1.89%) |
/settings/admin/orgMigrations/moveUserToOrg |
300.15 KB |
488.48 KB | 139.57% (🟡 +1.88%) |
/settings/admin/orgMigrations/removeTeamFromOrg |
281.24 KB |
469.58 KB | 134.16% (🟡 +1.89%) |
/settings/admin/orgMigrations/removeUserFromOrg |
281.26 KB |
469.59 KB | 134.17% (🟡 +1.89%) |
/settings/admin/organizations |
273.92 KB |
462.25 KB | 132.07% (🟡 +1.89%) |
/settings/admin/organizations/[id]/edit |
272.43 KB |
460.76 KB | 131.65% (🟡 +1.88%) |
/settings/admin/users |
274.59 KB |
462.92 KB | 132.26% (🟡 +1.89%) |
/settings/admin/users/[id]/edit |
374.11 KB |
562.44 KB | 160.70% (🟡 +1.88%) |
/settings/admin/users/add |
373.84 KB |
562.17 KB | 160.62% (🟡 +1.89%) |
/settings/billing |
272.23 KB |
460.57 KB | 131.59% (🟡 +1.89%) |
/settings/developer/api-keys |
276.43 KB |
464.76 KB | 132.79% (🟡 +1.89%) |
/settings/developer/webhooks |
276.35 KB |
464.69 KB | 132.77% (🟡 +1.88%) |
/settings/developer/webhooks/[id] |
277.38 KB |
465.71 KB | 133.06% (🟡 +1.89%) |
/settings/developer/webhooks/new |
277.43 KB |
465.76 KB | 133.08% (🟡 +1.88%) |
/settings/my-account/appearance |
295.85 KB |
484.18 KB | 138.34% (🟡 +1.88%) |
/settings/my-account/calendars |
283.45 KB |
471.79 KB | 134.80% (🟡 +1.88%) |
/settings/my-account/conferencing |
284.2 KB |
472.54 KB | 135.01% (🟡 +1.89%) |
/settings/my-account/general |
360.2 KB |
548.53 KB | 156.72% (🟡 +1.89%) |
/settings/my-account/out-of-office |
276.22 KB |
464.55 KB | 132.73% (🟡 +1.89%) |
/settings/my-account/profile |
409.08 KB |
597.42 KB | 170.69% (🟡 +1.89%) |
/settings/organizations/[id]/about |
158.44 KB |
346.77 KB | 99.08% (🟡 +1.87%) |
/settings/organizations/[id]/add-teams |
158.49 KB |
346.82 KB | 99.09% (🟡 +1.88%) |
/settings/organizations/[id]/onboard-members |
178.82 KB |
367.15 KB | 104.90% (🟡 +1.89%) |
/settings/organizations/[id]/set-password |
158.42 KB |
346.75 KB | 99.07% (🟡 +1.88%) |
/settings/organizations/appearance |
295.36 KB |
483.69 KB | 138.20% (🟡 +1.88%) |
/settings/organizations/billing |
272.27 KB |
460.6 KB | 131.60% (🟡 +1.89%) |
/settings/organizations/general |
352.74 KB |
541.07 KB | 154.59% (🟡 +1.89%) |
/settings/organizations/members |
445.46 KB |
633.8 KB | 181.09% (🟡 +1.88%) |
/settings/organizations/new |
158.44 KB |
346.78 KB | 99.08% (🟡 +1.88%) |
/settings/organizations/profile |
406.1 KB |
594.43 KB | 169.84% (🟡 +1.88%) |
/settings/organizations/teams/other |
272.93 KB |
461.27 KB | 131.79% (🟡 +1.89%) |
/settings/organizations/teams/other/[id]/appearance |
284.44 KB |
472.78 KB | 135.08% (🟡 +1.88%) |
/settings/organizations/teams/other/[id]/members |
279.39 KB |
467.73 KB | 133.64% (🟡 +1.88%) |
/settings/organizations/teams/other/[id]/profile |
477.67 KB |
666 KB | 190.29% (🟡 +1.88%) |
/settings/security/impersonation |
277.34 KB |
465.67 KB | 133.05% (🟡 +1.88%) |
/settings/security/password |
286.41 KB |
474.74 KB | 135.64% (🟡 +1.89%) |
/settings/security/sso |
282.38 KB |
470.71 KB | 134.49% (🟡 +1.89%) |
/settings/security/two-factor-auth |
280.91 KB |
469.24 KB | 134.07% (🟡 +1.88%) |
/settings/teams |
271.63 KB |
459.97 KB | 131.42% (🟡 +1.89%) |
/settings/teams/[id]/appearance |
284.43 KB |
472.76 KB | 135.08% (🟡 +1.88%) |
/settings/teams/[id]/billing |
272.27 KB |
460.6 KB | 131.60% (🟡 +1.89%) |
/settings/teams/[id]/members |
391.22 KB |
579.55 KB | 165.59% (🟡 +1.88%) |
/settings/teams/[id]/onboard-members |
178.01 KB |
366.34 KB | 104.67% (🟡 +1.89%) |
/settings/teams/[id]/profile |
478.49 KB |
666.82 KB | 190.52% (🟡 +1.89%) |
/settings/teams/[id]/sso |
282.9 KB |
471.23 KB | 134.64% (🟡 +1.88%) |
/settings/teams/new |
205.66 KB |
393.99 KB | 112.57% (🟡 +1.89%) |
/team/[slug]/[type] |
272.47 KB |
460.8 KB | 131.66% (🟡 +1.85%) |
/team/[slug]/[type]/embed |
272.5 KB |
460.83 KB | 131.67% (🟡 +1.85%) |
/teams |
265.33 KB |
453.66 KB | 129.62% (🟡 +1.88%) |
/upgrade |
265.55 KB |
453.89 KB | 129.68% (🟡 +1.89%) |
/workflows |
296.8 KB |
485.13 KB | 138.61% (🟡 +1.88%) |
/workflows/[workflow] |
417.49 KB |
605.82 KB | 173.09% (🟡 +1.89%) |
Details
Only the gzipped size is provided here based on an expert tip.
First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.
Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis
The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/-
@ciaranha - can you checkout preview for this PR - code looks good on my end.
@ciaranha - can you checkout preview for this PR - code looks good on my end.
These look great. Only slight nit is id probably reduce the padding inside the bottom sheet on mobile by at least 8px.
Hi @keithwillcode,
The test cases were failing because of below 2 reasons:
- There was no window.matchMedia in the testing environment (TypeError: window.matchMedia is not a function)
- DialogOverlay must be utilized within Dialog.
I have made changes for that, and now the tests for the Dialog component are passing smoothly.
I have also added the change mentioned by @ciaranha. Reduced the padding for Footer in the mobile view.
According to this example (https://codesandbox.io/p/sandbox/drawer-with-scale-forked-73f8jw?file=%2Fapp%2Fmy-drawer.tsx%3A1%2C1), for a scrollable drawer we need to add the children another div. Now the scrolling works fine in mobile. And I have removed the bottom rounded corners. Thanks for noticing @CarinaWolli.
First 2 issues (Unwanted padding, Dropdown cut-off) have been fixed and I have pushed the changes for them.
However, for the third one, I will have to make changes in EditLocationDialog.tsx. As the heading is centered in the Dialog as well.
For the last issue, when I passed "setEnableModalOpen" instead of "() => setEnableModalOpen(!enableModalOpen)" to onOpenChange prop the problem was solved (for both Desktop and mobile view). But I am thinking if it is a correct solution or not.
Please let me know if I should push the last 2 changes.
I found a similar case in conferencing.tsx (DisconnectIntegrationModal), which could be fixed using a similar approach. I think there might me some issue in the vaul library which is opening and closing the Drawer again and again in some Modals. To solve this, will have to check all Dialogs and solve the errors in a similar manner or add a prop to Dialog named "useDialogForMobile". So, when it is set to true the Dialog will be used instead of drawer for such cases.
Please let me know if I should push the last 2 changes.
Sounds good to me, let's push them
I found a similar case in conferencing.tsx (DisconnectIntegrationModal), which could be fixed using a similar approach. I think there might me some issue in the vaul library which is opening and closing the Drawer again and again in some Modals. To solve this, will have to check all Dialogs and solve the errors in a similar manner or add a prop to Dialog named "useDialogForMobile". So, when it is set to true the Dialog will be used instead of drawer for such cases.
We will need to go and test all modals to avoid breaking anything. I would definitely go for fixing the issues and using Drawers everywhere, if the issues are more complex we could temporary disable the Drawer for specific modals but I would like to avoid that if possible
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@types/[email protected], npm/@types/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected]
I have checked as many problems in the Dialogs as possible. Some of them have been fixed but can't find a solution for some. Therefore, I have added the prop useDialogForMobile to Dialog.tsx and used it wherever required.
Problems
- CSS problem in EditLocationDialog.tsx. Solution -> Aligned the title to the left (CSS problem)
- TwoFactorAuthentication. Solution -> Minor change in onOpenChange prop
- Removing Installed Apps had a problem while opening the Drawer. Solution -> Minor change in open and onOpenChange prop
- Problem in Create customized event names (Steps to reproduce: edit event type -> advanced tab). Solution -> Used the useDialogForMobile prop
- Reschedule booking not opening drawer after selecting a time slot. Solution -> Used the useDialogForMobile prop
Please let me know if I should push the last 2 changes.
Sounds good to me, let's push them
I found a similar case in conferencing.tsx (DisconnectIntegrationModal), which could be fixed using a similar approach. I think there might me some issue in the vaul library which is opening and closing the Drawer again and again in some Modals. To solve this, will have to check all Dialogs and solve the errors in a similar manner or add a prop to Dialog named "useDialogForMobile". So, when it is set to true the Dialog will be used instead of drawer for such cases.
We will need to go and test all modals to avoid breaking anything. I would definitely go for fixing the issues and using Drawers everywhere, if the issues are more complex we could temporary disable the Drawer for specific modals but I would like to avoid that if possible
Moving this to draft because there are still some issues. Thank you for the PR, we will come back to it when we have the bandwidth to carefully test this so we can make sure to not break anything 🙏
@anikdhabal or @Amit91848 one of you able to get this tested and pushed through? 🙏🏼
@anikdhabal or @Amit91848 one of you able to get this tested and pushed through? 🙏🏼
sure 🙏🏼
Hey @gramo37 could you pls fix the conflict?
I have resolved the merge conflict. Since many changes were required, I created a new pull request for it. Please refer to PR https://github.com/calcom/cal.com/pull/15632