Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
needs:design-review
|
AI review status for this pull request.
|
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.
The new lock keeps the page at its offset under a Dialog on an ordinary page. However, it now shares the root's overflow with MobileNav and changes how pages that style <body> scroll, so this needs changes before it can land. Please fix the following, each with a regression test:
MobileNavstill writesdocument.documentElement.style.overflowdirectly, so a BottomSheet opened from an open MobileNav and closed together with it leaves the root atoverflow: clipand the page unscrollable, and closing or unmounting MobileNav drops another overlay's lock; moving MobileNav ontoclaimRootScrollStylewould fix both.- On a page styled
html, body { height: 100% }andbody { overflow-x: hidden }, opening a Dialog moves the page to the top and it stays 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; walkingevent.composedPath()fixes it.
[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.
This claims the root's overflow, which MobileNav still writes directly (MobileNav.tsx:580, :583, :601). A BottomSheet opened from an open MobileNav saves clip here. When both close, MobileNav clears the root first and the sheet's release then restores clip, so the page can no longer scroll (reproduced in Chromium). Closing or unmounting MobileNav while a lock holds also clears this hidden. Moving MobileNav onto claimRootScrollStyle({overflow: 'clip'}) fixes both orders.
Separately, on a page whose <body> sets its own overflow (for example html, body { height: 100% } plus body { overflow-x: hidden }), a root overflow stops the body's value propagating to the viewport. The page then jumps to offset 0 and stays there after close.
There was a problem hiding this comment.
Fixed in 6eee897, in two parts. (1) A release now restores the page's own value only if nothing outside the claims changed that style meanwhile, so a sheet opened from an open MobileNav and closed with it no longer leaves the root at clip; a unit test covers that order. MobileNav moves onto claimRootScrollStyle in #8119, which also stops it from dropping another overlay's lock when it closes. (2) The overflow claim now goes on the element the viewport takes its overflow from: the body, on a page like the one described. A browser test with html, body { height: 100% } and body { overflow-x: hidden } keeps the offset and the content in place while the lock holds and after it ends, in Chromium and WebKit. It fails on the previous build.
| 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.
A document listener sees a touch from inside a shadow root retargeted to the host, so this walk never reaches a scroller inside the shadow tree and parentAcrossShadow has nothing to cross. Pans in such a scroller are cancelled on every touch browser while locked. Starting from event.composedPath()[0], or walking composedPath(), fixes it.
There was a problem hiding this comment.
Fixed in 6eee897: the guard now starts its walk from event.composedPath()[0], the innermost node of the composed path. A unit test pans a scroller inside an open shadow root and the pan goes through. It fails on the previous head.
PR Analysis Report
The exact-head preview is not available yet. Generated by PR Enrichment workflow | View current CI run |
|
Posted by Cindy's agent on her behalf. Parts of this PR re-implement behavior that already exists, or is being specified, as a shared part of core. Converging keeps one implementation per behavior:
This is about where the behavior lives, not about the PR's intent. |
|
Agreed: one implementation.
|
There was a problem hiding this comment.
The new lock keeps the page at its offset on ordinary pages and on pages whose <body> sets its own overflow, and the shadow-root and stuck-clip cases are fixed. One regression remains: MobileNav still writes document.documentElement.style.overflow itself, so when it closes or unmounts while another overlay is open, for example on a click that closes it and opens a Dialog rendered before it, it clears the lock and lets wheel and keyboard scroll the page behind that overlay, which main prevents today. Please move MobileNav onto claimRootScrollStyle in this PR, or land that part of #8119 with it, with tests for a close and an unmount while another lock holds.
- The touch guard fixes a pan's axis from its first
touchmove, so a horizontal swipe that starts with a tie or a vertical lean (for example 2 px by 2 px) is cancelled even when the strip under the finger can scroll; deciding after a small slop would avoid that.
[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.
This claim now shares the root's inline overflow with MobileNav, which still writes it directly (MobileNav.tsx:583, :601). A click that closes MobileNav and opens a Dialog rendered before it in the tree, a later close while an overlay opened from it stays open, or the unmount of a closed MobileNav while any overlay is open sets it back to '' while this lock still counts as held, so wheel and keyboard scroll the page behind the overlay; main keeps <body> pinned in each case. Moving MobileNav onto claimRootScrollStyle({overflow: 'clip'}) here (the change in #8119) fixes both.
There was a problem hiding this comment.
Fixed in 55ad273: MobileNav's clip now goes through claimRootScrollStyle, so closing or unmounting it leaves another overlay's lock in place. MobileNavRootScrollClaim.test.tsx covers a close and an unmount while a lock holds; both fail on the previous MobileNav.
| if (view == null) { | ||
| return false; | ||
| } | ||
| const isVertical = Math.abs(dy) >= Math.abs(dx); |
There was a problem hiding this comment.
The axis comes from the first non-zero move, a tie going vertical, and the verdict then holds for the whole touch. A horizontal swipe whose first touchmove is (2, 2) over a strip that can scroll, inside an overlay where nothing scrolls vertically, is cancelled together with the rest of the swipe, while (5, 0) passes. Deciding the axis once the move clears a small slop would avoid that.
There was a problem hiding this comment.
Fixed in 55ad273: the gesture host now judges a pan only once it clears 5 px (still under WebKit's scroll claim), so a (2, 2) first move no longer fixes the axis. A lock-level test swipes a horizontal strip after that wobble and fails on the previous head.
…held while a Dialog is open; URL text filled)
6eee897 to
0455794
Compare
|
Done in 55ad273: the guard's own document listeners are gone. useScrollLock holds the page through the gesture host's |
github-actions
Bot
removed
the
needs:design-review
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about MobileNav FR3, which says the page's root clips its overflow while the menu is open, whereas this change intentionally clips <body> on pages whose body owns overflow: an engineering owner records that decision by commenting /approve-spec e979167d93c0f0a2155a1730820e4c189d9d25c9 on this pull request with FR3 amended to match (you may do so yourself as an engineering owner), or MobileNav keeps the root clip. Whatever that decision, these changes are still needed:
- Land #8320 and the pull requests below it, then rebase so this pull request carries only its own change (today it brings 26,176 authored lines, including a superseded copy of #8320 that conflicts with
main), updatingBottomSheetReplace.test.tsx'sbody.style.positioncheck to observe the new lock and moving the MobileNav part out with the FR3 decision to get under 2,000 lines. - Attach real-device recordings of the iOS Safari (before and after 26.4) and Android Chrome behavior the change claims, as
spec:AST-094's platform support requires, and add MobileNav to the size report.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about MobileNav FR3, which says the page's root clips its overflow while the menu is open, whereas this change intentionally clips <body> on pages whose body owns overflow: an engineering owner records that decision by commenting /approve-spec 280d7758775c7852933994b98f3913bc54dc3a62 on this pull request with FR3 amended to match (you may do so yourself as an engineering owner), or MobileNav keeps the root clip. Whatever that decision, these changes are still needed:
- Land #8320 and the pull requests below it, then rebase so this pull request carries only its own change (today it brings 26,385 authored lines, including a copy of #8320 that its head has already moved past), and move the MobileNav part out with the FR3 decision to get under 2,000 lines.
- Make the
body.style.positioncheck atBottomSheetReplace.test.tsx:1346observe the new lock (for example the root'soverflow), since it can no longer fail. - Attach real-device recordings of the iOS Safari (before and after 26.4) and Android Chrome behavior the change claims, as
spec:AST-094's platform support requires.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about MobileNav FR3, which says the page's root clips its overflow while the menu is open, whereas this change intentionally clips <body> on pages whose body owns overflow: an engineering owner records that decision by commenting /approve-spec 27dd234d4f037084a581a88c82607507a1f74b0c on this pull request with FR3 amended to match (you may do so yourself as an engineering owner), or MobileNav keeps the root clip. Whatever that decision, these changes are still needed:
- Land #8320 and the pull requests below it, then rebase so this pull request carries only its own change (today it brings 26,627 authored lines, 24,601 of them #8320's), and bring that change's own 2,035 lines under 2,000.
- Make the
body.style.positioncheck atBottomSheetReplace.test.tsx:1346observe the new lock (for exampleisScrollLockActive()or the root'soverflow), since it can no longer fail. - Attach real-device recordings of the iOS Safari (before and after 26.4) and Android Chrome behavior the change claims, as
spec:AST-094's platform support requires.
[Automated review]
| // overflow: clip avoids creating a scroll container (unlike hidden), | ||
| // so there's no scroll bounce and no need to save/restore scroll position. | ||
| document.documentElement.style.overflow = 'clip'; | ||
| releaseRootStyleRef.current ??= claimRootScrollStyle({overflow: 'clip'}); |
There was a problem hiding this comment.
On a page whose <body> sets its own overflow, this claim clips <body> rather than the root, which MobileNav FR3 ("the page's root clips its overflow") does not allow as written. It needs an engineering owner's FR3 amendment, or a root target here.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about MobileNav FR3, which says the page's root clips its overflow while the menu is open, whereas this change intentionally clips <body> on pages whose body owns overflow: an engineering owner records that decision by commenting /approve-spec 3d23397a0afa85a5c3e6cb5be5ee918bedd044d6 on this pull request with FR3 amended to match (you may do so yourself as an engineering owner), or MobileNav keeps the root clip. Whatever that decision, these changes are still needed:
- Land #8320 and the pull requests below it, then rebase so this pull request carries only its own change (today it brings 26,677 authored lines, 24,651 of them #8320's), and bring that change's own 2,035 lines under 2,000.
- Make the
body.style.positioncheck atBottomSheetReplace.test.tsx:1346observe the new lock (for exampleisScrollLockActive()or the root'soverflow), since it can no longer fail. - Attach real-device recordings of the iOS Safari (before and after 26.4) and Android Chrome behavior the change claims, as
spec:AST-094's platform support requires.
[Automated review]
|
On the review of 3d23397:
|
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision: on pages whose <body> sets its own overflow, MobileNav now clips <body> instead of the root, which intentionally changes MobileNav FR3 ("the page's root clips its overflow"), so FR3 needs updating for that change, or the root clip has to stay. An engineering owner records the decision to update FR3 by commenting /approve-spec b19ca67832bad2ff3c92bf90b0b5b96128403757 on this pull request, and the author may do so as an engineering owner. Whatever is decided, two changes are needed:
- Land #8320 first and rebase, so this pull request carries only its own scroll-lock change (26,859 authored lines now, 2,038 of them its own), then keep it under 2,000 lines or explain why it cannot be split.
- Attach real-device recordings (a real iPhone, one of them before iOS 26.4, and a real Android phone in Chrome) for the iOS Safari and Android Chrome claims, as the touch presentations spec's platform support section requires, or drop those claims.
[Automated review]
| // overflow: clip avoids creating a scroll container (unlike hidden), | ||
| // so there's no scroll bounce and no need to save/restore scroll position. | ||
| document.documentElement.style.overflow = 'clip'; | ||
| releaseRootStyleRef.current ??= claimRootScrollStyle({overflow: 'clip'}); |
There was a problem hiding this comment.
On a page whose <body> sets its own overflow (root left visible), this claim lands on <body>, not the root, while MobileNav FR3 says "the page's root clips its overflow". That is the intended change the hold is about: it needs FR3 updated by an engineering owner, or the root clip kept.
There was a problem hiding this comment.
This pull request is on hold for an engineering-owner decision on two lock behaviors the current specs do not state. On pages whose <body> sets its own overflow, MobileNav now clips <body> instead of the root, which intentionally changes MobileNav FR3 ("the page's root clips its overflow"), so FR3 needs updating for that change or the root clip has to stay; and the lock now turns off pull-to-refresh behind every modal overlay, which no current record covers. An engineering owner records the decision to accept both, with FR3 updated to match, by commenting /approve-spec bd86f9d3b39562dcd3e3bede9d03c554f148297f on this pull request, and the author may do so as an engineering owner. Whatever is decided, these changes are still needed:
- Land #8320 first and rebase, so this pull request carries only its own scroll-lock change (26,917 authored lines today, 2,096 of them its own), then keep that under 2,000 lines or explain why it cannot be split.
- Make the gesture host attach for every pointer before it lands; the current touch-presentations spec (FR11, FR19, FR43, DEC-21) and the React runtime record's INV13 require it, and
GestureHost.tsstays inactive unless the primary pointer is coarse. - Fix
scripts/touch-support-bundle.test.mjs, whose root- and hooks-entry cases fail now thatuseScrollLockimportsgestures/pageHold, in this pull request or by landing #8671 first. - Attach real-device recordings for the iOS Safari and Android Chrome claims, as the touch-presentations spec's platform support requires: a real iPhone (one before iOS 26.4, including the first scrim drag after an overlay opens and a pinch that starts after a held one-finger drag) and a real Android phone in Chrome.
[Automated review]
| // overflow: clip avoids creating a scroll container (unlike hidden), | ||
| // so there's no scroll bounce and no need to save/restore scroll position. | ||
| document.documentElement.style.overflow = 'clip'; | ||
| releaseRootStyleRef.current ??= claimRootScrollStyle({overflow: 'clip'}); |
There was a problem hiding this comment.
On a page whose <body> sets its own overflow while the root stays visible, this claim lands on <body> (overflowTarget, packages/core/src/hooks/rootScrollStyles.ts:82-97), while MobileNav FR3 says "the page's root clips its overflow". Either FR3 is amended to name the element the viewport takes its overflow from, which an engineering owner records with /approve-spec on this pull request's head, or MobileNav keeps clipping the root.
| */ | ||
|
|
||
| import {useEffect} from 'react'; | ||
| import {claimRootScrollStyle, reportScrollLock} from './rootScrollStyles'; | ||
| import {holdPageScroll} from '../gestures/pageHold'; |
There was a problem hiding this comment.
This import (with scrollLockTouchGuard.ts:30's gestures/nativeScroll) puts packages/core/src/gestures/ code into the hooks and root entries, so scripts/touch-support-bundle.test.mjs fails its root- and hooks-entry cases (lines 103-112), which CI's node project collects. Carry the guard's update here or land #8671 first.
| * ONLY WHERE THE PRIMARY POINTER IS COARSE | ||
| * | ||
| * - Recognizers attach, and the host registers listeners, only while the | ||
| * primary pointer is coarse (`spec:AST-094` FR19, FR24; coarsePointer.ts). | ||
| * Where it is fine, an attach is recorded but nothing is installed: no | ||
| * document, window or element listener, no `touch-action` or other style, | ||
| * nothing delivered, so a finger on a touchscreen laptop gets the | ||
| * browser's defaults. The host follows the query: turning coarse installs | ||
| * it and styles every registration; turning fine cancels every touch in | ||
| * flight and removes it all. A development warning names the first attach | ||
| * under a fine pointer. |
There was a problem hiding this comment.
Current spec:AST-094 FR19 ("Recognizers attach for every pointer, with FR43's exceptions"), FR43, DEC-21 and architecture:react-component-runtime INV13 ("The gesture host attaches for every pointer, with no touch-only query") reverse the coarse-only rule this header and :503-507 implement; the merge-base text said coarse-only. Attach for every pointer with FR43's listed exceptions where #8320 lands.
There was a problem hiding this comment.
Approval of this pull request waits on an engineering owner, because two scroll-lock behaviors it wants go beyond the current specs: on pages whose <body> sets its own overflow, MobileNav now clips <body> instead of the root, which intentionally changes MobileNav FR3 ("the page's root clips its overflow"), so FR3 needs updating for that change or the root clip has to stay; and the lock now turns off pull-to-refresh behind every modal overlay, which no current spec covers, so an owner needs to provide or identify the spec for it. An engineering owner records the decision to accept both, with FR3 updated to match, by commenting /approve-spec 5d9fc1af8233c42ca9fe642b24ebde33ad956ef1 on this pull request, and the author may do so as an engineering owner. Whatever is decided, these changes are still needed:
- Land #8320 first and rebase, so this pull request carries only its own scroll-lock change (29,324 authored lines today, 2,096 of them its own), then keep that under 2,000 lines or explain why it cannot be split.
- Make the gesture host attach for every pointer before it lands, as the touch-presentations spec (FR11, FR19, FR43, DEC-21) and the React runtime record's INV13 require;
GestureHost.tsstays inactive unless the primary pointer is coarse. - Fix
scripts/touch-support-bundle.test.mjs, whose root- and hooks-entry cases fail now thatuseScrollLockimportsgestures/pageHold, here or by landing #8671 first. - Attach real-device recordings for the iOS Safari and Android Chrome claims, as the touch-presentations spec's platform support requires, including the first scrim drag after an overlay opens on an iPhone before iOS 26.4 and a pinch that follows a held one-finger drag.
[Automated review]
| // overflow: clip avoids creating a scroll container (unlike hidden), | ||
| // so there's no scroll bounce and no need to save/restore scroll position. | ||
| document.documentElement.style.overflow = 'clip'; | ||
| releaseRootStyleRef.current ??= claimRootScrollStyle({overflow: 'clip'}); |
There was a problem hiding this comment.
On a page whose <body> sets its own overflow while the root stays visible, this claim lands on <body> (overflowTarget, rootScrollStyles.ts:82-97), while MobileNav FR3 says "the page's root clips its overflow". The amendment asked for in the question comment needs an engineering owner's /approve-spec on this head, with FR3's text updated to match; until then this stays on hold.
| // the overlay. | ||
| const releaseRootStyle = claimRootScrollStyle({ | ||
| overflow: 'hidden', | ||
| overscrollBehaviorY: 'none', |
There was a problem hiding this comment.
overscroll-behavior-y: none on the root and <body> turns off Chrome's pull-to-refresh, and the page's own bounce, behind every modal overlay. No current record states that (BottomSheetSwitcher FR5 says only that the page does not scroll), so it waits on the same engineering-owner decision as the MobileNav FR3 amendment.
|
|
||
| import {useEffect} from 'react'; | ||
| import {claimRootScrollStyle, reportScrollLock} from './rootScrollStyles'; | ||
| import {holdPageScroll} from '../gestures/pageHold'; |
There was a problem hiding this comment.
This import (with scrollLockTouchGuard.ts:30's ../gestures/nativeScroll) puts packages/core/src/gestures/ code into the hooks and root entries, so scripts/touch-support-bundle.test.mjs:104-112 fails its root- and hooks-entry cases. Carry the guard's update in this pull request, or land #8671 first and rebase.
| * ONLY WHERE THE PRIMARY POINTER IS COARSE | ||
| * | ||
| * - Recognizers attach, and the host registers listeners, only while the | ||
| * primary pointer is coarse (`spec:AST-094` FR19, FR24; coarsePointer.ts). |
There was a problem hiding this comment.
Current spec:AST-094 FR19 ("Recognizers attach for every pointer, with FR43's exceptions"), FR43, DEC-21 and architecture:react-component-runtime INV13 ("The gesture host attaches for every pointer, with no touch-only query") reverse this coarse-only rule, which isActive and isConsumerActive (:909-921) follow. Conform it where #8320 lands, keeping FR43's listed exceptions.
| if (!isCoarsePointer(view)) { | ||
| return; | ||
| } | ||
| listen(doc, holds, true); |
There was a problem hiding this comment.
For an overlay's first contact, this adds the non-passive touchmove listener during that contact's pointerdown. The gestures README records that Safari ignores a listener added during a touch's touchstart for that touch (README.md:577-578), so a real-iPhone recording before iOS 26.4 of the first scrim drag after an overlay opens is what shows whether that drag is held.
Three recognizers beside main's tap, long press and pan, each reading its own UIKit table group: a swipe recognized from the path between events, a pinch whose velocity settles, and a screen-edge pan from the leading or trailing edge. The edge pan takes the horizontal axis only on a view that lies within that edge's band. The sides its edges resolve to under the view's writing direction and the edge zone are measured once, together, for touch-action and the touch-down decision alike, and again when the host reports a layout change; and it is a finger's only (a mouse, a trackpad or a pen never starts it). meta#8061's change, rebased onto main as one commit; its rotation and hover recognizers and its top and bottom edges, which no component uses yet (and rotation's threshold is unmeasured), land with their first users. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
… the gesture host The browser negotiation and the gesture host on top of the recognizers: touch-action derived from what is attached and never wider than the element's own (spec:AST-094 FR18, DEC-8), a native scroll probe that follows a slotted node into its shadow tree, the tap haptic, and the host that claims a finger, suppresses the click a claimed touch would send, and hands a touch to the browser when it takes over. The host serves a mouse, a pen and a finger alike (FR19, FR43). Where the primary pointer is fine, an element that uses a gesture keeps one passive pointerdown listener and the page has none until a press, whose listeners go once its contacts end (FR24); a right click, a pen's barrel button and a ctrl-click on macOS start no press. It gives a screen-edge pan its view's box and, through one ResizeObserver and no listener, reports a resize of that box or of the viewport, so the pan's edge-zone answer and the touch-action derived from it follow the layout (FR18, FR19, FR24, DEC-22). Recognizers report their kind, which the host and the touch-action rules read instead of checking a class (architecture:react-component-runtime INV13). meta#8071's and meta#8072's changes, with meta#8665's serving of every pointer, rebased onto main after meta#8061 as one commit; the host's hover routing lands with the hover recognizer's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…mator Core's internal touch foundation on the gesture recognizers (meta#8061) and the gesture host (meta#8071 + meta#8072): UIControl press tracking with its press profiles and hooks, built by createPressTracker; and the animator, which starts a motion from rest at its commit, holds the value on screen inline while a curve runs, and lets a call wait for a presented frame. It absorbs meta#8073 (closed: the press tracking this rests on) and meta#8340 (the animator's start and hold fixes), with the held dip's load fixes, and keeps meta#8641's rule: a gesture that cancels itself while its finger moves keeps that finger until it lifts. A gesture that ends on an unmount or a replaced node calls no listener from the effect. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
meta#8360's change on the internal touch foundation: one click guard shared by the menu press and the gesture host (its click listener stays once the menu press arms it, as its record has it, and lasts only while armed for the gesture host, FR24), the host as the one touch owner (finger scroll, page holds, attachments, the press tracker hook and a held control's kept touch, the long press's single source, presented hit testing on iOS WebKit), and the gesture browser proof in Chromium and WebKit with the README. It carries meta#8074, meta#8270, meta#8293, meta#8315, meta#8320 and meta#8356, and folds in meta#8358 (the long press's tolerance comment and an unused import). Rebased onto section 2 as one commit; the touch-action rule now comes with meta#8071, and meta#8072's claimed-touchend fix, which this line never had, comes back with meta#8309. The animator's busy proof is judged against the harness's compositor-only control (meta#8906), with a main-thread mutant as its negative, and the spring's curve is placed against the control's own clock, so the late frames a loaded runner sends fail nothing. With the host serving every pointer (meta#8071), this change's own pieces follow it: a press tracker takes a mouse press as a press, painting at once and braking no scroll (FR11); a finger's or a pen's hold that does something keeps text unselected under any primary pointer, while a mouse keeps its released behavior (FR41); a claimed pen's stylus touchend is cancelled as a finger's (FR21); and `perEventPointer`, from before FR43, changes nothing. These come from meta#8665, which keeps only the undecided rule for text selection during a mouse or pen drag. The hover recognizer and the host's hover routing land with hover's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
The scroll lock no longer pins <body>, which reset the page's offset to 0 (the page jumped and Safari's toolbar came back). It holds `overflow: hidden` on the root and, through the gesture host's page hold, cancels only a one-finger pan that nothing inside the overlay takes: a scroller, a range input, a pinch or a zoomed page's pan still goes through. The root's scroll styles become claim stacks (rootScrollStyles), so a lock and a pull to refresh compose. The top-layer check asks CSS.supports for `:modal` and `:popover-open` instead of catching errors from `matches`. The touch module bundle test names useScrollLock and the imports that reach it as carrying the page hold.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on two scroll-lock behaviors that current spec does not cover: MobileNav FR3 says the page's root clips its overflow while the drawer is open, but on pages whose <body> sets its own overflow this change clips <body> instead (FR3 would need amending), and the lock's overscroll-behavior-y: none turns off pull-to-refresh behind every modal overlay, which no current record states. An engineering owner records that decision by commenting /approve-spec 70e82f39fe9a19dae99a3916e7ae8f7c2751d441 on this pull request (the author may do so as an engineering owner), or asks for the root clip to stay and pull-to-refresh to be left alone. Whatever is decided, the change still needs:
- a rebase onto a landed #8360, so the diff carries only the scroll-lock commit, and that commit's 2,122 changed lines brought under 2,000 or a decomposition showing why it cannot be split;
- real-device recordings for the iOS Safari and Android Chrome claims, above all the first scrim drag after an overlay opens on iOS before 26.4, since the page hold adds its blocking
touchmoveduring that touch (packages/core/src/gestures/pageHold.ts:84-108).
[Automated review]
| // overflow: clip avoids creating a scroll container (unlike hidden), | ||
| // so there's no scroll bounce and no need to save/restore scroll position. | ||
| document.documentElement.style.overflow = 'clip'; | ||
| releaseRootStyleRef.current ??= claimRootScrollStyle({overflow: 'clip'}); |
There was a problem hiding this comment.
On a page whose <body> sets its own overflow while the root stays visible, this claim lands on <body> (overflowTarget, packages/core/src/hooks/rootScrollStyles.ts:82-97), while MobileNav FR3 says "the page's root clips its overflow". It needs FR3 amended through an engineering owner's /approve-spec on this revision, or a root target here.
| // the overlay. | ||
| const releaseRootStyle = claimRootScrollStyle({ | ||
| overflow: 'hidden', | ||
| overscrollBehaviorY: 'none', |
There was a problem hiding this comment.
overscroll-behavior-y: none on the root and <body> turns off Chrome's pull-to-refresh, and the page's own bounce, behind every modal overlay. No current record states that (BottomSheetSwitcher FR5 says only that the page does not scroll), so it waits on the same engineering-owner decision as the MobileNav clip.
| if (!isCoarsePointer(view)) { | ||
| return; | ||
| } | ||
| listen(doc, holds, true); |
There was a problem hiding this comment.
For an overlay's first contact this adds the blocking touchmove during that contact's pointerdown, while the gestures README says Safari honours a blocking listener only when it was registered before the touch (packages/core/src/gestures/README.md:241-243). A real-iPhone recording before iOS 26.4 of the first scrim drag after opening would show whether that drag is held; registering the hold's listeners when the lock starts under a coarse pointer would remove the doubt.
Depends on #8360 (the gesture host's page hold). Review only this PR's change: vjeux/astryx@fccb690...70e82f3
Why
Opening a Dialog, BottomSheet, Drawer or Lightbox on a scrolled page moved the page: the scroll lock pinned
<body>, soscrollYread 0, the page jumped, and Safari's toolbar came back.What led to it
Found while checking overlays on iOS for the touch-presentations program (
spec:AST-094). The pin exists because iOS Safari before 26.4 scrolls anoverflow: hiddenpage under a finger; pinning stops that but resets the offset to 0, which Safari reads as a return to the top.Options and tradeoffs
overflow: hiddenand cancel only the one-finger pans that would reach the page: keeps the offset and the toolbar. The cost: while a lock holds, those pans wait on script.Why this choice
The second keeps the page where it was. Anything inside the overlay that takes a pan still gets it: a scroller, a range slider, a pinch, a zoomed page's pan. Its touch listener exists only while a lock holds. No spec governs the pin (AST-017 FR3 calls it implementation);
useScrollLock's docs promise to keep the scroll position.How to check it
On the iOS simulator in Safari, or a phone:
Evidence
iOS 26.2 simulator, main and this PR:
Android 15 emulator, then with blur on:
A Dialog inside a scroller, previous head and this PR:
useScrollLockand AlertDialog, BottomSheet, CommandPalette, ContextMenu, DateInput, DateTimeInput, Dialog, Drawer, DropdownMenu, Layer, Lightbox, Markdown, MoreMenu, MultiSelector, Pagination, Popover, PowerSearch, Selector, Table, TimeInput and Tokenizer. Against fix(core): one click guard for the menu press model and the gesture host #8360, each grows 1.6–2.2 KB gzip JavaScript (useScrollLock0.6 → 2.8 KB), shared, so a page pays it once; CSS is unchanged.cache.test.mjs(npm lock) andstory-tree.test.ts, which fix(cli): a crashed run's release install is recognized by the Node it ran on, whatever its name #8520 and test(storybook): run the story-tree check where Storybook is built, and bring the tree back to its shape #8509 fix.