Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
meta-cla
Bot
added
the
CLA Signed
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request waits on an owner decision: spec:AST-094, which it adds as a draft, needs an engineering owner to approve, amend or reject FR1–FR23 with DEC-1 and DEC-2, including a requirement for the input stream itself, which no claim states yet, and whether FR4 (touch motion read from the UIKit table on every theme) or the new theme motion roles stand, and an engineering owner records that by commenting /approve-spec <full head SHA> on this pull request (the author may, as an engineering owner). Independently of that decision, please reduce the compare to this fix's own commit by landing or retargeting #8293 and its predecessors, since it currently carries 35,734 authored lines, and address the points below.
- Take the press feedback's selection lock only for touch, because it now changes stable pressables and pen input under an
[experimental]release note. - Remove the
platformadaptation axis from the theme template, sincedefineThemerefuses it. - Make
presentation="adaptive"honor a theme's overriddenmd, or stop promising that in the release note. - Align AST-094 FR8's 0.5 s cross-fade with the current reduced-motion record's 200 ms, cite only claims that exist (the body's FR1,
spec:AST-100, AST-094 OQ4 and AST-090), and describe the input stream's fix as the two cases it actually covers.
[Automated review]
| // 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), |
There was a problem hiding this comment.
This lock runs for every pointer except a mouse, so a pen, a fine pointer, also loses text selection on every stable pressable, and the release note calls it [experimental]. Current spec:AST-017 FR27 says an omitted opt-in must leave stable behavior unchanged, and FR59 allows a fine-pointer change from touch work only with a named reason and before-and-after evidence. Taking the lock only for event.pointerType === 'touch' matches the comment above.
| * are primary-pointer precision, contrast preference, motion preference, and | ||
| * platform (`ios` for the browsers of iPhone and iPad, `other` for the rest). |
There was a problem hiding this comment.
Adaptations refuse a platform condition (expandMotionRoles.test.ts pins that defineTheme throws), and spec:AST-012 FR2 closes when to width, height, pointer, contrast and motion. A theme author who follows this line gets an error; please drop the platform clause.
| export const COMPACT_TOUCH_QUERY = | ||
| DEFAULT_COMPACT_DEFINITION.coarsePointerQuery; |
There was a problem hiding this comment.
This is built once from DEFAULT_COMPACT_DEFINITION, so a theme that overrides md still switches the adaptive menus at 768 px, while spec:AST-012 FR9 uses the effective md point and .changeset/touch-presentation.md promises the theme's md. Either resolve the query from the active theme's compactDefinition(widthBreakpoints) or state the fixed 768 px in the release note.
| place, and any other move fades through. A navigation push or pop | ||
| cross-fades over 0.5 s on the ease-in-out curve; a sheet, a full-screen |
There was a problem hiding this comment.
A 0.5 s cross-fade under Reduce Motion contradicts the current reduced-motion design record's DR2 (opacity only, 200 ms or less), which the animator in this pull request already follows (timing.reducedMotionCrossFadeMs, 200 ms). Please align FR8 with it or list the amendment under Current-state impact for its owner.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The touch press feedback and the gesture recognizers now listen on the window's capture phase through one shared input stream: one passive listener per event type for both. An app's own capture-phase `stopPropagation` at the document no longer starves them, and a page of pressables registers no listener that can hold a scroll. |
There was a problem hiding this comment.
A capture-phase stopPropagation at the document never stopped another listener on the document, as the description notes; the cases this fixes are a window-capture stopPropagation and a document-capture stopImmediatePropagation. The press feedback's pointer listeners were already on the window, and touchend, click and contextmenu stay non-passive, so "one passive listener per event type" overstates it too.
| @@ -177,6 +190,13 @@ const STATIC_EXPORTS = { | |||
| types: './dist/utils/interactionOverlay.stylex.d.ts', | |||
| default: './dist/utils/interactionOverlay.stylex.js', | |||
| }, | |||
| // Proposed; pending owner review (spec:AST-094 OQ4): the Reduce Motion | |||
There was a problem hiding this comment.
spec:AST-094 has OQ1 and OQ2 only, so this OQ4 citation points nowhere; and as a non-experimental subpath, ./motion/reducedMotion.stylex would carry the stable compatibility promise once released.
| @@ -28,8 +28,13 @@ | |||
| * (`@media (pointer: coarse)`) and this writes `data-astryx-press="on"` when the | |||
| * press is believed and `data-astryx-press="fading"` for the release's exit. | |||
| * | |||
| * While a finger or a pen is down on a surface, its text selection and the | |||
| * touch callout are off (gestures/selectionLock.ts, shared with the gesture | |||
There was a problem hiding this comment.
selectionLock.ts moved to utils/ in this commit; this line and the SYNC list at line 36 still point to gestures/selectionLock.ts.
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 |
ae623bd to
5d972b9
Compare
218bbb3 to
ae64881
Compare
Both edit utils/pressFeedback.ts. meta#8118's controller (two slots, listeners only while a press is live, the coarse-gated clocks) is kept, and meta#8356's changes to the old controller are carried onto it: its listeners subscribe through the shared input stream (the keyboard stays on the window's bubble phase), a finger's press takes the selection lock where locksSelectionFor() says so, and isPressFeedbackInstalled is exported. The listener test reads the stream's window listeners; meta#8356's link-callout and pen tests are kept.
There was a problem hiding this comment.
This pull request waits on an owner decision: no spec on main covers delivering touch input through one capture-phase listener per event type on the window, and the cited spec:AST-094 FR1 exists only in the open #8394, where it covers measured motion values, so an engineering owner needs to provide or identify the governing claim and record that decision 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 reduce the compare to this fix's own commits by landing or retargeting #8293 and its predecessors, since it currently carries 35,419 authored lines, and address the points below.
- Split the theme motion roles into #8257, since their
--motion-*names do not originate intokens.stylex.ts(architecture:theme-tokensINV1) andsprings.menuReducedMotionis a role thatdesign:reduced-motionforbids any motion from playing. - Correct
.changeset/input-stream.mdto name the cases actually fixed (a window-capturestopPropagationand a document-capturestopImmediatePropagation) and to say thattouchend,clickandcontextmenustay non-passive. - Mark
.changeset/reduced-motion-record.mdas an internal[chore], and update thegestures/selectionLock.tsreferences inpressFeedback.ts.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The touch press feedback and the gesture recognizers now listen on the window's capture phase through one shared input stream: one passive listener per event type for both. An app's own capture-phase `stopPropagation` at the document no longer starves them, and a page of pressables registers no listener that can hold a scroll. |
There was a problem hiding this comment.
A capture-phase stopPropagation at the document never stopped another listener on the document, as the description notes. The cases this fixes are a window-capture stopPropagation and a document-capture stopImmediatePropagation, each registered first. The press feedback's pointer listeners were already on the window, and touchend, click and contextmenu (plus touchmove during a page hold) are non-passive, so "one passive listener per event type" overstates it too.
| } | ||
| const variable = motionRoleVariable(id); | ||
| try { | ||
| tokens[variable] = formatMotionRoleValue(kind, value); |
There was a problem hiding this comment.
This writes a --motion-* name built from a motion-table path into the theme's tokens, while the role's default lives in the motion table. architecture:theme-tokens INV1 says a core token name and its default originate in tokens.stylex.ts and that projections do not introduce names. Declaring each role there, or an owner amending INV1, would settle this; the roles have their own pull request in #8257.
| * iPhone 17 Pro, `_animateUsingSpringWithDampingRatio:response:tracking:` | ||
| * hooked with Reduce Motion on (1.0, 0.4), Oct 23 2026 | ||
| */ | ||
| menuReducedMotion: {response: 0.4, dampingRatio: 1}, |
There was a problem hiding this comment.
Registering this entry makes springs.menuReducedMotion a theme role (roleKinds.generated.ts:30, astryx docs motion). But design:reduced-motion DR1 and DR4 forbid any spring under reduced motion, and nothing reads the role. Keeping it out of the registered roles avoids publishing a theme knob that can never take effect.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The experimental touch animator follows design:reduced-motion. Under Reduce Motion every `animate` and `play` (travel or opacity, on a spring or any curve) becomes an opacity-only cross-fade of 200 ms on a coarse primary pointer, or for a call marked `touchRelease`, and completes at once on any other pointer; nothing moves, scales or springs, and travel lands with a plain style write while the element is transparent, never as an animated transform. `timing.reducedMotionCrossFadeMs` is now 200 ms and is no longer a theme role. The `reducedMotion` and `crossfade` call options are removed; `crossfadeAs` stays. `animator.reducedMotion()` reports the mode for feedback with no fade of its own, `reducedMotionFade()` gives the same fade to code beside the animator, and `reducedMotion.stylex.ts` holds its constants for StyleX transitions. |
There was a problem hiding this comment.
The animator has no export path at this head, so a [fix] note about "the experimental touch animator" and its removed call options describes an API that consumers never had. [chore] Internal:, as the other motion changesets use, would fit.
| @@ -28,8 +28,13 @@ | |||
| * (`@media (pointer: coarse)`) and this writes `data-astryx-press="on"` when the | |||
| * press is believed and `data-astryx-press="fading"` for the release's exit. | |||
| * | |||
| * While a finger or a pen is down on a surface, its text selection and the | |||
| * touch callout are off (gestures/selectionLock.ts, shared with the gesture | |||
There was a problem hiding this comment.
selectionLock.ts moved to utils/ in this pull request, but this line and the SYNC list at line 36 still point to gestures/selectionLock.ts.
| @@ -433,6 +464,14 @@ function removeListeners(): void { | |||
| * are reset with them, so a later install never inherits a gesture that | |||
| * never happened. | |||
| */ | |||
| /** | |||
There was a problem hiding this comment.
isPressFeedbackInstalled now sits between installPressFeedback's JSDoc (lines 454-466) and that function, so installPressFeedback loses its documentation. Moving this block above that JSDoc keeps both.
…xposes no ontouchstart is taken once Chromium fixes ontouchstart when the page is created, so turning touch emulation on later sends Touch Events without it: the host reads Pointer Events for the finger and must not read its Touch Events too. A one-finger long press fails here, the finger tracked twice.
…from their pointer events install() reads a finger from its pointer events when the window has no ontouchstart, but the four touch handlers still took the Touch Events a browser sends anyway (Chromium after touch emulation turns on), tracking the finger twice, so a one-finger long press never recognized. Each touch handler now returns while usesTouchEvents is false, after noting the fingers down, as install()'s comment promised.
…ter events) into meta#8320
…ter events) into meta#8293
…ter events) into meta#8356
There was a problem hiding this comment.
Approval waits on an engineering owner deciding whether Core should hear page-wide input on the window's capture phase, so that an app's earlier capture-phase stop cannot hide a touch from the gesture recognizers: no current spec record states that, and the cited spec:AST-094 FR1 text is not in the record. An engineering owner records that decision by providing or identifying the governing claim and commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision, please address the points below.
- Land or retarget #8293 and its predecessors so the compare holds only this fix: it currently carries 27,427 authored lines for a 1,532-line change.
- Scope the description, release note and README to what is restored: a pan that must claim its touch (a sheet pull, or a one-way swipe over a scrollable page) is still cancelled under both stops, because its blocking
touchmovelistener sits on the element below the app's stop. - Correct
.changeset/input-stream.md, which calls every listener passive and names a documentstopPropagationcase that never failed, along with the matching comments and the README's listener list. - Keep the shared listener in place when a type switches between passive and non-passive, or state that limit, since re-adding it lets app listeners registered after Core run before it.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The touch press feedback and the gesture recognizers now listen on the window's capture phase through one shared input stream: one passive listener per event type for both. An app's own capture-phase `stopPropagation` at the document no longer starves them, and a page of pressables registers no listener that can hold a scroll. |
There was a problem hiding this comment.
This note is the whole declaration (spec:AST-017 FR7), but the host's touchend, contextmenu and click subscriptions cancel while a claimer is attached (GestureHost.ts:1350, :1359, :1362) and touchmove turns non-passive while a page hold listens (pageHold.ts:192), so not every listener is passive. A document-capture stopPropagation also never stopped another listener on the document (GestureHost.test.ts:1884-1886); the cases fixed are a window-capture stopPropagation and a document-capture stopImmediatePropagation registered before Core.
| view.removeEventListener(type, native.listener, {capture: true}); | ||
| channel.native = null; | ||
| } | ||
| if (wanted == null) { | ||
| return; | ||
| } | ||
| const listener: InputListener = event => dispatch(channel, event); | ||
| view.addEventListener(type, listener, {capture: true, passive: !wanted}); |
There was a problem hiding this comment.
Removing and re-adding the native listener on a passivity switch appends it after every window-capture listener an app registered since Core's first subscription, so once a page hold starts listening, or the host's claimers change, an app's later stopImmediatePropagation hides those events from every Core subscriber. Keeping each type's listener in place across the switch (a type whose non-passive listener holds no scroll, such as touchend, click or contextmenu, can stay non-passive), or stating the limit in the header, would close this.
| // The blocking listener sits on the element itself (FR19: a scroll that | ||
| // starts elsewhere never waits on it), but every element shares the | ||
| // host's one handler, which routes through the arena. | ||
| if (needsListener && !registration.hasTouchMoveListener) { | ||
| element.addEventListener('touchmove', this.onBlockingTouchMove, { | ||
| passive: false, | ||
| }); | ||
| registration.hasTouchMoveListener = true; | ||
| } else if (!needsListener && registration.hasTouchMoveListener) { | ||
| element.removeEventListener('touchmove', this.onBlockingTouchMove); | ||
| registration.hasTouchMoveListener = false; | ||
| } |
There was a problem hiding this comment.
Under an app's window-capture stopPropagation or document-capture stopImmediatePropagation, this element listener never runs, so a pan that shares an axis with native scrolling (a sheet pull, a one-way swipe over a scrollable page) still cannot claim its touch and is cancelled when the browser takes it. The description, release note and README could say that recognizers now hear these touches while a claim still needs the event to reach the element.
| unless a subscriber cancels it: the host cancels `touchend`, `click` and | ||
| `contextmenu` there, never a `touchstart` or a `touchmove` (a blocking | ||
| `touchmove` sits on a claiming element, or on the window only while a page | ||
| hold is held), so a page of pressables blocks no scroll. |
There was a problem hiding this comment.
A blocking touchmove also sits on the document while a claiming element moves (GestureHost.ts:1132, and "Touches on moving elements" below) and on the touch target (GestureHost.ts:2095-2099), so this list is incomplete.
…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
…on/README.md) into facebook#8309
…; this pull request's presentation.ts rows kept beside them) into meta#8293
There was a problem hiding this comment.
Approval waits on an engineering owner deciding whether Core should hear page-wide input on the window's capture phase, so that an app's earlier capture-phase stop cannot hide a touch from the gesture recognizers: no current spec record states that, and the cited spec:AST-094 FR1 text is not in the record. An engineering owner records that decision by providing or identifying the governing spec and commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision, please address the points below.
- Land or retarget #8293 and its predecessors so the compare holds only this fix: it currently carries 27,477 authored lines for a 1,532-line change.
- Correct
.changeset/input-stream.md, which calls every listener passive and names a documentstopPropagationcase that never failed, along with the matching comments and the README's list of blocking listeners. - Scope the description, release note and README to what is restored: a pan that must claim its touch (a sheet's pull, or a one-way swipe over a scrollable page) still loses it to the browser's pan under both stops, because its blocking
touchmovelistener sits on the element below the app's stop. - Keep each type's shared listener in place when it switches between passive and cancelling, or loses and regains its last subscriber, or state that limit, since re-adding it lets app listeners registered after Core run before it.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The touch press feedback and the gesture recognizers now listen on the window's capture phase through one shared input stream: one passive listener per event type for both. An app's own capture-phase `stopPropagation` at the document no longer starves them, and a page of pressables registers no listener that can hold a scroll. |
There was a problem hiding this comment.
This is the whole release declaration (AST-017 FR7), and two parts of it do not match the code: the host's touchend, contextmenu and click subscriptions are non-passive while a claimer is attached (GestureHost.ts:1350-1362), and the shared touchmove is non-passive while a page hold listens (pageHold.ts:192). A document-capture stopPropagation never hid a listener on the same node; the cases this fixes are a window-capture stopPropagation and a document-capture stopImmediatePropagation registered before Core.
| view.removeEventListener(type, native.listener, {capture: true}); | ||
| channel.native = null; | ||
| } | ||
| if (wanted == null) { | ||
| return; | ||
| } | ||
| const listener: InputListener = event => dispatch(channel, event); | ||
| view.addEventListener(type, listener, {capture: true, passive: !wanted}); |
There was a problem hiding this comment.
Removing and re-adding the native listener on a passivity switch (or after the last subscriber leaves) appends it after every window-capture listener registered in the meantime, so an app listener added after Core's first subscription runs before Core from then on, and its stopImmediatePropagation hides the event from every subscriber. Keeping each type's listener in place (for example, leaving touchend, click and contextmenu non-passive once registered), or stating the limit here, would avoid that.
| // The window's capture phase, through the shared input stream | ||
| // (utils/inputStream.ts): the earliest point a page can intercept input, | ||
| // so an app's capture-phase stopPropagation at the document starves no | ||
| // recognizer, as UIKit's recognizers see touches before views do. Only | ||
| // the listeners that cancel are non-passive, and only while a claimer is | ||
| // attached (syncClaimingListeners); none of them is a touchstart or | ||
| // touchmove, so none of them holds a scroll. |
There was a problem hiding this comment.
Two things this comment implies do not hold: a document-capture stopPropagation never starved a listener on the same node, and a recognizer that must claim mid-gesture still cannot under the two stops this does fix, because its blocking touchmove runs on the element or touch target below the app's stop (:886-897, :2103-2107). Worth naming the fixed cases and that a claim still needs the event to reach the element.
… a mouse press At the document, an event from inside an open shadow root is retargeted to the shadow host, and so is a touch's target, so the host looked for participants from the shadow host and found none inside the root: the browser then sent its own mousedown and click. Both cases, a finger and a mouse, attach a tap to an element in the shadow root. Fail until the host resolves targets through the composed path.
…composed path Inside an open shadow root the document's listeners see the shadow host, in event.target and in a touch's target alike, so a recognizer on an element in the root never got its finger, and nothing claimed it. landedOn takes the composed path's first element for the event's own target and for a touch whose target is the event's (another finger keeps its own): a touch's start, a pointer's press and its pairing, the last touch a scroll reads, a cancelled pointer's touch and hover.
…he host resolves targets through the composed path) into meta#8320
There was a problem hiding this comment.
Approval waits on an engineering owner deciding whether Core should hear page-wide input on the window's capture phase, so that an app's earlier capture-phase stop cannot hide a touch from the gesture recognizers: no current spec record states that, and the cited spec:AST-094 FR1 text is not in the record. An engineering owner records that decision by providing or identifying the governing spec and commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision, please address the points below.
- Land or retarget #8293 and its predecessors so the compare holds only this fix: it currently carries 27,532 authored lines for a 1,532-line change.
- Scope the description, release note and comments to what is restored, and cover both stop cases with a real-touch browser test: a pan that must claim its touch (a sheet's pull, a one-way swipe over a scrollable page) still loses it to the browser under both stops, because its blocking
touchmovelistener sits on the element below the app's stop. - Correct
.changeset/input-stream.md, which calls every listener passive and names a documentstopPropagationcase that never failed, and state that the stream stays first on the window only until an event type's next passivity switch.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The touch press feedback and the gesture recognizers now listen on the window's capture phase through one shared input stream: one passive listener per event type for both. An app's own capture-phase `stopPropagation` at the document no longer starves them, and a page of pressables registers no listener that can hold a scroll. |
There was a problem hiding this comment.
This declares "one passive listener per event type" and a fixed document stopPropagation case, but the host's touchend, click and contextmenu subscriptions cancel while a claimer is attached, the page hold's touchmove cancels while held, and a document-capture stopPropagation never stopped a listener on the same node. The cases that changed are a window-capture stopPropagation and a document-capture stopImmediatePropagation registered before Core; please name those and say which subscriptions cancel.
| if (needsListener && !registration.hasTouchMoveListener) { | ||
| element.addEventListener('touchmove', this.onBlockingTouchMove, { | ||
| passive: false, | ||
| }); | ||
| registration.hasTouchMoveListener = true; |
There was a problem hiding this comment.
This element listener is where a recognizer claims its touch, and it sits below an app's window-capture stopPropagation or document-capture stopImmediatePropagation. Under those stops the stream hears the touch, but a pan that shares an axis with native scrolling still cannot cancel the browser's pan and is cancelled when the browser takes the touch, so the description's "a sheet can't be dragged" case stays broken there. Please scope the claim to what is restored and cover both stops with a real-touch browser test.
| return; | ||
| } | ||
| const listener: InputListener = event => dispatch(channel, event); | ||
| view.addEventListener(type, listener, {capture: true, passive: !wanted}); |
There was a problem hiding this comment.
Every passivity switch, and every re-subscription after a type's last subscriber leaves, re-adds this listener at the end of the window's capture list, so an app listener added after Core's first subscription runs first from then on. AST-094 FR19 and FR20 make those switches necessary, so please state that limit where the module says the window's capture phase is the earliest point.
| } | ||
| this.isBlockingForMovers = needed; | ||
| if (needed) { | ||
| this.doc.addEventListener('touchmove', this.onBlockingTouchMove, { |
There was a problem hiding this comment.
A non-passive touchmove on the document while a claiming element moves goes beyond AST-094 FR19, which allows the blocking listener only on "an element with a recognizer that must claim mid-gesture". This comes with #8293; it belongs in that pull request, conformed or with FR19 amended.
…adow root; a claimed touch's moves are cancelled where fingers come from pointer events With targets from the composed path, a finger inside an open shadow root sits below the scroller only along the flat tree, which Node.contains does not follow: a scroll under it did not take it, and its flick did not arm the scroll brake. And where the host reads fingers from pointer events (inputMode 'auto' with no ontouchstart), the blocking touchmove found no finger for a claimed pan's touch, so its moves went to the page. Three cases fail until the host compares along the flat tree and finds that finger; a fourth pins a finger scroll inside a shadow root.
… tree; a claimed touch finds its pointer finger wherever fingers come from pointer events containsAlongFlatTree walks parentAcrossShadow, so a finger inside an open shadow root, now that targets come from the composed path, counts as under a scroller outside it: a scroll there takes it, its flick arms the scroll brake, and a touch there brakes. cancelIfClaimed matches a touch to its pointer finger whenever the host reads fingers from pointer events, not only under inputMode 'pointer-events', so a claimed pan's moves are cancelled where a browser that exposes no ontouchstart still sends Touch Events.
… flat tree, and a claimed touch finds its pointer finger wherever fingers come from pointer events) into meta#8320
…r along the flat tree, and a claimed touch finds its pointer finger wherever fingers come from pointer events) into meta#8293
There was a problem hiding this comment.
Approval waits on an engineering owner deciding whether Core should hear page-wide input on the window's capture phase, so that an app's earlier capture-phase stop cannot hide a touch from the gesture recognizers: no current spec record states that, and the cited spec:AST-094 FR1 text is not in the record. An engineering owner records that decision by providing or identifying the governing spec and commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision, please address the points below.
- Land or retarget #8293 and its predecessors so the compare holds only this fix: it currently carries 27,652 authored lines for a 1,532-line change.
- Correct
.changeset/input-stream.md, which calls every listener passive and names a documentstopPropagationcase that never failed, along with the matching comments and README text. - Scope the description, release note and comments to what is restored, and cover both stop cases with a real-touch browser test: a pan that must claim its touch (a sheet's pull, a one-way swipe over a scrollable page) still loses it to the browser under both stops, because its blocking
touchmovelistener sits on the element below the app's stop. - State that the stream stays first on the window only until an event type's next passivity switch, which re-adds its listener behind app listeners registered after Core.
[Automated review]
|
Closing: carried by #8360 (this head, 886749b, is in it by hash). The review items:
|
Depends on #8293. Review only this pull request's change: vjeux/astryx@c1186e4...886749b
Why
An app's own capture-phase listener can silently switch off Astryx's touch handling. If the app shell calls
stopPropagationon the window, orstopImmediatePropagationon the document, from a listener registered before the first recognizer, the gesture host never sees the touch: a sheet can't be dragged and a row swipe never fires, with no error anywhere.What led to it
This came up in the UIKit-parity work on Astryx's touch components. In UIKit, gesture recognizers see a touch before any view does. Astryx's host listened at the document's capture phase, which runs after the window's, and each touch subsystem (the host, the press feedback, the page hold, the finger-scroll watch) registered its own page-wide listeners.
Options and tradeoffs
Why this choice
The window's capture phase is the earliest point the web offers, so recognizers see touches first, as in UIKit (
spec:AST-094FR1). One listener per type keeps the page's listener count flat however many components mount (FR24). The listener is passive unless a subscriber needs to cancel, and a subscriber that throws is reported without stopping the others.How to check it
event.stopPropagation()ontouchstart.Evidence
stopPropagationand its document-capturestopImmediatePropagationregistered first, fail before the change and pass after.