Skip to content

Comments

fix: prevent external CSS from breaking Cal.com embed#22778

Merged
hariombalhara merged 25 commits intocalcom:mainfrom
sahitya-chandra:cal-6163
Sep 22, 2025
Merged

fix: prevent external CSS from breaking Cal.com embed#22778
hariombalhara merged 25 commits intocalcom:mainfrom
sahitya-chandra:cal-6163

Conversation

@sahitya-chandra
Copy link
Member

@sahitya-chandra sahitya-chandra commented Jul 28, 2025

What does this PR do?

Visual Demo (For contributors especially)

A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).

Video Demo (if applicable):

  • Show screen recordings of the issue or feature.
  • Demonstrate how to reproduce the issue, the behavior before and after the change.

Image Demo (if applicable):

  • Add side-by-side screenshots of the original and updated change.
  • Highlight any significant change(s).

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

@sahitya-chandra sahitya-chandra requested a review from a team as a code owner July 28, 2025 13:19
@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Jul 28, 2025
@graphite-app graphite-app bot requested a review from a team July 28, 2025 13:19
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 28, 2025

"""

Walkthrough

The .cal-embed CSS class was updated to include display: block !important, color-scheme: unset !important, and background: transparent !important style rules. Additionally, the embed iframe initialization and the actOnColorScheme function in the embed iframe script now explicitly set the document's color scheme to the provided value or "unset" by default and set the body background to "transparent". These changes ensure the embed is not hidden by host page styles and maintain transparent backgrounds and neutral color schemes within the iframe.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Assessment against linked issues

Objective Addressed Explanation
Ensure .cal-embed is not hidden by overriding styles (add .cal-embed { display: block !important; }) (#22777, CAL-6163)
Fix iframe color-scheme issues by setting { color-scheme: unset } (#22777, CAL-6163)

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes were found.
"""

Note

🔌 MCP (Model Context Protocol) integration is now available in Early Access!

Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5062f60 and 9c82c7d.

📒 Files selected for processing (1)
  • packages/embeds/embed-core/src/embed-iframe.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/embeds/embed-core/src/embed-iframe.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sahitya-chandra sahitya-chandra marked this pull request as draft July 28, 2025 13:19
@github-actions github-actions bot added the embed area: embed, widget, react embed label Jul 28, 2025
@vercel
Copy link

vercel bot commented Jul 28, 2025

@sahitya-chandra is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@graphite-app
Copy link

graphite-app bot commented Jul 28, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (07/28/25)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add community label" took an action on this PR • (07/28/25)

1 label was added to this PR based on Keith Williams's automation.

@dosubot dosubot bot added the 🐛 bug Something isn't working label Jul 28, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔭 Outside diff range comments (1)
packages/embeds/embed-core/src/embed.css (1)

4-10: Second half of the bug-fix (iframe color-scheme) is missing

Issue #22777 / CAL-6163 also describes host CSS that applies
iframe { color-scheme: normal; }, breaking our transparent iframe.
This rule is not addressed here, so the embed can still appear with an incorrect background.

Add a targeted override:

+.cal-embed iframe {
+  /* Reset host-page rule to restore transparency */
+  color-scheme: unset !important;
+}

Without this, the PR only fixes half of the reported breakage.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9cd806f and aac57ff.

📒 Files selected for processing (1)
  • packages/embeds/embed-core/src/embed.css (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#22547
File: packages/embeds/embed-core/src/lib/eventHandlers/scrollByDistanceEventHandler.ts:11-14
Timestamp: 2025-07-16T11:46:28.759Z
Learning: In Cal.com's embed system, internal events like "__scrollByDistance" are fired by Cal.com's own code, so runtime validation of event data structure is unnecessary since TypeScript type system guarantees type safety for internal events.
packages/embeds/embed-core/src/embed.css (1)

Learnt from: hariombalhara
PR: #22547
File: packages/embeds/embed-core/src/lib/eventHandlers/scrollByDistanceEventHandler.ts:11-14
Timestamp: 2025-07-16T11:46:28.759Z
Learning: In Cal.com's embed system, internal events like "__scrollByDistance" are fired by Cal.com's own code, so runtime validation of event data structure is unnecessary since TypeScript type system guarantees type safety for internal events.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Detect changes

@sahitya-chandra sahitya-chandra marked this pull request as ready for review July 28, 2025 13:51
@anikdhabal anikdhabal requested a review from hariombalhara July 28, 2025 13:52
@anikdhabal
Copy link
Contributor

cc @hariombalhara for a quick review

embedStore.theme = window?.getEmbedTheme?.();

// Initialize with color-scheme: unset and transparent background to prevent opaque background issues
document.documentElement.style.colorScheme = "unset";
Copy link
Member

Choose a reason for hiding this comment

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

This is the cal.com app itself we don't want to force css here

Copy link
Member Author

Choose a reason for hiding this comment

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

@hariombalhara sir, I have removed the forced CSS. Please have a look !!

Comment on lines 680 to 681
document.documentElement.style.colorScheme = colorScheme || "unset";
document.body.style.background = "transparent";
Copy link
Member

Choose a reason for hiding this comment

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

I meant that we shouldn't modify anything in the iframe. Inside the iframe, cal.com code is there, we already handle that correctly.

min-height: 300px;
margin: 0 auto;
width: 100%;
display: block !important;
Copy link
Member

Choose a reason for hiding this comment

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

Can we confirm that the element that .cal-embed class always has display: block(and not flex or hidden or something else) ?

This is important because if there is some condition where that element's display becomes flex or hidden temporarily, it would stop working.

Copy link
Member

Choose a reason for hiding this comment

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

@sahitya-chandra Did you confirm this part?

Copy link
Member

Choose a reason for hiding this comment

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

Also, were you able to test it out. Please add a loom demonstrating that a webpage even after having these css props set for .cal-embed, the embed is still not affected.

@sahitya-chandra
Copy link
Member Author

sahitya-chandra commented Sep 3, 2025

@hariombalhara sir, I created a simple React app to test the embed, and then I added this external CSS in App.css file

iframe {
  display: none !important;
  border: 1px solid blue !important;
  border-radius: 5px !important;
  width: 50vw !important;
}

after adding this I test both main and this cal-6163 branch

main branch

Screencast.from.2025-09-01.00-27-01.webm

cal-6163 branch(current branch)

Screencast.from.2025-09-01.00-30-09.webm

@sahitya-chandra
Copy link
Member Author

Embed is not breaking after adding these new css but adding external css can still override the embed (but it did not break the embed)

@sahitya-chandra
Copy link
Member Author

@hariombalhara sir I have added the video proof, can you have a look

@hariombalhara
Copy link
Member

thanks, I will check soon!!

@hariombalhara hariombalhara enabled auto-merge (squash) September 8, 2025 10:16
@hariombalhara
Copy link
Member

Thanks @sahitya-chandra !! Approved

@sahitya-chandra
Copy link
Member Author

sahitya-chandra commented Sep 22, 2025

@hariombalhara sir this didn't get auto-merged yet!!

@hariombalhara
Copy link
Member

Getting it merged @sahitya-chandra !! Sorry

@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2025

E2E results are ready!

@hariombalhara hariombalhara merged commit 5463dc2 into calcom:main Sep 22, 2025
54 of 61 checks passed
@sahitya-chandra sahitya-chandra deleted the cal-6163 branch September 22, 2025 19:17
@sahitya-chandra
Copy link
Member Author

sahitya-chandra commented Sep 22, 2025

@hariombalhara thanks a lot sir, for guiding me throughout this PR...

saurabhraghuvanshii pushed a commit to saurabhraghuvanshii/cal.com that referenced this pull request Sep 24, 2025
* fix: added block display to .cal-embed

* fixed the iframe color-scheme to unset and bg transparent

* removed forced CSS

* changes reverted from iframe

* Add comments and remove background transparent as it is still not needed

---------

Co-authored-by: Anik Dhabal Babu <81948346+anikdhabal@users.noreply.github.com>
Co-authored-by: Hariom Balhara <hariombalhara@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working community Created by Linear-GitHub Sync embed area: embed, widget, react embed ready-for-e2e size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Embed: Websites override a few things on the page, breaking the embed in one way or the another

5 participants