fix: Embed booker top margin embed by hariombalhara · Pull Request #10357 · calcom/cal.diy

hariombalhara · 2023-07-25T08:23:39Z

What does this PR do?

Removes top margin again from embed which got added after the merge of fix: [CAL-2040] add spinner when bookerState is loading #9873. It's a weird side effect of that change.
Now, top margin is removed in a better way, mt-0 should haven't worked already(but it was working somehow) because the margin was coming due to flex align-items center and not through some explicitly added margin.
Also, fixes the failing embed test in main(which was in that PR but the PR got merged because the embed tests doesn't block the merge). PR would follow soon to make embed tests required.

Type of change

Bug fix (non-breaking change which fixes an issue)

How should this be tested?

Simply go to https://localhost:3000/free/30min/embed and notice that booker is centered vertically. With this fix, it would be back to top.

Mandatory Tasks

Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Checklist

I haven't added tests that prove my fix is effective or that my feature works

vercel · 2023-07-25T08:23:46Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
api	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 25, 2023 9:54am
cal-demo	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 25, 2023 9:54am
dev	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 25, 2023 9:54am

3 Ignored Deployments

Name	Status	Preview	Updated (UTC)
cal	⬜️ Ignored (Inspect)		Jul 25, 2023 9:54am
qa	⬜️ Ignored (Inspect)		Jul 25, 2023 9:54am
ui	⬜️ Ignored (Inspect)	Visit Preview	Jul 25, 2023 9:54am

github-actions · 2023-07-25T08:23:53Z

Thank you for following the naming conventions! 🙏

hariombalhara · 2023-07-25T08:33:40Z

            isEmbed && layout === BookerLayouts.MONTH_VIEW && "border-booker sm:border-booker-width",
-            !isEmbed && layout === BookerLayouts.MONTH_VIEW && "border-subtle",
-            // We don't want any margins for Embed. Any margin needed should be added by Embed user.
-            layout === BookerLayouts.MONTH_VIEW && isEmbed && "mt-0"


Removing the margin in the correct way now. It was working accidentally somehow but after an unrelated fix of returning null from the component in a case, it stopped working.

github-actions · 2023-07-25T08:40:25Z

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

alwaysmeticulous · 2023-07-25T08:40:40Z

🤖 Meticulous spotted visual differences in 32 of 230 screens tested: view and approve differences detected.

_{Last updated for commit eeb97ab. This comment will update as new commits are pushed.}

deploysentinel · 2023-07-25T08:41:29Z

Current Playwright Test Results Summary

✅ 97 Passing - ⚠️ 2 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 07/25/2023 09:52:31am UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: eeb97ab

Started: 07/25/2023 09:50:21am UTC

⚠️ Flakes

📄 packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 2 Flakes

Top 1 Common Error Messages

`null`	2 Test Cases Affected

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Popup Tests should be able to reschedule Retry 2 • Retry 1 • Initial Attempt	1.22% (2) 2 / 164 runs failed over last 7 days	98.17% (161) 161 / 164 runs flaked over last 7 days
Popup Tests should open embed iframe on click - Configured with light theme Retry 1 • Initial Attempt	0% (0) 0 / 164 runs failed over last 7 days	4.27% (7) 7 / 164 runs flaked over last 7 days

View Detailed Build Results

hariombalhara · 2023-07-25T09:58:37Z

+            const mainElBoundingRect = mainElement.getBoundingClientRect();
+            resolve(mainElBoundingRect.top);
+          } else {
+            setTimeout(tryGettingBoundingRect, 500);


With the recent change in Booker, main element isn't SSRd. SSR has null element rendered. It shows on client and thus we need to wait for the element to be available on client now.

hariombalhara added the 📉 regressing This used to work. Now it doesn't anymore. label Jul 25, 2023

hariombalhara changed the title ~~Fix booker top margin embed~~ fix: Embed booker top margin embed Jul 25, 2023

zomars added the core area: core, team members only label Jul 25, 2023

hariombalhara requested a review from a team July 25, 2023 08:25

sean-brydon previously approved these changes Jul 25, 2023

View reviewed changes

sean-brydon enabled auto-merge (squash) July 25, 2023 08:28

vercel Bot had a problem deploying to Preview – cal-demo July 25, 2023 08:30 Failure

Fix booker top margin embed

4bdf9f1

hariombalhara dismissed sean-brydon’s stale review via 4bdf9f1 July 25, 2023 08:31

hariombalhara force-pushed the fix/embed-margin branch from 5af1386 to 4bdf9f1 Compare July 25, 2023 08:31

hariombalhara commented Jul 25, 2023

View reviewed changes

vercel Bot deployed to Preview – ui July 25, 2023 08:36 View deployment

vercel Bot deployed to Preview – cal-demo July 25, 2023 08:37 View deployment

vercel Bot deployed to Preview – api July 25, 2023 08:39 View deployment

vercel Bot deployed to Preview – dev July 25, 2023 08:40 View deployment

hariombalhara mentioned this pull request Jul 25, 2023

chore: make Embed and AppStore Tests required #10358

Merged

vercel Bot had a problem deploying to Preview – dev July 25, 2023 09:42 Failure

Fix test

eeb97ab

hariombalhara force-pushed the fix/embed-margin branch from f346c13 to eeb97ab Compare July 25, 2023 09:42

vercel Bot deployed to Preview – dev July 25, 2023 09:47 View deployment

vercel Bot deployed to Preview – api July 25, 2023 09:52 View deployment

vercel Bot deployed to Preview – cal-demo July 25, 2023 09:54 View deployment

hariombalhara commented Jul 25, 2023

View reviewed changes

sean-brydon approved these changes Jul 25, 2023

View reviewed changes

sean-brydon merged commit 47739a7 into main Jul 25, 2023

sean-brydon deleted the fix/embed-margin branch July 25, 2023 09:58

joeauyeung pushed a commit that referenced this pull request Jul 26, 2023

Fix booker top margin embed (#10357)

c9c902f

sean-brydon pushed a commit that referenced this pull request Jul 31, 2023

Fix booker top margin embed (#10357)

daf3aa0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Embed booker top margin embed#10357

fix: Embed booker top margin embed#10357
sean-brydon merged 2 commits into
mainfrom
fix/embed-margin

hariombalhara commented Jul 25, 2023 •

edited

Loading

Uh oh!

vercel Bot commented Jul 25, 2023 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 25, 2023 •

edited

Loading

Uh oh!

hariombalhara Jul 25, 2023

Uh oh!

github-actions Bot commented Jul 25, 2023 •

edited

Loading

Uh oh!

alwaysmeticulous Bot commented Jul 25, 2023 •

edited

Loading

Uh oh!

deploysentinel Bot commented Jul 25, 2023 •

edited

Loading

Top 1 Common Error Messages

Test Case Results

Uh oh!

hariombalhara Jul 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

hariombalhara commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Type of change

How should this be tested?

Mandatory Tasks

Checklist

Uh oh!

vercel Bot commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hariombalhara Jul 25, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Next.js Bundle Analysis for @calcom/web

Uh oh!

alwaysmeticulous Bot commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deploysentinel Bot commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current Playwright Test Results Summary

⚠️ Flakes

Top 1 Common Error Messages

Test Case Results

Uh oh!

hariombalhara Jul 25, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hariombalhara commented Jul 25, 2023 •

edited

Loading

vercel Bot commented Jul 25, 2023 •

edited

Loading

github-actions Bot commented Jul 25, 2023 •

edited

Loading

github-actions Bot commented Jul 25, 2023 •

edited

Loading

alwaysmeticulous Bot commented Jul 25, 2023 •

edited

Loading

deploysentinel Bot commented Jul 25, 2023 •

edited

Loading