Repository navigation
Conversation
design:reduced-motion DR1 forbids a resize under reduced motion, and the determinate fill transitioned its width on every value change. Under prefers-reduced-motion: reduce the transition now takes no time, so the fill jumps. The indeterminate loop is unchanged (its policy is a separate decision). A source-build test reads the compiled rules and fails on the previous source.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsProgressBar (@astryxdesign/core) · View in Storybook
Component 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: 2 accessibility violation(s) found — 2 serious. ProgressBar - 1 issue(s)
Progressbar - 1 issue(s)
Visual Regression55 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 35 more. Generated by PR Enrichment workflow | Storybook | Sandbox | 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.
With Reduce Motion on, ProgressBar's fill now takes its new value at once with a mouse, trackpad or keyboard, which matches design:reduced-motion DR1 and DR3, and the new test fails on the previous source as described. Approval waits on one decision for touch screens: the same rule makes the fill jump on a coarse pointer, while DR2 currently asks for an opacity-only cross-fade of 200 ms or less there and lists "a change with no cross-fade" as a failure. Any engineering owner, the author included, needs to decide whether DR2 should allow an immediate change for an in-place value change like this one, and amend design:reduced-motion to say so, or whether the fill should cross-fade on touch, which this PR would then add.
[Automated review]
| // (design:reduced-motion DR1). | ||
| transitionDuration: { | ||
| default: durationVars['--duration-medium'], | ||
| '@media (prefers-reduced-motion: reduce)': '0s', |
There was a problem hiding this comment.
Under Reduce Motion this 0s also applies on a coarse (touch) pointer, so the fill jumps there too. design:reduced-motion DR2 currently asks for an opacity-only cross-fade of 200 ms or less on a coarse pointer and lists "a change with no cross-fade" as a failure, so this line waits on an engineering owner's decision: amend DR2 for in-place value changes, or add a touch cross-fade here.
…th fades in design:reduced-motion DR2 asks a touch screen for an opacity-only fade of 200 ms or less, not a jump. Under reduced motion on a coarse pointer the transitioning fill steps aside for a second fill keyed by the value: each change remounts it at the new length and it fades in (200 ms, easeInOut). With a fine pointer the fill still changes at once (DR3), and nothing animates width or transform under reduced motion (DR1). ProgressBar stays presentational: it remembers no previous value, so the old length does not fade out separately. The source-build test and a render test (the keyed fill remounts, the transitioning one stays) fail on the previous head.
meta-cla
Bot
added
the
CLA Signed
There was a problem hiding this comment.
With Reduce Motion on, ProgressBar's fill now changes at once with a mouse, trackpad or keyboard, which matches design:reduced-motion DR1 and DR3, but the touch-screen treatment can't be approved yet. It waits on an engineering owner's decision, which the author can make as one: either amend DR2 so an in-place value change like this one also changes at once on touch, or record an architecture:component-style-authoring INV7 exception for the second, always-mounted fill a touch cross-fade needs, and approve that record change here with /approve-spec <full head SHA>. Separately, the touch fade restarts from opacity 0 on every value change, so the filled part blanks each time and, with the value stepping every 50 ms, the fill never rises above 0.06 opacity; that needs fixing under either outcome.
[Automated review]
| style={{width: `${percentage}%`}} | ||
| /> | ||
| <div | ||
| key={percentage} |
There was a problem hiding this comment.
On a coarse pointer under Reduce Motion this key remounts the fill on every value change, and fillFadeIn (fill mode both) restarts it from opacity 0, so the part already filled blanks each time. With the value stepping every 50 ms the fill never rose above 0.06 opacity, behind a label that reads the real value. Whatever is decided for the touch treatment, the filled length needs to stay visible through a change, or the change should land at once.
| )} | ||
| style={{width: `${percentage}%`}} | ||
| /> | ||
| <div |
There was a problem hiding this comment.
This second fill stays mounted in every determinate bar and is display: none everywhere except touch with Reduce Motion, where the first one is hidden instead (the Default story goes from 111 to 112 elements under both pointers). architecture:component-style-authoring INV7 and spec:AST-017 FR60 allow a second copy only transiently, for exits, presentation switches, lifts and sheet handoffs, or through an owner exception recorded with before and after counts; none is recorded yet.
… any pointer The touch fade-in remounted the fill on every change and restarted it from opacity 0, so on a fast upload the filled part never showed (6% opacity at a 50 ms step), and its always-mounted second fill was a second copy (component-style-authoring INV7). This returns to the first head's behavior: under reduced motion the fill changes at once on any pointer, pending the owner's decision on recording that for in-place value changes in design:reduced-motion DR2.
There was a problem hiding this comment.
With Reduce Motion on, ProgressBar's fill now takes its new value at once with a mouse, trackpad or keyboard, which matches design:reduced-motion DR1 and DR3, and the new test fails on the previous source as described. On a touch screen the fill jumps too, while DR2 currently asks each change on a coarse pointer to cross-fade and lists "a change with no cross-fade" as a failure. Approval waits on an engineering owner deciding that, and the author can act as one: either amend DR2 so an in-place value change like this fill's length lands at once on any pointer and record it here by commenting /approve-spec <full head SHA>, or keep DR2 and have this PR add a touch cross-fade for the fill. If DR2 is amended, nothing else in this PR needs to change.
[Automated review]
|
Posted by Cindy's agent on her behalf. Cindy's decision for progress under Reduce Motion: in-place value changes, such as a progress fill's length, apply at once on any pointer. There's no cross-fade and no extra element, so progress stays readable at any update rate. |
…een and with a mouse alike D36: under reduced motion a progress fill's value change applies at once on any pointer, with no fade and no second fill. The source-build test now pins that no reduced-motion rule is scoped to a pointer and none fades or animates the fill; it fails on the touch-fade head (e6ed676).
There was a problem hiding this comment.
With Reduce Motion on, ProgressBar's fill now lands on its new value at once, which matches design:reduced-motion DR1 and DR3, and the new test fails on the previous source as described. On a touch screen it lands at once too, but DR2 on main still asks each change on a coarse pointer to cross-fade, so approval needs that record updated for in-place value changes, as #8645 proposes. Once #8645 is merged nothing else here needs to change; alternatively this PR can carry the same DR2 note, and an engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, which the author may do as an engineering owner.
[Automated review]
| const reduced = rules.filter(rule => | ||
| rule.startsWith('@media(prefers-reduced-motion:reduce)'), | ||
| ); | ||
| expect(reduced.filter(rule => rule.includes('pointer'))).toEqual([]); |
There was a problem hiding this comment.
Non-blocking: this filter keeps only rules that start with @media(prefers-reduced-motion:reduce). A touch rule written @media (pointer: coarse) and (prefers-reduced-motion: reduce) compiles to @media(pointer:coarse)and(prefers-reduced-motion:reduce){…} and never reaches the pointer or fade checks: the second commit's touch fade written that way passes all three tests. Filtering with rule.includes('prefers-reduced-motion:reduce') would catch either order.
Owner decision D36 (decided): under reduced motion, a progress fill's value change applies at once on any pointer, with no fade and no second fill. This PR does exactly that.
With Reduce Motion on, ProgressBar's determinate fill still grew or shrank to each new value over 410 ms. design:reduced-motion DR1 forbids a resize under reduced motion.
ProgressBar.reduced-motion.source-build.test.mjsreads the compiled rules: the reduced-motion width transition takes no time, and no reduced-motion rule is scoped to a pointer or fades or animates the fill, so a touch screen and a mouse behave alike. It fails on the previous source and on the touch-fade attempt.Evidence
Real Chrome (headless, Linux), on this PR's CI Storybook (
ProgressBar / Default) beside main's. The value changes from 20 to 90; frames run from the change to about 0.5 s later. Desktop uses a mouse (fine pointer); the phone uses touch (coarse pointer).Measured fill width after the change:
0s), with no fade.