Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
November 1, 2026 16:51
github-actions
Bot
added
the
needs:spec-owner-review
Contributor
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
I can't approve this preview yet: its head adds new normative text to spec:AST-094 (FR11 and FR19) and to architecture:react-component-runtime (INV13), plus Carousel touch paging that the Carousel record leaves uncontracted. An engineering owner, and the author is one, needs to approve or reject that direction by commenting /approve-spec 5cbc942ddc3350603b062c4e1ac4333e8f08192f, or say it will be decided in the stacked PRs instead. Whatever that decision, these points need changes before any of this code merges:
- At 28,000 changed lines, this bundles five PRs plus preview-only stories, so #8360, #8676, #8145, #8146 and #8153 should land separately, as planned.
- Carousel attaches the pager only under a coarse pointer (
Carousel.tsx:430-432), but AST-094 FR43 says Carousel's paging presentation follows the pager rule for a trackpad. useCarouselPagingwrapscreatePageritself instead of extendingusePager, anduseFingerScrollcopiesuseAttachment's attach-and-release logic.- The Carousel changeset says the keyboard keeps native snapping, but under a finger the arrow keys go through the pager.
[Automated review]
| const trackRef = useRef<HTMLDivElement>(null); | ||
| const {isTouch} = useTouchPresentation(); | ||
| const hasTrack = hasSnap && hasButtons; | ||
| const isTouchPaging = hasTrack && isTouch; |
There was a problem hiding this comment.
This attaches the pager only under a coarse pointer, so a trackpad swipe on a hasSnap carousel keeps native snap. AST-094 FR43 attaches gestures for every pointer, and its current-state impact says Carousel's paging presentation follows FR43's pager rule for a trackpad.
| }; | ||
| }); | ||
|
|
||
| const [pager] = useState<Pager>(() => |
There was a problem hiding this comment.
This builds a second React wrapper around createPager (state, a setOptions Effect, an attach Effect keyed on ref objects) beside usePager, which already owns that glue with useAttachmentPair. Extending usePager with an enabled flag and native hand-off hooks would let Carousel call it once (react-component-runtime INV16).
| 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.
This per-commit attach and the unmount release below repeat useAttachedNodes in useAttachment.ts. Letting useAttachment take a Document target would remove the copy.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Carousel: under a finger, a `hasSnap` carousel with its buttons pages like an iOS paging scroll view. Each swipe moves one child and settles on that child's start edge on UIKit's paging curve; the content follows the finger and rubber-bands at the ends, even while JavaScript is busy. A mouse, a trackpad and the keyboard keep native snapping, and so does a finger without the buttons, because the buttons are the single-pointer way to page. |
There was a problem hiding this comment.
Under a finger with buttons the keyboard does not keep native snapping: useCarouselPaging takes the arrow keys, Home and End (asserted in CarouselPaging.test.tsx, 'arrow keys move one child, Home and End to the ends').
meta#8360's change on the internal touch foundation: one click guard shared by the menu press and the gesture host (its click listener stays once the menu press arms it, as its record has it, and lasts only while armed for the gesture host, FR24), the host as the one touch owner (finger scroll, page holds, attachments, the press tracker hook and a held control's kept touch, the long press's single source, presented hit testing on iOS WebKit), its cases in the gesture browser proof (Chromium and WebKit) and the README. It carries meta#8074, meta#8270, meta#8293, meta#8315, meta#8320 and meta#8356, and folds in meta#8358 (the long press's tolerance comment and an unused import). Rebased onto main after section 2 as one commit; the touch-action rule came with meta#8071. Where fingers come from their pointer events, a claimed finger's touchend is cancelled from its element (FR21), under a fine primary pointer wherever the browser sends Touch Events and inside an open shadow root too; at most eight such lifts wait for their touchend, and each goes once taken or after 500 ms, kept across the host's uninstall, which can run in the microtask after the pointerup, before the touchend that comes later in the same task (FR24). An armed press's page listeners go once the arena has decided its last touch, a tap that waits on a double tap included, and after its last detach the host holds no listener, note, frame or timer (FR24). The animator's busy proof is judged against the harness's compositor-only control (meta#8906), with a main-thread mutant as its negative, and the spring's curve is placed against the control's own clock, so the late frames a loaded runner sends fail nothing. With the host serving every pointer (meta#8071), this change's own pieces follow it: a press tracker takes a mouse press as a press, painting at once and braking no scroll (FR11); a finger's or a pen's hold that does something keeps text unselected under any primary pointer, while a mouse keeps its released behavior (FR41); and `perEventPointer`, from before FR43, changes nothing. These come from meta#8665, which keeps only the undecided rule for text selection during a mouse or pen drag. The hover recognizer and the host's hover routing land with hover's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…finger landed On iOS WebKit the host resolves moving content at touch-down and asks the engine what is under the finger. When a busy main thread hands it the touch later, the content has moved on: the host now asks at the point where what was under the finger at the touch's timestamp is drawn, so a tap on a moving sheet's Done still clicks Done (spec:AST-094 FR11). A presented source reports its offset at a given time for this. The presented browser proof drives the sheet with Core's animator and a critically damped spring, up into place and down off the screen over the scrim: a tap on the drawn Done clicks it once, and a drag on the drawn sheet catches it where it is drawn, in both directions.
…the finger (FR11, FR19); INV13 names the shared input listener Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on that content goes to what was drawn under the finger at touch-down, and a tap there activates that element with one click while the browser's own click is cancelled or swallowed (FR11). While such content around a recognizer that claims moving touches is moving, the document may carry a non-passive touchmove listener that cancels only owned moves and comes off when the motion ends; while it is on, a native scroll waits for a free main thread (FR19). The gesture host, press feedback, page hold, finger-scroll watch and click guard hear document-wide input through one capture-phase listener per event type on the window (architecture:react-component-runtime INV13).
…view fixes A touch the host handles late now also leaves alone content that slid under the finger after it landed: the touch goes to what was beneath it then (spec:AST-094 FR11). The input-stream changeset says what moved: the gesture host's listeners, and the press feedback's scroll and visibility listeners, since its pointer listeners were already on the window's capture phase. The coarse-pointer gate reads TOUCH_QUERY from touchQueries.ts instead of spelling it again. The presented proof checks that the sheet moves on a spring (its speed changes along the animator's curve) and no longer compares the drawn box with the animator's offset: under a 3-CPU cap the two disagree by 3 to 27 px in about half the frames whatever reference is used, while the tap and drag cases go through the host's real path.
…e origin most frames agree with Under the CI lane's WebKit profile (four CPUs, one worker), "a spring keeps advancing every frame through a 1 s main-thread block" failed in two ways, each about once in a few hundred runs, and a failing run's frames show each cause: - The spring drifted 138.6 ms against the control. The control's clock took its origin from the first usable frame alone, reading that frame's direction from the next usable one: the frame sat at x 210 on its way out, the next usable one at x 210 after the turn, and the tie read as "back". The clock now lives in controlClock.ts and takes the origin most frames agree with. On that run's own frames it reads 6.5 ms (bound 12), and its unit case for that layout fails on the old reading (150.7 ms astray). - The spring "moved on 61 of the 62 frames the control moved". At the block's end it was in its tail, under two of the cast's 1 px steps a frame, where a frame delivered early shows no step and the next catches up two. The harness now reads such a tail at its own pace (busyWindowProof), and the proof judges the whole block: on that run's own frames the earlier proof fails it and this one passes. No bound changes.
… (architecture:public-component-api INV23) Main's lint rule now keeps every private data attribute in its owner module. This change had moved `data-astryx-haptic-switch` from the haptics module to utils/hapticSwitch.ts, so the click guard could skip the hidden switch without importing haptics (architecture:react-component-runtime INV13). The name now lives in privateAttributes/hapticSwitch.stylex.ts, modelled on segmentDisabled.stylex.ts: it exports markHapticSwitch, which the haptics module uses on its hidden switch, and isHapticSwitch, which the click guard reads. utils/hapticSwitch.ts and the deprecated gestures/hapticSwitch.ts re-export go, and the haptics module's baseline entry goes with its raw name. No behavior changes.
…y the gesture host and the selection lock The selection lock and the gesture host each kept a copy of two helpers: the iOS WebKit engine check, and the walk that asks whether a node is inside an element along the flat tree. The engine check now lives in utils/touchQueries.ts, the React-free half of touchPresentation.ts, as IOS_SUPPORTS_QUERY with one isIosWebKit(view) reader; touchPresentation.ts re-exports the query, so its export is unchanged. The walk is containsAlongFlatTree in utils/flatTree.ts, beside parentAcrossShadow. Unit cases cover both in their new homes. No behavior changes.
No Core or Lab component attaches a gesture recognizer yet, so the input-stream changeset's fix entry told consumers about a change they cannot meet. It now keeps `[fix]` for what ships, touch press feedback hearing scrolls and page hides on the window's capture phase, and a new `[chore] Internal:` changeset carries the recognizers' shared stream and its passive-unless-cancelled listeners.
…along the flat tree everywhere Two checks of whether a node is inside a moving element still stopped at a shadow boundary. presentedOffsetOf walked a node's light-DOM parents, so a control slotted into a moving shadow-tree panel read no offset and, on iOS, its press bounds stayed where the panel started; it now walks parentAcrossShadow, through the slot. The host's check for a claiming recognizer in or around a moving element used Node.contains, so a pan in an open shadow root of a moving sheet, or around a moving page in its own shadow root, left the document without its blocking touchmove; it now uses containsAlongFlatTree both ways, as presentedTarget does. A presented hit-testing case for each failed before.
…gh the shared input stream
While an element a claiming recognizer lives in moves on the compositor,
the host's blocking touchmove now subscribes to the window's capturing
touchmove through subscribeInput with {cancels: true}, as the page hold
does, instead of adding its own non-passive listener on the document. The
one input stream carries every page-level listener
(architecture:react-component-runtime INV13), and its window capture phase
sees every touch whatever the engine targets. The presented hit-testing
cases for that listener now check that the window's touchmove turns
non-passive while the element moves and passive again after, with no
touchmove listener on the document; both failed before.
…ng is verified Presented hit testing on iOS WebKit merges on unit tests that mimic its hit testing, the WebKit tap and drag proof with the correction off as its control, and the iOS Simulator; real-device evidence for it follows merge.
…trl-wheel (spec:AST-094 FR43) meta#8676's reviewed change, rebuilt as one commit on meta#8360's head: a wheel gesture drives the recognizers that allow its scroll type as a contact moving with the content, a pan along its own axis once every scroller between it and the page is at its edge, and a ctrl-wheel drives a pinch. A screen-edge pan stays a finger's.
…ain input reads as a mouse wheel, and a pan can take a swipe as one step (spec:AST-094 FR43) The round-4 trackpad behaviors, as a draft for the owner to try each one: - wheelSegmenter.ts splits the wheel stream into gestures and reads a trackpad's lift: once a run of shrinking deltas has lost a fifth of its size, momentum began. A pan the host follows ends there at the fingers' travel, released at their last move with their speed, and the momentum after it never moves the pan. Fingers that stop release still, 180 ms (vertical) or 220 ms (horizontal) after their last wheel. - Input the deltas cannot place reads as a mouse wheel: a delta a hair off a whole pixel, a scroll with no legacy wheelDelta (an engine that sends none), a legacy wheelDelta that is not three times the delta, or a 120 tick over a 40 px delta. - A pan created with wheelStep takes a trackpad swipe as one step: a short flick once the swipe travels 24 px, a slow swipe's travel carried across pauses, its slow tail absorbed, and one undo back to rest. A mouse wheel's gesture and a pinch keep their 100 ms end. In Chromium the harness's wheels arrive as notches (wheelDeltaY -120 whatever the delta), which now read as a mouse wheel, so the two trackpad-only browser cases run in WebKit, whose driver sends a trackpad's precise deltas. The lift's release is stamped at the fingers' last move: stamped at the lift, three held wheels later, it read as a stop and lost the fling.
…speed until the splitter decides, so it never stalls before a lift (spec:AST-094 FR43) The splitter reads a lift only after four shrinking deltas, so a pan that held them stood still for that stretch (about 64 to 130 ms) and then let go at the fingers' speed: a stop, then a go. Through the undecided stretch the pan now moves on from where it last was at the fingers' last speed, never past what that speed projects for the time since. At the lift it keeps that travel and lets go there, still moving at that speed, and nothing more moves. When the fingers carry on, their moves absorb what the stretch showed past their travel or short of it, half a move at a time, so the pan never steps back and ends on their travel; fingers that stop without a lift end on their travel where it runs ahead.
…t, and never a mouse wheel (spec:AST-094 FR43, DEC-24) The merged record decides it: a trackpad's two-finger scroll is direct manipulation, so a pan follows it where its record allows it; a mouse wheel's notches are a scrolling control, so a pan takes none, and a scroll without the trackpad signature, Shift+wheel included, counts as a wheel's. - A pan's allowedScrollTypes takes only 'continuous', and none by default, as UIKit's allowedScrollTypesMask: the sheet, the pager and swipe rows opt in. The host never feeds a wheel's notches to a pan, whatever a caller passes. ScrollType keeps 'discrete' for the pinch, which takes a ctrl-wheel from a trackpad or a mouse. - A scroll with Shift held reads as a mouse wheel's. - wheelStep goes: DEC-24 rejects a discrete step per trackpad swipe for rows and paging, and nothing used it. - The splitter's verification streams: ten swipes that each start in the last one's momentum, ten whose momentum runs out, and a slow drag with 150 ms pauses count ten, ten and one gestures.
UIKit's paging values for touch presentations: the thresholds that pick the page a release settles on, the settle that carries it there and setContentOffset(animated:), measured on iOS 26.2 and replayed against the recorded frames to under half a pixel. Not exported from the package entry point.
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.
Paging rides the existing `hasSnap`. Under a finger (a coarse primary
pointer), a `hasSnap` carousel with its buttons is a UIKit paging scroll
view whose pages are its children: the shared pager (`pages: 'children'`)
moves the track one child per swipe, each child resting at its start edge
as the native snap places it (padding kept, the last children where the
row ends), following the finger, rubber-banding, settling on UIKit's curve
and catchable. A mouse, a trackpad and the keyboard keep native snapping;
so does a finger without the buttons, the single-pointer way to page (WCAG
2.5.1).
What existing `hasSnap` carousels see under a finger: a fast swipe moves
one child instead of coasting across several, and the content no longer
scrolls natively there (no `scroll` events, `scrollLeft` 0);
`hasButtons={false}` keeps native scrolling. The buttons and the handle
keep their step (about a viewport, landing on a child); an arrow key moves
one child; with `hasLoop` a step past either end goes to the other, as it
does natively (no continuous loop), and a looping move's slides wait where
the pager goes (its own nearest-copy rule, so with two slides Next goes
forward). The edge fade follows the pager's page.
A snapping carousel with buttons holds its items in a track on every
pointer, so hydration never remounts them; a clipping scroller takes
`min-width: 0` to stay the carousel's width. The layers for the children
near the page on screen divide an ancestor's CSS zoom out, as the pager's
measure does.
Tests: a jsdom suite and a browser spec on `hasSnap` (full-width pages and
a card row: one card per swipe, the end clamp; padding's inset; the fade;
looping as a rewind; two looping slides; a busy main thread). Its holds
and storyboards take the motion at its first frame (the harness's
`armHold`), so a loaded runner cannot miss a short curve. The storyboards
judge every frame against UIKit's paging settle (`solvePaging`) for the
release velocity the curve starts from, within 1.5 px: bunched input on a
loaded runner makes the lift faster than the script's, and the settle then
overshoots further, as UIKit's does. Carousel joins
the touch-support bundle check's gesture users: its import carries the
pager's pan. A failed browser case's messages begin with a load report:
the touch input against the script it played (a stall, a burst) and the
longest gap between drawn frames. Docs, stories (Paging, Paging (Loop),
Paging (Cards), Paging (In a Long Page)) and the changeset say the same.
Two stories for trying the pager on a phone and on a Mac trackpad: five pages, and five pages that loop. Not for merge: the pager is internal to Core, and Storybook is published.
vjeux
force-pushed
the
preview/8146-pager-carousel
branch
from
November 2, 2026 01:14
5cbc942 to
6465321
Compare
There was a problem hiding this comment.
I can't approve this preview yet: Carousel's touch paging has no governing spec, since the Carousel record leaves snapping uncontracted, so an engineering owner needs to approve or reject that direction by commenting /approve-spec 64653210fc963b03907354e8273600c0ceb41f37 (the author can, as an engineering owner), or say it will be decided in #8153 instead. That decision settles which carousels page and how, whether the pager stays coarse-pointer only although AST-094's impact line says Carousel's paging follows FR43's trackpad rule, and whether hasButtons may choose the touch behavior under the public-component-api INV3 rule. Whatever it is, these need changes before any of this code merges:
- At 28,329 changed lines this bundles #8360 (already on
main), #8676, #8145, #8146 and #8153 plus preview-only stories, so the stacked PRs should land separately, as planned. - The gesture host latches trackpad wheels with its own
documentlistener (GestureHost.ts:2466-2475) instead of the window input stream INV13 requires. useCarouselPagingwrapscreatePageritself instead of extendingusePager.- The Carousel changeset says the keyboard keeps native snapping, but under a finger the arrow keys go through the pager.
[Automated review]
| const trackRef = useRef<HTMLDivElement>(null); | ||
| const {isTouch} = useTouchPresentation(); | ||
| const hasTrack = hasSnap && hasButtons; | ||
| const isTouchPaging = hasTrack && isTouch; |
There was a problem hiding this comment.
This attaches the pager only under a coarse pointer, so a trackpad swipe on a hasSnap carousel keeps native snap. AST-094 FR43 attaches gestures for every pointer, and its current-state impact says a trackpad swipe on Carousel's paging presentation follows FR43's pager rule; the pager already follows a trackpad.
| /** Route every wheel on the page to the live wheel gesture, from its claim. */ | ||
| private latchWheel(): void { | ||
| if (!this.isWheelLatched) { | ||
| this.doc.addEventListener('wheel', this.onWheel, { |
There was a problem hiding this comment.
The latch hears document-wide wheels through its own document capture listener. react-component-runtime INV13 puts the gesture host's document-wide input on the window's one capture listener per type; latching with subscribeInput(view, 'wheel', this.onWheel, {cancels: true}) and unsubscribing in endWheel keeps the off-element steps when an app's window-capture handler stops wheel outside one region.
| }; | ||
| }); | ||
|
|
||
| const [pager] = useState<Pager>(() => |
There was a problem hiding this comment.
This builds a second React wrapper around createPager (its own store, a setOptions Effect, an attach Effect keyed on ref objects) beside usePager, which owns that glue with useAttachmentPair. Extending usePager with an enabled flag and the native hand-off would let Carousel call it once (INV16).
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Carousel: under a finger, a `hasSnap` carousel with its buttons pages like an iOS paging scroll view. Each swipe moves one child and settles on that child's start edge on UIKit's paging curve; the content follows the finger and rubber-bands at the ends, even while JavaScript is busy. A mouse, a trackpad and the keyboard keep native snapping, and so does a finger without the buttons, because the buttons are the single-pointer way to page. |
There was a problem hiding this comment.
Under a finger with the buttons the keyboard does not keep native snapping: useCarouselPaging takes the arrow keys, Home and End, and scrollLeft stays 0. Suggest: a mouse and a trackpad keep native snapping; under a finger with the buttons the arrow keys move one child.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Preview build only, for a live check of the pager (#8146) on a phone and on a Mac trackpad. Do not merge, review or land; it closes after the check, and its branch is deleted.
core-carousel--pagingandcore-carousel--paging-loop. Carousel runs the pager under a finger.core-pager-preview--pagesandcore-pager-preview--loop. On a Mac, Carousel scrolls natively with snap, since it attaches the pager only under a coarse pointer.