Skip to content

fix(MobileNav): lock the page through the shared scroll lock - #8119

Draft
vjeux wants to merge 1786 commits into
meta:mainfrom
vjeux:uikit/c14-mobilenav-scroll-lock
Draft

vjeux wants to merge 1786 commits into
meta:mainfrom
vjeux:uikit/c14-mobilenav-scroll-lock

Conversation

@vjeux

@vjeux vjeux commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

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 auto and touch height="adaptive". MobileNav's own root lock, overflow: clip written 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):

drawer drag, page stays

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:

android drawer drag, page stays

The same drawer with the blur on (still): blur on.

  • MobileNav and AppShell unit suites: the root (or <body>, when its own overflow reaches the viewport) reads overflow: hidden while the drawer is open and is released when it closes, also when the dialog was closed natively before isOpen turned false.
  • The guard's own touch proofs, including iOS 26.2, are in fix(useScrollLock): keep the page where it is while an overlay is open #8098.

Problem and solution fit

MobileNav now calls useScrollLock(isOpen). The lock brings three things:

  • the shared root-style claim (overflow: hidden through claimRootScrollStyle);
  • the scrollbar gutter hold;
  • the touch guard, which cancels one-finger pans whose scroller chain reaches the page. Pans the drawer's own list can take, and pinch-zoom, still pass.

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

  • Removed: MobileNav's root overflow write and its own gutter hold.
  • Added: the one useScrollLock call, declared before the open/close effect so the gutter is measured before showModal().
  • The unmount, native-close and Activity-hidden paths release through the hook.
  • Changeset: [fix].

Scope

  • One defect is restored.
  • Separable contradictory or unsettled tagalongs were removed or split.
  • No new public API, visual representation, default, or interaction model is hidden under the bug-fix label.
  • Public text and artifacts contain no internal Meta context.

Testing

a mouse opening and closing MobileNav on a full-height page, before and after

@vercel

vercel Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Error Error Oct 29, 2026 10:59pm UTC

Request Review

@github-actions github-actions Bot added the needs:design-review Affects visuals — Design should review label Oct 24, 2026
@astracat-bot

astracat-bot Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
🟡 Reviewed (for maintainers only) Oct 29, 2026, 11:23 PM UTC

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 24, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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% } with body { overflow-x: hidden }, opening a Dialog now jumps the page to the top and leaves it there after close, because the root's overflow: hidden stops <body>'s overflow from propagating to the viewport.
  • The touch guard starts its walk from event.target, which a document listener sees as the shadow host, so pans inside a scroller in a shadow root are cancelled; starting from event.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-lock changeset 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'});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

Preview availability: Storybook and Sandbox are not both ready on the exact-head Vercel preview. CI concluded failure.

Component Size

Component Import before Import after Delta CSS before CSS after Delta
core/AlertDialog 56.9KB 58.8KB +1.9KB 12.4KB 12.4KB 0B
core/AppShell 57.9KB 59.9KB +2.1KB ⚠️ 11.6KB 11.6KB 0B
core/AvatarGroupOverflow 39.9KB 40.2KB +303B 6.4KB 6.4KB 0B
core/Banner 47.8KB 48.1KB +289B 9.9KB 9.9KB 0B
core/BaseTypeahead 54.3KB 54.6KB +251B 9.9KB 9.9KB 0B
core/BottomSheet 34.0KB 35.6KB +1.6KB 7.9KB 7.9KB 0B
core/BottomSheetSwitcher 14.8KB 16.5KB +1.7KB 3.3KB 3.3KB 0B
core/BreadcrumbItem 66.4KB 66.6KB +297B 11.0KB 11.0KB 0B
core/Button 43.3KB 43.5KB +273B 9.2KB 9.2KB 0B
core/Calendar 54.9KB 55.2KB +290B 10.0KB 10.0KB 0B
core/Carousel 50.2KB 50.5KB +325B 10.2KB 10.2KB 0B
core/ChatComposer 60.6KB 60.9KB +270B 10.8KB 10.8KB 0B
core/ChatComposerInput 55.7KB 56.0KB +264B 10.1KB 10.1KB 0B
core/ChatComposerTokenElement 55.7KB 56.0KB +264B 10.1KB 10.1KB 0B
core/ChatDictationButton 46.5KB 46.8KB +264B 9.5KB 9.5KB 0B
core/ChatLayout 48.8KB 49.1KB +273B 9.9KB 9.9KB 0B
core/ChatLayoutScrollButton 46.4KB 46.6KB +255B 9.6KB 9.6KB 0B
core/ChatSendButton 45.8KB 46.1KB +265B 9.2KB 9.2KB 0B
core/CheckboxInput 49.9KB 50.2KB +268B 9.3KB 9.3KB 0B
core/CheckboxListItem 54.0KB 54.3KB +236B 10.5KB 10.5KB 0B
core/ClickableCard 12.8KB 13.1KB +289B 6.9KB 6.9KB 0B
core/CodeBlock 60.5KB 60.8KB +278B 10.7KB 10.7KB 0B
core/Collapsible 11.7KB 12.0KB +291B 4.2KB 4.2KB 0B
core/CommandPalette 50.9KB 52.7KB +1.8KB 11.8KB 11.8KB 0B
core/CommandPaletteItem 8.4KB 8.7KB +280B 3.5KB 3.5KB 0B
core/ComplexSelector 55.8KB 56.1KB +313B 10.6KB 10.6KB 0B
core/ContextMenu 77.7KB 79.5KB +1.7KB 14.6KB 14.6KB 0B
core/ContextMenuItem 16.6KB 16.8KB +290B 5.2KB 5.2KB 0B
core/DateInput 99.9KB 101.7KB +1.8KB 14.8KB 14.8KB 0B
core/DateRangeInput 69.4KB 71.2KB +1.8KB 11.2KB 11.2KB 0B
core/DateTimeInput 110.1KB 111.9KB +1.8KB 15.2KB 15.2KB 0B
core/Dialog 16.2KB 17.9KB +1.7KB 7.1KB 7.1KB 0B
core/DialogHeader 48.8KB 49.1KB +280B 9.8KB 9.8KB 0B
core/Drawer 31.6KB 33.3KB +1.6KB 8.0KB 8.0KB 0B
core/DrawerHeader 48.9KB 49.2KB +249B 11.2KB 11.2KB 0B
core/DropdownMenu 72.7KB 73.0KB +286B 14.7KB 14.7KB 0B
core/DropdownMenuCheckboxItem 16.1KB 16.4KB +284B 5.6KB 5.6KB 0B
core/DropdownMenuItem 15.4KB 15.7KB +287B 5.2KB 5.2KB 0B
core/DropdownMenuRadioItem 16.2KB 16.5KB +275B 5.6KB 5.6KB 0B
core/DropdownMenuSubMenu 58.1KB 58.3KB +270B 9.5KB 9.5KB 0B
core/FileInput 54.1KB 54.3KB +252B 10.3KB 10.3KB 0B
core/IconButton 43.3KB 43.6KB +273B 9.2KB 9.2KB 0B
core/Item 11.4KB 11.6KB +273B 4.7KB 4.7KB 0B
core/LayerProvider 63.3KB 63.6KB +288B 10.1KB 10.1KB 0B
core/Lightbox 51.2KB 53.0KB +1.8KB 9.8KB 9.8KB 0B
core/Link 43.5KB 43.8KB +288B 7.6KB 7.6KB 0B
core/ListItem 53.5KB 53.7KB +257B 10.5KB 10.5KB 0B
core/Markdown 144.5KB 146.2KB +1.7KB 17.4KB 17.4KB 0B
core/MobileNav 51.0KB 53.1KB +2.1KB ⚠️ 10.1KB 10.1KB 0B
core/MobileNavToggle 46.2KB 46.5KB +259B 9.4KB 9.4KB 0B
core/MoreMenu 72.9KB 73.2KB +282B 14.7KB 14.7KB 0B
core/MultiSelector 76.2KB 77.9KB +1.7KB 14.6KB 14.6KB 0B
core/NumberInput 55.7KB 55.9KB +269B 10.7KB 10.7KB 0B
core/Outline 28.6KB 28.9KB +290B 4.0KB 4.0KB 0B
core/Pagination 87.7KB 89.4KB +1.7KB 15.3KB 15.3KB 0B
core/Popover 49.8KB 50.1KB +247B 12.1KB 12.1KB 0B
core/PowerSearch 155.1KB 156.9KB +1.8KB 18.6KB 18.6KB 0B
core/RadioListItem 52.1KB 52.4KB +278B 9.7KB 9.7KB 0B
core/SegmentedControlItem 8.6KB 8.9KB +285B 3.9KB 3.9KB 0B
core/SelectableCard 12.8KB 13.1KB +294B 7.1KB 7.1KB 0B
core/Selector 79.1KB 80.8KB +1.7KB 15.0KB 15.0KB 0B
core/SelectorOption 14.5KB 14.8KB +251B 5.1KB 5.1KB 0B
core/SideNav 58.0KB 60.1KB +2.1KB ⚠️ 10.6KB 10.6KB 0B
core/SideNavCollapseButton 46.6KB 46.9KB +242B 9.5KB 9.5KB 0B
core/SideNavHeading 54.7KB 55.0KB +248B 10.1KB 10.1KB 0B
core/SideNavItem 54.5KB 54.8KB +266B 10.1KB 10.1KB 0B
core/Step 32.4KB 32.7KB +279B 5.3KB 5.3KB 0B
core/Stepper 47.4KB 47.7KB +265B 9.6KB 9.6KB 0B
core/Switch 48.4KB 48.7KB +283B 9.3KB 9.3KB 0B
core/Tab 9.7KB 10.0KB +294B 4.1KB 4.1KB 0B
core/TabMenu 51.6KB 51.9KB +276B 10.0KB 10.0KB 0B
core/Table 84.0KB 85.7KB +1.7KB 15.1KB 15.1KB 0B
core/TableCell 78.8KB 80.5KB +1.7KB 14.8KB 14.8KB 0B
core/TableHeaderCell 78.6KB 80.3KB +1.7KB 14.8KB 14.8KB 0B
core/TableSelectionToolbar 52.4KB 52.6KB +278B 11.2KB 11.2KB 0B
core/TextInput 53.5KB 53.8KB +274B 10.5KB 10.5KB 0B
core/Thumbnail 50.7KB 50.9KB +274B 11.0KB 11.0KB 0B
core/TimeInput 86.8KB 88.7KB +1.9KB 13.8KB 13.8KB 0B
core/Timestamp 27.5KB 27.5KB -1B 9.5KB 9.5KB 0B
core/Toast 59.8KB 60.1KB +258B 9.7KB 9.7KB 0B
core/ToggleButton 44.1KB 44.4KB +243B 9.3KB 9.3KB 0B
core/Token 29.6KB 29.9KB +289B 4.5KB 4.5KB 0B
core/Tokenizer 64.8KB 65.1KB +306B 11.7KB 11.7KB 0B
core/TopNav 51.9KB 53.9KB +2.0KB ⚠️ 10.4KB 10.4KB 0B
core/TopNavHeading 54.2KB 54.4KB +231B 9.9KB 9.9KB 0B
core/TopNavItem 9.0KB 9.3KB +283B 3.8KB 3.8KB 0B
core/TopNavMegaMenu 54.2KB 54.4KB +268B 10.8KB 10.8KB 0B
core/TopNavMegaMenuItem 9.1KB 9.4KB +281B 4.1KB 4.1KB 0B
core/TopNavMenu 53.6KB 53.8KB +258B 10.1KB 10.1KB 0B
core/TreeList 32.5KB 32.8KB +274B 4.7KB 4.7KB 0B
core/Typeahead 61.5KB 61.7KB +268B 11.3KB 11.3KB 0B

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.

⚠️ Touch-only size budget

These components grew past the touch-only budget (2 KB gzip JavaScript or 1 KB gzip CSS per component). If the growth runs only on a coarse pointer, load its JavaScript lazily so the import does not carry it, or justify the cost in the pull request description; CSS stays in the package stylesheet, so justify it.

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

vjeux added a commit to vjeux/astryx that referenced this pull request Oct 25, 2026
…corded on the restacked heads (iOS 26.2 simulator, Chromium, Android emulator with Chromium 158)
@vjeux
vjeux force-pushed the uikit/c14-mobilenav-scroll-lock branch from c110de2 to 62fcf7d Compare October 25, 2026 04:48
vjeux added a commit to vjeux/astryx that referenced this pull request Oct 25, 2026
…rag with the backdrop blur off for capture, and a blur-on still

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.stylex is a new stable export outside the experimental boundary, and the theme template documents a platform adaptation condition that defineTheme rejects.
  • 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),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/cli/assets/theme.template.ts Outdated
Comment on lines +361 to +362
* are primary-pointer precision, contrast preference, motion preference, and
* platform (`ios` for the browsers of iPhone and iPad, `other` for the rest).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Comment thread scripts/sync-exports.js Outdated
@@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +108 to +110
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .changeset/touch-presentation.md Outdated
'@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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-authoring INV6).
  • Limit the press selection lock to touch or show pen evidence (spec:AST-017 FR59), 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 make reduced-motion-record.md an internal [chore] note.

[Automated review]

*/

import {useEffect} from 'react';
import {claimRootScrollStyle, reportScrollLock} from './rootScrollStyles';
import {getGestureHost} from '../gestures/GestureHost';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .changeset/reduced-motion-record.md Outdated
'@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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

vjeux added a commit to vjeux/astryx that referenced this pull request Oct 26, 2026
The MobileNav touch presentation proof needs no workflow step: pr-a11y
discovers it by name and runs it on a change to MobileNav.
vjeux added a commit to vjeux/astryx that referenced this pull request Oct 26, 2026
vjeux added 25 commits October 28, 2026 11:33
…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.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.mjs currently fails its root-entry and hooks-entry cases because useScrollLock now imports gestures/pageHold; the head that merges needs that test's replacement or a lock that keeps the page hold out of those entries.

[Automated review]

vjeux added a commit to vjeux/astryx that referenced this pull request Oct 30, 2026
Merged on meta#8098 (5d9fc1a), the base both carry, so that only meta#8119's own
MobileNav files apply on top of meta#8342 and meta#8098.
vjeux added a commit to vjeux/astryx that referenced this pull request Oct 30, 2026

This branch had an error being deployed

1 failed deployment
Preview — 284dbd5e Deployed Oct 29, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants