Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: No accessibility violations detected. Visual Regression55 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 35 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval is on hold for an owner decision on the draft spec spec:AST-094, which is the only authority this change cites. Its FR19 and FR20 also forbid the refresh pull's non-claiming, browser-shared pan as written. An engineering owner can approve the record, with FR19/FR20 amended for the refresh pull, by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Independently of that decision:
- Merge after #8045, #8062, #8074, #8076, #8086, #8098 and #8170 and rebase, since against
mainthis pull request carries about 28,000 lines, of which only 1,323 are its own commit. - Make the controlled
isRefreshingthe single source (useRefreshControl.ts:722-729), because today a pull-triggered refresh stays on while the prop readsfalse. - Apply
overscroll-behavior-y: noneonly while the scroll owner overflows (AST-025 FR14), and read the scroll-lock state withuseSyncExternalStoreinstead of an Effect (:444-447).
[Automated review]
| useEffect(() => { | ||
| if (isRefreshingProp === undefined) { | ||
| return; | ||
| } | ||
| dispatch( | ||
| isRefreshingProp ? {type: 'begin', source: 'program'} : {type: 'end'}, | ||
| ); | ||
| }, [dispatch, isRefreshingProp]); |
There was a problem hiding this comment.
This Effect is the only path from a controlled isRefreshing into the hook, and it runs only when the prop changes. After a pull triggers with the prop still false, the hook stays refreshing (spinner, aria-busy, 60 px inset) until the prop goes true and then false. An app that never commits true for that refresh cannot end it. The first render with isRefreshing set to true also returns false and is repaired after commit (react-component-runtime INV1/INV3, and its "not casually mirrored into a second source of truth" rule).
When controlled, return the prop, and after a trigger end the refresh on the next commit unless the prop became true.
| useEffect(() => { | ||
| setIsLocked(isScrollLockActive()); | ||
| return subscribeScrollLock(setIsLocked); | ||
| }, []); |
There was a problem hiding this comment.
This Effect sets state from an external store with no comment naming the system, and it starts from isLocked: false even when a lock already holds. For that first commit, the edge Effect above claims overscroll-behavior-y on the scroll owner.
const isLocked = useSyncExternalStore(subscribeScrollLock, isScrollLockActive, () => false);| const applyEdge = useCallback(() => { | ||
| const current = owner.current; | ||
| const near = | ||
| isPullEnabledRef.current && | ||
| scrollTopOf(current) < Math.max(heightOf(current), REFRESH_HEIGHT); | ||
| const state = edge.current; | ||
| if (!near || state.element !== current) { | ||
| releaseEdge(); | ||
| } | ||
| if (!near) { | ||
| return; | ||
| } | ||
| if (current == null) { | ||
| if (edge.current.releaseRoot == null) { | ||
| edge.current = { | ||
| element: null, | ||
| saved: null, | ||
| releaseRoot: claimRootScrollStyle({overscrollBehaviorY: 'none'}), | ||
| }; | ||
| } | ||
| return; | ||
| } | ||
| if (current.style.overscrollBehaviorY !== 'none') { | ||
| // First claim, or the owner's own render replaced the value. | ||
| edge.current = { | ||
| element: current, | ||
| saved: current.style.overscrollBehaviorY, | ||
| releaseRoot: null, | ||
| }; | ||
| current.style.overscrollBehaviorY = 'none'; | ||
| } | ||
| }, [releaseEdge]); |
There was a problem hiding this comment.
near never checks whether the owner overflows. A short or empty list in an overflow-y: auto container gets overscroll-behavior-y: none for the whole time the hook is mounted on a touch screen. AST-025 FR14 (current) allows containment "only on axes that currently overflow" to avoid exactly that dead scroll zone. Gate the claim on scrollHeight > clientHeight.
There was a problem hiding this comment.
Kept on purpose. This is the refresh pull's own containment, not ScrollableArea's, and it follows UIKit: a UIScrollView with a UIRefreshControl always bounces vertically (alwaysBounceVertical), so a list shorter than its scroll view still pulls and refreshes. That is one of this stack's interaction proofs ("a list shorter than the screen still pulls and refreshes").
Without the claim, a pull on a scroll owner that doesn't overflow chains to the page: the page rubber-bands under the pull, and on iOS Safari the page's own reload can fire. The claim is held only near the top, only while a control is mounted and the pull is enabled (iOS only), and it is dropped while a scroll lock holds the page.
If AST-025 FR14 should govern the refresh pull too, that needs an owner decision on the spec, and pulls on short lists would stop working.
6fad50f to
17019f4
Compare
There was a problem hiding this comment.
Approval is on hold for an engineering-owner decision on the draft spec spec:AST-094, the only authority this change cites: as written, its FR4/DEC-1 keep touch physics out of themes and its FR19/FR20 require a recognizer to claim its touches, while this hook plays theme-retunable motion and never claims. An engineering owner can approve the record with those claims amended by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision:
- Merge after #8045, #8062, #8074, #8076, #8086, #8098, #8170, #8257 and #8258 and rebase, since against
mainthis pull request carries 31,776 authored lines, of which 1,461 are its own commit. - Make the controlled
isRefreshingthe single source (useRefreshControl.ts:748-755), because today a pull-triggered refresh stays on while the prop readsfalse. - Apply
overscroll-behavior-y: noneonly while the scroll owner overflows (AST-025 FR14), read the scroll-lock state withuseSyncExternalStore, and document the three new experimental exports withstability: 'experimental'(AST-017 FR21).
[Automated review]
| useEffect(() => { | ||
| if (isRefreshingProp === undefined) { | ||
| return; | ||
| } | ||
| dispatch( | ||
| isRefreshingProp ? {type: 'begin', source: 'program'} : {type: 'end'}, | ||
| ); | ||
| }, [dispatch, isRefreshingProp]); |
There was a problem hiding this comment.
When controlled, this Effect is the only path from isRefreshing into the hook, and it runs only when the prop changes. After a pull triggers, or refresh() runs, with the prop at false, the hook stays refreshing (spinner, aria-busy, 60 px inset) until the prop goes true and then false across two commits, so an app that never commits true for that refresh cannot end it; mounting with true also renders false first (react-component-runtime INV1/INV3 and its controlled-state rule). When controlled, return the prop, and end a triggered refresh on the next commit unless the prop became true.
| const near = | ||
| isPullEnabledRef.current && | ||
| scrollTopOf(current) < Math.max(heightOf(current), REFRESH_HEIGHT); |
There was a problem hiding this comment.
near never checks whether the owner overflows, so a short or empty list in an overflow-y: auto container keeps overscroll-behavior-y: none for as long as the hook is mounted. AST-025 FR14 (current) allows containment only on axes that currently overflow; gate the claim on scrollHeight > clientHeight.
There was a problem hiding this comment.
Kept on purpose. This is the refresh pull's own containment, not ScrollableArea's, and it follows UIKit: a UIScrollView with a UIRefreshControl always bounces vertically (alwaysBounceVertical), so a list shorter than its scroll view still pulls and refreshes. That is one of this stack's interaction proofs ("a list shorter than the screen still pulls and refreshes").
Without the claim, a pull on a scroll owner that doesn't overflow chains to the page: the page rubber-bands under the pull, and on iOS Safari the page's own reload can fire. The claim is held only near the top, only while a control is mounted and the pull is enabled (iOS only), and it is dropped while a scroll lock holds the page.
If AST-025 FR14 should govern the refresh pull too, that needs an owner decision on the spec, and pulls on short lists would stop working.
| useEffect(() => { | ||
| setIsLocked(isScrollLockActive()); | ||
| return subscribeScrollLock(setIsLocked); | ||
| }, []); |
There was a problem hiding this comment.
This Effect sets state from an external store without naming it, and starts from isLocked: false even when a lock already holds, so on a client mount under a lock the edge Effect above claims the top edge for one commit. Reading it during render avoids both:
const isLocked = useSyncExternalStore(subscribeScrollLock, isScrollLockActive, () => false);
17019f4 to
489c802
Compare
There was a problem hiding this comment.
Approval is on hold for an engineering-owner decision on the draft spec spec:AST-094, the only authority this change cites: as written, its FR4/DEC-1 keep touch motion out of themes and its FR19/FR20 require a recognizer to claim its touches, while this hook plays theme-retunable motion and never claims. An engineering owner can approve the record with those claims amended by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision:
- Merge after the nine stacked predecessors and rebase, since against
mainthis pull request carries 31,908 authored lines, of which 1,593 are its own commit; before #8062 lands, its 744 px compact edge needs to follow the theme'smdpoint under the currentspec:AST-012FR9, and #8257'splatformcondition sits outside FR2. - Make the controlled
isRefreshingthe single source (useRefreshControl.ts:846-853), and start the end slide fromend()rather than an Effect that misses a refresh beginning and ending in one render (:679-691). - Apply
overscroll-behavior-y: noneonly while the scroll owner overflows (AST-025 FR14), read the scroll-lock state withuseSyncExternalStore, and document the three new experimental exports withstability: 'experimental'(AST-017 FR21).
[Automated review]
| useEffect(() => { | ||
| if (isRefreshing || pendingEnd.current == null) { | ||
| return; | ||
| } | ||
| endFrame.current = requestAnimationFrame(() => { | ||
| endFrame.current = requestAnimationFrame(() => { | ||
| endFrame.current = null; | ||
| const play = pendingEnd.current; | ||
| pendingEnd.current = null; | ||
| play?.(); | ||
| }); | ||
| }); | ||
| }, [isRefreshing]); |
There was a problem hiding this comment.
This Effect is the only thing that starts the slide end() parks in pendingEnd (:634), and it runs only when isRefreshing changes. When a refresh begins and ends in one render (refresh() from a timer, a network callback or an Effect, with an onRefresh that returns no pending promise, on a scroll owner whose content does not fill it), isRefreshing never changes, so the 60 px inset and the spinner stay on while the hook reports isRefreshing: false. react-component-runtime INV2 keeps work in the handler that can do it: end() can schedule the two frames itself, or this Effect can key on a per-end counter:
const [endCount, setEndCount] = useState(0);
// in end(): pendingEnd.current = ...; setEndCount(count => count + 1);
useEffect(() => {
// the same two frames
}, [endCount]);| export const COMPACT_QUERY = '(width < 744px), (height < 500px)'; | ||
|
|
||
| /** Both: presentations such as a sheet in place of a popover. */ | ||
| export const COMPACT_TOUCH_QUERY = | ||
| '(width < 744px) and (pointer: coarse), (height < 500px) and (pointer: coarse)'; |
There was a problem hiding this comment.
Current AST-012 FR9 makes a screen compact “while its viewport width is below the effective md point (FR1) or its viewport height is below 500”, and says every resolver that chooses a presentation by screen size, this touch resolver and Selector's adaptive presentation included, uses that one definition and “none keeps size numbers of its own” (public-component-api INV17 repeats it). These queries keep their own 744 px edge, so a coarse-pointer screen 744 to 767 px wide and at least 500 px tall, such as an iPad mini in portrait, gets the popover where FR9 gives the sheet, and a theme that moves md is ignored. Derive the width edge from the theme's effective md point, and update the ContextMenu and MoreMenu docs and the touch-presentation Changeset to match.
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
Current AST-012 FR2 closes the when vocabulary to width, height, pointer, contrast and motion, and says “Future environment fields require explicit additions to this closed vocabulary.” platform would be a released defineTheme input that the record does not admit; it needs an owner's FR2 amendment first, or it should leave this stack.
|
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.
|
489c802 to
25043e1
Compare
github-actions
Bot
added
the
needs:design-review
|
Done in 25043e1 (#8175 at 8bfbe7c). No |
There was a problem hiding this comment.
Approval is on hold for an engineering-owner decision on the draft spec spec:AST-094, the only authority this change cites, and on the current spec:AST-025 FR14: as written, AST-094's FR4/DEC-1 keep touch motion out of themes and its FR19/FR20 require a recognizer to claim its touches, and FR14 allows overscroll containment only on axes that overflow, while this hook plays theme-retunable motion, never claims, and keeps overscroll containment on scroll owners that do not overflow. An engineering owner can approve the records with those claims amended by commenting /approve-spec <full head SHA> once the amended records are on the head, and the author may do so as an engineering owner. Independently of that decision:
- Merge after the eleven stacked predecessors and rebase, since against
mainthis pull request carries 33,399 authored lines, of which 1,998 are its own commit; #8257'splatformtheme condition also sits outsidespec:AST-012FR2. - Play the end slide on every end rather than only when
isRefreshingchanges (refreshTouchController.ts:683-695), so arefresh()that settles in its own task cannot leave the inset and the spinner on. - Document the three new experimental exports with
stability: 'experimental'(AST-017 FR21), and name the external system in the Effects atuseRefreshControl.ts:211-231.
[Automated review]
| useEffect(() => { | ||
| if (isRefreshing || pendingEnd.current == null) { | ||
| return; | ||
| } | ||
| endFrame.current = requestAnimationFrame(() => { | ||
| endFrame.current = requestAnimationFrame(() => { | ||
| endFrame.current = null; | ||
| const play = pendingEnd.current; | ||
| pendingEnd.current = null; | ||
| play?.(); | ||
| }); | ||
| }); | ||
| }, [isRefreshing]); |
There was a problem hiding this comment.
This Effect is still the only thing that plays the slide end() parks at line 625, and it runs only when the hook's isRefreshing state changes across a commit. When refresh() runs outside a React event (a timer, a network callback, an Effect) with an onRefresh that settles in a microtask, the begin and the end land in one batch and the state never changes, so on a scroll owner that cannot scroll the inset away the content keeps its 60 px inset and the spinner keeps turning while isRefreshing reads false. end() can schedule its two frames itself, or this Effect can key on a per-end counter that end() bumps; a test that calls refresh() from a timer with a synchronous onRefresh would pin it.
| useEffect(() => { | ||
| if (isRefreshingProp === undefined) { | ||
| return; | ||
| } | ||
| dispatch( | ||
| isRefreshingProp ? {type: 'begin', source: 'program'} : {type: 'end'}, | ||
| ); | ||
| }, [dispatch, isRefreshingProp]); | ||
|
|
||
| // A refresh the hook started (a pull or `refresh()`) under a controlled | ||
| // `isRefreshing`: the parent took it if the prop is now true, otherwise it | ||
| // ends (a parent that keeps false, or sets true then false in one handler). | ||
| useEffect(() => { | ||
| if (!awaitingProp.current) { | ||
| return; | ||
| } | ||
| awaitingProp.current = false; | ||
| if (isRefreshingProp === false) { | ||
| dispatch({type: 'end'}); | ||
| } | ||
| }, [controlledCheck, dispatch, isRefreshingProp]); |
There was a problem hiding this comment.
Both Effects call dispatch, which sets isRefreshing and drives the touch layer's inset and slide and the announcement, but neither comment names the external system it synchronizes, which react-component-runtime INV3 asks of a new Effect. A sentence on each naming the touch layer and the announcer would meet it.
| export { | ||
| claimRootScrollStyle, | ||
| isScrollLockActive, | ||
| subscribeScrollLock, | ||
| } from '../hooks/rootScrollStyles'; | ||
| export type {RootScrollStyle} from '../hooks/rootScrollStyles'; |
There was a problem hiding this comment.
These three functions carry @experimental in their declarations, but no authored doc marks them, and AST-017 FR21 needs both before a surface sits outside the stable promise. A .doc.mjs with stability: 'experimental', as useTouchPresentation and useGestureRecognizer have, saying they may change or be removed in a patch release, would close it.
25043e1 to
525eee0
Compare
There was a problem hiding this comment.
Approval is on hold for an engineering-owner decision on the draft spec spec:AST-094, the only record this change names as its specification, and on the current spec:AST-025 FR14: as written, AST-094's FR4/DEC-1 keep touch motion out of themes and its FR19/FR20 require a recognizer to claim its touches, and FR14 allows overscroll containment only on axes that overflow, while this hook plays theme-resolved motion, never claims, and contains overscroll on scroll owners that do not overflow. An engineering owner can approve the records with those claims amended by commenting /approve-spec <full head SHA> once the amended records are on the head, and the author may do so as an engineering owner. Independently of that decision:
- Land the twelve stacked predecessors and rebase, since against
mainthis pull request carries 35,075 authored lines and its own commit is 2,010. - Start the end slide from
end()itself (refreshTouchController.ts:683-695), so arefresh()that begins and ends in one render cannot leave the inset and the spinner on. - Add real-browser evidence for the Safari, WebKit and Chromium behaviour the hook relies on (
spec:AST-013FR10), with iOS Safari for the reload and rubber-band claims. - Drop the
platformtheme condition or amendspec:AST-012FR2 first, document the three new experimental exports withstability: 'experimental'(spec:AST-017FR21), and name the external system in the Effects atuseRefreshControl.ts:211-231.
[Automated review]
| useEffect(() => { | ||
| if (isRefreshing || pendingEnd.current == null) { | ||
| return; | ||
| } | ||
| endFrame.current = requestAnimationFrame(() => { | ||
| endFrame.current = requestAnimationFrame(() => { | ||
| endFrame.current = null; | ||
| const play = pendingEnd.current; | ||
| pendingEnd.current = null; | ||
| play?.(); | ||
| }); | ||
| }); | ||
| }, [isRefreshing]); |
There was a problem hiding this comment.
This Effect is the only thing that plays the slide end() parks at line 625, and it runs only when the hook's isRefreshing state changes across a commit. An uncontrolled refresh() called outside a React event (a timer, a network callback, an Effect) with an onRefresh that settles within the same task sets isRefreshing to true and back to false before React renders, so this Effect never runs; on a container that cannot scroll the full 60 px, the content keeps its inset and the spinner keeps turning while isRefreshing reads false. Letting end() schedule its two frames itself, or keying this Effect on a counter that end() increments, fixes it; a test that calls refresh() from a timer with a synchronous onRefresh would pin it.
There was a problem hiding this comment.
Fixed in 035eaca: end() now increments a counter that this Effect is keyed on, so a refresh that begins and ends before React renders still slides off its inset two frames after the end commits. New test: refresh() from a timer with an onRefresh that settles at once returns the content to 0.
| * that travel: one write per move, no React render. The scroll owner's | ||
| * `overscroll-behavior-y` is `none` while it is within one viewport of its | ||
| * top, so the browser draws no rubber band (and Safari's own page reload | ||
| * never fires) where the pull is drawn instead. Scrolling stays native. |
There was a problem hiding this comment.
This states browser behaviour (no rubber band, Safari's reload kept away), as do lines 477-480, 620-624 and 731-737, but the hook's only evidence is jsdom. spec:AST-013 FR10 asks for real-browser evidence for paint and browser event timing, and for an iOS Safari check of a Safari claim: a Chromium and WebKit proof of the pull, settle and end on the interaction harness, plus an iOS Safari check, or narrower comments, would meet it.
There was a problem hiding this comment.
The real-browser evidence for these comments is in the component PR stacked on this one (#8175), since a pull needs the component's markup:
- Its interaction spec runs in Chromium and in WebKit (Playwright's container). It covers the pull, the trigger, the settle and the end, and that on the page the document's top edge belongs to the pull (the root's
overscroll-behavior-y). Under a mouse it stays the browser's. - Its iOS 26.2 simulator clips (Mobile Safari, HID touches) include a pull on the page with no reload.
- Its Android clips (Chrome 124 emulator) include a page pull with no reload; it was checked by the page's
performance.timeOriginand navigation type.
This PR keeps the jsdom tests of the mechanism; the comments describe what those proofs show.
| useEffect(() => { | ||
| if (isRefreshingProp === undefined) { | ||
| return; | ||
| } | ||
| dispatch( | ||
| isRefreshingProp ? {type: 'begin', source: 'program'} : {type: 'end'}, | ||
| ); | ||
| }, [dispatch, isRefreshingProp]); | ||
|
|
||
| // A refresh the hook started (a pull or `refresh()`) under a controlled | ||
| // `isRefreshing`: the parent took it if the prop is now true, otherwise it | ||
| // ends (a parent that keeps false, or sets true then false in one handler). | ||
| useEffect(() => { | ||
| if (!awaitingProp.current) { | ||
| return; | ||
| } | ||
| awaitingProp.current = false; | ||
| if (isRefreshingProp === false) { | ||
| dispatch({type: 'end'}); | ||
| } | ||
| }, [controlledCheck, dispatch, isRefreshingProp]); |
There was a problem hiding this comment.
Both Effects call dispatch, which sets isRefreshing, drives the touch layer's inset and slide, and announces, but neither comment names the external system it synchronizes, which react-component-runtime INV3 asks of a new Effect. A sentence on each naming the touch layer and the announcer would meet it.
There was a problem hiding this comment.
Fixed in 035eaca: each Effect now names what it syncs outside React, the touch layer (inset, spin, slide) and the announcer.
| export { | ||
| claimRootScrollStyle, | ||
| isScrollLockActive, | ||
| subscribeScrollLock, | ||
| } from '../hooks/rootScrollStyles'; | ||
| export type {RootScrollStyle} from '../hooks/rootScrollStyles'; |
There was a problem hiding this comment.
These three functions and RootScrollStyle carry @experimental in their declarations, but no authored doc marks them, and spec:AST-017 FR21 needs both before a surface sits outside the stable promise. A .doc.mjs with stability: 'experimental', like useTouchPresentation.doc.mjs, saying they may change or be removed in a patch release, would close it.
| /** | ||
| * The platform's web engine: `ios` is iOS and iPadOS WebKit (every browser | ||
| * there), detected with `@supports (-webkit-touch-callout: none)`; `other` | ||
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
spec:AST-012 FR2 closes the when vocabulary to width, height, pointer, contrast and motion, and says "Future environment fields require explicit additions to this closed vocabulary." platform would be a released defineTheme input the record does not admit, so it needs an owner's FR2 amendment first, or it should leave this stack.
| * // A softer sheet and a slower reduced-motion fade: | ||
| * motion: { | ||
| * springs: { 'springs.sheetPresent': { duration: 0.6, bounce: 0.1 } }, | ||
| * durations: { 'navigation.transitionMs': 300 }, |
There was a problem hiding this comment.
navigation.transitionMs is the navigation transition context's duration (motion/uikit/navigation.ts), not a Reduce Motion fade, and design:reduced-motion DR4 says theme values offer no way to keep a longer animation under reduced motion. Dropping "and a slower reduced-motion fade" keeps the example from promising what DR4 forbids.
…llowance under every primary pointer The useMenuPress record's FR9 puts the trigger's held finger under spec:AST-094 FR29 under every primary pointer (FR43), so a finger on a touchscreen laptop may drift 13 pt before the open, as on a phone. The trigger read (pointer: coarse) as its press started and took the long press's 10 px where the pointer was fine. Three cases fail until it does not: a 12 pt drift opens under a fine pointer too, a (12, 12) settle opens there, and nothing reads a pointer query, the trigger's press included.
…lowance under every primary pointer The useMenuPress record's FR9 governs the trigger's held finger by spec:AST-094 FR29 under every primary pointer (FR43). The trigger now hands the long press FR29's 13 px drift allowance whatever the primary pointer, so it no longer reads (pointer: coarse) as its press starts, and the menus drop their coarsePointer import. ContextMenu keeps the long press's own allowable movement. The changeset says the same.
Strict ESLint reported six unused bindings in files this pull request carries: the host's HAPTIC_SWITCH_ATTRIBUTE import, two unused imports in the pan's tests (scrollProbe, ScreenEdgePanGestureRecognizer), the swipe test's drift, the presented-hit-testing test's gestures, and a GestureHost test's before, which now asserts what it was read for: a second finger's moves leave the pan where it was.
…eal angular speed The release velocity of a rotation whose listener sets rotation = 0 on each change, at 2 degrees every 16 ms, read 4.363 rad/s against the 2.182 rad/s of the same stream without a re-base: the test asserted only more than 1.5. It now asserts the 2.18 rad/s (spec:AST-100 FR2), and fails.
The rotation's velocity tracker samples the raw unwrapped angle, which setting rotation does not move, so the setter's shift of the samples read each re-base as extra turn and doubled the release velocity of a listener that re-bases on every change. The setter no longer touches the tracker.
…mates while it runs The proof read the element's animations 230 ms in, after the dip had ended, and asserted only that every animated key is opacity, which an empty list passes: with the cross-fade removed from the plan (the move still landing), the proof still passed. It now reads the keyframe keys right after the call starts and asserts they include opacity and nothing else; the same mutant fails it. Chromium and WebKit pass. (F1's in-flight read from c648034.)
…roller, and its claimed moves are cancelled under pointer events Two comparisons stop at the shadow boundary, each failing a case: - once the host's listener on a recognizer's inner node has seen the finger, isFingerDownIn reads it as outside a light-DOM scroller around that root, so a scroll under it is not the finger's (FR20); - under inputMode 'pointer-events', a claimed pan on a shadow host cancels none of its finger's moves over the inner content, since the finger's target is the inner node and its touches reach the host retargeted (FR19).
…the flat tree isFingerDownIn and the pointer-events finger match compare with containsAlongFlatTree, as the scroll watch already does: a finger on content inside an open shadow root is down in the scroller around that root, and its touches, retargeted to the shadow host, match its pointer finger, so a claimed pan cancels its moves there (spec:AST-094 FR19, FR20).
…achments asks, and only then The host let the last attachment decide whether an element is an action group (useGestureRecognizer always passes the option), and never recomputed it at a detach: a plain attachment after the group's turned the group off, and a group's attachment released left a recognizer that never asked taking presses under a fine pointer. Fails until the host counts the attachments that ask.
…ents asks The host counts the attachments that pass actionGroup: true and keeps the element a group while one remains, recomputed at each detach. An attachment that passes false, as useGestureRecognizer does by default, no longer turns a group off, and a group's last asking attachment taking its leave ends the group.
… that content The presented hit test takes the document's elementFromPoint, which answers a shadow host for anything inside its open root, and compares with Node.contains, which stops at the boundary: a recognizer on a control inside a moving sheet's shadow root gets no finger, and the tap's click goes to the host. Fails until the hit test descends into open roots and compares along the flat tree.
Where a moving element is drawn away from its layout box, the host hit-tests what is drawn under the finger with the document's elementFromPoint, which answers a shadow host for anything inside its open root. The hit test now descends through each open root's own elementFromPoint and compares with the movers along the flat tree, so a recognizer on a control inside a moving sheet's shadow root gets its finger and the tap clicks that control.
…nt lets go The host's touch-action rules sit in an adopted sheet or a style element per document or shadow root, and were never removed, so a page whose last recognizer detached kept them, against spec:AST-094 FR24 (nothing touch-related remains after the last instance unmounts). Fails until a root's rules go with its last marked element.
…th its last marked element The touch-action writer records which elements it marked in each document or shadow root, and when the last one is unmarked (its last recognizer detached) it removes that root's adopted sheet or style element, so nothing touch-related remains after the last instance unmounts (spec:AST-094 FR24). A later mark writes the rules again; an element marked in another root first lets go of the old one.
… the inline style it reads - The motion registry, the host's engine gate and the README name the presented hit test as an engine workaround (spec:AST-013 FR6) with its review condition (re-measure on each iOS release; remove it once iOS WebKit hit-tests a compositor-moved element where it is drawn) and its regression fixture. - While a curve runs the animator holds the value it starts from inline, so presentation.ts and the README no longer say the inline style holds the end.
…core/internal/touch Core ships the internal entry @astryxdesign/core/internal/touch, which re-exports the gesture primitives for Astryx's own packages, and the Lab-import guard fails a Lab import of core outside core's exports (architecture:react-component-runtime INV13). The README and the barrel's header named internal workspace imports and said nothing here is exported; they now name that entry.
…ill needs the element, and the listeners that cancel The release note, the stream's header, the host's comment and the README said the stream is one passive listener per event type and that a document-capture stopPropagation no longer starves the recognizers. Such a stop never hid a listener on the same node, and some subscriptions cancel. They now say what changed: an app's window-capture stopPropagation, or a document-capture stopImmediatePropagation registered before core, no longer hides a touch; a recognizer that must claim still needs the touch to reach its element; the gesture host's touchend, click and contextmenu cancel while a claimer is attached and the page hold's touchmove while it holds; and the stream stays ahead of an app's window-capture listener only until a type's listener is re-added. The README's list of blocking touchmove listeners gains the claimed touch's target and the document while a claiming element moves.
…default it moves
The {stylexOptions} form of astryxStylex now passes StyleX
treeshakeCompensation: true unless the caller sets it. spec:AST-017 FR3
asks the release note to name the old and new default and the option
that restores the old one; it now does.
…er and for the two stops the input stream fixes - A still finger with sub-pixel jitter over a scroll view keeps its tap and scrolls nothing (spec:AST-094 FR18–FR22). - A tap is still recognized under an app's window-capture stopPropagation, and under a document-capture stopImmediatePropagation registered before core's: the recognizers hear touches on the window's capture phase. Chromium and WebKit pass.
…s once Where nothing that claims is attached, the host's touchend listener is passive and cannot cancel the engine's own click after a retargeted tap clicks what is drawn. The shared click guard swallows that late click whatever is attached; this case pins it, as asked on meta#8293.
…esture host's subscriptions The gesture host subscribes no click: the one cancelling click subscriber is the click guard's, added at a window's first arm and kept. The release note, the stream's header and the gestures README listed click with the host's touchend and contextmenu; they now name the guard's own.
There was a problem hiding this comment.
Approval is on hold for an engineering-owner decision on four current spec claims this refresh pull departs from: spec:AST-094 FR36–FR38 (it starts a refresh while the finger is still down, keeps it when a release moves back and settles from zero velocity), AST-094 FR4 (its springs and durations play as table constants, not motion roles), AST-094 FR43 (it pulls only under a coarse pointer, while FR43 has gestures work for every pointer) and spec:AST-025 FR14 (overscroll-behavior-y: none near the top on scroll owners that do not overflow). An engineering owner settles this by amending those claims for the refresh pull, or by asking for the hook to conform, and records the decision by commenting /approve-spec <full head SHA> on the pull request; the author may do so as an engineering owner. Whatever that decision, these changes are needed:
- Land the stacked predecessors and rebase, since against
mainthe branch carries 32,705 authored lines and 2,141 of them are this pull request's own. - Show the inset and spinner when a controlled refresh is already running as a server-rendered page hydrates on a touch screen (
useRefreshControl.ts:176-178, AST-094 FR13). - Share one
documentscroll listener across instances (refreshTouchController.ts:502-505, FR24) and stop reading layout once the pull has begun (:723-759, FR9). - Add real-browser evidence for the touch layer's named-browser workarounds or narrow them (
spec:AST-013FR10), assert the announcement's channel and update count (spec:AST-009FR4), and name what the four remaining Effects synchronize (architecture:react-component-runtimeINV3).
[Automated review]
| if (isTouchRef.current) { | ||
| touch.begin(); | ||
| } |
There was a problem hiding this comment.
On a server-rendered page whose controlled isRefreshing is already true, this begin runs after the hydrating commit, while useTouchPresentation still reports the pointer presentation, so touch.begin() is skipped; nothing runs it once the presentation turns to touch, so that refresh never shows its inset or spinner on a phone (AST-094 FR13). Running the touch layer's begin() when the pull becomes enabled while the model is refreshing with no inset, with a hydration test, would close it.
| document.addEventListener('scroll', onScroll, { | ||
| capture: true, | ||
| passive: true, | ||
| }); |
There was a problem hiding this comment.
This capture listener is per instance, so N enabled controls hold N document scroll listeners, and the test at useRefreshControl.test.tsx:617-646 pins that; AST-094 FR24's verification fails "a per-instance document or window listener". One module-level listener serving every mounted control would keep it at one.
| const scrolledBack = | ||
| scrollTopOf(owner.current) - geometry.scrollTop0 > 0.5; |
There was a problem hiding this comment.
This reads scrollTop on every move after the pan began, as :723-724, :732, :742 and :750 read style and layout at began, while AST-094 FR9 allows "no layout read after the gesture begins". Resolving the owner and its height at touch-down (onTouchDown already calls resolveOwner()) and taking the offset from the passive scroll listener would remove them.
Depends on #8170 and #8098 (the page's scroll-style owner). Carries #8562 (Core's internal entries) as a dependency; if the entries land differently, this follows her decision. Review only this PR's change: vjeux/astryx@94298d4...e6f4e63
Why
RefreshControl needs a pull that reads a finger at a scroll's top without taking it from native scrolling, so a scroll that runs out turns into the pull, as on iOS.
What led to it
Part of the touch-presentations program (
spec:AST-094). #8170 adds the pull's mapping and state machine; this adds the hook that reads the finger and drives the motion; RefreshControl (#8175) renders it.Options and tradeoffs
Why this choice
Watching keeps native scrolling in charge, as UIKit does. Settle, end and spin run on the compositor. Near the top the scroll owner's
overscroll-behavior-yisnone, so the browser draws no rubber band of its own; the scroll listener lives only while the pull can run, and a scroll lock stands the pull down. Under Reduce Motion the settle and the end cross-fade and the trigger turn is skipped. No haptic: UIKit has none. Callers see nothing new; core's internal entries gain six named re-exports.How to check it
The hook's unit tests drive real touch sequences through the gesture system:
onRefresh, and a promise keeps it refreshing until it settles.isRefreshingdrives the state from the first render.Browser, iOS and Android proofs come with the component (#8175).
Evidence
touch-support-bundlefails because this branch carries fix(useScrollLock): keep the page where it is while an overlay is open #8098's scroll lock; test(core): a touch component's import carries its gesture code, and nothing else does #8671's per-component test replaces it and passes here.CodeBlockHeaderPlugin.test.tsxred on atest-uishard is the race test(richtext): the code block header tests wait for the copy button's tooltip #8569 fixes.pnpm build,build-previews.mjs, the docsite build) passes locally on the same head: the failure is on Vercel's side.Stack (merge in this order): #8360 gesture foundation (carries #8076) → #8086 refresh values → #8170 pull and state machine → this PR → #8175 RefreshControl. It also needs #8098 (useScrollLock v2, on #8360).