Repository navigation
fix(core): a mouse or Space press on a touch-first device paints the pressed state - #8046
Merged
Merged
Conversation
vjeux
requested review from
cixzhang,
imdreamrunner and
josephfarina
as code owners
October 24, 2026 03:29
|
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
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
October 24, 2026 03:29
Contributor
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent 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: No accessibility violations detected. Visual Regression48 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 28 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
4 tasks done
Touch behaviours key on a coarse primary pointer, layout behaviours on a compact viewport, and presentations that need both (a sheet in place of a popover) ask for both. utils/touchPresentation.ts holds the three queries, SSR-safe hooks (useTouchPresentation; useMotionPresentation for components with no interaction to infer from: UIKit motion under a finger, theme tokens under a mouse) and a one-moment read. Styles write the same queries as StyleX media keys, and a test keeps core's compact-touch keys spelled as the shared query. A compact viewport is narrower than 744 px or shorter than 500 px, the closest a size gets to UIKit's compact size class: an iPad mini in portrait keeps the anchored popover and a phone in landscape gets the sheet; a Max-size iPhone in landscape is the known miss. The adaptive popover-or- sheet presentation reads the shared query; its docs, stories and tests follow.
This was referenced Oct 24, 2026
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
The mouse-press restoration works and leaves desktop unchanged, but approval waits on one owner decision and one fix.
- No current spec covers a held Space painting the CheckboxInput, Switch or RadioList indicator (desktop engines disagree), so an engineering owner settles it by commenting
/approve-spec <full head SHA>on this pull request, which the author may do as an engineering owner, or asks for the key path to paint only a focused element that is itself the pressable surface. - A mouse press dragged across DropdownMenu rows now keeps the pressed row painted beside the focused row, which
useMenuPressFR6 and DEC-6 rule out, so leave surfaces inside adata-astryx-menu-pressroot to the menu's own highlight.
[Automated review]
Comment on lines
+101
to
+108
| const SPACE_ACTIVATED_INPUT_TYPES: ReadonlySet<string> = new Set([ | ||
| 'button', | ||
| 'checkbox', | ||
| 'image', | ||
| 'radio', | ||
| 'reset', | ||
| 'submit', | ||
| ]); |
There was a problem hiding this comment.
checkbox and radio here make a held Space paint the pressable row around the focused input (CheckboxInput, Switch, RadioList). Chromium and WebKit desktop leave that row at rest for the same key press, because only the input matches :active. Firefox paints it, and no current spec settles which is intended, so this part waits on an owner decision.
Contributor
Author
There was a problem hiding this comment.
Fixed in 798d04e: Space now paints only a focused button that is itself the pressable surface, so the row around a checkbox or radio stays at rest, as in desktop Chromium and WebKit. Tests: the unit case "leaves the row around a focused checkbox or radio at rest" and the real-browser case "Space on a focused checkbox leaves its row at rest, as on a desktop". The browser case fails on the previous head and passes on main and here.
| // is fine; the touch treatment's CSS is inside `@media (pointer: coarse)`. | ||
| // Not tracking it there keeps desktop identical and costs the mouse path | ||
| // nothing. Only the primary button: that is the press `:active` paints. | ||
| if (event.button === 0 && stylesheetDroppedActive()) { |
There was a problem hiding this comment.
On a coarse primary pointer this also paints DropdownMenu rows, and buttonHeld keeps that paint through the drag, so a press dragged from one row to another leaves two rows lit. That conflicts with useMenuPress FR6 ("At most one row of a menu is lit"), and DEC-6 dropped the coarse-pointer row press paint for that reason. Skipping surfaces inside a [data-astryx-menu-press] root keeps menu rows on the focus highlight alone.
Contributor
Author
There was a problem hiding this comment.
Fixed in 798d04e: the mouse and Space path skips surfaces inside [data-astryx-menu-press], so the menu's highlight stays its only paint (FR6, DEC-6). Tests: the unit case "leaves a menu row to the menu" and the real-browser case "a mouse press dragged across menu rows lights only the row under the pointer". The browser case fails on the previous head and passes on main and here.
This was referenced Oct 24, 2026
A touch presentation is now also an idiom: ios for WebKit on iOS or iPadOS
under a coarse pointer (CSS.supports('-webkit-touch-callout: none'), which
no other engine implements), default everywhere else, Android included,
which keeps today's behaviour until a Material variant exists.
useTouchPresentation and readTouchPresentation report it, useTouchIdiom
reads it alone, and useMotionPresentation now chooses UIKit motion only in
the iOS idiom. Styles gate the same way with two nested StyleX keys, the
media query outside and the supports query inside (IOS_TOUCH_STYLE_KEYS),
as core's 16px input floor already does; a test keeps every such key in
core spelled and nested that way. A real-engine spec reads Chromium as an
Android phone and WebKit as default.
…documented behavior change - The resolver answers the same on every engine: the iOS idiom (useTouchIdiom, useMotionPresentation, the idiom field) is dropped, so nothing chooses a platform; touch motion reads the theme's motion roles. The iOS style-gate constants stay, internal, for a style iOS WebKit itself requires (core's 16px input floor), and their spelling test stays. - The changeset names the old and new default (sub-menu drill-in included) and the presentation prop that pins a component's presentation.
cixzhang
pushed a commit
that referenced
this pull request
Oct 24, 2026
## User impact In apps compiling Astryx from source with cascade layers (the Vite and PostCSS integrations, Storybook), a finger held on a ClickableCard, SelectableCard or interactive Thumbnail showed no pressed state; the precompiled stylesheet painted it. Now every build paints it on the first frame and fades it at the lift; mouse states are unchanged. | Finger held, then lifted (iOS 26.2) | Before | After | | --- | --- | --- | | ClickableCard |  |  | ## Problem and solution fit - **Why it occurs:** the cards wrote the touch press as an attribute arm of a custom property (`'--_press-overlay': {default: 'transparent', '[data-astryx-press="on"]': …}`). StyleX's layered output leaves its lowest priority group (every custom property's own declarations) unlayered and puts attribute arms in a priority layer. Unlayered wins whatever the specificity, so the default always won. - **Why it harms:** the finger gets no confirmation; a selection card toggles on release with no feedback during the hold. - **Why it matters:** one source painted differently by build, and jsdom (no cascade layers) passed. Plain `::after` properties keyed on the attribute now paint it, outranking the defaults in layered and flat stylesheets alike. ## Expected behavior and authority - The cards' docs: the overlay paints the pressed token under a finger (`data-astryx-press="on"`). - `usePressFeedback` docs: a marked surface paints the pressed token on the first frame of a believed press and fades it over the release. - History: the touch press model added these arms, asserted only in jsdom. ## Smallest restoration - ClickableCard, SelectableCard, Thumbnail: the custom-property press arms and `--_press-overlay-transition` go; on the press arms the `::after` gets `background-image: var(--_press-paint-image)`, `background-color: transparent` (so a fine pointer's `:active` cannot paint twice) and `transition-property: none`. - No API, token or mouse-path change. - Touches neighboring press code and `ci.yml` with #8046; whichever lands second rebases. ## Evidence - Before: in Storybook a held touch leaves each card at rest (255, pressed is 229); on the iOS simulator the controller writes `on` and the card stays white. - After: each card paints the desktop press's pixels, fades at the lift, and paints once beside `:active` (229, not 206). - A source guard (no custom property pairs its default with an attribute arm) and per-card jsdom assertions fail on the base; the browser contract fails 3 of 6 there. ## Scope - [x] One defect is restored. - [x] Separable contradictory or unsettled tagalongs were removed or split. - [x] No new public API, visual representation, default, or interaction model is hidden under the bug-fix label. - [x] Public text and artifacts contain no internal Meta context. ## Testing - `pnpm vitest run packages/core/src/ClickableCard packages/core/src/SelectableCard packages/core/src/Thumbnail packages/core/src/utils/customPropertyArmLayering.test.ts packages/core/src/theme/derivedVarRegistry.test.ts`, and the full core unit suite. - `pnpm exec playwright test packages/core/src/__tests__/CardPress.a11y.chromium.spec.ts --project chromium`: 9 passed; 3 fail on the base build, and a build without the colour yield fails the `:active` case with a double paint. Wired into `ci.yml`. "a desktop mouse is unchanged: hover, `:active`, release, leave" (per card) passes on the base build and here. - iOS simulator, Mobile Safari 26.2 (iPhone 17 Pro; the clips: iPhone 11), a real touch hold on ClickableCard: on the base `data-astryx-press="on"` is written and the `::after` paints nothing; after, it paints the pressed token and clears 200 ms after the lift. Text selection is off on the story root to keep iOS's long-press selection out of the frame. This is the WebKit leg: the contract's touch input is CDP, which only Chromium has. - `pnpm build:css`: the flat stylesheet orders the new arms above their defaults. - `pnpm -F @astryxdesign/core typecheck`, `pnpm check:changesets`, CI routing tests, lint and prettier on touched files.
vjeux
force-pushed
the
uikit/c01-coarse-press
branch
from
October 24, 2026 21:59
1e1aeec to
798d04e
Compare
Brings in the theme compact definition (spec:AST-012 FR9) this branch's resolver reads.
COMPACT_QUERY and COMPACT_TOUCH_QUERY are the theme's compact definition (spec:AST-012 FR9: width below the md point, 768 by default, or height below 500, the same on every platform), so useAdaptivePresentation and the sub-menu drill-in keep no size numbers of their own. Tests, docs, stories and the changeset follow: a coarse-pointer screen exactly 768 wide keeps the popover, and a phone in landscape gets the sheet.
…t width The previous commit's formatter pass rewrapped the whole file; this keeps the file as it was and changes only 744px to 768px in both languages.
There was a problem hiding this comment.
This looks ready to merge: on touch-first devices a held mouse, trackpad or Space press now shows the same pressed state as on desktop, desktop presses are unchanged, and both issues from the previous revision (Space painting checkbox, radio and switch rows, and a second lit row while dragging across a menu) are fixed. Two optional touch-ups: the usePressFeedback docs say the controller paints "every press" and "a Space-key press", while the key path only paints a focused native button that carries the marker and nothing inside a menu, so naming that condition as the changeset does would keep them exact; and the description's test counts predate the last commit (13 new unit tests fail on the base, and pressFeedback.test.ts now has 51 cases).
[Automated review]
| guidance: true, | ||
| description: | ||
| 'Let the composed style own the press on every pointer: it keeps `:active` for a mouse and drops it under `@media (pointer: coarse)`, so a finger sees only the press model. A rule of your own that paints `:active` would paint under a finger too, on the touch and through a scroll.', | ||
| 'Let the composed style own the press on every pointer: it keeps `:active` for a desktop mouse and drops it under `@media (pointer: coarse)`, where the controller paints every press (a finger through the press model, a mouse or Space key at once). A rule of your own that paints `:active` would paint under a finger too, on the touch and through a scroll.', |
There was a problem hiding this comment.
Optional wording: the controller doesn't paint every press here. The key path paints only a focused native <button> (or button-like <input>) that itself carries the marker, and neither the mouse nor the key path paints inside a [data-astryx-menu-press] root (pressFeedback.ts:342-345). Naming that condition, as the changeset already does, keeps the docs exact for a local <div role="button"> pressable.
Contributor
Author
|
Both are already in this head (103b1f4; fix commit 0933a68):
If either still reproduces, which surface and input does it show on? |
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
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.
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
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.
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
Main brings meta#8046, which gives the press controller a slot per press so a mouse or Space press on a touch-first device paints too. The conflict is the release timer: main's slot-based timer keeps meta#8661's tempo factor, so every press's release fade follows the theme's tempo, a mouse or Space press's included. A new case pins that: it fails with main's side of the timer. The no-icon-set search goldens conflict too; they are regenerated with experimental-isolation.test.mjs's documented --update (main's 369 matches plus this pull request's motion docs), and the diff names no icon set and nothing experimental.
3 tasks done
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
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.
cixzhang
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
# 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
cixzhang
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
# 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
cixzhang
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
# 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
cixzhang
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
# 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
cixzhang
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
# 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
cixzhang
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
# 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/cli/assets/docs/author-a-theme.doc.mjs # packages/cli/test/__golden__/no-icon-set/docs-topics.json # packages/core/src/utils/pressFeedback.ts
cixzhang
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
# 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
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
With main's coarse press for buttons (meta#8046) and the row press release (meta#8077) in this line, tapping the story's Insert button plays the button's own release on the next frame. The insert spec's one-frame check read every animation on the page, so it failed on that release (one start 16.7 ms after the others) while every animation of the update itself started on the same frame. It now reads the animations whose target is inside the list, whatever drives them: the new row's entrance and the glides of the four rows after it.
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
…tion to its tracker meta#8046 made the controller paint a mouse press itself where the primary pointer is coarse (a tablet with a trackpad). On an action its group tracks (family:action-groups), the group's tracker paints that press, so the controller must paint nothing there, as it already does for a finger; it paints the press, then turns the tracker's paint to 'fading' at the release. A Space press on the same action still paints: the group tracks pointers only (guard).
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
… its tracker Where the primary pointer is coarse the controller paints a mouse press itself (meta#8046). It now skips an action a group's tracker paints for a pointer's press, a mouse's included, as it already did for a finger, so the two never write the press attribute over each other. A Space press on the action still paints: the group tracks pointers only.
Draft
3 tasks done
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 28, 2026
…) into the touch-down highlight Takes meta#8077's press controller as it now stands (main's finger and button slots, meta#8046's mouse-and-Space press, and the release that ends with its animation) and keeps only this PR's behavior on it: under a finger a surface highlights in the frame the finger lands unless a scroller sits above it, where it waits the content-touch delay (spec:AST-094 FR11). That is pressGesture's down.delayed and the controller's isInsideScroller, with their tests. The two press profiles go: they had become identical, meta#8077 moved the row's release to its stylesheet, so Item and TreeList keep meta#8077's pressable props. This PR's listener arming and brake rewrite are not carried either; meta#8077's own listeners and brake stand.
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 29, 2026
…head Main's merges of meta#8045 and meta#8046 (the frame-evidence harness and the coarse press) conflict with the older foundation heads this branch carries, in the interaction harness files. meta#8106's current head carries that main and the foundation it now rests on, so taking it leaves this branch clean against main with nothing resolved by hand. A clean merge.
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 29, 2026
This pull request sat on meta#8076 and carried its old copies of the harness meta#8045 and meta#8046 landed on main. meta#8360 carries meta#8076 with main's harness, so after the merge this pull request's own diff is the presentation resolver's commits. The one conflict, the gestures README, keeps both sides: this pull request's paragraph on the presentation prop ends the touch presentation section, and meta#8360's migration section follows it.
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 29, 2026
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.
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 29, 2026
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 29, 2026
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Oct 29, 2026
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.
Review the fix commit; the two merge commits bring in main.
User impact
On a touch-first device with a trackpad or keyboard, a mouse press or Space on a focused button showed no pressed state on any shared-press control. Now those presses paint what a desktop press paints, hold while down, and fade at release.
Labels: the page's press state and events.
Problem and solution fit
:activearm under@media (pointer: coarse)and paints only finger presses. That query reads the primary pointer, so it drops:activefor a mouse and Space too.Expected behavior and authority
family:buttons: Button owns "focus and press feedback";activeis a shipped visual state.design:user-statesDR1: a pointer hold on the listed controls MUST paint--color-overlay-pressed.usePressFeedbackdocs keep:activefor a mouse.Smallest restoration
utils/pressFeedback.ts: a second slot for a primary mouse button or Space, used only where the shared touch resolver's key matches (TOUCH_QUERY, AST-012 FR2). It paints at once, ignores travel (as:activedoes), fades on the release clock, and drops onpointercancelordragstart. As on a desktop, Space paints only a focused button that is the surface itself; menu rows keep the menu's highlight (useMenuPressFR6).utils/pressGesture.ts:buttonDownandbuttonHeld.design:reduced-motionDR2).Evidence
:activepress's pixels and scale, then fades; a drag start drops it at once.Scope
Testing
pnpm vitest run packages/core/src/utils/pressFeedback.test.ts packages/core/src/utils/pressGesture.test.ts(49 + 29), and the full core unit suite.pnpm exec playwright test packages/core/src/__tests__/PressModality.a11y.chromium.spec.ts --project chromium(6 passed; 2 fail on the base build; the menu-row and checkbox-row cases pass on main and failed on this PR's previous head), routed by its own@pr-a11yscope line (Button and the press machinery), which the PR accessibility runner reads:pnpm test:a11y-contract:pr origin/main --dry-runlists it. The same spec ran locally in WebKit inside the Playwright container: the mouse, Space and desktop cases pass (the finger case needs CDP and skips there).(pointer: coarse)matches, the condition this fix keys on. A hardware Space key sends no key events to a page whose focused element is a button (re-checked on iOS 26.2 while recording), and the simulator turns host clicks into touches, so those paths are proven in Chromium and WebKit; the clips above are Chromium recordings (Playwright video, real mouse and keyboard input) of the base build and this one.pnpm typecheck,pnpm check:repo,pnpm check:changesets, CI routing tests, lint and prettier on touched files.Stack (merge in this order): this PR → #8117 → #8118.