Repository navigation
Conversation
vjeux
requested review from
cixzhang,
imdreamrunner and
josephfarina
as code owners
October 25, 2026 04:04
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
3 tasks done
|
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 25, 2026 04:04
![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. Measuring useLongPress travel as a distance, for ContextMenu's long press and the menu trigger hold, has no current spec claim, and this head adds draft spec:AST-094, whose FR12 expects context menus and menus to use their own measured long-press values. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Two changes are needed whatever the decision:
- Land #8356 and the PRs below it first, or rebase onto them, so this pull request carries only its long-press commit instead of about 35,800 authored lines across 230 files.
- The released 0.7.4 documents
moveCancelPxas a per-axis limit, so the distance rule needs a[breaking]Changeset in a declared minor that names the affected drift and the migration, not a patch[fix].
[Automated review]
| @@ -0,0 +1,7 @@ | |||
| --- | |||
| '@astryxdesign/core': patch | |||
There was a problem hiding this comment.
Latest stable 0.7.4 documents moveCancelPx as "Movement past this distance (px, either axis)" and checks each axis, so measuring it as a distance changes a released contract: a diagonal drift between 10 and 14.1 px opens the menu on 0.7.4 and cancels the press here. Under architecture:public-component-api INV9 and spec:AST-017 DEC-1 this needs a [breaking] Changeset in a declared minor that names this scenario and the migration. main declares 0.7.5, a patch, where FR31 admits no [breaking] Changeset.
meta-cla
Bot
added
the
CLA Signed
vjeux
force-pushed
the
uikit/f2-long-press-source
branch
from
October 25, 2026 06:52
f5b8437 to
52f8412
Compare
This was referenced Oct 25, 2026
vjeux
force-pushed
the
uikit/f2-long-press-source
branch
from
October 25, 2026 07:14
52f8412 to
6c03a93
Compare
vjeux
force-pushed
the
uikit/f2-long-press-source
branch
from
October 25, 2026 09:12
6c03a93 to
133c6f9
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner: no current spec states how far a held finger may travel before useLongPress cancels, so measuring that travel as a 10 px distance for ContextMenu's long press and the menu trigger hold needs an engineering owner to identify or approve the governing spec, which they record by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). Whatever that decision, these changes are needed:
- Land #8356 and the PRs below it first, or rebase onto them, so this pull request carries only its long-press commits instead of about 35,500 authored lines across 221 files.
- The released 0.7.4 documents
moveCancelPxas a per-axis limit, so a distance rule needs a[breaking]Changeset in a declared minor that names the affected drift and the migration, not a patch[fix]. packages/core/src/hooks/useLongPress.doc.mjsstill describes the per-axis rule in both of itsmoveCancelPxdescriptions.- The trigger hold also runs for a pen, so
spec:AST-017FR59 needs a named reason and pen before-and-after evidence, or the pen keeps the per-axis rule.
[Automated review]
| ) { | ||
| // A distance, as UIKit measures `allowableMovement`: a (7, 7) drift | ||
| // keeps the press, a (7.2, 7.2) one ends it. | ||
| if (Math.hypot(point.x - from.x, point.y - from.y) > moveCancelPx) { |
There was a problem hiding this comment.
Latest stable 0.7.4 documents moveCancelPx as “px, either axis” and tests each axis, so this distance check changes a released contract: a (9, 9) drift kept the press before and cancels it now. If the distance is approved, it needs a [breaking] Changeset in a declared minor (spec:AST-017 FR31), and useLongPress.doc.mjs lines 48 and 97 need the new rule as well.
| @@ -0,0 +1,7 @@ | |||
| --- | |||
| '@astryxdesign/core': patch | |||
There was a problem hiding this comment.
main declares 0.7.5, a patch, and this changes the moveCancelPx rule 0.7.4 documents, so it cannot ship as patch [fix]: spec:AST-017 FR31 admits a [breaking] Changeset only once main declares a minor.
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 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 |
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.
…llowance under every primary pointer The useMenuPress record's FR9 puts the trigger's held finger under spec:AST-094 FR29 under every primary pointer (FR43), so a finger on a touchscreen laptop may drift 13 pt before the open, as on a phone. The trigger read (pointer: coarse) as its press started and took the long press's 10 px where the pointer was fine. Three cases fail until it does not: a 12 pt drift opens under a fine pointer too, a (12, 12) settle opens there, and nothing reads a pointer query, the trigger's press included.
…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.
meta#8360 carries this pull request's predecessors and the review fixes of the foundation it holds, so this pull request's own diff is now its two commits: useLongPress's doc fix and the unused import dropped from the UIKit gestures table. The merge is clean.
There was a problem hiding this comment.
Changes requested: the long-press change itself is a sound refactor that gives the long-press numbers one source with no behavior change, but this pull request now carries all of the open #8360, so 27,233 authored lines would land under its title.
- Land #8360 first and bring this branch up to
main, or move the two remaining commits (theuseLongPressheader and the dropped import) into #8360 and close this pull request. - Point the description at #8360 rather than the closed #8356, and retitle the pull request for what it holds once #8360 has landed.
- The carried gesture host still attaches recognizers only under a coarse pointer, which current
spec:AST-094FR19 and FR43 no longer allow; that fix belongs on #8360.
[Automated review]
Contributor
Author
Contributor
Author
|
Folded into #8360 under the maintainers' section plan; the commit credits this PR's authors. Closing; the branch stays. |
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 #8360. Review only this pull request's change: vjeux/astryx@75392b2...a1585b3
Why
useLongPress's comments described its movement tolerance as a distance and the context menu's allowance as settled, but the hook measures each axis, the released rule, and the allowance is unmeasured. The UIKit gestures table also imported a press value it never used.What led to it
The single source for a long press's numbers, which this pull request introduced, now rides in #8360, which merges first in the maintainer's order. These two follow-ups remain.
Options and tradeoffs
Why this choice
No behavior changes. The comments now say
useLongPressmeasures each axis, as released, and that the context menu's allowance stays a guess until measured (spec:AST-094FR2).How to check it
hooks/useLongPress.ts's tolerance comment. Expected: it says each axis.Evidence
useLongPress.tsand one import line inmotion/uikit/gestures.ts.