Repository navigation
fix(Spinner): a turning ring wakes the main thread once in 100 turns - #9936
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
Contributor
|
AI review status for this pull request.
|
Contributor
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| BaseTypeahead | 55.3 kB (+1 B) · 9.7 kB CSS (+22 B) | — | — · — · — |
| Spinner · Storybook | 18.5 kB (+1 B) · 5.7 kB CSS (+15 B) | 322 (+1) | — · — · — |
| DateTimeInput | 173.2 kB (+7 B) · 17.2 kB CSS (+8 B) | — | — · — · — |
83 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| Toast | 59.6 kB (+4 B) · 9.0 kB CSS (+11 B) | — | — · — · — |
| Carousel | 49.7 kB (+4 B) · 9.5 kB CSS (+10 B) | — | — · — · — |
| ChatSendButton | 45.7 kB (+1 B) · 8.7 kB CSS (+13 B) | — | — · — · — |
| IconButton | 45.3 kB (+1 B) · 8.7 kB CSS (+13 B) | — | — · — · — |
| NumberInput | 55.1 kB (+2 B) · 10.2 kB CSS (+12 B) | — | — · — · — |
| SideNav | 71.5 kB (0) · 10.1 kB CSS (+14 B) | — | — · — · — |
| SideNavCollapseButton | 46.1 kB (+1 B) · 8.8 kB CSS (+13 B) | — | — · — · — |
| TextInput | 53.0 kB (+2 B) · 9.9 kB CSS (+12 B) | — | — · — · — |
| Banner | 47.2 kB (+1 B) · 9.2 kB CSS (+12 B) | — | — · — · — |
| BreadcrumbItem | 68.7 kB (+1 B) · 10.9 kB CSS (+12 B) | — | — · — · — |
| ChatLayout | 48.4 kB (0) · 9.3 kB CSS (+13 B) | — | — · — · — |
| ChatLayoutScrollButton | 45.8 kB (0) · 8.9 kB CSS (+13 B) | — | — · — · — |
| CodeBlock | 59.1 kB (0) · 10.0 kB CSS (+13 B) | — | — · — · — |
| DateRangeInput | 71.3 kB (+1 B) · 11.2 kB CSS (+12 B) | — | — · — · — |
| FileInput | 53.8 kB (0) · 9.7 kB CSS (+13 B) | — | — · — · — |
| MobileNavToggle | 45.6 kB (0) · 8.7 kB CSS (+13 B) | — | — · — · — |
| SideNavItem | 54.8 kB (+2 B) · 9.8 kB CSS (+11 B) | — | — · — · — |
| Stepper | 46.8 kB (+1 B) · 8.9 kB CSS (+12 B) | — | — · — · — |
| ToggleButton | 46.1 kB (0) · 8.8 kB CSS (+13 B) | — | — · — · — |
| Tokenizer | 69.9 kB (+3 B) · 17.1 kB CSS (+10 B) | — | — · — · — |
| Button | 45.3 kB (−1 B) · 8.7 kB CSS (+13 B) | — | — · — · — |
| Calendar | 54.4 kB (0) · 9.3 kB CSS (+12 B) | — | — · — · — |
| ChatComposer | 61.5 kB (+1 B) · 10.6 kB CSS (+11 B) | — | — · — · — |
| ChatComposerInput | 59.4 kB (0) · 10.0 kB CSS (+12 B) | — | — · — · — |
| ChatComposerTokenElement | 59.4 kB (0) · 10.0 kB CSS (+12 B) | — | — · — · — |
| ChatDictationButton | 45.8 kB (−1 B) · 8.8 kB CSS (+13 B) | — | — · — · — |
| DialogHeader | 48.1 kB (+1 B) · 10.2 kB CSS (+11 B) | — | — · — · — |
| PowerSearch | 241.3 kB (+5 B) · 21.9 kB CSS (+7 B) | — | — · — · — |
| Selector | 78.5 kB (+2 B) · 15.8 kB CSS (+10 B) | — | — · — · — |
| SideNavHeading | 59.8 kB (+1 B) · 10.0 kB CSS (+11 B) | — | — · — · — |
| TableSelectionToolbar | 54.8 kB (+2 B) · 11.8 kB CSS (+10 B) | — | — · — · — |
| TextArea | 48.2 kB (+3 B) · 8.8 kB CSS (+9 B) | — | — · — · — |
| Timestamp | 21.3 kB (0) · 8.9 kB CSS (+12 B) | — | — · — · — |
| TopNavMegaMenu | 54.3 kB (+1 B) · 10.6 kB CSS (+11 B) | — | — · — · — |
| TopNavMenu | 59.1 kB (+1 B) · 9.9 kB CSS (+11 B) | — | — · — · — |
| Typeahead | 63.2 kB (0) · 11.2 kB CSS (+12 B) | — | — · — · — |
| DrawerHeader | 48.3 kB (+2 B) · 11.6 kB CSS (+9 B) | — | — · — · — |
| LayerProvider | 63.3 kB (−1 B) · 9.4 kB CSS (+12 B) | — | — · — · — |
| Markdown | 145.5 kB (−6 B) · 18.5 kB CSS (+17 B) | — | — · — · — |
| MobileNav | 52.2 kB (+2 B) · 9.3 kB CSS (+9 B) | — | — · — · — |
| AppShell | 71.2 kB (+1 B) · 12.2 kB CSS (+9 B) | — | — · — · — |
| ChatToolCalls | 25.9 kB (0) · 8.2 kB CSS (+10 B) | — | — · — · — |
| DateInput | 151.0 kB (+1 B) · 15.5 kB CSS (+9 B) | — | — · — · — |
| Lightbox | 52.5 kB (−2 B) · 9.1 kB CSS (+12 B) | — | — · — · — |
| TimeInput | 137.2 kB (+1 B) · 14.5 kB CSS (+9 B) | — | — · — · — |
| TopNav | 53.7 kB (0) · 9.6 kB CSS (+10 B) | — | — · — · — |
| TopNavHeading | 59.2 kB (−1 B) · 9.8 kB CSS (+11 B) | — | — · — · — |
| AlertDialog | 83.8 kB (+1 B) · 13.1 kB CSS (+8 B) | — | — · — · — |
| ComplexSelector | 55.7 kB (0) · 10.5 kB CSS (+9 B) | — | — · — · — |
| MultiSelector | 73.1 kB (0) · 15.3 kB CSS (+9 B) | — | — · — · — |
| Popover | 52.0 kB (+1 B) · 12.8 kB CSS (+8 B) | — | — · — · — |
| Switch | 87.4 kB (0) · 9.5 kB CSS (+9 B) | — | — · — · — |
| Table | 84.2 kB (+2 B) · 16.2 kB CSS (+7 B) | — | — · — · — |
| TabMenu | 56.5 kB (−1 B) · 9.9 kB CSS (+10 B) | — | — · — · — |
| Thumbnail | 49.6 kB (+1 B) · 10.3 kB CSS (+8 B) | — | — · — · — |
| ContextMenu | 74.9 kB (+1 B) · 15.5 kB CSS (+7 B) | — | — · — · — |
| TableHeaderCell | 75.6 kB (+1 B) · 15.7 kB CSS (+7 B) | — | — · — · — |
| ChatMessageList | 22.8 kB (0) · 6.9 kB CSS (+7 B) | — | — · — · — |
| Pagination | 87.3 kB (+1 B) · 16.1 kB CSS (+6 B) | — | — · — · — |
| TableCell | 76.1 kB (+2 B) · 16.0 kB CSS (+5 B) | — | — · — · — |
| CommandPaletteInput | 26.9 kB (0) · 7.4 kB CSS (+6 B) | — | — · — · — |
| DropdownMenu | 73.5 kB (−1 B) · 15.7 kB CSS (+7 B) | — | — · — · — |
| MoreMenu | 73.8 kB (−1 B) · 15.7 kB CSS (+7 B) | — | — · — · — |
| CheckboxInput | 48.7 kB (−2 B) · 8.7 kB CSS (+7 B) | — | — · — · — |
| DropdownMenuSubMenu | 57.4 kB (0) · 8.9 kB CSS (+4 B) | — | — · — · — |
| ListItem | 52.8 kB (+1 B) · 10.0 kB CSS (+3 B) | — | — · — · — |
| lab/EmojiPicker | 70.7 kB (+3 B) · 722 B CSS (0) | — | — · — · — |
| CheckboxListItem | 53.5 kB (−1 B) · 10.0 kB CSS (+3 B) | — | — · — · — |
| CommandPalette | 51.0 kB (0) · 12.3 kB CSS (+2 B) | — | — · — · — |
| richtext/RichTextEditor | 278.3 kB (+2 B) · 2.6 kB CSS (0) | — | — · — · — |
| richtext/RichTextEditorToolbar | 225.4 kB (+2 B) · 454 B CSS (0) | — | — · — · — |
| lab/ChatEmojiPicker | 65.9 kB (+1 B) · 773 B CSS (0) | — | — · — · — |
| lab/ChatReactionBar | 66.4 kB (+1 B) · 1.1 kB CSS (0) | — | — · — · — |
| lab/CommentThread | 256.3 kB (+1 B) · 2.4 kB CSS (0) | — | — · — · — |
| lab/TourStep | 58.0 kB (+1 B) · 590 B CSS (0) | — | — · — · — |
| lab/TransferListSelector | 82.9 kB (+1 B) · 1.9 kB CSS (0) | — | — · — · — |
| lab/CommentComposer | 104.8 kB (0) · 1.9 kB CSS (0) | — | — · — · — |
| lab/DiffViewer | 71.2 kB (0) · 2.1 kB CSS (0) | — | — · — · — |
| lab/Schedule | 60.6 kB (0) · 3.9 kB CSS (0) | — | — · — · — |
| lab/TransferList | 75.2 kB (0) · 1.7 kB CSS (0) | — | — · — · — |
| richtext/RichTextView | 258.8 kB (0) · 2.3 kB CSS (0) | — | — · — · — |
| lab/Comments | 261.6 kB (−4 B) · 2.5 kB CSS (0) | — | — · — · — |
| lab/ListInput | 81.9 kB (−6 B) · 1.6 kB CSS (0) | — | — · — · — |
Package size: @astryxdesign/core 6.39 MB packed (+305 B); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 8 rows, core 2 rows; run pnpm api:report.
Wide change (shared styles: packages/core/src/theme/themeMotion.test.ts): add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.
Generated by PR Enrichment workflow | View full report
cixzhang
marked this pull request as ready for review
November 4, 2026 11:34
cixzhang
force-pushed
the
fix/spinner-iteration-wakes
branch
from
November 4, 2026 13:18
7179879 to
2f89b74
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Changes requested: one wording fix before approval. The rotor change keeps the ring's look, period, phase and reduced-motion rate, and the themeMotion.test.ts guard now passes.
- The changeset (
.changeset/spinner-iteration-wakes.md:5) and the title say a turning ring no longer wakes the main thread and that Chromium recalculated style about seven times a second for each spinning ring, but the ring still crosses an iteration boundary, and wakes the main thread, once every 100 turns (every 52.5 s at a 525 ms turn, every 300 s under reduced motion), and rings in phase share each boundary (your table shows 13.9/s for 20 rings), so please reword both to say the wake drops from every turn to once in 100 turns.
The comment at Spinner.tsx:219-220 repeats the per-ring figure; a non-blocking rewrite is suggested inline.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] Spinner: a turning ring no longer wakes the main thread. In a React app, Chromium recalculated style about seven times a second for each spinning ring. Now it does no main-thread work while the ring turns. The ring looks and turns the same, and reduced motion still slows it to 3 seconds a turn. |
Contributor
There was a problem hiding this comment.
With 100 turns in one iteration, the ring still crosses an iteration boundary, and wakes the main thread, once every 100 turns: every 52.5 s at a 525 ms turn, every 300 s under reduced motion. Rings turn in phase and share each boundary, so the cost is not per ring (the PR's table shows 7.3/s for one ring and 13.9/s for 20). Suggested wording:
[fix] Spinner: a turning ring now wakes the main thread once every 100 turns instead of every turn. In a React app, Chromium recalculated style at the end of each turn, about seven times a second with one ring at a 525 ms turn. The ring looks and turns the same, and reduced motion still slows it to 3 seconds a turn.
The title needs the same change, for example fix(Spinner): a turning ring wakes the main thread once in 100 turns.
Comment on lines
+217
to
+222
| * Chromium wakes the main thread at each iteration boundary of a CSS | ||
| * animation when the document listens for `animationiteration`. React | ||
| * listens for it on every root. With one turn per iteration, each ring causes | ||
| * a burst of style recalculations about seven times a second. With 100 turns, | ||
| * that burst occurs once in 100 turns. Each turn is a full 360 degrees, so the | ||
| * ring looks the same at every moment. |
Contributor
There was a problem hiding this comment.
Non-blocking: a burst comes once per turn, about twice a second at a 525 ms turn, not seven times a second, and rings in phase share it. A shorter version that keeps only the reason:
* Chromium wakes the main thread at each iteration boundary while the
* document listens for `animationiteration`. React listens for it on every
* root. One iteration holds 100 turns, so the wake comes once in 100 turns.
* Each turn is 360 degrees, so the ring looks the same.| willChange: 'transform', | ||
| // Reduced motion slows the turn but never stops it: a still ring reads as | ||
| // broken. The status role and its name still say that work is busy. | ||
| // Each duration is one turn times the turns in one iteration. |
Contributor
There was a problem hiding this comment.
Non-blocking: this restates the calc() below it; it can go.
cixzhang
marked this pull request as draft
November 4, 2026 13:43
cixzhang
marked this pull request as ready for review
November 4, 2026 13:48
cixzhang
force-pushed
the
fix/spinner-iteration-wakes
branch
from
November 4, 2026 14:09
2f89b74 to
e2d3119
Compare
cixzhang
changed the title
cixzhang
marked this pull request as draft
November 4, 2026 14:09
cixzhang
marked this pull request as ready for review
November 4, 2026 14:10
Contributor
There was a problem hiding this comment.
Approved: the changeset and the title now say the main-thread wake drops from every turn to once in 100 turns, which matches the change. The ring's look, period, phase and reduced-motion rate are unchanged, and the unit suites and the changed Chromium spec pass on this head.
[Automated review]
11 tasks done
cixzhang
added a commit
to korkt-kim/astryx
that referenced
this pull request
Nov 5, 2026
Resolves the conflicts with meta#8177, meta#9936 and meta#8256 (Spinner's rotor, wake-once-per-100-turns and CircularProgress) and the adopters test's PowerSearch and InfoTip changes. The Spinner record takes main's FR3, system_specs and verification rows, with this branch's approved "indicator" wording, AST-017/AST-020 additions and announcement notes applied onto them.
cixzhang
pushed a commit
that referenced
this pull request
Nov 5, 2026
…t channel (#7910) * fix(Spinner): share rendered-text announcements Use the shared rendered-text announcement path for Spinner while keeping composed indicators silent. Let Lab loading owners announce busy starts, including the Comments history dialog through its shared modal channel. Fixes #7855 Co-authored-by: Cindy Zhang <cindyxz@meta.com> * Merge main to resolve PR #7910 conflicts Preserve Spinner's rendered-text fallback alongside owner-provided announcers, keep direct module imports, and retain Comments' persistent error region together with its busy-start announcement. * refactor(lab): reuse status announcements for busy loaders * Merge main to resolve PR #7910 conflicts Carry the approved Spinner contract to docs/specs/components while preserving the existing announcement changes and upstream Typeahead and Tokenizer updates. * Merge main into korkt-kim/fix-issue-7855 Resolves the conflicts with #8177, #9936 and #8256 (Spinner's rotor, wake-once-per-100-turns and CircularProgress) and the adopters test's PowerSearch and InfoTip changes. The Spinner record takes main's FR3, system_specs and verification rows, with this branch's approved "indicator" wording, AST-017/AST-020 additions and announcement notes applied onto them. * chore: comments give their reasons without record citations The comments this branch added cite knowledge records (spec IDs and FR numbers), which strict lint's no-comment-provenance rule rejects. Each now keeps only the reason it gives; the records still hold the requirements. Comments only: no code changes. * chore(lab): remove the old useStatusAnnouncement left by the merge This branch moved packages/lab/src/Comments/useStatusAnnouncement.ts to packages/lab/src/useStatusAnnouncement.ts; the merge of main brought the old copy back. Nothing imports it.
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.
User impact
Every React app that shows a Core
Spinnerpaid a small, constant main-thread cost while the ring turned. In Chromium, a turning ring caused a short burst of style recalculations at the end of every turn (about seven recalculations a second at a 525 ms turn), even though the turn itself runs on the compositor. Rings in phase share each burst. On a page with many spinners, or inside a moving layer such as a pull-to-refresh band, that is wasted CPU and battery, and style work competes with input and scrolling. After this change, that wake comes once every 100 turns (every 52.5 s, or 300 s under reduced motion) instead of every turn. The ring looks and turns exactly as before.Problem and solution fit
animationiterationlistener, so it can dispatch the event. React registers that listener on every root. Spinner's rotation was one turn per iteration (525 ms), so every turn crossed a boundary.The fix keeps the same compositor-only
transformanimation and puts 100 full turns in one iteration, with 100 times the duration. Each turn still takes one period (--duration-slow-minbefore tempo, or 3 s under reduced motion). A full turn is 360 degrees, so every frame is the same as before. The boundary, and its wake, now occurs once every 100 turns.Expected behavior and authority
No behavior change: the ring's look, rate, direction, phase and reduced-motion rate are the same.
component:SpinnerFR3 (one cycle per--duration-slow-min, linear, indefinite; 3 s underprefers-reduced-motion: reduce, never stopped) and FR4 (all rings in phase) are unchanged. The record's motion check "a ring that paints every frame" is extended: the Chromium spec now also requires zero style recalculations while rings turn.Smallest restoration
Two values in
Spinner.tsx: the keyframes end atrotate(36000deg)instead ofrotate(360deg), andanimation-durationis multiplied by the same 100 turns, for both the default and the reduced-motion duration. No JavaScript, state, DOM, props, theme variables, or timing changes.Evidence
Real Chromium (Playwright, M5 Mac), Storybook built from
mainvs this branch,core-spinner--default, 10 s windows, idle page. 20 rings are DOM copies of the first; the copies are not phase-pinned, so they form a second boundary group, which is why "before" doubles. Windows contain no 100-turn boundary.Cause, isolated in plain documents (one 525 ms CSS rotation, nothing else): no listener 0 recalcs/s; an
animationstart/animationendlistener 0/s; ananimationiterationlistener 7.2/s. A Web Animations rotation, which fires no CSS events, 0/s.Pull-to-refresh demo (
Lab/Pull to refreshControlled story from #9865, held refreshing, Spinner visible, 5 s): style recalcs 4.4–4.6/s before, 0.4/s after (the remaining 0.4/s is the story's own layout, unchanged by the Spinner); CPU 0.13–0.20% before, 0.03–0.04% after. The "after" here applies this branch's two rotor values to that preview.Visual parity. Screenshots of
core-spinner--sizes(sm, md, lg, xl) andcore-spinner--shades(default, subtle, onMedia, inherit), with every animation paused at the same phase: 0/8 … 7/8 of a turn and 1 ms before a turn ends. This branch was sampled in the first turn, turn 57, and the last turn before the iteration wraps;mainin its only turn. All 54 frames are byte-identical tomain's frame at the same phase (9 distinct phases per story).Reduced motion. The same comparison under
prefers-reduced-motion: reduce: the period is 3000 ms per turn on both, iterations infinite, and all 54 frames are byte-identical tomain.Scope
Testing
vitest run packages/core/src/Spinner: 51 passed. New: the keyframes turn by a whole number of turns (at least 100) and the duration is that many periods; 20 spinners render once and never again over 10 s of fake frames and timers.SpinnerMotion.a11y.chromium.spec.ts(--project=chromium): 6 passed. The rate, reduced-motion and storyboard tests now read the period as the iteration duration divided by the turns in the keyframes. "Turning paints nothing" now also bounds style recalculations over 2 s at 2 (CI measured 1, local 0); againstmain's Storybook it fails with 15.themeMotion.test.ts: the pre-tempo-first check for Spinner now matches thevar()inside the turn multiplier.CircularProgress) moves this rotor into a shared module. Whichever lands second carries the two values with it;CircularProgressonmainkeeps its own rotation and is not touched here.