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.
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
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 |
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.
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.
|
Folded in the four points: MobileNav FR3 has its clip follow the page's scroller (the root, or AlertDialog's layout tests stub the page hold, whose pointer read is the lock's, so their guard still sees only the layout's queries. |
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the three current-record amendments this change makes: MobileNav FR3 would have the drawer clip <body> instead of the root on pages whose <body> sets its own overflow, and BottomSheetSwitcher FR5 and Drawer FR5 would add that pull-to-refresh and the page's bounce stop behind a modal overlay, which current spec does not state. An engineering owner records that decision by commenting /approve-spec 4721503acd64d4a362cfe8c8a027036f5763a499 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, before approval the change still needs:
- a rebase onto a landed #8360, so the diff carries only the scroll-lock commit;
- real-device recordings for the iOS Safari and Android Chrome claims (an iPhone below iOS 26.4 dragging the scrim right after an overlay opens, a drag that becomes a pinch, and an Android phone pulling down at the top), which AST-094's platform support asks for.
[Automated review]
| @@ -136,7 +136,7 @@ Consumer migration instructions belong in consumer docs and release notes. | |||
| | ---- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------- | | |||
| | FR1 | MobileNav renders one native `<dialog>` Navigation overlay containing a Drawer with a stable Header, Close button, and scrollable Content region. The overlay carries `mobile-nav` with the resolved `side`; Drawer, Header, and Content carry no MobileNav public target. | Current source, docs, and tests | | |||
| | FR2 | `isOpen` and `onOpenChange` win when given; otherwise the drawer reads and changes the enclosing AppShell's drawer state. The dialog uses AppShell's drawer id when there is one, else its own; a caller `id` replaces either (Current-state impact). | Current source and tests | | |||
| | FR3 | Opening calls `showModal()`, and records the focused element as the opener; where focus lands inside the open drawer is uncontracted here. While open, the page's root clips its overflow and keeps its scrollbar gutter; closing releases both. | Current source and tests | | |||
| | FR3 | Opening calls `showModal()`, and records the focused element as the opener; where focus lands inside the open drawer is uncontracted here. While open, the page's scroller clips its overflow and keeps its scrollbar gutter: the root, or `<body>` when `<body>` sets its own overflow and the root's stays `visible`; closing releases both. | Current source and tests | | |||
There was a problem hiding this comment.
This rewrites current FR3 ("the page's root clips its overflow") so the drawer clips <body> when <body> sets its own overflow, which MobileNav.tsx:597 already does through rootScrollStyles.ts:82-97. It needs an engineering owner's /approve-spec at this head before it can stand.
| @@ -143,7 +143,7 @@ Consumer migration instructions belong in consumer docs and release notes. | |||
| | FR2 | Each child sheet owns one native dialog. Opening a sheet presents its dialog once; a handoff presents the entering sheet's dialog above the retained one and closes the retained dialog only after its panel has faded. Returning to a sheet whose dialog is still open beneath a newer one re-presents it so the interactive sheet is the topmost layer. | DEC-1 | | |||
| | FR3 | During a handoff, the entering sheet is above the retained sheet. A taller retained sheet aligns down to a shorter entering sheet, and the retained sheet fades only after required transform motion completes. Under reduced motion the handoff follows `design:reduced-motion`: no transform motion runs, and the sheets cross-fade (DR2) or change at once (DR3) in the same order. | Current source and transition tests; `design:reduced-motion` DR2, DR3 | | |||
| | FR4 | Closing retains the outgoing sheet through its exit, then closes the dialog. A modal flow restores focus to the element captured when that modal flow opened. | Current source and focus tests | | |||
| | FR5 | `hasScrim=true` uses `showModal()`, a native backdrop, and native focus containment, and locks page scroll: the page does not scroll while a modal flow is presented; the lock is held continuously across a handoff, never released and re-acquired between steps; it still holds beneath a `Dialog` opened from a step; it is released after the flow's exit (DEC-1). `hasScrim=false` uses `show()` without a backdrop or scroll lock and leaves the page interactive. In a non-modal flow the sheet dialogs order themselves with local values inside the switcher's isolated flow wrapper (DEC-1); the page-level value that wrapper carries remains a `spec:AST-027` conformance gap. | DEC-1; `spec:AST-027` FR2, FR4, FR8 | | |||
| | FR5 | `hasScrim=true` uses `showModal()`, a native backdrop, and native focus containment, and locks page scroll: the page does not scroll while a modal flow is presented, and pull-to-refresh and the page's bounce stop behind it (`overscroll-behavior-y: none`), since the page is held under a modal presentation (`spec:AST-094` FR19), as behind a UIKit modal presentation; the lock is held continuously across a handoff, never released and re-acquired between steps; it still holds beneath a `Dialog` opened from a step; it is released after the flow's exit (DEC-1). `hasScrim=false` uses `show()` without a backdrop or scroll lock and leaves the page interactive. In a non-modal flow the sheet dialogs order themselves with local values inside the switcher's isolated flow wrapper (DEC-1); the page-level value that wrapper carries remains a `spec:AST-027` conformance gap. | DEC-1; `spec:AST-027` FR2, FR4, FR8 | | |||
There was a problem hiding this comment.
The added pull-to-refresh and bounce stop (overscroll-behavior-y: none, useScrollLock.ts:81-84) is new direction: AST-094 FR19 lets a modal scroll lock cancel a lone finger's page pan but says nothing about overscroll. The same text in Drawer.spec.md:163 waits on the same engineering-owner decision.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the three current-record amendments this change makes: MobileNav FR3 would have the drawer clip <body> instead of the root on pages whose <body> sets its own overflow, and BottomSheetSwitcher FR5 and Drawer FR5 would add that pull-to-refresh and the page's bounce stop behind a modal overlay, which current spec does not state. An engineering owner records that decision by commenting /approve-spec 423aee10cdc73339f4d081587540632fdc6e1700 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, before approval the change still needs:
- a rebase onto a landed #8360, so the diff carries only the scroll-lock commit and merges cleanly again (it now conflicts with the base in
packages/core/src/gestures/GestureHost.ts); - real-device recordings for the iOS Safari and Android Chrome claims (an iPhone below iOS 26.4 dragging the scrim right after an overlay opens, a drag that becomes a pinch, and an Android phone pulling down at the top), which AST-094's platform support asks for.
[Automated review]
| @@ -136,7 +136,7 @@ Consumer migration instructions belong in consumer docs and release notes. | |||
| | ---- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------- | | |||
| | FR1 | MobileNav renders one native `<dialog>` Navigation overlay containing a Drawer with a stable Header, Close button, and scrollable Content region. The overlay carries `mobile-nav` with the resolved `side`; Drawer, Header, and Content carry no MobileNav public target. | Current source, docs, and tests | | |||
| | FR2 | `isOpen` and `onOpenChange` win when given; otherwise the drawer reads and changes the enclosing AppShell's drawer state. The dialog uses AppShell's drawer id when there is one, else its own; a caller `id` replaces either (Current-state impact). | Current source and tests | | |||
| | FR3 | Opening calls `showModal()`, and records the focused element as the opener; where focus lands inside the open drawer is uncontracted here. While open, the page's root clips its overflow and keeps its scrollbar gutter; closing releases both. | Current source and tests | | |||
| | FR3 | Opening calls `showModal()`, and records the focused element as the opener; where focus lands inside the open drawer is uncontracted here. While open, the page's scroller clips its overflow and keeps its scrollbar gutter: the root, or `<body>` when `<body>` sets its own overflow and the root's stays `visible`; closing releases both. | Current source and tests | | |||
There was a problem hiding this comment.
This rewrites current FR3 ("the page's root clips its overflow") so the drawer clips <body> when <body> sets its own overflow, which MobileNav.tsx:597 already does through rootScrollStyles.ts:82-97. It needs an engineering owner's /approve-spec at this head before it can stand; PR1 below still says "the root overflow".
| @@ -143,7 +143,7 @@ Consumer migration instructions belong in consumer docs and release notes. | |||
| | FR2 | Each child sheet owns one native dialog. Opening a sheet presents its dialog once; a handoff presents the entering sheet's dialog above the retained one and closes the retained dialog only after its panel has faded. Returning to a sheet whose dialog is still open beneath a newer one re-presents it so the interactive sheet is the topmost layer. | DEC-1 | | |||
| | FR3 | During a handoff, the entering sheet is above the retained sheet. A taller retained sheet aligns down to a shorter entering sheet, and the retained sheet fades only after required transform motion completes. Under reduced motion the handoff follows `design:reduced-motion`: no transform motion runs, and the sheets cross-fade (DR2) or change at once (DR3) in the same order. | Current source and transition tests; `design:reduced-motion` DR2, DR3 | | |||
| | FR4 | Closing retains the outgoing sheet through its exit, then closes the dialog. A modal flow restores focus to the element captured when that modal flow opened. | Current source and focus tests | | |||
| | FR5 | `hasScrim=true` uses `showModal()`, a native backdrop, and native focus containment, and locks page scroll: the page does not scroll while a modal flow is presented; the lock is held continuously across a handoff, never released and re-acquired between steps; it still holds beneath a `Dialog` opened from a step; it is released after the flow's exit (DEC-1). `hasScrim=false` uses `show()` without a backdrop or scroll lock and leaves the page interactive. In a non-modal flow the sheet dialogs order themselves with local values inside the switcher's isolated flow wrapper (DEC-1); the page-level value that wrapper carries remains a `spec:AST-027` conformance gap. | DEC-1; `spec:AST-027` FR2, FR4, FR8 | | |||
| | FR5 | `hasScrim=true` uses `showModal()`, a native backdrop, and native focus containment, and locks page scroll: the page does not scroll while a modal flow is presented, and pull-to-refresh and the page's bounce stop behind it (`overscroll-behavior-y: none`), since the page is held under a modal presentation (`spec:AST-094` FR19), as behind a UIKit modal presentation; the lock is held continuously across a handoff, never released and re-acquired between steps; it still holds beneath a `Dialog` opened from a step; it is released after the flow's exit (DEC-1). `hasScrim=false` uses `show()` without a backdrop or scroll lock and leaves the page interactive. In a non-modal flow the sheet dialogs order themselves with local values inside the switcher's isolated flow wrapper (DEC-1); the page-level value that wrapper carries remains a `spec:AST-027` conformance gap. | DEC-1; `spec:AST-027` FR2, FR4, FR8 | | |||
There was a problem hiding this comment.
The added pull-to-refresh and bounce stop (overscroll-behavior-y: none, useScrollLock.ts:81-84) is new direction: AST-094 FR19 lets a modal scroll lock cancel a lone finger's page pan but says nothing about overscroll. The same text in Drawer.spec.md:163 waits on the same engineering-owner decision.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the three current-record amendments this change makes: MobileNav FR3 would have the drawer clip <body> instead of the root on pages whose <body> sets its own overflow, and BottomSheetSwitcher FR5 and Drawer FR5 would add that pull-to-refresh and the page's bounce stop behind a modal overlay, which current spec does not state. An engineering owner records that decision by commenting /approve-spec 3e964de1701e0abd7cdae882ea4688957bc55a08 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, before approval the change still needs:
- a rebase onto a landed #8360, so the diff carries only the scroll-lock commit;
- real-device recordings for the iOS Safari and Android Chrome claims (an iPhone below iOS 26.4 dragging the scrim right after an overlay opens, a drag that becomes a pinch, and an Android phone pulling down at the top), which AST-094's platform support asks for.
[Automated review]
| @@ -136,7 +136,7 @@ Consumer migration instructions belong in consumer docs and release notes. | |||
| | ---- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------- | | |||
| | FR1 | MobileNav renders one native `<dialog>` Navigation overlay containing a Drawer with a stable Header, Close button, and scrollable Content region. The overlay carries `mobile-nav` with the resolved `side`; Drawer, Header, and Content carry no MobileNav public target. | Current source, docs, and tests | | |||
| | FR2 | `isOpen` and `onOpenChange` win when given; otherwise the drawer reads and changes the enclosing AppShell's drawer state. The dialog uses AppShell's drawer id when there is one, else its own; a caller `id` replaces either (Current-state impact). | Current source and tests | | |||
| | FR3 | Opening calls `showModal()`, and records the focused element as the opener; where focus lands inside the open drawer is uncontracted here. While open, the page's root clips its overflow and keeps its scrollbar gutter; closing releases both. | Current source and tests | | |||
| | FR3 | Opening calls `showModal()`, and records the focused element as the opener; where focus lands inside the open drawer is uncontracted here. While open, the page's scroller clips its overflow and keeps its scrollbar gutter: the root, or `<body>` when `<body>` sets its own overflow and the root's stays `visible`; closing releases both. | Current source and tests | | |||
There was a problem hiding this comment.
This rewrites current FR3 ("the page's root clips its overflow") so the drawer clips <body> when <body> sets its own overflow, which MobileNav.tsx:597 already does through rootScrollStyles.ts:82-97. It needs an engineering owner's /approve-spec at this head before it can stand; PR1 below still says "the root overflow".
| @@ -143,7 +143,7 @@ Consumer migration instructions belong in consumer docs and release notes. | |||
| | FR2 | Each child sheet owns one native dialog. Opening a sheet presents its dialog once; a handoff presents the entering sheet's dialog above the retained one and closes the retained dialog only after its panel has faded. Returning to a sheet whose dialog is still open beneath a newer one re-presents it so the interactive sheet is the topmost layer. | DEC-1 | | |||
| | FR3 | During a handoff, the entering sheet is above the retained sheet. A taller retained sheet aligns down to a shorter entering sheet, and the retained sheet fades only after required transform motion completes. Under reduced motion the handoff follows `design:reduced-motion`: no transform motion runs, and the sheets cross-fade (DR2) or change at once (DR3) in the same order. | Current source and transition tests; `design:reduced-motion` DR2, DR3 | | |||
| | FR4 | Closing retains the outgoing sheet through its exit, then closes the dialog. A modal flow restores focus to the element captured when that modal flow opened. | Current source and focus tests | | |||
| | FR5 | `hasScrim=true` uses `showModal()`, a native backdrop, and native focus containment, and locks page scroll: the page does not scroll while a modal flow is presented; the lock is held continuously across a handoff, never released and re-acquired between steps; it still holds beneath a `Dialog` opened from a step; it is released after the flow's exit (DEC-1). `hasScrim=false` uses `show()` without a backdrop or scroll lock and leaves the page interactive. In a non-modal flow the sheet dialogs order themselves with local values inside the switcher's isolated flow wrapper (DEC-1); the page-level value that wrapper carries remains a `spec:AST-027` conformance gap. | DEC-1; `spec:AST-027` FR2, FR4, FR8 | | |||
| | FR5 | `hasScrim=true` uses `showModal()`, a native backdrop, and native focus containment, and locks page scroll: the page does not scroll while a modal flow is presented, and pull-to-refresh and the page's bounce stop behind it (`overscroll-behavior-y: none`), since the page is held under a modal presentation (`spec:AST-094` FR19), as behind a UIKit modal presentation; the lock is held continuously across a handoff, never released and re-acquired between steps; it still holds beneath a `Dialog` opened from a step; it is released after the flow's exit (DEC-1). `hasScrim=false` uses `show()` without a backdrop or scroll lock and leaves the page interactive. In a non-modal flow the sheet dialogs order themselves with local values inside the switcher's isolated flow wrapper (DEC-1); the page-level value that wrapper carries remains a `spec:AST-027` conformance gap. | DEC-1; `spec:AST-027` FR2, FR4, FR8 | | |||
There was a problem hiding this comment.
The added pull-to-refresh and bounce stop (overscroll-behavior-y: none, useScrollLock.ts:81-84) is new direction: AST-094 FR19 lets a modal scroll lock cancel a lone finger's page pan but says nothing about overscroll. The same text in Drawer.spec.md:163 waits on the same engineering-owner decision.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on the three current-record amendments this change makes: MobileNav FR3 would have the drawer clip <body> instead of the root on pages whose <body> sets its own overflow, and BottomSheetSwitcher FR5 and Drawer FR5 would add that pull-to-refresh and the page's bounce stop behind a modal overlay, which current spec does not state. An engineering owner records that decision by commenting /approve-spec 9821890c91955bbe26f7ab6642e811ea95903bfd 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, before approval the change still needs:
- a rebase onto a landed #8360, so the diff carries only the scroll-lock commit;
- real-device recordings for the iOS Safari and Android Chrome claims (an iPhone below iOS 26.4 dragging the scrim right after an overlay opens, a drag that becomes a pinch, and an Android phone pulling down at the top), which AST-094's platform support asks for.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on five current-record edits at this head: MobileNav FR3 (the drawer clips <body> on pages whose <body> sets its own overflow), BottomSheetSwitcher FR5 and Drawer FR5 (pull-to-refresh and the page's bounce stop behind a modal overlay), and the carried #8360 additions to AST-094 FR11 and FR19 and react-component-runtime INV13; an engineering owner records it by commenting /approve-spec 1e22e47861575b299c184427d5261b5e540d20fc on this pull request, which the author may do as an engineering owner. Whatever is decided, these changes are still needed:
- Land #8360 and rebase, so this pull request carries only the scroll-lock commit.
- Move MobileNav onto
useScrollLock, or say why it cannot use it: its own copy of the lock gets neither the page hold nor the overscroll stop. - Reword the changeset's MobileNav sentence, since on the released version closing MobileNav never released another overlay's lock.
- Add real iPhone and Android recordings for the iOS Safari and Android Chrome claims, as AST-094 asks.
[Automated review]
| '@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, drags on a native range input, pinch-zoom, and one-finger drags on a pinch-zoomed page (they pan the zoomed view) still work. The lock no longer pins `<body>`, and `window.scrollY` keeps its value while the lock holds. The lock cancels those pans itself, leaving a touch a gesture recognizer owns to it, and judges each pan after a small slop so a swipe's first wobble does not fix the wrong axis. It follows the browser's scroll chain, so an overlay rendered inside a scrolling container still holds the page: a modal dialog's pans go to the page, not to that container. It also sets `overscroll-behavior-y: none` on the root and `<body>`, which turns off pull-to-refresh (Chrome on Android) while an overlay is open. 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.
The last sentence says MobileNav "no longer releases another overlay's lock", but the released MobileNav never did: the released lock pins only <body> and MobileNav writes only the root's overflow, so closing or unmounting MobileNav left a Dialog's lock in place. Suggested wording: "MobileNav's clip goes through the same page scroll-style claims as the lock, so the two never overwrite each other."
| // 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.
MobileNav still runs its own copy of the page lock here (the gutter hold at line 583, this claim, and the releases at 600-601 and 618-619) beside useScrollLock, so it gets neither the page hold nor the overscroll stop. Could MobileNav call useScrollLock(isOpen) instead, with a clip option on the shared hook if it needs one, or could the description say why the shared lock cannot serve it?
meta#8360's change on the internal touch foundation: one click guard shared by the menu press and the gesture host (its click listener stays once the menu press arms it, as its record has it, and lasts only while armed for the gesture host, FR24), the host as the one touch owner (finger scroll, page holds, attachments, the press tracker hook and a held control's kept touch, the long press's single source, presented hit testing on iOS WebKit), its cases in the gesture browser proof (Chromium and WebKit) and the README. It carries meta#8074, meta#8270, meta#8293, meta#8315, meta#8320 and meta#8356, and folds in meta#8358 (the long press's tolerance comment and an unused import). Rebased onto main after section 2 as one commit; the touch-action rule came with meta#8071. Where fingers come from their pointer events, a claimed finger's touchend is cancelled from its element (FR21), under a fine primary pointer wherever the browser sends Touch Events and inside an open shadow root too; at most eight such lifts wait for their touchend, and each goes once taken or after 500 ms, kept across the host's uninstall, which can run in the microtask after the pointerup, before the touchend that comes later in the same task (FR24). An armed press's page listeners go once the arena has decided its last touch, a tap that waits on a double tap included, and after its last detach the host holds no listener, note, frame or timer (FR24). The animator's busy proof is judged against the harness's compositor-only control (meta#8906), with a main-thread mutant as its negative, and the spring's curve is placed against the control's own clock, so the late frames a loaded runner sends fail nothing. With the host serving every pointer (meta#8071), this change's own pieces follow it: a press tracker takes a mouse press as a press, painting at once and braking no scroll (FR11); a finger's or a pen's hold that does something keeps text unselected under any primary pointer, while a mouse keeps its released behavior (FR41); and `perEventPointer`, from before FR43, changes nothing. These come from meta#8665, which keeps only the undecided rule for text selection during a mouse or pen drag. The hover recognizer and the host's hover routing land with hover's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…finger landed On iOS WebKit the host resolves moving content at touch-down and asks the engine what is under the finger. When a busy main thread hands it the touch later, the content has moved on: the host now asks at the point where what was under the finger at the touch's timestamp is drawn, so a tap on a moving sheet's Done still clicks Done (spec:AST-094 FR11). A presented source reports its offset at a given time for this. The presented browser proof drives the sheet with Core's animator and a critically damped spring, up into place and down off the screen over the scrim: a tap on the drawn Done clicks it once, and a drag on the drawn sheet catches it where it is drawn, in both directions.
…the finger (FR11, FR19); INV13 names the shared input listener Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on that content goes to what was drawn under the finger at touch-down, and a tap there activates that element with one click while the browser's own click is cancelled or swallowed (FR11). While such content around a recognizer that claims moving touches is moving, the document may carry a non-passive touchmove listener that cancels only owned moves and comes off when the motion ends; while it is on, a native scroll waits for a free main thread (FR19). The gesture host, press feedback, page hold, finger-scroll watch and click guard hear document-wide input through one capture-phase listener per event type on the window (architecture:react-component-runtime INV13).
…view fixes A touch the host handles late now also leaves alone content that slid under the finger after it landed: the touch goes to what was beneath it then (spec:AST-094 FR11). The input-stream changeset says what moved: the gesture host's listeners, and the press feedback's scroll and visibility listeners, since its pointer listeners were already on the window's capture phase. The coarse-pointer gate reads TOUCH_QUERY from touchQueries.ts instead of spelling it again. The WebKit interaction job's scope covers every module the gesture host now bundles: hooks/isRtlElement and utils' flatTree, hapticSwitch, inputStream, clickGuard, selectionLock and coarsePointer. The scope test's example of a util outside it moves from clickGuard, which the gesture fixture now bundles, to focusReturn. The presented proof checks that the sheet moves on a spring (its speed changes along the animator's curve) and no longer compares the drawn box with the animator's offset: under a 3-CPU cap the two disagree by 3 to 27 px in about half the frames whatever reference is used, while the tap and drag cases go through the host's real path.
…e origin most frames agree with Under the CI lane's WebKit profile (four CPUs, one worker), "a spring keeps advancing every frame through a 1 s main-thread block" failed in two ways, each about once in a few hundred runs, and a failing run's frames show each cause: - The spring drifted 138.6 ms against the control. The control's clock took its origin from the first usable frame alone, reading that frame's direction from the next usable one: the frame sat at x 210 on its way out, the next usable one at x 210 after the turn, and the tie read as "back". The clock now lives in controlClock.ts and takes the origin most frames agree with. On that run's own frames it reads 6.5 ms (bound 12), and its unit case for that layout fails on the old reading (150.7 ms astray). - The spring "moved on 61 of the 62 frames the control moved". At the block's end it was in its tail, under two of the cast's 1 px steps a frame, where a frame delivered early shows no step and the next catches up two. The harness now reads such a tail at its own pace (busyWindowProof), and the proof judges the whole block: on that run's own frames the earlier proof fails it and this one passes. No bound changes.
… (architecture:public-component-api INV23) Main's lint rule now keeps every private data attribute in its owner module. This change had moved `data-astryx-haptic-switch` from the haptics module to utils/hapticSwitch.ts, so the click guard could skip the hidden switch without importing haptics (architecture:react-component-runtime INV13). The name now lives in privateAttributes/hapticSwitch.stylex.ts, modelled on segmentDisabled.stylex.ts: it exports markHapticSwitch, which the haptics module uses on its hidden switch, and isHapticSwitch, which the click guard reads. utils/hapticSwitch.ts and the deprecated gestures/hapticSwitch.ts re-export go, the haptics module's baseline entry goes with its raw name, and the WebKit interaction job's scope covers the new module. No behavior changes.
The scroll lock no longer pins <body>, which reset the page's offset to 0 (the page jumped and Safari's toolbar came back). It holds `overflow: hidden` on the page's scroller and, through the gesture host's page hold, cancels only a one-finger pan that nothing inside the overlay takes: a scroller, a range input, a pinch or a zoomed page's pan still goes through. The root's scroll styles become claim stacks (rootScrollStyles), so a lock and a pull to refresh compose, and MobileNav's clip moves onto them. The top-layer check asks CSS.supports for `:modal` and `:popover-open` instead of catching errors from `matches`. The touch module bundle test names useScrollLock and the imports that reach it as carrying the page hold. The records state the clip on the page's scroller (MobileNav FR3) and that pull-to-refresh and the page's bounce stop behind a modal overlay (BottomSheetSwitcher FR5, Drawer FR5). AlertDialog's layout tests stub the page hold, whose pointer read belongs to the lock, so their guard still sees only the layout's own media queries.
Depends on #8360 (the gesture host's page hold). Review only this PR's change: vjeux/astryx@a57e57e...5adb879
Behavior-module PR: scroll lock, MobileNav root claim.
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.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, and anything inside the overlay that takes a pan still gets it (a scroller, a range slider, a pinch). Its touch listener exists only while a lock holds. No spec governs the pin (AST-017 FR3 calls it implementation). Pull-to-refresh and the page's bounce stop behind the overlay (
overscroll-behavior-y: none): the page is held under a modal presentation (spec:AST-094FR19), as in UIKit. BottomSheetSwitcher FR5 and Drawer FR5 say so, and MobileNav FR3 has its clip follow the page's scroller.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:

useScrollLockgrows from 3.7 to 5.9 KB gzip JavaScript, and AlertDialog, BottomSheet, CommandPalette, DateInput, DateTimeInput, Dialog, Drawer, Lightbox, PowerSearch, Table and TimeInput grow 1.6–2.0 KB each, paid once per page. The touch module bundle test lists 22 public imports that reach the page hold ("useScrollLock's page hold, and the imports that reach it"); the other ten (ContextMenu, DropdownMenu, Layer, Markdown, MoreMenu, MultiSelector, Pagination, Popover, Selector and Tokenizer) stay within 0.05 KB. CSS is unchanged.check-knowledge,specs-for-file) pass, 309 tests;check:knowledgeand changeset coverage pass.Writing rubric results