Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
the
needs:design-review
|
AI review status for this pull request.
|
b339f1f to
52da7a5
Compare
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.
Approval needs an engineering owner's decision first. ProgressBar's new touch presentation (linear motion at 1 s per full track, a 16 px minimum fill, a transparent fill at zero, and no change under Reduce Motion) rests only on the draft spec:AST-094 this pull request adds, and its FR4 would narrow architecture:theme-tokens to pointer presentations. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and you can do that yourself as an engineering owner. Please also add ## Writing rubric results for the record to the description, and settle the Reduce Motion exception against FR8. Independently of that decision:
- A value that returns to where it came from mid-flight (10% → 90% → 10%) runs about 400 ms although
--_progress-bar-durationsays 800 ms, because the browser shortens reversed CSS transitions; a 10 → 90 → 10 case inProgressBarTouch.a11y.chromium.spec.tswould catch it. - A mark that falls inside the 16 px minimum fill keeps
data-placement="track"and the track mark color. - FR7 cites
spec:AST-090, which is not in the repository. - The diff is 16,490 authored lines because it carries the whole predecessor stack; landing #8196, #8211 and their predecessors first would leave this pull request's own 765-line commit to review.
[Automated review]
| const durationMs = | ||
| motion.percentage === percentage | ||
| ? motion.durationMs | ||
| : Math.abs(percentage - motion.percentage) * TOUCH_MS_PER_PERCENT; |
There was a problem hiding this comment.
Returning to the previous value mid-flight doesn't get this duration. In Chromium, 10% → 90% → 10% (the reverse sent at the half) writes --_progress-bar-duration: 800ms, but the transition runs 400 ms: the after-change transform equals the running transition's start value, so CSS's reversal rule scales the duration by the completed fraction. A retarget to any other value (90 → 30) keeps its 600 ms. If the model-change timing is the contract, keep a reversal's target from matching the running transition's start value, or drive the touch fill with a Web Animation, and add a 10 → 90 → 10 case to ProgressBarTouch.a11y.chromium.spec.ts.
github-actions
Bot
removed
the
needs:design-review
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: ProgressBar's iOS touch presentation (linear motion for the 1 s whole-track motion role from the drawn position, a 16 px minimum fill, no fill at zero, and no change under Reduce Motion) rests only on the draft spec:AST-094 this pull request adds, whose FR4 narrows architecture:theme-tokens and whose FR4 and DEC-1 reject the theme motion roles the same head adds. An engineering owner records that decision, including whether the theme roles stay with AST-094 amended or are removed, by commenting /approve-spec <full head SHA> on this pull request, and you can do that yourself as an engineering owner. Please also add ## Writing rubric results for the record to the description. Independently of that decision:
- A return to the previous value mid-flight (10% → 90% → 10%) runs 400 ms although 0.8 s is declared, because the browser shortens a reversed CSS transition.
- A theme that sets
progress.activityIndicator.cycleMsbreaks the Spinner's stepping, since the spoke delays stay at fixed 100 ms steps. - The new
platformadaptation condition is outside the closed vocabulary ofspec:AST-012FR2, so it needs removing or that record amended first. - CI is red: the motion topic's CLI golden (
docs-topics.json) is stale after the new role row, and no Changeset names@astryxdesign/cli. astryx docs motionsays sheets, menus and navigation play roles that nothing reads yet, and a mark inside the 16 px minimum keeps its track styling.- AST-094 FR7 cites a
spec:AST-090that is not in the repository, and the diff is 19,100 authored lines because it carries the predecessor stack; landing those first would leave this pull request's own two commits.
[Automated review]
| const change = | ||
| motion.percentage === percentage | ||
| ? motion.change | ||
| : Math.abs(percentage - motion.percentage) / 100; |
There was a problem hiding this comment.
A return to the previous value mid-flight doesn't get this duration. In Chromium with the iOS gate emulated, 10% → 90% → 10% (the reverse sent at the half) computes transition-duration: 0.8s, but the transition runs 400 ms: the after-change transform equals the running transition's start value, so the CSS reversal rule scales the duration by the completed fraction. A retarget to another value (90 → 30) keeps its 600 ms. Keeping a reversal's end value from matching the running transition's start, or driving the touch fill with a Web Animation, would keep the model-change timing; a 10 → 90 → 10 case in ProgressBarTouch.a11y.chromium.spec.ts would catch it.
| // spoke partway through its cycle, so every spoke is in step from the first | ||
| // frame. | ||
| const spokeStyles = stylex.create({ | ||
| s0: {animationDelay: '-600ms'}, |
There was a problem hiding this comment.
These delays assume the 800 ms cycle, but the spokes' animation-duration reads the progress.activityIndicator.cycleMs role a theme may retune. With the role at 1600ms, the head sits on spoke 6 and dims for 800 ms, then sweeps the other seven spokes in 700 ms. Writing each delay as a fraction of the role, such as calc(var(--motion-progress-activity-indicator-cycle-ms, 800ms) * -0.75), keeps one spoke per eighth of the cycle.
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
spec:AST-012 FR2 (current) closes the when vocabulary to width, pointer, contrast and motion, and says future environment fields "require explicit additions to this closed vocabulary". This adds a fifth field without that amendment; neither ProgressBar nor Spinner needs it.
| content: [ | ||
| { | ||
| type: 'prose', | ||
| text: 'On a touch screen, sheets, alerts, menus, navigation and scrolling play the springs, durations and curves measured from iOS. Each value is a motion role named by its table path, such as `springs.sheetPresent` or `timing.curves.easeInOut`. A theme retunes a role through `motion.springs`, `motion.durations` or `motion.curves`, at its root or in an adaptation rule; a role it leaves alone keeps its measured default.', |
There was a problem hiding this comment.
At this head no sheet, alert, menu, navigation or scroll component reads a motion role: only ProgressBar and Spinner read a --motion-* variable, and only on iOS. A theme author following this page (and the example's springs.sheetPresent or springs.navigationPush) would see nothing change.
| "best-practices" | ||
| ], | ||
| "skeleton": "e648d618f9141bbd7f15bf970ba9e0d41b59ff9ae89e0cbbb2042e98bd5f0109", | ||
| "skeleton": "a86303e70a6cb63931e8e29c3b470b94f8edcc0b15576a907680a376213faddc", |
There was a problem hiding this comment.
This skeleton predates the progress.progressView.wholeTrackMs row the last commit adds to motion.doc.mjs; experimental-isolation.test.mjs now computes a different hash for the motion topic, which fails CI test-node (1/4). Regenerating the goldens fixes it.
| follows. | ||
| - **FR7 — Exits animate as UIKit's do.** A touch presentation MUST play the | ||
| exit its UIKit counterpart plays. **Proposed; pending owner review:** this | ||
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 |
There was a problem hiding this comment.
No spec:AST-090 record exists at this head or on main, so an owner approving this record cannot read what FR7 departs from.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: ProgressBar's iOS touch presentation (linear motion for the 1 s whole-track motion role from the drawn position, a 16 px minimum fill, no fill at zero, and no change under Reduce Motion) rests only on the draft spec:AST-094 this pull request adds, whose FR4 narrows architecture:theme-tokens and whose FR4 and DEC-1 reject the theme motion roles the same head adds. An engineering owner records that decision, including whether the theme roles stay with AST-094 amended or are removed, by commenting /approve-spec <full head SHA> on this pull request, and you can do that yourself as an engineering owner. Please also add ## Writing rubric results for the new record to the description. Independently of that decision:
- A return to the previous value mid-flight (10% → 90% → 10%) runs 400 ms although 0.8 s is declared, because the browser shortens a reversed CSS transition.
- A theme that sets
progress.activityIndicator.cycleMsbreaks the Spinner's stepping, since the spoke delays stay at fixed 100 ms steps. - The new
platformadaptation condition is outside the closed vocabulary ofspec:AST-012FR2, so it needs removing or that record amended first. astryx docs motionand the theme template describe sheets, menus and navigation playing roles that nothing reads yet, and a mark inside the 16 px minimum keeps its track styling.- AST-094 contradicts the code it ships (FR4 and DEC-1, FR8, FR13) and cites a
spec:AST-090that is not in the repository, and the diff is 19,367 authored lines because it carries the predecessor stack, so landing those first would leave this pull request's own commits.
[Automated review]
|
Posted by Cindy's agent on her behalf. Parts of this PR re-implement behavior that already exists, or is being specified, as a shared part of core. Converging keeps one implementation per behavior:
This is about where the behavior lives, not about the PR's intent. |
|
Agreed: one implementation.
|
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the draft spec:AST-094 this pull request adds: ProgressBar's iOS touch presentation (linear motion for the progress.progressView.wholeTrackMs role from the drawn fill, a 16 px minimum, a transparent zero, and landing at once under Reduce Motion) has no current spec authority, and AST-094's FR4 and DEC-1 reject the theme motion roles the same head adds. An engineering owner records that decision, including whether a determinate value change falls under design:reduced-motion and whether the theme roles stay with AST-094 amended or are removed, by commenting /approve-spec <full head SHA> on this pull request; you can do that yourself as an engineering owner. Independently of that decision, these need changes:
- Under reduced motion a coarse pointer outside iOS still sees the fill slide for 300 ms, because the generated rule is
(prefers-reduced-motion: reduce) and (not (pointer: coarse)), although the Changesets say a new value lands at once on every pointer. - The 744 px / 500 px compact edge contradicts current
spec:AST-012FR9 andarchitecture:public-component-apiINV17 (width below the effectivemdpoint, from the theme's one definition), and theplatformcondition contradicts AST-012 FR2. - The themable
timing.reducedMotionCrossFadeMsrole (500 ms by default) contradictsdesign:reduced-motionDR4 and DR2, andastryx docs motionsays sheets, menus and navigation play roles that nothing reads yet. - AST-094 contradicts the code it ships (FR4 and DEC-1, FR8, FR13) and cites a
spec:AST-090that is not in the repository; the request to take the touch gate from the shared resolver is still open. - The diff is 19,833 authored lines because it carries #8196, #8211 and their stack; landing or retargeting them would leave about 1,195 lines of this pull request's own.
[Automated review]
| // conflict with it. | ||
| transitionDuration: { | ||
| default: durationVars['--duration-medium'], | ||
| '@media (prefers-reduced-motion: reduce)': '0s', |
There was a problem hiding this comment.
StyleX emits this as @media (prefers-reduced-motion: reduce) and (not (pointer: coarse)), because of the sibling (pointer: coarse) key below. On a coarse pointer without the iOS gate (an Android phone) the fill therefore keeps var(--duration-medium): in Chromium with touch emulation and reduced motion, 10% → 90% runs a 300 ms cubic-bezier(0.24, 1, 0.4, 1) transform transition. Both Changesets say a new value lands at once on every pointer. Adding the reduced-motion 0s inside the coarse branch, outside the @supports gate (or declaring it after the coarse key), and a Chromium case with hasTouch and reduced motion that expects no running transition would close it.
| export const TOUCH_QUERY = '(pointer: coarse)'; | ||
|
|
||
| /** The viewport is compact: layout behaviours. */ | ||
| export const COMPACT_QUERY = '(width < 744px), (height < 500px)'; |
There was a problem hiding this comment.
Current spec:AST-012 FR9 (and architecture:public-component-api INV17) defines compact as a viewport width below the effective md point or a height below 500, exported once by the theme, and says every resolver, "including the web touch presentation resolver", uses it with none keeping "size numbers of its own". This constant uses 744 px, so on a 744-767 px coarse-pointer screen presentation="adaptive" opens a popover where FR9 makes the screen compact (at 750 px: a bottom sheet on the base, a popover here), and a theme that moves md changes nothing. main now exports compactDefinition for this.
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
Current spec:AST-012 FR2 lists width, height, pointer, contrast and motion and says "Future environment fields require explicit additions to this closed vocabulary." This adds a fifth field without that amendment, and neither ProgressBar nor Spinner reads it.
| * Cross-Fade Transitions on (opacity CABasicAnimation 0.5 s, | ||
| * (0.42, 0, 0.58, 1)), Oct 23 2026 | ||
| */ | ||
| reducedMotionCrossFadeMs: 500, |
There was a problem hiding this comment.
This duration is a themable role (the animator reads it through resolveMotion, and the docs show a theme setting it to 300 ms). Current design:reduced-motion DR4 says theme values offer "no way to keep travel, scaling, springs, or a longer animation under reduced motion", and DR2 caps a touch cross-fade at 200 ms; the 500 ms default exceeds that too. Excluding it from the roles and capping the animator's fade would keep both.
| content: [ | ||
| { | ||
| type: 'prose', | ||
| text: 'On a touch screen, sheets, alerts, menus, navigation and scrolling play the springs, durations and curves measured from iOS. Each value is a motion role named by its table path, such as `springs.sheetPresent` or `timing.curves.easeInOut`. A theme retunes a role through `motion.springs`, `motion.durations` or `motion.curves`, at its root or in an adaptation rule; a role it leaves alone keeps its measured default.', |
There was a problem hiding this comment.
At this head no sheet, alert, menu, navigation or scroll component reads a motion role: only ProgressBar and Spinner read a --motion-* variable, and only under the iOS gate. A theme author retuning springs.sheetPresent from this page would see nothing change.
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 | ||
| (reduced motion closes at once) for touch presentations (owners: ernestt, | ||
| cixzhang). | ||
| - **FR8 — Reduce Motion substitutes, never jumps.** Under |
There was a problem hiding this comment.
ProgressBar lands at once under Reduce Motion at this head, which this requirement and its verification row ("an instant jump under Reduce Motion ... fails") reject; FR4 and DEC-1 likewise reject the theme motion roles the same head adds, and FR13 rejects the literal gate keys. The 0.5 s navigation cross-fade here also exceeds the 200 ms current design:reduced-motion DR2 allows.
github-actions
Bot
added
the
needs:design-review
|
Done in c2f1979, d85e70e and f10050c (head 6a1aba3): the timing, the minimum fill and the empty fill key on |
An attach reads the element's own will-change in a task of its own. A shared animator (animatorFor) is never detached, so its node can leave the document with that read still waiting, and the read then forces a style pass on a node with no style: a test run that mounts and unmounts many pagers piled up such reads until a later wait timed out. The read now skips a node out of the document; a promotion that needs it reads it then, as one on a node attached in the same task does. A detach and dispose already cancel it. The new test fails on the previous code (one read of a removed node); the in-task read test now puts its element in the document, where the read happens.
…(spec:AST-094 FR18, DEC-13) A screen-edge pan attached to a stack container, as wide as the screen, with a touch 4 px from its edge: inside the 40.2 px band the pan decides at UIKit's 15 px whatever the element's width, and then tracks the travel past it. From several edges too. Both fail while a wider element decides at FR19's 10 px.
…'s width (spec:AST-094 FR18, DEC-13) FR18: inside the 40.2 px band a screen-edge pan decides at UIKit's 15 px screen-edge hysteresis; outside it the author's scrolling is untouched and FR19's 10 px governs every other claim. The pan decided at 10 px on any element wider than the band, so NavigationStack's edge pan, attached to the stack container, began 5 px early. The edge zone still decides only the touch-action; a touch outside every band still fails at once. The two cases that pinned the 10 px decision go.
…er's move keeps that finger until it lifts) into the experimental touch exports
…than the band (spec:AST-094 FR18, DEC-22) FR18 as amended in meta#8631 (DEC-22): the band's pan-x claim and 15 px start hold only where the screen-edge pan is attached to an element within the band; on an element that reaches beyond it, whose touch-action cannot claim per touch, a touch in the band decides at FR19's 10 px like every other claim. This restores the 10 px decision 3d85c30 removed and keeps 15 px on an element within the band. The cases pin DEC-22 both ways (a wide element with a touch at x=4 begins at 10, an in-band strip at 15, and from several edges at 10 on a wide element); the 10 px cases fail on 3d85c30.
…xperimental touch exports The motion README's Files table is the union: main's rows, with meta#8663's resolveSnapPoint.ts and selectPage.ts, plus this branch's, in main's order. # Conflicts: # packages/core/src/motion/README.md
The gesture README promises that usePressTracker rebuilds its tracker when trackingExpansion, highlightDelay, cancelTravel or quickTapFlash change, from the next touch. That fix (c248459) was lost in a later merge of the press-tracking line: the tracker was built once, so a changed option never applied. It is memoized on those four again; the node attachment, keyed on the tracker, moves to the new one and disposes the old. The callbacks keep being read through the ref on every touch. The new test fails before this change (the same tracker after each option change) and passes after it.
There was a problem hiding this comment.
Approval waits on an engineering owner identifying the spec that governs ProgressBar's fill geometry under a finger: the 16 px minimum above zero and the transparent fill at zero. No current record covers that geometry, and spec:AST-094 FR42 and DEC-20 (the default look grows no box under touch) bear on it. An engineering owner records the decision by commenting /approve-spec with the full head SHA on this pull request, and the author may do so as one. Whatever the decision, these changes are needed:
- Land #8076 and #8196 on their own and rebase, so this branch carries only its 1,368-line ProgressBar change instead of 18,172 authored lines.
- With forced colors and a finger, the fill animates
widthfor up to a second, which AST-094 FR5 rules out, so let the value land at once there. - Register the touch duration and curve as motion roles of the
progresstable (AST-094 FR4, DEC-5) and add the role test. - Add real-device recordings on an iPhone and an Android phone, plus a WebKit busy-main-thread proof, or narrow the changeset's every-touch-device claim.
[Automated review]
| }, | ||
| transitionProperty: { | ||
| default: 'transform', | ||
| '@media (forced-colors: active)': 'width', |
There was a problem hiding this comment.
Under forced colors with a finger this transitions width with the touch duration (line 331): 800 ms linear for 10% → 90%, at 50% of the track after 400 ms in Chromium phone emulation. spec:AST-094 FR5 says a touch presentation "MUST NOT animate layout properties". A '@media (forced-colors: active) and (pointer: coarse)': '0s' duration key after the coarse one lands the value at once there and leaves the mouse path alone.
| * (progressBarTouch.stylex.ts). | ||
| */ | ||
| export function registerProgressBarTable(): void { | ||
| registerMotionTable('progress', progressBar); |
There was a problem hiding this comment.
This registers progress with no roles, so the touch duration and curve the fill plays are not motion roles. spec:AST-094 FR4 says every duration and curve a touch presentation plays "MUST be a motion role named for its use", and DEC-5 has a component's measured roles join through the generated role table. List progressView.wholeTrackMs and a curve leaf as roles here, and add FR4's role test.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger (a coarse primary pointer, on every touch device), a determinate ProgressBar moves to a new value the way iOS's progress view does: linearly, taking one second per full track of change (0.8 s from 10% to 90%), and from wherever the fill is drawn when the value changes again mid-way. Above zero the fill never draws shorter than two track heights, and at zero it is not drawn. The motion runs on the compositor, so it keeps going while a script runs; under reduced motion a new value lands at once. A mouse or trackpad keeps the theme's duration and easing. |
There was a problem hiding this comment.
"On every touch device" and "keeps going while a script runs" are claims about iOS Safari and Android Chrome. spec:AST-094 Platform support asks for real-device evidence for those (spec:AST-013 FR10), and the recordings come from a simulator and an emulator. Add device recordings, or narrow the wording to what was shown.
… s of wall clock
The two proofs that a Reduce Motion dip held before its first frame
still lands (after the hold is finished, and after it is resumed) polled
the box for 2 s of wall clock. On a loaded machine the page's main
thread ran the dip's next step late and the poll ran out first: one
WebKit run read {opacity: 0, translate: 'none'} with four WebKit
containers on the box, then passed in 530 ms on a rerun. They now wait
until the box runs no animation, on the animations' own finished
promises (including the fade back in the dip starts when its fade out
ends), and then assert the landed state. The test's own timeout still
bounds them.
With a 2.5 s main-thread block induced right after the release, both
proofs failed before this change (Timeout 2000ms exceeded while waiting
on the predicate) and pass after, 3 runs each in Chromium.
Main merged meta#8045 and meta#8046. The one conflict is utils/pressFeedback.ts: it takes main's merged meta#8046 text (the finger and button slots, the mouse and key press) and keeps this pull request's own additions, the one writer of the press attribute (writePress, paintPress, readPressPaint) and the header line naming them.
# Conflicts: # internal/interaction-harness/README.md # internal/interaction-harness/src/busy.test.ts # internal/interaction-harness/src/busy.ts # internal/interaction-harness/src/frames.ts # internal/interaction-harness/src/harness.interaction.browser.spec.ts
Main's button slot, merged with meta#8046, ends a press's release at once when the next press starts, by clearing the press attribute directly. On a row another painter holds, that clear erased the hold, which this pull request's one writer refuses. A mouse press on a held row, released, then a press elsewhere must leave the row held. Fails until the early end goes through the one writer.
finish() writes none through writePress, as this pull request's controller paints, so a held row stays its owner's when the next press ends a release early, under the finger and the button slots alike.
Both upstream heads are folded onto main 70ea859 (her meta#8045 frame evidence and meta#8046 coarse press): meta#8446 takes main's harness files, and meta#8073 carries the press controller onto meta#8046's slots. meta#8076 has no change of its own in either set of files, so the merge takes theirs with no hand resolution, and meta#8076 is clean against main again.
jsdom serves no cross-origin sheet, so the catch around the touch tests' rule reader could only hide a real error.
…cknowledgement Each frame's acknowledgement settles before the session detaches, and a failure fails the test instead of being swallowed.
There was a problem hiding this comment.
Approval waits on an engineering owner identifying the spec that governs ProgressBar's fill geometry under a finger, the 16 px minimum above zero and the transparent fill at zero: no current record covers it, and spec:AST-094 FR42 and DEC-20 (the default look grows no box under touch) bear on it. An engineering owner records that decision by commenting /approve-spec with the full head SHA on this pull request, and you can do that yourself as an engineering owner. Whatever the decision, these changes are needed:
- Land #8076 and #8196 on their own and rebase, so this branch carries only its ProgressBar change (about 1,350 of 16,431 authored lines).
- With forced colors and a finger the fill animates
widthfor up to a second, which AST-094 FR5 rules out; letting the value land at once there would conform. - Register the touch duration and its linear curve as motion roles of the
progresstable, with the role test AST-094 FR4 asks for. - Add real-device recordings on an iPhone and an Android phone, or narrow the changeset's every-touch-device and busy-script claims.
[Automated review]
| }, | ||
| transitionProperty: { | ||
| default: 'transform', | ||
| '@media (forced-colors: active)': 'width', |
There was a problem hiding this comment.
Under forced colors with a finger, width stays the transitioned property while the coarse duration and curve below still apply (lines 331 and 338): a 10% → 90% change runs an 800 ms linear width transition, at 50% of the track after 400 ms in Chromium phone emulation. spec:AST-094 FR5 says a touch presentation "MUST NOT animate layout properties". A '@media (forced-colors: active) and (pointer: coarse)': '0s' duration key after the coarse one lands the value at once there and leaves the mouse path alone.
| // would halve a return mid-flight (10 → 90 → 10). It takes three | ||
| // values because two would repeat on that return. | ||
| default: | ||
| 'translateX(calc(max(var(--_progress-bar-fill, 0%), 16px) - 100% + var(--_progress-bar-flip, 0) * 0.01px))', |
There was a problem hiding this comment.
Held for an engineering owner: the 16 px minimum (and the transparent fill at zero, line 344) has no current spec authority. spec:AST-094 covers touch timing (FR1, DEC-1), leaves platform look and feel out of scope, and its FR42 and DEC-20 say the default look grows no box under touch; at 1% on a 370 px track this draws 16 px where a mouse draws 3.7 px.
| * (progressBarTouch.stylex.ts). | ||
| */ | ||
| export function registerProgressBarTable(): void { | ||
| registerMotionTable('progress', progressBar); |
There was a problem hiding this comment.
This registers progress with no roles, so the touch duration and curve the fill plays are not motion roles. spec:AST-094 FR4 says every duration and curve a touch presentation plays "MUST be a motion role named for its use", and DEC-5 has a component's measured roles join through the generated role table. Listing progressView.wholeTrackMs and a linear curve leaf as roles here (as registerReducedMotionCrossFade does for timing) and adding FR4's role test would conform.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger (a coarse primary pointer, on every touch device), a determinate ProgressBar moves to a new value the way iOS's progress view does: linearly, taking one second per full track of change (0.8 s from 10% to 90%), and from wherever the fill is drawn when the value changes again mid-way. Above zero the fill never draws shorter than two track heights, and at zero it is not drawn. The motion runs on the compositor, so it keeps going while a script runs; under reduced motion a new value lands at once. A mouse or trackpad keeps the theme's duration and easing. |
There was a problem hiding this comment.
"On every touch device" and "keeps going while a script runs" are claims about iOS Safari and Android Chrome, and spec:AST-094 Platform support asks for real-device evidence for those (spec:AST-013 FR10); the recordings come from a simulator and an emulator. Under forced colors the fill also animates width on the main thread today. Add device recordings, or narrow the wording to what was shown.
| * attached to the pull request. | ||
| * | ||
| * SYNC: Keep in step with ProgressBar.tsx (`styles.fill`, `fillEmpty`) and | ||
| * motion/uikit/progress.ts (`progressView`). |
There was a problem hiding this comment.
This SYNC note names motion/uikit/progress.ts, which does not exist; the table this spec imports (line 32) is motion/uikit/progressBar.ts.
Depends on #8196 and #8076. Review only this PR's change: vjeux/astryx@aa56e3d...bd4ecc6
User impact
Part of making Astryx match iOS under a finger (
spec:AST-094): a ProgressBar eases to each value in 300 ms, while iOS's moves linearly, one track per second, from wherever the fill is. ProgressBar now does that on touch devices; a mouse keeps the theme's timing.Options and tradeoffs:
Visual authority
DDrow reused or amended.Design decisionstable by a team design owner.docs/design/.Settled by
spec:AST-094DEC-1 and FR1: UIKit's measured timing, minimum fill and empty zero are the touch default. Under Reduce Motion a new value lands at once (design:reduced-motionDEC-3).Before and after
10% → 90% under a finger (Chromium), before then after, a row per 50 ms:
Mobile Safari, a 0.7 s busy script; main above, this PR below:
Android emulator:
Mid-flight, minimum, zero, right to left:
State matrix
Size (gzip, ProgressBar's import, #8196 + #8076 → head): JavaScript 8,011 → 9,450 B (+1,439), CSS 6,953 → 7,137 B (+184);
astryx.css35,538 → 35,719 B (+181).Scope
Testing
What I ran and what it showed: