[Refactor] Replace hand-rolled overlay logic with useOverlay hook#198
Merged
[Refactor] Replace hand-rolled overlay logic with useOverlay hook#198
Conversation
- Extract portal, ref-counted scroll lock (iOS-safe), focus trap, inert, reduced-motion, and focus restore into useOverlay hook - Rewrite BottomSheet to portal via createPortal, fixing stacking context fragility from inline fixed positioning - Migrate DrawerLayout drawer to useOverlay + portal, removing duplicated scroll lock and focus trap effects - Add --text-tertiary to both themes, fixing undefined CSS variable in GatewayDebugLog
Contributor
|
Hi @samzong, DetailsInstructions for interacting with me using comments are available here. |
…itive # Conflicts: # packages/pwa/src/components/BottomSheet.tsx # packages/pwa/src/components/GatewayDebugLog.tsx # packages/pwa/src/components/SettingsSheet.tsx # packages/pwa/src/views/DrawerLayout.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace duplicated overlay infrastructure (scroll lock, focus trap, escape handling, body overflow) across BottomSheet and DrawerLayout with a single
useOverlayhook. All overlays now render through portals, fixing stacking context fragility and adding properinert, reduced-motion support, and iOS-safe scroll lock.Type of change
[Refactor]internal cleanupWhy is this needed?
The prior BottomSheet and DrawerLayout each hand-rolled identical overlay logic with known defects:
position: fixedbreaks under ancestortransform/will-change/filterdocument.body.style.overflow = "hidden"fails on iOS Safari and races when multiple overlays coexistrequestAnimationFramenever cancelled on unmountinert— background content remains focusable and interactive to assistive technologyprefers-reduced-motion— spring animations run regardless of user preference--text-tertiaryused in GatewayDebugLog but never definedWhat changed?
hooks/useOverlay.ts: portal target, ref-counted scroll lock (iOSposition:fixed+ scrollY save/restore),inerton#root, scoped focus trap viaonKeyDownon container,requestAnimationFramecleanup,isConnectedguard on focus restore,useReducedMotionfrom framer-motionBottomSheet.tsx: delegates overlay concerns touseOverlay, renders viacreatePortalDrawerLayout.tsx: removed hand-written scroll lock effect (lines 40-49) and focus trap effect (lines 51-88), drawer now portals viauseOverlay--text-tertiary: #838b97to both dark and light themes inindex.cssArchitecture impact
docs/architecture-invariants.md: noneLinked issues
Closes #
Validation
pnpm lintpnpm testpnpm check:ui-contractpnpm check(full gate: lint + architecture + ui-contract + renderer-copy + i18n + dead-code + format + typecheck + test)Commands, screenshots, or notes:
Screenshots or recordings
No visual changes. Overlay behavior (animation, drag-to-dismiss, backdrop, escape) is identical. The only visible difference is
--text-tertiarynow resolves correctly in GatewayDebugLog (previously fell through to inherited color).Release note
NONE.Checklist
[Refactor]