Repository navigation
fix(BottomSheet): a replaced sheet moves down to a shorter entering sheet again - #8920
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
…heet again In the replace handoff between standalone sheets, a taller retained sheet is meant to move down to the entering sheet's resting top edge while the entering sheet rises, and fade only after both motions finish (component:BottomSheet FR10). The stack measured that resting edge from the entering panel's parent element. A standalone panel sits directly in its dialog, which spans the viewport, so the parent's top was always 0, the offset always 0, and the retained sheet stayed at its own height and faded out above the entering sheet. The resting top is now read from the entering panel itself: its box less the translation its entrance is still applying, whatever element hosts it. BottomSheetSwitcher steps keep their own measurement and are unchanged. Under reduced motion the retained sheet still does not move.
6918cda to
5674494
Compare
There was a problem hiding this comment.
Looks good: measuring the entering panel itself restores the component:BottomSheet FR10 alignment for standalone replace handoffs, and the new replace cases fail without the fix and pass with it. In Chromium and WebKit the standalone Hug content flow now matches the BottomSheetSwitcher handoff frame for frame, while reduced motion still keeps the replaced sheet in place. The CI run for this commit was cancelled before it finished, so it needs a fresh run before merging; nothing else is needed.
[Automated review]
There was a problem hiding this comment.
Looks good: measuring the entering panel itself restores the component:BottomSheet FR10 alignment for standalone replace handoffs, and the five replace cases you named fail without the fix and pass with it. In Chromium the standalone Hug content handoff now matches the BottomSheetSwitcher handoff frame for frame (and at landing in WebKit), while reduced motion still keeps the replaced sheet in place with unchanged timing. No CI workflow has reported on this commit yet, so let the required checks run before merging.
[Automated review]
f7f107f to
0a3a0de
Compare
There was a problem hiding this comment.
Looks good: measuring the entering panel itself restores the component:BottomSheet FR10 alignment for standalone replace handoffs, and the five replace cases you named fail without the fix and pass with it. In Chromium the standalone Hug content handoff now matches the BottomSheetSwitcher handoff frame for frame (and at landing in WebKit), while reduced motion still keeps the replaced sheet in place with unchanged timing. The CI run for this commit was cancelled before its jobs finished, so the required checks need a fresh run before merging.
[Automated review]
0a3a0de to
649334c
Compare
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: The accessibility audit did not run on this pull request. It runs after merges to Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
User impact
A flow written the way the
BottomSheetSwitcherdeprecation recommends — each step its ownBottomSheet, one state value naming the open step — runs the replace handoff between steps. On every switch to a shorter step, the taller sheet being replaced stayed at its own height while the new sheet rose in front of it, and then the band of it showing above the new sheet faded out. The sheet's height changed by a fade, not by the sheet adjusting. This hit every replace to a shorter sheet, modal andhasScrim={false}, on touch and mouse alike.BottomSheetSwitcherflows were not affected.Problem and solution fit
alignmentOffsetForPanels(inbottomSheetStack.ts) read where the entering sheet would rest fromenteringPanel.parentElement. The handoff was written when a standalone panel sat inside a positioner wrapper. feat(Popover): presentation — a BottomSheet on a phone when asked, as iOS presents a popover #8229 made the open dialog the panel's positioner and removed that wrapper. feat(core): BottomSheet stacks by open order, replaces on a same-update switch, and BottomSheetSwitcher is deprecated #7648 merged after it, so the parent became the dialog, which spans the viewport. Its top is 0, so the offset was always 0 and the retained sheet never moved. Switcher steps keep their positioner and their own measurement, which is why the switcher still aligns.The fix measures the entering panel itself: its box, less the translation its entrance is still applying. That is where it comes to rest, whatever element hosts it.
Expected behavior and authority
component:BottomSheetFR10 (current) says the opening sheet "enters above the retained sheet; a retained sheet whose top edge is above the entering sheet's resting top edge moves down to meet it at the same time; the retained sheet fades only after both motions complete". The FR10–FR11 verification row requires per-frame parity of the standalone flow with the switcher's handoff.component:BottomSheetSwitcherFR3 describes the same alignment for switcher steps. TheBottomSheetbuilder doc ("Replacing a sheet") promises it to consumers.design:reduced-motionDR1: the retained sheet does not travel under reduced motion. This is unchanged.packages/core/src/BottomSheetandpackages/core/src/Layer, ends at feat(core): BottomSheet stacks by open order, replaces on a same-update switch, and BottomSheetSwitcher is deprecated #7648, the first commit with a standalone replace that does not align.No record changes: the records already describe the expected behavior.
Smallest restoration
One measurement function in
bottomSheetStack.tschanges. There is no API change, no style or token change, no record change, andBottomSheetSwitchercode is untouched.Evidence
90da4a7,Core/BottomSheetReplace"Hug content", sampled per frame in Chromium:translateYstays 0. The visible sheet top holds at 434 px for about 500 ms, then jumps to 590 px as the retained band fades, with no intermediate position.BottomSheetSwitchercounterpart within 0.3 px on all 32 measured switches (4 story pairs × 4 switches × phone and desktop).BottomSheetSwitcherstories match v0.7.4 within 0.5 px per frame (5 stories × 4 switches × phone and desktop), before and after this change. The three switcher stories added after v0.7.4 (stacked over a sheet, stacked under a sheet, typing over an advancing flow) are identical on main and with this change.MobilePowerSearchtouch flow is unchanged. Its steps stack, and all are the sametallheight.BottomSheetReplace.test.tsxnow lays each step out as a browser does: the dialog spans the viewport and the panel sits in it. Before, the tests gave the dialog a box it never has. Five cases fail on main and pass here:Scope
Testing
pnpm exec vitest run packages/core/src/BottomSheet packages/core/src/Layer packages/lab/src/MobilePowerSearch packages/lab/src/MobileTokenizer: 23 files, 818 tests passed.pnpm build, all passed:@astryxdesign/core:tsc --project tsconfig.json --noEmitandtypecheck:docs;@astryxdesign/lab:typecheck:docs;@astryxdesign/storybook:typecheck;internal/a11y-spec:typecheck;@astryxdesign/cli:typecheck:strictandtypecheck:template-docs.ASTRYX_STRICT_LINT=1 eslint --max-warnings=0on the changed files, plus Prettier.pnpm check:repo(includescheck:knowledgeandcheck:changesets), andgit diff --check.prefers-reduced-motion: reduce. Each panel's position,transformand opacity, and the visible sheet top, were sampled every animation frame across each switch. Builds compared: the main build, a local Storybook build of v0.7.4, and a local Storybook build of this change. Stories:BottomSheetSwitcherstory with a step switch;BottomSheetReplacemirrors;BottomSheetStacking"Orders drill-in";MobilePowerSearch"Touch flow" (Filters → Add filter → a field's value step).