Repository navigation
chore(core): a pager that pages like a UIKit paging scroll view - #8146
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
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approval waits for an engineering owner to approve or amend the draft spec:AST-094 this pull request adds, because the pager, gesture and motion code all rest on its FR1–FR23 and DEC-1/DEC-2, including the changes it proposes to spec:AST-025 FR14, architecture:theme-tokens, design:user-states, family:buttons and architecture:interaction-modality. An engineering owner records that decision 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, these changes are needed:
- Land the stack first so this pull request carries only the pager commit instead of 27,423 authored lines across 127 files.
- Leave vertical swipes and pinches that start on a settling track to the browser, since the zero-duration catch claims the touch (
usePager.ts:245). - Stop firing
onIndexChangefrom Effects, and re-attach when the viewport or track node is replaced (usePager.ts:681-702). - Start a drag that catches an overscrolled track from its unbanded position, re-place the track when
setOptionschangesgaporloop, and fire oneonSettleper settle.
[Automated review]
| const catcher = new LongPressGestureRecognizer({ | ||
| minimumPressDuration: 0, | ||
| allowableMovement: Number.POSITIVE_INFINITY, | ||
| shouldBegin: () => settleHandle != null && !isDragging, | ||
| }); | ||
| catcher.recognizeSimultaneouslyWith(pan); |
There was a problem hiding this comment.
While a settle runs, this zero-duration long press begins on touch-down and claims the touch, so the gesture host cancels every touchmove carrying that finger: a vertical swipe or a pinch that starts here never reaches the page, even after the horizontal pan fails. Could the catch stop the settle without claiming, or fail once the drag leaves the pan's axis?
There was a problem hiding this comment.
Superseded by the later review of the same line; fixed in b366394 (see the reply there).
| const current = presentation(); | ||
| stopSettle(); | ||
| isDragging = true; | ||
| dragStart = current; | ||
| dragPosition = current; |
There was a problem hiding this comment.
current is the on-screen (already rubber-banded) position, but the first move rubber-bands dragStart + dx again, so a track caught 100 px past the first page jumps to about 48 px past it. Starting from rubberBandUnclamp(current, 0, max, pageWidth) when not looping keeps it under the finger, as motion/rubberBand.ts describes.
There was a problem hiding this comment.
Superseded by the later review of the same line; fixed in b366394 (see the reply there).
| function setOptions(update: Partial<PagerOptions>): void { | ||
| options = {...options, ...update}; | ||
| if (options.loop !== true && (target < 0 || target > count() - 1)) { | ||
| goTo(clampIndex(target), {animated: false, reason: 'controlled'}); | ||
| } | ||
| } |
There was a problem hiding this comment.
Only a shrinking count moves the track here. A gap change leaves it at the old stride (120 px off on page 3 for 0 → 40 with 402 px pages), and turning loop off on virtual page 7 of 5 clamps to page 4 rather than the shown page 2. Wrapping the target and re-setting x as measure() does would keep the page in place.
There was a problem hiding this comment.
Superseded by the later review of the same line; fixed in b366394 (see the reply there).
| useEffect(() => { | ||
| if (index != null && index !== pager.index) { | ||
| pager.goTo(index, {reason: 'controlled'}); | ||
| } | ||
| }, [pager, index]); |
There was a problem hiding this comment.
goTo reports synchronously, so this Effect fires onIndexChange(i, 'controlled') in response to the index the owner just set (architecture:react-component-runtime INV2, INV3). Applying a controlled change without echoing the callback, and naming the pager this Effect synchronizes, would fix it; the attach Effect above also keys on the ref objects, so a replaced viewport or track is never re-attached (INV6).
There was a problem hiding this comment.
Superseded by the later review of the same line; fixed in b366394 (see the reply there).
PR Analysis Report🔎 Preview ·
|
c0143ed to
5f923b9
Compare
There was a problem hiding this comment.
Approval waits for an engineering owner to approve or amend the draft spec:AST-094 this pull request adds, and to decide four additions no current spec covers: theme motion roles (which AST-094's own FR4 and DEC-1 reject), the platform adaptation condition (spec:AST-012 FR2), the /motion/experimental and /gestures/experimental subpaths (spec:AST-017 FR21), and the new compact-touch query for presentation="adaptive" (component:Selector DEC-1). An engineering owner records that decision 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, these changes are needed:
- Land #8145, #8076 and their predecessors first so this pull request carries only the pager commit instead of 31,213 authored lines across 176 files.
- Leave vertical swipes and pinches that start on a settling track to the browser, since the zero-duration catch claims the touch (
usePager.ts:250-255). - Stop firing
onIndexChangefrom Effects and re-attach a replaced viewport or track (usePager.ts:729-750). - Keep
astryx docs motionfrom promising motion roles no shipped component reads, and reject unknown or wrong-group role ids indefineTheme. - Fix the jump when a drag catches an overscrolled track, the track position after a
gaporloopchange, and the secondonSettleafter a held catch.
[Automated review]
| const catcher = new LongPressGestureRecognizer({ | ||
| minimumPressDuration: 0, | ||
| allowableMovement: Number.POSITIVE_INFINITY, | ||
| shouldBegin: () => settleHandle != null && !isDragging, | ||
| }); | ||
| catcher.recognizeSimultaneouslyWith(pan); |
There was a problem hiding this comment.
While a settle runs, this zero-duration long press recognizes on touch-down and claims the touch, so the gesture host cancels every touchmove of that finger: a vertical swipe or pinch that starts here never reaches the page, even after the horizontal pan fails. Could the catch stop the settle without claiming, or fail once the drag leaves the pan's axis?
There was a problem hiding this comment.
Fixed in b366394: the settle catcher no longer claims the touch (claimsTouches is false), so a vertical swipe or a pinch that starts on a settling track reaches the page. Test: "a vertical swipe that starts on a settling track still reaches the page" (failed before).
| const current = presentation(); | ||
| stopSettle(); | ||
| isDragging = true; | ||
| dragStart = current; | ||
| dragPosition = current; |
There was a problem hiding this comment.
current is the on-screen, already rubber-banded position, and the first move bands dragStart + dx again: a track caught 115 px past the first page jumps to about 56 px on the next 1 px move. Starting from rubberBandUnclamp(current, 0, max, pageWidth) when not looping keeps it under the finger.
There was a problem hiding this comment.
Fixed in b366394: a drag that catches a track past an end starts from rubberBandUnclamp(current, …), so the first move keeps it under the finger. Test: "a drag that catches an overscrolled track starts from under the finger, with no jump" (failed before).
| if (!isDragging && settleHandle == null && animator != null) { | ||
| // Held, then lifted without dragging: settle on the nearest page. | ||
| const position = presentation(); | ||
| const next = selectPage({ | ||
| position, | ||
| velocity: 0, | ||
| stride: stride(), | ||
| count: count(), | ||
| startIndex: stride() > 0 ? Math.round(position / stride()) : target, | ||
| loop: options.loop === true, | ||
| }); | ||
| setTarget(next, 'drag', true); | ||
| settleFrom(position, 0); | ||
| } |
There was a problem hiding this comment.
If the settle finished inside the 85 ms window, rest() already ran; this branch then re-settles on the same page and calls onSettle(i, i) a second time. Re-settling only when the catch timer actually stopped a settle avoids the duplicate.
There was a problem hiding this comment.
Fixed in b366394: the lift re-settles only when the catch actually stopped a settle. Test: "one onSettle per settle when a finger lands as the settle ends and holds" (failed before).
| function setOptions(update: Partial<PagerOptions>): void { | ||
| options = {...options, ...update}; | ||
| if (options.loop !== true && (target < 0 || target > count() - 1)) { | ||
| goTo(clampIndex(target), {animated: false, reason: 'controlled'}); | ||
| } | ||
| } |
There was a problem hiding this comment.
Only a shrinking count moves the track. A gap change from 0 to 40 on page 3 leaves it at −1,200 px while page 3 sits at 1,320 px, and turning loop off on virtual page 7 of 5 lands on page 4 instead of the page shown, 2. Wrapping the target and re-setting x as measure() does would keep the page in place.
There was a problem hiding this comment.
Fixed in b366394: a gap change re-places the track at the target page's new offset, and turning loop off keeps the page shown. Tests: "a gap change keeps the track on its page" and "turning loop off on a virtual page lands on the page shown" (both failed before).
| useEffect(() => { | ||
| if (index != null && index !== pager.index) { | ||
| pager.goTo(index, {reason: 'controlled'}); | ||
| } | ||
| }, [pager, index]); |
There was a problem hiding this comment.
goTo reports synchronously, so this Effect fires onIndexChange(i, 'controlled') for the index the owner just set (architecture:react-component-runtime INV2, INV3). Applying a controlled change without echoing the callback would fix it; the attach Effect above keys on the ref objects, so a replaced viewport or track is never re-attached (INV6).
There was a problem hiding this comment.
Fixed in b366394: a controlled move (the index prop, or a count that clamps the page) is no longer reported back; each Effect names what it synchronizes; attachment follows the viewport and track nodes. Tests: "a controlled index change does not echo onIndexChange" and "attaches again when the viewport element is replaced" (both failed before).
| content: [ | ||
| { | ||
| type: 'prose', | ||
| text: 'On a touch screen, sheets, alerts, menus, navigation and scrolling play the springs, durations and curves measured from iOS. Each value is a motion role named by its table path, such as `springs.sheetPresent` or `timing.curves.easeInOut`. A theme retunes a role through `motion.springs`, `motion.durations` or `motion.curves`, at its root or in an adaptation rule; a role it leaves alone keeps its measured default.', |
There was a problem hiding this comment.
No shipped component reads these roles yet: inside core only the unexported usePager (its three paging.* roles) and the experimental animator resolve them, so a theme that sets springs.sheetPresent changes nothing in a shipped component. Could this section wait until a component reads them, or say which surfaces read which roles?
There was a problem hiding this comment.
Not this PR's code: that section comes from the motion-roles foundation below this commit; this PR only regenerates the table for the three paging.* roles. Routed to its owner.
| const registered = motionRole(id); | ||
| if (registered !== undefined && registered.kind !== kind) { | ||
| throw new Error( | ||
| `${location}: "${id}" is a ${registered.kind}; set it under the matching group.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Nothing on the defineTheme path imports the motion tables, so motionRole(id) is undefined here: springs.sheetPresnt and a duration under a spring role are both written without an error. Validating against a role list available at theme time would catch typos and wrong groups.
There was a problem hiding this comment.
Not this PR's code: role validation in defineTheme belongs to the theme/motion foundation below this commit, and is routed to its owner.
5f923b9 to
b366394
Compare
|
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. |
There was a problem hiding this comment.
Approval waits for an engineering owner to approve or amend the draft spec:AST-094 this pull request adds, and to decide three additions no current spec covers: theme motion roles (which AST-094's own FR4 and DEC-1 reject), the platform adaptation condition (spec:AST-012 FR2), and the /motion/experimental and /gestures/experimental subpaths (spec:AST-017 FR21). An engineering owner records that decision 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, these changes are needed:
- Land #8145, #8076 and their predecessors first so this pull request carries only the pager commit instead of 31,638 authored lines across 176 files.
- Make the compact-touch query follow
spec:AST-012FR9 (width below the effectivemdpoint or height below 500), since 744 px leaves coarse-pointer screens 744–767 px wide and 500 px or taller without the sheet (utils/touchPresentation.ts:75-79), and namepresentation="popover"in its changeset. - Follow
design:reduced-motionunder Reduce Motion: cross-fade the pager's settle andgoToin 200 ms or less instead of keeping the slide (usePager.ts:433,:453), and keep the reduced-motion fade out of the theme roles. - Keep
astryx docs motionfrom promising motion roles that no shipped component reads, and reject unknown or wrong-group role ids indefineTheme.
The pager issues raised on the previous revision are fixed.
[Automated review]
| 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.
These numbers decide when presentation="adaptive" opens the sheet on five shipped components. spec:AST-012 FR9 on main defines compact as a viewport width below the effective md point (768 by default) or a height below 500, and says no resolver keeps size numbers of its own; with 744 px, a 752 × 1000 coarse-pointer tablet now gets the anchored popover instead of the sheet. Could both queries come from FR9's definition instead?
| return; | ||
| } | ||
| follow( | ||
| animator.play({x: toTrackCurve(curve, sign())}, {reducedMotion: 'keep'}), |
There was a problem hiding this comment.
reducedMotion: 'keep' keeps the settle and goTo sliding under Reduce Motion. design:reduced-motion DR2, now current on main, asks a transition started by a touch drag's release to change opacity only and finish in 200 ms or less (and DR3 asks for an immediate change with a fine pointer). Could the pager cross-fade to the target page under reduced motion, with the tests updated to match?
| const registered = motionRole(id); | ||
| if (registered !== undefined && registered.kind !== kind) { | ||
| throw new Error( | ||
| `${location}: "${id}" is a ${registered.kind}; set it under the matching group.`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Nothing on the defineTheme path imports the motion tables, so motionRole(id) is undefined here and this check never runs: springs.sheetPresnt writes --motion-springs-sheet-presnt, and durations: {'springs.sheetPresent': 300} writes a duration into a spring variable, both without an error. Could ids and groups be validated against a role list available at theme time?
| content: [ | ||
| { | ||
| type: 'prose', | ||
| text: 'On a touch screen, sheets, alerts, menus, navigation and scrolling play the springs, durations and curves measured from iOS. Each value is a motion role named by its table path, such as `springs.sheetPresent` or `timing.curves.easeInOut`. A theme retunes a role through `motion.springs`, `motion.durations` or `motion.curves`, at its root or in an adaptation rule; a role it leaves alone keeps its measured default.', |
There was a problem hiding this comment.
No shipped component reads these roles yet: only the unexported usePager and the experimental animator resolve them, so a theme that sets springs.sheetPresent changes nothing a person sees. Could this section wait until a component reads them, or say which surfaces do?
| * `scrollViewWillEndDragging` velocity reads 0 for steady releases at | ||
| * 175 and 200 pt/s and keeps 0.249 pt/ms (2026-10-23) | ||
| */ | ||
| velocityFloor: 225, |
There was a problem hiding this comment.
225 is "the bracket's middle until a finer sweep pins it", but the tag above reads @source measured. The table's convention labels such a value a guess that names the measurement replacing it; could it be marked that way?
|
Agreed: one implementation.
|
b366394 to
974edc7
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner: the gesture system this pull request carries includes four touch behaviours that current spec:AST-094 does not settle, namely a perEventPointer press tracker's touch listeners and an actionGroup's recognizers where the primary pointer is fine (FR19, FR24; #8469 and #8622), counting only a finger's first uncancelable touchmove as the browser taking the touch (FR20; #8456), and keeping a self-cancelled gesture's moving finger claimed until it lifts, which FR14 supports but FR19's exceptions do not list (#8641). Any engineering owner, the author included as one, can approve all four for this head by commenting /approve-spec <full head SHA>, or the spec changes that would cover them can land on main first. Independently of that decision, these changes are still needed:
- Land #8145 and the stack beneath it, or retarget onto #8145, so this pull request carries only the pager's 4,192 lines instead of 29,528 across 155 files, and leave the
astryxStylexfix to #8075, which carries the same files. - In
usePager, keeponSettleout of the Effect and its cleanup when an unmount, a replaced node orisPanEnabledends a drag that rests on a page, and observe the track whenpagesbecomes'children'after attach. - Remove the guessed
selectPagepaging rule or point it at the measured table, fix the three comments that misstate the code (motion/paging.ts,utils/pressFeedbackImports.test.ts,utils/pressFeedback.ts), and add real-device recordings of the pager in iOS Safari and Android Chrome.
[Automated review]
| detachTouch = () => { | ||
| detachCatch(); | ||
| detachPan(); | ||
| removeCatchListener(); | ||
| removePanListener(); | ||
| }; |
There was a problem hiding this comment.
detachPan() runs before removePanListener(): the host's detach cancels a live pan (gestures/GestureHost.ts:522-526), this pager's listener then runs the release, and a drag that rests on a page settles with no steps, so rest() calls onSettle synchronously from the unmount cleanup or the attach Effect. That breaks the header's promise at :50-52 and architecture:react-component-runtime INV2. Removing the listeners before detaching the recognizers (or dropping the cancel's release while detaching) would keep it out.
| // Synchronizes the pan recognizer (the gesture host) with isPanEnabled. | ||
| useEffect(() => { | ||
| pager.pan.setEnabled(isPanEnabled); | ||
| }, [pager, isPanEnabled]); |
There was a problem hiding this comment.
setEnabled(false) cancels a live pan (gestures/GestureRecognizer.ts:637-639) from this Effect, and a drag resting on a page then calls onSettle, plus a waiting onIndexChange, synchronously here. Routing the toggle through the pager so it runs inside deferringCallbacks would move them to a frame, as the other owner-caused callbacks already are.
| const unobserve = observeResize(nextViewport, measure); | ||
| // Children pages also move when the track's own size changes (a child | ||
| // added, removed or resized). | ||
| const unobserveTrack = isChildPages() | ||
| ? observeResize(nextTrack, measure) | ||
| : () => {}; |
There was a problem hiding this comment.
The track is observed only when pages: 'children' is already set at attach. Switching pages later goes through applyOptions (:1075-1104), which measures once and installs no observer, so a child added, removed or resized afterwards leaves stale offsets until the viewport resizes or a drag begins, against the option's own contract (:212-214).
| * distance, from the current offset. A theme's duration and curve | ||
| * (resolveMotion) replace the table's. |
There was a problem hiding this comment.
No theme reaches this: the only caller passes the paging table's constants (hooks/usePager.ts:1060-1068), and spec:AST-094 FR4 keeps touch motion internal (DEC-11). Saying that goTo plays the table's internal constants would match the code (and the test named “takes a theme's duration and curve”).
| * @input The source of utils/pressFeedback.ts and every module it imports | ||
| * @output The press feedback controller's import graph reaches no module | ||
| * under gestures/: a page whose only touch work is press feedback carries | ||
| * none of the gesture system (`spec:AST-094` FR24, FR43: pointer queries |
There was a problem hiding this comment.
spec:AST-094 has no FR43: its requirements end at FR42. Citing FR24 alone, or the record that states the pointer-query rule, would point readers at a real requirement.
|
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. |
There was a problem hiding this comment.
This pull request isn't ready to approve yet. Two gesture behaviours it carries lack current spec authority: counting only a finger's first uncancelable touchmove as the browser taking the touch (against spec:AST-094 FR20, proposed in #8456) and keeping a self-cancelled gesture's moving finger claimed until it lifts (not among FR19's three exceptions, #8641). An engineering owner needs to approve both by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or those amendments need to land on main first. Independently of that decision, please address the following:
- Land the stack beneath this pull request or retarget it onto #8145, and leave the
astryxStylexfix to #8075, since its 32,114 authored lines carry far more than the pager. - Attach the pager's recognizers for every pointer, with the pan taking wheel and trackpad scrolling, as current AST-094 FR19 and FR43 require.
- Read
goTo's duration and curve through motion roles the theme's motion knobs reach (FR4, DEC-15). - Keep
onSettleout of the Effect when an unmount orisPanEnabledends a drag resting on a page, and observe the track whenpagesbecomes'children'after attach. - Add real-device recordings of the pager in iOS Safari and Android Chrome.
[Automated review]
| // The touch presentation (a coarse primary pointer): the recognizers | ||
| // attach only there (spec:AST-094 FR19). | ||
| const isTouch = useMediaQuery(TOUCH_QUERY); |
There was a problem hiding this comment.
Current spec:AST-094 FR19 reads “Recognizers attach for every pointer, with FR43's exceptions”, FR43 adds that “a pan also takes wheel and trackpad scrolling”, and architecture:react-component-runtime INV13 says the gesture host attaches “with no touch-only query”. Gating the attach on TOUCH_QUERY leaves a mouse, a trackpad or a touchscreen laptop with no drag or swipe at all. Attach the pan and the catch on every pointer and drop the gate; the header at :56-58 and the tests at usePager.test.ts:948, :978 and :1573 still follow FR19's earlier wording.
| play( | ||
| solveAnimatedPageScroll({ | ||
| from, | ||
| to, | ||
| durationMs: paging.animatedScroll.durationMs, | ||
| curve: paging.animatedScroll.curve, | ||
| velocity, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
This plays the paging table's 300 ms and its bézier as literals. Current AST-094 FR4: “A component MUST read touch motion through these roles and MUST NOT play a literal. A theme's knob values MUST reach the motion at its start”; DEC-15 supersedes the DEC-11 the motion modules cite, and spec:AST-108 FR4 keeps only finger tracking, momentum, the rubber band and the other listed motion fixed, not a programmatic page move. Register the animated scroll's duration and curve as motion roles and read them through resolveMotion, so a theme's tempo reaches goTo as it reaches every other touch motion.
| detachTouch = () => { | ||
| detachCatch(); | ||
| detachPan(); | ||
| removeCatchListener(); | ||
| removePanListener(); | ||
| }; |
There was a problem hiding this comment.
detachCatch() and detachPan() run while onCatch and onPan are still subscribed, and the host's detach cancels a live recognizer (gestures/GestureHost.ts:608-612). A drag resting on a page (a slow pan's first frames, its translation leaving out the hysteresis) then settles with no steps, so rest() calls onSettle, and a waiting onIndexChange, synchronously from the unmount's or the replaced node's Effect, against INV2 and the header's promise at :50-52. Remove the listeners first, as useGestureRecognizer keeps its own cancel quiet (gestures/useGestureRecognizer.ts:123, :159).
| // Synchronizes the pan recognizer (the gesture host) with isPanEnabled. | ||
| useEffect(() => { | ||
| pager.pan.setEnabled(isPanEnabled); | ||
| }, [pager, isPanEnabled]); |
There was a problem hiding this comment.
setEnabled(false) cancels a live pan synchronously (gestures/GestureRecognizer.ts:631-640), so turning isPanEnabled off while a drag rests on a page delivers onSettle from inside this Effect (INV2). Run it inside deferringCallbacks, or behind the same guard useGestureRecognizer uses (:136), and add a test that toggles it mid-drag.
| const unobserve = observeResize(nextViewport, measure); | ||
| // Children pages also move when the track's own size changes (a child | ||
| // added, removed or resized). | ||
| const unobserveTrack = isChildPages() | ||
| ? observeResize(nextTrack, measure) | ||
| : () => {}; |
There was a problem hiding this comment.
The track is observed only when pages: 'children' is already set at attach. Switching pages later goes through applyOptions (:1075-1104), which measures once and installs no observer, and the attach Effect re-runs only for a new node or a pointer change, so a child added, removed or resized afterwards leaves stale page offsets, against the option's contract at :212-214. Install the track observer when pages changes, or always observe the track.
| for (const other of this.arena?.involvedRecognizers ?? []) { | ||
| if ( | ||
| other !== this && | ||
| other instanceof PanGestureRecognizer && |
There was a problem hiding this comment.
INV13: “a recognizer is told apart by its kind field, never by instanceof”, and its verification fails “an instanceof check on a recognizer”. The host already tells a pan apart by its family (isPanRecognizer, gestures/GestureHost.ts:299-307); the pager can do the same.
| * distance, from the current offset. A theme's duration and curve | ||
| * (resolveMotion) replace the table's. |
There was a problem hiding this comment.
No theme duration or curve reaches this: the only caller passes the paging table's constants (hooks/usePager.ts:1060-1068), and resolveMotion returns the value it is handed. Either route goTo through motion roles (AST-094 FR4) or say that it plays the table's values.
| if (!event.cancelable) { | ||
| // The browser no longer asks the page about these fingers. WebKit | ||
| // stops asking when it takes the touch to pan (its pointercancel comes | ||
| // with it); Chromium also stops after one unconsumed touchmove, even | ||
| // when `touch-action` keeps it from panning the way the finger went (a | ||
| // horizontal drag under pan-y). So a finger's FIRST uncancelable move | ||
| // counts as the browser's claim only if the browser could pan along | ||
| // the finger's main axis since touch-down; later ones add nothing, and | ||
| // pointercancel or a scroll still report any real pan. | ||
| const placed = new Map(inputs.map(input => [input.id, input])); | ||
| const taken = ids.filter(id => { | ||
| const touch = this.arena.getTouch(id); | ||
| if ( | ||
| touch == null || | ||
| touch.takenByBrowser || | ||
| this.arena.isClaimed(id) || | ||
| touch.scroll?.allowsNativePan === false || | ||
| this.judgedUncancelable.has(id) | ||
| ) { | ||
| return false; | ||
| } | ||
| this.judgedUncancelable.add(id); | ||
| const input = placed.get(id); | ||
| if (input == null || touch.scroll == null) { | ||
| return true; | ||
| } | ||
| const dx = input.x - touch.startX; | ||
| const dy = input.y - touch.startY; | ||
| return Math.abs(dx) >= Math.abs(dy) | ||
| ? touch.scroll.canScroll(dx, 0) | ||
| : touch.scroll.canScroll(0, dy); | ||
| }); | ||
| if (taken.length > 0) { | ||
| this.browserTook(taken, time); |
There was a problem hiding this comment.
Current AST-094 FR20 says “an uncancelable touchmove … MUST count as the browser taking the touch”; counting only a finger's first one, and only where the browser could pan along it, is what open #8456 proposes and an engineering owner has not approved, so this waits on that decision.
|
There was a problem hiding this comment.
This pull request isn't ready to approve yet. Two gesture behaviours it carries lack current spec authority: counting only a finger's first uncancelable touchmove as the browser taking the touch changes spec:AST-094 FR20 (the update proposed in #8456), and keeping a self-cancelled gesture's moving finger claimed until it lifts is not one of FR19's three exceptions (#8641), so an engineering owner needs to approve both by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or those spec updates need to land on main first. This push fixes the callbacks from an unmount or isPanEnabled and the children-pages track observer; independently of the spec decision, please also address the following:
- Land the stack beneath this pull request or retarget it onto #8145, and leave the
astryxStylexfix to #8075, since its 32,287 authored lines carry far more than the pager. - Attach the pager's recognizers for every pointer, with the pan taking wheel and trackpad scrolling, as current AST-094 FR19 and FR43 require.
- Read
goTo's duration and curve through motion roles the theme's knobs reach (FR4, DEC-15), and tell a pan apart by its family rather thaninstanceof(INV13). - Add real-device recordings of the pager in iOS Safari and Android Chrome.
[Automated review]
| const [reported, setReported] = useState(0); | ||
| // The touch presentation (a coarse primary pointer): the recognizers | ||
| // attach only there (spec:AST-094 FR19). | ||
| const isTouch = useMediaQuery(TOUCH_QUERY); |
There was a problem hiding this comment.
Current spec:AST-094 FR19 reads “Recognizers attach for every pointer, with FR43's exceptions”, FR43 adds that “a pan also takes wheel and trackpad scrolling”, and architecture:react-component-runtime INV13 says the gesture host attaches “with no touch-only query”. Gating the pan and the catch on TOUCH_QUERY leaves a mouse, a trackpad or a touchscreen laptop with no drag at all. Attach on every pointer and drop this gate, with the tests at usePager.test.ts:985, :1015 and :1721 that assert it.
| play( | ||
| solveAnimatedPageScroll({ | ||
| from, | ||
| to, | ||
| durationMs: paging.animatedScroll.durationMs, | ||
| curve: paging.animatedScroll.curve, | ||
| velocity, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
This plays the paging table's 300 ms and its bézier as literals. Current AST-094 FR4: “A component MUST read touch motion through these roles and MUST NOT play a literal. A theme's knob values MUST reach the motion at its start”; DEC-15 supersedes the DEC-11 that motion/resolveMotion.ts cites. Read the duration and curve through motion roles so a theme's tempo reaches goTo.
| for (const other of this.arena?.involvedRecognizers ?? []) { | ||
| if ( | ||
| other !== this && | ||
| other instanceof PanGestureRecognizer && |
There was a problem hiding this comment.
INV13: “a recognizer is told apart by its kind field, never by instanceof”, and its verification fails “an instanceof check on a recognizer”. The host already tells a pan apart by its family (gestures/GestureHost.ts:299-307); read that here.
| * distance, from the current offset. A theme's duration and curve | ||
| * (resolveMotion) replace the table's. |
There was a problem hiding this comment.
No theme duration or curve reaches this: the only caller passes the paging table's constants (hooks/usePager.ts:1066-1074). Either route goTo through motion roles (FR4) or drop this sentence.
| if (!event.cancelable) { | ||
| // The browser no longer asks the page about these fingers. WebKit | ||
| // stops asking when it takes the touch to pan (its pointercancel comes | ||
| // with it); Chromium also stops after one unconsumed touchmove, even | ||
| // when `touch-action` keeps it from panning the way the finger went (a | ||
| // horizontal drag under pan-y). So a finger's FIRST uncancelable move | ||
| // counts as the browser's claim only if the browser could pan along | ||
| // the finger's main axis since touch-down; later ones add nothing, and | ||
| // pointercancel or a scroll still report any real pan. | ||
| const placed = new Map(inputs.map(input => [input.id, input])); | ||
| const taken = ids.filter(id => { | ||
| const touch = this.arena.getTouch(id); | ||
| if ( | ||
| touch == null || | ||
| touch.takenByBrowser || | ||
| this.arena.isClaimed(id) || | ||
| touch.scroll?.allowsNativePan === false || | ||
| this.judgedUncancelable.has(id) | ||
| ) { | ||
| return false; | ||
| } | ||
| this.judgedUncancelable.add(id); | ||
| const input = placed.get(id); | ||
| if (input == null || touch.scroll == null) { | ||
| return true; | ||
| } | ||
| const dx = input.x - touch.startX; | ||
| const dy = input.y - touch.startY; | ||
| return Math.abs(dx) >= Math.abs(dy) | ||
| ? touch.scroll.canScroll(dx, 0) | ||
| : touch.scroll.canScroll(0, dy); | ||
| }); | ||
| if (taken.length > 0) { | ||
| this.browserTook(taken, time); |
There was a problem hiding this comment.
Current AST-094 FR20 says “an uncancelable touchmove … MUST count as the browser taking the touch”; counting only a finger's first one, and only where the browser could pan along the finger's main axis, is the rule open #8456 proposes. It needs an engineering owner's approval for this head, or #8456 on main.
| } | ||
| if (next === 'cancelled' || next === 'failed') { | ||
| if (next === 'cancelled') { | ||
| this.releaseClaim(recognizer, this.movingIds); |
There was a problem hiding this comment.
Keeping a self-cancelled gesture's moving finger claimed (this.movingIds) keeps its touchmoves cancelled with no recognizer active on it. FR14 supports it, but FR19's three exceptions to “A touchmove of a touch no recognizer owns MUST NOT be cancelled” do not include it; it waits on an engineering owner's approval or an FR19 amendment (#8641).
There was a problem hiding this comment.
This pull request isn't ready to approve yet. It carries three spec changes from #8360 that current spec doesn't contain: the spec:AST-094 FR11 sentence that makes a tap on moving content activate what is drawn (current FR11 keeps the browser's click), the FR19 sentence that lets the document carry a non-passive touchmove (current FR19 allows that listener only on an element with a claiming recognizer), and the architecture:react-component-runtime INV13 sentence on the shared input stream. An engineering owner needs to approve them by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or they need to land on main first. Independently of that decision, please:
- Attach the pager's recognizers for every pointer, with the pan following a trackpad swipe, as current FR19 and FR43 require, and fix the comments and tests that cite FR19 for a coarse-only attach.
- Read
goTo's duration and curve through a motion role so a theme's tempo reaches it (FR4,spec:AST-108FR3), and drop themotion/paging.tscomment that says a theme already does. - Land #8360, #8676 and #8145 first (this change is 24,866 authored lines), extend and call
useAttachmentinstead of copying it inusePager.ts, and keep.changeset/input-stream.md's[fix]to the press controller change that ships. - Add real-device recordings for the iOS Safari and Android Chrome claims, as AST-094's platform support asks.
[Automated review]
| attached.touch = isTouch; | ||
| attached.detach = | ||
| viewport != null && track != null | ||
| ? pager.attach(viewport, track, {touch: isTouch}) |
There was a problem hiding this comment.
Current spec:AST-094 FR19 reads “Recognizers attach for every pointer, with FR43's exceptions”, and FR43 adds “A pager follows a trackpad swipe”. Passing touch: isTouch leaves a mouse drag, a trackpad swipe and a finger on a touchscreen laptop with nothing attached. Attach for every pointer, with the pan taking allowedScrollTypes: ['continuous'], and correct the header and tests that cite FR19 for the coarse-only rule.
| play( | ||
| solveAnimatedPageScroll({ | ||
| from, | ||
| to, | ||
| durationMs: paging.animatedScroll.durationMs, | ||
| curve: paging.animatedScroll.curve, | ||
| velocity, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
This plays paging.animatedScroll's 300 ms and bézier as literals. FR4: “A component MUST read touch motion through these roles and MUST NOT play a literal. A theme's knob values MUST reach the motion at its start”; with spec:AST-108 FR3, a theme at tempo 1.5 would slow every other touch motion but not this one. Register the role and resolve it through resolveMotion.
| const attachedRef = useRef<{ | ||
| viewport: HTMLElement | null; | ||
| track: HTMLElement | null; | ||
| touch: boolean; | ||
| detach: (() => void) | null; | ||
| }>({viewport: null, track: null, touch: false, detach: null}); | ||
| useIsomorphicLayoutEffect(() => { | ||
| const viewport = viewportRef.current; | ||
| const track = trackRef.current; | ||
| const attached = attachedRef.current; | ||
| if ( | ||
| viewport === attached.viewport && | ||
| track === attached.track && | ||
| isTouch === attached.touch | ||
| ) { | ||
| return; | ||
| } | ||
| attached.detach?.(); | ||
| attached.viewport = viewport; | ||
| attached.track = track; | ||
| attached.touch = isTouch; | ||
| attached.detach = | ||
| viewport != null && track != null | ||
| ? pager.attach(viewport, track, {touch: isTouch}) | ||
| : null; | ||
| }); | ||
| useIsomorphicLayoutEffect( | ||
| () => () => { | ||
| attachedRef.current.detach?.(); | ||
| attachedRef.current = { | ||
| viewport: null, | ||
| track: null, | ||
| touch: false, | ||
| detach: null, | ||
| }; | ||
| }, | ||
| [pager], | ||
| ); |
There was a problem hiding this comment.
This re-implements gestures/useAttachment.ts (follow the ref's node after each commit, re-attach on a key change, release at unmount) for two nodes and a key. Extending useAttachment to take the viewport and track pair keeps one copy of that lifecycle.
| * distance, from the current offset. A theme's duration and curve | ||
| * (resolveMotion) replace the table's. |
There was a problem hiding this comment.
No theme duration or curve reaches this: the only caller passes the paging table's constants (hooks/usePager.ts:1072-1080), and motion/uikit/paging.ts says there is no theme role. Either route goTo through a role or drop this sentence.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The gesture recognizers now hear page-wide input through one shared stream on the window's capture phase, where the touch press feedback's pointer listeners already were, so an app's window-capture `stopPropagation`, or a document-capture `stopImmediatePropagation` registered before Core, no longer hides a touch from them. The press feedback's scroll and visibility listeners join the same stream. A recognizer that must claim a touch mid-gesture still needs the touch to reach its element. Each event type's listener is passive unless a subscriber cancels it (the gesture host's `touchend` and `contextmenu` while a claiming recognizer or press tracker is attached, the page hold's `touchmove` while it holds, and the click guard's `click` while it is armed, and from the menu press model's first arm on), so a page of pressables registers no listener that can hold a scroll. |
There was a problem hiding this comment.
No Core or Lab component attaches a gesture recognizer at this head (the only caller is the unexported usePager), so this [fix] tells consumers about a change they cannot meet. Keep [fix] for what ships, the press controller's scroll and visibility listeners moving to the window's capture phase, and move the recognizer part to a [chore] line.
|
Stacked on #8676 → #8360; the AST-094 sentences are #8360's. FR19's every-pointer attach is #8692 (stacked on this). Fixed in df42226: |
There was a problem hiding this comment.
Changes are needed before this can be approved. The pager follows spec:AST-094's paging rules, but it attaches its recognizers only under a coarse pointer, while FR19 and FR43 require every pointer, and the diff still carries #8360, #8676 and #8145 (the FR11, FR19 and INV13 spec sentences are decided with #8360).
- Attach the pager's pan and catch for every pointer, with the trackpad, or stop labelling the touch-only stage as FR19 in
usePager.tsand its tests. - Land or retarget onto the stack so this diff is the pager commit.
- Have
useFingerScrollcalluseAttachmentinstead of copying its lifecycle, and keep the[fix]in.changeset/input-stream.mdto what ships. - Add real-device iOS Safari and Android Chrome recordings of the busy-thread settle and the catch.
[Automated review]
| useAttachmentPair( | ||
| viewportRef, | ||
| trackRef, | ||
| (viewport, track) => pager.attach(viewport, track, {touch: isTouch}), |
There was a problem hiding this comment.
This attaches the pan and the catch only under a coarse pointer, but spec:AST-094 FR19 says "Recognizers attach for every pointer, with FR43's exceptions" and FR43 says "A pager follows a trackpad swipe". A mouse drag, a trackpad swipe or a finger on a touchscreen laptop would page nothing. Attach for every pointer (with the pan's allowedScrollTypes: ['continuous']), or stop citing FR19 for this stage here, at :57-59, :1162, :1236-1237 and in the tests.
| const attachedRef = useRef<{node: Node; detach: () => void} | null>(null); | ||
| // Follows the node on every commit: a ref that now points at another | ||
| // element moves the watch to it. | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
These two Effects copy useAttachment's lifecycle (useAttachment.ts:115-138: follow the node after each commit, release and re-attach, release at unmount). Let useAttachment take a Document (or a node getter) and call it once here, so a fix to one cannot miss the other.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The gesture recognizers now hear page-wide input through one shared stream on the window's capture phase, where the touch press feedback's pointer listeners already were, so an app's window-capture `stopPropagation`, or a document-capture `stopImmediatePropagation` registered before Core, no longer hides a touch from them. The press feedback's scroll and visibility listeners join the same stream. A recognizer that must claim a touch mid-gesture still needs the touch to reach its element. Each event type's listener is passive unless a subscriber cancels it (the gesture host's `touchend` and `contextmenu` while a claiming recognizer or press tracker is attached, the page hold's `touchmove` while it holds, and the click guard's `click` while it is armed, and from the menu press model's first arm on), so a page of pressables registers no listener that can hold a scroll. |
There was a problem hiding this comment.
No Core or Lab component attaches a gesture recognizer at this head, so this [fix] announces a recognizer fix no consumer can meet. Keep [fix] for what ships (the press feedback's scroll and visibility listeners joining the window's capture-phase stream) and move the recognizer sentence to a [chore] line.
|
Every-pointer attach (FR19) and trackpad following (FR43) are now in this PR at 9d14d11, with the pan on |
There was a problem hiding this comment.
Changes are needed before this can be approved. The pager now takes a mouse drag and a trackpad swipe as spec:AST-094 FR19 and FR43 require, but a trackpad swipe still breaks FR43's pager rules, and the diff still carries #8360, #8676 and #8145 (25,476 authored lines) and no longer merges cleanly with main.
- Cap a trackpad swipe at one page from where it began, ignore a speed spike under a tenth of a page, and start a swipe during a settle from that settle's page (the rules staged in #8692); the browser proof that expects the track to pass the next page should then flip.
- Let
useFingerScrollcall the shared attachment lifecycle inuseAttachment.tsinstead of its own copy, and keep.changeset/input-stream.md's[fix]to what ships today. - Add real-device recordings (an iPhone in Safari, an Android phone in Chrome) for the Mobile Safari claims the pager relies on.
[Automated review]
| // The pan's `scrollVelocity()`: spec:AST-100's one filter, read as | ||
| // UIScrollView reads it. A finger still for more than 40 ms before the | ||
| // lift releases at 0 (FR2), where UIScrollView keeps the speed. The | ||
| // content moves against the finger in LTR (sign()). | ||
| const velocity = | ||
| state === 'ended' ? sign() * recognizer.scrollVelocity().x : 0; | ||
| const next = releasePage(dragPosition, velocity, dragStartIndex); | ||
| setTarget(next, 'drag', true); | ||
| settleFrom( | ||
| dragPosition, | ||
| pagingSettleVelocity(velocity), | ||
| pan.pointerType === 'touch', | ||
| ); |
There was a problem hiding this comment.
A trackpad swipe releases here like a finger's: any velocity of 300 px/s or more pages, whatever the travel. spec:AST-094 FR43 says a pager commits "never on a speed spike that moved it less than a tenth of a page", and follows a swipe "never more than one page from where the swipe began" (the browser proof at usePager.interaction.browser.spec.ts:1360 asserts the opposite). Folding in #8692's cap and spike guard here would make this head conform.
| pageWidth > 0 ? pageWidth : 1, | ||
| ); | ||
| dragPosition = current; | ||
| dragStartIndex = nearestPage(current); |
There was a problem hiding this comment.
A swipe that lands during a settle starts from the nearest page, so a second swipe before the track passes halfway lands on the page the first was already heading for. FR43: "a swipe that lands during a settle starts from that settle's page, so back-to-back swipes page on".
| const attachedRef = useRef<{node: Node; detach: () => void} | null>(null); | ||
| // Follows the node on every commit: a ref that now points at another | ||
| // element moves the watch to it. | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
These two Effects still copy the lifecycle useAttachment.ts:103-139 (useAttachedNodes) owns, which this head extended for the pager's pair. Let it take a Document (or a node getter) and call it once here, so a fix to one cannot miss the other.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The gesture recognizers now hear page-wide input through one shared stream on the window's capture phase, where the touch press feedback's pointer listeners already were, so an app's window-capture `stopPropagation`, or a document-capture `stopImmediatePropagation` registered before Core, no longer hides a touch from them. The press feedback's scroll and visibility listeners join the same stream. A recognizer that must claim a touch mid-gesture still needs the touch to reach its element. Each event type's listener is passive unless a subscriber cancels it (the gesture host's `touchend` and `contextmenu` while a claiming recognizer or press tracker is attached, the page hold's `touchmove` while it holds, and the click guard's `click` while it is armed, and from the menu press model's first arm on), so a page of pressables registers no listener that can hold a scroll. |
There was a problem hiding this comment.
No Core or Lab component attaches a gesture recognizer at this head, so this [fix] announces a recognizer change no consumer can meet. Keep [fix] for the press feedback's scroll and visibility listeners joining the window's capture-phase stream, and move the recognizer sentence to a [chore] line.
There was a problem hiding this comment.
Changes are needed before this can be approved. The pager now follows spec:AST-094 FR43's trackpad swipe rules for evenly spaced pages, but the diff still carries #8360, #8676 and #8145 (25,726 authored lines), and a few fixes remain in the code it carries.
- Land the stack beneath first (or retarget onto #8145) so this pull request is the pager commit.
- Let
useFingerScrollcall the shared attachment lifecycle inuseAttachment.tsinstead of keeping its own copy. - Measure the trackpad spike guard in pages rather than in viewport widths, so
pages: 'children'follows FR43, and add a test for it. - Add real-device recordings (an iPhone in Safari, an Android phone in Chrome) for the Mobile Safari and Android Chrome claims, as AST-094's platform support asks.
[Automated review]
| if ( | ||
| isScrollContact(recognizer) && | ||
| Math.abs(dragPosition - offsetOf(dragStartIndex)) < | ||
| STILL_RELEASE_FRACTION * pageWidth | ||
| ) { | ||
| velocity = 0; | ||
| } |
There was a problem hiding this comment.
pageWidth is the viewport's width, but with pages: 'children' a page is a child's spacing, so a fast trackpad swipe of 30 px over 160 px cards in a 390 px viewport (19% of a page) releases still, while spec:AST-094 FR43 only stills a spike that "moved it less than a tenth of a page". Measuring in pages would fix it: Math.abs(pageAt(dragPosition) - pageAt(offsetOf(dragStartIndex))) < STILL_RELEASE_FRACTION, with a children case beside the 42 px spike test.
| const attachedRef = useRef<{node: Node; detach: () => void} | null>(null); | ||
| // Follows the node on every commit: a ref that now points at another | ||
| // element moves the watch to it. | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
These two Effects still copy the lifecycle useAttachment.ts:103-139 (useAttachedNodes) owns. Let it take a Document (or a node getter) and call it once here, so a fix to one cannot miss the other.
|
Fixed in 4d92fe5: the trackpad spike guard measures the swipe in its pages' own spacing ( #8360 and #8676 have merged; #8145 is still open beneath this PR. This head adds one commit (the compare link at the top), and the PR becomes the pager commit alone once #8145 lands. |
|
/approve-spec 4d92fe5 |
There was a problem hiding this comment.
Changes are needed before this can be approved. The paging rule and DEC-25 are now owner-approved, and the pager follows them, including FR43's trackpad rules for children pages.
- Add
useAttachmentto the WebKit interaction job's path filter in.github/workflows/ci.yml(line 2419): the pager now importshooks/useAttachment.ts, so the routing test "keeps everything the pager bundle bundles in scope" fails and the requiredtestcheck is red. - Land #8145 first so this diff is the pager commit alone, and declare that remainder a behavior-module pull request; today the diff is 6,588 authored lines that carry #8145.
- Add real-device evidence, such as a real iPhone Safari recording, for the two Mobile Safari behaviours the pager relies on (the freeze behind the two-frame report delay and the unpainted pages behind the layer advice,
usePager.ts:452-455and:779-786), as AST-094's Platform support requires.
Two small non-blocking notes are inline.
[Automated review]
| const latestRef = useRef(options); | ||
| // The controlled `index` of the last commit, read when the pager reports. | ||
| const indexRef = useRef(index); | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
Non-blocking: this Effect names nothing it synchronizes (architecture:react-component-runtime INV3), and the comment above it describes only indexRef. The sibling hooks add a line such as: "Syncs the callback and controlled-index refs with the newest render; the pager's callbacks read them."
| // The pan's `scrollVelocity()`: the shared release filter, read as | ||
| // UIScrollView reads it. A finger still for more than 40 ms before the | ||
| // lift releases at 0, where UIScrollView keeps the speed. The content | ||
| // moves against the finger in LTR (sign()). |
There was a problem hiding this comment.
Non-blocking (INV29): this restates scrollVelocity() and sign() and hard-codes the release filter's internal 40 ms. Keeping only the quirk would do: "A still finger releases at 0, where UIScrollView keeps the speed."
commented
Nov 2, 2026
|
Decision on item 3 (posted by the owner's spec agent): the real-device evidence for the pager's two Mobile Safari behaviors is the owner's own test of this pager on a real iPhone, on the preview, on 2026-11-01 ("phone good"), together with the simulator recordings and the CI WebKit lane. That meets AST-094's Platform support for merge. More device recordings can follow after merge on the device page; they do not block this PR. |
commented
Nov 2, 2026
|
/approve-spec 4d92fe5 |
commented
Nov 2, 2026
|
Fixed in 599c5a7:
|
commented
Nov 2, 2026
|
/approve-spec 599c5a7 |
left a comment
There was a problem hiding this comment.
Changes are needed before this can be approved. The AST-094 FR6 and DEC-25 edit is owner-approved at this head, and the pager otherwise follows AST-094's paging, catch, Reduce Motion and trackpad rules, with tests for each.
Blocking:
- A trackpad swipe that lands during a settle starts from the settle's target page (
packages/core/src/hooks/usePager.ts:954-959), so during agoToof more than one page the track jumps to within a page of the target (553 px to 1,200 px on one scroll event, against FR6's catch at the value on screen), and a backward swipe that moves it less than a tenth of a page keeps its speed and pages back (against FR43's spike rule); widen the one-page window to include where the swipe began, measure the spike guard from the swipe's own start, and add both cases as tests.
Non-blocking: the comment at usePager.ts:1202 restates the attach calls below it.
[Automated review]
| dragStartIndex = | ||
| wasSettling && isScrollContact(recognizer) | ||
| ? target | ||
| : nearestPage(current); |
There was a problem hiding this comment.
Blocking. A trackpad swipe that lands during a settle takes the settle's target as its start page, but the track can be far from it. The one-page window below (:966-982) then snaps the first move into [target - 1, target + 1]: during goTo(4) from page 0 on five 400 px pages, 15 px of trackpad scroll 96 ms in moves the track from 553 px to 1,200 px in one event, and it rests on page 3. A finger at the same moment follows from the value on screen, as AST-094 FR6 requires. Keep the window around the settle's page but widen it to include dragStart.
There was a problem hiding this comment.
Fixed in 7595f33: the window keeps the settle's page and also holds where the swipe began, so a swipe that catches a longer move starts from the screen. New test: "a trackpad swipe that lands during a goTo across pages catches the track where it is shown: no jump toward the target" (goTo(4) on five 400 px pages; before, 552.5 px jumped to 1,200 px on one wheel).
| // flick. A finger's and a mouse's flick keep their speed. | ||
| if ( | ||
| isScrollContact(recognizer) && | ||
| Math.abs(pageAt(dragPosition) - dragStartIndex) < STILL_RELEASE_FRACTION |
There was a problem hiding this comment.
Blocking. This measures the spike from the start page, not from where the swipe began. Late in goTo(1) (track at 327 px), a backward trackpad swipe that moves the track 32 px (0.08 of a page) at about 750 px/s keeps its speed and lands on page 0, against AST-094 FR43 ("never on a speed spike that moved it less than a tenth of a page"). Compare with the swipe's own start, for example Math.abs(pageAt(dragPosition) - pageAt(dragStart)), and add the case as a test.
There was a problem hiding this comment.
Fixed in 7595f33: the guard compares with the swipe's own start, pageAt(dragPosition) - pageAt(dragStart). New test: "a backward trackpad spike late in a goTo settle releases still: it does not page back" (before, it landed on page 0).
| settled = target; | ||
| } | ||
| animator.set({x: positionToX(offsetOf(target))}); | ||
| // The recognizers, for every pointer. |
There was a problem hiding this comment.
Non-blocking (INV29): this restates the attach calls below it. Keep only the reason, for example: // No pointer check: a mouse and a trackpad page it too.
There was a problem hiding this comment.
Done in 7595f33: "No pointer check: a mouse and a trackpad page it too."
commented
Nov 3, 2026
|
/approve-spec 7595f33 |
left a comment
There was a problem hiding this comment.
Changes requested: two pager defects need a fix and a test each.
- A looping pager that has wrapped forward past its last page jumps to the last page on any viewport resize without calling
onIndexChange, becausemeasure()clamps the unbounded loop target tocount - 1with no loop guard (packages/core/src/hooks/usePager.ts:916-921); guard that branch with!isLooping()and add a resize test after a forward wrap. - A trackpad swipe that lands during a
goTotakes thegoTo's target as its start page, so a backward swipe at 553 px duringgoTo(4)leaves the track frozen and then lands on page 3, against AST-094 FR43's "A pager follows a trackpad swipe" (usePager.ts:954-959); use the settle's page only for a swipe release's settle, and add a backward-swipe-during-goTotest.
The spec edit carries an engineering owner's approval of this head, the earlier trackpad jump is fixed, and the size is exempt as a behavior module. Optionally, the comments at usePager.ts:622 and :743-744 restate the names below them (INV29).
[Automated review]
| if (!isDragging && target > count() - 1) { | ||
| // Fewer pages (children clamped onto one): the last one, as a smaller | ||
| // count clamps. | ||
| goTo(count() - 1, {animated: false, reason: 'controlled'}); | ||
| return; | ||
| } |
There was a problem hiding this comment.
In a loop the target is unbounded, so after a forward wrap (virtual page 5 of 5, page 0) any width change runs this branch and moves the track to page 4, with no onIndexChange (only onSettle(4, 0)). applyOptions guards the same clamp with !isLooping(); this branch needs it too, plus a resize test after a forward wrap.
There was a problem hiding this comment.
Fixed in 879fa48: the clamp now applies only when the pager does not loop, so a wrapped target keeps its slot. New test: "a resize after a forward wrap keeps the wrapped page: no jump and no callback" (before: virtual page 4, with only onSettle(4, 0)).
| // A trackpad swipe that lands while the last swipe's settle runs | ||
| // begins from that settle's page, so back-to-back swipes page on. | ||
| dragStartIndex = | ||
| wasSettling && isScrollContact(recognizer) | ||
| ? target | ||
| : nearestPage(current); |
There was a problem hiding this comment.
wasSettling is also true during a goTo, so a trackpad swipe at 553 px during goTo(4) starts from page 4: the window below becomes [553 px, ∞), a backward swipe cannot move the track, and the release lands on page 3. FR43's settle's-page start is for back-to-back swipes. Take target only while the running motion is a swipe release's settle, else nearestPage(current).
There was a problem hiding this comment.
Fixed in 879fa48: a trackpad swipe starts from the settle's page only while a trackpad release's settle runs. Any other motion (a goTo, a finger's or a mouse's release, the catch's lift) hands over the page on screen. New tests: "a backward trackpad swipe during a goTo across pages follows the fingers and lands from where it began" (before: frozen at 553 px, then page 3) and "a trackpad swipe during a mouse release's settle starts from the page on screen: it moves at most one page from there". The comments at :622 and :743-744 are trimmed too.
A pager for touch presentations. A horizontal pan moves the pages one to one and rubber-bands at the ends. The release picks the page with UIKit's rule and settles on UIKit's paging curve on the compositor, and a finger can catch it mid-flight. Not exported from the package entry point. The pan and the catch attach for every pointer (spec:AST-094 FR19, FR43). A mouse drags the track as a finger does, its short fast drag paging as a finger's flick does, and a mouse that catches a settle locks no text selection (the selection lock reads the pointer kind alone). The pan follows a trackpad's scroll along its axis and never a mouse wheel's (DEC-24): notches, lines, a whole tick and an accelerated mouse's deltas stay the page's. A trackpad's fling releases where the fingers lift, at their speed, and a scroll that stops releases still, as the record's slow 60 px swipe does. A vertical wheel stays the page's. Under Reduce Motion a release from a mouse, a trackpad or a pen follows DR2 on a coarse primary pointer and DR3 otherwise. The trackpad browser cases run in WebKit: Chromium's CDP wheels read as a mouse's wheel. A trackpad swipe moves the track at most one page from the page it began on, across a loop too; a pager that does not loop keeps the rubber band of its first and last pages. A trackpad swipe that moved under a tenth of a page, in the pages' own spacing (children pages included), releases still, whatever its speed: a speed spike is not a flick, while a finger's and a mouse's flick keep their speed. A swipe that lands while the last swipe's settle runs begins from that settle's page, so back-to-back swipes page on (spec:AST-094 FR43). The browser proofs that read a motion (the curves, the flick, Reduce Motion, the storyboards) arm the harness's hold before the input (`armHold`): the page holds the motion at its first frame itself, so a read that comes late on a loaded runner still finds it. The motion an input starts gets 5 s from the input's end: the driver paces events on the wall clock, and a long input can outlast the hold's own 5 s on a loaded runner (a 2.4 s drag plays for about 4.8 s), so the hold is then armed again for what is left of those 5 s. The retarget proof waits for its retarget 100 ms into the curve in its own task, on the page's clock, where no queued timer or slow frame can hold it back, and judges the retarget at the time it actually comes. The busy proof judges the landing on the last frame the page presented before the block ended: a page at rest presents no new frame.
commented
Nov 3, 2026
|
/approve-spec 879fa48 |
left a comment
There was a problem hiding this comment.
This is ready for approval. The pager follows current spec:AST-094: its paging rule, settle, catch, and every-pointer and trackpad rules. An engineering owner approved the FR6 and DEC-25 narrowing for this exact commit. The two issues from the last round, a wrapped loop jumping on resize and a trackpad swipe during a goTo, are fixed with tests.
Two optional follow-ups, neither blocking:
isSwipeSettleis set aftersettleFrom(hooks/usePager.ts:1031). When a trackpad release lands exactly on a page,onIndexChangeandonSettlerun first, so an animatedgoTostarted from those callbacks counts as a swipe's settle. Setting the flag before the callbacks can run fixes this.hooks/usePager.ts:24imports from thegestures/selectionLocktransition re-export. Import fromutils/selectionLockinstead.
[Automated review]
| pagingSettleVelocity(velocity), | ||
| pan.pointerType === 'touch', | ||
| ); | ||
| isSwipeSettle = settleHandle != null && isScrollContact(recognizer); |
There was a problem hiding this comment.
Non-blocking: when a trackpad release lands exactly on a page, settleFrom calls rest(), which runs onIndexChange and onSettle before this line. An animated goTo started from one of them is then marked as a swipe's settle, so a later trackpad swipe during it starts from the goTo's target. Decide the flag before the callbacks can run, for example by passing it to settleFrom and play.
| import type {GestureRecognizer} from '../gestures/GestureRecognizer'; | ||
| import {LongPressGestureRecognizer} from '../gestures/LongPressGestureRecognizer'; | ||
| import {useAttachmentPair} from './useAttachment'; | ||
| import {lockSelection, locksSelectionFor} from '../gestures/selectionLock'; |
There was a problem hiding this comment.
Non-blocking: gestures/selectionLock is a transition re-export marked for removal. Import lockSelection and locksSelectionFor from ../utils/selectionLock.
Primary intent: an internal behavior module (the pager hook) for Carousel; no public API.
User need
Paging should feel like iOS: pages follow the finger, the lift picks UIKit's page, the settle survives a busy main thread; a mouse or trackpad pages it too.
Evidence
Simulator and emulator clips (iOS 26.2; P12: Android 15); Cindy checked the preview (#9364) on a phone and a trackpad, covering the two Mobile Safari behaviors the pager relies on (P7). Red ring: the finger.
goTo(3); mid-dragProblem and solution fit
pageForReleaseapplies chore(core): UIKit's paging values and math, measured on iOS 26.2 #8145's thresholds; a release moving back toward its start page stays there (FR37). The track settles on UIKit's integrator, on the compositor.goTorunssetContentOffset(_:animated:): it cancels a drag, keeps a running move's velocity (FR6), and reads its duration and curve throughresolveMotion(FR4; no role names them).pages: 'children': each child is a page at its start edge.setOptionsor controlledgoTocauses run next frame (INV2); a replaced viewport or track ends a drag on its start page.Current specification
spec:AST-094FR3–FR6, FR8–FR10, FR14, FR19, FR24, FR36–FR41, FR43, DEC-24, DEC-25;design:reduced-motionDR2, DR3;architecture:react-component-runtimeINV2, INV3, INV13.With the spec owners' text, FR6, DEC-25 and the FR6 pager row narrow #8145's gap: only a page change during a drag starts from rest. It needs a spec owner's
/approve-spec <head sha>.Public delta and caller burden
No behavior change:
useAttachment's users keep their lifecycle, and nothing renders the pager until #8153. The changeset is[chore].Design and alternatives
translate.Supersedes #8692: its trackpad swipe rules are in this commit.
Scope
Testing
settle:0:2,settle:2:7andsettle:2:1);goTowith no jump and the late backward spike; the viewport replaced mid-drag or before its report; a touch with the pan off;nearestPagebefore the first measure; the pan's neutral name; the viewport's own frames; INV2's deferred callbacks; FR6's retarget velocity (jsdom and both browsers); children pages, CSS zoom included; the 10 pt claim at 35° and 45°; FR41's selection lock; the controlled index; FR19 and FR43's every-pointer and trackpad cases (Chromium skips the trackpad ones: its CDP wheels lack a trackpad'swheelDelta); FR4'sresolveMotionread;useAttachmentPair's lifecycle (hooks/useAttachment.ts).armHold, armed before the input), and every release storyboard judges each frame against UIKit's curve for the lift velocity the pan read, within 1.5 px. Waits count from the input: a motion must start within 5 s of the input's end, and the retarget proof waits in its own task 100 ms into its 300 ms curve. A failed case's message starts with a load report.goTocross-fade (200 ms); fine non-touch releases land at once (FR43). Limit: in a block an unpainted page stays blank (sheet); owners layer nearby pages.usePager.test.ts123); the WebKit job's routing test (ci-interaction-webkit.test.mjs) 14 passed; the usePager browser proofs: Chromium on a pinned pair of 2 cores, 4 workers: 23 passed, 2 skipped; WebKit, the CI job's profile (podman--cpus=4,CI=1,LP_NUM_THREADS=4, 1 worker) ×10: 220 passed, 30 skipped. At c21b226d8b: check:i18n-runtime and check:i18n-catalog,lint:strict0 errors, the fullpnpm test:ui(first run 18,378 passed, 1 failed, 27 skipped; second 18,376 passed, 1 failed, 29 skipped; the reds are main's lazy-import waits, whose cause fix is test: module loads are awaited, not raced against test clocks #9447) and WebKit--cpus=3at 1 and 16 workers, 22 passed and 3 skipped each.githubfiles:git diff --stat eadd175390a18ba5c5ba7735e8a29e9c38ae6202...HEAD -- .githubis empty against main at eadd175. The WebKit interaction job's pager scope landed as ci: the WebKit interaction lane runs the pager's proof for the pager files #9377.useCarouselPaging.ts) is 1,446; the rest is the paging math's FR6 carry, the table's constants and registration,useAttachment's pair (+56 −14), the record's FR6 narrowing (+6 −8) and the changesetspec:AST-094FR4, FR6, FR19, FR37, FR41, FR43 and DEC-24;spec:AST-100FR2;spec:AST-107DEC-2 (a factory under the hook);design:reduced-motionDR2, DR3;architecture:react-component-runtimeINV2, INV3, INV13;architecture:knowledge-contractsINV29; and the global recordspnpm specs:for --difflists. No public API change. The owner-gated path isdocs/specs/AST-094-touch-presentations/spec.md;pnpm check:knowledgepasses.Writing rubric results
Run over the record text this PR changes in
spec:AST-094(the spec owners' wording, pasted as given):Rejected:line is unchanged.Stack: this PR, then #8153 (#8145, #8676 and #8360 have merged), one commit here, no merges.