Skip to content

fix(core): gesture hooks follow their element; recognizer plumbing is protected - #8315

Closed
vjeux wants to merge 1052 commits into
meta:mainfrom
vjeux:uikit/f2-hook-fixes
Closed

vjeux wants to merge 1052 commits into
meta:mainfrom
vjeux:uikit/f2-hook-fixes

Conversation

@vjeux

@vjeux vjeux commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Depends on #8309. Review only this pull request's change: vjeux/astryx@58dbb1a...2e77071

Why

Astryx's gesture hooks called a component's handlers at the wrong times. A recognizer unmounted mid-gesture reported cancelled from React's cleanup, and a press tracker's timer could call back after unmount, so a component could set state on something already gone. A touch that stopped a flicking list still counted as a press by default, and a component that changed a press tracker's options after mount was ignored.

What led to it

This is part of the UIKit-parity work on Astryx's touch components. Reviews of #8073, #8076, a context menu and a number input found these. React's rules for Astryx say a component's callback never fires from an Effect or from cleanup (architecture:react-component-runtime INV2, INV3).

Options and tradeoffs

  • Document the timing and let each component guard its own handlers: every caller repeats the guard, and one that forgets breaks quietly.
  • Recreate the tracker whenever an option changes: simple, but a press in progress would be dropped.
  • Fix it in the two hooks: silence them from the moment a component unmounts, and keep one tracker for the component's life that reads its options through refs.

Why this choice

The hooks are the only place every component passes through, so one fix covers all of them, and a press in progress survives an option change. Each recognizer's internal plumbing becomes protected, so components can no longer call it by accident; the arena and the host reach it through one internal accessor. A press stopping a flick no longer presses, because the brake now defaults to the host's.

How to check it

  1. Unmount a component while one of its gestures is mid-drag. Expected: its handler hears nothing after unmount.
  2. On a phone, flick a list of pressable rows and touch it to stop the scroll. Expected: no row activates.
  3. Change a press tracker's options while a finger is down on it. Expected: the press continues and the new options apply from the next read.
  4. Disable a control under a resting finger, then lift. Expected: the press ends silently, with no late activation.

Evidence

  • Each case fails before the change, including a type test showing the recognizer plumbing is no longer callable from outside a subclass.
  • The existing hook, arena and browser suites pass unchanged.

@vercel

vercel Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Oct 27, 2026 10:38am UTC

Request Review

@astracat-bot

astracat-bot Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
❓ Needs maintainer input (for maintainers only) Oct 29, 2026, 2:49 PM UTC

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 24, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on an engineering owner's decision on the draft spec:AST-094, which this diff adds as the only authority for the gesture and motion system: an engineering owner records it by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), and should also settle whether FR4's "physics are not themable" or the new theme motion roles win. Independently of that decision, these changes are needed:

  • Rebase onto main once the predecessor pull requests land, so this pull request carries only its 15-file hook fix instead of 29,286 changed lines.
  • Build the compact-touch query from the theme's exported compact definition (spec:AST-012 FR9) instead of the hardcoded 744px width in touchPresentation.ts.
  • Drop the platform adaptation condition, or amend the closed condition vocabulary of spec:AST-012 FR2 first, and if the theme motion roles stay, refuse role ids the motion tables do not define.
  • End an in-flight press with touchCancel when usePressTracker replaces its tracker or element, and add null-detachment and StrictMode tests for both hooks.

[Automated review]


/** Both: presentations such as a sheet in place of a popover. */
export const COMPACT_TOUCH_QUERY =
'(width < 744px) and (pointer: coarse), (height < 500px) and (pointer: coarse)';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This keeps its own width (744px). Current spec:AST-012 FR9 defines compact as width below the theme's effective md point (768 by default) or height below 500, and says no resolver keeps size numbers of its own; main now exports that definition as compactDefinition(). After rebasing, build these queries from it, or get FR9 amended first.

* is everything else. Pair `ios` with `pointer: 'coarse'` for a touch
* profile, such as touch motion tuned per platform.
*/
platform?: 'ios' | 'other';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spec:AST-012 FR2 closes the when vocabulary (width, height, pointer, contrast, motion) and says future environment fields need an explicit addition to it. A platform axis needs that amendment before it ships as a stable theme field.

) {
return;
}
attached?.detach();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this runs mid-press (an element swap or a changed profile option), the old tracker's dispose() clears touchId without emitting, so the consumer sees touchDown but never touchUpInside, touchUpOutside or touchCancel. Ending a tracked press with touchCancel before disposing (the unmount flag already silences it on unmount) would keep the control-event stream balanced.

`${location}: a motion role is named by its table path, such as "sheet.present" or "springs.navigationPush".`,
);
}
const registered = motionRole(id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

motionRole(id) only finds roles whose table module was imported, and no theme module imports one, so in defineTheme this check never fires; together with the shape-only isMotionRoleId, a misspelled role ('springs.sheetPresnt') is accepted and writes a variable nothing reads. Validating against a static list of the tables' roles would turn that into an error.

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR

🧪 Sandbox Preview

View Sandbox for this PR

Component Size

Component Import before Import after Delta CSS before CSS after Delta
core/AlertDialog 56.9KB 57.2KB +294B 12.4KB 12.4KB 0B
core/AppShell 57.9KB 58.1KB +242B 11.6KB 11.6KB 0B
core/AvatarGroupOverflow 39.9KB 40.2KB +303B 6.4KB 6.4KB 0B
core/Banner 47.8KB 48.1KB +289B 9.9KB 9.9KB 0B
core/BaseTypeahead 54.3KB 54.6KB +251B 9.9KB 9.9KB 0B
core/BreadcrumbItem 66.4KB 66.6KB +297B 11.0KB 11.0KB 0B
core/Button 43.3KB 43.5KB +273B 9.2KB 9.2KB 0B
core/Calendar 54.9KB 55.2KB +290B 10.0KB 10.0KB 0B
core/Carousel 50.2KB 50.5KB +316B 10.2KB 10.2KB 0B
core/ChatComposer 60.6KB 60.9KB +270B 10.8KB 10.8KB 0B
core/ChatComposerInput 55.7KB 56.0KB +264B 10.1KB 10.1KB 0B
core/ChatComposerTokenElement 55.7KB 56.0KB +264B 10.1KB 10.1KB 0B
core/ChatDictationButton 46.5KB 46.8KB +264B 9.5KB 9.5KB 0B
core/ChatLayout 48.8KB 49.1KB +273B 9.9KB 9.9KB 0B
core/ChatLayoutScrollButton 46.4KB 46.6KB +255B 9.6KB 9.6KB 0B
core/ChatSendButton 45.8KB 46.1KB +265B 9.2KB 9.2KB 0B
core/CheckboxInput 49.9KB 50.2KB +273B 9.3KB 9.3KB 0B
core/CheckboxListItem 54.0KB 54.3KB +244B 10.5KB 10.5KB 0B
core/ClickableCard 12.8KB 13.1KB +289B 6.9KB 6.9KB 0B
core/CodeBlock 60.5KB 60.8KB +278B 10.7KB 10.7KB 0B
core/Collapsible 11.7KB 12.0KB +291B 4.2KB 4.2KB 0B
core/CommandPalette 50.9KB 51.2KB +281B 11.8KB 11.8KB 0B
core/CommandPaletteItem 8.4KB 8.7KB +280B 3.5KB 3.5KB 0B
core/ComplexSelector 55.8KB 56.1KB +318B 10.6KB 10.6KB 0B
core/ContextMenu 77.7KB 78.0KB +275B 14.6KB 14.6KB 0B
core/ContextMenuItem 16.6KB 16.8KB +280B 5.2KB 5.2KB 0B
core/DateInput 99.9KB 100.1KB +243B 14.8KB 14.8KB 0B
core/DateRangeInput 69.4KB 69.6KB +266B 11.2KB 11.2KB 0B
core/DateTimeInput 110.1KB 110.3KB +248B 15.2KB 15.2KB 0B
core/DialogHeader 48.8KB 49.1KB +280B 9.8KB 9.8KB 0B
core/DrawerHeader 48.9KB 49.2KB +249B 11.2KB 11.2KB 0B
core/DropdownMenu 72.7KB 73.0KB +278B 14.7KB 14.7KB 0B
core/DropdownMenuCheckboxItem 16.1KB 16.4KB +284B 5.6KB 5.6KB 0B
core/DropdownMenuItem 15.4KB 15.7KB +287B 5.2KB 5.2KB 0B
core/DropdownMenuRadioItem 16.2KB 16.5KB +275B 5.6KB 5.6KB 0B
core/DropdownMenuSubMenu 58.1KB 58.3KB +270B 9.5KB 9.5KB 0B
core/FileInput 54.1KB 54.4KB +262B 10.3KB 10.3KB 0B
core/IconButton 43.3KB 43.6KB +273B 9.2KB 9.2KB 0B
core/Item 11.4KB 11.6KB +273B 4.7KB 4.7KB 0B
core/LayerProvider 63.3KB 63.6KB +288B 10.1KB 10.1KB 0B
core/Lightbox 51.2KB 51.4KB +249B 9.8KB 9.8KB 0B
core/Link 43.5KB 43.8KB +288B 7.6KB 7.6KB 0B
core/ListItem 53.5KB 53.7KB +268B 10.5KB 10.5KB 0B
core/Markdown 144.5KB 144.7KB +247B 17.4KB 17.4KB 0B
core/MobileNav 51.0KB 51.3KB +264B 10.1KB 10.1KB 0B
core/MobileNavToggle 46.2KB 46.5KB +259B 9.4KB 9.4KB 0B
core/MoreMenu 72.9KB 73.2KB +278B 14.7KB 14.7KB 0B
core/MultiSelector 76.2KB 76.4KB +270B 14.6KB 14.6KB 0B
core/NumberInput 55.7KB 56.0KB +278B 10.7KB 10.7KB 0B
core/Outline 28.6KB 28.9KB +290B 4.0KB 4.0KB 0B
core/Pagination 87.7KB 88.0KB +263B 15.3KB 15.3KB 0B
core/Popover 49.8KB 50.1KB +238B 12.1KB 12.1KB 0B
core/PowerSearch 155.1KB 155.3KB +263B 18.6KB 18.6KB 0B
core/RadioListItem 52.1KB 52.4KB +286B 9.7KB 9.7KB 0B
core/SegmentedControlItem 8.6KB 8.9KB +285B 3.9KB 3.9KB 0B
core/SelectableCard 12.8KB 13.1KB +294B 7.1KB 7.1KB 0B
core/Selector 79.1KB 79.4KB +268B 15.0KB 15.0KB 0B
core/SelectorOption 14.5KB 14.8KB +251B 5.1KB 5.1KB 0B
core/SideNav 58.0KB 58.3KB +272B 10.6KB 10.6KB 0B
core/SideNavCollapseButton 46.6KB 46.9KB +242B 9.5KB 9.5KB 0B
core/SideNavHeading 54.7KB 55.0KB +248B 10.1KB 10.1KB 0B
core/SideNavItem 54.5KB 54.8KB +266B 10.1KB 10.1KB 0B
core/Step 32.4KB 32.7KB +279B 5.3KB 5.3KB 0B
core/Stepper 47.4KB 47.7KB +265B 9.6KB 9.6KB 0B
core/Switch 48.4KB 48.7KB +285B 9.3KB 9.3KB 0B
core/Tab 9.7KB 10.0KB +294B 4.1KB 4.1KB 0B
core/TabMenu 51.6KB 51.9KB +276B 10.0KB 10.0KB 0B
core/Table 84.0KB 84.2KB +253B 15.1KB 15.1KB 0B
core/TableCell 78.8KB 79.1KB +267B 14.8KB 14.8KB 0B
core/TableHeaderCell 78.6KB 78.8KB +272B 14.8KB 14.8KB 0B
core/TableSelectionToolbar 52.4KB 52.6KB +278B 11.2KB 11.2KB 0B
core/TextInput 53.5KB 53.8KB +281B 10.5KB 10.5KB 0B
core/Thumbnail 50.7KB 50.9KB +274B 11.0KB 11.0KB 0B
core/TimeInput 86.8KB 87.1KB +266B 13.8KB 13.8KB 0B
core/Timestamp 27.5KB 27.5KB -1B 9.5KB 9.5KB 0B
core/Toast 59.8KB 60.1KB +258B 9.7KB 9.7KB 0B
core/ToggleButton 44.1KB 44.4KB +243B 9.3KB 9.3KB 0B
core/Token 29.6KB 29.9KB +289B 4.5KB 4.5KB 0B
core/Tokenizer 64.8KB 65.1KB +306B 11.7KB 11.7KB 0B
core/TopNav 51.9KB 52.1KB +277B 10.4KB 10.4KB 0B
core/TopNavHeading 54.2KB 54.4KB +231B 9.9KB 9.9KB 0B
core/TopNavItem 9.0KB 9.3KB +283B 3.8KB 3.8KB 0B
core/TopNavMegaMenu 54.2KB 54.4KB +268B 10.8KB 10.8KB 0B
core/TopNavMegaMenuItem 9.1KB 9.4KB +281B 4.1KB 4.1KB 0B
core/TopNavMenu 53.6KB 53.8KB +258B 10.1KB 10.1KB 0B
core/TreeList 32.5KB 32.8KB +274B 4.7KB 4.7KB 0B
core/Typeahead 61.5KB 61.7KB +268B 11.3KB 11.3KB 0B

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 Summary

Gzip figures measure each package's entry file, not the code an import pulls in.

Package Entry (ESM) Entry gzip before Entry gzip after Delta
@astryxdesign/core 4.9KB 1.2KB 1.2KB 0B
Package stylesheet Gzip before Gzip after Delta
@astryxdesign/core 34.6KB 34.6KB 0B

Accessibility Audit

Status: No accessibility violations detected.

Visual Regression

55 of 502 shot(s) changed.

A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.

component story theme mode pixels
Selector Indicator Space Bottom Sheet Narrow End RTL neutral light 753,933
Selector Indicator Space Bottom Sheet Wide Start neutral light 740,072
RichTextEditor Markdown parity: side by side probe dark 558,840
RichTextEditor Markdown parity: side by side probe light 540,756
Chat Full AI Chat probe light 230,823
Chat Full AI Chat probe dark 229,805
AppShell With Mobile Nav probe light 217,010
AppShell With Mobile Nav probe dark 216,960
Collapsible Dividers probe dark 66,445
Collapsible Dividers probe light 66,445
AppShell Playground neutral dark 65,198
AppShell Playground neutral light 63,788
Chat Default neutral dark 36,304
Chat Default neutral light 35,767
Chat Empty State probe light 23,531
Chat Empty State probe dark 22,622
ChatAutoScroll Scroll Behavior Comparison neutral dark 15,378
ChatAutoScroll Scroll Behavior Comparison neutral light 14,107
Chat With Attachments probe light 13,406
Chat With Attachments probe dark 11,232

and 35 more.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on engineering-owner decisions: the draft spec:AST-094, which this diff adds as the only authority for the gesture and motion system, needs an engineering owner to record a decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), including whether AST-094 FR4 and DEC-1 or the new theme motion roles win, and the new stable @astryxdesign/core/motion/reducedMotion.stylex subpath needs an engineering owner to name its governing spec, or it should move behind the experimental boundary. Independently of those decisions, these changes are needed:

  • Rebase onto main once the predecessor pull requests land, so this pull request carries only its 15-file hook fix instead of 31,705 authored lines.
  • Keep a usePressTracker press alive when its tracker is rebuilt (an element swap, an option change, or a new ref object on a re-render), or end it with touchCancel, and add re-render, null-detachment and StrictMode tests for both hooks.
  • Keep the text-selection and iOS callout lock out of the stable press controller that Link, Button and other components use, or file it as a stable change, since an [experimental] changeset cannot change stable behaviour (spec:AST-017 FR22, FR27).
  • Drop the platform adaptation condition or amend spec:AST-012 FR2 first, read the theme's effective md point in the touch resolver (FR9), and correct the motion-roles and experimental-subpaths changesets.

[Automated review]

@@ -283,6 +301,9 @@ function onPointerDown(event: PointerEvent): void {
gesture: idlePressGesture(),
pointerId: event.pointerId,
timer: null,
// A held finger never starts text selection on the surface (UIKit's
// controls and cells never do), on any coarse pointer.
unlock: lockSelection([element], event.target),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes the stable press controller set -webkit-touch-callout, -webkit-user-select and user-select to none on every usePressFeedback surface (Link, Button, Item, ClickableCard, …) for any touch or pen press, so on iOS a long press on an Astryx Link no longer offers the link callout, and text in a pressable row can no longer be selected with a long press. That is a stable runtime change, which spec:AST-017 FR22 and FR27 keep out of an [experimental] changeset. Keep the lock to the experimental gesture host's press-tracked controls, or file it as a stable change under owner-approved authority.

Comment thread scripts/sync-exports.js Outdated
@@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AST-094 at this head has only OQ1 and OQ2, so this OQ4 citation points nowhere, and ./motion/reducedMotion.stylex ships as a stable public subpath (no /experimental, no @experimental marker, under a [fix] changeset). Either give it a governing record an engineering owner approves, or move it behind the experimental boundary (spec:AST-017 FR21, FR27) or keep it internal to core.

},
}),
[
ref,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With ref in these dependencies, a caller that passes a new ref object on each render (the case the new "a new ref object" test covers) gets a new tracker on every render. If it paints from onHighlightChange with setState, the re-render after touchDown disposes the tracker that owns the touch, the arena hands the lift to that disposed tracker, and no touchUpInside arrives. Keying the tracker on the element (or updating one tracker in place) and ending a tracked press with touchCancel before disposal would keep taps working; a re-render-mid-press test would catch it.

Comment thread .changeset/touch-motion-theme-roles.md Outdated
'@astryxdesign/cli': patch
---

[feat] Themes can retune touch motion roles. `defineTheme({motion})` takes `springs`, `durations` and `curves` keyed by motion role, the table path of a measured touch motion value (`springs.sheetPresent`, `timing.reducedMotionCrossFadeMs`), at the root or in an adaptation rule, and writes one `--motion-*` variable per role it sets. A role the theme leaves out keeps its measured default, an unknown role id is refused, and the duration scale (`fast`, `medium`, `ratio`) is optional when a theme sets only roles. No shipped component reads the roles yet. Adaptation rules gain a `platform` condition: `ios` for the browsers of iPhone and iPad, 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timing.reducedMotionCrossFadeMs is not in MOTION_ROLE_KINDS at this head (reduced-motion-record.md says it is no longer a theme role), so a theme that follows this note throws "is not a motion role". Name a role the generated list has, such as navigation.transitionMs.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holding for an engineering-owner decision on actionGroup: it lets an action group's recognizers take mouse, pen and finger presses where the primary pointer is fine, which spec:AST-094 FR19 and architecture:react-component-runtime INV13 currently rule out, and the family:action-groups FR5 it cites is not on main yet (#8622). Approval needs an engineering owner to provide or identify the governing spec for that addition, either by landing #8622 with an FR19/INV13 exception or by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision, please land #8309 first or rebase onto it, so this pull request carries only its own hook change (about 1,400 lines) instead of the whole stack (about 20,200), and correct the gestures README and barrel, which still send Lab through workspace imports that INV13 and the stack's own Lab-import guard rule out.

[Automated review]

* primary pointer is fine too (`AttachOptions.actionGroup`).
* @default false
*/
actionGroup?: boolean;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actionGroup turns on recognizers and listeners where the primary pointer is fine, which spec:AST-094 FR19 and architecture:react-component-runtime INV13 currently rule out, and the family:action-groups FR5 it cites is still proposed in #8622. It needs that record (with the exception) or an engineering owner's decision before it can merge.

Comment on lines +3 to +6
A gesture recognizer system with UIKit's semantics, for the touch
presentations of Astryx core components. Components import it from
`../gestures`, and Lab through internal workspace imports; nothing here is
exported from the package.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

architecture:react-component-runtime INV13 says Core and Lab reach these modules through @astryxdesign/core/internal/touch, and scripts/lab-core-imports.test.mjs fails any Lab import around core's exports, so "Lab through internal workspace imports" points Lab authors at an import the guard rejects. The barrel's header (index.ts:7-8) says the same.

An attach reads the element's own will-change in a task of its own. A
shared animator (animatorFor) is never detached, so its node can leave the
document with that read still waiting, and the read then forces a style
pass on a node with no style: a test run that mounts and unmounts many
pagers piled up such reads until a later wait timed out. The read now
skips a node out of the document; a promotion that needs it reads it then,
as one on a node attached in the same task does. A detach and dispose
already cancel it.

The new test fails on the previous code (one read of a removed node); the
in-task read test now puts its element in the document, where the read
happens.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holding for an engineering-owner decision on actionGroup: it lets an action group's recognizers take mouse, pen and finger presses where the primary pointer is fine, which spec:AST-094 FR19 and architecture:react-component-runtime INV13 currently rule out, and the family:action-groups FR5 it cites is not on main yet (#8622). Approval needs an engineering owner to provide or identify the governing spec for that addition, either by landing #8622 with an FR19/INV13 exception or by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision:

  • Land #8309 first or rebase onto it, so this pull request carries only its own hook change (about 1,400 lines) instead of the whole stack (about 19,700).
  • If actionGroup stays, keep an element a group when a second hook on it leaves the option out; today the last attachment wins (GestureHost.ts:382).
  • Correct the gestures README and barrel, which still send Lab through workspace imports that INV13 and the stack's own Lab-import guard rule out.

[Automated review]

): () => void {
const registration = this.registrationFor(element);
registration.manageStyle = options.manageStyle ?? registration.manageStyle;
registration.actionGroup = options.actionGroup ?? registration.actionGroup;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actionGroup is stored once per element, but each attachment sets it: useGestureRecognizer always passes actionGroup (default false), so a second hook on the same element that leaves the option out overwrites a group's true, and nothing recomputes it on detach. Under a fine pointer the group then stops serving presses, depending only on hook order. If the option stays, pass it only when the caller sets it and keep the element a group while any attachment asks for it.

Comment on lines +3 to +6
A gesture recognizer system with UIKit's semantics, for the touch
presentations of Astryx core components. Components import it from
`../gestures`, and Lab through internal workspace imports; nothing here is
exported from the package.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

architecture:react-component-runtime INV13 routes Lab to these modules through core's @astryxdesign/core/internal/touch subpath, and scripts/lab-core-imports.test.mjs fails any Lab import of core that core's exports do not list, so "Lab through internal workspace imports" points Lab authors at an import the guard rejects. The barrel header (index.ts:7-8) says the same.

vjeux added 5 commits October 26, 2026 22:21
…sture system (spec:AST-094 FR24, FR43)

utils/pressFeedback.ts reaches gestures/coarsePointer.ts, so a page whose
only touch work is press feedback carries a gesture module, and a bundle
check that counts gestures/ code finds it in the root and hooks entries.
The coarse-pointer query belongs with the environment queries. This test
walks the controller's import graph and fails until it reaches no module
under gestures/.
…ries in utils/ (spec:AST-094 FR43)

gestures/coarsePointer.ts becomes utils/coarsePointer.ts, beside
utils/touchQueries.ts, with its test; the gesture host's selection lock,
the press controller and the pointer test helper import it from there. The
press controller's import graph now reaches no module under gestures/, so
a page whose only touch work is press feedback carries none of the gesture
system. No behavior changes.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holding for an engineering-owner decision on actionGroup: it lets an action group's recognizers take mouse, pen and finger presses where the primary pointer is fine, which spec:AST-094 FR19 and architecture:react-component-runtime INV13 currently rule out, and the family:action-groups FR5 it cites is not on main yet (#8622). Approval needs an engineering owner to provide or identify the governing spec for that addition, either by landing #8622 with an FR19/INV13 exception or by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision:

  • Land #8309 first or rebase onto it, so this pull request carries only its own hook change (about 1,400 lines) instead of the whole stack (about 19,800).
  • If actionGroup stays, keep an element a group while any of its attachments asks for it; today hook order decides, and the group outlives the hook that asked (GestureHost.ts:382).
  • Correct the gestures README and barrel, which send Lab through workspace imports that INV13 and the stack's own Lab-import guard rule out.

[Automated review]

* primary pointer is fine too (`AttachOptions.actionGroup`).
* @default false
*/
actionGroup?: boolean;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

family:action-groups FR5 is not on main yet (#8622), and current spec:AST-094 FR19 and architecture:react-component-runtime INV13 say that no recognizer attaches where the primary pointer is fine. This option turns on exactly that path (mouse, pen and finger presses served to the group's recognizers, claimed clicks swallowed), so it needs an engineering owner's decision before it lands.

): () => void {
const registration = this.registrationFor(element);
registration.manageStyle = options.manageStyle ?? registration.manageStyle;
registration.actionGroup = options.actionGroup ?? registration.actionGroup;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actionGroup lives on the element's one registration, but every attachment overwrites it: useGestureRecognizer always passes actionGroup (default false), and nothing recomputes it on detach. Under a fine pointer, a group whose actionGroup hook attaches before another hook on the same element stops taking presses, and one whose hook attaches last keeps serving the other recognizers after that hook detaches. Passing the option only when the caller sets it, and keeping the element a group while any of its attachments asks for it (recomputed on detach), would fix both.


A gesture recognizer system with UIKit's semantics, for the touch
presentations of Astryx core components. Components import it from
`../gestures`, and Lab through internal workspace imports; nothing here is

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

architecture:react-component-runtime INV13 routes Lab to these modules through core's @astryxdesign/core/internal/touch subpath, and scripts/lab-core-imports.test.mjs fails any Lab import that goes around core's exports, so “internal workspace imports” points Lab authors at an import the guard rejects. The barrel's header (index.ts:7-8) says the same.

* @input The source of utils/pressFeedback.ts and every module it imports
* @output The press feedback controller's import graph reaches no module
* under gestures/: a page whose only touch work is press feedback carries
* none of the gesture system (`spec:AST-094` FR24, FR43: pointer queries

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current spec:AST-094 has no FR43 (its requirements end at FR41). Citing FR24 alone, or the record that states where pointer queries live, keeps this header accurate.

vjeux added 9 commits October 26, 2026 23:57
…(spec:AST-094 FR18, DEC-13)

A screen-edge pan attached to a stack container, as wide as the screen,
with a touch 4 px from its edge: inside the 40.2 px band the pan decides at
UIKit's 15 px whatever the element's width, and then tracks the travel past
it. From several edges too. Both fail while a wider element decides at
FR19's 10 px.
…'s width (spec:AST-094 FR18, DEC-13)

FR18: inside the 40.2 px band a screen-edge pan decides at UIKit's 15 px
screen-edge hysteresis; outside it the author's scrolling is untouched and
FR19's 10 px governs every other claim. The pan decided at 10 px on any
element wider than the band, so NavigationStack's edge pan, attached to the
stack container, began 5 px early. The edge zone still decides only the
touch-action; a touch outside every band still fails at once. The two
cases that pinned the 10 px decision go.
… 15 px whatever the element's width) into the experimental touch exports
…er's move keeps that finger until it lifts) into the experimental touch exports
…than the band (spec:AST-094 FR18, DEC-22)

FR18 as amended in meta#8631 (DEC-22): the band's pan-x claim and 15 px start
hold only where the screen-edge pan is attached to an element within the
band; on an element that reaches beyond it, whose touch-action cannot
claim per touch, a touch in the band decides at FR19's 10 px like every
other claim. This restores the 10 px decision 3d85c30 removed and keeps
15 px on an element within the band. The cases pin DEC-22 both ways (a wide
element with a touch at x=4 begins at 10, an in-band strip at 15, and from
several edges at 10 on a wide element); the 10 px cases fail on 3d85c30.
…an the edge band decides at 10 px again, DEC-22) into the experimental touch exports
…ouch decides at 15 px on an element within the band and at 10 px beyond it, DEC-22; meta#8641 bec2283) into meta#8270
…8641's moving-touch claim kept on this line's internals delivery) into meta#8315

# Conflicts:
#	packages/core/src/gestures/GestureArena.ts

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holding for an engineering-owner decision on actionGroup: it lets an action group's recognizers take mouse, pen and finger presses where the primary pointer is fine, which spec:AST-094 FR19 and architecture:react-component-runtime INV13 currently rule out, and the family:action-groups FR5 it cites is not on main yet (#8622). Approval needs an engineering owner to provide or identify the governing spec for that addition, either by landing #8622 with an FR19/INV13 exception or by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision:

  • Land #8309 first or rebase onto it, so this pull request carries only its own hook change (about 1,400 lines) instead of the whole stack (about 19,900).
  • If actionGroup stays, keep an element a group while any of its attachments asks for it; today hook order decides, and the group outlives the hook that asked (GestureHost.ts:382).
  • Correct the gestures README and barrel, which send Lab through workspace imports that INV13 and the stack's own Lab-import guard rule out, and the FR43 citation in pressFeedbackImports.test.ts, which current AST-094 does not have.

[Automated review]

): () => void {
const registration = this.registrationFor(element);
registration.manageStyle = options.manageStyle ?? registration.manageStyle;
registration.actionGroup = options.actionGroup ?? registration.actionGroup;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useGestureRecognizer always passes actionGroup (default false, useGestureRecognizer.ts:109, :154), so this ?? lets the last attachment decide whether the element is a group, and the detach closure (:395-415) never recomputes it. With two hooks on one element and only one asking for a group, a fine-pointer press either reaches no recognizer or reaches the one that never asked, and keeps doing so after the group hook detaches, depending only on hook order. If the option stays: pass it only when the caller sets it, and keep the element a group while any attachment asks for it, recomputed on detach.


A gesture recognizer system with UIKit's semantics, for the touch
presentations of Astryx core components. Components import it from
`../gestures`, and Lab through internal workspace imports; nothing here is

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

architecture:react-component-runtime INV13 says Core and Lab reach these modules through @astryxdesign/core/internal/touch, and scripts/lab-core-imports.test.mjs fails a Lab import of core outside core's exports, which has no ./internal/* subpath yet. Suggest saying Lab reaches them only through that subpath once it exists; index.ts:7-8 repeats the same route.

* @input The source of utils/pressFeedback.ts and every module it imports
* @output The press feedback controller's import graph reaches no module
* under gestures/: a page whose only touch work is press feedback carries
* none of the gesture system (`spec:AST-094` FR24, FR43: pointer queries

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current spec:AST-094 has no FR43 (its requirements end at FR41). Cite FR24 alone, or the record that states where pointer queries live.

vjeux added 4 commits October 27, 2026 03:09
…xperimental touch exports

The motion README's Files table is the union: main's rows, with meta#8663's
resolveSnapPoint.ts and selectPage.ts, plus this branch's, in main's
order.

# Conflicts:
#	packages/core/src/motion/README.md

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holding for an engineering-owner decision on actionGroup: it lets an action group's recognizers take mouse, pen and finger presses where the primary pointer is fine, which spec:AST-094 FR19 and architecture:react-component-runtime INV13 currently rule out, and the family:action-groups FR5 it cites is not on main yet (#8622). Approval needs an engineering owner to provide or identify the governing spec for that addition, either by landing #8622 with an FR19/INV13 exception or by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision:

  • Land #8309 first or rebase onto it, so this pull request carries only its own hook change (about 1,400 lines) instead of the whole stack (about 19,900).
  • If actionGroup stays, keep an element a group while any of its attachments asks for it; today hook order decides, and the group outlives the hook that asked (GestureHost.ts:382).
  • Point Lab at @astryxdesign/core/internal/touch in the gestures README and barrel, as INV13 and the stack's own Lab-import guard require, and fix the FR43 citation in pressFeedbackImports.test.ts, which current AST-094 does not have.

[Automated review]

* primary pointer is fine too (`AttachOptions.actionGroup`).
* @default false
*/
actionGroup?: boolean;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

family:action-groups FR5 is not on main yet (#8622), and current spec:AST-094 FR19 and architecture:react-component-runtime INV13 say that where the primary pointer is fine no recognizer attaches and no touch listener is registered. With actionGroup: true the host serves mouse, pen and finger presses to the group's recognizers there, so this needs an engineering owner's decision (landing #8622 with an FR19/INV13 exception, or /approve-spec <full head SHA> on this pull request) before it lands.

): () => void {
const registration = this.registrationFor(element);
registration.manageStyle = options.manageStyle ?? registration.manageStyle;
registration.actionGroup = options.actionGroup ?? registration.actionGroup;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useGestureRecognizer always passes actionGroup (default false, useGestureRecognizer.ts:109, :154), so this ?? lets the last attachment decide whether the element is a group, and the detach closure (:395-415) never recomputes it. With two hooks on one element under a fine pointer, attach order decides whether a mouse press reaches any recognizer, and a recognizer that never asked keeps taking presses after the group hook's attachment is released. Keeping the element a group while any of its attachments asks for it (recomputed on detach), and passing the option only when the caller sets it, would fix both.


A gesture recognizer system with UIKit's semantics, for the touch
presentations of Astryx core components. Components import it from
`../gestures`, and Lab through internal workspace imports; nothing here is

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INV13 says Core and Lab reach these modules through @astryxdesign/core/internal/touch, which main now ships (packages/core/src/internal/touch.ts re-exports the arena, the recognizer base, tap and long press from this directory), and scripts/lab-core-imports.test.mjs fails a Lab import of core outside core's exports. Naming that entry here, and in index.ts's header, keeps Lab authors on the route the guard allows; “nothing here is exported from the package” is no longer accurate either.

* @input The source of utils/pressFeedback.ts and every module it imports
* @output The press feedback controller's import graph reaches no module
* under gestures/: a page whose only touch work is press feedback carries
* none of the gesture system (`spec:AST-094` FR24, FR43: pointer queries

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current spec:AST-094 has no FR43 (its requirements end at FR42). Cite FR24 alone, or the record that states where pointer queries live.

@vjeux

vjeux commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Closing: carried by #8360 (this head, 2e77071, is in it by hash). The review items:

This branch was successfully deployed

1 active deployment
Preview — 2e77071d Deployed Oct 27, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants