Skip to content

test(BottomSheet): the settle proofs freeze the scene at the lift - #10098

Merged
cixzhang merged 2 commits into
meta:mainfrom
vjeux:bottomsheet-storyboard-freeze-lift
Nov 5, 2026
Merged

cixzhang merged 2 commits into
meta:mainfrom
vjeux:bottomsheet-storyboard-freeze-lift

Conversation

@vjeux

@vjeux vjeux commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Maintainer impact

BottomSheetDetent's "storyboard: drag to the half detent and settle" fails on busy runners with storyboard bottomsheet-settle: no running animation to hold. In Chromium pinned to two CPUs with 4 workers, main failed 4 of 20. It also failed once on a shared machine at one worker. The recorded settle curve ("the settle lands on the detent, its curve recorded frame by frame") holds the settle the same late way. With its hold 600 ms late, it fails every time.

Intended invariant

component:BottomSheet FR19 and its verification row: the settle to a detent with its curve recorded, and the storyboard of the drag and the settle. What the two proofs assert is unchanged.

Change

Both proofs held the settle with a hold that the test sent after the lift. Before it, the test waited to see the settle start. The settle lasts about 300 ms, so on a busy runner it could end before the hold arrived. The page now freezes the scene in the lift's own task, before the sheet hears the lift. It calls installPageFreeze before the story loads, then freezePageOn(page, 'touchend') before the drag. The settle then starts held, and its timers stay held. thawPage replaces thawTimers. The settle-curve proof now installs the timer gate too, which a freeze needs. Its evidence file keeps the held transition's timing. One test file changes.

Evidence

One machine and one build (Storybook at main 1dce318). Chromium, no retries; each failure recorded by test, error and duration. Counts are under Testing.

  • Failure before the change: pinned to two CPUs with 4 workers, main's storyboard failed 4 of 20 (no running animation to hold). With each proof's hold made 600 ms late, both of main's proofs failed 5 of 5.
  • Success after the change: in the same runs, the frozen lift passed every time, pinned, alone and with the pause. The whole spec file passes in Chromium and WebKit.
  • Product/runtime behavior verified unchanged: the diff is one test file.
  • CI impact: none; no workflow or CI script changes.

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

# Chromium pinned to two CPUs, 4 workers
taskset -c <two CPUs> npx playwright test -c playwright.interaction.config.ts \
  --project=chromium --workers=4 --retries=0 --repeat-each=10 \
  -g "drag to the half detent and settle" \
  packages/core/src/BottomSheet/__tests__/BottomSheetDetent.interaction.browser.spec.ts
# The same for the settle curve: -g "the settle lands on the detent"
# The same, unpinned at one worker, at the same moment
taskset -c <other CPUs> npx playwright test … --workers=1 (same filters)
# The late-hold controls
… --workers=1 --repeat-each=5 -g "after a 600 ms pause"

Counts, both versions interleaved in one run:

  • Storyboard, pinned: main failed 4 of 20, this change 0 of 20. Unpinned at one worker, at the same moment: 0 of 20 each.
  • Settle curve, pinned and unpinned: 10 of 10 each, for main and this change.
  • Late-hold control: main failed 5 of 5 for each proof (no running animation to hold, no transform transition held). This change passed 5 of 5 for each.
  • Whole file on this change: Chromium (pinned to two CPUs, one worker) 6 of 6. WebKit (CI container profile) 4 passed, with its 2 skips (the blocked-main-thread settle and the Chromium-only rendering trace).

The arms above are in a commit not for landing, bf1c42d259, on top of this change. It adds main's two proofs under their own titles. It also adds both versions of each with the hold made 600 ms late.

@vercel

vercel Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 5, 2026 8:27pm UTC

Request Review

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

astracat-bot Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 5, 2026, 8:34 PM UTC

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · 77efddf

Storybook · Sandbox · built from this PR's head

Modified components

Component Size LoC Visual · A11y · RTL
BottomSheet 56.9 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 19 rows, core 23 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

The storyboard and the recorded settle curve held the settle with a
hold sent after the drag's lift. The settle lasts about 300 ms, so on
a busy runner it could end before the hold arrived ("storyboard
bottomsheet-settle: no running animation to hold"). The page now
freezes the scene in the lift's own task, before the sheet hears the
lift, so the settle starts held. What the proofs assert is unchanged.
@vjeux
vjeux force-pushed the bottomsheet-storyboard-freeze-lift branch from 2918f09 to d68e7d7 Compare November 5, 2026 16:22
@vjeux vjeux changed the title test(BottomSheet): the storyboard freezes the scene at the lift test(BottomSheet): the settle proofs freeze the scene at the lift Nov 5, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending CI: CI has not run on this head yet.

Both BottomSheet settle proofs now freeze the page on the lift, so the settle starts held instead of racing a hold sent from the test, and what they assert and record is unchanged. With the hold made 600 ms late, the current proofs fail every time and these pass, in Chromium and WebKit. Next step: this is a draft from a fork, so mark it ready for review and have a maintainer add the ci:run label so CI runs.

  • Non-blocking: the description says FR19's verification row covers the storyboard, but that row lists the settle curve and no storyboard; naming the spec file there would be accurate.

[Automated review]

@vjeux
vjeux marked this pull request as ready for review November 5, 2026 16:44
@cixzhang cixzhang added the ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) label Nov 5, 2026
@cixzhang
cixzhang marked this pull request as draft November 5, 2026 16:45
@cixzhang
cixzhang marked this pull request as ready for review November 5, 2026 16:45
@cixzhang
cixzhang marked this pull request as draft November 5, 2026 16:46
@cixzhang
cixzhang marked this pull request as ready for review November 5, 2026 16:46

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending CI: CI is still running on this head.

Both BottomSheet settle proofs now freeze the page on the lift, so the settle starts held instead of racing a hold sent from the test, and what they assert and record is unchanged. With the hold made 600 ms late, the current proofs fail every time and these pass; the spec file passes in Chromium and WebKit, including in CI's interaction legs.

  • Non-blocking: the description says FR19's verification row covers the storyboard, but that row lists the settle curve and no storyboard; naming the spec file there would be accurate.

[Automated review]

@cixzhang
cixzhang merged commit d6efbde into meta:main Nov 5, 2026
66 checks passed
@vjeux
vjeux deleted the bottomsheet-storyboard-freeze-lift branch November 5, 2026 23:40

This branch was successfully deployed

1 active deployment
Preview — 77efddf4 Deployed Nov 5, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants