Repository navigation
Conversation
…ugh the layer runtime's presentation hold useSelectorPresentation (Selector and MultiSelector) kept two latches of its own: activePresentationRef, the presentation an opening chose, and isSheetPresented, the sheet kept mounted after it closes until BottomSheet's focus handoff reaches the trigger. Both now use the layer runtime's presentation hold (Layer/presentationHold): createPresentationHold taken at show, and usePresentationHold over the sheet's open state, released where the old state was cleared. No behavior change; Selector and MultiSelector tests pass unchanged.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
Contributor
PR Analysis Report
|
There was a problem hiding this comment.
Recommending approval. useSelectorPresentation now keeps the opening's presentation and the closing bottom sheet through the layer runtime's presentation hold, as architecture:layer-runtime INV14 asks, and Selector and MultiSelector behave as they do on the base branch: the unit suites pass, a presentation-flip check matches state for state, and Chromium captures of both surfaces are pixel-identical. Two small follow-ups:
- The layer-runtime record's current-gaps paragraph on the base branch names only Popover as using the shared presentation hold and still lists Selector among components that build their own; please amend it in whichever of #8587 and this PR lands last.
- CI runs only for pull requests into
mainand release branches, so the Selector specs the description mentions have not run on this branch yet.
[Automated review]
8 tasks done
This branch was successfully deployed
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.
Stacked on #8587 (the layer runtime's presentation hold). The base is its branch, so this diff is only the Selector adoption.
What it does
useSelectorPresentation, the presentation controller Selector and MultiSelector share, kept two latches of its own:activePresentationRef: the presentation an opening chose (sheet or popover). It's now acreatePresentationHold, taken atshow().isSheetPresented: the sheet kept mounted after it closes, until BottomSheet's final-focus handoff reaches the trigger. It's now ausePresentationHoldover the sheet's open state.Both are released exactly where the old state was cleared: when the trigger takes focus after the sheet's exit, and, for the sheet hold, when a popover opens instead.
architecture:layer-runtimeINV14 names this job, keeping a layer's chosen presentation from the open until its close settles, as the runtime's.No behavior change
Selector's and MultiSelector's tests are the oracle and pass unchanged (486), including the sheet presentation's exit and its focus return to the trigger. The real-browser check is their specs, which this PR's CI runs because the change touches the Selector folder.
Size (gzip KB, minified)
The same method as #8587: a fresh
packages/coredist at the base and at this head; esbuild bundle, minify, splitting, ESM, packages external.Testing
pnpm vitest run --project ui packages/core/src/Selector packages/core/src/MultiSelector: 486 pass.[chore]changeset.