Repository navigation
Conversation
|
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
meta-cla
Bot
added
the
CLA Signed
![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 is waiting on a spec decision. Its branch adds the draft spec:AST-094 (FR1–FR23, DEC-1, DEC-2), which also proposes amending current architecture:theme-tokens, design:user-states DR1 and DR3, family:buttons and spec:AST-025 FR14. An engineering owner needs to approve or reject it, and update or decline those records, by commenting /approve-spec <full head SHA> here; you can record that yourself if you are an engineering owner. Whatever that decision, these points need changes:
- Restack once the predecessors land, so this pull request carries only the reduced-motion commit instead of 41 commits and about 29,000 authored lines.
- Button and the three cards now import
../motion/reducedMotion.stylex, which core's exports map does not list, soastryx swizzleproduces an unresolvable import and CI'stest-node (4/4)fails (swizzle.test.mjs:153). - On a coarse pointer,
design:reduced-motionDR2 asks for an opacity-only fade, but Button and the cards still transition colours, borders and shadow (Button.tsx:113–114,SelectableCard.tsx:80), and the DR2 test checks only durations. - AST-094 FR8 (a 0.5 s navigation fade, sheets and menus on springs, a throw that keeps moving) contradicts
design:reduced-motionDR1 and DR2 and should be conformed to them.
[Automated review]
| import type {BaseProps} from '../BaseProps'; | ||
| import type {Elevation, SizeValue} from '../utils/types'; | ||
| import * as stylex from '@stylexjs/stylex'; | ||
| import {reducedMotionFadeConsts} from '../motion/reducedMotion.stylex'; |
There was a problem hiding this comment.
astryx swizzle rewrites this import to @astryxdesign/core/motion/reducedMotion.stylex, but core's exports map does not list that subpath, so a swizzled Button (and ClickableCard, SelectableCard, Thumbnail) gets an import that does not resolve. CI's test-node (4/4) fails on it at packages/cli/api/swizzle/swizzle.test.mjs:153. Moving the constants into an exported .stylex module (for example utils/interactionOverlay.stylex.ts, which already holds pressConsts) would fix it.
| default: 'background-image, background-color, color, opacity, transform', | ||
| // Under reduced motion nothing scales, so nothing transitions transform. | ||
| '@media (prefers-reduced-motion: reduce) and (pointer: coarse)': | ||
| 'background-image, background-color, color, opacity', |
There was a problem hiding this comment.
Under (prefers-reduced-motion: reduce) and (pointer: coarse) this keeps background-image, background-color and color transitioning for 200 ms. design:reduced-motion DR2 requires these transitions to "change opacity only", and its verification row fails "a property other than opacity". The policy's own example in motion/reducedMotion.ts (lines 24–28) uses 'opacity' alone for this key.
| @@ -77,8 +78,22 @@ const styles = stylex.create({ | |||
| ':is(:disabled,[aria-disabled="true"])': 'default', | |||
| }, | |||
| transitionProperty: 'box-shadow, border-color', | |||
There was a problem hiding this comment.
With the new coarse-pointer duration at lines 89–90, box-shadow and border-color now animate for 200 ms under reduced motion, but DR2 allows only opacity. Carrying the change on opacity (for example, fading the existing ::after by opacity) would conform.
| on: a transition that travels becomes a cross-fade carrying the same change | ||
| of state. An entrance lands in place and fades in, an exit fades out in | ||
| place, and any other move fades through. A navigation push or pop | ||
| cross-fades over 0.5 s on the ease-in-out curve; a sheet, a full-screen |
There was a problem hiding this comment.
FR8 asks for a 0.5 s navigation cross-fade, sheets that fade on their presentation spring, menus on a Reduce Motion spring, and a throw that keeps its motion. Touch presentations key on a coarse pointer (FR13), where the current design:reduced-motion record requires an opacity-only change of 200 ms or less, including for a transition the release of a touch drag starts (DR2), and forbids springs (DR1). Please conform FR8 to DR1 and DR2.
| try { | ||
| const {element} = await transitionsOf(button.page, 0); | ||
| expect(element).toBeGreaterThan(0); | ||
| expect(element).toBeLessThanOrEqual(200); |
There was a problem hiding this comment.
This proves the length of the DR2 fade but not that it is opacity-only. Also reading transitionProperty on the element and on its ::after, and expecting only opacity, would catch the colour transitions at Button.tsx:113–114.
| '@astryxdesign/cli': patch | ||
| --- | ||
|
|
||
| [feat] Themes can retune touch motion. `defineTheme({motion})` takes `springs`, `durations` and `curves` keyed by motion role, the table path of a value touch presentations play (`springs.sheetPresent`, `timing.reducedMotionCrossFadeMs`), at the root or in an adaptation rule. A role the theme leaves out keeps its measured default, and the duration scale (`fast`, `medium`, `ratio`) is optional when a theme sets only roles. Adaptation rules gain a `platform` condition, `ios` or `other`. `astryx docs motion` lists every motion role with its CSS variable and default, and the theme authoring guide and template describe the new fields. |
There was a problem hiding this comment.
timing.reducedMotionCrossFadeMs is excluded from the roles (motion/uikit/timing.ts:158), and .changeset/reduced-motion-record.md says it is no longer a theme role. Naming it here tells theme authors they can lengthen the Reduce Motion fade, which design:reduced-motion DR4 forbids.
| @@ -247,9 +255,15 @@ export interface DefineThemeInput { | |||
| * // Snappy: { fast: 100, medium: 250, ratio: 0.75 } | |||
| * // Default: { fast: 175, medium: 410, slow: 975, ratio: 0.75 } | |||
| * // Cinematic: { fast: 200, medium: 500, slow: 1200, ratio: 0.7 } | |||
| * | |||
| * // A softer sheet and a slower reduced-motion fade: | |||
There was a problem hiding this comment.
The example sets navigation.transitionMs, but its caption promises "a slower reduced-motion fade", which DR4 rules out and the code no longer allows.
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsButton (@astryxdesign/core) · View in Storybook
ClickableCard (@astryxdesign/core) · View in Storybook
SelectableCard (@astryxdesign/core) · View in Storybook
Thumbnail (@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: 5 accessibility violation(s) found — 5 serious. Button pattern - 1 issue(s)
Checkbox pattern - 1 issue(s)
ClickableCard - 1 issue(s)
SelectableCard - 1 issue(s)
Thumbnail - 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 |
65b726d to
ffc29bb
Compare
ffc29bb to
474f6e3
Compare
474f6e3 to
f639ec3
Compare
There was a problem hiding this comment.
This pull request is waiting on a spec decision: its branch adds the draft spec:AST-094 (FR1–FR23, DEC-1, DEC-2), which proposes amendments to current architecture:theme-tokens, architecture:interaction-modality, design:user-states DR1 and DR3, family:buttons and spec:AST-025 FR14, and a new stable @astryxdesign/core/motion/reducedMotion.stylex export that its own code marks as pending owner review. An engineering owner needs to approve or reject them by commenting /approve-spec <full head SHA> on this pull request; you can record that yourself if you are an engineering owner. Whatever that decision, these points need changes:
- Restack once the predecessors land, so this pull request carries only the reduced-motion commit instead of 69 commits and about 30,600 authored lines.
- On a coarse pointer,
design:reduced-motionDR2 asks for an opacity-only fade, but Button and the cards still transition colours, borders and shadow (Button.tsx:113–114,SelectableCard.tsx:80), and the DR2 test checks only durations. - AST-094 FR8 (a 0.5 s navigation fade, sheets and menus on springs, a throw that keeps moving) contradicts DR1 and DR2, as do the
springs.menuReducedMotiontheme role and the animator changeset's spring fade. - The theme template documents a
platformadaptation axis thatdefineThemenow rejects (packages/cli/assets/theme.template.ts:361–362).
[Automated review]
| * `from` includes its point; `below` excludes it. The other supported axes | ||
| * are primary-pointer precision, contrast preference, and motion preference. | ||
| * are primary-pointer precision, contrast preference, motion preference, and | ||
| * platform (`ios` for the browsers of iPhone and iPad, `other` for the rest). |
There was a problem hiding this comment.
This sentence documents a platform adaptation axis, but the stack removed it: CONDITION_AXES in packages/core/src/theme/themeAdaptations.ts ends at motion, and assertAllowedKeys throws ….when.platform is not supported. A theme author who follows the template gets an error from defineTheme. Restoring the base wording ("primary-pointer precision, contrast preference, and motion preference") would fix it.
| * iPhone 17 Pro, `_animateUsingSpringWithDampingRatio:response:tracking:` | ||
| * hooked with Reduce Motion on (1.0, 0.4), Oct 23 2026 | ||
| */ | ||
| menuReducedMotion: {response: 0.4, dampingRatio: 1}, |
There was a problem hiding this comment.
springs.menuReducedMotion is registered as a theme motion role (it appears in astryx docs motion as --motion-springs-menu-reduced-motion). design:reduced-motion DR4 says a theme value offers no way to keep springs under reduced motion, and DR1 forbids springing; excluding it from the roles, as timing.ts does for reducedMotionCrossFadeMs, would keep the theme surface within DR4.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] The touch presentations' animator plays any closed-form curve (such as a spring chasing a moving target), drives a pseudo-element such as a dialog's ::backdrop, fades on a presentation's own spring under Reduce Motion (with an entrance or exit role for a slide that changes no opacity), and starts a call from rest when the browser commits it, as Core Animation does. Not exported from the package root; touch presentations reach it through the experimental subpaths (see their entry). |
There was a problem hiding this comment.
This release note says the animator "fades on a presentation's own spring under Reduce Motion", while .changeset/reduced-motion-record.md says it cross-fades in 200 ms and removes call options that no release ever shipped. Both land in the same release; describing only the final behaviour would keep the notes consistent with design:reduced-motion DR1 and DR2.
| @@ -177,6 +190,13 @@ const STATIC_EXPORTS = { | |||
| types: './dist/utils/interactionOverlay.stylex.d.ts', | |||
| default: './dist/utils/interactionOverlay.stylex.js', | |||
| }, | |||
| // Proposed; pending owner review (spec:AST-094 OQ4): the Reduce Motion | |||
There was a problem hiding this comment.
This new stable export is justified by "spec:AST-094 OQ4", but the record has only OQ1 and OQ2. The subpath sits outside any experimental boundary, so once released it falls under the stable compatibility promise; it needs an owner decision, and the citation should point at what actually covers 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.
spec:AST-090 does not exist on main or on this branch, so FR7's "departs from the draft spec:AST-090 FR1 … and FR10" (and the same citation under Current-state impact) points at nothing an owner can read.
| const read = new Set( | ||
| [ | ||
| ...css.matchAll( | ||
| new RegExp(`var\\(\\s*(--(?:${names})[a-z0-9]+)\\s*[,)]`, 'g'), |
There was a problem hiding this comment.
Matching --(?:astryx|x)[a-z0-9]+ also catches a product's own variables such as --xl or --xsmall read from a StyleX style and declared in global CSS; the build then warns that the declaration "would be dropped in every browser", or fails with undefinedVariables: 'error'. Deriving the generated names from the StyleX rules the build collected would avoid the false report.
f639ec3 to
f876650
Compare
There was a problem hiding this comment.
This pull request is waiting on an engineering-owner decision: its stacked commits add public theme fields (motion.springs, motion.durations, motion.curves), an undefinedVariables option for astryxStylex, and an internal touch animator, gesture system and interaction harness, none of which a current spec covers. Approval needs an engineering owner to identify or provide the governing spec for these additions, or to approve them as proposed, by commenting /approve-spec <full head SHA> here; you can record that yourself as an engineering owner. Whatever that decision, these points need changes:
- Restack once the predecessors land, so this pull request carries only the reduced-motion commit instead of about 29,700 authored lines.
- On a coarse pointer,
design:reduced-motionDR2 asks for an opacity-only fade, but Button and the cards still animate colour, border and shadow (Button.tsx:112–113,SelectableCard.tsx:79), and the contract checks only durations. - The velocity tracker offers four selectable filters and keeps a non-zero release after a rest (
VelocityTracker.ts:43,VelocityTracker.test.ts:99), which the currentspec:AST-100FR2 andarchitecture:interaction-modalityINV10 rule out. springs.menuReducedMotionshould not be a theme role under DR4, and the new build check also reports an app's own variables such as--xlas undeclared (packages/build/src/vite.ts:411–432).
[Automated review]
| * header describes, `uikit` by default. | ||
| * @experimental | ||
| */ | ||
| export type VelocityStrategy = 'uikit' | 'lsq2' | 'iosFling' | 'window'; |
There was a problem hiding this comment.
spec:AST-100 FR2 and architecture:interaction-modality INV10, current on main, allow one release filter (iOS's blend) and require a release velocity of 0 after the contact stays still longer than the stillness interval. This union adds three alternative filters, PanGestureRecognizer's velocityStrategy option exposes them, and the default "never decays" (line 19). Keeping only the blend, adding the stillness reset and dropping the option would conform.
| }, | ||
| ); | ||
|
|
||
| it('uikit reports the last moving value after a rest, as UIKit does', () => { |
There was a problem hiding this comment.
This pins the release spec:AST-100 FR2 rules out: its verification row fails "a non-zero release after a still pause". Once the tracker resets after the stillness interval, this case should expect 0 for a lift after a pause longer than that interval.
| }, [ref, recognizer, manageStyle]); | ||
|
|
||
| useIsomorphicLayoutEffect(() => { | ||
| recognizer.setEnabled(isEnabled); |
There was a problem hiding this comment.
When isEnabled turns false mid-gesture, setEnabled cancels the recognizer (GestureRecognizer.ts:557–573); cancelled is an action state, so the listener above calls the caller's onGesture from inside this layout effect. architecture:react-component-runtime INV2 keeps consumer callbacks out of effects, and INV3 asks a new effect to name the external system it synchronizes.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The experimental touch animator follows design:reduced-motion. Under Reduce Motion every `animate` and `play` (travel or opacity, on a spring or any curve) becomes an opacity-only cross-fade of 200 ms on a coarse primary pointer, or for a call marked `touchRelease`, and completes at once on any other pointer; nothing moves, scales or springs, and travel lands with a plain style write while the element is transparent, never as an animated transform. `timing.reducedMotionCrossFadeMs` is now 200 ms and is no longer a theme role. The `reducedMotion` and `crossfade` call options are removed; `crossfadeAs` stays. `animator.reducedMotion()` reports the mode for feedback with no fade of its own, `reducedMotionFade()` gives the same fade to code beside the animator, and `reducedMotion.stylex.ts` holds its constants for StyleX transitions. |
There was a problem hiding this comment.
The animator is internal with no experimental subpath, and no release shipped the reducedMotion and crossfade options this note says are removed. Describing it as internal, as the sibling gesture-* and uikit-* changesets do, would keep the release notes accurate.
| "rootDir": "./src", | ||
| "paths": { | ||
| "@astryxdesign/core/motion/*": ["../core/dist/motion/*"], | ||
| "@astryxdesign/core/gestures": ["../core/dist/gestures/index"], |
There was a problem hiding this comment.
No Lab file imports these paths at this head, architecture:interaction-modality INV10 says Lab cannot import the velocity tracker that @astryxdesign/core/gestures exposes, and core's exports map lists none of these subpaths, so a Lab import would not resolve for npm consumers. Dropping this block and its twin in tsconfig.build.json until Lab needs an exported path would avoid that.
There was a problem hiding this comment.
Changes requested: under reduced motion on a touch screen, Button, ClickableCard, SelectableCard and Thumbnail still animate colour, border colour and shadow for 200 ms, while design:reduced-motion DR2 allows only an opacity cross-fade there. Please carry those state changes on opacity (for example by fading the ::after overlay) and have the DR2 browser test assert transition-property as well as duration. The diff against main also carries about 19,800 lines from the stacked predecessors listed in the description (#8045, #8056, #8061, #8067, #8071–#8074, #8258, #8076) and the build fix merged into them (#8075), so please land those first so this pull request shows only its own change. The press-scale and fine-pointer fixes behave as intended in Storybook.
[Automated review]
| // DR4). | ||
| '@media (prefers-reduced-motion: reduce) and (not (pointer: coarse))': | ||
| '0s', | ||
| '@media (prefers-reduced-motion: reduce) and (pointer: coarse)': '200ms', |
There was a problem hiding this comment.
Under (prefers-reduced-motion: reduce) and (pointer: coarse) the border keeps transition-property: border-color (L181) for 200 ms, and the ::after overlay's background-color (L118–119) gets the same 200 ms at L134–135. design:reduced-motion DR2 asks for an opacity-only change on a coarse pointer, and its verification row fails “a property other than opacity”. Carrying the border and overlay change on a layer whose opacity fades under this arm would conform.
| // (motion/reducedMotion.stylex.ts), spelled out because that module is | ||
| // internal and a swizzled component imports only exported paths; | ||
| // motion/reducedMotionValues.test.ts holds them equal. | ||
| '@media (prefers-reduced-motion: reduce) and (pointer: coarse)': |
There was a problem hiding this comment.
As in ClickableCard, this arm gives the overlay's background-color transition (L221–222) 200 ms on a coarse pointer, where design:reduced-motion DR2 allows only opacity. Fading the overlay by opacity under this arm would conform.
startCompositorTrace keeps every Display::FrameDisplayed in the browser, and those events carry no page identity: with a second page animating, a stall inside the block passed busyWindowProof with presented, crediting the other page's frames. It now throws when another page is open as it starts or stops, and says the record is browser-wide (JSDoc and README). Negative control: another animating page makes it refuse to start, and a page opened mid-trace makes stop() refuse.
A bare relative ASTRYX_WEBKIT_SHADER_CACHE was mounted as a named volume instead of a host directory, and an empty one made mkdirSync throw. The script now resolves a set value with path.resolve and falls back to the default cache directory on an empty one.
…e pointer, and no menu observes the pointer query (useMenuPress FR9; spec:AST-094 FR24) Where the primary pointer is fine, a finger held on a DropdownMenu trigger keeps the released behavior, whose tolerance is 10 px on each axis: an (8, 8) settle opened the menu, and the long press's distance rule now cancels it. And every useMenuPress caller, each menu and sub-menu, subscribed to (pointer: coarse) though only the trigger reads it, when its press starts. Both fail until the trigger takes its tolerance at the press.
…rts, and no menu observes the pointer query (spec:AST-094 FR29, FR24; useMenuPress FR9) Every useMenuPress caller, each menu and sub-menu, subscribed to (pointer: coarse) through useSyncExternalStore, though only the trigger's held finger reads it. The trigger now reads the pointer as its press starts and hands useLongPress that press's tolerance (start's new tolerance argument): FR29's 13px drift where the primary pointer is coarse, the long press's own 10px where it is fine, as the released behavior keeps (on each axis, with meta#8358's measure). A changeset declares the 13px a finger now gets on phones and tablets.
…entation says useLongPress documents its tolerance as movement past moveCancelPx on either axis, a released contract, and ContextMenu's measured hold and the menu trigger's released behavior keep it. This pull request had changed it to a distance, which cancels a (9, 9) drift the contract keeps. Fails until the measure is the documented one again.
…ocumented; only the defaults have one source useLongPress documents movement past moveCancelPx on either axis, and its callers keep that: ContextMenu's measured hold is per axis, and the menu trigger's released behavior keeps the hook's tolerance (useMenuPress FR9). The defaults, 500ms and 10px, still come from the long-press recognizer's table, which measures its own tolerance as a distance, as UIKit's does. The changeset now says what this is: one source for the numbers, no behavior change; useLongPress.doc.mjs was right all along.
…adow root; a claimed touch's moves are cancelled where fingers come from pointer events With targets from the composed path, a finger inside an open shadow root sits below the scroller only along the flat tree, which Node.contains does not follow: a scroll under it did not take it, and its flick did not arm the scroll brake. And where the host reads fingers from pointer events (inputMode 'auto' with no ontouchstart), the blocking touchmove found no finger for a claimed pan's touch, so its moves went to the page. Three cases fail until the host compares along the flat tree and finds that finger; a fourth pins a finger scroll inside a shadow root.
… tree; a claimed touch finds its pointer finger wherever fingers come from pointer events containsAlongFlatTree walks parentAcrossShadow, so a finger inside an open shadow root, now that targets come from the composed path, counts as under a scroller outside it: a scroll there takes it, its flick arms the scroll brake, and a touch there brakes. cancelIfClaimed matches a touch to its pointer finger whenever the host reads fingers from pointer events, not only under inputMode 'pointer-events', so a claimed pan's moves are cancelled where a browser that exposes no ontouchstart still sends Touch Events.
… flat tree, and a claimed touch finds its pointer finger wherever fingers come from pointer events) into meta#8320
…r along the flat tree, and a claimed touch finds its pointer finger wherever fingers come from pointer events) into meta#8293
…handlers type UseLongPressHandlers.start takes a point only again, as released: the per-press tolerance useMenuPress's trigger chooses as its press starts lives on the internal useLongPressDetector and LongPressDetectorHandlers, which the hooks entry does not export. useLongPress returns the same detector, typed as before.
|
Posted by Cindy's agent on her behalf: closed and reopened only to restart this PR's CI on the current workflow after tonight's queue-wide cancel; nothing in the PR changed. |
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.
# 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 # packages/core/src/utils/pressFeedback.ts
meta#8076's new head carries main 70ea859 with meta#8073's writer of the press attribute on main's press controller, and its README names both writers. utils/pressFeedback.ts keeps that controller with this line's own changes on it: the pointer, drag, scroll and visibility listeners through the shared input stream (meta#8356) and touchcancel only while the primary pointer is coarse (meta#8270); the key path stays at the window's bubble phase, so a Space press the page cancels paints nothing. usePressTracker stays in its own module, one tracker for a component's life (meta#8315), so meta#8073's rebuilt-tracker hook and its test are not taken. The harness files are meta#8076's.
…s, and the action-group tracker writes its own The header and writePress called it the one writer of the press attribute, but main's action-group tracker writes its groups' presses itself. They now say what holds, as meta#8076's gestures README does.
…he press attribute The release note said every press painter writes data-astryx-press through one internal writer, but the action-group tracker (hooks/actionGroupTracker.ts) writes its groups' presses itself, as the gestures README and utils/pressFeedback.ts already say. The note now says paintPress writes every press it tracks and the action-group tracker writes its own.
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: under Reduce Motion on a touch screen, ClickableCard, SelectableCard and Thumbnail now switch their press and selection colours at once with no cross-fade, which relies on reading design:reduced-motion DR2's in-place value exception (DEC-3), written for continuous values such as a progress fill, as covering colour state changes. An engineering owner records that ruling by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; otherwise the cards need an opacity cross-fade of 200 ms or less. The Button press-scale fix and the cards' mouse and trackpad fix read correctly against component:Button FR15 and DR1 and DR3. Independently of that decision:
- Please land the stacked predecessors first, or rebase onto
main, so this pull request carries only its own seven-file change instead of about 27,300 lines. - In the stack, the gesture host attaches recognizers and press trackers only where the primary pointer is coarse, while current
spec:AST-094FR19 and FR43 require gestures for every pointer (packages/core/src/gestures/GestureHost.tsL14–22).
[Automated review]
| * - Recognizers attach, and the host registers listeners, only while the | ||
| * primary pointer is coarse (`spec:AST-094` FR19, FR24; coarsePointer.ts). | ||
| * Where it is fine, an attach is recorded but nothing is installed: no | ||
| * document, window or element listener, no `touch-action` or other style, | ||
| * nothing delivered, so a finger on a touchscreen laptop gets the | ||
| * browser's defaults. The host follows the query: turning coarse installs | ||
| * it and styles every registration; turning fine cancels every touch in | ||
| * flight and removes it all. A development warning names the first attach | ||
| * under a fine pointer. |
There was a problem hiding this comment.
Current spec:AST-094 FR19 reads "Recognizers attach for every pointer, with FR43's exceptions", and FR43 "Gestures attach for every pointer, a mouse, a pen, and a finger, not only a coarse primary pointer". This gate, and the development warning further down that cites FR19 for it, keeps every recognizer and press tracker inactive on a desktop. Attach for every pointer with FR43's exceptions (row swipe under a mouse drag, pinch under a pen, screen-edge pan under a mouse, trackpad or pen), keeping FR24's fine-pointer listener budget.
| // Reduced motion on a touch screen: only opacity may cross-fade | ||
| // (design:reduced-motion DR2), so a selection's ring and border change | ||
| // at once. | ||
| '@media (prefers-reduced-motion: reduce) and (pointer: coarse)': |
There was a problem hiding this comment.
On a touch screen with Reduce Motion this leaves only opacity to transition, and no hover, press or selection state changes opacity, so the selection ring and border colour switch at once. design:reduced-motion DR2 asks for an opacity cross-fade of a state change on a coarse pointer unless it is an in-place value change (DEC-3); whether these colour changes count as one is waiting on an engineering owner's ruling.
Review this PR alone: vjeux/astryx@uikit/f2-click-guard...uikit/c01-reduced-motion
User impact
Part of the UIKit-parity work on Astryx's buttons. For people who turn on reduced motion, a pressed Button still shrinks to 98%, and Button's and the cards' hover and press colors still fade, border included. The setting asks for neither.
Problem and solution fit
What led to it:
design:reduced-motionbecame current, and checking Button and the cards against it found that Button's press scale has no reduced-motion rule and the cards keep their color, border and shadow transitions on every pointer.Under reduced motion, Button stays full size and animates nothing. The cards change at once with a mouse; on a touch screen they cross-fade opacity only, within 200 ms, and their colors, border and selection ring change at once.
Expected behavior and authority
design:reduced-motionDR1 (no scaling), DR2 (a touch screen may cross-fade opacity within 200 ms), DR3 (any other pointer changes at once), DR4 (no theme opt-out); #8645 DEC-3.Smallest restoration
Tradeoff: the values repeat in each component, held by that test. No API change.
Evidence
Scope
Testing
To check by hand: in Storybook, turn on reduced motion (the OS setting, or DevTools' "Emulate prefers-reduced-motion"). Press a Button with the mouse: it doesn't shrink and its color changes at once. With touch emulation, tap a card: only its overlay fades, within 0.2 s; selecting a SelectableCard changes its border and ring at once.
What was run: a Chromium test under emulated reduced motion reads the computed transforms and transitions. Before this change a pressed Button scaled and the cards ran color transitions; after, it passes. Unit tests fail if any reduced-motion value drifts from the shared policy, and a swizzle test checks that a swizzled Button's imports resolve. These ran locally on this head; GitHub Actions isn't running on this repository right now.
Button.tsx and the cards' overlay also change in #8118; whichever lands second rebases.
Stack: on #8360, which carries #8076 and the gesture foundation.