Conversation
Signed-off-by: Simo <simo@6529.io>
📝 WalkthroughWalkthroughThe changes implement platform-specific optimizations and viewport management for mobile applications. Key updates include iOS detection in the mobile wrapper dialog, viewport meta tag preservation and management via Capacitor setup, CSS refinements for mobile dialogs with GPU-accelerated transforms, and improved input handling in the Lexical editor with better null-safety checks. Changes
Sequence DiagramssequenceDiagram
participant App as App Initialization
participant Setup as CapacitorSetup
participant Meta as Viewport Meta Tag
participant DOM as DOM/Rendering
App->>Setup: Component mounts (isCapacitor flag)
Setup->>Meta: Query or create viewport meta tag
Setup->>Meta: Store original viewport content
alt isCapacitor is true
Setup->>Meta: Set fixed viewport (no-zoom, viewport-fit=cover)
else isCapacitor is false
Setup->>Meta: Restore original viewport content
end
Meta->>DOM: Viewport configuration applied
Setup->>DOM: Apply capacitor-native class to body
DOM->>DOM: Render with platform-specific layout
Note over Setup: Cleanup restores original viewport on unmount
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/waves/CreateDropInput.tsx (1)
184-192: Guard against null before callingfocus().
The current cast can throw if the contenteditable element is missing (e.g., before mount or after unmount).🐛 Proposed fix
focus: () => { - ( - editorRef.current?.querySelector( - '[contenteditable="true"]' - ) as HTMLElement - ).focus(); + const editable = editorRef.current?.querySelector( + '[contenteditable="true"]' + ) as HTMLElement | null; + editable?.focus(); },
🧹 Nitpick comments (2)
components/mobile-wrapper-dialog/MobileWrapperDialog.tsx (2)
93-94: Minor redundancy:touchActionis also set via CSS.The
touch-action: manipulationis already applied to.mobile-wrapper-dialogelements inglobals.scss(lines 206-213). The inline style here is redundant but ensures the style is applied regardless of CSS load order.Consider removing the inline style if you're confident the CSS is always loaded:
♻️ Optional: Remove redundant inline style
<DialogPanel className={panelClassNames} - style={{ touchAction: "manipulation" }} onClick={(e) => e.stopPropagation()} >
38-38: Consider adding a fallback for in-app and older embedded browsers.The
svhunit has ~94% support in modern browsers (Chrome 108+, Firefox 101+, Safari 15.4+), but is not consistently supported in in-app browsers (Messenger, older Android webviews). For these environments, the browser won't recognize thesvhvalue. Use a progressive enhancement pattern with@supportsto safely fall back to100vhfor browsers that don't support the newer viewport units, ensuring consistent behavior across all target platforms.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/mobile-wrapper-dialog/MobileWrapperDialog.tsxcomponents/providers/CapacitorSetup.tsxcomponents/waves/CreateDropInput.tsxstyles/globals.scss
⏰ 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: Analyze (javascript-typescript)
🔇 Additional comments (10)
components/waves/CreateDropInput.tsx (3)
3-4: No review comment.Also applies to: 12-13, 38-43
157-169: Placeholder logic looks good.
232-253: ContentEditable interaction tweaks look solid.components/providers/CapacitorSetup.tsx (3)
18-46: Cleanup function may cause unintended viewport restoration.The cleanup function restores the original viewport on every effect cleanup (including when
isCapacitorchanges). Since the effect body at lines 37-38 already handles restoration whenisCapacitorbecomes false, the cleanup will immediately run again and restore the same value - this is benign but redundant.However, there's a subtle issue: if the component unmounts while
isCapacitoris false (and originalViewport was never set becauseisCapacitorstarted false), the cleanup does nothing (which is correct). But if the component mounts withisCapacitor=true, sets the viewport, then unmounts, the cleanup correctly restores. This logic appears sound.One edge case: if
isCapacitortoggles rapidly, the nullish coalescing on line 30 ensuresoriginalViewport.currentcaptures only the first observed value, which is the intended behavior.
19-19: SSR guard is appropriate.The
typeof document === "undefined"check correctly prevents DOM manipulation during server-side rendering.
21-28: Meta tag creation logic is sound.The nullish coalescing with
createElementand thenameattribute check ensure the element is only appended when newly created. Good defensive approach.styles/globals.scss (2)
206-213: Good iOS zoom prevention pattern.Setting
font-size: 16pxon form inputs prevents iOS Safari's auto-zoom behavior (triggered for inputs < 16px). Combined withtouch-action: manipulationto disable double-tap zoom, this effectively addresses common mobile zoom issues.
581-584: Formatting-only change.The transition property was reformatted to multi-line for readability. No functional change.
components/mobile-wrapper-dialog/MobileWrapperDialog.tsx (2)
46-48: iOS GPU transform exclusion is a known workaround.Disabling
transform-gpuandwill-change-transformon iOS avoids known rendering issues (flickering, z-index problems) with these properties on Safari. This is a reasonable trade-off for stability.
39-44: Height calculation change fromdvhtomin(vh, svh).This change affects dialog sizing behavior. Using
min(100vh, 100svh)will consistently use the smaller viewport height (accounting for browser chrome), whereasdvhdynamically adjusts. This prevents layout shifts when mobile browser UI appears/disappears, which aligns with the zoom fix objective.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.