Skip to content

fix: Embed: data-cal-origin not used by modal(which is used by both element click popup and floating button popup)#12075

Merged
PeerRich merged 1 commit intomainfrom
fix/add-missing-cal-origin-support-modal
Nov 2, 2023
Merged

fix: Embed: data-cal-origin not used by modal(which is used by both element click popup and floating button popup)#12075
PeerRich merged 1 commit intomainfrom
fix/add-missing-cal-origin-support-modal

Conversation

@hariombalhara
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara commented Oct 25, 2023

What does this PR do?

Fixes #12073

Type of change

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

How should this be tested?

Set data-cal-origin on the element click popup button and see that the cal link is still being opened from default app.cal.local:3000 and not the provided value

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

vercel Bot commented Oct 25, 2023

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

Name Status Preview Comments Updated (UTC)
ai ❌ Failed (Inspect) Oct 25, 2023 5:40am
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 25, 2023 5:40am
5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
api ⬜️ Ignored (Inspect) Visit Preview Oct 25, 2023 5:40am
cal ⬜️ Ignored (Inspect) Visit Preview Oct 25, 2023 5:40am
cal-demo ⬜️ Ignored (Inspect) Visit Preview Oct 25, 2023 5:40am
qa ⬜️ Ignored (Inspect) Oct 25, 2023 5:40am
ui ⬜️ Ignored (Inspect) Visit Preview Oct 25, 2023 5:40am

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 25, 2023

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link to collect XP and win prizes!

@zomars zomars added the core area: core, team members only label Oct 25, 2023
calLink: string;
queryObject?: PrefillAndIframeAttrsConfig & { guest?: string | string[] };
calOrigin?: string;
calOrigin: string | null;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it required so that it's never missed

const iframe = this.cal.createIframe({
calLink,
queryObject: withColorScheme(Cal.getQueryObject(config), containerEl),
calOrigin: calConfig.calOrigin,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though createIframe fallbacks to it, but it's better to be explcit.

iframe = this.cal.createIframe({
calLink,
queryObject,
calOrigin: calOrigin || null,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what actually fixes the bug.

@github-actions
Copy link
Copy Markdown
Contributor

📦 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! 🙌

@hariombalhara hariombalhara changed the title Add calOrigin fix: Embed - data-cal-origin not used by modal(which is used by both element click popup and floating button popup) Oct 25, 2023
@hariombalhara hariombalhara marked this pull request as ready for review October 25, 2023 05:38
@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Oct 25, 2023

Current Playwright Test Results Summary

✅ 208 Passing - ⚠️ 7 Flaky

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

(Last updated on 10/25/2023 05:41:29am UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: bfe32dc

Started: 10/25/2023 05:38:09am UTC

⚠️ Flakes

📄   apps/web/playwright/event-types.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
Event Types tests user has at least 2 events
Retry 1Initial Attempt
0% (0) 0 / 283 runs
failed over last 7 days
0.35% (1) 1 / 283 run
flaked over last 7 days
Event Types tests user can add new event type
Retry 1Initial Attempt
0% (0) 0 / 283 runs
failed over last 7 days
1.06% (3) 3 / 283 runs
flaked over last 7 days

📄   apps/web/playwright/login.2fa.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
2FA Tests should allow a user to enable 2FA and login using 2FA
Retry 1Initial Attempt
0% (0) 0 / 273 runs
failed over last 7 days
29.30% (80) 80 / 273 runs
flaked over last 7 days

📄   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 open embed iframe on click - Configured with light theme
Retry 1Initial Attempt
0% (0) 0 / 282 runs
failed over last 7 days
45.74% (129) 129 / 282 runs
flaked over last 7 days
Popup Tests should be able to reschedule
Retry 1Initial Attempt
6.76% (19) 19 / 281 runs
failed over last 7 days
90.75% (255) 255 / 281 runs
flaked over last 7 days

📄   apps/web/playwright/profile.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Teams Profile page is loaded for users in Organization
Retry 1Initial Attempt
0% (0) 0 / 274 runs
failed over last 7 days
33.21% (91) 91 / 274 runs
flaked over last 7 days

📄   apps/web/playwright/webhook.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
FORM_SUBMITTED on submitting team form, triggers team webhook
Retry 1Initial Attempt
0% (0) 0 / 276 runs
failed over last 7 days
11.59% (32) 32 / 276 runs
flaked over last 7 days

View Detailed Build Results


@hariombalhara hariombalhara requested a review from a team October 30, 2023 04:29
@hariombalhara hariombalhara changed the title fix: Embed - data-cal-origin not used by modal(which is used by both element click popup and floating button popup) fix: Embed: data-cal-origin not used by modal(which is used by both element click popup and floating button popup) Oct 30, 2023
@PeerRich PeerRich merged commit f044c2d into main Nov 2, 2023
@PeerRich PeerRich deleted the fix/add-missing-cal-origin-support-modal branch November 2, 2023 19:40
zomars pushed a commit that referenced this pull request Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CAL-2644] Not able to configure origin for element click popup with React snippet.

3 participants