Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
There was a problem hiding this comment.
This pull request is on hold for an owner decision. It carries #8064's MobileNav close behavior, which blocks every tap until the slide-out ends and returns focus only then, and no current spec decides that behavior; an engineering owner needs to approve or reject it by commenting /approve-spec <full head SHA> on this pull request, and as an engineering owner you may record that yourself. Whatever the decision, the following still need fixing, each with a regression test:
- On a page styled
html, body { height: 100% }withbody { overflow-x: hidden }, opening a Dialog now jumps the page to the top and leaves it there after close, because the root'soverflow: hiddenstops<body>'s overflow from propagating to the viewport. - The touch guard starts its walk from
event.target, which adocumentlistener sees as the shadow host, so pans inside a scroller in a shadow root are cancelled; starting fromevent.composedPath()[0]fixes it. - The MobileNav fix targets iOS Safari, so it needs an iOS Safari run on this head showing that a drag on the open drawer leaves the page still while the drawer's list still scrolls.
- The
mobile-nav-shared-scroll-lockchangeset says a pull-to-refresh control sees the lock, but no such control is part of this change.
[Automated review]
|
|
||
| // Taken before the root's overflow hides the scrollbar. | ||
| const gutter = holdScrollbarGutter(document.documentElement); | ||
| const releaseRootStyle = claimRootScrollStyle({overflow: 'hidden'}); |
There was a problem hiding this comment.
On a page styled html, body { height: 100% } with body { overflow-x: hidden }, <body>'s overflow propagates to the viewport only while the root's overflow is visible. This claim stops that, so <body> becomes its own 100%-tall scroller and the viewport offset clamps to 0. Releasing the claim does not bring the offset back. In Chromium a Dialog on such a page moved scrollY 600 → 0 and left it at 0 after close, where main restores 600. Detecting that case (root overflow visible, <body>'s computed overflow not visible) and claiming <body>, or saving and restoring the offset, would keep the page where it is.
| return; | ||
| } | ||
| // The browser picks the scroller once per gesture; so does the guard. | ||
| verdict = isPanAbsorbedBeforePage(event.target, dx, dy) |
There was a problem hiding this comment.
This listener sits on document, where event.target for a touch inside a shadow root is retargeted to the shadow host. The walk therefore never reaches a scroller inside the shadow tree, and the pan is cancelled. In Chromium an 80 px swipe moved a shadow-root scroller 0 px, against 76 px for a light-DOM scroller in the same open Dialog. Starting from event.composedPath()[0], or walking composedPath(), would let those scrollers take the pan.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] MobileNav locks the page through the shared scroll lock (`useScrollLock`) while it is open and until its slide-out ends. On iOS a drag on the open drawer no longer scrolls the page behind it, and other overlays and the pull-to-refresh control see the lock. |
There was a problem hiding this comment.
No pull-to-refresh control exists in this change or tree, and subscribeScrollLock has no production subscriber, so "and the pull-to-refresh control see the lock" promises something users will not find. Dropping that clause keeps the note accurate.
PR Analysis Report
Component Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included.
|
| Component | JavaScript | CSS |
|---|---|---|
| core/AppShell | +2.1KB |
0B |
| core/MobileNav | +2.1KB |
0B |
| core/SideNav | +2.1KB |
0B |
| core/TopNav | +2.0KB |
0B |
Bundle Size Summary
Gzip figures measure each package's entry file, not the code an import pulls in.
| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |
|---|---|---|---|---|
| @astryxdesign/core | 4.9KB | 1.2KB | 1.2KB | 0B |
| Package stylesheet | Gzip before | Gzip after | Delta |
|---|---|---|---|
| @astryxdesign/core | 34.6KB | 34.6KB | 0B |
Accessibility Audit
Status: No accessibility violations detected.
Visual Regression
55 of 502 shot(s) changed.
A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.
| component | story | theme | mode | pixels |
|---|---|---|---|---|
| Selector | Indicator Space Bottom Sheet Narrow End RTL | neutral | light | 753,933 |
| Selector | Indicator Space Bottom Sheet Wide Start | neutral | light | 739,916 |
| RichTextEditor | Markdown parity: side by side | probe | dark | 558,840 |
| RichTextEditor | Markdown parity: side by side | probe | light | 540,756 |
| Chat | Full AI Chat | probe | light | 230,823 |
| Chat | Full AI Chat | probe | dark | 229,805 |
| AppShell | With Mobile Nav | probe | light | 217,010 |
| AppShell | With Mobile Nav | probe | dark | 216,960 |
| Collapsible | Dividers | probe | dark | 66,445 |
| Collapsible | Dividers | probe | light | 66,445 |
| AppShell | Playground | neutral | dark | 65,198 |
| AppShell | Playground | neutral | light | 63,788 |
| Chat | Default | neutral | dark | 36,304 |
| Chat | Default | neutral | light | 35,767 |
| Chat | Empty State | probe | light | 23,531 |
| Chat | Empty State | probe | dark | 22,622 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | dark | 15,378 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | light | 14,107 |
| Chat | With Attachments | probe | light | 13,406 |
| Chat | With Attachments | probe | dark | 11,232 |
and 35 more.
Generated by PR Enrichment workflow | View full report
c110de2 to
62fcf7d
Compare
…rag with the backdrop blur off for capture, and a blur-on still
There was a problem hiding this comment.
This pull request is on hold for an engineering owner's decision. The draft spec:AST-094 it adds is the only record behind the experimental gesture and motion subpaths and the theme motion roles in this diff, so an engineering owner needs to approve or reject it by commenting /approve-spec with this pull request's full head SHA; as an engineering owner, you may record that decision yourself. Whatever the decision, the following need fixing:
- The diff carries the whole open stack under #8098 (about 34,000 authored lines), so land those pull requests first and rebase until this one holds only its 96-line MobileNav commit.
- The stable press controller (
pressFeedback.ts) now turns off text selection and the touch callout under every touch press but ships under an[experimental]changeset, so keep the lock to the experimental press tracker or release it as a stable change. @astryxdesign/core/motion/reducedMotion.stylexis a new stable export outside the experimental boundary, and the theme template documents aplatformadaptation condition thatdefineThemerejects.- In
spec:AST-094, FR8's 0.5 s Reduce Motion cross-fade conflicts with the current 200 ms rule, FR4 says touch physics are not themable while this diff adds theme motion roles, and FR7 cites a spec that does not exist; three changesets also misdescribe what this head ships.
[Automated review]
| @@ -283,6 +301,9 @@ function onPointerDown(event: PointerEvent): void { | |||
| gesture: idlePressGesture(), | |||
| pointerId: event.pointerId, | |||
| timer: null, | |||
| // A held finger never starts text selection on the surface (UIKit's | |||
| // controls and cells never do), on any coarse pointer. | |||
| unlock: lockSelection([element], event.target), | |||
There was a problem hiding this comment.
This lock now runs for every touch or pen press on every usePressFeedback surface (Button, Link, Switch, Tab, Item and 26 more), writing user-select, -webkit-user-select and -webkit-touch-callout as none until the lift. That is a stable runtime change, which spec:AST-017 FR22 keeps out of an [experimental] changeset, and on iOS it should also hide the link callout on every Link. Keeping the lock to the experimental press tracker and recognizers with preventsTextSelection would match the changeset.
| * are primary-pointer precision, contrast preference, motion preference, and | ||
| * platform (`ios` for the browsers of iPhone and iPad, `other` for the rest). |
There was a problem hiding this comment.
defineTheme has no platform condition (CONDITION_AXES is width, pointer, contrast and motion), and when: {platform: 'ios'} throws "… is not supported". spec:AST-012 FR2 closes the vocabulary, so this sentence should go back to "primary-pointer precision, contrast preference, and motion preference".
| @@ -177,6 +190,13 @@ const STATIC_EXPORTS = { | |||
| types: './dist/utils/interactionOverlay.stylex.d.ts', | |||
| default: './dist/utils/interactionOverlay.stylex.js', | |||
| }, | |||
| // Proposed; pending owner review (spec:AST-094 OQ4): the Reduce Motion | |||
There was a problem hiding this comment.
spec:AST-094 has only OQ1 and OQ2, and this subpath ships with no @experimental marker, so once released it is a stable export (spec:AST-017 FR22; FR27 asks for experimental exports and names for CSS and tokens). Exporting these constants from motion/experimental, or holding the export until an owner approves it, keeps it inside the boundary.
| cross-fades over 0.5 s on the ease-in-out curve; a sheet, a full-screen | ||
| modal or an alert fades on its own presentation spring and drops its travel | ||
| and scale; menus open and close on UIKit's Reduce Motion spring. Motion that |
There was a problem hiding this comment.
The current design:reduced-motion DR2 says a Reduce Motion transition "MUST change opacity only and MUST finish in 200 ms or less", and this head's animator uses 200 ms (motion/uikit/timing.ts:151). A 0.5 s push or pop and a spring-timed sheet fade contradict it, and the Current-state impact does not name that record.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] `useScrollLock`, and so Dialog, BottomSheet, Drawer and Lightbox, keeps the page where it is while an overlay is open. The lock used to pin `<body>` with `position: fixed` at minus the scroll offset and scroll back on release: the page's offset read 0 for the whole lock, and on iOS Safari the jump to the top brought the toolbar back, where it stayed after the overlay closed. Now the page gets `overflow: hidden` (on the root, or on `<body>` when the body's own overflow is the one the viewport uses), and on touch screens a one-finger pan that would scroll the page is cancelled, which iOS Safari before 26.4 needs because it scrolls an `overflow: hidden` page under a finger. Pans that a scroller inside the overlay can take, and pinch-zoom, still work. The lock no longer pins `<body>`, and `window.scrollY` keeps its value while the lock holds. Pans are judged through the shared gesture host, after a small slop so a swipe's first wobble does not fix the wrong axis. MobileNav's own clip goes through the same page scroll-style claims, so closing or unmounting it no longer releases another overlay's lock. |
There was a problem hiding this comment.
At this head MobileNav has no clip of its own; it calls useScrollLock (see mobile-nav-shared-scroll-lock.md, released with this note). The note also leaves out that the lock writes overscroll-behavior-y: none on the root and <body>, which turns off pull-to-refresh while an overlay is open.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] `presentation="adaptive"` on DropdownMenu, ContextMenu, MoreMenu, Selector and MultiSelector, and a DropdownMenu's default sub-menu drill-in, now use the theme's compact definition with a coarse primary pointer: a viewport narrower than the theme's `md` point (768px by default) or shorter than 500px. This is a behavior change. The old default was a bottom sheet at 768px wide and below on a coarse pointer. Now a coarse-pointer screen exactly 768px wide keeps the anchored popover, and a phone in landscape (wider than 768px but shorter than 500px) gets the bottom sheet and the drilled-in sub-menu. To pin a component's presentation regardless of the rule, pass `presentation="popover"` or `presentation="bottom-sheet"`. Mouse and keyboard users on desktop see no change, because every case requires a coarse pointer. |
There was a problem hiding this comment.
COMPACT_TOUCH_QUERY is built from DEFAULT_COMPACT_DEFINITION (touchPresentation.ts:71), so a theme that overrides md does not move this edge. Either say 768px, or make the resolver read the theme's effective md as spec:AST-012 FR9 describes.
62fcf7d to
edc6cee
Compare
…ock) as a dependency
There was a problem hiding this comment.
This pull request is on hold for an engineering owner's decision: alongside the MobileNav fix, its compare view ships stable additions that no current spec decides (defineTheme touch motion roles, the press-feedback selection lock for touch and pen, and the astryxStylex undefinedVariables option) and the internal gesture and motion system behind them. An engineering owner needs to approve or reject them by commenting /approve-spec edc6cee7bf1e515195ab93322157d722ca2d0fe0 on the pull request; as an engineering owner, the author may record that decision too. Whatever the decision, these need changes before approval:
- Land or drop the #8098 stack first and rebase, so this pull request carries only its 96-line MobileNav commit instead of 34,611 authored lines.
- Keep the gesture host out of
useScrollLock's static imports, since CI's size report shows Dialog, BottomSheet, Drawer, Lightbox, MobileNav and 18 other components each growing by about 17 KB gzip for a touch-only page hold (architecture:component-style-authoringINV6). - Limit the press selection lock to touch or show pen evidence (
spec:AST-017FR59), and release it and the new build option under accurate changeset tags. - Correct
scroll-lock-keeps-the-page.md, which still describes a MobileNav clip, and makereduced-motion-record.mdan internal[chore]note.
[Automated review]
| */ | ||
|
|
||
| import {useEffect} from 'react'; | ||
| import {claimRootScrollStyle, reportScrollLock} from './rootScrollStyles'; | ||
| import {getGestureHost} from '../gestures/GestureHost'; |
There was a problem hiding this comment.
This static import puts the whole gesture host (arena, pan and tap recognizers, the UIKit gesture table) into every component that reaches useScrollLock: CI's size report shows Dialog going from 13,787 to 30,964 gzip bytes and 22 other components growing by about 17 KB each. The page hold only needs document and isTouchClaimed, which answers false while no recognizer is attached. architecture:component-style-authoring INV6 asks a touch-only feature over 2 KB to load lazily or justify the cost; reading an already-created host from a small registry module, treating no host as nothing claimed, would keep the hold and drop the weight.
| @@ -283,6 +301,9 @@ function onPointerDown(event: PointerEvent): void { | |||
| gesture: idlePressGesture(), | |||
| pointerId: event.pointerId, | |||
| timer: null, | |||
| // A held finger never starts text selection on the surface (UIKit's | |||
| // controls and cells never do), on any coarse pointer. | |||
| unlock: lockSelection([element], event.target), | |||
There was a problem hiding this comment.
onPointerDown returns early only for mouse, so a pen takes this lock as a finger does. spec:AST-017 FR59 lets touch work change what fine-pointer users do only for a defect they hit or a named cross-device reason, with before-and-after evidence, and none is in this pull request. Limiting the lock to pointerType === 'touch', or naming the reason and showing pen evidence, would settle it; the [fix] label also needs a documented contract to restore.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] `useScrollLock`, and so Dialog, BottomSheet, Drawer and Lightbox, keeps the page where it is while an overlay is open. The lock used to pin `<body>` with `position: fixed` at minus the scroll offset and scroll back on release: the page's offset read 0 for the whole lock, and on iOS Safari the jump to the top brought the toolbar back, where it stayed after the overlay closed. Now the page gets `overflow: hidden` (on the root, or on `<body>` when the body's own overflow is the one the viewport uses), and on touch screens a one-finger pan that would scroll the page is cancelled, which iOS Safari before 26.4 needs because it scrolls an `overflow: hidden` page under a finger. Pans that a scroller inside the overlay can take, and pinch-zoom, still work. The lock no longer pins `<body>`, and `window.scrollY` keeps its value while the lock holds. Pans are judged through the shared gesture host, after a small slop so a swipe's first wobble does not fix the wrong axis. MobileNav's own clip goes through the same page scroll-style claims, so closing or unmounting it no longer releases another overlay's lock. |
There was a problem hiding this comment.
At this head MobileNav has no clip of its own; it calls useScrollLock (see mobile-nav-shared-scroll-lock.md, released with this note), so the last sentence describes code that is gone. The note also leaves out that the lock writes overscroll-behavior-y: none on the root and <body>, which turns off Chrome on Android's pull-to-refresh while an overlay is open.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The experimental touch animator follows design:reduced-motion. Under Reduce Motion every `animate` and `play` (travel or opacity, on a spring or any curve) becomes an opacity-only cross-fade of 200 ms on a coarse primary pointer, or for a call marked `touchRelease`, and completes at once on any other pointer; nothing moves, scales or springs, and travel lands with a plain style write while the element is transparent, never as an animated transform. `timing.reducedMotionCrossFadeMs` is now 200 ms and is no longer a theme role. The `reducedMotion` and `crossfade` call options are removed; `crossfadeAs` stays. `animator.reducedMotion()` reports the mode for feedback with no fade of its own, `reducedMotionFade()` gives the same fade to code beside the animator, and `reducedMotion.stylex.ts` holds its constants for StyleX transitions. |
There was a problem hiding this comment.
This head exports no touch animator (no motion entry in packages/core/package.json), so a [fix] note about its options tells consumers about an API they cannot import. Its siblings use [chore] Internal: …; this one should too.
| '@astryxdesign/build': patch | ||
| --- | ||
|
|
||
| [fix] `astryxStylex` resolves `defineConsts` values read from another file. With the `{stylexOptions}` form (the one Storybook uses), a file that read only constants from a `.stylex.ts` module lost that import once StyleX compiled it, so the constants never reached the CSS: a constant used as a media-query key failed the build ("Invalid empty selector") and one used as a value shipped as a custom property nothing declares. A constant defined on the library side of the layer split and read on the product side stayed unresolved in either form. A production build now also warns, naming the variable and the rule that reads it, when its stylesheet reads a StyleX-generated custom property that nothing declares; set `undefinedVariables: 'error'` to fail the build instead. |
There was a problem hiding this comment.
The cross-file defineConsts fix fits [fix], but the new undefinedVariables option and its default production warning are a compatible capability, which spec:AST-017 FR7 releases as [feat]. A separate changeset for the option would keep each category accurate.
…s, and the action-group tracker writes its own The header and writePress called it the one writer of the press attribute, but main's action-group tracker writes its groups' presses itself. They now say what holds, as meta#8076's gestures README does.
…he press attribute The release note said every press painter writes data-astryx-press through one internal writer, but the action-group tracker (hooks/actionGroupTracker.ts) writes its groups' presses itself, as the gestures README and utils/pressFeedback.ts already say. The note now says paintPress writes every press it tracks and the action-group tracker writes its own.
…llowance under every primary pointer The useMenuPress record's FR9 puts the trigger's held finger under spec:AST-094 FR29 under every primary pointer (FR43), so a finger on a touchscreen laptop may drift 13 pt before the open, as on a phone. The trigger read (pointer: coarse) as its press started and took the long press's 10 px where the pointer was fine. Three cases fail until it does not: a 12 pt drift opens under a fine pointer too, a (12, 12) settle opens there, and nothing reads a pointer query, the trigger's press included.
…lowance under every primary pointer The useMenuPress record's FR9 governs the trigger's held finger by spec:AST-094 FR29 under every primary pointer (FR43). The trigger now hands the long press FR29's 13 px drift allowance whatever the primary pointer, so it no longer reads (pointer: coarse) as its press starts, and the menus drop their coarsePointer import. ContextMenu keeps the long press's own allowable movement. The changeset says the same.
Strict ESLint reported six unused bindings in files this pull request carries: the host's HAPTIC_SWITCH_ATTRIBUTE import, two unused imports in the pan's tests (scrollProbe, ScreenEdgePanGestureRecognizer), the swipe test's drift, the presented-hit-testing test's gestures, and a GestureHost test's before, which now asserts what it was read for: a second finger's moves leave the pan where it was.
…eal angular speed The release velocity of a rotation whose listener sets rotation = 0 on each change, at 2 degrees every 16 ms, read 4.363 rad/s against the 2.182 rad/s of the same stream without a re-base: the test asserted only more than 1.5. It now asserts the 2.18 rad/s (spec:AST-100 FR2), and fails.
The rotation's velocity tracker samples the raw unwrapped angle, which setting rotation does not move, so the setter's shift of the samples read each re-base as extra turn and doubled the release velocity of a listener that re-bases on every change. The setter no longer touches the tracker.
…mates while it runs The proof read the element's animations 230 ms in, after the dip had ended, and asserted only that every animated key is opacity, which an empty list passes: with the cross-fade removed from the plan (the move still landing), the proof still passed. It now reads the keyframe keys right after the call starts and asserts they include opacity and nothing else; the same mutant fails it. Chromium and WebKit pass. (F1's in-flight read from c648034.)
…roller, and its claimed moves are cancelled under pointer events Two comparisons stop at the shadow boundary, each failing a case: - once the host's listener on a recognizer's inner node has seen the finger, isFingerDownIn reads it as outside a light-DOM scroller around that root, so a scroll under it is not the finger's (FR20); - under inputMode 'pointer-events', a claimed pan on a shadow host cancels none of its finger's moves over the inner content, since the finger's target is the inner node and its touches reach the host retargeted (FR19).
…the flat tree isFingerDownIn and the pointer-events finger match compare with containsAlongFlatTree, as the scroll watch already does: a finger on content inside an open shadow root is down in the scroller around that root, and its touches, retargeted to the shadow host, match its pointer finger, so a claimed pan cancels its moves there (spec:AST-094 FR19, FR20).
…achments asks, and only then The host let the last attachment decide whether an element is an action group (useGestureRecognizer always passes the option), and never recomputed it at a detach: a plain attachment after the group's turned the group off, and a group's attachment released left a recognizer that never asked taking presses under a fine pointer. Fails until the host counts the attachments that ask.
…ents asks The host counts the attachments that pass actionGroup: true and keeps the element a group while one remains, recomputed at each detach. An attachment that passes false, as useGestureRecognizer does by default, no longer turns a group off, and a group's last asking attachment taking its leave ends the group.
… that content The presented hit test takes the document's elementFromPoint, which answers a shadow host for anything inside its open root, and compares with Node.contains, which stops at the boundary: a recognizer on a control inside a moving sheet's shadow root gets no finger, and the tap's click goes to the host. Fails until the hit test descends into open roots and compares along the flat tree.
Where a moving element is drawn away from its layout box, the host hit-tests what is drawn under the finger with the document's elementFromPoint, which answers a shadow host for anything inside its open root. The hit test now descends through each open root's own elementFromPoint and compares with the movers along the flat tree, so a recognizer on a control inside a moving sheet's shadow root gets its finger and the tap clicks that control.
…nt lets go The host's touch-action rules sit in an adopted sheet or a style element per document or shadow root, and were never removed, so a page whose last recognizer detached kept them, against spec:AST-094 FR24 (nothing touch-related remains after the last instance unmounts). Fails until a root's rules go with its last marked element.
…th its last marked element The touch-action writer records which elements it marked in each document or shadow root, and when the last one is unmarked (its last recognizer detached) it removes that root's adopted sheet or style element, so nothing touch-related remains after the last instance unmounts (spec:AST-094 FR24). A later mark writes the rules again; an element marked in another root first lets go of the old one.
… the inline style it reads - The motion registry, the host's engine gate and the README name the presented hit test as an engine workaround (spec:AST-013 FR6) with its review condition (re-measure on each iOS release; remove it once iOS WebKit hit-tests a compositor-moved element where it is drawn) and its regression fixture. - While a curve runs the animator holds the value it starts from inline, so presentation.ts and the README no longer say the inline style holds the end.
…core/internal/touch Core ships the internal entry @astryxdesign/core/internal/touch, which re-exports the gesture primitives for Astryx's own packages, and the Lab-import guard fails a Lab import of core outside core's exports (architecture:react-component-runtime INV13). The README and the barrel's header named internal workspace imports and said nothing here is exported; they now name that entry.
…ill needs the element, and the listeners that cancel The release note, the stream's header, the host's comment and the README said the stream is one passive listener per event type and that a document-capture stopPropagation no longer starves the recognizers. Such a stop never hid a listener on the same node, and some subscriptions cancel. They now say what changed: an app's window-capture stopPropagation, or a document-capture stopImmediatePropagation registered before core, no longer hides a touch; a recognizer that must claim still needs the touch to reach its element; the gesture host's touchend, click and contextmenu cancel while a claimer is attached and the page hold's touchmove while it holds; and the stream stays ahead of an app's window-capture listener only until a type's listener is re-added. The README's list of blocking touchmove listeners gains the claimed touch's target and the document while a claiming element moves.
…default it moves
The {stylexOptions} form of astryxStylex now passes StyleX
treeshakeCompensation: true unless the caller sets it. spec:AST-017 FR3
asks the release note to name the old and new default and the option
that restores the old one; it now does.
…er and for the two stops the input stream fixes - A still finger with sub-pixel jitter over a scroll view keeps its tap and scrolls nothing (spec:AST-094 FR18–FR22). - A tap is still recognized under an app's window-capture stopPropagation, and under a document-capture stopImmediatePropagation registered before core's: the recognizers hear touches on the window's capture phase. Chromium and WebKit pass.
…s once Where nothing that claims is attached, the host's touchend listener is passive and cannot cancel the engine's own click after a retargeted tap clicks what is drawn. The shared click guard swallows that late click whatever is attached; this case pins it, as asked on meta#8293.
…esture host's subscriptions The gesture host subscribes no click: the one cancelling click subscriber is the click guard's, added at a window's first arm and kept. The release note, the stream's header and the gestures README listed click with the host's touchend and contextmenu; they now name the guard's own.
There was a problem hiding this comment.
Changes requested. Switching MobileNav to the shared useScrollLock is small and matches the current MobileNav and touch-presentation requirements, but this pull request still carries the open #8098, #8320 and #8076 stack, about 29,000 authored lines beside MobileNav's 217. Please merge it after that stack lands and rebase so only the MobileNav files remain.
scripts/touch-support-bundle.test.mjscurrently fails its root-entry and hooks-entry cases becauseuseScrollLocknow importsgestures/pageHold; the head that merges needs that test's replacement or a lock that keeps the page hold out of those entries.
[Automated review]
Depends on #8098 (and its predecessors). Review only this PR's change.
User impact
On iOS 26.2, with the page scrolling, a drag on the open MobileNav drawer's body scrolls the page behind it. This reproduced in 3 of 4 simulator runs, in AppShell
autoand touchheight="adaptive". MobileNav's own root lock,overflow: clipwritten on<html>, is ignored for touch.Evidence
iOS 26.2 simulator, Safari, real touches (a dot under the finger), AppShell
auto: the page is scrolled to 500 px, the drawer opens, and the finger drags up and down on the drawer's body. The page stays at its offset throughout (the readout, bottom left):Android 15 emulator, Chromium 158, a real finger, the same path. The backdrop blur is off for capture, because the emulator's software GPU draws it at about 12 fps:
The same drawer with the blur on (still): blur on.
<body>, when its own overflow reaches the viewport) readsoverflow: hiddenwhile the drawer is open and is released when it closes, also when the dialog was closed natively beforeisOpenturned false.Problem and solution fit
MobileNav now calls
useScrollLock(isOpen). The lock brings three things:overflow: hiddenthroughclaimRootScrollStyle);The page offset never moves, and overlays open together share one reference-counted lock.
Expected behavior and authority
A modal drawer keeps the page behind it still, as a UIKit sheet does. MobileNav adopts useScrollLock v2 (#8098) in its own PR.
Smallest restoration
useScrollLockcall, declared before the open/close effect so the gutter is measured beforeshowModal().[fix].Scope
Testing
scripts/touch-support-bundle.test.mjs's "is absent from the root entry" and "is absent from the hooks entry" fail because MobileNav now reaches the shared scroll lock's page hold; test(core): a touch component's import carries its gesture code, and nothing else does #8671 replaces them with a per-component check that lists the components that use a gesture.pnpm exec vitest run packages/core/src/MobileNav packages/core/src/AppShell packages/core/src/Layer packages/core/src/hooks packages/core/src/__tests__ packages/core/src/AlertDialog: 1189 passed; the root typecheck,CI=true pnpm lint,pnpm check:repoand the docs golden pass. On fix(useScrollLock): keep the page where it is while an overlay is open #8098 @ 3d23397 (the shared scroll lock, on the gesture line at fix(core): gesture recognizer and arena fixes from review #8320 @ ca71fce with feat(core): the internal touch foundation: press tracking and the animator #8076 @ 3fb31dd), merged into this branch's previous head; fix(useScrollLock): keep the page where it is while an overlay is open #8098's new case for a page whose<body>sets its own overflow reads the shared lock'shiddenon<body>.scroll-lock-keeps-the-page.mdno longer describes a MobileNav clip: here MobileNav takes the shared lock, as its own note says.overflow: clipon the root or<body>), the gutter hold and the native-close release.overflow: clip.html, body { height: 100% },body { overflow-x: hidden }) scrolled to 2,182 px, main sends the page to its top and leaves it there (Chromium when the drawer opens, WebKit when it closes); with this change it stays at 2,182 px in both engines. Main's root clip stops<body>'s overflow from reaching the viewport, so the full-height<body>becomes a scroller of its own; the lock clips<body>instead, asscroll-lock-keeps-the-page.mdsays, and the changeset names the fix. Without the full-height reset, neither moves.test-ui (4/4)red is the richtext race test(richtext): the code block header tests wait for the copy button's tooltip #8569 fixes (CodeBlockHeaderPlugin.test.tsx), not this change.