[No-QA] refactor: introduce shared Overlay primitives#94127
Conversation
|
@linhvovan29546 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9bb15f5e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95b790bd9a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9540d0a93e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@TaduJR This PR introduce ~4000 lines of new code have no production consumer — nothing in Modal, Popover, or PopoverMenu imports from it. The unit tests verify internal logic, but real integration (focus, navigation, Onyx state) is untested until M2-M5. Could we include at least one thin integration proof (e.g., migrating a single popover to FloatingHost) to validate the foundation end-to-end before building on top of it? cc @roryabraham, what are your thoughts? |
Explanation of Change
M1 of a five-PR split of the Modal v2 / Dialog / Popover v2 / PopoverMenu v2 decomposition. This PR ships the shared bottom layer — a new
Overlay/primitive folder plus a few small cross-feature hooks/libs — that the next four PRs compose on. No consumer migration here; every existing user-facing surface is byte-identical tomain.Contents:
src/components/Overlay/:DismissableLayer,FloatingHost,Portal,Presence,AnimatedSurface,createHeadingSystem, plus 12 hooks (Esc handling, anchored positioning, aria-hide, body-scroll lock, focus management, etc.) and 7 libs (anchor measurement, dismissable-layer stack store, external-store factory, etc.)useCallbackRef,useControlledState,useDisclosureStatecomposeEventHandlers,isHTMLElementFocusTrapForModal.shouldReturnFocusaccepts() => booleanso M2 can resolve the Composer-focus baton at restore timePressableRefElementfromPressable/GenericPressable/types.tsso M1'smeasureAnchor/overlayStorecan type their anchor refsOVERLAY_FADE_IN_WEB/OVERLAY_FADE_OUT_WEB), 1 style (overlayPortalHost)Fixed Issues
$ #91238
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari