Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This needs an owner decision before it can be approved, and it needs changes either way. NavigationStack's touch bar look relies on spec:AST-094 FR13 and FR4, which are still in open spec pull requests (#8316, #8378, #8394), and main has no NavigationStack record, so an engineering owner needs to identify or provide the current spec that governs this appearance; an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and as an engineering owner you may do so yourself. Independently of that decision, please:
- rebase onto
mainonce the 16 stacked pull requests land, so the diff is this change alone (it is 52,339 authored lines today against a 1,840-line commit); - keep Lab's back swipe off core's gesture tracker, as
architecture:interaction-modalityINV10 requires, or amend INV10 onmainfirst; - reclassify the row-press change that hides consumers'
xstyle::beforedecoration under a finger, which ships as[fix]; - record the
architecture:component-style-authoringINV7 element counts for the elements the bar change adds, and drop the nonexistent@astryxdesign/core/motion/experimentalpath from two changesets.
[Automated review]
| import { | ||
| PanGestureRecognizer, | ||
| ScreenEdgePanGestureRecognizer, | ||
| useGestureRecognizer, | ||
| } from '@astryxdesign/core/gestures'; |
There was a problem hiding this comment.
The back swipes now run on core's PanGestureRecognizer and ScreenEdgePanGestureRecognizer, and through them core's VelocityTracker, which the gestures barrel re-exports for Lab. architecture:interaction-modality INV10 says Lab cannot import the tracker and keeps one local tracker with the same blend and stillness interval. Either keep a Lab-local tracker here or amend INV10 on main first.
There was a problem hiding this comment.
This comes from the back swipe in #8108, which this pull request is stacked on, and its owner is taking INV10 up there. This pull request's own commit does not touch the tracker.
| opacity: { | ||
| default: null, | ||
| [COARSE]: { | ||
| default: 0, | ||
| [ENABLED]: {default: null, [PRESSED_ON]: 1, [PRESSED_HELD]: 1}, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Under a coarse pointer this gives every interactive row's ::before opacity: 0 at rest, so a consumer's existing xstyle ::before decoration disappears; list-row-press-compositor.md ships that as [fix]. spec:AST-017 DEC-1 asks for the consumer scenario that stops working to be named and kept out of a patch release.
There was a problem hiding this comment.
This is the row-press change from #8114, lower in the stack, and its owner has it.
| <div {...stylex.props(styles.edgeSentinelAnchor)}> | ||
| <div {...edge.sentinelProps} data-navigation-scroll-edge-sentinel /> | ||
| </div> | ||
| {props.children} | ||
| </div> | ||
| </div> | ||
| <div {...edge.layerProps} data-navigation-scroll-edge /> |
There was a problem hiding this comment.
The anchor, sentinel and veil mount on every screen under every pointer, beside the clip box and row wrapper at lines 791-792. architecture:component-style-authoring INV7 asks for before-and-after element counts for added elements and puts decoration such as the veil on a pseudo-element; the anchor could go if the content wrapper positioned the sentinel.
There was a problem hiding this comment.
Done at 9f51e05. The sentinel's positioned wrapper is gone: it sits at its static position in the scroller. The description records the element counts before and after, with the reason each remaining element is needed: the sentinel is what IntersectionObserver watches, the veil is a layer the shared animator fades with inline values, and the moving cut needs an overflow box and a counter-moved row to stay on the compositor.
There was a problem hiding this comment.
A correction to the veil's reason above. The animator can drive a pseudo-element, but it holds a pseudo-element's resting value with a filling animation, and that keeps the layer composited at rest. This hook is shared with AppShell. There, a composited layer under the sticky header makes iOS 26.2 drop the header's items for about two frames on every programmatic scroll. Reduced to plain HTML: with no layer, 0 of 5 takes blank; with a composited veil that never switches, 5 of 5 do. The description now says so.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [experimental] The `uikit` table in `@astryxdesign/core/motion/experimental` gains the navigation values the lab NavigationStack runs on, each measured on iOS 26.2: `navigation.contentPopHysteresis` (the content back swipe's 27 pt start), `navigation.interactivePopVelocitySamples` (the release velocity averages the last 3), `navigation.interactivePopRubberBand` (past its start the back swipe's screen follows at 0.71 of the finger) and `springs.navigationInteractivePopReturn` (its return when released there), `springs.navigationBackButton` (the back chevron's fade), and `navigation.backChevronPushDelayMs` (when the chevron starts fading in on a push). The catalog gains `@astryx.navigationStack.back`. |
There was a problem hiding this comment.
@astryxdesign/core/motion/experimental does not exist: core's exports list no such subpath and the motion modules are internal. Rewrite this as an internal [chore] note like the other motion changesets; uikit-navigation-large-title-rise.md has the same problem.
meta-cla
Bot
added
the
CLA Signed
dfc5838 to
9f51e05
Compare
|
Thanks. At 9f51e05:
|
There was a problem hiding this comment.
This needs an engineering owner's spec decision before it can be approved, and it needs changes either way. NavigationStack's touch bar appearance under a finger (no background or hairline, the veil once content is 10px under the bar, the covered bar cut at the top card's leading edge) has no current spec on main: spec:AST-094 is still in open pull requests, and its FR13 and FR4, as proposed in #8394, cover when a touch presentation applies and that its motion is themeable, not how the bar looks. An engineering owner needs to provide or identify the claim that governs this appearance, or record the decision by commenting /approve-spec <full head SHA> on this pull request; as an engineering owner, you may do so yourself. Independently of that decision:
- the veil's new
will-change: opacitycomposites AppShell's touch header at rest as well, the condition your own iOS 26.2 reduction says blanks its items on programmatic scrolls; scope it to NavigationStack and re-check AppShell, or carry the veil on a pseudo-element; - rebase onto
mainonce the 19 stacked pull requests land or are dropped, so the diff is this change alone (52,398 authored lines today); - the stacked code still runs Lab's back swipes on core's gesture tracker against
architecture:interaction-modalityINV10, ships the row-press::beforechange as[fix]although consumers must move their decoration, and changes AppShell's header andheightcontract without amendingcomponent:AppShell.
[Automated review]
| willChange: { | ||
| default: null, | ||
| '@media (pointer: coarse)': 'opacity', | ||
| }, |
There was a problem hiding this comment.
This composites the veil at rest for every caller under a finger, AppShell's touch header included. The pull request keeps the veil an element because a veil composited at rest under AppShell's sticky header blanks its items on iOS 26.2 programmatic scrolls, and these lines recreate that condition. Pass the will-change through NavigationStack's layerXstyle only and repeat the AppShell check on iOS 26.2, or carry the veil on a pseudo-element.
9f51e05 to
74e0a84
Compare
|
An update on the three points that come from the pull requests below this one:
|
There was a problem hiding this comment.
This needs two engineering-owner spec decisions before it can be approved, and it needs changes either way. NavigationStack's touch bar appearance under a finger (no background or hairline, the veil once content is 10px under the bar, the covered bar cut at the top card's leading edge) has no current spec on main, since spec:AST-094 is still in open pull requests and there is no NavigationStack record, and the back swipes running on core's gesture recognizers need the architecture:interaction-modality INV10 amendment you sent to be made on main. An engineering owner needs to provide or identify the claim for the appearance and update INV10, or record each decision by commenting /approve-spec <full head SHA> on this pull request; as an engineering owner, you may do so yourself. Independently of those decisions:
- Under a finger, the Large title story shows the large title through the clear bar beside the faded-in inline title from about 55px to 65px of scroll, so start the veil or clip the large title where it reaches the bar.
- Rebase onto
mainonce the stacked pull requests land or are dropped, so the diff is this change alone (52,635 authored lines today). - The stacked code still changes AppShell's header and
heightcontract without amendingcomponent:AppShell, hides a row'sxstyle::beforeunder a finger in a[fix](your reported fix is not in this head yet), and prunes search state in a layout effect againstarchitecture:react-component-runtimeINV1. - Add the
architecture:component-style-authoringINV6 gzip size report for NavigationStack and AppShell.
[Automated review]
| </div> | ||
| ) : null} | ||
| <div ref={content.ref}> | ||
| <div {...edge.sentinelProps} data-navigation-scroll-edge-sentinel /> |
There was a problem hiding this comment.
Under a finger the bar is now clear, but this sentinel sits below the large title, so the veil comes in only once the large title has passed the bar and 10px more, while the inline title takes over as soon as the large title's bottom passes the bar's (lines 529-547). In the Large title story, from 55px to 65px of scroll the inline title is at opacity 1, the veil at 0 and the large title still shows in the bar: the two titles overlap. With a mouse the opaque bar hides it. Starting the veil, or clipping the large title, where the large title reaches the bar would close the gap.
| // Where the screen's scroll edge sentinel sits: the top of its content, | ||
| // below the large title and the search field (an expanded large title is | ||
| // outside iOS's veil). The sentinel is absolutely positioned in it. | ||
| // The scroll edge sentinel stays at its static position, the top of the |
There was a problem hiding this comment.
This first paragraph still describes an absolutely positioned sentinel in a positioned wrapper; the paragraph after it says the sentinel stays at its static position with no wrapper. The first one can go.
| useLayoutEffect(() => { | ||
| const present = new Set(view.models.map(model => model.key)); | ||
| const forget = (records: Map<string, unknown> | Set<string>) => { | ||
| for (const key of [...records.keys()]) { | ||
| if (!present.has(key)) { | ||
| records.delete(key); | ||
| } | ||
| } | ||
| }; | ||
| forget(searchingRef.current); | ||
| forget(heldCollapsesRef.current); | ||
| forget(searchRestRef.current); | ||
| forget(freshLiftsRef.current); | ||
| setLifts(current => { | ||
| const gone = [...current.keys()].filter(key => !present.has(key)); | ||
| if (gone.length === 0) { | ||
| return current; | ||
| } | ||
| const next = new Map(current); | ||
| for (const key of gone) { | ||
| next.delete(key); | ||
| } | ||
| return next; | ||
| }); | ||
| }, [view.models]); |
There was a problem hiding this comment.
This layout effect sets state (setLifts) from view.models and names no external system, which architecture:react-component-runtime INV1 and INV3 rule out. Dropping the gone keys in the same update that produces the new view, or keeping the lift with the screen's mounted card, would avoid the repair render.
|
Posted by Cindy's agent on her behalf. This rides #8142: Cindy asked there for an approach within her guardrails (themeable visuals through the existing theme infrastructure, at most one new theme role, following the theming spec, respecting variants), and that same approach should cover NavigationStack. |
|
This follows the approach proposed on #8142 (#8142 (comment)): the same hook, the same single motion role, and the band painted from the screen's existing background token until NavigationStack's bar has a theme target. I'll update this pull request together with #8142 once the approach is agreed. |
74e0a84 to
8363522
Compare
There was a problem hiding this comment.
This needs an engineering owner's spec decision before it can be approved, and it needs changes either way. NavigationStack's touch bar appearance (no background or hairline under a coarse pointer, the veil once a screen's content is 10px under the bar, the covered bar cut at the top card's edge) has no current spec: spec:AST-094 governs touch behaviour only and leaves each component's presentation to its own record, and main has no NavigationStack record. An engineering owner needs to state that appearance in a record on main, or record the decision by commenting /approve-spec <full head SHA> on this pull request; as an engineering owner, you may do so yourself. Independently of that decision:
registry-contract (preview)fails because Lab's entry now imports@astryxdesign/core/motion/*,@astryxdesign/core/gesturesand@astryxdesign/core/utils/touchPresentation, which core does not export andspec:AST-017FR21 keeps internal.- AppShell's import grows by 23,522 bytes gzip with code that works only under a coarse pointer, so report component sizes and load that code lazily or justify it, as
architecture:component-style-authoringINV6 asks. - Under a finger the large title shows through the clear bar beside the inline title for about 10px of scroll, so start the veil where the large title reaches the bar.
- Each screen's scroll edge effect adds its own document-level
scrollendlistener againstspec:AST-094FR24, and the veil's springs need to be FR4 motion roles. - Rebase onto
mainonce the stacked pull requests land, so the diff is this change alone (46,083 authored lines today).
[Automated review]
| springs, | ||
| } from '@astryxdesign/core/motion/uikit'; | ||
| import {useAnimator} from '@astryxdesign/core/motion/useAnimator'; | ||
| import {useScrollEdgeEffect} from '@astryxdesign/core/motion/useScrollEdgeEffect'; |
There was a problem hiding this comment.
Core's exports do not list ./motion/useScrollEdgeEffect (nor the other motion/*, gestures and utils/touchPresentation paths Lab now imports), and spec:AST-017 FR21 keeps them internal. Outside the workspace's own resolvers they do not resolve: CI's registry-contract (preview) fails at this head with 23 “Could not resolve” errors from Lab's built NavigationStack and SearchField files, this import among them.
| import {focusOutlineProps} from '../utils/focusOutline.stylex'; | ||
| import {useMediaQuery} from '../hooks/useMediaQuery'; | ||
| import {useTouchPresentation} from '../utils/touchPresentation'; | ||
| import {useScrollEdgeEffect} from '../motion/useScrollEdgeEffect'; |
There was a problem hiding this comment.
This static import brings the gesture host, the finger-scroll watch and the animator into every AppShell import, mouse included; CI's size analysis puts AppShell's import at 80,359 bytes gzip against 56,837 at the merge base. architecture:component-style-authoring INV6 asks for the size report and for coarse-pointer-only code over 2 KB to load lazily or be justified.
| const unsubscribeFingers = host.onFingersChange(updatePromotion); | ||
| // After the finger-scroll watch has heard the same `scrollend`. | ||
| const onScrollEnd = () => queueMicrotask(updatePromotion); | ||
| doc.addEventListener('scrollend', onScrollEnd, PASSIVE_CAPTURE); |
There was a problem hiding this comment.
One document-level scrollend listener per running effect, and NavigationStack runs one effect per mounted screen. spec:AST-094 FR24 says the number of document- or window-level touch listeners must not grow with component instances; the shared finger-scroll watch already keeps one per document (gestures/fingerScroll.ts).
| * Registers the bars table (motion/roles.ts). It lists no roles: the scroll | ||
| * edge effect's springs stay internal constants until the public | ||
| * motion-token set is decided (`spec:AST-094` FR4, DEC-11), and the switch, | ||
| * the extent and the veil are not motion (not springs, durations or curves). | ||
| * Nothing registers on import: the component that animates with this table | ||
| * (AppShell; Lab's NavigationStack) hands it to the scroll edge effect | ||
| * (motion/useScrollEdgeEffect.ts), which calls it once before its first | ||
| * animation (it is idempotent). | ||
| */ | ||
| export function registerBarsTable(): void { | ||
| registerMotionTable('bars', bars); |
There was a problem hiding this comment.
With no roles registered, the veil's fades play these springs as plain constants. spec:AST-094 FR4 asks for every spring a touch presentation plays to be a motion role named for its use; DEC-11 keeps the roles internal (not theme-settable) rather than removing them.
startCompositorTrace keeps every Display::FrameDisplayed in the browser, and those events carry no page identity: with a second page animating, a stall inside the block passed busyWindowProof with presented, crediting the other page's frames. It now throws when another page is open as it starts or stops, and says the record is browser-wide (JSDoc and README). Negative control: another animating page makes it refuse to start, and a page opened mid-trace makes stop() refuse.
…NavigationStack bar look
…on drives Under load, in a frame that comes a few milliseconds late, WebKit evaluates two layers that animate on one clock about a millisecond apart, their boxes and their computed translates alike, so two layers read in one frame disagree by a pixel or two while the screen shows them together. "a back swipe holds the revealed bar still..." held the top bar to its card within 1 px through the release's spring back as well, and failed there (1.03 px; 2 of 10 runs at 6 workers). traceBars now reads each layer's computed translate (at rest a layer's box is its translate, checked when the trace starts) and records whether an animation drives the frame; the proof keeps 1 px while the finger drives both layers and takes the other bar proofs' 1 % of the width through the release.
…e demoted The story's own first motion leaves its layers promoted for the animator's linger, half a second after that motion ends. The push started about 100 ms after it, so the root bar title's demotion (a style pass and one Layout) came 386-400 ms into the push, at the 380 ms edge of the counted window, and inside it when the push's first frame came late under load (373 ms; 2 of 24 runs at 8 workers). The proof now waits until no layer is promoted before it clicks, so the window holds only the push's own work.
…st's cadence Under load the screencast delivers frames at its own rate: 13 in the judged window where the 60 Hz count wanted at least 15, while the title changed on every frame delivered. The proof now counts against the cast's measured cadence, the median interval between the frames it delivered in the window (never faster than 60 Hz); a frozen title delivers none there and still fails.
The bases merged with none of this PR's own change, so a compare from here shows only that change. The bases both add to the shared motion notes, the UIKit table's entry header and the en catalog; each keeps both sides' entries. Both re-export useAnimator from Core's internal motion entry; it keeps one, meta#8181's, with its type.
… a mouse press At the document, an event from inside an open shadow root is retargeted to the shadow host, and so is a touch's target, so the host looked for participants from the shadow host and found none inside the root: the browser then sent its own mousedown and click. Both cases, a finger and a mouse, attach a tap to an element in the shadow root. Fail until the host resolves targets through the composed path.
…composed path Inside an open shadow root the document's listeners see the shadow host, in event.target and in a touch's target alike, so a recognizer on an element in the root never got its finger, and nothing claimed it. landedOn takes the composed path's first element for the event's own target and for a touch whose target is the event's (another finger keeps its own): a touch's start, a pointer's press and its pairing, the last touch a scroll reads, a cancelled pointer's touch and hover.
…he host resolves targets through the composed path) into meta#8320
There was a problem hiding this comment.
This needs an engineering owner's spec decision before it can be approved, and it needs changes either way. NavigationStack's touch bar appearance (no background or hairline under a coarse pointer, the veil once a screen's content is 10px under the bar, the covered bar cut at the top card's edge) has no current spec: spec:AST-094 governs touch behaviour and leaves each component's presentation to its own record, and main has no NavigationStack record, so an engineering owner needs to state that appearance in a record on main, or record the decision by commenting /approve-spec <full head SHA> on this pull request; as an engineering owner, you may do so yourself. Independently of that decision:
- Under a finger, a stack that opens two screens deep keeps the root's veil on at the top after Back, because the root offset is measured once from a hidden screen's empty first report (
useScrollEdgeEffect.ts:323-336). - Large-title screens show both titles through the clear bar for about 10px of scroll, so start the veil where the inline title takes over (
NavigationStack.tsx:545-547,:681-682). - Each screen's effect adds its own document
scrollendlistener againstspec:AST-094FR24, the veil's springs need to be FR4 motion roles, and Lab should reach the animator through@astryxdesign/core/internal/touchasarchitecture:react-component-runtimeINV13 names, unless an engineering owner widens INV13. - Rebase onto
mainonce the stacked pull requests land, so the diff is this change alone (44,759 authored lines today, including AppShell's touch header andheight="adaptive", which contradictcomponent:AppShellFR7 and its height contract, and a pinned search field that sits 44px below the bar).
[Automated review]
| if (rootOffset == null && container != null) { | ||
| const bounds = entry.rootBounds; | ||
| const paddingEdge = | ||
| container.getBoundingClientRect().top + container.clientTop; | ||
| const offset = | ||
| bounds == null ? 0 : Math.round(bounds.top - rootInset - paddingEdge); | ||
| rootOffset = offset; | ||
| if (offset !== 0) { | ||
| // WebKit: observe again from the padding edge; this report was | ||
| // measured from the content box. | ||
| observer.disconnect(); | ||
| observe(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
A screen two levels down mounts with content-visibility: hidden, so this first report has an empty root rectangle (top 0, height 0 in Chromium). The offset comes out as 0 - 44 - paddingEdge (−60 in the Pushed two levels story), the observer is recreated with a -104px margin, and rootOffset is never measured again: after two Backs the root sits at scroll 0 with its veil stuck at opacity 1. Measuring only from a report whose root rectangle has size (or re-measuring when the screen is revealed) avoids it; a proof for a stack that opens two deep would catch it.
| </div> | ||
| ) : null} | ||
| <div ref={content.ref}> | ||
| <div {...edge.sentinelProps} data-navigation-scroll-edge-sentinel /> |
There was a problem hiding this comment.
On a large-title screen this sentinel sits below the large title, while the inline title takes over once the large title's bottom passes the bar's (:545-547). Under a finger the clear bar therefore shows the large title behind the inline one for about 10px of scroll: at 60px the large title spans −16px to 39px of the bar, the inline title is at opacity 1 and the veil at 0. Starting the veil where the inline title takes over (or clipping the large title at the bar's bottom) would fix it.
| const unsubscribeFingers = host.onFingersChange(updatePromotion); | ||
| // After the finger-scroll watch has heard the same `scrollend`. | ||
| const onScrollEnd = () => queueMicrotask(updatePromotion); | ||
| doc.addEventListener('scrollend', onScrollEnd, PASSIVE_CAPTURE); |
There was a problem hiding this comment.
One document-level scrollend listener per running effect, and NavigationStack runs one effect per mounted screen. spec:AST-094 FR24: “The number of document- or window-level touch listeners MUST NOT grow with the number of component instances”. The finger-scroll watch already keeps one per document for this event (gestures/fingerScroll.ts:131-132); the promotion update could ride it or the gesture host.
| * animation (it is idempotent). | ||
| */ | ||
| export function registerBarsTable(): void { | ||
| registerMotionTable('bars', bars); |
There was a problem hiding this comment.
With no roles listed, the veil's fades play bars.scrollEdgeEffect.appear and .disappear as plain constants. spec:AST-094 FR4 asks for every spring a touch presentation plays to be a motion role named for its use; DEC-11 keeps roles internal rather than absent, so listing these two as internal roles would keep them out of the public theme set.
| springs, | ||
| } from '@astryxdesign/core/internal/motion'; | ||
| import {useAnimator} from '@astryxdesign/core/internal/motion'; | ||
| import {useScrollEdgeEffect} from '@astryxdesign/core/internal/motion'; |
There was a problem hiding this comment.
architecture:react-component-runtime INV13 has Core and Lab reach the shared touch modules, the animator included, through @astryxdesign/core/internal/touch; this adds the scroll edge effect to the second entry, @astryxdesign/core/internal/motion. Re-exporting from internal/touch, or an engineering owner widening INV13, would settle it.
| }, | ||
| searchPinned: { | ||
| position: 'sticky', | ||
| top: BAR_HEIGHT, |
There was a problem hiding this comment.
The scroller is already padded 44px for the bar (:403), so top: 44 pins the field 88px below the screen's top: at rest an empty 44px band sits under the bar and the field covers the first row's label, and scrolled rows pass through the band, under either pointer in Chromium. top: 0 (with a story for hidesSearchWhenScrolling={false}) would pin it under the bar.
…s its divider with a coarse-pointer rule, and drops its layer where no scrollend fires Review findings on meta#8142: - With a mouse, a variant added by module augmentation painted a surface header. A page-scrolling shell now keeps each variant's own fill (none for an augmented one) and adds only the coarse-pointer transparent rule. - The section divider was dropped through the resolver's guess, so a server render with `defaultIsMobile` lost it. It stays in the markup, and a coarse-pointer rule takes its width away on the touch bar. - Where the browser sends no `scrollend`, the layer stayed promoted after a fling (re-checked only at a finger change). The scroller's scroll events now time a re-check 200 ms after the last one, past the finger-scroll watch's 150 ms spell. - One document listener per effect: `scrollend`, or `scroll` where the browser sends none. This base has no shared input stream (it arrives with meta#8665's line); the effect moves onto it there. - The WebKit workaround names its removal condition. Red first: the five new cases fail on ae06884.
…slates, the Layout/Paint proof waits for demotion, the handover counts at the cast's cadence) into the bar look's bases
…) into the NavigationStack bar look # Conflicts: # packages/core/src/AppShell/AppShell.tsx # packages/core/src/AppShell/AppShellScrollEdge.test.tsx # packages/core/src/hooks/useScrollEdgeEffect.ts # packages/core/src/motion/useScrollEdgeEffect.test.tsx
There was a problem hiding this comment.
This needs an engineering owner's spec decision before it can be approved, and it needs changes either way. NavigationStack's touch bar appearance (no background or hairline under a coarse pointer, the veil once a screen's content is 10px under the bar, the covered bar cut at the top card's edge) has no current spec: spec:AST-094 governs touch behaviour and leaves each component's presentation to its own record, and main has no NavigationStack record, so an engineering owner needs to state that appearance in a record on main, or record the decision by commenting /approve-spec <full head SHA> on this pull request; as an engineering owner, you may do so yourself. Independently of that decision:
- Under a finger, a stack that opens two screens deep keeps the root's veil on at the top after Back, because the root offset is measured once from a hidden screen's empty first report (
useScrollEdgeEffect.ts:338-352). - Large-title screens show both titles through the clear bar for about 10px of scroll, so start the veil where the inline title takes over (
NavigationStack.tsx:545-547,:681-682). - Each screen's effect adds its own document listener against
spec:AST-094FR24, the veil's springs need to be FR4 motion roles, and Lab should reach the animator through@astryxdesign/core/internal/touchasarchitecture:react-component-runtimeINV13 names, unless an engineering owner widens INV13. - Rebase onto
mainonce the stacked pull requests land, so the diff is this change alone (45,071 authored lines today, including AppShell's touch header andheight="adaptive", which contradictcomponent:AppShellFR7 and its height contract, and a pinned search field that sits 44px below the bar).
[Automated review]
| const paddingEdge = | ||
| container.getBoundingClientRect().top + container.clientTop; | ||
| const offset = | ||
| bounds == null ? 0 : Math.round(bounds.top - rootInset - paddingEdge); |
There was a problem hiding this comment.
This offset is measured once, from the first report. A NavigationStack screen that mounts neither on top nor directly under it has content-visibility: hidden (NavigationStack.tsx:432-434, :2368), so its first report has an empty root rectangle (top 0, height 0): with the scroller 16px down this computes −60, observes again with a −104px top margin, and never measures again. Once Back reveals the screen, its sentinel sits above that shrunken root at rest, so the veil stays at opacity 1 at scroll 0. Take the measurement only from a report whose root rectangle has size (or measure again when the screen is revealed), and add a proof for a stack that opens two screens deep.
| updatePromotion(); | ||
| }, PROMOTION_RECHECK_MS); | ||
| }; | ||
| doc.addEventListener(scrollType, onScrollEvent, PASSIVE_CAPTURE); |
There was a problem hiding this comment.
One capture listener on the document per running effect, scrollend or, without it, scroll (which then runs on every scroll event in the document), and NavigationStack runs one effect per mounted screen. spec:AST-094 FR24: “The number of document- or window-level touch listeners MUST NOT grow with the number of component instances”. The finger-scroll watch already keeps one listener per document for both events (gestures/fingerScroll.ts:131-132); the promotion can be updated from it or from the gesture host.
| </div> | ||
| ) : null} | ||
| <div ref={content.ref}> | ||
| <div {...edge.sentinelProps} data-navigation-scroll-edge-sentinel /> |
There was a problem hiding this comment.
On a large-title screen this sentinel sits below the large title, while the inline title takes over once the large title's bottom passes the bar's (:545-547). With the bar now clear under a finger, the large title shows through it under the inline title for about 10px of scroll, until the veil comes in. Start the veil where the inline title takes over under a finger (or clip the large title at the bar's bottom), and add a large-title case to NavigationStackScrollEdge.interaction.browser.spec.ts.
| * animation (it is idempotent). | ||
| */ | ||
| export function registerBarsTable(): void { | ||
| registerMotionTable('bars', bars); |
There was a problem hiding this comment.
With no roles listed, the veil's fades play bars.scrollEdgeEffect.appear and .disappear as plain constants. spec:AST-094 FR4 asks for every spring a touch presentation plays to be a motion role named for its use, and DEC-11 keeps those roles internal rather than absent; listing both springs as internal roles keeps them out of the public theme set.
| springs, | ||
| } from '@astryxdesign/core/internal/motion'; | ||
| import {useAnimator} from '@astryxdesign/core/internal/motion'; | ||
| import {useScrollEdgeEffect} from '@astryxdesign/core/internal/motion'; |
There was a problem hiding this comment.
architecture:react-component-runtime INV13 has Core and Lab reach the shared touch modules, the animator included, through @astryxdesign/core/internal/touch; this adds the scroll edge effect to a second entry, @astryxdesign/core/internal/motion, beside the animator Lab already takes from it. Re-export what Lab needs from internal/touch, or have an engineering owner widen INV13 to core's ./internal/* entries.
| }, | ||
| searchPinned: { | ||
| position: 'sticky', | ||
| top: BAR_HEIGHT, |
There was a problem hiding this comment.
The scroller is already padded 44px for the bar (:403), so top: 44 pins the field 88px below the screen's top: at rest an empty 44px band sits under the bar and the field covers the first row, and scrolled rows pass through that band. top: 0 puts it right under the bar; no story uses hidesSearchWhenScrolling={false} yet.
| }, | ||
| // Where the screen's scroll edge sentinel sits: the top of its content, | ||
| // below the large title and the search field (an expanded large title is | ||
| // outside iOS's veil). The sentinel is absolutely positioned in it. |
There was a problem hiding this comment.
This paragraph still says the sentinel “is absolutely positioned in it”; the paragraph below says it stays at its static position with no wrapper. The first one can go.
|
Posted by Cindy's agent on her behalf: closed and reopened only to restart this PR's CI on the current workflow after tonight's queue-wide cancel; nothing in the PR changed. |
Stacked on #8204 (NavigationStack search) and #8142 (the scroll edge effect). Review only this PR's change: vjeux/astryx@98fded9...297d301
Carries #8562 (Core's internal entries) as a dependency; if the entries land differently, this follows her decision.
User impact
Under a finger (
spec:AST-094FR13), NavigationStack's bars become iOS 26's: no background and no hairline. Once content is 10 px under the clear bar, a veil of the body colour fades in behind the title (iOS 26.2's scroll edge effect). Each screen's veil rides its card through a push, a pop or a back swipe. A mouse keeps today's opaque bar.Visual authority
#8142's look (AppShell's touch bar) on Lab's NavigationStack: the decision governing #8142 governs this.
Before and after
iOS 26.2 Safari, before (left) and after: a finger drags the list.
A push, 4× slower: the incoming screen cuts the old title at its edge, as UIKit's does.
Android 15 (emulator), mask off, then a still with it on:
State matrix
spec:AST-094DEC-11); a programmatic scroll switches it at once, with no compositing layer (feat(AppShell): under a finger, the pinned touch header is an iOS 26 bar with its scroll edge effect #8142's iOS 26.2 fix).Design
spec:AST-094FR4): it moves into core's motion modules; Lab takes it through@astryxdesign/core/internal/motion(chore(core): Lab reaches Core's pan recognizer, gesture types, touch presentation and motion through internal entries #8562).architecture:component-style-authoringINV7): per screen the veil and its sentinel, per bar the clip box and its row; counts in Testing.Size (INV6,
analyze-pr, gzip, base → head): NavigationStack's import 64,053 → 66,452 B of JavaScript, CSS 1,105 → 1,180 B; AppShell's 85,267 → 85,645 B, CSS 12,281 → 12,305 B.Scope
Testing
pnpm exec vitest run packages/core/src/{motion,AppShell,gestures,theme,internal,Layout} packages/core/src/utils/touchPresentation.test.ts packages/lab/src/NavigationStack: 90 files, 2924 tests pass,motion/uikit/tableImports.test.tsincluded. New jsdom cases: the effect registers the springs it was handed before observing (nothing under a mouse); a scroll container's root is measured once and observed again from its padding edge where the engine starts it at the content box; a programmatic switch is written without promotion; a finger's landing composites the veil in a scroll container until it lifts, and through its momentum untilscrollend; the caller's sentinel styles apply, and NavigationStack's sentinel is the content's first child, with no wrapper. The finger cases declare a coarse primary pointer, since the shared gesture host listens only there (spec:AST-094FR19, FR24).pnpm test:interaction NavigationStack AppShellScrollEdge gestures.browseragainst a freshpnpm storybook:build: 116 pass in Chromium; 99 pass and 17 Chromium-only skips in WebKit. New proofs under a finger: the incoming screen cuts the old title while the main thread is blocked; mid-push the cut rides the incoming card's leading edge, left to right and right to left (within 2 px, read from the layers' animated translate: in a burst of animation frames WebKit can return one layer's box a frame behind its animation); at rest the covered bar is cut away and the top bar whole; Back reveals it whole; a push during a push; a pointer that turns fine after a push leaves the revealed bar whole (fails before the fix). feat(lab): NavigationStack hands its bar to a search field #8204's two opaque-bar proofs now run with a mouse, each asserting a fine pointer.RasterDecoderImpl::DoEndRasterCHROMIUM); no stall later in a drag.registry-contract (preview)(DOCSITE_TARGET=canary node apps/docsite/scripts/generate-data.mjs, thennode internal/shadcn-registry/verify-full-catalog.mjs): 1038 items and 1042 routes build, with Lab taking Core's modules throughinternal/motionandinternal/touch; the codemod (lab-internal-imports.mjs) leaves 0 imports to move.scripts/touch-support-bundle.test.mjs› "is absent from the root entry" is red: AppShell's touch bar (feat(AppShell): under a finger, the pinned touch header is an iOS 26 bar with its scroll edge effect #8142) imports the gesture host statically, as INV13 prescribes, so the test's whole-root claim can't hold; F1's per-component bundle test, test(core): a touch component's import carries its gesture code, and nothing else does #8671, lists AppShell as a gesture user.pnpm a11y:audit -- --components AppShell,NavigationStack: no new violations.pnpm exec vitest run --project node packages/cli/test: 309 pass. Rootpnpm typecheckafterpnpm build(CI's order),check:changesets,check:knowledge, the CI duration guard,internalModulesNotExperimental,scripts/lab-core-imports.test.mjs,scripts/core-internal-entries.test.mjs, eslint, prettier and the pre-commit checks pass.