Repository navigation
test(core): BottomSheet detents, proven with the interaction harness - #10055
Conversation
The "Snap points" story's detents under real touch input: the drag follows the finger and a slow release settles on the nearest detent, a downward flick dismisses, the settle's sampled curve matches its pinned timing function, the settle keeps advancing through a blocked main thread, it does no Layout or Paint per frame, and storyboards of the drag and the settle. The harness README and config name it as the spec to copy. The busy-main-thread proof now reads where the settle ends from the marker chip the frames track, which sits inset in the sheet, instead of from the sheet's own top: with the sheet's top, the judged window ended with the marker 11 px short of its rest, about 98 ms into the settle, and an upward settle read as a 428 ms stall. A check pins that the judged window runs until the marker is within 2 px of its rest.
…erting it The settle test asserted today's settle, the theme's 300 ms --ease-standard transition, frame by frame. That curve starts a release at rest at about 1,400 px/s, where spec:AST-094 FR38 starts a BottomSheet's release at the finger's speed, so the test pinned a curve the spec rules out. It now records the curve, its easing, its duration and its frame-by-frame fit, in the evidence and an annotation, as it already did for the velocity hand-off, and keeps asserting where the settle lands: the half detent.
BottomSheet FR19 states the release rule the spec pins (spec:AST-094 FR36, FR37) and the settle's present-tense gap against FR38. The spec imports test from @astryxdesign/a11y-spec/playwright, as every browser spec does. The core devDependency on the harness is already on main.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
|
/approve-spec 278777f |
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| BottomSheet | 56.6 kB (0) · 7.5 kB CSS (0) | — | ✓ · ✓ · — |
| BottomSheetSwitcher | 17.4 kB (0) · 3.4 kB CSS (0) | — | ✓ · ✓ · — |
Package size: no published package changed; no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 9 rows, core 4 rows; run pnpm api:report.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested. The detent spec is a useful exemplar and passes in Chromium, but FR19 needs two wording fixes, and the WebKit run needs a look before this lands.
- FR19 (
docs/specs/components/BottomSheet/BottomSheet.spec.md:171) says a projection past the dismiss point closes the sheet, but only aninfosheet closes (BottomSheet.tsx:907); aformorrequiredsheet settles on its shortest detent, as FR7 says, and FR19 should say so. - FR19's implementation-gap note gives a fixed 300 ms and about 1,400 px/s, but the settle runs over the theme's
--duration-medium(410 ms with Core's default tokens, 300 ms in the neutral theme) and starts at a speed set by the distance left (about 1,415 px/s for this spec's 102 px, about 280 px/s for 20 px), so please state it in those terms. - The "settle keeps advancing every frame while the main thread is blocked" case failed in 5 of 6 WebKit runs, holding still for about 380–470 ms once the transition ends, while the harness's own compositor controls passed in the same WebKit; CI's WebKit leg runs changed interaction specs, so please run the WebKit project and either fix the case or skip it in WebKit with the reason and record that gap in FR19.
Non-blocking: the uikit-parity annotation computes initialVelocity from the modelled curve rather than from the samples.
[Automated review]
| | FR16 | An open modal sheet is a focus region through Core's shared focus-region module (`component:Dialog/DEC-2`): in the top sheet, Tab from the last tabbable control moves focus to the first and Shift+Tab from the first moves focus to the last, so focus never leaves the sheet for the browser's own interface. A covered or closing sheet does not answer Tab, and a non-modal sheet contains nothing (FR11). A switcher step (the deprecated BottomSheetSwitcher) is the exception to DEC-2, as `component:Dialog` DEC-2 states. | `component:Dialog/DEC-2` | | ||
| | FR17 | A BottomSheet open on the server, by its own `isOpen` or as a `BottomSheetSwitcher`'s active sheet, renders closed until hydration (no open styles, no `aria-modal`, the markup of a closed sheet) and opens in the render after hydration, the first that can enter its native host; it never claims a modality it does not have (`spec:AST-020` FR1, WCAG 2.2 SC 4.1.2). The server and hydrating renders produce the same markup (`spec:AST-013` FR9), and the open after hydration is settled, with no entry motion; later opens and closes animate as usual (`spec:AST-013` FR13). | `spec:AST-020` FR1 | | ||
| | FR18 | A finger's pull up past the tallest detent stretches the sheet on `spec:AST-094` FR3's rubber band, the same on every pull, with no hold. The sheet's surface reaches below the screen far enough for the whole stretch and the release, so the page under it never shows. A pull on the sheet's body stretches it as a pull on the grabber does, once the content has no room left to scroll that way: content that fits stretches at once, and a long list scrolls to its end first, then holds still while the sheet stretches. At every detent, a list that reaches its end mid-pull holds still while the sheet takes the rest of the pull. A sheet the caller sizes or styles keeps 48 px below the screen, and its stretch stops there. An inline block size, any `xstyle` and any class name count, because the sheet cannot read their rules. A mouse or a pen keeps its own resisted stretch, at most 48 px (`spec:AST-017` FR59). | DEC-3; `spec:AST-094` FR3 | | ||
| | FR19 | A release settles on the detent nearest its projected position (`spec:AST-094` FR36); a projection past the dismiss point closes the sheet. Implementation gap: the settle plays the theme's 300 ms `--ease-standard` from about 1,400 px/s rather than a spring from the finger's speed (`spec:AST-094` FR38). | `spec:AST-094` FR36, FR37, FR38 | |
There was a problem hiding this comment.
Two statements here do not match the shipped sheet:
- "a projection past the dismiss point closes the sheet": only an
infosheet closes (BottomSheet.tsx:907passesisSwipeDismissAllowed={purpose === 'info'}); aformorrequiredsheet settles on its shortest detent (useSheetGestures.ts:1161-1176), as FR7 says. Suggest: "aninfosheet whose projection passes the dismiss point closes; aformorrequiredsheet settles on its shortest detent (FR7)". - "the theme's 300 ms
--ease-standardfrom about 1,400 px/s": the settle is--duration-medium(410 ms with Core's default tokens, 300 ms in the neutral theme) on--ease-standard, and its start speed scales with the distance left (about 1,415 px/s for the spec's 102 px, about 280 px/s for 20 px). Suggest: "the settle plays--ease-standardover the theme's--duration-medium, at a speed set by the distance left rather than the finger's".
| type: 'busy-main-thread', | ||
| description: proof.summary, | ||
| }); | ||
| expect(proof.verdict.failures, proof.summary).toEqual([]); |
There was a problem hiding this comment.
This case failed in 5 of 6 WebKit runs (--project webkit): 50–70% of frames moved, with the longest stall at 376–469 ms from about +300 ms, when the settle's transition ends. The harness's own "compositor animation advances every frame through a blocked main thread" and "block that starts during a just-started transition" controls passed in the same WebKit. CI's WebKit leg adds changed interaction specs, so please run it there. Then either fix the case, or skip it in WebKit with the reason and record the gap against spec:AST-094 FR5 in FR19.
| }); | ||
| // UIKit parity metric, reported rather than asserted until the spring lands: | ||
| // the settle's initial velocity vs the finger's at release (here 0). | ||
| const initialVelocity = ((expectedSettle(1) - expectedSettle(0)) / 1) * 1000; // px/s over the first ms |
There was a problem hiding this comment.
Non-blocking: initialVelocity comes from the modelled expectedSettle, so the uikit-parity annotation reports the expected curve's slope even if the played curve differs. Computing it from samples[0] and samples[1] would report what the sheet actually played.
…WebKit skips the blocked-thread case FR19: only an info sheet closes on a projection past the dismiss point; form and required settle on the shortest detent (FR7). The settle's gap is stated as --ease-standard over --duration-medium at a distance-scaled speed. In WebKit the settle stops short of its detent while the main thread is blocked: the spec skips that case there and FR19 records the gap. The parity annotation reads the settle's start speed from its first two frames.
|
/approve-spec f3fab68 |
There was a problem hiding this comment.
Changes requested for one sentence in FR19. The earlier points are fixed: FR19 now gates the swipe dismissal on purpose, its settle gap no longer fixes one release's numbers, the blocked-thread case skips WebKit with a matching FR19 gap, and the uikit-parity annotation reads the played curve. All six cases pass in Chromium at this head.
- Miss: FR19 (
docs/specs/components/BottomSheet/BottomSheet.spec.md:171) citesspec:AST-094FR37 ("a release whose velocity points back toward the start MUST cancel"), but the shipped release still closes the sheet or moves it to the next detent when its projection gets there (useSheetGestures.ts:1161,:1183), so please add a third gap note such as "Implementation gap: a release whose velocity points back toward its start still commits when its projection passes the dismiss point or lies nearer the next detent (spec:AST-094FR37)."
CI has not run on this draft head yet.
[Automated review]
| | FR16 | An open modal sheet is a focus region through Core's shared focus-region module (`component:Dialog/DEC-2`): in the top sheet, Tab from the last tabbable control moves focus to the first and Shift+Tab from the first moves focus to the last, so focus never leaves the sheet for the browser's own interface. A covered or closing sheet does not answer Tab, and a non-modal sheet contains nothing (FR11). A switcher step (the deprecated BottomSheetSwitcher) is the exception to DEC-2, as `component:Dialog` DEC-2 states. | `component:Dialog/DEC-2` | | ||
| | FR17 | A BottomSheet open on the server, by its own `isOpen` or as a `BottomSheetSwitcher`'s active sheet, renders closed until hydration (no open styles, no `aria-modal`, the markup of a closed sheet) and opens in the render after hydration, the first that can enter its native host; it never claims a modality it does not have (`spec:AST-020` FR1, WCAG 2.2 SC 4.1.2). The server and hydrating renders produce the same markup (`spec:AST-013` FR9), and the open after hydration is settled, with no entry motion; later opens and closes animate as usual (`spec:AST-013` FR13). | `spec:AST-020` FR1 | | ||
| | FR18 | A finger's pull up past the tallest detent stretches the sheet on `spec:AST-094` FR3's rubber band, the same on every pull, with no hold. The sheet's surface reaches below the screen far enough for the whole stretch and the release, so the page under it never shows. A pull on the sheet's body stretches it as a pull on the grabber does, once the content has no room left to scroll that way: content that fits stretches at once, and a long list scrolls to its end first, then holds still while the sheet stretches. At every detent, a list that reaches its end mid-pull holds still while the sheet takes the rest of the pull. A sheet the caller sizes or styles keeps 48 px below the screen, and its stretch stops there. An inline block size, any `xstyle` and any class name count, because the sheet cannot read their rules. A mouse or a pen keeps its own resisted stretch, at most 48 px (`spec:AST-017` FR59). | DEC-3; `spec:AST-094` FR3 | | ||
| | FR19 | A release settles on the detent nearest its projected position (`spec:AST-094` FR36). An `info` sheet whose projection passes the dismiss point closes; a `form` or `required` sheet settles on its shortest detent (FR7). Implementation gap: the settle plays `--ease-standard` over the theme's `--duration-medium`, at a speed set by the distance left rather than the finger's (`spec:AST-094` FR38). Implementation gap: in WebKit, the settle stops advancing while the main thread is blocked (`spec:AST-094` FR5). | `spec:AST-094` FR5, FR36, FR37, FR38; FR7 | |
There was a problem hiding this comment.
This row cites spec:AST-094 FR37 ("A release whose velocity points back toward the start MUST cancel, whatever the travel so far"), but the shipped release judges only the projected position (useSheetGestures.ts:1161 and :1183, with dir from the net drag at :1414). An info sheet dragged past its dismiss line and lifted while moving back up at 20 px/s still closes, and a drag four fifths of the way to the half detent lifted the same way still lands on it. Suggest a third gap note: "Implementation gap: a release whose velocity points back toward its start still commits when its projection passes the dismiss point or lies nearer the next detent (spec:AST-094 FR37)."
A release whose velocity points back toward its start still commits when its projection passes the dismiss point or lies nearer the next detent.
|
/approve-spec 469b67d |
There was a problem hiding this comment.
Changes requested for one more correction in FR19. The FR37 gap note is in place, all six cases pass in Chromium, the four WebKit cases pass, and the WebKit stall FR19 records reproduces when the blocked-thread case runs there.
- Miss: FR19 (
docs/specs/components/BottomSheet/BottomSheet.spec.md:171) says aninfosheet closes whenever its projection passes the dismiss point and a release lands on the detent nearest its projection, but the shipped sheet also requires the net drag to point that way (useSheetGestures.ts:1161and:1183,snapOffsets.ts:233-243), so a release dragged up and then flung down past its start stays open on its starting detent; please qualify both sentences, for example "A release settles on the detent nearest its projected position among its starting detent and the detents past it in the drag's direction (spec:AST-094FR36). Aninfosheet dragged down whose projection passes the dismiss point closes; aformorrequiredsheet settles on its shortest detent (FR7)."
The description's "CI's WebKit leg runs the other five cases" should say four, because the Layout/Paint case is Chromium-only. CI has not run on this draft head yet.
[Automated review]
| | FR16 | An open modal sheet is a focus region through Core's shared focus-region module (`component:Dialog/DEC-2`): in the top sheet, Tab from the last tabbable control moves focus to the first and Shift+Tab from the first moves focus to the last, so focus never leaves the sheet for the browser's own interface. A covered or closing sheet does not answer Tab, and a non-modal sheet contains nothing (FR11). A switcher step (the deprecated BottomSheetSwitcher) is the exception to DEC-2, as `component:Dialog` DEC-2 states. | `component:Dialog/DEC-2` | | ||
| | FR17 | A BottomSheet open on the server, by its own `isOpen` or as a `BottomSheetSwitcher`'s active sheet, renders closed until hydration (no open styles, no `aria-modal`, the markup of a closed sheet) and opens in the render after hydration, the first that can enter its native host; it never claims a modality it does not have (`spec:AST-020` FR1, WCAG 2.2 SC 4.1.2). The server and hydrating renders produce the same markup (`spec:AST-013` FR9), and the open after hydration is settled, with no entry motion; later opens and closes animate as usual (`spec:AST-013` FR13). | `spec:AST-020` FR1 | | ||
| | FR18 | A finger's pull up past the tallest detent stretches the sheet on `spec:AST-094` FR3's rubber band, the same on every pull, with no hold. The sheet's surface reaches below the screen far enough for the whole stretch and the release, so the page under it never shows. A pull on the sheet's body stretches it as a pull on the grabber does, once the content has no room left to scroll that way: content that fits stretches at once, and a long list scrolls to its end first, then holds still while the sheet stretches. At every detent, a list that reaches its end mid-pull holds still while the sheet takes the rest of the pull. A sheet the caller sizes or styles keeps 48 px below the screen, and its stretch stops there. An inline block size, any `xstyle` and any class name count, because the sheet cannot read their rules. A mouse or a pen keeps its own resisted stretch, at most 48 px (`spec:AST-017` FR59). | DEC-3; `spec:AST-094` FR3 | | ||
| | FR19 | A release settles on the detent nearest its projected position (`spec:AST-094` FR36). An `info` sheet whose projection passes the dismiss point closes; a `form` or `required` sheet settles on its shortest detent (FR7). Implementation gap: the settle plays `--ease-standard` over the theme's `--duration-medium`, at a speed set by the distance left rather than the finger's (`spec:AST-094` FR38). Implementation gap: in WebKit, the settle stops advancing while the main thread is blocked (`spec:AST-094` FR5). Implementation gap: a release whose velocity points back toward its start still commits when its projection passes the dismiss point or lies nearer the next detent (`spec:AST-094` FR37). | `spec:AST-094` FR5, FR36, FR37, FR38; FR7 | |
There was a problem hiding this comment.
Miss from the earlier review: the first two sentences leave out the drag direction the shipped sheet applies. It closes only when dir > 0 && projected > dismissOffset (useSheetGestures.ts:1161, dir is the sign of the net drag), and it settles only on the starting detent or one past it in the drag direction (useSheetGestures.ts:1183, snapOffsets.ts:233-243). On the Snap points story, a release from the half detent that moves 280 px up and then 200 px down, lifting at 2,000 px/s downward, stays open on the half detent although its projection passes the dismiss point; that is the cancel spec:AST-094 FR37 asks for. Suggested wording:
A release settles on the detent nearest its projected position among its starting detent and the detents past it in the drag's direction (`spec:AST-094` FR36). An `info` sheet dragged down whose projection passes the dismiss point closes; a `form` or `required` sheet settles on its shortest detent (FR7).
… a gap against AST-094 FR36
|
/approve-spec 0618174 |
There was a problem hiding this comment.
Changes requested for one correction in FR19. All six cases pass in Chromium at this head, and the spec reads well as the harness exemplar.
- FR19's new gap note (
docs/specs/components/BottomSheet/BottomSheet.spec.md:171) calls the return to the starting detent after a drag up and a downward fling an FR36 gap, but that release's velocity points back toward its start,spec:AST-094FR37 requires exactly that cancel ("MUST cancel, whatever the travel so far"), and FR19's own FR37 note says the same release still commits; please delete the note, state the cancel ("a release whose velocity points back toward its start returns to its starting detent (spec:AST-094FR37)"), and limit the FR37 note to a release that follows a net downward drag and projects past the dismiss point, or whose projection lies nearer the next detent in the drag's direction.
Non-blocking: FR19's verification row leaves out the rendering-trace case and describes Layout/Paint as checked under the main-thread block, which that case does not do. CI has not run on this draft head yet.
[Automated review]
| | FR16 | An open modal sheet is a focus region through Core's shared focus-region module (`component:Dialog/DEC-2`): in the top sheet, Tab from the last tabbable control moves focus to the first and Shift+Tab from the first moves focus to the last, so focus never leaves the sheet for the browser's own interface. A covered or closing sheet does not answer Tab, and a non-modal sheet contains nothing (FR11). A switcher step (the deprecated BottomSheetSwitcher) is the exception to DEC-2, as `component:Dialog` DEC-2 states. | `component:Dialog/DEC-2` | | ||
| | FR17 | A BottomSheet open on the server, by its own `isOpen` or as a `BottomSheetSwitcher`'s active sheet, renders closed until hydration (no open styles, no `aria-modal`, the markup of a closed sheet) and opens in the render after hydration, the first that can enter its native host; it never claims a modality it does not have (`spec:AST-020` FR1, WCAG 2.2 SC 4.1.2). The server and hydrating renders produce the same markup (`spec:AST-013` FR9), and the open after hydration is settled, with no entry motion; later opens and closes animate as usual (`spec:AST-013` FR13). | `spec:AST-020` FR1 | | ||
| | FR18 | A finger's pull up past the tallest detent stretches the sheet on `spec:AST-094` FR3's rubber band, the same on every pull, with no hold. The sheet's surface reaches below the screen far enough for the whole stretch and the release, so the page under it never shows. A pull on the sheet's body stretches it as a pull on the grabber does, once the content has no room left to scroll that way: content that fits stretches at once, and a long list scrolls to its end first, then holds still while the sheet stretches. At every detent, a list that reaches its end mid-pull holds still while the sheet takes the rest of the pull. A sheet the caller sizes or styles keeps 48 px below the screen, and its stretch stops there. An inline block size, any `xstyle` and any class name count, because the sheet cannot read their rules. A mouse or a pen keeps its own resisted stretch, at most 48 px (`spec:AST-017` FR59). | DEC-3; `spec:AST-094` FR3 | | ||
| | FR19 | A release settles on the detent nearest its projected position (`spec:AST-094` FR36). An `info` sheet whose projection passes the dismiss point closes; a `form` or `required` sheet settles on its shortest detent (FR7). Implementation gap: the settle plays `--ease-standard` over the theme's `--duration-medium`, at a speed set by the distance left rather than the finger's (`spec:AST-094` FR38). Implementation gap: in WebKit, the settle stops advancing while the main thread is blocked (`spec:AST-094` FR5). Implementation gap: a release whose velocity points back toward its start still commits when its projection passes the dismiss point or lies nearer the next detent (`spec:AST-094` FR37). Implementation gap: the release considers only its starting detent and the detents past it in the direction of the net drag, and closes only on a net downward drag, so a drag up then flung down past its start stays on its starting detent (`spec:AST-094` FR36). | `spec:AST-094` FR5, FR36, FR37, FR38; FR7 | |
There was a problem hiding this comment.
The last gap note files a drag up then flung down past its start, which stays on its starting detent, as a gap against spec:AST-094 FR36. That release's velocity points back toward its start, and FR37 says "A release whose velocity points back toward the start MUST cancel, whatever the travel so far", so staying is the required result. The restriction changes no other release (snapOffsets.ts:226-245, useSheetGestures.ts:1161). It also contradicts the FR37 note just before it, which says such a release "still commits when its projection passes the dismiss point". On the Snap points story, 280 px up then 200 px down, lifting at 2,000 px/s, stays on the half detent; 60 px up then 120 px down closes. Suggested wording:
A release settles on the detent nearest its projected position (`spec:AST-094` FR36); a release whose velocity points back toward its start returns to its starting detent (`spec:AST-094` FR37). … Implementation gap: a release whose velocity points back toward its start still commits when it follows a net downward drag and its projection passes the dismiss point, or when its projection lies nearer the next detent in the drag's direction (`spec:AST-094` FR37).
and delete the FR36 gap note.
| | FR16 | `BottomSheet.test.tsx` focus-region suite | Modal and non-modal sheets; Tab and Shift+Tab at the sheet's edge | Tab from the last control leaves a modal sheet, or a non-modal sheet captures Tab | `audit:BottomSheet/accessibility` | | ||
| | FR17 | `BottomSheet.ssr.test.tsx`; `BottomSheetSwitcher.test.tsx` flow that mounts open | Open on the server; hydration; a closed sheet's markup | An open server render with `aria-modal`, open styles, or markup unlike a closed one, a hydration error, no `showModal()` after hydration, an entry motion on the open right after hydration, or none on a later open fails. | `audit:BottomSheet/accessibility` | | ||
| | FR18 | `useSheetGestures.test.ts` stretch suites; `BottomSheetPanel.test.tsx` reserve cases; `BottomSheetDrag.a11y.chromium.spec.ts` | Three finger pulls past the top; short content pulled up on its body at five points, the first pull and every one after it; a long list pulled past its end; a mouse pull and a pen pull | A capped or held stretch, a pull that stretches less than the one before it, a body pull on short content that moves nothing, a long list that never stretches the sheet once at its end or rubber-bands itself while the sheet stretches, or a reserve under 40% of the viewport on a device with a touch pointer fails, unless the caller sizes or styles the sheet (48 px). | `audit:BottomSheet/behavior` | | ||
| | FR19 | `BottomSheetDetent.interaction.browser.spec.ts` | On the Snap points story under real touch input: a slow release between detents, a 2,000 px/s downward flick, the settle to a detent with its curve recorded (FR19's gap), and, in Chromium, the settle through a 700 ms main-thread block (`spec:AST-094` FR5; WebKit's gap is skipped) | A slow release that lands off the nearest detent, a flick past the dismiss point that leaves the sheet open, a settle that ends off the detent, or a settle that stalls or lays out or paints on a frame while the main thread is blocked fails | `audit:BottomSheet/behavior` | |
There was a problem hiding this comment.
Non-blocking: the states column does not list the rendering-trace case, and the failure column says "lays out or paints on a frame while the main thread is blocked", while that case traces a settle with no block. Suggest "a settle that stalls while the main thread is blocked, or lays out or paints during the settle".
…releases that still commit A release whose velocity points back toward its start returns to its starting detent (spec:AST-094 FR37). The gap covers the releases that still commit: after a net downward drag past the dismiss point, or nearer the next detent in the drag's direction. The FR36 gap note is withdrawn. The verification row lists the rendering-trace case.
|
/approve-spec 38c85d0 |
There was a problem hiding this comment.
Changes requested for one more line in FR19. The fourth gap note is gone, FR19 now states FR37's cancel with a matching gap note, and all six cases pass in Chromium at this head.
- Miss: FR19 (
docs/specs/components/BottomSheet/BottomSheet.spec.md:171) says a release settles on the detent nearest itsspec:AST-094FR36 projection, but the shipped sheet projects nothing after 48 px of travel or less (packages/core/src/BottomSheet/useSheetGestures.ts:1124-1127, pinned by the "does not flick-dismiss on a fast but short nudge" unit test), so a 32 px flick at 2,000 px/s on the Snap points story leaves theinfosheet open; please add a gap note such as "a release after 48 px of travel or less projects nothing: it settles on the detent nearest where the finger lifted, so a short fast flick never closes the sheet (spec:AST-094FR36)".
Non-blocking: FR19's verification row could also name useSheetGestures.test.ts and snapOffsets.test.ts, which cover its form/required clause and its FR37 cancel. CI has not run on this draft head yet.
[Automated review]
| | FR16 | An open modal sheet is a focus region through Core's shared focus-region module (`component:Dialog/DEC-2`): in the top sheet, Tab from the last tabbable control moves focus to the first and Shift+Tab from the first moves focus to the last, so focus never leaves the sheet for the browser's own interface. A covered or closing sheet does not answer Tab, and a non-modal sheet contains nothing (FR11). A switcher step (the deprecated BottomSheetSwitcher) is the exception to DEC-2, as `component:Dialog` DEC-2 states. | `component:Dialog/DEC-2` | | ||
| | FR17 | A BottomSheet open on the server, by its own `isOpen` or as a `BottomSheetSwitcher`'s active sheet, renders closed until hydration (no open styles, no `aria-modal`, the markup of a closed sheet) and opens in the render after hydration, the first that can enter its native host; it never claims a modality it does not have (`spec:AST-020` FR1, WCAG 2.2 SC 4.1.2). The server and hydrating renders produce the same markup (`spec:AST-013` FR9), and the open after hydration is settled, with no entry motion; later opens and closes animate as usual (`spec:AST-013` FR13). | `spec:AST-020` FR1 | | ||
| | FR18 | A finger's pull up past the tallest detent stretches the sheet on `spec:AST-094` FR3's rubber band, the same on every pull, with no hold. The sheet's surface reaches below the screen far enough for the whole stretch and the release, so the page under it never shows. A pull on the sheet's body stretches it as a pull on the grabber does, once the content has no room left to scroll that way: content that fits stretches at once, and a long list scrolls to its end first, then holds still while the sheet stretches. At every detent, a list that reaches its end mid-pull holds still while the sheet takes the rest of the pull. A sheet the caller sizes or styles keeps 48 px below the screen, and its stretch stops there. An inline block size, any `xstyle` and any class name count, because the sheet cannot read their rules. A mouse or a pen keeps its own resisted stretch, at most 48 px (`spec:AST-017` FR59). | DEC-3; `spec:AST-094` FR3 | | ||
| | FR19 | A release settles on the detent nearest its projected position (`spec:AST-094` FR36); a release whose velocity points back toward its start returns to its starting detent (`spec:AST-094` FR37). An `info` sheet whose projection passes the dismiss point closes; a `form` or `required` sheet settles on its shortest detent (FR7). Implementation gap: the settle plays `--ease-standard` over the theme's `--duration-medium`, at a speed set by the distance left rather than the finger's (`spec:AST-094` FR38). Implementation gap: in WebKit, the settle stops advancing while the main thread is blocked (`spec:AST-094` FR5). Implementation gap: a release whose velocity points back toward its start still commits when it follows a net downward drag and its projection passes the dismiss point, or when its projection lies nearer the next detent in the drag's direction (`spec:AST-094` FR37). | `spec:AST-094` FR5, FR36, FR37, FR38; FR7 | |
There was a problem hiding this comment.
Miss from the earlier reviews: the first sentence says a release settles at its spec:AST-094 FR36 projection, but the shipped sheet projects nothing after 48 px of travel or less (useSheetGestures.ts:1124-1127, PROJECTION_MIN_TRAVEL = 48; core's "does not flick-dismiss on a fast but short nudge" test pins it). On the Snap points story, a 32 px flick at 2,000 px/s (an FR36 projection of about 1,030 px) leaves the info sheet on its tall detent, while an 80 px flick at the same speed closes it. Suggest a fourth gap note:
Implementation gap: a release after 48 px of travel or less projects nothing: it settles on the detent nearest where the finger lifted, so a short fast flick never closes the sheet (`spec:AST-094` FR36).
…gainst AST-094 FR36 The verification row also names useSheetGestures.test.ts and snapOffsets.test.ts.
|
/approve-spec 739dae1 |
There was a problem hiding this comment.
Changes requested for one clause in FR19. The 48 px travel-floor gap note and the two unit-test files are in, and all six cases pass in Chromium at this head.
- FR19's new gap note (
docs/specs/components/BottomSheet/BottomSheet.spec.md:171), worded as the previous review suggested, says a short fast flick never closes the sheet, but a release under the floor is still checked against the dismiss point where the finger lifts (packages/core/src/BottomSheet/useSheetGestures.ts:1161), so a 40 px flick at 2,000 px/s from the documentedsnapPoints={['96px', '50%']}peek closes theinfosheet; please reword it, for example "a release after 48 px of travel or less projects nothing: it is judged where the finger lifted, so a short fast flick closes aninfosheet only when the finger itself lifts past the dismiss point (spec:AST-094FR36)".
CI has not run on this draft head yet.
[Automated review]
| | FR16 | An open modal sheet is a focus region through Core's shared focus-region module (`component:Dialog/DEC-2`): in the top sheet, Tab from the last tabbable control moves focus to the first and Shift+Tab from the first moves focus to the last, so focus never leaves the sheet for the browser's own interface. A covered or closing sheet does not answer Tab, and a non-modal sheet contains nothing (FR11). A switcher step (the deprecated BottomSheetSwitcher) is the exception to DEC-2, as `component:Dialog` DEC-2 states. | `component:Dialog/DEC-2` | | ||
| | FR17 | A BottomSheet open on the server, by its own `isOpen` or as a `BottomSheetSwitcher`'s active sheet, renders closed until hydration (no open styles, no `aria-modal`, the markup of a closed sheet) and opens in the render after hydration, the first that can enter its native host; it never claims a modality it does not have (`spec:AST-020` FR1, WCAG 2.2 SC 4.1.2). The server and hydrating renders produce the same markup (`spec:AST-013` FR9), and the open after hydration is settled, with no entry motion; later opens and closes animate as usual (`spec:AST-013` FR13). | `spec:AST-020` FR1 | | ||
| | FR18 | A finger's pull up past the tallest detent stretches the sheet on `spec:AST-094` FR3's rubber band, the same on every pull, with no hold. The sheet's surface reaches below the screen far enough for the whole stretch and the release, so the page under it never shows. A pull on the sheet's body stretches it as a pull on the grabber does, once the content has no room left to scroll that way: content that fits stretches at once, and a long list scrolls to its end first, then holds still while the sheet stretches. At every detent, a list that reaches its end mid-pull holds still while the sheet takes the rest of the pull. A sheet the caller sizes or styles keeps 48 px below the screen, and its stretch stops there. An inline block size, any `xstyle` and any class name count, because the sheet cannot read their rules. A mouse or a pen keeps its own resisted stretch, at most 48 px (`spec:AST-017` FR59). | DEC-3; `spec:AST-094` FR3 | | ||
| | FR19 | A release settles on the detent nearest its projected position (`spec:AST-094` FR36); a release whose velocity points back toward its start returns to its starting detent (`spec:AST-094` FR37). An `info` sheet whose projection passes the dismiss point closes; a `form` or `required` sheet settles on its shortest detent (FR7). Implementation gap: the settle plays `--ease-standard` over the theme's `--duration-medium`, at a speed set by the distance left rather than the finger's (`spec:AST-094` FR38). Implementation gap: in WebKit, the settle stops advancing while the main thread is blocked (`spec:AST-094` FR5). Implementation gap: a release whose velocity points back toward its start still commits when it follows a net downward drag and its projection passes the dismiss point, or when its projection lies nearer the next detent in the drag's direction (`spec:AST-094` FR37). Implementation gap: a release after 48 px of travel or less projects nothing: it settles on the detent nearest where the finger lifted, so a short fast flick never closes the sheet (`spec:AST-094` FR36). | `spec:AST-094` FR5, FR36, FR37, FR38; FR7 | |
There was a problem hiding this comment.
The last gap note overstates the floor, and the wording is the one the previous review suggested. A release under 48 px projects nothing, but it is still checked against the dismiss point where the finger lifts (useSheetGestures.ts:1161), and that point sits 40% of the shortest detent's visible height below it. On the documented snapPoints={['96px', '50%']} peek it is 38.4 px down: in real-touch Chromium a 2,000 px/s flick from the peek stays at 32 px of travel and closes the info sheet at 40 px and 44 px (a 40 px slow drag closes it too).
Suggested wording:
Implementation gap: a release after 48 px of travel or less projects nothing: it is judged where the finger lifted, so a short fast flick closes an `info` sheet only when the finger itself lifts past the dismiss point (`spec:AST-094` FR36).
…ere the finger lifted
|
/approve-spec 6e86aad |
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet, because the pull request is a draft; it runs once the pull request is marked ready for review.
FR19's travel-floor gap note now matches the shipped release, and all six detent cases pass in Chromium at this head, as do the unit tests FR19's verification row names.
[Automated review]
There was a problem hiding this comment.
Approved pending CI: the full CI re-run on this head, including the interaction legs that run the new spec, has not finished yet.
The merge from main leaves the pull request's own changes untouched, and all six detent cases still pass in Chromium against the updated interaction harness, as do the unit tests FR19's verification row names.
[Automated review]
…ttomsheet-detent-spec # Conflicts: # docs/specs/components/BottomSheet/BottomSheet.spec.md
|
/approve-spec 22808b4 |
github-actions
Bot
added
needs:spec-owner-review
There was a problem hiding this comment.
Approved pending CI: the full CI run on this head, including the interaction legs that run the new spec, has not finished yet.
component:BottomSheet FR19 carries an engineering owner's approval for this head, and the merge from main keeps both FR19 and the new FR20 rows intact. On a Storybook built from this head, all six detent cases pass in Chromium, and in WebKit four pass while the blocked-thread and rendering-trace cases skip as designed.
[Automated review]
cixzhang
added
the
ci:run
Supersedes #8423 by @vjeux: his three commits, cherry-picked onto today's
mainwith his authorship, plus a record line and two small follow-ups. Test-only: no library code, so there is no changeset.Maintainer impact
BottomSheet's detent motion had no frame evidence. This adds one component spec that uses each interaction-harness instrument on the sheet, whose settle exercises them all. The harness README and config name it as the spec to copy.
Current specification
On the "Snap points" story, under real touch input, the spec pins four outcomes. Each is already decided:
spec:AST-094FR36.infosheet closes this way (FR7), and the story's sheet isinfo.--ease-standardover the theme's--duration-medium, at a speed set by the distance left rather than the finger's.spec:AST-094FR5. This holds in Chromium. In WebKit the settle stops short of its detent while the main thread is blocked, so the spec skips that case in WebKit.component:BottomSheetgains FR19. It states the release rule (FR36's projection, FR37's cancel for a release moving back toward its start, and FR7'sinfo-only swipe dismissal) and carries four present-tense implementation-gap notes: the settle's curve against FR38; the WebKit stall against FR5; the releases moving back toward their start that still commit, against FR37; and the 48 px travel floor below which a release projects nothing, against FR36. The verification row names this spec,useSheetGestures.test.tsandsnapOffsets.test.ts, andverified_byadds this spec.Change
BottomSheetDetent.interaction.browser.spec.ts(vjeux): the four outcomes plus drag and settle storyboards. Each test names the harness instrument it uses.playwright.interaction.config.ts(vjeux): name this spec as the exemplar.uikit-parityannotation reads the settle's start speed from its first two sampled frames;testfrom@astryxdesign/a11y-spec/playwright, as every browser spec does;main, so that line was dropped.Evidence
All runs are real Chromium (phone viewport, touch) against Storybooks built from this branch.
topon the main thread instead of a compositedtransform):check:knowledgepass.Writing rubric results
The rubric in
docs/contributing/spec-writing.mdwas run overcomponent:BottomSheetFR19, its verification row andverified_by.