Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request needs an engineering owner's decision before it can be approved: the 0 ms touch arm for the checkbox and radio marks rests only on the draft spec:AST-094 added here, whose FR4 intentionally narrows architecture:theme-tokens to pointer presentations. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so if they are an engineering owner), or amends the record; what has to be settled is whether touch presentations may leave the theme's duration and easing tokens, and how FR13's rule against component-owned media queries applies to the inline (pointer: coarse) arms. Independently of that decision:
- Please land the stacked predecessors first and rebase, so this pull request is its own 687-line commit rather than 13,623 authored lines.
- Please take the new
pr-interactionCI job out with the harness and either fold it into an existing surface owner or admit it through an AST-030 amendment, since FR11 forbids a separately routed lane for new coverage.
[Automated review]
| // Under a finger the mark changes in the frame the selection lands, as | ||
| // iOS's selection marks do (motion/uikit/selection.ts); a mouse keeps | ||
| // the theme's duration. | ||
| '@media (pointer: coarse)': MARK_CHANGE, |
There was a problem hiding this comment.
This arm is the primary change, and its only authority is the draft spec:AST-094 added in this pull request (FR4 narrows architecture:theme-tokens, pending owner review). Note too that the proposed FR13 says a component must not write its own media query for its touch presentation, which this inline (pointer: coarse) arm (here and in RadioIndicator.tsx) would contradict once AST-094 is approved as written.
| if git fetch --quiet --no-tags --depth=1 origin "$BASE_SHA" \ | ||
| && git show "$BASE_SHA:.github/scripts/pr-scope-touches.sh" > "$helper"; then | ||
| scope=0 | ||
| bash "$helper" "$BASE_SHA" '^(internal/interaction-harness/|playwright\.interaction\.config\.ts$|packages/[^/]+/src/|apps/storybook/)' || scope=$? |
There was a problem hiding this comment.
This job routes itself with its own path pattern over packages/[^/]+/src/ and apps/storybook/. spec:AST-030 FR11 says new coverage inside an existing surface must join that surface's owners and must not create a separately routed lane, and FR10 says a new test type is not a new surface. Fold these proofs into an existing owner, or admit the lane with an AST-030 amendment first.
| where a switch outside any scroll view is needed. | ||
|
|
||
| Done (iOS 26.2 simulator, Oct 23 2026; in the switch's own table, | ||
| `uikit/switch.ts`): M-SW02, M-SW03, M-SW04, M-SW05, M-SW06, M-SW09 (UISwitch |
There was a problem hiding this comment.
uikit/switch.ts (and uikit/contextMenu.ts further down) do not exist at this head, and scroll.ts, springs.ts and timing.ts cite a sources.test.ts that is really uikit/uikit.test.ts. Point these notes at files that exist, or drop them until those tables land.
40d3a28 to
ecdce12
Compare
|
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.
|
ecdce12 to
33c5a7d
Compare
vjeux
changed the title
33c5a7d to
9c8c5e6
Compare
|
Pushed in 9c8c5e6: the marks' touch and Reduce Motion arms now import the shared keys ( |
9c8c5e6 to
b5999ef
Compare
There was a problem hiding this comment.
Approval needs an engineering owner to settle the governing spec for the one-frame checkbox and radio change under a coarse pointer: spec:AST-094 leaves which components get a touch presentation to each component's own record, which neither indicator has as current authority, and its FR4 requires touch durations to be theme motion roles rather than the build-time 0ms the marks play. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or asks for the marks to keep --duration-fast under touch until the change is a motion role. Whatever is decided, these still need changes:
- Land the predecessor pull requests and rebase onto
main, which this branch conflicts with, so this diff is the 767-line selection-mark change; it carries 31,025 authored lines today. - The changeset says a mouse's change under Reduce Motion is "now immediate", but it already was.
packages/build/README.mdstill documents a production-build warning that no code emits.- The gestures README names a
useMotionPresentation()hook that does not exist. - AST-094 asks for real-device evidence of the iOS Safari and Android Chrome behaviour, and the recordings come from the iOS simulator and a Chromium 158 setup.
[Automated review]
| // immediate (design:reduced-motion DR3); a finger's already is. | ||
| // The resolver's and the policy's keys, spelled: an imported at-rule | ||
| // key lands in a different StyleX priority layer than a literal one. | ||
| '@media (pointer: coarse)': MARK_CHANGE, |
There was a problem hiding this comment.
This arm plays 0ms from a build-time mirror rather than a theme motion role. spec:AST-094 FR4 says a component "MUST NOT play a literal", and its Non-goals leave which components get a touch presentation to each component's own record (component:CheckboxIndicator is draft; RadioIndicator has none). It needs an engineering owner's /approve-spec on the exact head, or a motion role for the mark's change; the same applies to RadioIndicator.tsx:52.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger (a coarse primary pointer, on every touch device), a checkbox or radio mark now changes in the frame the selection lands, the way UIKit shows a selection mark. The box or circle used to fade between its two colors over the theme duration, so a tap first showed half-filled frames. With a mouse the marks keep the theme duration and easing, and under Reduce Motion a mouse's change is now immediate. |
There was a problem hiding this comment.
A mouse's change under Reduce Motion was already immediate: the base declares '@media (prefers-reduced-motion: reduce)': '0s' for every pointer, and this change only narrows that key to fine pointers. Suggest "stays immediate", or drop the clause.
|
|
||
| Unsupported declarations fail the build by default with StyleX's replacement guidance. Set `propertyValidationMode` explicitly in `stylexOverrides` (modern API) or `stylexOptions` (legacy API) only when a migration needs the upstream `warn` or `silent` behavior. | ||
|
|
||
| A production build also checks the StyleX stylesheet for generated custom properties (`--<prefix><hash>`) that it reads but no stylesheet in the build declares, which browsers drop: a `.stylex.ts` export read in a style that is not a `defineVars`/`defineConsts` value, or one whose module never reached the build. It warns, naming the variable and the rule that reads it. Only stylesheets count as declarations, so a property the app sets at runtime (an inline style) is reported too: give it a name outside the StyleX class-name prefixes. |
There was a problem hiding this comment.
No code at this head emits this warning: the revert "astryxStylex keeps no undeclared-variable warning" removed the check with its fixtures, tests and changeset. Drop this paragraph with it.
| is right. Behaviour reads | ||
| `useTouchPresentation()` (SSR-safe), a decision taken once reads | ||
| `readTouchPresentation()`, and a component with no interaction to infer | ||
| from reads `useMotionPresentation()`: UIKit timings under a finger, theme |
There was a problem hiding this comment.
useMotionPresentation() is not defined anywhere at this head; name a hook that exists, or drop the sentence.
There was a problem hiding this comment.
This change needs an engineering owner's decision before it can be approved: spec:AST-094 FR4 says a component must read touch motion through motion roles and must not play a literal, and the checkbox and radio marks play a 0 ms value from the UIKit table's mirror with no motion role and no component record that owns their touch presentation. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner); otherwise the marks keep --duration-fast under touch until their record names the presentation and the value is a motion role. These changes are needed either way:
- Land #8331 and #8076 first and rebase, so this pull request carries only its own change instead of about 23,000 stacked lines.
- Add recordings from a real iPhone in Safari and a real Android phone in Chrome, which AST-094's platform support section requires; simulator and emulator recordings do not cover it.
- Fix the changeset's claim that a mouse's change under Reduce Motion is "now immediate" (it already was), and the stacked docs that name
useMotionPresentation()or say a theme may retune motion roles.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger (a coarse primary pointer, on every touch device), a checkbox or radio mark now changes in the frame the selection lands, the way UIKit shows a selection mark. The box or circle used to fade between its two colors over the theme duration, so a tap first showed half-filled frames. With a mouse the marks keep the theme duration and easing, and under Reduce Motion a mouse's change is now immediate. |
There was a problem hiding this comment.
The base indicators already set '@media (prefers-reduced-motion: reduce)': '0s' for every pointer, so a mouse's change under Reduce Motion was already immediate. Drop the clause, or say it stays immediate.
| is right. Behaviour reads | ||
| `useTouchPresentation()` (SSR-safe), a decision taken once reads | ||
| `readTouchPresentation()`, and a component with no interaction to infer | ||
| from reads `useMotionPresentation()`: UIKit timings under a finger, theme |
There was a problem hiding this comment.
useMotionPresentation() exists nowhere in the tree. Point to useTouchPresentation() or drop the sentence.
| @@ -71,3 +72,13 @@ export const navigation = { | |||
| */ | |||
| interactivePopFinishThreshold: 0.5, | |||
| } as const; | |||
|
|
|||
| /** | |||
| * Makes each spring, duration and curve above a motion role a theme may | |||
There was a problem hiding this comment.
This registers with no roles, so none of the values above becomes a role, and spec:AST-094 FR4 says a theme cannot override roles yet (DEC-11). Describe them as internal constants.
| @@ -122,3 +124,16 @@ export const scroll = { | |||
| */ | |||
| scrollToTopRate: 0.99035, | |||
| } as const; | |||
|
|
|||
| /** | |||
| * Makes each spring, duration and curve above a motion role a theme may | |||
There was a problem hiding this comment.
Same as navigation.ts: no roles are passed, so nothing here becomes a role, and no theme may retune a role yet (spec:AST-094 DEC-11).
| import {resolveMotion} from '../motion/resolveMotion'; | ||
| import {registerSpringsTable, springs} from '../motion/uikit'; | ||
|
|
||
| // Once, so the springs follow the theme's roles: |
There was a problem hiding this comment.
Themes do not set motion roles yet (spec:AST-094 FR4, DEC-11); this line and line 69 promise theme control that does not exist.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: the one-frame touch presentation for the checkbox and radio marks has no current component record, and its 0 ms is a build-time literal where spec:AST-094 FR4 requires a motion role. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or asks that the marks keep --duration-fast under touch until the indicators' record names the presentation and the 0 ms is a role in the generated table. Whatever the decision, please:
- Land #8076, #8331 and their predecessors first, so this diff holds only the selection-mark change (765 of its 22,487 changed lines).
- Add real-device recordings in iOS Safari and Android Chrome, which
spec:AST-094requires for claims about those browsers. - Fix the changeset, which says a mouse's change under Reduce Motion is now immediate although it already was.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: no current record gives the checkbox and radio marks a 0 ms touch presentation (spec:AST-094 leaves each component's touch presentation to its own record), and the marks play a build-time constant where AST-094 FR4 requires a motion role. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; the alternative is keeping --duration-fast under touch until the indicators' records name the presentation and the value is a motion role. Independently of that decision, please:
- Land #8076 and #8331 first, or rebase once they land, so the diff is only the selection-mark change (765 of 21,162 authored lines).
- Fix the changeset's line 5, which says a mouse's Reduce Motion change is now immediate although it already was.
- Add recordings from a physical iPhone in Safari and a physical Android phone in Chrome, which
spec:AST-094requires for claims about those browsers.
[Automated review]
| // immediate (design:reduced-motion DR3); a finger's already is. | ||
| // The resolver's and the policy's keys, spelled: an imported at-rule | ||
| // key lands in a different StyleX priority layer than a literal one. | ||
| '@media (pointer: coarse)': MARK_CHANGE, |
There was a problem hiding this comment.
No current record decides that this mark changes in 0 ms under a finger: spec:AST-094 leaves each component's touch presentation to the component's own record, and its FR4 asks for a motion role rather than a constant. An engineering owner decides this; RadioIndicator.tsx:52 carries the same arm.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger (a coarse primary pointer, on every touch device), a checkbox or radio mark now changes in the frame the selection lands, the way UIKit shows a selection mark. The box or circle used to fade between its two colors over the theme duration, so a tap first showed half-filled frames. With a mouse the marks keep the theme duration and easing, and under Reduce Motion a mouse's change is now immediate. |
There was a problem hiding this comment.
A mouse's change under Reduce Motion was already immediate: both marks set '@media (prefers-reduced-motion: reduce)': '0s' before this change. "Is now immediate" announces a change nobody gets; drop the clause or say it stays immediate.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: no current record gives the checkbox and radio marks a 0 ms touch presentation (spec:AST-094 leaves each component's touch presentation to its own record), and the marks play a build-time constant where AST-094 FR4 requires a motion role. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; otherwise the marks keep --duration-fast under touch until the indicators' record names the presentation and the value is a motion role. Independently of that decision, please:
- Land #8076 and #8331 first, or rebase once they land, so the diff holds only the selection-mark change (765 of its 20,885 authored lines).
- Fix line 5 of the changeset, which says a mouse's change under Reduce Motion is now immediate although it already was.
- Add recordings from a physical iPhone in Safari and a physical Android phone in Chrome, which
spec:AST-094requires for claims about those browsers.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger (a coarse primary pointer, on every touch device), a checkbox or radio mark now changes in the frame the selection lands, the way UIKit shows a selection mark. The box or circle used to fade between its two colors over the theme duration, so a tap first showed half-filled frames. With a mouse the marks keep the theme duration and easing, and under Reduce Motion a mouse's change is now immediate. |
There was a problem hiding this comment.
A mouse's change under Reduce Motion was already immediate: both marks set '@media (prefers-reduced-motion: reduce)': '0s' for every pointer before this change, and this change only narrows that key to fine pointers. "Is now immediate" announces a change nobody gets; drop the clause or say it stays immediate.
| // immediate (design:reduced-motion DR3); a finger's already is. | ||
| // The resolver's and the policy's keys, spelled: an imported at-rule | ||
| // key lands in a different StyleX priority layer than a literal one. | ||
| '@media (pointer: coarse)': MARK_CHANGE, |
There was a problem hiding this comment.
No current record decides that this mark changes in 0 ms under a finger: spec:AST-094 leaves each component's touch presentation to the component's own record (component:CheckboxIndicator is draft; RadioIndicator has none), and its FR4 asks for a motion role rather than a constant. An engineering owner decides this; RadioIndicator.tsx:52 carries the same arm.
There was a problem hiding this comment.
Approval needs a governing spec for this addition: an engineering owner has to decide whether CheckboxIndicator and RadioIndicator get this one-frame touch presentation, which no current spec grants, and whether its 0 ms may stay a table constant where spec:AST-094 FR4 asks for a named motion role. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and as an engineering owner the author may do so; either way, two changes remain:
- Move the #8076 and #8331 work out of this branch (land those first, then rebase) so it carries only the 835-line selection-mark change instead of more than 20,000 changed lines.
- Add a tap recording of the checkbox and radio marks from a real iPhone in Safari and a real Android phone in Chrome, which
spec:AST-094requires for claims about those browsers.
[Automated review]
| // immediate (design:reduced-motion DR3); a finger's already is. | ||
| // The resolver's and the policy's keys, spelled: an imported at-rule | ||
| // key lands in a different StyleX priority layer than a literal one. | ||
| '@media (pointer: coarse)': MARK_CHANGE, |
There was a problem hiding this comment.
This one-frame touch presentation needs an engineering owner's decision: spec:AST-094 leaves which components get a touch presentation to each component's own record (neither indicator has a current one), and its FR4 asks for a named motion role where this arm plays the selectionMotionConsts constant. The same applies to RadioIndicator.tsx:52.
| // touch page here is the engine's own touch emulation; real iOS Safari and | ||
| // Android Chrome are the simulator and emulator recordings. |
There was a problem hiding this comment.
spec:AST-094 Platform support asks for real-device evidence for every claim about iOS Safari and Android Chrome behaviour, so simulator and emulator recordings do not cover the “every touch device” claim; a tap recording from a real iPhone in Safari and a real Android phone in Chrome is still needed.
There was a problem hiding this comment.
Approval needs a governing spec for this addition: an engineering owner has to decide whether CheckboxIndicator and RadioIndicator get this one-frame touch presentation, which no current spec grants, and whether its 0 ms may stay a table constant where spec:AST-094 FR4 asks for a named motion role. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and as an engineering owner the author may do so; either way, two changes remain:
- Move the #8076 and #8331 work out of this branch (land those first, then rebase) so it carries only the 848-line selection-mark change instead of more than 17,000 changed lines.
- Add a tap recording of the checkbox and radio marks from a real iPhone in Safari and a real Android phone in Chrome, which
spec:AST-094requires for claims about those browsers.
[Automated review]
| // immediate (design:reduced-motion DR3); a finger's already is. | ||
| // The resolver's and the policy's keys, spelled: an imported at-rule | ||
| // key lands in a different StyleX priority layer than a literal one. | ||
| '@media (pointer: coarse)': MARK_CHANGE, |
There was a problem hiding this comment.
This 0 ms arm is the decision the review is waiting on: spec:AST-094 leaves each component's touch presentation to its own record, and its FR4 asks that a touch duration be a motion role rather than a constant. An engineering owner records the decision with /approve-spec <full head SHA>; RadioIndicator.tsx line 52 is the same arm.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger (a coarse primary pointer, on every touch device), a checkbox or radio mark now changes in the frame the selection lands, the way UIKit shows a selection mark. The box or circle used to fade between its two colors over the theme duration, so a tap first showed half-filled frames. With a mouse the marks keep the theme duration and easing, and under Reduce Motion they still change at once. |
There was a problem hiding this comment.
“on every touch device” is a claim about iOS Safari and Android Chrome behaviour, and spec:AST-094 Platform support asks for real-device evidence for those (spec:AST-013 FR10). The description's recordings come from the iOS simulator and an Android emulator; a tap recording from a physical iPhone in Safari and a physical Android phone in Chrome would cover it.
There was a problem hiding this comment.
Approval needs a governing spec for this addition: an engineering owner has to decide whether CheckboxIndicator and RadioIndicator get this one-frame touch presentation, which no current spec grants (spec:AST-094 leaves that to each component's own record), and whether its 0 ms may stay a table constant where spec:AST-094 FR4 asks for a named motion role. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Whatever the decision, two changes remain:
- Land #8076 and #8331 first, or take them out of this branch, and rebase, so this pull request carries only the 848-line selection-mark change instead of 16,758 changed lines.
- Add tap recordings of both marks from a physical iPhone in Safari and a physical Android phone in Chrome, which
spec:AST-094Platform support requires for the "every touch device" claim, or narrow that claim.
[Automated review]
There was a problem hiding this comment.
Thanks for this. Under a finger, the checkbox and radio marks now change in the same frame as the selection, and in Chromium phone emulation the difference is clear: 40 ms after a tap the current mark is still mid-fade, while this branch already shows the final colours. The mouse fade and both Reduce Motion paths are unchanged.
Waiting on an owner decision. No current spec gives these indicators a touch presentation. The touch-presentations spec (AST-094) leaves that to each component's own record, the CheckboxIndicator record is still a draft that does not mention touch, and RadioIndicator has no record. AST-094 FR4 also asks that touch durations be named motion roles rather than literals, and the 0 ms here is a table constant; the description marks FR4 as pending owner review. An engineering owner can provide or identify the governing spec, or record the decision by commenting /approve-spec <full head SHA> on this pull request. The author may do so, as an engineering owner. The alternative is to keep --duration-fast under touch until the indicators' record names the presentation and the 0 ms is a role.
Needed either way:
- Scope: this branch also carries #8076 and #8331, and those two make up almost all of its 16,930 authored lines; the selection-mark change itself is 848. Please land them first, or drop them from this branch, and rebase so this pull request contains only the selection-mark change.
- Device evidence: the changeset says the marks change in one frame "on every touch device", and AST-094 asks for real-device evidence for claims about iOS Safari and Android Chrome. The recordings so far come from an iOS simulator and from an Android emulator running Chromium. Please attach tap recordings from a physical iPhone in Safari and a physical Android phone in Chrome, or narrow the wording to what the evidence shows.
[Automated review]
| // immediate (design:reduced-motion DR3); a finger's already is. | ||
| // The resolver's and the policy's keys, spelled: an imported at-rule | ||
| // key lands in a different StyleX priority layer than a literal one. | ||
| '@media (pointer: coarse)': MARK_CHANGE, |
There was a problem hiding this comment.
Under a coarse pointer this arm plays 0ms from a defineConsts mirror of selection.mark.changeMs. AST-094 FR4 asks that every touch duration be a motion role ("MUST NOT play a literal"), and no current record gives CheckboxIndicator or RadioIndicator a touch presentation (AST-094 Non-goals). This waits on an engineering owner: comment /approve-spec <full head SHA>, or keep --duration-fast under touch until the indicators' record names the presentation. The same applies to RadioIndicator.tsx:52.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger (a coarse primary pointer, on every touch device), a checkbox or radio mark now changes in the frame the selection lands, the way UIKit shows a selection mark. The box or circle used to fade between its two colors over the theme duration, so a tap first showed half-filled frames. With a mouse the marks keep the theme duration and easing, and under Reduce Motion they still change at once. |
There was a problem hiding this comment.
"on every touch device" is a claim about iOS Safari and Android Chrome. AST-094's Platform support asks for real-device evidence for those claims (spec:AST-013 FR10), and the recordings so far come from an iOS simulator and from an Android emulator running Chromium. Please attach physical-device tap recordings, or narrow this sentence to a coarse primary pointer.
There was a problem hiding this comment.
Approval waits on an engineering owner providing or identifying the governing spec for the one-frame checkbox and radio change under a coarse pointer: spec:AST-094 leaves which components get a touch presentation to each component's own record, which neither indicator has as current authority, and its FR4 asks for a motion role where the marks play a 0 ms table constant. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so, as an engineering owner), or asks for the marks to keep --duration-fast under touch until their record names the presentation and the value is a motion role. Whatever is decided, please land #8076, #8331 and the #8360 stack first, or drop them from this branch, and rebase: they are 27,521 of the 28,369 changed lines, beside the 848-line selection-mark change.
[Automated review]
| // immediate (design:reduced-motion DR3); a finger's already is. | ||
| // The resolver's and the policy's keys, spelled: an imported at-rule | ||
| // key lands in a different StyleX priority layer than a literal one. | ||
| '@media (pointer: coarse)': MARK_CHANGE, |
There was a problem hiding this comment.
This 0 ms arm is the decision the review waits on: spec:AST-094 leaves which components get a touch presentation to each component's own record (component:CheckboxIndicator is draft and silent on touch; RadioIndicator has none), and its FR4 asks for a motion role where this plays the selectionMotionConsts mirror of selection.mark.changeMs. An engineering owner records it with /approve-spec <full head SHA>; RadioIndicator.tsx:52 carries the same arm.
…n prelude StyleX 0.19.0's dev runtime keys a conditional rule whose prelude has more than one parenthesized condition by the prelude alone, so jsdom got only the first rule written under, say, @media (prefers-reduced-motion: reduce) and (pointer: coarse), and a test that read a later one read nothing. vitest.config.ts now names internal/test-utils/src/stylexInject.ts as the babel plugin's runtimeInjection: StyleX's own inject, plus each rule it drops, inserted once into a sheet of its own. Test-only: no StyleX code is patched, and the build keeps every rule as before. It comes out at the StyleX bump to a release that carries meta/stylex#1991, which fixes the key upstream. Red first: the new stylexMultiConditionMedia test (two styles, and one style with two declarations, under one two-condition query) failed with StyleX's own injection; on meta#8226 + meta#8682 + main, Indicator/selectionMotion.test.ts failed 2 of 6. Both pass with it, and the full ui suite on main has the same results (15,951 passing, plus the new test).
iOS has no checkbox or radio control; a choice is marked on its row, and on iOS 26.2 the mark changes in the frame the selection lands with no animation of its own: the checkmark accessory of a single-choice list in Settings, the completion circle in Reminders, a collection-view list's multiselect accessory, with and without Reduce Motion. CheckboxIndicator and RadioIndicator faded their box or circle between its two colors over the theme's fast duration under every pointer, so a tap showed half-filled frames first. Under a coarse primary pointer the fade now takes the UIKit table's value, one frame (motion/uikit/selection.ts, mirrored for the stylesheet in Indicator/selectionMotion.stylex.ts and held to it by selectionMotion.test.ts). A mouse keeps the theme's duration and easing tokens; under Reduce Motion a mouse's change is immediate, in the instant key of the reduced-motion policy (design:reduced-motion DR3), and a finger's already is. The browser spec proves both in Chromium and WebKit: every frame of a tap shows the mark wholly in its old or its new colour, a busy main thread cannot stretch the change, and a mouse keeps the fade.
bf74a5b to
4707533
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner deciding the governing spec for the one-frame checkbox and radio change under a coarse pointer; the review found no blocking code defect.
What has to be settled:
spec:AST-094leaves which components get a touch presentation to each component's own record, andcomponent:CheckboxIndicatorandcomponent:Indicator(which coversRadioIndicator) do not name one yet.- AST-094 FR4 asks that a touch duration be a registered motion role, while both indicators play
0msfrom adefineConstsmirror ofselection.mark.changeMs.
An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; the alternatives are for the indicators' records to name the presentation (as component:ProgressBar FR4 does) with its value as a registered role, or for the marks to keep --duration-fast under touch. In Chromium phone emulation a tap no longer starts the five 125 ms colour transitions, and the mouse fade and Reduce Motion behaviour are unchanged. Three new comments could be trimmed to their reason (non-blocking, noted inline).
[Automated review]
| // mouse's change is immediate too. | ||
| // The keys are spelled out: an imported at-rule key lands in another | ||
| // StyleX priority layer than a literal one. | ||
| '@media (pointer: coarse)': MARK_CHANGE, |
There was a problem hiding this comment.
This arm is the decision the review waits on. spec:AST-094 leaves which components get a touch presentation to each component's own record, and component:CheckboxIndicator does not name one; AST-094 FR4 also asks that a touch duration be a registered motion role, while this reads a defineConsts mirror of selection.mark.changeMs. An engineering owner can approve it as written with /approve-spec <full head SHA>, or the record can name the presentation and the table can register the value as a role first.
| // Under a finger the mark changes in the frame the selection lands, as | ||
| // UIKit's marks do (motion/uikit/selection.ts). Under Reduce Motion a | ||
| // mouse's change is immediate too. | ||
| // The keys are spelled out: an imported at-rule key lands in another | ||
| // StyleX priority layer than a literal one. |
There was a problem hiding this comment.
Non-blocking: "Under Reduce Motion a mouse's change is immediate too." restates the narrowed Reduce Motion key on lines 49–50. A shorter comment that keeps only the reason and the quirk:
// Under a finger the mark changes in the frame the selection lands, as
// UIKit's marks do. The keys are literal: an imported at-rule key lands in
// another StyleX priority layer.| import {selectionMotionConsts} from './selectionMotion.stylex'; | ||
| import type {IndicatorProps} from './types'; | ||
|
|
||
| /** How long the mark takes to change under a finger: one frame. */ |
There was a problem hiding this comment.
Non-blocking: this comment restates the constant. Naming it TOUCH_MARK_CHANGE says the same thing without a comment (the same applies in RadioIndicator.tsx).
| // Under a finger the mark changes in the frame the selection lands, as | ||
| // UIKit's marks do (motion/uikit/selection.ts). Under Reduce Motion a | ||
| // mouse's change is immediate too. | ||
| // The keys are spelled out: an imported at-rule key lands in another | ||
| // StyleX priority layer than a literal one. |
There was a problem hiding this comment.
Non-blocking: same as in CheckboxIndicator.tsx; the Reduce Motion sentence restates the narrowed key on lines 53–54.
|
Superseded: under the owner's ruling, main's indicators already play the theme duration on every pointer and change at once under Reduce Motion; SelectableCard's Reduce Motion arm lands in #8326. |
…y an at-rule prelude (#8695) * test: the test runtime keeps every StyleX rule under a multi-condition prelude StyleX 0.19.0's dev runtime keys a conditional rule whose prelude has more than one parenthesized condition by the prelude alone, so jsdom got only the first rule written under, say, @media (prefers-reduced-motion: reduce) and (pointer: coarse), and a test that read a later one read nothing. vitest.config.ts now names internal/test-utils/src/stylexInject.ts as the babel plugin's runtimeInjection: StyleX's own inject, plus each rule it drops, inserted once into a sheet of its own. Test-only: no StyleX code is patched, and the build keeps every rule as before. It comes out at the StyleX bump to a release that carries meta/stylex#1991, which fixes the key upstream. Red first: the new stylexMultiConditionMedia test (two styles, and one style with two declarations, under one two-condition query) failed with StyleX's own injection; on #8226 + #8682 + main, Indicator/selectionMotion.test.ts failed 2 of 6. Both pass with it, and the full ui suite on main has the same results (15,951 passing, plus the new test). * test: the StyleX test injection lets an insertRule error throw The setup caught and dropped any error from inserting a rule StyleX's dev runtime had dropped, so a rule jsdom refused would have gone missing without a trace. It never fired: the full ui suite passes with the catch removed (15,952 tests). A refused rule now throws with the rule's text. * test: the StyleX test injection restores every rule its dev runtime keys by a prelude alone Review: StyleX 0.19.0's dev runtime keys a rule by an at-rule's prelude alone not only under a top-level multi-condition query, but also under one nested in a one-condition query (`@supports (…) { @media (…) and (…) {…} }`), under a named container query, and under `@starting-style`; the injection restored only the first case, so a test that read a later rule under the others still read nothing. The injection now keys rules with a copy of StyleX's own getSeenRuleKey, which StyleX does not export, and restores every later rule whose key is an at-rule's prelude rather than a selector, once each. It also skips the group last-member rule, which the test setup refuses on purpose (its marker now lives in jsdomRefusedRules.ts, shared with setup.ts): a refusal of that rule threw out of the component's import. Other refusals still throw. And it records the first rule under a key as present, so the same rule compiled in two modules is not copied into the test sheet. * Merge remote-tracking branch 'origin/main' into vjeux/stylex-multi-condition * test: the StyleX test injection keeps its record across vi.resetModules() CodeBunny on #8695: the record of restored rules lived in module state, which vi.resetModules() resets while StyleX's injector keeps its keys, so restored rules were inserted again and a later rule imported first after a reset was lost. The record lives on globalThis; a guard case resets between fixtures under one prelude. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
User need
On a phone, a tapped checkbox or radio fades between its colours over 125 ms, so the first frames show a pale box or two half-filled radios. UIKit changes a selection mark in the frame the selection lands. Under a finger, on every touch device, Astryx's marks now do too.
Evidence
iOS 26.2 Mobile Safari (the magenta ring is the finger) and Android 15 Chromium 158 (the dot is the finger), recorded on this PR's earlier code; the engine runs in Testing prove this head. Before is main:
Problem and solution fit
selection.mark.changeMs(0), from a newselectionentry.Current specification
spec:AST-094FR1, FR2, FR13, FR24: measured values, the same on every touch platform, keyed on a coarse pointer, nothing changed under a mouse. The one-frame value stays an internal constant, because FR4 names no motion role for it.design:reduced-motionDR2, DR3.Public delta and caller burden
selectioninmotion/uikit, not exported from the package entry point.Design and alternatives
Its predecessors #8331 and #8076 are merged, so this branch is now one commit on main. #8246 is stacked on it.
Scope
Testing
Local gates at #8246's head 1d2f527c7f, which holds this tree (4707533, on main ce667be) plus only SelectableCard files:
pnpm build,pnpm api:report --check,pnpm typecheckandpnpm review:pre(at this head and at feat(SelectableCard): under a finger the selection ring and border change in one frame #8246's): clean.check:repo,check:changesets,check:knowledgeandcheck:i18n-runtime: clean.selectionMotion.test.tsfail on main's indicators.