Embed form flickering bug #658
Conversation
WalkthroughThe pull request focuses on refining the dark mode functionality across the client-side codebase. The changes primarily involve updating how dark mode is applied by consistently targeting the document's root element ( Changes
Possibly related PRs
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)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
client/lib/forms/public-page.js (1)
86-86: Graceful Fallback for Dark Mode Check
The nullish coalescing (??) expression is well-chosen. It ensures that ifelem.valueis not defined, the logic still correctly falls back todocument.documentElement.Consider adding an early return or additional guards if
finalElementis not present, to safeguard against uninitialized states in certain SSR or hydration edge cases.client/pages/forms/[slug]/index.vue (1)
162-166: Duplicate Visibility Logic in onMounted
The same code block removing the 'hidden' class occurs again in onMounted. While it clarifies the logic flow, ensure you truly need to call it both times. If not, consider consolidating or extracting into a shared routine to avoid potential confusion or duplication.- nextTick(() => { - document.documentElement.classList.remove('hidden') - }) ... - nextTick(() => { - document.documentElement.classList.remove('hidden') - }) + function removeHiddenClass() { + nextTick(() => document.documentElement.classList.remove('hidden')) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/lib/forms/public-page.js(5 hunks)client/pages/forms/[slug]/index.vue(4 hunks)
🔇 Additional comments (9)
client/lib/forms/public-page.js (4)
1-1: Use ofdocument.documentElementfor Dark Mode is Well-Advised
Shifting the dark mode parent fromdocument.bodytodocument.documentElementensures the styling is applied across the entire page rather than just the body element. This helps avoid partial or inconsistent styling in various edge cases.
10-10: EnsureelemValidity Before Assigning
AssigningdarkModeNodeParenttoelem ?? document.documentElementis sound. However, ifelemis not a valid HTML element, it might introduce subtle behavior. Ensure that calls tohandleDarkModepass a valid HTML element or null.
69-69: RefinedeffectiveElemComputation
Relying ondocument.documentElementrather thandocument.bodyis consistent and more future-proof. Good alignment with theprocess.clientcheck ensures this code only runs on the client side.
133-134: Consistent Removal of 'dark' Class
Removing the 'dark' class directly fromdocument.documentElementis consistent with the refactor. This ensures a complete switch back to light mode without potential orphan 'dark' classes lingering.client/pages/forms/[slug]/index.vue (5)
139-140: Dark & Transparent Mode Handling
CallinghandleDarkModeandhandleTransparentModewith theformvalues centralizes the mode logic. This is consistent with the adjustments made inpublic-page.js.
142-145: Fixing Flicker by Removing 'hidden' Class
Your approach to remove the 'hidden' class fromdocument.documentElementafter loading addresses potential flickering issues. This ensures the content becomes visible once relevant data is loaded.
250-256: Reactive HTML Class Computation
Using a computed propertygetHtmlClassto conditionally setdarkandhiddenclasses is a clean approach that aligns well with Vue’s reactivity model. This helps reduce direct DOM manipulation.
259-261: Dynamic HTML Attributes
Applying dynamic attributes (dir,class,lang) based on the form’s properties is a good pattern, offering flexibility for i18n and theme handling.
281-281: Script Injection for iFrame Resizing
AddingiframeResizer.contentWindow.min.jsis useful if the form is hosted in an iframe. Ensure that the resource is always available or handle potential network issues gracefully.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements