Repository navigation
Conversation
vjeux
requested review from
cixzhang,
imdreamrunner and
josephfarina
as code owners
October 24, 2026 13:44
|
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.
|
Contributor
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsSelectableCard (@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. Checkbox pattern - 1 issue(s)
SelectableCard - 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 waits on an engineering owner's decision: the touch timing here rests only on the draft spec:AST-094 FR1, FR2 and FR4 that this pull request adds, and FR4 would narrow the current theme-tokens INV3 (components consume motion roles) to pointer presentations. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and you may do so yourself as an engineering owner. Independent of that decision, these need changes:
- Land or drop the #8226 stack and rebase, so this pull request is the 367-line SelectableCard commit rather than 13,990 authored lines (the rebase also clears the
pr-a11y"No tests found" failure). - Make the new
pr-interactionjob part of the existing UI test owner and itstestjoin, or leave it out, since AST-030 FR11 and FR7 rule out a separately routed lane whose failure fails no join. - Add the writing-rubric results for
spec:AST-094to the description, as the knowledge-contracts record requires for a new record. - Describe the touch arm as following a coarse primary pointer, not "a finger" versus "a mouse", in the Indicator docs and both changesets.
[Automated review]
| # them, so a change to an instrument is proven by its negative controls | ||
| # before a component spec relies on it. Chromium only on these runners; | ||
| # see the harness README for running WebKit. | ||
| pr-interaction: |
There was a problem hiding this comment.
This job routes itself (Check interaction scope) and is not in the test join, so a failing interaction proof fails none of the required joins. AST-030 FR11 asks new coverage inside an existing surface to join that surface's test owner instead of adding a separately routed lane, and FR7 asks an owned lane's failure to fail its join.
| // do (motion/uikit/selection.ts). The ring cannot interpolate with the | ||
| // card's resting shadow, so a fade would split it from the border. A | ||
| // mouse keeps the theme's duration. | ||
| '@media (pointer: coarse)': MARK_CHANGE, |
There was a problem hiding this comment.
(pointer: coarse) follows the device's primary pointer, not the input that made the selection, so a trackpad on a coarse-primary device also gets 0 ms and a finger on a fine-primary laptop keeps the fade. Whether touch marks may leave --duration-fast at all is the open decision on draft AST-094 FR1 and FR4.
vjeux
force-pushed
the
uikit/c06-card-mark
branch
from
October 24, 2026 22:52
feacfda to
c68e13e
Compare
Draft
4 tasks done
Contributor
|
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. |
Contributor
Author
|
Agreed: one implementation.
|
vjeux
force-pushed
the
uikit/c06-card-mark
branch
from
October 24, 2026 23:41
c68e13e to
a782f5b
Compare
vjeux
changed the title
Contributor
Author
|
Pushed in a782f5b: the card's mark keys on |
vjeux
force-pushed
the
uikit/c06-card-mark
branch
from
October 25, 2026 00:11
a782f5b to
7bb7354
Compare
Contributor
Author
|
Pushed in 7bb7354: the card's touch and Reduce Motion arms now import the shared keys ( |
vjeux
force-pushed
the
uikit/c06-card-mark
branch
from
October 25, 2026 01:01
7bb7354 to
c30794b
Compare
Contributor
Author
…lowance under every primary pointer The useMenuPress record's FR9 governs the trigger's held finger by spec:AST-094 FR29 under every primary pointer (FR43). The trigger now hands the long press FR29's 13 px drift allowance whatever the primary pointer, so it no longer reads (pointer: coarse) as its press starts, and the menus drop their coarsePointer import. ContextMenu keeps the long press's own allowable movement. The changeset says the same.
Strict ESLint reported six unused bindings in files this pull request carries: the host's HAPTIC_SWITCH_ATTRIBUTE import, two unused imports in the pan's tests (scrollProbe, ScreenEdgePanGestureRecognizer), the swipe test's drift, the presented-hit-testing test's gestures, and a GestureHost test's before, which now asserts what it was read for: a second finger's moves leave the pan where it was.
…eal angular speed The release velocity of a rotation whose listener sets rotation = 0 on each change, at 2 degrees every 16 ms, read 4.363 rad/s against the 2.182 rad/s of the same stream without a re-base: the test asserted only more than 1.5. It now asserts the 2.18 rad/s (spec:AST-100 FR2), and fails.
The rotation's velocity tracker samples the raw unwrapped angle, which setting rotation does not move, so the setter's shift of the samples read each re-base as extra turn and doubled the release velocity of a listener that re-bases on every change. The setter no longer touches the tracker.
…mates while it runs The proof read the element's animations 230 ms in, after the dip had ended, and asserted only that every animated key is opacity, which an empty list passes: with the cross-fade removed from the plan (the move still landing), the proof still passed. It now reads the keyframe keys right after the call starts and asserts they include opacity and nothing else; the same mutant fails it. Chromium and WebKit pass. (F1's in-flight read from c648034.)
…roller, and its claimed moves are cancelled under pointer events Two comparisons stop at the shadow boundary, each failing a case: - once the host's listener on a recognizer's inner node has seen the finger, isFingerDownIn reads it as outside a light-DOM scroller around that root, so a scroll under it is not the finger's (FR20); - under inputMode 'pointer-events', a claimed pan on a shadow host cancels none of its finger's moves over the inner content, since the finger's target is the inner node and its touches reach the host retargeted (FR19).
…the flat tree isFingerDownIn and the pointer-events finger match compare with containsAlongFlatTree, as the scroll watch already does: a finger on content inside an open shadow root is down in the scroller around that root, and its touches, retargeted to the shadow host, match its pointer finger, so a claimed pan cancels its moves there (spec:AST-094 FR19, FR20).
…achments asks, and only then The host let the last attachment decide whether an element is an action group (useGestureRecognizer always passes the option), and never recomputed it at a detach: a plain attachment after the group's turned the group off, and a group's attachment released left a recognizer that never asked taking presses under a fine pointer. Fails until the host counts the attachments that ask.
…ents asks The host counts the attachments that pass actionGroup: true and keeps the element a group while one remains, recomputed at each detach. An attachment that passes false, as useGestureRecognizer does by default, no longer turns a group off, and a group's last asking attachment taking its leave ends the group.
… that content The presented hit test takes the document's elementFromPoint, which answers a shadow host for anything inside its open root, and compares with Node.contains, which stops at the boundary: a recognizer on a control inside a moving sheet's shadow root gets no finger, and the tap's click goes to the host. Fails until the hit test descends into open roots and compares along the flat tree.
Where a moving element is drawn away from its layout box, the host hit-tests what is drawn under the finger with the document's elementFromPoint, which answers a shadow host for anything inside its open root. The hit test now descends through each open root's own elementFromPoint and compares with the movers along the flat tree, so a recognizer on a control inside a moving sheet's shadow root gets its finger and the tap clicks that control.
…nt lets go The host's touch-action rules sit in an adopted sheet or a style element per document or shadow root, and were never removed, so a page whose last recognizer detached kept them, against spec:AST-094 FR24 (nothing touch-related remains after the last instance unmounts). Fails until a root's rules go with its last marked element.
…th its last marked element The touch-action writer records which elements it marked in each document or shadow root, and when the last one is unmarked (its last recognizer detached) it removes that root's adopted sheet or style element, so nothing touch-related remains after the last instance unmounts (spec:AST-094 FR24). A later mark writes the rules again; an element marked in another root first lets go of the old one.
… the inline style it reads - The motion registry, the host's engine gate and the README name the presented hit test as an engine workaround (spec:AST-013 FR6) with its review condition (re-measure on each iOS release; remove it once iOS WebKit hit-tests a compositor-moved element where it is drawn) and its regression fixture. - While a curve runs the animator holds the value it starts from inline, so presentation.ts and the README no longer say the inline style holds the end.
…core/internal/touch Core ships the internal entry @astryxdesign/core/internal/touch, which re-exports the gesture primitives for Astryx's own packages, and the Lab-import guard fails a Lab import of core outside core's exports (architecture:react-component-runtime INV13). The README and the barrel's header named internal workspace imports and said nothing here is exported; they now name that entry.
…ill needs the element, and the listeners that cancel The release note, the stream's header, the host's comment and the README said the stream is one passive listener per event type and that a document-capture stopPropagation no longer starves the recognizers. Such a stop never hid a listener on the same node, and some subscriptions cancel. They now say what changed: an app's window-capture stopPropagation, or a document-capture stopImmediatePropagation registered before core, no longer hides a touch; a recognizer that must claim still needs the touch to reach its element; the gesture host's touchend, click and contextmenu cancel while a claimer is attached and the page hold's touchmove while it holds; and the stream stays ahead of an app's window-capture listener only until a type's listener is re-added. The README's list of blocking touchmove listeners gains the claimed touch's target and the document while a claiming element moves.
…default it moves
The {stylexOptions} form of astryxStylex now passes StyleX
treeshakeCompensation: true unless the caller sets it. spec:AST-017 FR3
asks the release note to name the old and new default and the option
that restores the old one; it now does.
…er and for the two stops the input stream fixes - A still finger with sub-pixel jitter over a scroll view keeps its tap and scrolls nothing (spec:AST-094 FR18–FR22). - A tap is still recognized under an app's window-capture stopPropagation, and under a document-capture stopImmediatePropagation registered before core's: the recognizers hear touches on the window's capture phase. Chromium and WebKit pass.
…s once Where nothing that claims is attached, the host's touchend listener is passive and cannot cancel the engine's own click after a retargeted tap clicks what is drawn. The shared click guard swallows that late click whatever is attached; this case pins it, as asked on meta#8293.
This pull request sat on meta#8073's old head and carried old copies of the harness meta#8045 and meta#8046 landed on main. meta#8360 carries that foundation with main's harness, so after the merge this pull request's own diff is its three commits (the touch style keys, internal, and the press paint's priority over the coarse-pointer :active reset). The merge is clean.
The touch keys are internal, and core spells these conditions as literal keys wherever a style needs them. The stylex file's header called it the one place core spells them and cited spec:AST-012 FR2, and a test title said "for other packages": the header now names the keys as the reference spellings each literal is pinned to (spec:AST-094 FR13, spec:AST-012 FR9), and the test holds the same keys the resolver defines.
…r, not the devices A changeset tells consumers what changes, so it names the condition (a coarse primary pointer) and no longer claims "every touch device"; the simulator and emulator evidence stays in the pull request's description.
…t the devices A changeset tells consumers what changes, so it names the condition (a coarse primary pointer) and no longer claims "every touch device"; the simulator and emulator evidence stays in the pull request's description.
4 tasks done
There was a problem hiding this comment.
Approval waits on an engineering owner providing or identifying the governing spec for the one-frame (0 ms) selection mark under a coarse pointer: spec:AST-094 FR4 requires every duration a touch presentation plays to be a motion role, and no current component record gives SelectableCard, CheckboxIndicator or RadioIndicator a touch presentation, so an owner either accepts the mark as it stands or asks for selection.mark.changeMs to become a registered role, with the component records stating the presentation, first. 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, these changes are needed:
- Land #8226 and the rest of its stack first and rebase, so this pull request carries the 612-line card change instead of 28,981 lines.
- Correct
SelectableCard.doc.mjs: with a mouse only the border fades (and not under Reduce Motion), while the ring changes at once. - Add before-and-after mouse evidence for the Reduce Motion change, as
spec:AST-017FR59 asks.
[Automated review]
| name: 'isSelected', | ||
| type: 'boolean', | ||
| description: | ||
| 'Controlled selection state. Under a finger (a coarse primary pointer) the selection ring and border change in one frame; with a mouse they fade over the theme’s fast duration.', |
There was a problem hiding this comment.
With a mouse only the border fades: the inset ring cannot interpolate with the card's resting shadow, so it changes at once (the comment at SelectableCard.tsx:90–91 says the same), and under Reduce Motion the border changes at once too on this head. Suggested wording: "with a mouse the border fades over the theme’s fast duration (at once under Reduce Motion) and the ring changes at once."
| // once (design:reduced-motion DR3); a finger's change 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 part that waits on an engineering-owner decision: spec:AST-094 FR4 asks every duration a touch presentation plays to be a motion role read through its role, and selectionMotionConsts.markChangeMs mirrors a table value that is not a registered role. The literal @media (pointer: coarse) key itself conforms to FR13 (DEC-6).
vjeux
marked this pull request as draft
October 29, 2026 22:34
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #8226. Review only this PR's change: vjeux/astryx@bf74a5b...f69f26c.
User need
On a phone, tapping a SelectableCard changes its selection mark in two steps. The inset ring appears (or vanishes) at once, while the border fades over the theme's fast duration (125 ms), so for several frames the card that lost the selection still shows a gray outline. UIKit changes a selection mark in the frame the selection lands. Under a finger, on every touch device, the card now does too.
Problem and solution fit
Under a coarse primary pointer (spelled as
touchKeys.touch) the card's box-shadow and border-color transition takesselection.mark.changeMs(0), through the mirror the checkbox and radio marks read (#8226), so ring and border change together. A mouse keeps the theme's fade and, under Reduce Motion, changes at once (spelled asreducedMotionKeys.instant).Current specification
spec:AST-094FR1, FR13, FR24, with feat(core): under a finger a selection mark changes in one frame #8226's FR4 note.design:reduced-motion(current) DR2, DR3.motion/uikit/selection.ts, feat(core): under a finger a selection mark changes in one frame #8226): a collection-view list's multiselect accessory, the closest counterpart to a selectable card, changes without animation.Public delta and caller burden
analyze-pr.js).Design and alternatives
Evidence
A tap moving the selection from Basic to Pro, ¼ speed: iOS 26.2 Mobile Safari (the magenta ring is the finger; before is #8226) and Android 15 Chrome (before is main). Before, Basic's border fades after Pro's ring appears; after, both cards change in one frame:
Scope
Testing
CI=true pnpm lint0 errors;pnpm buildthen rootpnpm typecheckclean; the full core suite (16 workers) passes with no failure (686 files, 16,764 tests); SelectableCardMark and SelectionMarks pass in Chromium and WebKit and TabPressPointer in Chromium (14 and 12 cases); a mouse click still selects a card. The node project's only reds are main's:story-tree.test.ts(main's A11y story titles, fixed in test(storybook): run the story-tree check where Storybook is built, and bring the tree back to its shape #8509) andcache.test.mjs's two npm-lock cases (a Node started under another file name, fixed in fix(cli): a crashed run's release install is recognized by the Node it ran on, whatever its name #8520).CI=true pnpm lint0 errors;pnpm buildthen rootpnpm typecheckclean; SelectableCardMark and SelectionMarks in Chromium 12/12 and WebKit 12/12; units 16,081 (test(Item): wait for the swipe layer's module, not a one-second window #8352's swipe warm-up timed out under load and passes alone; CodeBlockHeaderPlugin's plaintext-fence case is test(richtext): the code block header tests wait for the copy button's tooltip #8569's race); a mouse click still selects a card.MultiSelectorbottom-sheet test timed out on a loaded box: its lazy BottomSheet's first load outlasts the 1 s query, which test(Selector, MultiSelector): wait for the on-demand sheet module before the first open #8642 preloads; it passes alone and in its file); SelectableCardMark and SelectionMarks in Chromium 12/12 and WebKit 12/12;lab-core-importspasses;pnpm buildthen rootpnpm typecheckclean;CI=true pnpm lint0 errors; a mouse click still selects a card.lab-core-importspasses;pnpm buildthen rootpnpm typecheckclean;CI=true pnpm lint0 errors; a mouse click still selects a card.pnpm buildthen rootpnpm typecheckclean;CI=true pnpm lint0 errors; a mouse click still selects a card.pnpm vitest run SelectableCard: 14 pass.SelectableCardMark.test.tsxchecks that the coarse-pointer arm takes the table's 0 ms with no platform gate, that a mouse keepsvar(--duration-fast), and that a mouse under Reduce Motion takes0sin the policy's instant key.pnpm test:interaction SelectableCardMark, Chromium and WebKit (the official Playwright image), 5/5 each: under a finger a tap creates no transition and the first computed ring and border are final on both cards; every frame of a tap shows each card wholly selected or not (storyboard); the mouse keeps the border transition; under Reduce Motion a finger and a mouse change at once.