CC-101: Improve Component Library Mobile Support#30
Conversation
Resolves CC-101
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request updates CSS layouts across six UI component modules to account for device safe-area insets. Changes include incorporating 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Drawer/Drawer.module.scss`:
- Line 248: The global padding line that adds env(safe-area-inset-top) is
applied to all drawers; change it to conditional padding based on the drawer
anchor using the data attribute data-swipe-direction (e.g., apply
env(safe-area-inset-top) only when data-swipe-direction="top",
env(safe-area-inset-bottom) only when data-swipe-direction="bottom", and
similarly env(safe-area-inset-left/right) for left/right) so header/content
spacing for Drawer components is correct; update both occurrences of the padding
rule to use direction-scoped selectors that add the appropriate safe-area inset
for that anchor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d30061f6-a3f0-4872-bde3-b67b2a9efa7f
📒 Files selected for processing (7)
src/components/AppNavigation/AppNavigation.module.scsssrc/components/AppNavigation/components/MobileNavigation/MobileNavigation.module.scsssrc/components/DialogManager/Dialog.module.scsssrc/components/Drawer/Drawer.module.scsssrc/components/InputPanel/InputPanel.module.scsssrc/components/Menu/components/MobileMenu/MobileMenu.module.scsssrc/components/Tabs/components/TabsList/TabsList.module.scss
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Tabs/components/TabsList/TabsList.module.scss (1)
34-37: 🎯 Functional Correctness | 🟡 MinorAdd
env(..., 0px)fallbacks for footer safe-area insets inTabsList.module.scssThis footer block uses
env(safe-area-inset-bottom/right/left)without fallbacks, which can make thecalc(...)invalid in environments whereenv()fallback behavior isn’t available. Other parts of the repo already useenv(..., 0px)in some safe-area calculations, so adding it here improves robustness.Suggested fix
- height: calc(var(--app-bar-height) + env(safe-area-inset-bottom)); - min-height: calc(var(--app-bar-height) + env(safe-area-inset-bottom)); - padding: calc(var(--spacing) - var(--border-width)) 0 calc(var(--spacing) + env(safe-area-inset-bottom)); + height: calc(var(--app-bar-height) + env(safe-area-inset-bottom, 0px)); + min-height: calc(var(--app-bar-height) + env(safe-area-inset-bottom, 0px)); + padding: calc(var(--spacing) - var(--border-width)) 0 calc(var(--spacing) + env(safe-area-inset-bottom, 0px)); @@ - padding: 0 calc(var(--page-padding) + env(safe-area-inset-right)) 0 calc(var(--page-padding) + env(safe-area-inset-left)); + padding: 0 calc(var(--page-padding) + env(safe-area-inset-right, 0px)) 0 calc(var(--page-padding) + env(safe-area-inset-left, 0px));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Tabs/components/TabsList/TabsList.module.scss` around lines 34 - 37, Replace raw env(safe-area-inset-...) usages with fallbacks so calc() won't become invalid where env() lacks support: update the three occurrences in TabsList.module.scss (the height, min-height and padding lines using env(safe-area-inset-bottom)) to use env(safe-area-inset-bottom, 0px); also scan the same module for any env(safe-area-inset-left/right) and add , 0px fallbacks there as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Select/Select.hooks.ts`:
- Around line 54-65: The subscribe function currently creates an anonymous
resize handler and never removes the global listener, and
useSafeCollisionPadding calls useSyncExternalStore without a SSR-safe
getServerSnapshot; implement a getServerSnapshot function that returns a safe
fallback for SSR and pass it as the third argument to useSyncExternalStore
(referencing getSnapshot/getServerSnapshot and useSafeCollisionPadding), and
refactor subscribe so the resize handler is a named/closed-over function
(reference measureSafeAreas, cachedSafeAreas, listeners, initialized) that is
added once and removed via window.removeEventListener when the last subscriber
unsubscribes (i.e., on listeners.delete(listener) if listeners.size === 0 also
remove the global handler and reset initialized as needed).
In `@src/components/Select/Select.module.scss`:
- Around line 52-54: The popup's min-width (min-width: var(--anchor-width)) can
exceed the available space and ignore max-width; in Select.module.scss locate
the rule that sets min-width: var(--anchor-width) (the popup/select dropdown
styles) and change it to constrain the min to the smaller of anchor and
available width by using CSS min() (or an equivalent clamp) so the popup will
never force a width larger than var(--available-width) while still honoring the
anchor size; keep the existing max-width: var(--available-width) and max-height
unchanged.
---
Outside diff comments:
In `@src/components/Tabs/components/TabsList/TabsList.module.scss`:
- Around line 34-37: Replace raw env(safe-area-inset-...) usages with fallbacks
so calc() won't become invalid where env() lacks support: update the three
occurrences in TabsList.module.scss (the height, min-height and padding lines
using env(safe-area-inset-bottom)) to use env(safe-area-inset-bottom, 0px); also
scan the same module for any env(safe-area-inset-left/right) and add , 0px
fallbacks there as needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 322fa0ec-c332-4d9e-aa0d-871d687e8f17
📒 Files selected for processing (10)
src/components/AppNavigation/AppNavigation.module.scsssrc/components/AppNavigation/components/MobileNavigation/MobileNavigation.module.scsssrc/components/DialogManager/Dialog.module.scsssrc/components/Drawer/Drawer.module.scsssrc/components/InputPanel/InputPanel.module.scsssrc/components/Menu/components/MobileMenu/MobileMenu.module.scsssrc/components/Select/Select.hooks.tssrc/components/Select/Select.module.scsssrc/components/Select/Select.tsxsrc/components/Tabs/components/TabsList/TabsList.module.scss
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/components/AppNavigation/components/MobileNavigation/MobileNavigation.module.scss`:
- Line 69: The padding shorthand in MobileNavigation.module.scss currently uses
a fixed bottom value of 1rem; change the bottom padding to include the bottom
safe-area inset so the bottom spacing becomes 1rem plus
env(safe-area-inset-bottom) (i.e., replace the bottom component of the existing
padding shorthand with a calc that adds env(safe-area-inset-bottom) to the 1rem
value) so menu items won't sit under the home indicator.
In `@src/components/FooterBar/FooterBar.tsx`:
- Around line 31-41: The useEffect sets the CSS custom property
'--footer-bar-height' via the ResizeObserver but the cleanup only disconnects
the observer; update the cleanup returned by useEffect in FooterBar.tsx (the
effect using contentRef and ResizeObserver) to both disconnect the observer and
clear the CSS property (e.g., call
document.documentElement.style.removeProperty('--footer-bar-height') or set it
to an empty string) so the variable is removed when the component unmounts.
- Line 27: The default prop portalTarget = document.body will throw during SSR;
change FooterBar (and likewise AppNavigation) so portalTarget is not evaluated
at module/render time: remove the document.body default (use undefined or null
as the prop default) and inside the component (e.g., in FooterBar) compute a
safe runtime target by checking typeof document !== "undefined" (or using
useEffect to set a ref/state) and fallback to document.body only on the client
before creating the portal; update any usages that rely on the old default to
accept an optional portalTarget.
In `@src/components/Tabs/components/TabsList/TabsList.hooks.tsx`:
- Line 20: The JSX fragment return in TabsList.hooks.tsx currently includes a
trailing space ("<>{children} </>") that will render extra whitespace; update
the return in the component hook to remove the trailing space so it renders as a
plain fragment containing children (i.e., change the fragment around children to
have no extra spaces) — locate the return that uses the fragment with children
and remove the space before the closing tag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 78171a57-624f-4677-8468-622413cb3e3e
📒 Files selected for processing (22)
src/components/AppNavigation/AppNavigation.module.scsssrc/components/AppNavigation/components/MobileNavigation/MobileNavigation.module.scsssrc/components/DialogManager/Dialog.module.scsssrc/components/Drawer/Drawer.module.scsssrc/components/FooterBar/FooterBar.module.scsssrc/components/FooterBar/FooterBar.module.scss.d.tssrc/components/FooterBar/FooterBar.stories.tsxsrc/components/FooterBar/FooterBar.tsxsrc/components/FooterBar/FooterBarActions.module.scsssrc/components/FooterBar/FooterBarActions.module.scss.d.tssrc/components/FooterBar/FooterBarActions.tsxsrc/components/FooterBar/index.tssrc/components/InputPanel/InputPanel.module.scsssrc/components/Menu/components/MobileMenu/MobileMenu.module.scsssrc/components/Select/Select.hooks.tssrc/components/Select/Select.module.scsssrc/components/Select/Select.tsxsrc/components/Tabs/components/TabsList/TabsList.hooks.tsxsrc/components/Tabs/components/TabsList/TabsList.module.scsssrc/components/Tabs/components/TabsList/TabsList.tsxsrc/index.tssrc/style/variables.scss
Resolves CC-101
Summary by CodeRabbit
New Features
Style