-
Notifications
You must be signed in to change notification settings - Fork 39
feat: clean ui for iframes #268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes introduce iframe detection to the Docusaurus site by adding a new JavaScript file and updating the site's configuration to load it. Corresponding CSS rules are added to modify the site's layout and hide certain elements when the site is viewed inside an iframe, based on a custom data attribute. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant iframe-detector.js
participant HTML Document
Note over Browser: DOMContentLoaded event
Browser->>iframe-detector.js: Load and execute
iframe-detector.js->>iframe-detector.js: isInIframe()
iframe-detector.js->>HTML Document: Set data-iframe="true" if in iframe
HTML Document->>Browser: CSS applies styles based on data-iframe attribute
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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
🧹 Nitpick comments (2)
static/js/iframe-detector.js (1)
1-4
: Refine header comment for accuracy
The top comment says "sets a CSS variable" but the script actually toggles adata-iframe
attribute on the root element. Consider updating the description to reflect its behavior precisely.src/css/custom.css (1)
74-93
: Optional: Consolidate repetitive selectors
To improve maintainability, you could group the attribute prefix into a single block and leverage a CSS custom mixin or variable. For example::root[data-iframe="true"] { --iframe-hide-selectors: .navbar, header, footer, .theme-doc-sidebar-container, .theme-doc-toc-desktop, .theme-doc-toc-mobile, nav.pagination-nav, div[role="complementary"], aside; --iframe-main-container: main[class^="docMainContainer_"]; } :root[data-iframe="true"] var(--iframe-hide-selectors) { display: none !important; } :root[data-iframe="true"] var(--iframe-main-container) { max-width: 100% !important; width: 100% !important; } :root[data-iframe="true"] var(--iframe-main-container) > div { padding-top: 1rem !important; }This reduces duplication and centralizes the attribute selector.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docusaurus.config.js
(1 hunks)src/css/custom.css
(1 hunks)static/js/iframe-detector.js
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
static/js/iframe-detector.js (2)
7-15
: Iframe detection logic is solid
The use ofwindow.self !== window.top
combined with a try/catch for cross-origin frames correctly determines when the page is embedded.
17-24
: Attribute toggling is clear and idempotent
applyIframeMode()
cleanly adds or removesdata-iframe="true"
on the<html>
element, enabling scoped CSS for iframe contexts.docusaurus.config.js (1)
42-45
: Script inclusion looks correct
Adding{ src: "/js/iframe-detector.js", async: false }
ensures the detector runs synchronously. Verify thatstatic/js/iframe-detector.js
is served at/js/iframe-detector.js
in production (Docusaurus should mapstatic/
to the site root).src/css/custom.css (2)
74-84
: Iframe-specific hiding is effective
The selectors underhtml[data-iframe="true"]
correctly target and hide the navbar, sidebars, footer, etc., in iframe mode.
86-93
: Layout expansion for iframe contexts is correct
Setting the main container towidth: 100%
and adding top padding ensures full-bleed content inside an iframe.
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Before Submitting This PR, Please Ensure You Have Completed The Following:
Summary by CodeRabbit