test(BottomSheet): check the handoff's scrim and focus with the handoff held by vjeux · Pull Request #9235 · meta/astryx

vjeux · 2026-11-01T00:17:47Z

Maintainer impact

Two handoff checks in BottomSheetNativeModal.a11y.browser.spec.ts depend on how fast the machine runs, even though the switcher is correct:

The scrim check samples the backdrop once per frame for 1.1 s and needs more than 20 frames.
The Tab check presses Tab through the roughly 650 ms handoff and needs more than 2 presses to land in it.

On a slower machine fewer fit. In WebKit with 2 CPUs and 16 workers, the scrim check fails 2 of 6 runs on main (8 and 15 samples). This came up while stress-testing the scroll-lock change in #8098 (spec:AST-094).

Intended invariant

Unchanged: one steady scrim through a handoff (no dip, no double dim); focus only in the entering sheet's dialog; the retained dialog inert and aria-hidden until it closes.

Change

Both tests hold the handoff phase by phase with the interaction harness instead of racing it:

Once the open sheet is at rest (waitForRest), panel transitions are stretched to 1000 s and the harness's timer gate freezes the page's timers, so the panels' transitionend backstop can't advance the handoff. A hold sent from the test arrives a round trip after a transition starts, and a 300 ms one can end before then; a stretched one is still near its start. Held transitions are read by progress, so their length doesn't change the reading.
Each phase is caught with watchAnimations on the panels that move in it and held with holdAnimations. releaseAnimations({finish: true}) moves the flow on.
The scrim test reads the combined dim at 0–100% of each phase (slide, then fade) in 5% steps with seekAnimations, and once after the retained dialog closes. It also fails if any ::backdrop transition ran or is running through the handoff, so a takeover that fades can't pass between two reads. WebKit reports such a transition on the dialog itself, with an empty pseudoElement, so an opacity transition on a sheet dialog counts too.
The Tab test presses Tab six times per held phase.
Both start the handoff with click() on Continue. A pointer press runs its own frames and timers, which the gate would hold.
The harness's @position and README now say an accessibility spec may use its holds and timer gate.

The assertions are unchanged.

Evidence

Failure before the change: on main in WebKit with 2 CPUs and 16 workers (×3, two runs), the scrim check failed 2 of 6 runs. With 3 CPUs and 16 workers, and in the CI WebKit job's profile, it passes. With a takeover forced to fade over 410 ms, earlier heads of this PR still passed: in WebKit when the read landed 160, 200 or 250 ms late, and in Chromium at 200 or 250 ms.
Success after the change: at 771358b the file passed every run:
- WebKit, the CI job's profile (4 CPUs, CI=1, 1 worker, ×10): 40/40.
- WebKit, 3 CPUs and 1 worker (×3): 12/12.
- WebKit, 3 CPUs and 16 workers (×3): 12/12.
- WebKit, 2 CPUs and 16 workers (×3, two runs): 24/24.
- Chromium on a 2-core pin with 4 workers (×3): 12/12.
Still red on real defects:
- A forced double dim (0.75 > 0.525).
- A forced dip (a backdrop transition runs).
- A takeover fading over 410 ms, read at once or 160, 200 or 250 ms late, in both Chromium and WebKit (a backdrop transition is running).
- An entering sheet made inert (focus on body).
- A retained dialog without inert (the slide's first Tab).
Product/runtime behavior verified unchanged: test-only, no source files touched.

Scope

No intended public API, product behavior, visual direction, or policy change.
Product changes discovered during the work were removed or split.
Public text and artifacts contain no internal Meta context.

Testing

Local gates at 771358b:

Prettier and CI-mode eslint pass on the changed files, and the core and harness type checks pass.
The changeset coverage, changeset, SYNC and knowledge checks pass, and the build and Storybook build succeed.
The runs listed above.

It merges cleanly with main.

vercel · 2026-11-01T00:17:52Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
astryx	Ready	Preview	Nov 1, 2026 3:43am UTC

astracat-bot · 2026-11-01T00:17:58Z

AI review status for this pull request.

Review status	Updated
🟢 Reviewed (for maintainers only)	Nov 1, 2026, 3:54 AM UTC

github-actions · 2026-11-01T00:19:38Z

PR Analysis Report

🔎 Preview · `771358b`

Storybook · Sandbox · built from this PR's head

Modified components

Component	Size	LoC
BottomSheet	33.8 kB (0) · 7.4 kB CSS (0)	—
BottomSheetSwitcher	14.9 kB (0) · 3.4 kB CSS (0)	—

Package size: no published package changed; no new dependencies.

Not run on this PR: add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.

_{Generated by PR Enrichment workflow | View full report}

astracat-bot

Changes requested: holding the handoff phase by phase is the right fix for these flaky checks, but two parts need another pass.

The scrim takeover after the retained dialog closes is still read once on the real clock (lines 577–578), so please hold or assert the dialogs' ::backdrop transitions through it, since a takeover that fades on the 410 ms scrim curve passes once that read lands about 160 ms late.
Please reuse waitForRest, holdAnimations, seekAnimations and releaseAnimations from internal/interaction-harness/src/curves.ts (extending them if pausing at transitionrun is needed) instead of the page-side versions, and update the harness's @position and README now that an accessibility spec depends on it.

[Automated review]

astracat-bot · 2026-11-01T00:36:57Z

+  await expect(page.locator('dialog[open]')).toHaveCount(1);
+  const after = await combinedDim();


This is the only read of the takeover, and it runs on the real clock: the dialogs' ::backdrop transition is neither held nor stretched (only .astryx-bottom-sheet is). If the takeover lost its 0 s pin, the entering backdrop would fade in over the 410 ms --duration-medium curve and be back above steady * 0.9 about 160 ms in, so this check catches it only when the read lands sooner. Could the takeover be held or stepped by progress too, or could the test assert that no ::backdrop transition is running on an open sheet dialog here?

astracat-bot · 2026-11-01T00:36:57Z

+ * transition by its progress, so its length doesn't change what they read.
+ * Also installs the page-side reads `nextHandoffPhase` and `combinedDim` use.
+ */
+async function holdHandoff(page: Page) {


@astryxdesign/interaction-harness already covers this flow: waitForRest, holdAnimations, seekAnimations (with END_INSET_MS) and releaseAnimations({finish: true}) in curves.ts. The harness hold also takes ::backdrop transitions, which this copy misses. Could the test call those helpers, and extend holdAnimations if pausing at transitionrun really is needed?

vjeux · 2026-11-01T01:57:49Z

Both points are addressed at c072a18:

The takeover. The scrim check now fails if any ::backdrop transition ran (transitionrun) or is still running (document.getAnimations()) through the handoff, so a takeover that fades fails however late the read lands. I forced a takeover to fade over 410 ms: it fails read at once, and read 160, 200 or 250 ms late. The previous head passed the 200 and 250 ms reads.
The harness. The holds now come from curves.ts as main has it: waitForRest, then per phase watchAnimations and holdAnimations, seekAnimations for the 5% steps, and releaseAnimations({finish: true}). No transitionrun pause was needed. Panel transitions are stretched to 1000 s, so one held a round trip after it starts is still near its start. The harness's @position and README now name accessibility specs.

astracat-bot

Changes requested: holding the handoff phase by phase with the interaction harness makes both checks deterministic, and a double dim, a retained dialog without inert and an inert entering dialog now fail in both Chromium and WebKit. One gap remains: the takeover guard keeps only transitions whose pseudoElement is '::backdrop' (lines 314 and 549), but WebKit reports an empty pseudoElement for ::backdrop transitions, so in WebKit a takeover that fades still passes once the read at line 542 lands about 160 ms after the close. Please also count an opacity transition targeted at the sheet dialog in both the transitionrun listener and the getAnimations() scan; with that change a forced 410 ms takeover fade fails at every read delay in both engines.

[Automated review]

astracat-bot · 2026-11-01T02:20:20Z

+    document.addEventListener(
+      'transitionrun',
+      event => {
+        if (event.pseudoElement === '::backdrop') {


WebKit dispatches a ::backdrop transition's transitionrun on the dialog with pseudoElement === '', so this never records it there: a forced 410 ms takeover fade passes in WebKit once the read at line 542 lands about 160 ms late. Counting the dialog's own opacity transition too closes it in both engines (these dialogs run no opacity transition of their own):

if ( event.pseudoElement === '::backdrop' || (event.target instanceof HTMLDialogElement && event.propertyName === 'opacity') ) {

astracat-bot · 2026-11-01T02:20:20Z

+    ...(window as unknown as {__backdropRuns: string[]}).__backdropRuns,
+    ...document.getAnimations().flatMap(animation => {
+      const effect = animation.effect as KeyframeEffect | null;
+      return effect?.pseudoElement === '::backdrop'


Same in the scan: WebKit's KeyframeEffect.pseudoElement is '' for the running ::backdrop transition, so it is dropped here as well. Matching effect?.target instanceof HTMLDialogElement with transitionProperty === 'opacity' alongside the '::backdrop' check catches it.

…ff held The scrim and Tab checks for a switcher handoff in BottomSheetNativeModal.a11y.browser.spec.ts counted frames and Tab presses inside a handoff running in wall time. Both now hold each handoff phase with the interaction harness (sheet panel transitions stretched, held with holdAnimations and read with seekAnimations; the page's timers frozen) and read it at fixed points: the scrim at 0-100% of each phase in 5% steps, Tab six times per phase. The scrim check also fails if any backdrop transition runs through the handoff, so a takeover that fades can't pass between two reads. The harness's entry point and README say an accessibility spec may use its holds now.

vjeux · 2026-11-01T03:35:40Z

Fixed at 771358b. Both the transitionrun listener and the getAnimations() scan now also count an opacity transition targeted at a sheet dialog, where WebKit reports a ::backdrop transition with an empty pseudoElement.

With a takeover forced to fade over 410 ms, the check now fails in both Chromium and WebKit whether the read lands at once or 160, 200 or 250 ms late. At c072a18, WebKit passed the 160, 200 and 250 ms reads. The unmutated test still passes in both engines.

astracat-bot

Approval recommended: holding the handoff phase by phase with the interaction harness makes both checks independent of machine speed, and the widened guard now fails a takeover that fades in WebKit as well as Chromium. A forced double dim, a fading takeover read up to 600 ms late, a retained dialog without inert and an inert entering dialog each fail the new tests in both engines, so no further changes are needed.

[Automated review]

meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Nov 1, 2026

vercel Bot deployed to Preview November 1, 2026 00:30

View deployment

astracat-bot Bot requested changes Nov 1, 2026

View reviewed changes

cixzhang added the ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) label Nov 1, 2026

cixzhang marked this pull request as ready for review November 1, 2026 00:51

cixzhang requested review from cixzhang, imdreamrunner and josephfarina as code owners November 1, 2026 00:51

vjeux force-pushed the uikit/bottom-sheet-handoff-held-checks branch from efdb459 to c072a18 Compare November 1, 2026 01:57

vercel Bot deployed to Preview November 1, 2026 02:04

View deployment

astracat-bot Bot requested changes Nov 1, 2026

View reviewed changes

vjeux force-pushed the uikit/bottom-sheet-handoff-held-checks branch from c072a18 to 771358b Compare November 1, 2026 03:35

vercel Bot deployed to Preview November 1, 2026 03:43

View deployment

astracat-bot Bot approved these changes Nov 1, 2026

View reviewed changes

cixzhang merged commit efb6b30 into meta:main Nov 1, 2026
52 checks passed

vjeux deleted the uikit/bottom-sheet-handoff-held-checks branch November 1, 2026 03:57

vjeux mentioned this pull request Nov 1, 2026

test(interaction-harness): a hold the page arms itself takes a short animation on its first frame #9230

Merged

3 tasks done

		await expect(page.locator('dialog[open]')).toHaveCount(1);
		const after = await combinedDim();

Conversation

vjeux commented Nov 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Maintainer impact

Intended invariant

Change

Evidence

Scope

Testing

Uh oh!

vercel Bot commented Nov 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astracat-bot Bot commented Nov 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Nov 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Analysis Report

🔎 Preview · 771358b

Modified components

Uh oh!

astracat-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

astracat-bot Bot Nov 1, 2026

Choose a reason for hiding this comment

Uh oh!

astracat-bot Bot Nov 1, 2026

Choose a reason for hiding this comment

Uh oh!

vjeux commented Nov 1, 2026

Uh oh!

astracat-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

astracat-bot Bot Nov 1, 2026

Choose a reason for hiding this comment

Uh oh!

astracat-bot Bot Nov 1, 2026

Choose a reason for hiding this comment

Uh oh!

vjeux commented Nov 1, 2026

Uh oh!

astracat-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vjeux commented Nov 1, 2026 •

edited

Loading

vercel Bot commented Nov 1, 2026 •

edited

Loading

astracat-bot Bot commented Nov 1, 2026 •

edited

Loading

github-actions Bot commented Nov 1, 2026 •

edited

Loading

🔎 Preview · `771358b`