Repository navigation
fix(gestures): pointer-read fingers get the blur refusal and the pinch rule of the Touch Events path - #8309
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
![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 is on hold until an engineering owner decides the draft spec:AST-094 it implements: the six gesture-host fixes rest on its FR19 and FR20, and the 10 px scroll rule and the touch-action intersection go beyond what FR20 and FR18 say, so an engineering owner needs to approve or amend the record by commenting /approve-spec <full head SHA> on this pull request (the author may do so if they are an engineering owner). Separately from that decision, the diff against main still carries the whole unmerged stack, about 28,900 changed lines, so it should shrink to this commit before it merges. Please also address:
- The compact-touch query in
utils/touchPresentation.tshard-codes 744 px, while currentspec:AST-012FR9 places the width edge at the theme's effectivemdpoint. when.platforminthemeAdaptations.tsis outside the closed condition set of currentspec:AST-012FR2.- The stylus, Safari scroll-wait and still-finger fixes need real-browser evidence under current
spec:AST-013FR10, from iPadOS and iOS Safari for the first two.
[Automated review]
| export const COMPACT_QUERY = '(width < 744px), (height < 500px)'; | ||
|
|
||
| /** 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)'; |
There was a problem hiding this comment.
Current spec:AST-012 FR9 defines compact as a viewport width below the effective md point (768 unless a theme overrides it) or a height below 500, says native presentations do not map compact to platform size classes, and names the web touch presentation resolver as a user of that one definition. These 744 px and 500 px literals give coarse-pointer viewports 744–767 px wide the popover where FR9 gives the sheet, and they ignore a theme's md.
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
platform is a new when field, but current spec:AST-012 FR2 says conditions are “named, closed, and composable” and lists width, height, pointer, contrast and motion. Remove it, or have an owner amend AST-012 first.
| ) { | ||
| return null; | ||
| } | ||
| if (attribute === PRESSABLE_ATTRIBUTE && isPressFeedbackInstalled()) { |
There was a problem hiding this comment.
isPressFeedbackInstalled() is module-wide, while the controller only listens on the global window and document. A delegated press installed on another document (an iframe's) stays idle whenever the main document mounts an Astryx pressable. Comparing doc with the controller's document keeps the guard to the case it is for.
| Math.hypot(touch.x - touch.startX, touch.y - touch.startY) >= | ||
| UIKIT_GESTURES.press.slop.value |
There was a problem hiding this comment.
Draft spec:AST-094 FR20 says “a native scroll under the touch MUST count as the browser taking the touch”, with no travel condition, so this rule needs FR20 amended by an owner. A scroll during a held touch is browser event timing, so current spec:AST-013 FR10 also asks for a real-browser proof of the still-finger case, not only the jsdom one.
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| lab/MobilePowerSearch | 213.9 kB (+240 B) · 1.2 kB CSS (0) | — | — · — · — |
| Switch · Storybook | 87.0 kB (+225 B) · 10.5 kB CSS (0) | 1,719 (0) | ✓ · ✓ · ✓ |
| SegmentedControl | 81.9 kB (+224 B) · 7.8 kB CSS (0) | — | — · — · — |
2 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| DateTimeInput | 158.7 kB (+223 B) · 17.0 kB CSS (0) | — | — · — · — |
| Slider · Storybook | 88.8 kB (+214 B) · 9.4 kB CSS (0) | 2,326 (0) | ✓ · ✓ · ✓ |
Package size: @astryxdesign/core 6.09 MB packed (+2.2 kB); no new dependencies.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
41f2816 to
8a73f1e
Compare
github-actions
Bot
added
the
needs:design-review
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision on the scroll-lock requirements it carries from #8098 into component:BottomSheetSwitcher FR5, component:Drawer FR5 and component:MobileNav FR3, which no current spec states; an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may, as an engineering owner), or by approving the same text in #8098 and landing it first. Whatever is decided, two changes are still needed:
- Land #8360 and #8098 first, so this diff carries only its own 483 lines instead of 18,489.
- Keep the blur refusal working under a fine primary pointer: the blur removes the press's page listeners (
GestureHost.ts:2918-2923) before a pen's or pointer-read finger's later lift, so its click still lands; a case whose lift comes in a later task would catch it.
[Automated review]
| // not click (FR28). A mouse button held through the blur keeps its click. | ||
| for (const [pointerId, kind] of this.pointerKinds) { | ||
| if (kind !== 'mouse') { | ||
| this.cancelledPointers.add(pointerId); |
There was a problem hiding this comment.
Under a fine primary pointer this id is never read: onBlur calls releaseArmedPress right after this, its microtask finds no arena touch, and shouldListen (line 1536) uninstalls the page pointerup subscription. The pen's or pointer-read finger's later lift then reaches no host listener and its click lands. Keeping the pointerup and pointercancel subscriptions while cancelledPointers holds an id, plus a fine-pointer case whose lift comes after a microtask, would close it; a mouse press keeps today's teardown.
| expect(click.defaultPrevented).toBe(false); | ||
| }); | ||
|
|
||
| it('forgets a blur-cancelled pen whose contact ends in pointercancel: a later pen with its id keeps its click (spec:AST-094 FR28)', () => { |
There was a problem hiding this comment.
This case still passes with the pointercancel delete (GestureHost.ts:2712) removed: the later pen pointerdown clears the id at GestureHost.ts:2501 anyway. Asserting on what the pointercancel alone leaves (a lift after it arms no guard), or naming the case for what it pins, would make it guard its rule.
| // A claimed lift's note stays: its touchend comes in a later task than | ||
| // the pointerup, often after an armed press's listeners have gone, and | ||
| // the element's own listener still takes it (noteClaimedLift). |
There was a problem hiding this comment.
When the last attachment goes, cleanup has already removed the element's touchend listener (line 1232), so a kept note and its timer wait up to 500 ms with nothing left to take them, against FR24's "nothing touch-related remains after the last instance unmounts". Clearing the notes in uninstallIfIdle's idle branch would keep the armed-press case and drop them there.
|
Rebased onto #8098's new head, which carries #8360's lifecycle round. The pointers a blur cancelled now go after the last detach, as the host's other notes do: a pen held through a blur and then detached is a new case in the "nothing left after the last detach" block, and it failed before. Local gates at ca56e89 are in the description. |
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision on the scroll-lock requirements it carries from #8098 into component:BottomSheetSwitcher FR5, component:Drawer FR5 and component:MobileNav FR3, which no current spec states; an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may, as an engineering owner), or by approving and landing the same text in #8098. Whatever that decision, two changes are still needed:
- Land #8360 and #8098 first, so this diff carries only its own 507 lines instead of 19,128.
- Keep the blur refusal working under a fine primary pointer: the blur removes the page
pointeruplistener before a pen or pointer-read finger lifts, so its click still lands, which the changeset says cannot happen.
[Automated review]
| @@ -1780,49 +2640,126 @@ export class GestureHost { | |||
| } | |||
| const id = this.pointerIds.get(event.pointerId); | |||
| if (id == null) { | |||
| this.releaseArmedPress(); | |||
| // A contact the lost focus cancelled: its lift clicks nothing (FR28). | |||
| if (this.cancelledPointers.delete(event.pointerId)) { | |||
There was a problem hiding this comment.
Under a fine primary pointer this branch never runs after a blur: onBlur calls releaseArmedPress(), whose microtask finds no arena touch, turns shouldListen() false and uninstalls the window pointerup subscription before the pen (or a pointer-read finger) lifts, so the click lands. Keeping the page pointerup/pointercancel subscriptions while cancelledPointers is non-empty, plus a fine-pointer case that lifts in a later task, would close it.
| expect(click.defaultPrevented).toBe(false); | ||
| }); | ||
|
|
||
| it('forgets a blur-cancelled pen whose contact ends in pointercancel: a later pen with its id keeps its click (spec:AST-094 FR28)', () => { |
There was a problem hiding this comment.
This case still passes with the cancelledPointers.delete in onPointerCancel removed, because the later pen pointerdown clears the id on its own. Asserting that a lift right after the pointercancel arms no click guard would pin the rule.
There was a problem hiding this comment.
This pull request needs an engineering owner's decision before it can be approved, and changes besides. The scroll-lock commit carried from #8098 writes new requirements into component:BottomSheetSwitcher FR5, component:Drawer FR5 and component:MobileNav FR3 that current spec does not state; an engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request (the author may, as an engineering owner), or by approving them in #8098 and landing it first. Whatever that decision:
- Under a fine primary pointer the blur releases the press's page listeners before the lift (
GestureHost.ts:3006-3011,:1441-1452), so a blur-cancelled pen still clicks; keep thepointerupsubscription while a refused lift is owed and add a fine-pointer case. .changeset/gesture-host-fixes.mdpromises that refusal everywhere and announces stylus handlingmainalready has, and the description's arena claim has no code change behind it.- The diff carries 19,430 lines of open #8360 and #8098 against this commit's 507; land those first.
[Automated review]
| @@ -1780,49 +2676,126 @@ export class GestureHost { | |||
| } | |||
| const id = this.pointerIds.get(event.pointerId); | |||
| if (id == null) { | |||
| this.releaseArmedPress(); | |||
| // A contact the lost focus cancelled: its lift clicks nothing (FR28). | |||
| if (this.cancelledPointers.delete(event.pointerId)) { | |||
There was a problem hiding this comment.
Under a fine primary pointer this branch never runs after a blur: onBlur (:3006-3011) calls releaseArmedPress(), whose microtask finds no arena touch, turns shouldListen() (:1579-1603) false and uninstalls the window pointerup subscription, so the pen's later lift clicks. FR43 covers that pen and FR28 says nothing acts after a lost focus. Keeping the page pointerup and pointercancel subscriptions while cancelledPointers holds an id, with a fine-pointer case whose lift comes in a later task, would close it.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: gesture host fixes from review. A claimed pen holds only its own stylus touches, and a finger is matched to its scroller, its target and a moving element along the flat tree, through slots and shadow roots. A disabled recognizer brings no listener; where a finger comes from its pointer events, a claimed finger's touchend is cancelled too, a touchscreen laptop's included and inside an open shadow root, a hold's that claims with its lift included; and a pen or a pointer-read finger the window's blur cancelled clicks nothing at its later lift, while a mouse keeps its click. Not exported from the package entry point. |
There was a problem hiding this comment.
Two parts of this note claim more than the diff does: the blur refusal holds only where the primary pointer is coarse (every case runs on the coarse default, and the fine-pointer lift still clicks), and "A claimed pen holds only its own stylus touches" is what main already does (GestureHost.ts:1348-1356 there). Qualifying the first and dropping the second keeps the release note accurate.
| expect(click.defaultPrevented).toBe(false); | ||
| }); | ||
|
|
||
| it('forgets a blur-cancelled pen whose contact ends in pointercancel: a later pen with its id keeps its click (spec:AST-094 FR28)', () => { |
There was a problem hiding this comment.
This case still passes with the cancelledPointers.delete in onPointerCancel (GestureHost.ts:2790) removed, because the later pen pointerdown clears the id at :2573 on its own. Asserting that a lift right after the pointercancel arms no guard would pin the rule the name describes.
There was a problem hiding this comment.
This pull request needs an engineering owner's decision before it can be approved, and changes besides. The scroll-lock commit carried from #8098 writes new requirements into component:BottomSheetSwitcher FR5, component:Drawer FR5 and component:MobileNav FR3 that current spec does not state; an engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request (the author may, as an engineering owner), or by approving them in #8098 and landing it first. Whatever that decision:
- Under a fine primary pointer the blur releases the press's page listeners before the lift (
GestureHost.ts:3042-3047), so a blur-cancelled pen still clicks; keep thepointerupsubscription while a refused lift is owed and add a fine-pointer case whose lift comes in a later task. .changeset/gesture-host-fixes.mdpromises that refusal everywhere, says a disabled recognizer brings no listener (it still brings passive ones), and announces stylus handlingmainalready has; the description's arena claim has no code change behind it.- The iOS WebKit check and the flat-tree walk carried from #8360 are each written twice (
GestureHost.ts,utils/selectionLock.ts); share them throughutils/touchQueries.tsandutils/flatTree.ts. - The diff carries 19,586 lines of open #8360 and #8098 against this commit's 507; land those first.
[Automated review]
| @@ -1780,49 +2712,126 @@ export class GestureHost { | |||
| } | |||
| const id = this.pointerIds.get(event.pointerId); | |||
| if (id == null) { | |||
| this.releaseArmedPress(); | |||
| // A contact the lost focus cancelled: its lift clicks nothing (FR28). | |||
| if (this.cancelledPointers.delete(event.pointerId)) { | |||
There was a problem hiding this comment.
Under a fine primary pointer this never runs for a blur-cancelled pen: onBlur calls releaseArmedPress, the arena has no touch left a microtask later, shouldListen() turns false and uninstall() drops this pointerup subscription, so the later lift's click lands. Keeping the page pointerup/pointercancel subscriptions while cancelledPointers holds an id would fix it; a fine-pointer case whose lift comes a task after the blur would pin it.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: gesture host fixes from review. A claimed pen holds only its own stylus touches, and a finger is matched to its scroller, its target and a moving element along the flat tree, through slots and shadow roots. A disabled recognizer brings no listener; where a finger comes from its pointer events, a claimed finger's touchend is cancelled too, a touchscreen laptop's included and inside an open shadow root, a hold's that claims with its lift included; and a pen or a pointer-read finger the window's blur cancelled clicks nothing at its later lift, while a mouse keeps its click. Not exported from the package entry point. |
There was a problem hiding this comment.
Three claims here go further than the code: the blur refusal holds only under a coarse primary pointer; a disabled recognizer still brings passive touchmove/touchend/touchcancel listeners on the finger's target (only the blocking one is gone); and a claimed pen already holds its stylus touches on main.
| expect(click.defaultPrevented).toBe(false); | ||
| }); | ||
|
|
||
| it('forgets a blur-cancelled pen whose contact ends in pointercancel: a later pen with its id keeps its click (spec:AST-094 FR28)', () => { |
There was a problem hiding this comment.
This case still passes with the cancelledPointers.delete in onPointerCancel removed, because the later pen pointerdown clears the id anyway. Asserting that a pointerup right after the pointercancel arms no guard would pin the rule.
There was a problem hiding this comment.
This pull request needs an engineering owner's decision before it can be approved, and changes besides. The scroll-lock commit it carries from #8098 writes new requirements into the BottomSheetSwitcher and Drawer FR5 records (no pull-to-refresh or bounce behind a modal flow or drawer) and the MobileNav FR3 record (the clip moves to <body> when <body> sets its own overflow); an engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request (the author may, as an engineering owner), or by approving and landing #8098 first. Independently of that decision:
- Under a fine primary pointer the blur releases the press's page listeners before the pen lifts, so its click still lands; keep the
pointerupandpointercancelsubscriptions while a refused id is pending and add a fine-pointer case whose lift comes in a later task. .changeset/gesture-host-fixes.mdand the description claim more than the change does: qualify the blur sentence until the fix lands, say "no blocking listener", and drop the stylus sentence and the arena claim.- Land #8360 and #8098 first so this diff carries only its own 507 lines instead of 19,667.
[Automated review]
| // not click (FR28). A mouse button held through the blur keeps its click. | ||
| for (const [pointerId, kind] of this.pointerKinds) { | ||
| if (kind !== 'mouse') { | ||
| this.cancelledPointers.add(pointerId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Under a fine primary pointer this id is recorded, but onBlur then calls releaseArmedPress, which uninstalls the page pointerup subscription a microtask later (shouldListen is false once no press is armed). The pen's later lift never reaches onPointerUp, so its click lands. A jsdom probe with the lift one task after the blur shows click.defaultPrevented === false under a fine pointer. Keeping the pointerup/pointercancel subscriptions while cancelledPointers is non-empty would close it; a fine-pointer case with a later-task lift would pin it.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: gesture host fixes from review. A claimed pen holds only its own stylus touches, and a finger is matched to its scroller, its target and a moving element along the flat tree, through slots and shadow roots. A disabled recognizer brings no listener; where a finger comes from its pointer events, a claimed finger's touchend is cancelled too, a touchscreen laptop's included and inside an open shadow root, a hold's that claims with its lift included; and a pen or a pointer-read finger the window's blur cancelled clicks nothing at its later lift, while a mouse keeps its click. Not exported from the package entry point. |
There was a problem hiding this comment.
Three parts of this line overstate the change: the blur refusal holds only under a coarse primary pointer today (see the comment in GestureHost.ts); a disabled recognizer's target still gets passive touchmove/touchend/touchcancel listeners, so "no blocking listener" is the accurate claim; and "A claimed pen holds only its own stylus touches" is already what main does.
| expect(click.defaultPrevented).toBe(false); | ||
| }); | ||
|
|
||
| it('forgets a blur-cancelled pen whose contact ends in pointercancel: a later pen with its id keeps its click (spec:AST-094 FR28)', () => { |
There was a problem hiding this comment.
This case still passes with the cancelledPointers.delete in onPointerCancel removed: the later pen pointerdown clears the id on its own. Asserting on a lift right after the pointercancel (no guard armed) would pin the rule.
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved, plus a few fixes. The scroll-lock commit carried from #8098 turns off pull-to-refresh and the page's bounce behind every modal overlay and moves MobileNav's clip to <body> on pages whose <body> sets its own overflow, but no current spec states either; an engineering owner can settle that by commenting /approve-spec <full head SHA> on this pull request (the author may, as an engineering owner) or by approving and landing the same text in #8098 first.
- Under a fine primary pointer the window blur removes the host's page
pointeruplistener before a pen's lift, so a blur-cancelled pen still clicks; keep that listener while a refusal is owed and add a fine-pointer case whose lift comes a task later. - The release notes and description claim more than the change does: the blur sentence holds only for a coarse pointer, a disabled recognizer still gets passive listeners,
GestureArena.tschanges only a comment, and closing MobileNav still drops another lock's scrollbar gutter. - Landing #8098 first brings this diff from 2,663 changed lines down to its own 507, and the
pointercanceltest should assert on what the cancel alone leaves.
[Automated review]
| for (const [pointerId, kind] of this.pointerKinds) { | ||
| if (kind !== 'mouse') { | ||
| this.cancelledPointers.add(pointerId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Under a fine primary pointer this id is recorded, but the blur's releaseArmedPress() then uninstalls the page pointerup subscription in a microtask (no arena touch is left, so shouldListen() turns false), and a pen lift that comes a task later reaches no host listener: its click lands. Keeping the page pointerup/pointercancel subscriptions while cancelledPointers is non-empty would close it; a fine-pointer case with the lift after await of one task pins it.
| --- | ||
|
|
||
| [chore] Internal: gesture host fixes from review. A claimed pen holds only its own stylus touches, and a finger is matched to its scroller, its target and a moving element along the flat tree, through slots and shadow roots. Not exported from the package entry point. | ||
| [chore] Internal: gesture host fixes from review. A claimed pen holds only its own stylus touches, and a finger is matched to its scroller, its target and a moving element along the flat tree, through slots and shadow roots. A disabled recognizer brings no listener; where a finger comes from its pointer events, a claimed finger's touchend is cancelled too, a touchscreen laptop's included and inside an open shadow root, a hold's that claims with its lift included; and a pen or a pointer-read finger the window's blur cancelled clicks nothing at its later lift, while a mouse keeps its click. Not exported from the package entry point. |
There was a problem hiding this comment.
Two parts of this line claim more than the code does: "clicks nothing at its later lift" holds only where the primary pointer is coarse (see the comment on cancelEverything), and "A disabled recognizer brings no listener" — the finger's target still gets passive touchmove, touchend and touchcancel listeners; "no blocking listener" is what the change and its test show.
| expect(click.defaultPrevented).toBe(false); | ||
| }); | ||
|
|
||
| it('forgets a blur-cancelled pen whose contact ends in pointercancel: a later pen with its id keeps its click (spec:AST-094 FR28)', () => { |
There was a problem hiding this comment.
This case still passes with the cancelledPointers.delete in onPointerCancel removed, because the later pen pointerdown clears the id on its own. Lifting right after the pointercancel (no new press) and asserting the click guard stays disarmed would pin the rule the title names.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] `useScrollLock`, and so Dialog, BottomSheet, Drawer and Lightbox, keeps the page where it is while an overlay is open. The lock used to pin `<body>` with `position: fixed` at minus the scroll offset and scroll back on release: the page's offset read 0 for the whole lock, and on iOS Safari the jump to the top brought the toolbar back, where it stayed after the overlay closed. Now the page gets `overflow: hidden` (on the root, or on `<body>` when the body's own overflow is the one the viewport uses), and on touch screens a one-finger pan that would scroll the page is cancelled, which iOS Safari before 26.4 needs because it scrolls an `overflow: hidden` page under a finger. Pans that a scroller inside the overlay can take, drags on a native range input, pinch-zoom, and one-finger drags on a pinch-zoomed page (they pan the zoomed view) still work. The lock no longer pins `<body>`, and `window.scrollY` keeps its value while the lock holds. The lock cancels those pans itself, leaving a touch a gesture recognizer owns to it, and judges each pan after a small slop so a swipe's first wobble does not fix the wrong axis. It follows the browser's scroll chain, so an overlay rendered inside a scrolling container still holds the page: a modal dialog's pans go to the page, not to that container. It also sets `overscroll-behavior-y: none` on the root and `<body>`, which turns off pull-to-refresh (Chrome on Android) while an overlay is open. MobileNav's own clip goes through the same page scroll-style claims, so closing or unmounting it no longer releases another overlay's lock. |
There was a problem hiding this comment.
"closing or unmounting it no longer releases another overlay's lock" holds for overflow only: MobileNav's own scrollbar-gutter hold still restores the root's gutter it saw at open, so closing it under a Dialog's lock drops the gutter while overflow: hidden holds, and after both close the root keeps scrollbar-gutter: stable. Either narrow the sentence to the clip or move the gutter into the same stacked claims.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision, and three fixes are needed whatever they decide.
The decision is whether a window blur during a pen press, or during a press by a finger the gesture host reads from pointer events, ends it with nothing acting on every surface the host serves (Switch, Slider and SegmentedControl now attach to it), which spec:AST-094 FR28 states only for menus and family:action-groups FR3 only for action groups, while component:Switch FR15 says a hold toggles at its release. 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.
Required changes:
- Under a fine primary pointer the blur removes the page
pointeruplistener a microtask before the lift, so a blur-cancelled pen still clicks (a Switch still toggles), and the case named for that scenario (GestureHost.test.ts:2664) lifts in the same task, before the listener goes. - On the pointer path, a pinch whose first finger lands on plain page and whose second lands on a one-finger pan still lets the pan claim, because only contacts that reached the arena are counted (
GestureHost.ts:3536-3600), against FR10. - The changeset says a blur-cancelled pen clicks nothing and a second finger "wherever it lands" leaves the page its pinch, and the fine-pointer case's name claims coverage it lacks; fix the behavior or narrow the wording.
The pointercancel case at GestureHost.test.ts:2715 also passes with the rule it names removed; asserting on a lift right after the cancel would pin it.
[Automated review]
| // The browser may keep a pen or a pointer-read finger down: its lift must | ||
| // not click. A mouse button held through the blur keeps its click. | ||
| for (const [pointerId, kind] of this.pointerKinds) { | ||
| if (kind !== 'mouse') { | ||
| this.cancelledPointers.add(pointerId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Under a fine primary pointer this refusal never runs for a lift that comes after the blur's microtask: onBlur calls releaseArmedPress, whose microtask drops the armed press, shouldListen turns false and uninstall removes the page pointerup listener that refuseCancelledLift hangs on. A pen held on a Switch through an app switch still toggles it on a desktop pen tablet. Keeping the page pointerup and pointercancel listeners while cancelledPointers is non-empty would close it.
| if ( | ||
| event.pointerType === 'touch' && | ||
| this.arena.touches.some( | ||
| touch => | ||
| touch.pointerType === 'touch' && | ||
| touch.phase !== 'ended' && | ||
| touch.phase !== 'cancelled', | ||
| ) | ||
| ) { | ||
| // Fingers from pointer events: a second one, wherever it lands, | ||
| // leaves the two to the page's pinch, as onTouchStart does. | ||
| this.failUnclaimedLoneFingerPans(); | ||
| } |
There was a problem hiding this comment.
This counts only arena touches, and a finger on an element with no participant returns at line 3566 before it gets there. So with the first finger on plain page and the second on a one-finger pan, neither check fires and the pan claims mid-pinch (FR10), while the Touch Events path, which counts event.touches, fails it. One live-finger count that includes untracked fingers, used here and at line 3595, would match it.
| expect(states).not.toContain('ended'); | ||
| }); | ||
|
|
||
| it("refuses its click to a pen the window blur cancelled under a fine primary pointer too, where the blur takes the page's pointerup listener before the lift (spec:AST-094 FR28)", () => { |
There was a problem hiding this comment.
The pointerup here runs in the same task as the blur, before the microtask that removes the page listener, so this passes even though a later lift is not refused under a fine pointer. await Promise.resolve() (or a task) before the lift, as the fine-pointer touchend case further down does, makes it fail until the listener is kept.
| expect(click.defaultPrevented).toBe(false); | ||
| }); | ||
|
|
||
| it('forgets a blur-cancelled pen whose contact ends in pointercancel: a later pen with its id keeps its click (spec:AST-094 FR28)', () => { |
There was a problem hiding this comment.
Low: this still passes with the cancelledPointers.delete in onPointerCancel removed, because the later pointerdown with the same id clears it anyway. Asserting that a lift right after the pointercancel arms no guard would pin the rule.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: where the gesture host reads a finger from its pointer events, the finger follows the Touch Events path's rules. A second finger, wherever it lands, leaves both to the page's pinch, and a pen or a finger the window's blur cancelled clicks nothing at its later lift, while a mouse keeps its click. A disabled recognizer brings no blocking listener. Not exported from the package entry point. |
There was a problem hiding this comment.
Both promises have gaps at this revision: a blur-cancelled pen still clicks under a fine primary pointer, and a pinch whose first finger lands on plain page still lets a one-finger pan claim when fingers come from pointer events. Fix those, or qualify the sentences until then.
| private readonly processed = new WeakSet<Event>(); | ||
| /** pointerId → arena id, for live mouse and pen contacts. */ | ||
| private readonly pointerIds = new Map<number, number>(); | ||
| /** pointerId → its kind, for the same contacts. */ |
There was a problem hiding this comment.
Non-blocking: this restates the Map type, and "the same contacts" inherits the comment above, though pointer-read fingers are recorded too. Something like /** Each live contact's kind: a blur refuses a pen's or a finger's lift, never a mouse's. */ keeps only the reason.
|
Thanks. At bac5a17 the owner's ruling is recorded as AST-094 FR44: a window blur, or the browser cancelling the pointer, ends every touch or pen press the host tracks, and nothing acts at the later lift; a mouse button held through the blur keeps its click. FR28 and action-groups FR3 point to it, and the owner's stamp follows at this head.
|
|
/approve-spec bac5a17 |
There was a problem hiding this comment.
Changes requested. The blur refusal for pens and pointer-read fingers matches the newly approved AST-094 FR44, and the coarse-pointer pinch fix and the disabled-recognizer listener gate hold. Three items remain:
- Under a fine primary pointer a first pointer-read finger resting on plain page is never counted (
GestureHost.tslines 3550-3557), so a one-finger pan still claims the pinch against FR10 and the changeset; count a touchpointerdownwhoseisPrimaryis false as a second finger and add the fine-pointer case, or narrow the changeset. - When a blurred pen's lift never reaches the page, its id stays in
cancelledPointersand the armed press never ends under a fine pointer (GestureHost.tslines 1805-1809), so the page listeners stay on after every later release against FR24; bound the wait and add a case whose lift never comes. - FR44's verification row asks for Switch and Slider cases under a coarse and a fine pointer, but the Switch case runs only under a fine pointer and the Slider case only under a coarse one; add the missing two or narrow the row.
Non-blocking: the comment on pointerKinds (line 721) restates the map's type, and "the same contacts" leaves out the pointer-read fingers it also records.
[Automated review]
| if (event.pointerType === 'touch') { | ||
| this.heardFingers.add(event.pointerId); | ||
| if (this.heardFingers.size > 1) { | ||
| // Fingers from pointer events: a second one, wherever either lands, | ||
| // leaves the two to the page's pinch, as onTouchStart does. | ||
| this.failUnclaimedLoneFingerPans(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Under a fine primary pointer the host listens only during an armed press, so a first finger resting on plain page never reaches this count; when the second finger lands on a one-finger pan the count is 1 and the pan claims the pinch (FR10). A touch pointerdown whose isPrimary is false means another finger is already down, wherever it landed, so counting it as a second finger would close the gap; a fine-pointer case would pin it.
| if ( | ||
| this.isArmedPress && | ||
| this.arena.touches.length === 0 && | ||
| this.cancelledPointers.size === 0 | ||
| ) { |
There was a problem hiding this comment.
If a blurred pen's lift never reaches the page (another window took focus, or the page went hidden) and the pen's next contact has another pointer id, cancelledPointers keeps the old id and this press never ends under a fine pointer: the page listeners stay on after every later release (FR24). Bounding the wait, for example forgetting noted pens when another pen press begins, and a case whose lift never comes would fix it.
| | FR42 | Default-theme geometry tests under a coarse and a fine pointer, and hit-testing between adjacent targets | every component at phone width with a finger and with a mouse; a dense toolbar; a chip and its remove button; a Switch beside a label link | a default-theme box that grows under touch or at phone width other than with its text, or, where no component or module record sets the hit area, a touch target under 44 px where neighbors allow it or a hit area that overlaps a neighbor's target, fails | | ||
| | FR43 | Browser tests with a mouse, a trackpad, a pen, a finger, and the keyboard, and resource counts | a pan by drag and by wheel; a slider's thumb under a wheel and a trackpad scroll; a ctrl-wheel pinch; a long press, a right click, and Shift+F10; a mouse press; a handle drag; a row swipe with a mouse and a trackpad; a component with no gesture | a gesture missing under a mouse or pen, a mouse-drag swipe action, a pen pinch, an edge pan under a mouse, a trackpad, or a pen, a press delay under a fine pointer, a wheel or trackpad scroll that changes a thumb's value or does not reach the scroll under it, an idle listener in a component with no gesture, or added stylesheet bytes, fails | | ||
| | FR43 trackpad | Unit tests on timed wheel streams; WebKit browser tests with trackpad streams (Chromium's driver wheels read as a mouse wheel) | a form and a required sheet pulled 30 to 240 px by a finger and by a trackpad; mouse lines, notches, ticks and accelerated deltas over a sheet, a pager and a row; ten swipes each starting mid-momentum, ten whose momentum ends, and a slow drag with 150 ms pauses; a trackpad flick and a slow stopped drag on an info sheet; a fling, a slow 60 px swipe, a 42 px spike and back-to-back swipes on a pager; a row opened and swiped back within one gesture, and a long fast swipe; a pinch ending; a trackpad pan through a window blur and through a hidden page | a trackpad stretch that differs from a finger's for the same travel (120 px gives 54 px on the reference sheet), a mouse wheel that moves a sheet, a pager or a row, splitter counts other than ten, ten and one, momentum that moves a surface, a flick released with no velocity, a pager that travels past one page or pages on a spike, an info sheet a flick leaves open or a slow stopped drag closes, an open row that reveals its other side within one gesture, a trackpad pan a window blur or a hidden page leaves mid-gesture, or a pinch that ends after 100 ms fails | | ||
| | FR44 | Unit tests of a pen press a window blur ends, on a Switch, a Slider and the host, under a coarse and a fine pointer | a pen press on a Switch and on a Slider, blur mid-press: nothing toggles or commits; a pen whose contact ends in `pointercancel` | with the host keeping no note of a blur-cancelled pen, the Switch toggles at the pen's click | |
There was a problem hiding this comment.
This row asks for the Switch and the Slider under a coarse and a fine pointer, but the Switch case runs only under a fine pointer and the Slider case only under a coarse one. Adding the two missing cases, or narrowing the row, keeps the record and the tests in step.
| private readonly processed = new WeakSet<Event>(); | ||
| /** pointerId → arena id, for live mouse and pen contacts. */ | ||
| private readonly pointerIds = new Map<number, number>(); | ||
| /** pointerId → its kind, for the same contacts. */ |
There was a problem hiding this comment.
Non-blocking: this restates the map's type, and "the same contacts" points at pointerIds' "live mouse and pen contacts", although pointer-read fingers are recorded too. Suggested: /** Each live contact's kind. A blur refuses a pen's or a finger's lift, never a mouse's. */
|
Thanks. At 7bded4d:
|
There was a problem hiding this comment.
Changes requested. The blur refusal for pens and pointer-read fingers matches the owner-approved AST-094 FR44, and the three earlier fixes (the fine-pointer pinch, the page listeners after a lost pen lift, and the Switch and Slider cases under both pointers) are in place. Two host issues remain:
- The second pinch check after delivery (
GestureHost.ts:2956-2960and:3616-3623) also fails a second finger's own one-finger pan while the first finger's drag is claimed, which main allows and FR10 does not ask for ("no drag is claimed"), so run it only while no touch is claimed and add that case. - A pointer-read finger whose lift never reaches the page stays in
heardFingers(GestureHost.ts:3565), so under a coarse pointer every later lone finger counts as a second finger and its one-finger pan fails; resetting the set at a primary finger'spointerdown, with the second finger marked non-primary in the two pinch cases, fixes it.
Optionally, add heardFingers to the "nothing left after the last detach" field list.
[Automated review]
| // Again once the new fingers are delivered: a pinch's second finger may | ||
| // have landed on another one-finger pan, the next row. | ||
| if (this.fingerTargets.length > 1) { | ||
| this.failUnclaimedLoneFingerPans(); | ||
| } |
There was a problem hiding this comment.
This runs whether or not another finger's drag is claimed, and failUnclaimedLoneFingerPans only checks the pan's own touches. So while finger 1 drags a claimed one-finger pan, a second finger landing on another one-finger pan (another row, a second Slider thumb) has its pan cancelled; main lets it begin. FR10 keeps the page's pinch only when "no drag is claimed", and a claimed drag already blocks the pinch. Suggest guarding this call and the one at :3616-3623 with !this.arena.touches.some(t => this.arena.isClaimed(t.id)), plus a case for the claimed-drag order; with that guard every existing case still passes.
| return; | ||
| } | ||
| if (event.pointerType === 'touch') { | ||
| this.heardFingers.add(event.pointerId); |
There was a problem hiding this comment.
An id leaves heardFingers only at its pointerup, its pointercancel or the uninstall. When a blur-cancelled finger's lift never reaches the page (the case dropCancelledLifts handles for the refusal), the id stays here, and under a coarse pointer the host never uninstalls, so every later lone finger counts as a second finger and its one-finger pan fails. A touch pointerdown with isPrimary true means no other finger is down, so resetting the set to this finger there fixes it; the cases at GestureHost.test.ts:5338 and :5396 then need their second finger sent with isPrimary: false, as browsers do.
|
Thanks. At cc7ccc3:
|
There was a problem hiding this comment.
Changes requested. The blur refusal for pens and pointer-read fingers matches the owner-approved AST-094 FR44, and both earlier pinch findings are fixed and covered. Two host gaps remain:
refuseCancelledLift(GestureHost.ts:4043) arms the click guard withouttakesLateMouse, so a blur-refused finger read from pointer events still sends WebKit's latemousedownandmouseupto the page, unlike the Touch Events refusal (:3214) and the claimed lift (:3697); pass{takesLateMouse: event.pointerType === 'touch'}and add a case.- Under a fine pointer with Touch Events,
onPointerDownreturns for a finger (:3549-3556) beforedropCancelledLifts, so once a blur-cancelled pen's lift is lost, later finger presses leave the page listeners installed until a mouse or pen press (FR24); drop the note before that return as well and add a case.
Optionally, the comment at GestureHost.ts:2903 restates the isEnabled checks below it.
[Automated review]
| * and a finger's touchend after it is cancelled. | ||
| */ | ||
| private refuseCancelledLift(event: PointerEvent): void { | ||
| armClickGuard(this.doc); |
There was a problem hiding this comment.
This arms the guard without takesLateMouse, then cancels the finger's touchend (lines 4044-4046). WebKit sends a late mousedown and mouseup after a held tap's cancelled touchend, so a finger read from pointer events and held through a blur still reaches the page's mouse handlers, while the Touch Events refusal (line 3214) and the claimed lift (line 3697) take both. armClickGuard(this.doc, {takesLateMouse: event.pointerType === 'touch'}) closes it; a case that dispatches a mousedown after the refused lift fails today and passes with that change.
| // host never heard the first. | ||
| this.failUnclaimedLoneFingerPans(); | ||
| } | ||
| this.dropCancelledLifts(event); |
There was a problem hiding this comment.
A Touch Events finger's pointerdown returns at lines 3549-3556 before it reaches this call. So under a fine primary pointer with Touch Events, a lost pen lift's note survives every later finger press, and releaseArmedPress keeps the page listeners installed after each release until a mouse or pen press (FR24). Calling this.dropCancelledLifts(event) before that return as well ends the wait at any press, as the description says.
| toLock.push({id, target, consumerElement}); | ||
| } | ||
| // The finger's events keep going to what the engine hit. | ||
| // Only an enabled participant brings a blocking listener. |
There was a problem hiding this comment.
Non-blocking (INV29): this restates the isEnabled checks below it. Drop it, or keep only the reason: "A disabled recognizer claims nothing, so it must not hold a scroll."
|
Thanks. At 421b17f:
|
There was a problem hiding this comment.
Changes requested. The blur refusal for pens and pointer-read fingers matches the owner-approved AST-094 FR44, and both earlier findings are fixed; one regression remains.
- Moving the
touchstartpinch check ahead of the input-path return (GestureHost.ts:2838-2842) makes it run after Chromium has already delivered a second finger from itspointerdown, so where Touch Events arrive withoutontouchstart(touch emulation turned on after load) that finger's own one-finger pan is cancelled beside a claimed drag, which main keeps; putting the check back after theusesTouchEventsandtakeEventreturn fixes it, since the pointer path'sisPrimaryrule already covers pointer-read fingers.
A case in the "no ontouchstart" block that sends each finger's pointerdown and then its touchstart would pin this.
[Automated review]
| // A second finger leaves the two to the page's pinch, whichever events | ||
| // the host reads fingers from. | ||
| if (this.fingerTargets.length > 1) { | ||
| this.failUnclaimedLoneFingerPans(); | ||
| } |
There was a problem hiding this comment.
Ahead of the input-path return, this check now runs where the host reads fingers from pointer events too. Chromium sends a finger's pointerdown before its touchstart, so by now the second finger is already delivered, and with no isFingerClaimed() guard its own one-finger pan is cancelled beside a claimed drag (Touch Events without ontouchstart, as after touch emulation turns on); main keeps it. The pointer path's isPrimary rule at 3558-3564 and 3608-3616 already covers pointer-read fingers, so putting this back after the usesTouchEvents and takeEvent return fixes it, and the existing cases still pass.
… the pinch rule of the Touch Events path Where the host reads a finger from its pointer events (inputMode 'pointer-events', or a window with no ontouchstart, as after touch emulation turns on), a pen or a finger the window's blur cancelled clicked at its later lift, so a Switch toggled. Under a fine primary pointer the blur also took the page's pointerup listener before the lift. A second finger beside a first resting on plain page let a one-finger pan claim a pinch. And a target got its blocking listener without the host asking whether a recognizer there was enabled. AST-094 FR44 records the rule for every press the host tracks: a window blur, or the browser cancelling the pointer, ends it on every surface and for every pointer, and nothing acts at the later lift. FR28 and action-groups FR3 point to it, and a verification row names its tests. The pointer path keeps the kind of each live contact. A blur marks every pen and pointer-read finger, whose later lift clicks nothing and sends the page no late mouse events, while a mouse keeps its click. An armed press keeps its page listeners until that lift, or the next press. On the pointer path a finger that is not the primary pointer is a second finger, so it fails the unclaimed one-finger pans unless a drag is claimed. Only an enabled participant brings a blocking listener. Cases for each fail before this change; a pen press on a Switch that a blur ends toggles nothing, and one on a Slider commits nothing. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
|
Thanks. At eafaa1c the Touch Events pinch check is back after the input-path return, as on main, so a pointer-read second finger keeps its own pan beside a claimed drag even when Touch Events still arrive. A case in the "no ontouchstart" block covers it, and the pointer path's own rule still covers a pointer-read pinch. |
There was a problem hiding this comment.
Recommended for approval once the pull request leaves draft. The blur refusal for pens and pointer-read fingers matches spec:AST-094 FR44, which an engineering owner approved, the pinch and listener changes follow FR10 and FR19, and the earlier touchstart ordering regression is fixed.
- Optional: a cross-reference from
component:SwitchFR15 to FR44 would state in the Switch record that a press a blur ends does not toggle. - Optional: the description says browser-proof results follow, but none are listed.
[Automated review]
|
/approve-spec eafaa1c |
One commit on main. It absorbs the closed #8336, whose authors the commit credits.
User impact
Where the host reads fingers from pointer events (
inputMode: 'pointer-events', or noontouchstart), a pen or a finger the window's blur cancelled clicked at its later lift, so a Switch toggled. Under a fine pointer the blur even removed thepointeruplistener first. A second finger beside one resting on plain page let a one-finger pan claim a pinch. A disabled recognizer still added a blocking listener.Problem and solution fit
The host remembers each blur-cancelled pen or pointer-read finger until its lift, which clicks nothing. The pointer path reads a non-primary finger as a second one. Only an enabled participant adds a blocking listener.
Expected behavior and authority
spec:AST-094(current): FR10, FR19, FR24, and FR44, added beside FR28 as the owner ruled: "A window blur, or the browser cancelling the pointer, ends any press the gesture host tracks, on every surface and for every touch and pen, and nothing acts at the later lift; a mouse button held through the blur keeps its click." FR28 andfamily:action-groupsFR3 point to it.Smallest restoration
GestureHost.tsonly. A lost lift's wait ends at the next press, as the click guard reads one.Evidence
Writing rubric results
Scope
Testing
Local gates at eafaa1c, on main ce667be:
tsc --project packages/core/tsconfig.json --noEmit, strict ESLint and Prettier on the changed files,check:*andpnpm review:pre: clean.--cpus=4,CI=1,LP_NUM_THREADS=4, one worker), ten runs, 640 passed and 120 skipped, none failed.