cal.com
cal.com copied to clipboard
fix: embed hides floating button
What does this PR do?
- hides embed floating button on click to prevent it from overlapping on Modal content
Fixes #8851
https://github.com/calcom/cal.com/assets/56112399/02362901-8a5b-478b-9278-b08323c49549
Environment: Staging(main branch) / Production
Type of change
- Bug fix (non-breaking change which fixes an issue)
How should this be tested?
- [x] Test A
- [x] Test B
Checklist
- I haven't added tests that prove my fix is effective or that my feature works
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| ui | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Aug 1, 2023 8:45am |
@riteshsp2000 is attempting to deploy a commit to the cal Team on Vercel.
A member of the Team first needs to authorize it.
📦 Next.js Bundle Analysis for @calcom/web
This analysis was generated by the Next.js Bundle Analysis action. 🤖
Eight 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] |
396.53 KB |
550.62 KB | 157.32% (🟢 -0.45%) |
/[user]/[type]/embed |
396.56 KB |
550.64 KB | 157.33% (🟢 -0.46%) |
/d/[link]/[slug] |
396.54 KB |
550.63 KB | 157.32% (🟢 -0.45%) |
/org/[orgSlug]/[user]/[type] |
396.68 KB |
550.77 KB | 157.36% (🟢 -0.45%) |
/org/[orgSlug]/[user]/[type]/embed |
396.71 KB |
550.79 KB | 157.37% (🟢 -0.45%) |
/org/[orgSlug]/team/[slug]/[type] |
396.57 KB |
550.66 KB | 157.33% (🟢 -0.45%) |
/team/[slug]/[type] |
396.54 KB |
550.63 KB | 157.32% (🟢 -0.45%) |
/team/[slug]/[type]/embed |
396.57 KB |
550.66 KB | 157.33% (🟢 -0.45%) |
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 "+/-
can you uncommit yarn.lock please? 🙏
@riteshsp2000 Can you add proper tests that you did under 'How should this be tested?'
@riteshsp2000 Can you add proper tests that you did under 'How should this be tested?'
@hariombalhara haven't added the required unit tests. How about I write a unit test that checks if the button is hidden or not after the modal is opened and vice a versa
You can add a list of manual tests there instead of mentioning just Test A, Test B. There are e2e tests for embed. But there are no tests for floating popup as of now. You can add that in a separate PR if you would like https://github.com/calcom/cal.com/blob/main/packages/embeds/embed-core/playwright/tests/action-based.e2e.ts#L104
You can add a list of manual tests there instead of mentioning just Test A, Test B. There are e2e tests for embed. But there are no tests for floating popup as of now. You can add that in a separate PR if you would like https://github.com/calcom/cal.com/blob/main/packages/embeds/embed-core/playwright/tests/action-based.e2e.ts#L104
@hariombalhara I have been trying to run the embed tests specifically but got an error that playwright.config.ts is not found. I checked the test command and the config file mentioned here doesn't actually exist. What should I be doing now?
https://github.com/calcom/cal.com/blob/8ad513e4ece852128a0bff7a36eb862af495ca33/packages/embeds/embed-core/package.json#L29
@riteshsp2000 Ah, sorry for the trouble. There was a refactor some time back and the command isn't updated.
You need to run yarn e2e:embed from the root of the repo to run embed-core tests.
@riteshsp2000 Also, I am okay to merge without automation tests. They can be done in a different PR. I just need you to list down the manual testing that you have done in the PR.
@riteshsp2000 Also, I am okay to merge without automation tests. They can be done in a different PR. I just need you to list down the manual testing that you have done in the PR.
@hariombalhara Sorry for the delay, was traveling for the last few days. Have updated the PR description with all sorts of tests performed by me.
This PR is being marked as stale due to inactivity.
Thank you for following the naming conventions! 🙏
@riteshsp2000 I guess you are involved somewhere. I would take over this PR as it's tagged for this milestone.