Repository navigation
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
The exact-head preview is not available yet. Generated by PR Enrichment workflow | View current CI run |
41f2816 to
8a73f1e
Compare
github-actions
Bot
added
the
needs:design-review
|
This push (bae8ff7) replaces the next-frame write with a mark, and the new cases failed first.
|
There was a problem hiding this comment.
This pull request is on hold for one engineering-owner decision. The carried rule that counts a finger's uncancelable touchmove as the browser's only when it is the first one and the browser could pan along it narrows current spec:AST-094 FR20. An engineering owner records that decision by approving #8456 or by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Whatever that decision, these still need changes:
- The host's
touch-actionmark is an unlayered, zero-specificity rule, so it overrides any author value in a cascade layer (Astryx's own@layer astryx-baseand@layer productincluded) and any*rule, which leaves the derived value wider than the author's and contradicts FR18. - The diff carries 21,184 authored lines of the unmerged stack against this pull request's own 1,137, so the predecessors should land first and the
astryxStylexchange should move to #8075. - The pen fix needs iPadOS Safari evidence under
spec:AST-013FR10, and the 10 px scroll rule needs real-touch cases at 9 px and 10 px straight and at (7, 7) and (8, 8), with a drag proof that can fail on the rule. - A finger's target should get the blocking listener only for an enabled recognizer (
GestureHost.ts:873), and the README should stop promising thatusePressTrackerrebuilds its tracker.
[Automated review]
| } | ||
| if (!entry.values.has(value)) { | ||
| entry.values.add(value); | ||
| entry.css += `:where([${TOUCH_ACTION_ATTRIBUTE}="${value}"]){touch-action:${value}}`; |
There was a problem hiding this comment.
This rule is unlayered and has zero specificity, so it beats every author touch-action declared in a cascade layer: Astryx's own stylesheet (@layer astryx-base, scripts/build-css.mjs:367) and product styles built by @astryxdesign/build (@layer product). Because adopted sheets come last, it also beats an unlayered * rule. In Chromium, a @layer astryx-base pinch-zoom and a @layer product ID rule's none both compute to manipulation once the element is marked. The next frame's read then records no author value (GestureHost.ts:592-596), so the derived value stays wider than the author's, against current spec:AST-094 FR18 ("never wider than it"). Declaring the rule in a cascade layer that Astryx's layer order registers before reset, with a real-browser case whose author value sits in @layer astryx-base, would fix it.
| this.watchTarget( | ||
| touch.identifier, | ||
| target, | ||
| recognizers.some(claimsWhileMoving), |
There was a problem hiding this comment.
claimsWhileMoving does not check isEnabled here, while the element listener does (r.isEnabled && claimsWhileMoving(r), line 464), and participantsAt includes disabled recognizers. So whenever anything else keeps the host listening, a finger landing on an element whose only claiming recognizer is disabled gets a blocking target listener, and a scroll starting there waits for the main thread, against FR19. recognizers.some(r => r.isEnabled && claimsWhileMoving(r)), with a case in the passive-listener test, would fix it.
| if (inputs.length === 0) { | ||
| return; | ||
| } | ||
| if (!event.cancelable) { |
There was a problem hiding this comment.
This rule narrows current spec:AST-094 FR20, which counts every uncancelable touchmove as the browser taking the touch. Here a finger's first uncancelable touchmove counts only where the browser could pan along its main axis, and later ones never count. #8456 proposes the amendment and has no owner decision yet. An engineering owner records one by approving #8456 or by commenting /approve-spec <full head SHA> on this pull request.
| return; | ||
| } | ||
| } | ||
| if (moved.some(touch => (touch as StylusTouch).touchType === 'stylus')) { |
There was a problem hiding this comment.
touchType === 'stylus' exists only in iPadOS Safari, so the defect this fixes reproduces only there, and its one test dispatches a synthetic event in jsdom. Current spec:AST-013 FR10 asks for evidence from that browser: iPadOS Safari, with a pen drawing on a claimed element while a finger scrolls elsewhere, before and after.
| }); | ||
| const start = await center(page, 'feedRow'); | ||
| await clearLog(page); | ||
| await driver.play(drag(start, {x: start.x, y: start.y - 60}, 30)); |
There was a problem hiding this comment.
This drag travels 60 px, past feedTap's default 45 px allowableMovement. The tap therefore fails by its own movement whether or not the scroll takes the touch, and the test passes with the scroll rule removed. AST-094's FR18–FR22 verification row asks for real-touch cases at 9 px and 10 px straight and at (7, 7) and (8, 8). A consumer that tolerates that movement, such as a long press with a large allowableMovement, would show the take at the boundary.
| it off owns that end (no consumer callback from an Effect). A handler that | ||
| wants UIKit's `.cancelled` calls `recognizer.setEnabled(false)` itself, | ||
| which reports it at once. | ||
| - `usePressTracker` rebuilds its tracker when `trackingExpansion`, |
There was a problem hiding this comment.
usePressTracker builds its tracker once in useState and never rebuilds it (useGestureRecognizer.ts:136-172, and its attachment depends only on [tracker]). Changing these four options after mount therefore has no effect. Either rebuild the tracker when they change, or say they are read once.
There was a problem hiding this comment.
This pull request is on hold for one engineering-owner decision: the carried rule that counts a finger's uncancelable touchmove as the browser's only when it is the first one and the browser could pan along it narrows current spec:AST-094 FR20, so an engineering owner needs to update that spec by approving #8456, or record the decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). Whatever that decision, these still need changes:
- The host's
touch-actionmark is an unlayered zero-specificity rule, so it overrides any author value in a cascade layer (Astryx's own@layer astryx-baseand@layer productincluded) or a*rule, which leaves the derived value wider than the author's and contradicts FR18. - The diff carries 21,320 authored lines of the unmerged stack against this pull request's own 1,243, so the predecessors should land first and the
astryxStylexchange should move to #8075. - The pen fix needs iPadOS Safari evidence under
spec:AST-013FR10, and the 10 px scroll rule needs real-touch cases at 9 px and 10 px straight and at (7, 7) and (8, 8), with a drag proof that can fail on the rule. - A pen whose touch the window blur cancelled should lift on nothing as a finger now does, a finger's target should get the blocking listener only for an enabled recognizer (
GestureHost.ts:887), and the README should stop promising thatusePressTrackerrebuilds its tracker.
[Automated review]
| } | ||
| if (!entry.values.has(value)) { | ||
| entry.values.add(value); | ||
| entry.css += `:where([${TOUCH_ACTION_ATTRIBUTE}="${value}"]){touch-action:${value}}`; |
There was a problem hiding this comment.
This rule is unlayered, so it beats every layered author touch-action (Astryx's own @layer astryx-base and @layer product) and any * rule: with a long press attached, Chromium keeps manipulation in force over an author's pinch-zoom or none there, against spec:AST-094 FR18. Declaring the mark in a layer that Astryx's layer-order statements register before reset would let every author rule outrank it.
| this.watchTarget( | ||
| touch.identifier, | ||
| target, | ||
| recognizers.some(claimsWhileMoving), |
There was a problem hiding this comment.
Unlike the element listener (line 475), this ignores isEnabled, and participantsAt includes disabled recognizers, so a finger on an element whose only claiming recognizer is disabled gets a blocking touchmove listener on its target (FR19). recognizers.some(r => r.isEnabled && claimsWhileMoving(r)) would match the element listener.
| ids.push(id); | ||
| } | ||
| } | ||
| this.pointerIds.clear(); |
There was a problem hiding this comment.
Pen contacts are dropped here without being remembered, so a pen the blur cancelled finds no id at its pointerup (lines 1373-1376), arms no swallower, and its untracked stylus touchend is not cancelled: its click still lands, while a finger's is refused. FR28 covers menus driven by "a finger or pen".
|
Deployment failed for project astryx with the following error: |
There was a problem hiding this comment.
This pull request is on hold for one engineering-owner decision: the carried rule that counts a finger's uncancelable touchmove as the browser's only when it is the first one and the browser could pan along it narrows current spec:AST-094 FR20, so FR20 needs updating (approving #8456 does that), or an engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, which the author may do as an engineering owner. Whatever that decision, these still need changes:
- Land the unmerged stack first or retarget: the diff carries 21,586 authored lines against this pull request's own 1,304, and two carried modules (Lab's internal-module resolvers and multi-edge screen-edge pans) contradict records that have since landed on
main. - Add real-touch browser cases for the 10 px scroll rule at 9 px and 10 px straight and at (7, 7) and (8, 8), and iPadOS Safari evidence for the pen fix (
spec:AST-013FR10). - Give a finger's target the blocking listener only for an enabled recognizer (
GestureHost.ts:887), and update the README and host header, which still say thetouch-actionis set inline.
[Automated review]
| this.watchTarget( | ||
| touch.identifier, | ||
| target, | ||
| recognizers.some(claimsWhileMoving), |
There was a problem hiding this comment.
participantsAt returns disabled recognizers too (lines 765-768), and claimsWhileMoving ignores isEnabled, unlike the element listener at line 475. A finger landing on an element whose only claiming recognizer was attached through the host and then turned off with setEnabled(false) still gets a non-passive touchmove listener on its target, so a scroll starting there waits on a busy main thread, against spec:AST-094 FR19 ("Only an element with a recognizer that must claim mid-gesture registers that blocking listener"). Suggest recognizers.some(r => r.isEnabled && claimsWhileMoving(r)), with a case beside the passive-listener test.
| ids.push(id); | ||
| } | ||
| } | ||
| this.pointerIds.clear(); |
There was a problem hiding this comment.
Pens are dropped here without being remembered, so a pen the blur cancelled finds no id at its pointerup (lines 1373-1376), arms no swallower, and where the browser keeps the contact alive its click reaches the page. Fingers are now refused (lines 1018-1020); FR28 says losing focus means nothing acts. Remembering cancelled pen pointer ids, as cancelledFingers does, would close it.
| * layer (`reset`), so any author rule, in any layer or none, or inline | ||
| * value outranks the host's (`spec:AST-094` FR18, DEC-8) |
There was a problem hiding this comment.
This holds only where reset is registered before every author layer. In Chromium 149, an author touch-action: none in a layer the page registers before reset, or in a shadow root whose styles lack Astryx's order statement, loses to this rule, and the next-frame read then records no author value, so manipulation stays in force. Every setup Astryx documents registers reset first (spec:AST-080 FR3), so stating that precondition here and in the README would make the claim accurate.
| - **`touch-action`.** Each element gets the intersection of what its | ||
| recognizers leave the browser, and never more than the element's own | ||
| value allows: a caller's explicit `touch-action` (a trigger's `none`) | ||
| stays in force, as the intersection of both. It is set inline on attach |
There was a problem hiding this comment.
Since the mark, the value is no longer set inline on attach: the host sets data-astryx-touch-action under a @layer reset rule and writes inline only an intersection with the author's value. The header comment at GestureHost.ts:37-40 says the same and needs the same update.
| }); | ||
| const start = await center(page, 'feedRow'); | ||
| await clearLog(page); | ||
| await driver.play(drag(start, {x: start.x, y: start.y - 60}, 30)); |
There was a problem hiding this comment.
This drag travels 60 px, past the feed tap's default 45 px allowableMovement, so the tap fails by its own movement whether or not the scroll takes the touch, and the case cannot fail on the 10 px rule. The current spec:AST-094 FR18–FR22 verification row asks for real-touch cases at 9 px and 10 px straight and at (7, 7) and (8, 8); the boundary cases this push adds are jsdom only.
| edge => edge === 'top' || edge === 'bottom', | ||
| ); | ||
| return { | ||
| panX: !horizontal, |
There was a problem hiding this comment.
With more than one horizontal edge this drops pan-x from the whole element, so a page's horizontal scrolling stops everywhere on it. Current spec:AST-094 FR18 keeps "the author's scrolling … untouched" outside the 40.2 px band, and DEC-13 rejects "claiming pan-x across the whole element". This came in with the recognizers beneath this pull request and should conform there.
|
Posted by Cindy's agent on her behalf. Cindy's decision on gestures on the web: gestures attach for every pointer, not only touch.
This supersedes the earlier exclusion of mouse press paint. The |
There was a problem hiding this comment.
This pull request is on hold for one engineering-owner decision: the carried rule that counts a finger's uncancelable touchmove as the browser's only when it is the first one and the browser could pan along it narrows current spec:AST-094 FR20, so FR20 needs updating (approving #8456 does that), or an engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, which the author may do as an engineering owner. Whatever that decision, these still need changes:
- Land the unmerged stack first or retarget: the diff carries 18,542 authored lines against this pull request's own 1,733.
- Make the real-touch cases at 10 px and (8, 8) able to fail (they drag toward the top of a list that is already at its top, so the list never moves), and add iPadOS Safari evidence for the pen fix (
spec:AST-013FR10). - Give a finger's target the blocking listener only for an enabled recognizer (
GestureHost.ts:949), and update the README and host header, which still say thetouch-actionis set inline.
[Automated review]
| if (!event.cancelable) { | ||
| // The browser no longer asks the page about these fingers. WebKit | ||
| // stops asking when it takes the touch to pan (its pointercancel comes | ||
| // with it); Chromium also stops after one unconsumed touchmove, even | ||
| // when `touch-action` keeps it from panning the way the finger went (a | ||
| // horizontal drag under pan-y). So a finger's FIRST uncancelable move | ||
| // counts as the browser's claim only if the browser could pan along | ||
| // the finger's main axis since touch-down; later ones add nothing, and | ||
| // pointercancel or a scroll still report any real pan. | ||
| const placed = new Map(inputs.map(input => [input.id, input])); | ||
| const taken = ids.filter(id => { | ||
| const touch = this.arena.getTouch(id); | ||
| if ( | ||
| touch == null || | ||
| touch.takenByBrowser || | ||
| this.arena.isClaimed(id) || | ||
| touch.scroll?.allowsNativePan === false || | ||
| this.judgedUncancelable.has(id) | ||
| ) { | ||
| return false; | ||
| } | ||
| this.judgedUncancelable.add(id); | ||
| const input = placed.get(id); | ||
| if (input == null || touch.scroll == null) { | ||
| return true; | ||
| } | ||
| const dx = input.x - touch.startX; | ||
| const dy = input.y - touch.startY; | ||
| return Math.abs(dx) >= Math.abs(dy) | ||
| ? touch.scroll.canScroll(dx, 0) | ||
| : touch.scroll.canScroll(0, dy); | ||
| }); | ||
| if (taken.length > 0) { | ||
| this.arena.browserTookOver(taken, time); | ||
| } | ||
| } |
There was a problem hiding this comment.
This rule narrows current spec:AST-094 FR20, which says an uncancelable touchmove MUST count as the browser taking the touch: here only a finger's first one counts, and only where the browser could pan along the finger's main axis. #8456 proposes the amendment and has no owner decision yet; an engineering owner records the decision by approving #8456 or by commenting /approve-spec <full head SHA> on this pull request.
| this.watchTarget( | ||
| touch.identifier, | ||
| target, | ||
| recognizers.some(claimsWhileMoving), |
There was a problem hiding this comment.
claimsWhileMoving ignores isEnabled here, while the element listener checks r.isEnabled && claimsWhileMoving(r) (line 506), and participantsAt includes disabled recognizers. A finger landing on an element whose only claiming recognizer was attached and then disabled with setEnabled(false) gets a blocking target listener whenever something else keeps the host listening, so a scroll starting there waits on the main thread (FR19). recognizers.some(r => r.isEnabled && claimsWhileMoving(r)) fixes it.
| await driver.play(drag(at, {x: at.x + travel.dx, y: at.y + travel.dy}, 6)); | ||
| await page.waitForTimeout(100); | ||
| const taps = await page.evaluate( | ||
| () => (window as unknown as {__taps: string[]}).__taps, | ||
| ); | ||
| const scrolled = (await scrollTop(page, 'list')) > 0; | ||
| if (travel.keeps) { | ||
| // Under 10 px of travel: a tap, and no scroll took the touch. | ||
| expect(taps).toContain('ended'); | ||
| expect(scrolled).toBe(false); | ||
| } else { | ||
| // From 10 px the browser may scroll (WebKit on a phone starts at 10; | ||
| // Chromium waits longer): if it does, the scroll takes the touch and | ||
| // there is no tap; if it does not, the tap stands. | ||
| expect(scrolled && taps.includes('ended')).toBe(false); |
There was a problem hiding this comment.
This drags the finger down over a fresh list at its top (openTapInScroller), toward an edge it cannot scroll past, so scrolled never holds in any engine and the 10 px and (8, 8) cases always take the "tap stands" branch: they pass with the take-distance check removed. Holding the finger after the move, scrolling the list from script as the still-finger case does, then lifting would make them fail when the 10 px rule breaks, which the FR18–FR22 row asks for.
| if (moved.some(touch => (touch as StylusTouch).touchType === 'stylus')) { | ||
| for (const id of this.pointerIds.values()) { | ||
| if (this.arena.isClaimed(id)) { | ||
| // A pen whose pointer a recognizer owns: its stylus touches too, | ||
| // never a finger no recognizer owns. | ||
| event.preventDefault(); | ||
| return; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
touchType === 'stylus' exists only in iPadOS Safari, so the defect this fixes reproduces only there, and its one test dispatches a synthetic event in jsdom. Current spec:AST-013 FR10 asks for evidence from that browser: iPadOS Safari with a pen drawing on a claimed element while a finger scrolls elsewhere.
| private cancelEverything(time: number): void { | ||
| const ids = [...this.pointerIds.values()]; | ||
| for (const id of ids) { | ||
| this.unlockSelection(id); | ||
| } | ||
| for (const identifier of [...this.touchIds.keys()]) { | ||
| // The browser may keep the touch alive: its lift must not click. | ||
| this.cancelledFingers.add(identifier); | ||
| const id = this.forgetFinger(identifier); | ||
| if (id != null) { | ||
| ids.push(id); | ||
| } | ||
| } | ||
| this.pointerIds.clear(); |
There was a problem hiding this comment.
Fingers cancelled here are remembered in cancelledFingers, but pens are dropped with pointerIds, so a pen's later pointerup finds no id and returns before armSwallower (lines 1630-1633), and its click reaches the page where the browser keeps the contact alive. FR28 says losing focus means nothing acts, for a finger or a pen; remembering cancelled pen pointer ids and arming the swallower at their pointerup would close it.
| * the derived value in TOUCH_ACTION_ATTRIBUTE, and its document or shadow | ||
| * root holds one zero-specificity rule per value in the first cascade | ||
| * layer (`reset`), so any author rule, in any layer or none, or inline | ||
| * value outranks the host's (`spec:AST-094` FR18, DEC-8) |
There was a problem hiding this comment.
An author rule in a layer the page registers before reset, or in a shadow root whose styles lack Astryx's order statement, loses to this rule, and the next-frame read then records no author value, so the derived value stays wider than the author's. Saying the guarantee holds where reset is registered first (spec:AST-080 FR3) would keep this accurate.
| stays in force, as the intersection of both. It is set inline on attach | ||
| and restored on detach (opt out with `manageStyle: false`): |
There was a problem hiding this comment.
The host now marks the element under a stylesheet rule and writes inline only an intersection, so "set inline on attach" no longer describes it; the host header (GestureHost.ts:43-46) says the same.
| - `usePressTracker` rebuilds its tracker when `trackingExpansion`, | ||
| `highlightDelay`, `cancelTravel` or `quickTapFlash` change, from the next | ||
| touch, and reads its callbacks (`isBraking` included) on every touch. |
There was a problem hiding this comment.
usePressTracker builds its tracker once in useState and attaches it only on [tracker] (useGestureRecognizer.ts:136-195), so changing these four options after mount has no effect. Either rebuild the tracker when they change or say they are read once.
There was a problem hiding this comment.
This pull request is on hold for one engineering-owner decision: the carried rule that counts a finger's uncancelable touchmove as the browser's only when it is the first one and the browser could pan along it narrows current spec:AST-094 FR20, so FR20 needs updating (approving #8456 does that), or an engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request, which the author may do as an engineering owner. Whatever that decision, these still need changes:
- Land the unmerged stack first or retarget: the diff carries 18,644 authored lines against this pull request's own 1,733.
- Make the real-touch cases at 9 px, 10 px, (7, 7) and (8, 8) able to fail, since they pass with the 10 px rule removed or never reached because the list they drag over cannot scroll, and add iPadOS Safari evidence for the pen fix (
spec:AST-013FR10) or drop that claim from the changeset. - Give a finger's target the blocking listener only for an enabled recognizer (
GestureHost.ts:949), and update the README and host header, which still say thetouch-actionis set inline.
[Automated review]
| const dx = input.x - touch.startX; | ||
| const dy = input.y - touch.startY; | ||
| return Math.abs(dx) >= Math.abs(dy) | ||
| ? touch.scroll.canScroll(dx, 0) | ||
| : touch.scroll.canScroll(0, dy); |
There was a problem hiding this comment.
This narrows current spec:AST-094 FR20, which says an uncancelable touchmove MUST count as the browser taking the touch: here only a finger's first one counts, and only where the browser could pan along its main axis. #8456 proposes the amendment and has no owner decision yet; an engineering owner records the decision by approving #8456 or by commenting /approve-spec <full head SHA> on this pull request.
| // From 10 px the browser may scroll (WebKit on a phone starts at 10; | ||
| // Chromium waits longer): if it does, the scroll takes the touch and | ||
| // there is no tap; if it does not, the tap stands. | ||
| expect(scrolled && taps.includes('ended')).toBe(false); |
There was a problem hiding this comment.
These four cases drag the finger down over a list created at its top, so the list never scrolls and the 10 px and (8, 8) cases always take this branch. In Chromium all four pass with the take distance removed and with it never reached, while the FR18–FR22 row asks them to fail on "a scroll that does not take the touch at 10 px". Holding the finger after the move, scrolling the list from script as the still-finger case does, then lifting would make them fail.
| this.watchTarget( | ||
| touch.identifier, | ||
| target, | ||
| recognizers.some(claimsWhileMoving), |
There was a problem hiding this comment.
claimsWhileMoving ignores isEnabled here, while the element listener checks r.isEnabled && claimsWhileMoving(r) (line 506), and participantsAt includes disabled recognizers. A finger landing on an element whose only claiming recognizer was turned off with setEnabled(false) still gets a blocking target listener (FR19); recognizers.some(r => r.isEnabled && claimsWhileMoving(r)) fixes it.
| return; | ||
| } | ||
| } | ||
| if (moved.some(touch => (touch as StylusTouch).touchType === 'stylus')) { |
There was a problem hiding this comment.
touchType === 'stylus' exists only in iPadOS Safari, and the one case for this branch dispatches a synthetic event in jsdom. spec:AST-013 FR10 asks for evidence from that browser for the pen fix .changeset/gesture-host-fixes.md announces; an iPadOS Safari recording of a pen drawing on a claimed element while a finger scrolls elsewhere would cover it, or the claim can stay with #8072, where this code comes from.
| ids.push(id); | ||
| } | ||
| } | ||
| this.pointerIds.clear(); |
There was a problem hiding this comment.
Pens are dropped here without being remembered, so a blur-cancelled pen's pointerup finds no id (lines 1630-1633), arms no swallower, and its click lands where the browser keeps the contact alive, while a finger's is refused. Remembering cancelled pen pointer ids, as cancelledFingers does, would close it (FR28: nothing acts).
| * the derived value in TOUCH_ACTION_ATTRIBUTE, and its document or shadow | ||
| * root holds one zero-specificity rule per value in the first cascade | ||
| * layer (`reset`), so any author rule, in any layer or none, or inline | ||
| * value outranks the host's (`spec:AST-094` FR18, DEC-8) |
There was a problem hiding this comment.
This holds only where reset is registered before every author layer: an author touch-action in a layer the page registers first, or in a shadow root without Astryx's order statement, loses to this rule, and the next-frame read then records no author value. Saying so here and in the README would keep it accurate (spec:AST-080 FR3).
| recognizers leave the browser, and never more than the element's own | ||
| value allows: a caller's explicit `touch-action` (a trigger's `none`) | ||
| stays in force, as the intersection of both. It is set inline on attach | ||
| and restored on detach (opt out with `manageStyle: false`): |
There was a problem hiding this comment.
The host now marks the element under a stylesheet rule and writes inline only an intersection, so "set inline on attach" no longer describes it; the host header (GestureHost.ts:43-46) says the same.
| which reports it at once. | ||
| - `usePressTracker` rebuilds its tracker when `trackingExpansion`, | ||
| `highlightDelay`, `cancelTravel` or `quickTapFlash` change, from the next | ||
| touch, and reads its callbacks (`isBraking` included) on every touch. |
There was a problem hiding this comment.
usePressTracker builds its tracker once in useState and attaches it only on [tracker] (useGestureRecognizer.ts:136-195), so changing these four options after mount has no effect. Either rebuild the tracker when they change or say they are read once.
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision on two carried touch rules that go beyond current spec:AST-094: counting a finger's uncancelable touchmove as the browser's only when it is the first one and the browser could pan along it (FR20 says every one counts; approving #8456 would settle it), and keeping a moving finger claimed after its gesture cancels itself, which keeps cancelling moves FR19 says no unowned touch may lose (approving #8641 would settle it). An engineering owner records the decision by approving those pull requests or by commenting /approve-spec <full head SHA> here, which the author may do as an engineering owner. Whatever that decision, these still need changes:
- Land the unmerged stack first or retarget: the diff carries 18,734 authored lines against this pull request's own 1,698.
- Make the real-touch cases at 9 px, 10 px, (7, 7) and (8, 8) able to fail, since they pass with the 10 px rule removed or never reached because the list they drag over cannot scroll, and add iPadOS Safari evidence for the pen fix (
spec:AST-013FR10) or drop that claim from the changeset. - Give a finger's target the blocking listener only for an enabled recognizer (
GestureHost.ts:949), and correct the host header and README, which still say thetouch-actionis set inline, and the README's claim thatusePressTrackerrebuilds when its options change.
[Automated review]
| this.watchTarget( | ||
| touch.identifier, | ||
| target, | ||
| recognizers.some(claimsWhileMoving), |
There was a problem hiding this comment.
claimsWhileMoving ignores isEnabled here, while the element listener checks r.isEnabled && claimsWhileMoving(r) (line 506), and participantsAt includes disabled recognizers. A finger landing on an element whose only claiming recognizer was turned off with setEnabled(false) still gets a blocking target listener (spec:AST-094 FR19); recognizers.some(r => r.isEnabled && claimsWhileMoving(r)) fixes it.
| const placed = new Map(inputs.map(input => [input.id, input])); | ||
| const taken = ids.filter(id => { | ||
| const touch = this.arena.getTouch(id); | ||
| if ( | ||
| touch == null || | ||
| touch.takenByBrowser || | ||
| this.arena.isClaimed(id) || | ||
| touch.scroll?.allowsNativePan === false || | ||
| this.judgedUncancelable.has(id) | ||
| ) { | ||
| return false; | ||
| } | ||
| this.judgedUncancelable.add(id); | ||
| const input = placed.get(id); | ||
| if (input == null || touch.scroll == null) { | ||
| return true; | ||
| } | ||
| const dx = input.x - touch.startX; | ||
| const dy = input.y - touch.startY; | ||
| return Math.abs(dx) >= Math.abs(dy) | ||
| ? touch.scroll.canScroll(dx, 0) | ||
| : touch.scroll.canScroll(0, dy); |
There was a problem hiding this comment.
This narrows current spec:AST-094 FR20, which says an uncancelable touchmove MUST count as the browser taking the touch: here only a finger's first one counts, and only where the browser could pan along its main axis. #8456 proposes the amendment and has no owner decision yet; an engineering owner records the decision by approving #8456 or by commenting /approve-spec <full head SHA> on this pull request.
| } | ||
| if (next === 'cancelled' || next === 'failed') { | ||
| if (next === 'cancelled') { | ||
| this.releaseClaim(recognizer, this.movingIds); |
There was a problem hiding this comment.
Keeping the claim here means the host keeps cancelling this finger's touchmove after the only recognizer that owned it has cancelled, so the page neither scrolls nor pinch-zooms with that finger until it lifts. Current spec:AST-094 FR19 says a touchmove of a touch no recognizer owns MUST NOT be cancelled outside its three exceptions, and FR14, which #8641 cites, decides between a scroll area and the surface around it. This waits on an engineering-owner decision (approving #8641, or /approve-spec <full head SHA> here).
| // From 10 px the browser may scroll (WebKit on a phone starts at 10; | ||
| // Chromium waits longer): if it does, the scroll takes the touch and | ||
| // there is no tap; if it does not, the tap stands. | ||
| expect(scrolled && taps.includes('ended')).toBe(false); |
There was a problem hiding this comment.
These four cases drag the finger down over a list created at its top, so the list never scrolls and the 10 px and (8, 8) cases always take this branch. In Chromium all four pass with the take distance removed and with it never reached, while the current FR18–FR22 row asks them to fail on "a scroll that does not take the touch at 10 px". Holding the finger after the move, scrolling the list from script as the still-finger case does, then lifting would make them fail.
| ids.push(id); | ||
| } | ||
| } | ||
| this.pointerIds.clear(); |
There was a problem hiding this comment.
Pens are dropped here without being remembered, so a blur-cancelled pen's pointerup finds no id (lines 1630-1633), arms no swallower, and its click lands where the browser keeps the contact alive, while a finger's is refused. Remembering cancelled pen pointer ids, as cancelledFingers does, would close it (FR28: nothing acts).
| * the derived value in TOUCH_ACTION_ATTRIBUTE, and its document or shadow | ||
| * root holds one zero-specificity rule per value in the first cascade | ||
| * layer (`reset`), so any author rule, in any layer or none, or inline | ||
| * value outranks the host's (`spec:AST-094` FR18, DEC-8) |
There was a problem hiding this comment.
This holds only where reset is declared before every author layer, as Astryx's own stylesheets declare it (spec:AST-080 FR3): an author touch-action in a layer declared earlier, or in a shadow root whose own styles declare a layer before the adopted sheet, loses to this rule, and the next-frame read then records no author value. Saying so here and in the README would keep it accurate.
| - **`touch-action`.** Each element gets the intersection of what its | ||
| recognizers leave the browser, and never more than the element's own | ||
| value allows: a caller's explicit `touch-action` (a trigger's `none`) | ||
| stays in force, as the intersection of both. It is set inline on attach |
There was a problem hiding this comment.
The host now marks the element under a stylesheet rule and writes inline only an intersection, so "set inline on attach" no longer describes it; the host header (GestureHost.ts:43-46) says the same.
| it off owns that end (no consumer callback from an Effect). A handler that | ||
| wants UIKit's `.cancelled` calls `recognizer.setEnabled(false)` itself, | ||
| which reports it at once. | ||
| - `usePressTracker` rebuilds its tracker when `trackingExpansion`, |
There was a problem hiding this comment.
usePressTracker builds its tracker once in useState with these four values (useGestureRecognizer.ts:136-172) and never rebuilds it, so changing them after mount has no effect. Either rebuild the tracker when they change or say they are read once.
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision on two carried touch rules that go beyond current spec:AST-094: counting a finger's uncancelable touchmove as the browser's only when it is the first one and the browser could pan along it, where FR20 counts every one (FR20 needs updating; approving #8456 would do that), and keeping a moving finger claimed after its gesture cancels itself, which keeps cancelling moves FR19 says no unowned touch may lose (an owner needs to identify or provide the spec that governs it; approving #8641 would settle it). An engineering owner records the decision by approving those pull requests or by commenting /approve-spec <full head SHA> here, and the author may do so as an engineering owner. Whatever that decision, these still need changes:
- Land the unmerged stack first or retarget: the diff carries 18,738 authored lines against this pull request's own 1,698.
- Make the real-touch cases at 9 px, 10 px, (7, 7) and (8, 8) able to fail, since they drag over a list that cannot scroll and so pass with the 10 px rule removed, and add iPadOS Safari evidence for the pen fix (
spec:AST-013FR10) or drop that claim from the changeset. - Give a finger's target the blocking listener only for an enabled recognizer (
GestureHost.ts:949), and correct the README's "set inline",usePressTrackerand Lab-import descriptions.
[Automated review]
| if (inputs.length === 0) { | ||
| return; | ||
| } | ||
| if (!event.cancelable) { |
There was a problem hiding this comment.
This counts a finger's first uncancelable touchmove as the browser taking it only where the browser could pan along the finger's main axis, and later ones never. Current spec:AST-094 FR20 says "pointercancel, an uncancelable touchmove, or a native scroll under the touch MUST count as the browser taking the touch". #8456 proposes the amendment and has no engineering-owner decision yet, so this rule waits on that decision.
| } | ||
| if (next === 'cancelled' || next === 'failed') { | ||
| if (next === 'cancelled') { | ||
| this.releaseClaim(recognizer, this.movingIds); |
There was a problem hiding this comment.
Keeping the moving touches claimed here means the host keeps cancelling that finger's touchmove (GestureHost.ts:1085-1094) after the only recognizer that claimed it has cancelled, so the page neither scrolls nor pinch-zooms with it until the lift. Current spec:AST-094 FR19 says a touchmove of a touch no recognizer owns MUST NOT be cancelled outside three exceptions (a modal scroll lock, a control that keeps its press through a drag, a scroll-edge pan), and FR14 decides between a scroll area and the surface around it. This waits on an engineering-owner decision (#8641).
| const driver = await openTapInScroller(page, browserName); | ||
| await logTouchMoves(page); | ||
| const at = await center(page, 'row'); | ||
| await driver.play(drag(at, {x: at.x + travel.dx, y: at.y + travel.dy}, 6)); |
There was a problem hiding this comment.
The finger moves down over a list that openTapInScroller has just built at its top with overscroll-behavior: contain, so the list never scrolls and scrolled is always false: the 10 px and (8, 8) cases always take the "the tap stands" branch, and all four pass with the take distance removed or never reached. Holding the finger after its move, scrolling the list from script as the still-finger case does (lines 537-542), then lifting would make each case pin the 10 px rule that the FR18–FR22 verification row asks for.
| this.watchTarget( | ||
| touch.identifier, | ||
| target, | ||
| recognizers.some(claimsWhileMoving), |
There was a problem hiding this comment.
claimsWhileMoving ignores isEnabled here, while the element listener checks r.isEnabled && claimsWhileMoving(r) (line 506), and participantsAt includes disabled recognizers. A finger landing on an element whose only claiming recognizer is attached but disabled gets a blocking target listener whenever something else keeps the host listening, so a scroll starting there waits on a busy main thread (FR19). recognizers.some(r => r.isEnabled && claimsWhileMoving(r)), with a case, would fix it.
| ids.push(id); | ||
| } | ||
| } | ||
| this.pointerIds.clear(); |
There was a problem hiding this comment.
Fingers are remembered in cancelledFingers above, but pens are dropped here, so a blur-cancelled pen's later pointerup finds no id (lines 1630-1633) and returns before armSwallower; where the browser keeps that contact alive, its click lands, while a finger's is refused. Remembering cancelled pen pointer ids the same way would keep FR28's "nothing acts" for a pen too.
| it off owns that end (no consumer callback from an Effect). A handler that | ||
| wants UIKit's `.cancelled` calls `recognizer.setEnabled(false)` itself, | ||
| which reports it at once. | ||
| - `usePressTracker` rebuilds its tracker when `trackingExpansion`, |
There was a problem hiding this comment.
usePressTracker builds its tracker once in useState with these four values (useGestureRecognizer.ts:136-172) and never rebuilds it, so changing them after mount has no effect. Either rebuild the tracker when they change, with a test, or say here that they are read once.
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision on two carried touch rules that go beyond current spec:AST-094: counting only a finger's first uncancelable touchmove, and only where the browser could pan along the finger, as the browser taking it, where FR20 counts every one (FR20 needs updating; approving #8456 would do that), and keeping a moving finger claimed after its gesture cancels itself, which keeps cancelling moves FR19 says no unowned touch may lose (an owner needs to identify or provide the governing spec; approving #8641 would settle it). An engineering owner records the decision by approving those pull requests or by commenting /approve-spec <full head SHA> here, and the author may do so as an engineering owner. Whatever that decision, these still need changes:
- Conform the host to current FR19, FR41 and FR43, which now say gestures attach for every pointer: today nothing but an action group attaches where the primary pointer is fine, and a pen's hold takes no selection lock there.
- Land the unmerged stack first or retarget: the diff carries 18,861 authored lines against this pull request's own 1,821.
- Make the real-touch cases at 9 px, 10 px, (7, 7) and (8, 8) able to fail, since they drag over a list that cannot scroll, and add iPadOS Safari evidence for the pen fix (
spec:AST-013FR10) or drop that claim from the changeset. - Give a finger's target the blocking listener only for an enabled recognizer, and expire claimed-lift records that no
touchendconsumed.
[Automated review]
| } | ||
| const dx = input.x - touch.startX; | ||
| const dy = input.y - touch.startY; | ||
| return Math.abs(dx) >= Math.abs(dy) |
There was a problem hiding this comment.
This counts only a finger's first uncancelable touchmove, and only where the browser could pan along the finger's main axis, as the browser taking it. Current spec:AST-094 FR20 says "pointercancel, an uncancelable touchmove, or a native scroll under the touch MUST count as the browser taking the touch". #8456 proposes the amendment and has no engineering-owner decision yet, so this rule waits on that decision.
| } | ||
| if (next === 'cancelled' || next === 'failed') { | ||
| if (next === 'cancelled') { | ||
| this.releaseClaim(recognizer, this.movingIds); |
There was a problem hiding this comment.
Keeping the moving touches claimed here means the host keeps cancelling that finger's touchmove (GestureHost.ts:1133-1150) after the only recognizer that claimed it has cancelled, so the page neither scrolls nor pinch-zooms with it until the lift. Current spec:AST-094 FR19 says a touchmove of a touch no recognizer owns MUST NOT be cancelled outside its three exceptions, and FR14 does not cover a gesture that cancelled itself. This waits on an engineering-owner decision (#8641).
| } | ||
|
|
||
| private shouldListen(): boolean { | ||
| if (!this.isPrimaryPointerCoarse()) { |
There was a problem hiding this comment.
Current spec:AST-094 FR19 now says "Recognizers attach for every pointer, with FR43's exceptions", FR43 that "Gestures attach for every pointer, a mouse, a pen, and a finger", and architecture:react-component-runtime INV13 that the host attaches "with no touch-only query". This gate (with refresh at 515-517 and applyStyle at 568-570) still turns every gesture off under a fine pointer except an action group's, so it needs to follow FR43's exceptions and FR24's fine-pointer listener budget instead.
| pointerType: string, | ||
| view: Window | null | undefined, | ||
| ): boolean { | ||
| return pointerType !== 'mouse' && isCoarsePointer(view); |
There was a problem hiding this comment.
Current spec:AST-094 FR41 says "A pen's hold acts as a finger's", without a coarse-pointer condition, so a pen's hold on a hold-driven element where the primary pointer is fine should take this lock too; GestureHost.test.ts:1692 pins the old behavior.
| const driver = await openTapInScroller(page, browserName); | ||
| await logTouchMoves(page); | ||
| const at = await center(page, 'row'); | ||
| await driver.play(drag(at, {x: at.x + travel.dx, y: at.y + travel.dy}, 6)); |
There was a problem hiding this comment.
The finger moves down over a list that openTapInScroller has just built at its top with overscroll-behavior: contain, so the list never scrolls and scrolled is always false: the 10 px and (8, 8) cases always take the "the tap stands" branch, and all four pass with or without the take distance. Holding the finger after its move, scrolling the list from script as the still-finger case does (lines 537-542), then lifting would make each case pin the 10 px rule the FR18–FR22 verification row asks for.
| this.watchTarget( | ||
| touch.identifier, | ||
| target, | ||
| recognizers.some(claimsWhileMoving), |
There was a problem hiding this comment.
claimsWhileMoving ignores isEnabled here, while the element listener checks r.isEnabled && claimsWhileMoving(r) (line 517), and participantsAt includes disabled recognizers. A finger landing on an element whose only claiming recognizer is attached but disabled gets a blocking target listener whenever something else keeps the host listening (FR19). recognizers.some(r => r.isEnabled && claimsWhileMoving(r)), with a case, would fix it.
| ids.push(id); | ||
| } | ||
| } | ||
| this.pointerIds.clear(); |
There was a problem hiding this comment.
Fingers are remembered in cancelledFingers above, but pens are dropped here, so a blur-cancelled pen's later pointerup finds no id (lines 1685-1688) and returns before armSwallower; where the browser keeps that contact alive, its click lands. Remembering cancelled pen pointer ids the same way would keep FR28's "nothing acts" for a pen too.
| * place on the same target (`spec:AST-094` FR21). | ||
| */ | ||
| private cancelClaimedLift(event: TouchEvent): void { | ||
| if (!event.cancelable || event.defaultPrevented) { |
There was a problem hiding this comment.
When this touchend is uncancelable or already prevented, the matching claimedLifts record is never removed, and records are capped at eight but never expire, so a later lift within 1 px on the same target has its touchend cancelled and loses its click. Removing the matching record before this early return, or dropping records older than the stray-click window, would close it.
| it off owns that end (no consumer callback from an Effect). A handler that | ||
| wants UIKit's `.cancelled` calls `recognizer.setEnabled(false)` itself, | ||
| which reports it at once. | ||
| - `usePressTracker` rebuilds its tracker when `trackingExpansion`, |
There was a problem hiding this comment.
usePressTracker builds its tracker once in useState with these four values (useGestureRecognizer.ts:136-172) and never rebuilds it, so changing them after mount has no effect. Either rebuild the tracker when they change, with a test, or say here that they are read once.
|
On the review, in this push. It now sits on #8360, so the diff is this pull request's own:
|
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision on two carried touch rules that go beyond current spec:AST-094: counting only a finger's first uncancelable touchmove, and only where the browser could pan along it, as the browser taking the touch (FR20 needs updating for that; approving #8456 would do it), and keeping a moving finger claimed after its own gesture cancels itself (an owner needs to provide or identify the governing spec, since FR19 says an unowned touch's moves are never cancelled; approving #8641 would settle it). An engineering owner records the decision by approving those pull requests or by commenting /approve-spec <full head SHA> here, and the author may do so as an engineering owner. Whatever that decision, these still need changes:
- Land #8360 first, so this diff carries only its own 252 lines instead of 27,466.
- Conform the host to current FR19, FR41 and FR43, which say gestures attach for every pointer; today nothing but an action group or a
perEventPointerparticipant attaches under a fine primary pointer. - Add a real-touch case for the claimed-lift
touchendcancel with fingers read from pointer events, and make the 9 px, 10 px, (7, 7) and (8, 8) cases scroll the list under a held finger. - Cancel that lift wherever fingers come from pointer events, including
'auto'input with noontouchstart, and update the description, which still cites #8270 and the dropped pen fix.
[Automated review]
| // Pointer Events only: a claimed finger's touchend is not the host's own | ||
| // document listener's to cancel, so the element cancels it (FR21). | ||
| const needsEndListener = | ||
| this.inputMode === 'pointer-events' && |
There was a problem hiding this comment.
This keys the lift cancel to the explicit 'pointer-events' mode, but the host also reads fingers from their pointer events under 'auto' when the window exposes no ontouchstart (:1521-1522, :2021-2023), where Touch Events still arrive. There a claimed finger's touchend still goes through, which spec:AST-094 FR21 forbids. Keying this and the record at :2685 on the condition the move path uses would cover both.
| ids.push(id); | ||
| } | ||
| } | ||
| this.pointerIds.clear(); |
There was a problem hiding this comment.
Pointer-tracked contacts are dropped here without being remembered, so a blur-cancelled pen's later pointerup (or a finger's, where fingers come from pointer events) returns at :2679-2681 before any record or click guard, and its click lands, while a Touch Events finger's lift is refused (:2073-2076, FR28).
| // From 10 px the browser may scroll (WebKit on a phone starts at 10; | ||
| // Chromium waits longer): if it does, the scroll takes the touch and | ||
| // there is no tap; if it does not, the tap stands. | ||
| expect(scrolled && taps.includes('ended')).toBe(false); |
There was a problem hiding this comment.
This branch accepts the tap whenever the engine has not scrolled yet, and the 9 px and (7, 7) cases put no scroll under the finger, so a take distance moved either way can stay green (architecture:component-test-sufficiency INV6). Holding the finger after the move, scrolling the list from script as the still-finger case does (:529-553), then lifting would pin 10 px exactly.
| * | ||
| * ONLY WHERE THE PRIMARY POINTER IS COARSE | ||
| * | ||
| * - Recognizers attach, and the host registers listeners, only while the |
There was a problem hiding this comment.
Current spec:AST-094 FR19 and FR43 now say recognizers attach for every pointer, with FR43's exceptions, and architecture:react-component-runtime INV13 says the host has no touch-only query; this gate (and utils/selectionLock.ts:66) still follows the replaced coarse-only wording.
|
Fixed in this push:
This now sits on #8665, so the host attaches for every pointer here. FR20 (#8456) and the claim after a cancel (#8641) stay the owner's decisions. |
There was a problem hiding this comment.
This pull request is on hold for engineering-owner decisions on three touch rules beyond current spec:AST-094: this head's own edit to FR41 for claimed mouse and pen drags (FR41's sentence that a mouse keeps its released behavior needs that update approved), the first-uncancelable-touchmove rule that FR20 would need updating for (#8456), and keeping a moving finger claimed after its gesture cancels itself, for which an owner needs to provide or identify the governing spec, since FR19 says an unowned touch's moves are never cancelled (#8641). An engineering owner approves the FR41 edit by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner; the other two are decided by approving #8456 and #8641. Whatever those decisions, these still need changes:
- Land #8360 and #8665 first, so this diff carries only its own 499 lines instead of 28,871.
- Add iPadOS Safari evidence for the claimed pen's stylus
touchendcancel, or leave that branch out. - Rewrite the description for this head, naming the merged #8665 and the FR41 edit with its writing-rubric results.
[Automated review]
| (`spec:AST-017` FR59; DEC-9). A mouse or pen drag that a recognizer claims | ||
| takes no new text selection for that drag only. A claim made after the | ||
| drag has moved clears the page's selection; a press claimed at its own | ||
| pointerdown, before it moves, keeps it. An unclaimed mouse drag keeps its | ||
| released behavior. |
There was a problem hiding this comment.
These sentences add a requirement to current spec:AST-094 FR41, whose existing text says a mouse otherwise keeps its released behavior. A record change needs an engineering owner's exact-head approval (/approve-spec <full head SHA> on this pull request), and the description needs the record's ## Writing rubric results.
| if (hasStylusTouch(event)) { | ||
| // A pen's lift is its pointer's: the stylus touchend only refuses the | ||
| // compatibility mouse events and click when a gesture took the pen. | ||
| const isClaimed = | ||
| this.isPenLiftClaimed || | ||
| [...this.pointerIds.values()].some(id => this.arena.isClaimed(id)); | ||
| this.isPenLiftClaimed = false; | ||
| if (isClaimed && event.cancelable) { | ||
| event.preventDefault(); | ||
| } | ||
| } |
There was a problem hiding this comment.
touchType === 'stylus' exists only in iPadOS Safari, so this cancel reproduces only there, and its two cases are jsdom with synthetic events. spec:AST-013 FR10 asks for evidence from that browser: an iPadOS Safari recording of a claimed and an unclaimed pen lift would cover it, or the branch can wait until one exists.
| if (!event.cancelable) { | ||
| // The browser no longer asks the page about these fingers. WebKit | ||
| // stops asking when it takes the touch to pan (its pointercancel comes | ||
| // with it); Chromium also stops after one unconsumed touchmove, even | ||
| // when `touch-action` keeps it from panning the way the finger went (a | ||
| // horizontal drag under pan-y). So a finger's FIRST uncancelable move | ||
| // counts as the browser's claim only if the browser could pan along | ||
| // the finger's main axis since touch-down; later ones add nothing, and | ||
| // pointercancel or a scroll still report any real pan. | ||
| const placed = new Map(inputs.map(input => [input.id, input])); | ||
| const taken = ids.filter(id => { | ||
| const touch = this.arena.getTouch(id); | ||
| if ( | ||
| touch == null || | ||
| touch.takenByBrowser || | ||
| this.arena.isClaimed(id) || | ||
| touch.scroll?.allowsNativePan === false || | ||
| this.judgedUncancelable.has(id) | ||
| ) { | ||
| return false; | ||
| } | ||
| this.judgedUncancelable.add(id); | ||
| const input = placed.get(id); | ||
| if (input == null || touch.scroll == null) { | ||
| return true; | ||
| } | ||
| const dx = input.x - touch.startX; | ||
| const dy = input.y - touch.startY; | ||
| return Math.abs(dx) >= Math.abs(dy) | ||
| ? touch.scroll.canScroll(dx, 0) | ||
| : touch.scroll.canScroll(0, dy); | ||
| }); | ||
| if (taken.length > 0) { | ||
| this.browserTook(taken, time); | ||
| } | ||
| } |
There was a problem hiding this comment.
Counting only a finger's first uncancelable touchmove, and only where the browser could pan along it, narrows current FR20 ("an uncancelable touchmove ... MUST count as the browser taking the touch"). It waits on an engineering-owner decision in #8456.
| if (next === 'cancelled') { | ||
| this.releaseClaim(recognizer, this.movingIds); |
There was a problem hiding this comment.
Keeping a moving finger claimed after its only claimer cancels itself keeps its touchmoves cancelled, where FR19 says a touch no recognizer owns MUST NOT have its moves cancelled outside three exceptions. FR14 decides who gets a drag that starts in a scroll area, not this case; it waits on an owner decision in #8641.
| * root holds one zero-specificity rule per value in the first cascade | ||
| * layer (`reset`), so any author rule, in any layer or none, or inline | ||
| * value outranks the host's (`spec:AST-094` FR18, DEC-8); a root's rules |
There was a problem hiding this comment.
This holds only when reset is the first declared layer. An app that declares its own layers before Astryx's stylesheet loses its touch-action to the derived value, which can be wider (FR18). Stating that precondition here and in the README, or placing the mark where an earlier author layer still wins, would close it.
Three recognizers beside main's tap, long press and pan, each reading its own UIKit table group: a swipe recognized from the path between events, a pinch whose velocity settles, and a screen-edge pan from the leading or trailing edge. The edge pan takes the horizontal axis only on a view that lies within that edge's band. The sides its edges resolve to under the view's writing direction and the edge zone are measured once, together, for touch-action and the touch-down decision alike, and again when the host reports a layout change; and it is a finger's only (a mouse, a trackpad or a pen never starts it). meta#8061's change, rebased onto main as one commit; its rotation and hover recognizers and its top and bottom edges, which no component uses yet (and rotation's threshold is unmeasured), land with their first users. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
… the gesture host The browser negotiation and the gesture host on top of the recognizers: touch-action derived from what is attached and never wider than the element's own (spec:AST-094 FR18, DEC-8), a native scroll probe that follows a slotted node into its shadow tree, the tap haptic, and the host that claims a finger, suppresses the click a claimed touch would send, and hands a touch to the browser when it takes over. The host serves a mouse, a pen and a finger alike (FR19, FR43). Where the primary pointer is fine, an element that uses a gesture keeps one passive pointerdown listener and the page has none until a press, whose listeners go once its contacts end (FR24); a right click, a pen's barrel button and a ctrl-click on macOS start no press. It gives a screen-edge pan its view's box and, through one ResizeObserver and no listener, reports a resize of that box or of the viewport, so the pan's edge-zone answer and the touch-action derived from it follow the layout (FR18, FR19, FR24, DEC-22). Recognizers report their kind, which the host and the touch-action rules read instead of checking a class (architecture:react-component-runtime INV13). meta#8071's and meta#8072's changes, with meta#8665's serving of every pointer, rebased onto main after meta#8061 as one commit; the host's hover routing lands with the hover recognizer's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…mator Core's internal touch foundation on the gesture recognizers (meta#8061) and the gesture host (meta#8071 + meta#8072): UIControl press tracking with its press profiles and hooks, built by createPressTracker; and the animator, which starts a motion from rest at its commit, holds the value on screen inline while a curve runs, and lets a call wait for a presented frame. It absorbs meta#8073 (closed: the press tracking this rests on) and meta#8340 (the animator's start and hold fixes), with the held dip's load fixes, and keeps meta#8641's rule: a gesture that cancels itself while its finger moves keeps that finger until it lifts. A gesture that ends on an unmount or a replaced node calls no listener from the effect. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
meta#8360's change on the internal touch foundation: one click guard shared by the menu press and the gesture host (its click listener stays once the menu press arms it, as its record has it, and lasts only while armed for the gesture host, FR24), the host as the one touch owner (finger scroll, page holds, attachments, the press tracker hook and a held control's kept touch, the long press's single source, presented hit testing on iOS WebKit), and the gesture browser proof in Chromium and WebKit with the README. It carries meta#8074, meta#8270, meta#8293, meta#8315, meta#8320 and meta#8356, and folds in meta#8358 (the long press's tolerance comment and an unused import). Rebased onto section 2 as one commit; the touch-action rule now comes with meta#8071, and meta#8072's claimed-touchend fix, which this line never had, comes back with meta#8309. The animator's busy proof is judged against the harness's compositor-only control (meta#8906), with a main-thread mutant as its negative, and the spring's curve is placed against the control's own clock, so the late frames a loaded runner sends fail nothing. With the host serving every pointer (meta#8071), this change's own pieces follow it: a press tracker takes a mouse press as a press, painting at once and braking no scroll (FR11); a finger's or a pen's hold that does something keeps text unselected under any primary pointer, while a mouse keeps its released behavior (FR41); a claimed pen's stylus touchend is cancelled as a finger's (FR21); and `perEventPointer`, from before FR43, changes nothing. These come from meta#8665, which keeps only the undecided rule for text selection during a mouse or pen drag. The hover recognizer and the host's hover routing land with hover's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
The scroll lock no longer pins <body>, which reset the page's offset to 0 (the page jumped and Safari's toolbar came back). It holds `overflow: hidden` on the root and, through the gesture host's page hold, cancels only a one-finger pan that nothing inside the overlay takes: a scroller, a range input, a pinch or a zoomed page's pan still goes through. The root's scroll styles become claim stacks (rootScrollStyles), so a lock and a pull to refresh compose. The top-layer check asks CSS.supports for `:modal` and `:popover-open` instead of catching errors from `matches`. The touch module bundle test names useScrollLock and the imports that reach it as carrying the page hold.
…come from pointer events Where a finger comes from its pointer events, a claimed finger's touchend is cancelled too (spec:AST-094 FR21), on the host's shared blocking handler and only for an enabled recognizer; a lift's claim is read after the lift, so a hold that claims with it cancels its touchend; a blur-cancelled pen's lift is refused. A second finger fails a one-finger pan wherever fingers come from, and the arena cancels a moving recognizer the browser takes. meta#8309's and meta#8336's changes, rebased onto meta#8098 as one commit. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
There was a problem hiding this comment.
This pull request is on hold for engineering-owner decisions on two touch rules it carries beyond current spec:AST-094. The first keeps a moving finger claimed after its gesture is cancelled during that move: an owner needs to provide or identify the spec that governs it, since FR19 says an unowned touch's moves are never cancelled outside its three exceptions (#8641). The second counts only a finger's first uncancelable touchmove, and only where the browser could pan along it, which needs FR20 updated (#8456). An engineering owner decides by approving #8641 and #8456, or by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner); whatever they decide, these still need changes:
- Land #8360 and #8098 first, so this diff carries only its own 713 lines instead of 31,161.
- Cancel the claimed lift under a fine primary pointer too: the
touchendlistener is added only when the primary pointer is coarse (GestureHost.ts:1016-1018), so where fingers come from pointer events under a fine primary pointer (a touchscreen laptop), a claimed finger still sendsmousedownandmouseup. - Add iPadOS Safari evidence for the stylus
touchendcancel, or leave that branch out.
[Automated review]
| // (FR21). | ||
| const needsEndListener = | ||
| this.readsFingersFromPointerEvents() && | ||
| this.isCoarse && |
There was a problem hiding this comment.
With this.isCoarse here, a touchscreen laptop (fine primary pointer) whose fingers come from pointer events gets no touchend listener. The record pushed at pointerup (:2597-2608) is never consumed, and the claimed finger's mousedown and mouseup still reach the page; the document handler returns at :2063-2065. FR21 and FR43 cover this pointer, and the touchmove listener below has no coarse condition. Dropping the condition, with a fine-pointer case, would close it.
| isSamePlace(lift, { | ||
| x: touch.clientX, | ||
| y: touch.clientY, | ||
| target: touch.target, |
There was a problem hiding this comment.
At this element listener, touch.target is retargeted to the shadow host when the finger landed inside an open shadow root below the element. The record holds the innermost node from landedOn(pointerup), so the two never match there. landedOn(end, touch.target), as onTouchStart uses at :1776, would compare like with like.
| // The browser may keep a pen or a pointer-read finger down: its lift must | ||
| // not click (FR28). | ||
| for (const pointerId of this.pointerIds.keys()) { | ||
| this.cancelledPointers.add(pointerId); |
There was a problem hiding this comment.
This adds mouse pointer ids too (pointerIds holds every primary press, :2487), so a mouse press the blur cancelled loses its click at the lift. The comments here and at :422-426 name only pens and pointer-read fingers. An id whose contact ends in pointercancel is also never removed.
| } | ||
| if (next === 'cancelled' || next === 'failed') { | ||
| if (next === 'cancelled') { | ||
| this.releaseClaim(recognizer, this.movingIds); |
There was a problem hiding this comment.
Keeping the moving fingers claimed on every cancel inside a move keeps the host cancelling their touchmoves after no live recognizer owns them. Current spec:AST-094 FR19 says an unowned touch's moves MUST NOT be cancelled outside three exceptions. This waits on an engineering-owner decision (#8641).
| if (inputs.length === 0) { | ||
| return; | ||
| } | ||
| if (!event.cancelable) { |
There was a problem hiding this comment.
Counting only a finger's first uncancelable touchmove, and only where the browser could pan along its main axis, narrows current spec:AST-094 FR20 ("an uncancelable touchmove ... MUST count as the browser taking the touch"). This waits on an engineering-owner decision (#8456).
| readonly target: unknown; | ||
| } | ||
|
|
||
| /** A touchstart and a pointerdown describe one finger: same target, same spot. */ |
There was a problem hiding this comment.
This doc comment belonged to isSamePlace; with CLAIMED_LIFT_MS inserted below it, it now documents nothing. Moving the constant above it keeps both.
Section 3 of the merge plan: #8360 → #8098 → #8309 → #8763 → #8342. One commit on #8098 holding only this change. It absorbs #8336 (a second finger on a one-finger pan, and the arena's cancel), which closes with a link here; the commit credits its authors.
User impact
Where the host reads a finger from its pointer events (
inputMode: 'pointer-events', or a window with noontouchstart, as after touch emulation turns on), a gesture that claimed the finger cancelled its moves but not its lift: the engine still sentmousedownandmouseupto what was under the finger. A pen the window's blur cancelled clicked at its later lift. A second finger did not fail a one-finger pan there. And a disabled recognizer that would claim a touch still gave its target a blocking listener, so a scroll waited on the main thread for nothing.Problem and solution fit
The fix reads the lift's own outcome, as the Touch Events path does: it notes where a claimed lift landed, cancels the matching
touchendon the host's shared blocking handler, and lets the note expire after 500 ms. A lift's claim is read after the lift, so a hold whose time comes with the lift cancels itstouchendtoo. A contact the lost focus cancelled is remembered by pointer id until its lift, which then clicks nothing. A target gets a blocking listener only for an enabled participant. From #8336, a second finger fails a one-finger pan wherever fingers come from, and the arena cancels a moving recognizer the browser takes.Expected behavior and authority
spec:AST-094(current): FR19 (only an owner claims), FR20 (the browser taking a touch), FR21 (no click after a gesture that took the touch) and FR28 (a lost focus ends the gesture). #8360 lists the claimedtouchendunder pointer-event fingers as its known gap until this change.Smallest restoration
Host and arena only: the lift note, the shared handler's
touchend, the blur-cancelled pen and the enabled check inGestureHost.ts, and the moving recognizer's cancel inGestureArena.ts. Cancelling everytouchendon an element with a recognizer would cancel taps nothing took; deciding at thepointerupwould miss a hold that claims with its lift.Evidence
mousedownandmouseupto its element (real touch in Chromium).'auto'input with noontouchstart, the blur-cancelled pen, the disabled recognizer's listeners, a second finger on a one-finger pan), and a no-expiry mutant fails the expiry case; browser cases at 9 px, 10 px, (7, 7) and (8, 8) hold the finger while a list scrolls under it.Scope
Testing
Local gates at 5fcc1c5, one commit on #8098's 70e82f3 holding only this change (7 files, +687/−26):
pnpm test:ui(core UI): 688 files, 16,852 tests passed.pnpm exec vitest run --project node: 806 files passed; the only failures are main's own story-tree and release-cache reds, tracked in test(storybook): run the story-tree check where Storybook is built, and bring the tree back to its shape #8509 and fix(cli): a crashed run's release install is recognized by the Node it ran on, whatever its name #8520.playwright.interaction.config.ts, a phone in Chromium and WebKit): Chromium 54 passed; WebKit 45 passed and 9 skipped where its driver has no native scroll, fling, pinch-zoom or pen.tsc --project packages/core/tsconfig.json --noEmit, strict ESLint and Prettier on every changed file: clean.pnpm check:repo: clean.