Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
meta-cla
Bot
added
the
CLA Signed
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: against main, this pull request's diff includes all of #8665 (29,050 of its 29,164 authored lines), so it can't be reviewed or merged as the 642-line createPressTracker change it describes. Please land #8665 first and rebase this branch onto main, so the diff is the factory change alone. The factory itself follows architecture:react-component-runtime INV13 and keeps the tracker's behavior; while rebasing, please also fix the changeset and gestures README sentence that calls every touch module outside the recognizer family a factory, since VelocityTracker is still a class (INV13 lists it as a known gap).
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: press tracking is built by `createPressTracker(options)`, and `PressTracker` is the interface of what it returns, as every touch module outside the recognizer family is built from a factory. `usePressTracker` and its callers do not change. Not exported from the package entry point. |
There was a problem hiding this comment.
VelocityTracker is still a class outside the recognizer family (packages/core/src/gestures/VelocityTracker.ts:59), the gap architecture:react-component-runtime INV13 records, so this release note overstates the rule. Suggest "…as every touch module outside the recognizer family is, apart from VelocityTracker", or dropping the clause; the same sentence is in packages/core/src/gestures/README.md:180-181.
There was a problem hiding this comment.
Incremental review of d714d09..3e4d7c0; prior findings carried: spec_authority, authored_loc, context, user_impact, api, evidence_by_spec_claim, concrete_defects, disposition
Requesting changes: the changeset and gestures README now describe the factory rule accurately, naming VelocityTracker as the one class left outside the recognizer family, so that earlier note is resolved. Against main, the diff still includes all of #8665 (29,050 of its 29,165 authored lines), so it can't be reviewed or merged as the 643-line createPressTracker change it describes. Please land #8665 first and rebase this branch onto main, so the diff is the factory change alone; nothing else blocks the factory change.
[Automated review]
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved, and the diff also needs to be split. Against main, it adds a sentence to spec:AST-094 FR41 that the current record does not state (“A mouse or pen drag that a recognizer claims suppresses text selection for that drag only; an unclaimed mouse drag keeps today's behavior.”); an engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Separately, 28,082 of the diff's 28,197 authored lines are #8665's open stack, so please land #8665 first and rebase this branch onto main, leaving the 647-line createPressTracker change, which follows architecture:react-component-runtime INV13 and shows no defect.
[Automated review]
| (`spec:AST-017` FR59; DEC-9). A mouse or pen drag that a recognizer claims | ||
| suppresses text selection for that drag only; an unclaimed mouse drag keeps | ||
| today's behavior. |
There was a problem hiding this comment.
This sentence is new normative text for FR41 that main does not state: today FR41 says a mouse “otherwise keeps its released behavior”. It arrives here from #8665's stack, and an engineering owner needs to approve or reject it; approval is recorded by commenting /approve-spec <full head SHA> on the pull request.
meta#8665's review fixes: an armed press under a fine pointer brings the claiming listeners, keeps a kept press's finger and goes when the page hides; a press claimed at its own pointerdown keeps the page's selection; FR41 says what a claimed drag does with the selection. One conflict, in finePointerListeners.test.ts's imports: this pull request's createPressTracker with meta#8665's new recognizer imports, and meta#8665's new kept-press case builds its tracker with createPressTracker, since the class is gone here.
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved, and the diff also needs to be split. Against main it adds text to spec:AST-094 FR41 that the current record does not state (“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.”), with a matching verification row; an engineering owner records approval by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Separately, 28,320 of the diff's 28,445 authored lines are #8665's open stack, so please land #8665 first and rebase this branch onto main, leaving the 667-line createPressTracker change, which follows architecture:react-component-runtime INV13 and shows no defect.
[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.
This is new normative FR41 text that main does not state: today FR41 says a mouse “otherwise keeps its released behavior”. It arrives from #8665's stack, rewritten there since the earlier review, with the matching verification row at line 663; an engineering owner needs to approve or reject it, recorded by commenting /approve-spec <full head SHA> on the pull request.
github-actions
Bot
added
the
needs:design-review
|
Fixed in 3cdb556: the untracked-touch path passes |
github-actions
Bot
added
the
needs:spec-owner-review
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved, and the diff also needs to shrink to its own commit. Against main it rewrites component:BottomSheetSwitcher FR5 and component:Drawer FR5 (pull-to-refresh and the page's bounce stop behind a modal) and component:MobileNav FR3 (the clip moves to the page's scroller), which the current records do not state; an engineering owner records approval by commenting /approve-spec <full head SHA> on the pull request, and the author may do so as an engineering owner. Separately, nearly all of the diff (19,148 of its per-commit lines) comes from #8360, #8098 and #8309, so please land those and rebase so this shows only the modifier-keys and press-clocks change, which looks correct and is covered by its new tests.
[Automated review]
| | FR3 | During a handoff, the entering sheet is above the retained sheet. A taller retained sheet aligns down to a shorter entering sheet, and the retained sheet fades only after required transform motion completes. Under reduced motion the handoff follows `design:reduced-motion`: no transform motion runs, and the sheets cross-fade (DR2) or change at once (DR3) in the same order. | Current source and transition tests; `design:reduced-motion` DR2, DR3 | | ||
| | FR4 | Closing retains the outgoing sheet through its exit, then closes the dialog. A modal flow restores focus to the element captured when that modal flow opened. | Current source and focus tests | | ||
| | FR5 | `hasScrim=true` uses `showModal()`, a native backdrop, and native focus containment, and locks page scroll: the page does not scroll while a modal flow is presented; the lock is held continuously across a handoff, never released and re-acquired between steps; it still holds beneath a `Dialog` opened from a step; it is released after the flow's exit (DEC-1). `hasScrim=false` uses `show()` without a backdrop or scroll lock and leaves the page interactive. In a non-modal flow the sheet dialogs order themselves with local values inside the switcher's isolated flow wrapper (DEC-1); the page-level value that wrapper carries remains a `spec:AST-027` conformance gap. | DEC-1; `spec:AST-027` FR2, FR4, FR8 | | ||
| | FR5 | `hasScrim=true` uses `showModal()`, a native backdrop, and native focus containment, and locks page scroll: the page does not scroll while a modal flow is presented, and pull-to-refresh and the page's bounce stop behind it (`overscroll-behavior-y: none`), since the page is held under a modal presentation (`spec:AST-094` FR19), as behind a UIKit modal presentation; the lock is held continuously across a handoff, never released and re-acquired between steps; it still holds beneath a `Dialog` opened from a step; it is released after the flow's exit (DEC-1). `hasScrim=false` uses `show()` without a backdrop or scroll lock and leaves the page interactive. In a non-modal flow the sheet dialogs order themselves with local values inside the switcher's isolated flow wrapper (DEC-1); the page-level value that wrapper carries remains a `spec:AST-027` conformance gap. | DEC-1; `spec:AST-027` FR2, FR4, FR8 | |
There was a problem hiding this comment.
This adds a requirement the current FR5 does not state: pull-to-refresh and the page's bounce stop behind a modal flow (overscroll-behavior-y: none). spec:AST-094 FR19, cited here, only lets a modal scroll lock cancel an unowned one-finger pan. An engineering owner approves or rejects it by commenting /approve-spec <full head SHA> on the pull request.
| | FR3 | The desktop inline-size budget MUST accept pixel numbers or CSS lengths and remain bounded by the viewport. At or below 640px, page-reveal mode MUST preserve 56px and use `width` as a cap; full-mobile mode MUST override `width` with `100dvw`. | Current docs, styles, and width tests | | ||
| | FR4 | Visibility through Drawer-owned behavior MUST follow `isOpen`. Uncanceled Escape routed by the shared stack and uncanceled modal-backdrop activation request `false` when `purpose` allows (FR15), and a `DrawerHeader` close action requests `false` through the callback it is given; Drawer prevents its handled native `cancel` event from closing the dialog directly. During controlled exit, Drawer keeps its host and content region mounted while rendering the caller's current `children`. | Current prop contract and lifecycle tests | | ||
| | FR5 | At open time, `hasScrim={true}` MUST select native `showModal()` with `aria-modal`, a focus trap, body scroll locking, and a visible backdrop; `hasScrim={false}` MUST select a manual Popover API host that leaves the page behind interactive. Changing `hasScrim` while open is not a supported native-mode transition. | Current source, docs, and initial-mode tests | | ||
| | FR5 | At open time, `hasScrim={true}` MUST select native `showModal()` with `aria-modal`, a focus trap, page scroll locking, and a visible backdrop; under the lock, pull-to-refresh and the page's bounce stop (`overscroll-behavior-y: none`), since the page is held under a modal presentation (`spec:AST-094` FR19), as behind a UIKit modal presentation; `hasScrim={false}` MUST select a manual Popover API host that leaves the page behind interactive. Changing `hasScrim` while open is not a supported native-mode transition. | Current source, docs, and initial-mode tests | |
There was a problem hiding this comment.
Same new requirement as BottomSheetSwitcher FR5: the overscroll stop under the lock is not in the current FR5, which says "body scroll locking". It needs an engineering owner's approval, recorded with /approve-spec <full head SHA>.
| | FR1 | MobileNav renders one native `<dialog>` Navigation overlay containing a Drawer with a stable Header, Close button, and scrollable Content region. The overlay carries `mobile-nav` with the resolved `side`; Drawer, Header, and Content carry no MobileNav public target. | Current source, docs, and tests | | ||
| | FR2 | `isOpen` and `onOpenChange` win when given; otherwise the drawer reads and changes the enclosing AppShell's drawer state. The dialog uses AppShell's drawer id when there is one, else its own; a caller `id` replaces either (Current-state impact). | Current source and tests | | ||
| | FR3 | Opening calls `showModal()`, and records the focused element as the opener; where focus lands inside the open drawer is uncontracted here. While open, the page's root clips its overflow and keeps its scrollbar gutter; closing releases both. | Current source and tests | | ||
| | FR3 | Opening calls `showModal()`, and records the focused element as the opener; where focus lands inside the open drawer is uncontracted here. While open, the page's scroller clips its overflow and keeps its scrollbar gutter: the root, or `<body>` when `<body>` sets its own overflow and the root's stays `visible`; closing releases both. | Current source and tests | |
There was a problem hiding this comment.
This moves the clip from the page's root to the page's scroller (<body> when <body> sets its own overflow and the root stays visible), which the current FR3 does not state; it needs an engineering owner's approval, recorded with /approve-spec <full head SHA>.
…king ships only the press clocks A gesture touch carries the modifier keys its press began with, so a group can leave a Ctrl-, Cmd- or Shift-click on a link to the browser. Press tracking reads only the timing table's press clocks, with a bundle guard that keeps the host's module graph small (architecture:component-style-authoring INV6), and the README documents the recognizers' kind and the press tracker factory. meta#8763's and meta#8797's changes, rebased onto meta#8309 as one commit; its press tracker factory and recognizer kind now come with meta#8076 and meta#8071.
3cdb556 to
dbe81cd
Compare
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved, and the diff also needs to shrink to its own commit. Against main it adds requirements the current records do not state to spec:AST-094 FR11 and FR19, architecture:react-component-runtime INV13, component:BottomSheetSwitcher FR5, component:Drawer FR5 and component:MobileNav FR3, all from the stack beneath this branch; an engineering owner records approval by commenting /approve-spec <full head SHA> on the pull request, and the author may do so as an engineering owner. Separately, 19,430 of the diff's 19,597 authored lines belong to #8360, #8098 and #8309, so those should land first and this branch be rebased onto main, leaving its own 173-line commit, which reads correctly; in that stack, useFingerScroll should call the shared useAttachment instead of copying its node-following.
[Automated review]
| `family:action-groups` member tracks activates at the lift (FR26), and a toggle | ||
| under `spec:AST-020` FR9's slide-off exception (Switch, Checkbox, | ||
| CheckboxListItem) may also complete on a lift after the finger slid off. | ||
| CheckboxListItem) may also complete on a lift after the finger slid off. Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on moving content goes to what was drawn under the finger at touch-down, and a tap there (no recognizer claimed it, the browser did not take it, and it stayed within 45 pt) MUST activate it: the browser's own click for that touch is cancelled or swallowed, and that element receives one click, an editable focused first. |
There was a problem hiding this comment.
New normative text from #8360's stack: a MUST that a tap on compositor-moving content activates what was drawn under the finger with one click, the browser's own click cancelled or swallowed. Current FR11 says a control's activation "stays the browser's click" with two exceptions, so this adds a third. It needs an engineering owner's approval, recorded with /approve-spec <full head SHA>.
| element whose recognizers decide only at the lift keeps its `touchmove` | ||
| listener passive, and its `touchend` listener stays cancelable so the | ||
| gesture can cancel the click (FR21). Known limit: a native scroll that starts on an element with a | ||
| gesture can cancel the click (FR21). While an element that holds, or sits inside, an element with a recognizer that claims moving touches moves on the compositor (iOS WebKit), the document MAY carry a non-passive `touchmove` listener; it cancels only the moves of a touch a recognizer owns under this rule and comes off when the motion ends. While it is on, a native scroll that starts anywhere on the page waits for a free main thread. Known limit: a native scroll that starts on an element with a |
There was a problem hiding this comment.
New MAY from #8360's stack: a document-level non-passive touchmove listener while such content moves, during which every native scroll on the page waits for the main thread. Current FR19 says "Only an element with a recognizer that must claim mid-gesture registers that blocking listener". It needs an engineering owner's approval, recorded with /approve-spec <full head SHA>.
| small glue in its own build over these modules. The gesture host attaches for | ||
| every pointer, with no touch-only query; under a fine pointer only a component that | ||
| uses a gesture listens (`spec:AST-094` FR19, FR24, FR43). The modules are not public exports and are not | ||
| uses a gesture listens (`spec:AST-094` FR19, FR24, FR43). The gesture host, press feedback, page hold, finger-scroll watch and click guard hear document-wide input through one capture-phase listener per event type on the window, present while a subscriber needs it. They run in subscription order, before any document or element listener; it is passive unless a subscriber cancels the event. The modules are not public exports and are not |
There was a problem hiding this comment.
New INV13 text from #8360's stack: the touch modules hear document-wide input through one capture-phase window listener per event type, in subscription order. Current INV13 does not state it. It needs an engineering owner's approval, recorded with /approve-spec <full head SHA>.
…king ships only the press clocks A gesture touch carries the modifier keys its press began with, so a group can leave a Ctrl-, Cmd- or Shift-click on a link to the browser. Press tracking reads only the timing table's press clocks, with a bundle guard that keeps the host's module graph small (architecture:component-style-authoring INV6), and the README documents the recognizers' kind and the press tracker factory. meta#8763's and meta#8797's changes, rebased onto meta#8309 as one commit; its press tracker factory and recognizer kind now come with meta#8076 and meta#8071.
dbe81cd to
38070ea
Compare
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved, and the diff also needs to shrink to its own commit. Against main it adds requirements the current records do not state to spec:AST-094 FR11 and FR19, architecture:react-component-runtime INV13, component:BottomSheetSwitcher FR5, component:Drawer FR5 and component:MobileNav FR3, all from the stack beneath this branch; an engineering owner records approval by commenting /approve-spec <full head SHA> on the pull request, and the author may do so as an engineering owner. Separately, 19,586 of the diff's 19,753 authored lines belong to #8360, #8098 and #8309, so those should land first and this branch be rebased onto main, leaving its own 173-line commit, which reads correctly and whose new tests fail before it and pass with it. In that stack, useFingerScroll should call the shared useAttachment instead of copying its node-following.
[Automated review]
| const attachedRef = useRef<{node: Node; detach: () => void} | null>(null); | ||
| // Follows the node on every commit: a ref that now points at another | ||
| // element moves the watch to it. | ||
| useIsomorphicLayoutEffect(() => { | ||
| const node = isDocument(scroller) ? scroller : scroller.current; | ||
| if (attachedRef.current?.node === node) { | ||
| return; | ||
| } | ||
| attachedRef.current?.detach(); | ||
| attachedRef.current = | ||
| node == null | ||
| ? null | ||
| : { | ||
| node, | ||
| detach: watchFingerScrolls( | ||
| getGestureHost(isDocument(node) ? node : node.ownerDocument), | ||
| node, | ||
| ), | ||
| }; | ||
| }); | ||
| // Synchronizes with the gesture host's finger-scroll watch: unmounting | ||
| // releases the watch this hook attached. | ||
| useIsomorphicLayoutEffect( | ||
| () => () => { | ||
| attachedRef.current?.detach(); | ||
| attachedRef.current = null; | ||
| }, | ||
| [], | ||
| ); |
There was a problem hiding this comment.
These two layout Effects re-implement the node-following useAttachment already owns (gestures/useAttachment.ts:73-98: follow the node behind a ref after each commit, release a replaced node, release at unmount), which useGestureRecognizer and usePressTracker call. architecture:react-component-runtime INV16 asks for a shared concern to be extended in its owner and called once. Suggest letting useAttachment take a document as well as an element ref and calling it here once, so a fix to replaced-node release reaches the finger-scroll watch too.
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), its cases in the gesture browser proof (Chromium and WebKit) and 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 main after section 2 as one commit; the touch-action rule came with meta#8071. Where fingers come from their pointer events, a claimed finger's touchend is cancelled from its element (FR21), under a fine primary pointer wherever the browser sends Touch Events and inside an open shadow root too; at most eight such lifts wait for their touchend, and each goes once taken or after 500 ms, kept across the host's uninstall, which can run in the microtask after the pointerup, before the touchend that comes later in the same task (FR24). An armed press's page listeners go once the arena has decided its last touch, a tap that waits on a double tap included, and after its last detach the host holds no listener, note, frame or timer (FR24). 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); 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>
…finger landed On iOS WebKit the host resolves moving content at touch-down and asks the engine what is under the finger. When a busy main thread hands it the touch later, the content has moved on: the host now asks at the point where what was under the finger at the touch's timestamp is drawn, so a tap on a moving sheet's Done still clicks Done (spec:AST-094 FR11). A presented source reports its offset at a given time for this. The presented browser proof drives the sheet with Core's animator and a critically damped spring, up into place and down off the screen over the scrim: a tap on the drawn Done clicks it once, and a drag on the drawn sheet catches it where it is drawn, in both directions.
…the finger (FR11, FR19); INV13 names the shared input listener Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on that content goes to what was drawn under the finger at touch-down, and a tap there activates that element with one click while the browser's own click is cancelled or swallowed (FR11). While such content around a recognizer that claims moving touches is moving, the document may carry a non-passive touchmove listener that cancels only owned moves and comes off when the motion ends; while it is on, a native scroll waits for a free main thread (FR19). The gesture host, press feedback, page hold, finger-scroll watch and click guard hear document-wide input through one capture-phase listener per event type on the window (architecture:react-component-runtime INV13).
…view fixes A touch the host handles late now also leaves alone content that slid under the finger after it landed: the touch goes to what was beneath it then (spec:AST-094 FR11). The input-stream changeset says what moved: the gesture host's listeners, and the press feedback's scroll and visibility listeners, since its pointer listeners were already on the window's capture phase. The coarse-pointer gate reads TOUCH_QUERY from touchQueries.ts instead of spelling it again. The WebKit interaction job's scope covers every module the gesture host now bundles: hooks/isRtlElement and utils' flatTree, hapticSwitch, inputStream, clickGuard, selectionLock and coarsePointer. The scope test's example of a util outside it moves from clickGuard, which the gesture fixture now bundles, to focusReturn. The presented proof checks that the sheet moves on a spring (its speed changes along the animator's curve) and no longer compares the drawn box with the animator's offset: under a 3-CPU cap the two disagree by 3 to 27 px in about half the frames whatever reference is used, while the tap and drag cases go through the host's real path.
…e origin most frames agree with Under the CI lane's WebKit profile (four CPUs, one worker), "a spring keeps advancing every frame through a 1 s main-thread block" failed in two ways, each about once in a few hundred runs, and a failing run's frames show each cause: - The spring drifted 138.6 ms against the control. The control's clock took its origin from the first usable frame alone, reading that frame's direction from the next usable one: the frame sat at x 210 on its way out, the next usable one at x 210 after the turn, and the tie read as "back". The clock now lives in controlClock.ts and takes the origin most frames agree with. On that run's own frames it reads 6.5 ms (bound 12), and its unit case for that layout fails on the old reading (150.7 ms astray). - The spring "moved on 61 of the 62 frames the control moved". At the block's end it was in its tail, under two of the cast's 1 px steps a frame, where a frame delivered early shows no step and the next catches up two. The harness now reads such a tail at its own pace (busyWindowProof), and the proof judges the whole block: on that run's own frames the earlier proof fails it and this one passes. No bound changes.
… (architecture:public-component-api INV23) Main's lint rule now keeps every private data attribute in its owner module. This change had moved `data-astryx-haptic-switch` from the haptics module to utils/hapticSwitch.ts, so the click guard could skip the hidden switch without importing haptics (architecture:react-component-runtime INV13). The name now lives in privateAttributes/hapticSwitch.stylex.ts, modelled on segmentDisabled.stylex.ts: it exports markHapticSwitch, which the haptics module uses on its hidden switch, and isHapticSwitch, which the click guard reads. utils/hapticSwitch.ts and the deprecated gestures/hapticSwitch.ts re-export go, the haptics module's baseline entry goes with its raw name, and the WebKit interaction job's scope covers the new module. No behavior changes.
…y the gesture host and the selection lock The selection lock and the gesture host each kept a copy of two helpers: the iOS WebKit engine check, and the walk that asks whether a node is inside an element along the flat tree. The engine check now lives in utils/touchQueries.ts, the React-free half of touchPresentation.ts, as IOS_SUPPORTS_QUERY with one isIosWebKit(view) reader; touchPresentation.ts re-exports the query, so its export is unchanged. The walk is containsAlongFlatTree in utils/flatTree.ts, beside parentAcrossShadow. Unit cases cover both in their new homes. No behavior changes.
No Core or Lab component attaches a gesture recognizer yet, so the input-stream changeset's fix entry told consumers about a change they cannot meet. It now keeps `[fix]` for what ships, touch press feedback hearing scrolls and page hides on the window's capture phase, and a new `[chore] Internal:` changeset carries the recognizers' shared stream and its passive-unless-cancelled listeners.
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 page's scroller 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, and MobileNav's clip moves onto them. 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. The records state the clip on the page's scroller (MobileNav FR3) and that pull-to-refresh and the page's bounce stop behind a modal overlay (BottomSheetSwitcher FR5, Drawer FR5). AlertDialog's layout tests stub the page hold, whose pointer read belongs to the lock, so their guard still sees only the layout's own media queries.
…h rule of the Touch Events path Where a finger comes from its pointer events, the host does what its Touch Events path does (spec:AST-094 FR10, FR19, FR28): the lift of a pen or a pointer-read finger the window's blur cancelled is refused, while a mouse's keeps its click, and a contact that ends in pointercancel is forgotten; a second finger fails an unclaimed one-finger pan wherever it lands; and a target gets a blocking listener only for an enabled participant. The arena cancels a moving recognizer the browser takes. A refused lift is noted as a claimed lift is, and the pointers a blur cancelled go, as the host's other notes do, after its last detach (FR24). The claimed lift's touchend cancel these build on comes with meta#8360. meta#8309's and meta#8336's changes, rebased onto meta#8098 as one commit. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…king ships only the press clocks A gesture touch carries the modifier keys its press began with, so a group can leave a Ctrl-, Cmd- or Shift-click on a link to the browser. Press tracking reads only the timing table's press clocks, with a bundle guard that keeps the host's module graph small (architecture:component-style-authoring INV6), and the README documents the recognizers' kind and the press tracker factory. meta#8763's and meta#8797's changes, rebased onto meta#8309 as one commit; its press tracker factory and recognizer kind now come with meta#8076 and meta#8071.
38070ea to
ffb66b1
Compare
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved, and the diff also needs to shrink to its own commit. Against main it adds requirements the current records do not state to spec:AST-094 FR11 and FR19, architecture:react-component-runtime INV13, component:BottomSheetSwitcher FR5, component:Drawer FR5 and component:MobileNav FR3; an engineering owner approves or rejects them by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision:
- #8360, #8098 and #8309 need to land first, so this diff is its own 173-line commit instead of 19,834 lines.
hooks/scrollLockTouchGuard.ts(#8098) re-implements the edge checks and scroll-chain walk ofgestures/nativeScroll.ts; extend the native-scroll probe and call it instead.gestures/useFingerScroll.ts(#8360) re-implements the node-following ofuseAttachment; call it instead.
[Automated review]
| function canMoveY(element: Element, dy: number, range: number): boolean { | ||
| // A finger moving down drags the content down: scrollTop decreases. | ||
| return dy > 0 | ||
| ? element.scrollTop > EDGE_TOLERANCE - 0.5 | ||
| : element.scrollTop < range - EDGE_TOLERANCE + 0.5; | ||
| } | ||
|
|
||
| function canMoveX( | ||
| element: Element, | ||
| dx: number, | ||
| range: number, | ||
| isRtl: boolean, | ||
| ): boolean { | ||
| const min = isRtl ? -range : 0; | ||
| const max = isRtl ? 0 : range; | ||
| // A finger moving right drags the content right: scrollLeft decreases. | ||
| return dx > 0 | ||
| ? element.scrollLeft > min + EDGE_TOLERANCE - 0.5 | ||
| : element.scrollLeft < max - EDGE_TOLERANCE + 0.5; | ||
| } |
There was a problem hiding this comment.
These edge checks re-implement canMoveY and canMoveX from gestures/nativeScroll.ts:189-212 (the same ±0.5 px tolerance and RTL scrollLeft range), and isPanAbsorbedBeforePage below walks the same parentAcrossShadow chain as createNativeScrollProbe. nativeScroll.ts imports nothing of the gesture host, so the page hold could call it: export the edge helpers, or a chain walk that stops before the page and reports a contained edge, and delete these copies so a fix to one reaches both.
Section 3 of the merge plan: #8360 → #8098 → #8309 → #8763 → #8342. One commit on #8309 holding only this change. It absorbs #8797 (the modifier keys), which closes with a link here; the commit credits its authors. The press tracker factory and the recognizers'
kindit used to carry now come with #8076 and #8071.Maintainer impact
architecture:component-style-authoringINV6).kindor the press tracker factory.Intended invariant
A gesture touch reports the modifier keys its press began with; press tracking reads only the press clocks, and the host's module graph stays inside its bundle guard. Nothing a control does changes.
Change
types.ts,GestureArena.ts,GestureHost.ts: a gesture touch carriesctrlKey,metaKey,shiftKeyandaltKeyas they were when its press began, read from the pointer or touch event; so does a finger that lands where nothing takes part, as recognizers tracking another finger hear it.PressTracker.ts: imports the press clocks from the timing table, not the whole table;GestureHost.bundle.test.tspins it.README.md: each recognizer reports itskind, which code reads in place ofinstanceof(architecture:react-component-runtimeINV13), and press tracking is built bycreatePressTracker, withVelocityTrackerthe one class INV13 lists as a gap.Evidence
pointerdownortouchstart.Scope
Testing
Local gates at ffb66b1, one commit on #8309 holding only this change (8 files, +162/−11):
pnpm test:ui(the UI tests of Core, Lab and Richtext): 717 files, 17,766 tests passed.pnpm exec vitest run --project node, after building Core: 838 files passed, the WebKit job's scope test among them; the only failures are two cases inpackages/cli/foundation/release/cache.test.mjs, which fail on main as well (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 63 passed; WebKit 52 passed and 11 skipped, for input WebKit's driver does not provide. Under CI's WebKit lane profile (a container capped at 4 CPUs withCI=1, renderer threads matched, one worker), ten runs: 520 passed, none failed.tsc --project packages/core/tsconfig.json --noEmit, strict ESLint and Prettier on every changed file: clean.