Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
needs:design-review
|
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 on an engineering-owner decision: against main this pull request adds the draft spec:AST-094 (FR1–FR23, DEC-1, DEC-2), the only authority for the paging math and the motion stack under it, so an engineering owner needs to approve, narrow or reject it by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). That decision has to settle whether FR3, which says a release picks its rest point by projecting its velocity, covers the measured 300 pt/s paging threshold, and the proposed amendments of theme-tokens INV3, user-states DR1, the buttons family's press feedback and AST-025 FR14. Separately from that decision, these changes are needed:
- Land the predecessor pull requests first: against
mainthis one carries 13,835 authored lines, while its own commit is 1,053. - Join the new
pr-interactionCI job to an existing owner (AST-030 FR11 and FR7); today a failing interaction proof fails no required check. - Under Reduce Motion, a move that starts during a fade leaves the element at its mid-fade opacity, or at 0 with
reducedMotion: 'instant'(packages/core/src/motion/animator.tslines 1346 and 1578). paging.release.velocityFloor(250 pt/s) zeroes the 249 pt/s settle velocity its own measurement says UIKit keeps (packages/core/src/motion/uikit/paging.tsline 121), andhandle.startedreports the call time after a second call from rest takes over (animator.tsline 1075).- Add
## Writing rubric resultsfor the new record to the description.
[Automated review]
| * `scrollViewWillEndDragging` velocity reads 0 for steady releases at | ||
| * 175 and 200 pt/s and keeps 0.249 pt/ms (2026-10-23) | ||
| */ | ||
| velocityFloor: 250, |
There was a problem hiding this comment.
This floor zeroes a 249 pt/s settle velocity, but the @source above says the scrollViewWillEndDragging velocity "keeps 0.249 pt/ms". Put the floor inside the measured bracket (above 200, at most 249 pt/s), tagged GUESS with a measurement task until it is pinned, and update the 249 case in paging.test.ts line 309.
There was a problem hiding this comment.
Fixed in f4e9c59: velocityFloor is now 225 pt/s, inside the measured (200, 249] bracket, and the 249 pt/s case keeps its velocity (paging.test.ts, which failed before).
| # them, so a change to an instrument is proven by its negative controls | ||
| # before a component spec relies on it. Chromium only on these runners; | ||
| # see the harness README for running WebKit. | ||
| pr-interaction: |
There was a problem hiding this comment.
No join needs pr-interaction (test at line 887, build at line 1264), so a failing interaction proof fails no required check. AST-030 FR11 asks new coverage inside an existing surface to join that surface's owners without a separately routed lane, and FR7 that an owned lane's failure fails its join: list it in the test join and in ci-test-join.mjs, or admit the lane through an owner-approved FR10 amendment.
There was a problem hiding this comment.
Not this PR's code: the pr-interaction job comes from the interaction-harness foundation below this commit in the stack, and is routed to its owners.
| ): Partial<Record<MotionProperty, Segment | null>> => { | ||
| const next: Partial<Record<MotionProperty, Segment | null>> = {}; | ||
| const opacityNow = valueAt('opacity', time); | ||
| const opacityTo = targets.opacity ?? opacityNow; |
There was a problem hiding this comment.
When the call names no opacity, this ends the Reduce Motion dip at the opacity on screen rather than where a running fade is going. A fade from 0 to 1 interrupted at 0.3577 by animate({x: 120}) leaves the element at 0.3577, or at 0 with 'instant' because animate deletes the segment before its valueAt fallback (lines 1577–1578). Ending the dip at finalValue('opacity') and leaving opacity alone in 'instant' mode fixes both.
There was a problem hiding this comment.
Not this PR's code: the animator belongs to the motion foundation below this commit in the stack, and the Reduce Motion fade case is routed to its owner.
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
7464cb9 to
f4e9c59
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 on an engineering-owner decision: against main this pull request adds the draft spec:AST-094 (FR1–FR23, DEC-1, DEC-2), the only authority for the paging math and the motion stack under it, so an engineering owner needs to approve, narrow or reject it by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). That decision has to settle whether FR3, which says a release picks its rest point by projecting its velocity, covers the measured 300 pt/s paging threshold, and the proposed amendments of theme-tokens INV3, user-states DR1, the buttons family's press feedback and AST-025 FR14. The settle-velocity floor is fixed; separately from the owner decision, these changes are needed:
- Conform the Reduce Motion substitution (FR8, the 500 ms
timing.reducedMotionCrossFadeMsdefault, presentation-spring fades, throws that keep moving) todesign:reduced-motionDR2, current onmain, which allows only an opacity change of 200 ms or less, also after a touch release, or propose that record's amendment in FR8. - Land the predecessor pull requests first: against
mainthis one carries 13,837 authored lines, while its own commit is 1,055. - Join the
pr-interactionCI job to an existing owner (AST-030 FR11 and FR7), and fix the Reduce Motion fade that leaves an element at its mid-fade opacity (animator.tslines 1346 and 1578) and thehandle.startedtakeover (animator.tsline 1075). - Add
## Writing rubric resultsfor the new record, and tagvelocityFlooras an estimate with a measurement task.
[Automated review]
| place, and any other move fades through. A navigation push or pop | ||
| cross-fades over 0.5 s on the ease-in-out curve; a sheet, a full-screen | ||
| modal or an alert fades on its own presentation spring and drops its travel | ||
| and scale; menus open and close on UIKit's Reduce Motion spring. Motion that | ||
| follows a finger, such as a throw, MUST keep its motion. **Proposed; pending |
There was a problem hiding this comment.
design:reduced-motion, current on main since this stack was written, requires every such transition on touch to change opacity only and finish in 200 ms or less (DR2), including one a touch release starts, which "cross-fades as DR2 says, because a finger started it". The 0.5 s navigation cross-fade, the presentation-spring fades and the throw that keeps moving contradict it, and the note below names only the draft design:motion DR5, which now defers to that record. Conform FR8 to it, or propose its amendment here for its owner to rule on.
| * Cross-Fade Transitions on (opacity CABasicAnimation 0.5 s, | ||
| * (0.42, 0, 0.58, 1)), Oct 23 2026 | ||
| */ | ||
| reducedMotionCrossFadeMs: 500, |
There was a problem hiding this comment.
Under current design:reduced-motion DR2 a substituted cross-fade on touch must finish in 200 ms or less. As the animator's default fade (animator.ts lines 1366–1393) this 500 ms value, like the presentation-spring fades (404–588 ms) and the travel that 'keep' and decay keep after a release, would put every touch presentation built on the animator over that limit. Cap the substituted fade at 200 ms, or get the record amended, before a component adopts it.
| * `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 sits inside the measured bracket and the 249 pt/s case now keeps its velocity. The comment calls it "the bracket's middle until a finer sweep pins it", but the only tag is @source measured:, so uikit.test.ts counts it as measured and no MEASUREMENTS.md row asks for the sweep. Per the folder's README, add @source GUESS — the middle of the measured (200, 249] bracket and an @measure task listed in MEASUREMENTS.md.
There was a problem hiding this comment.
Fixed in e7e2c50: velocityFloor is tagged @source GUESS — the middle of the measured (200, 249] bracket with @measure M-PG13, and M-PG13 is listed in MEASUREMENTS.md (uikit.test.ts checks both).
|
Agreed: one implementation.
|
f4e9c59 to
e7e2c50
Compare
There was a problem hiding this comment.
Approval waits on an engineering-owner decision: current spec:AST-094 says a release picks its rest point by projecting its velocity (FR3), hands its velocity to the motion that follows while a new target keeps the current velocity (FR6), and plays touch motion through motion roles (FR4, with spec:AST-108 FR3), while this paging records a measured 300 pt/s page threshold, starts the settle at rest below 225 pt/s, drops the velocity in the animated page scroll and keeps its motion out of the roles. An engineering owner can record that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner) and then amend AST-094 or add the pager's own record to match, or say which part must change. Separately, these changes are needed:
- Land the predecessor pull requests (#8360, #8076, #8446) and rebase, or rebase the 8 paging files onto
mainalone: againstmainthis diff carries 27,972 authored lines, while its own change is 742. - Undo the three edits that still assume
scroll.pagingis gone: the M-PG1 row inMEASUREMENTS.md, theuikit/scroll.tsrow in the motion README, and thescroll.paging.snapentry invalues.test.ts.
[Automated review]
7df00ca to
fc47044
Compare
github-actions
Bot
added
the
needs:spec-owner-review
There was a problem hiding this comment.
Approval waits on an engineering-owner decision: the measured paging here departs from current spec:AST-094 FR3 (a speed threshold instead of a projected rest point), FR6 (the settle and the animated page scroll drop the release velocity) and FR4 (paging motion outside the motion roles, with no pager record), and the branch also carries new FR11 and FR19 text and an INV13 sentence from #8360. An engineering owner can record that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), then amend AST-094 or add the pager's record, or say which part must change. Separately, these changes are needed:
- Land #8360 and #8676 and rebase, so the diff is the 8-file paging change rather than 20,147 authored lines against
main. - Have
useFingerScrollcalluseAttachmentinstead of copying its attach and release Effects. - Move the page-rule and FR37 test claims in the description to #8146, where that code and those tests live.
- Restore
scroll.pagingin the M-PG1 row, the motion README andvalues.test.ts, fix theusePagerheader, and make the INV13 sentence match the documenttouchmoveandwheellisteners.
[Automated review]
| 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(() => { | ||
| const node = isDocument(scroller) ? scroller : scroller.current; | ||
| if (attachedRef.current?.node === node) { | ||
| return; | ||
| } | ||
| attachedRef.current?.detach(); | ||
| attachedRef.current = | ||
| node == null | ||
| ? null | ||
| : { | ||
| node, | ||
| detach: watchFingerScrolls( | ||
| getGestureHost(isDocument(node) ? node : node.ownerDocument), | ||
| node, | ||
| ), | ||
| }; | ||
| }); | ||
| // Synchronizes with the gesture host's finger-scroll watch: unmounting | ||
| // releases the watch this hook attached. | ||
| useIsomorphicLayoutEffect( | ||
| () => () => { | ||
| attachedRef.current?.detach(); | ||
| attachedRef.current = null; | ||
| }, | ||
| [], | ||
| ); |
There was a problem hiding this comment.
These two Effects repeat useAttachment (gestures/useAttachment.ts lines 68–98): attach after each commit, release a replaced node, release at unmount. Let useAttachment take a Document target (or a node getter) and hand back the attached node, then call it here once, so a fix to node replacement or StrictMode replay reaches every gesture hook.
| small glue in its own build over these modules. The gesture host attaches for | ||
| every pointer, with no touch-only query; under a fine pointer only a component that | ||
| uses a gesture listens (`spec:AST-094` FR19, FR24, FR43). The modules are not public exports and are not | ||
| uses a gesture listens (`spec:AST-094` FR19, FR24, FR43). 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, present while a subscriber needs it. They run in subscription order, before any document or element listener; it is passive unless a subscriber cancels the event. The modules are not public exports and are not |
There was a problem hiding this comment.
The gesture host also registers a non-passive document touchmove while a claiming element moves (GestureHost.ts lines 1633–1638) and a non-passive document-capture wheel while a wheel gesture is latched (lines 2468–2476), outside the window stream. Either route both through subscribeInput or name them in this sentence.
| * and `solveAnimatedPageScroll` (`setContentOffset(_:animated:)`). The page | ||
| * a release settles on is the pager's own rule, on the motion foundation's | ||
| * release policy (resolveSnapPoint.ts) | ||
| * @position Pure math; no DOM, no React. Read by hooks/usePager.ts |
There was a problem hiding this comment.
hooks/usePager.ts does not exist at this head; say the pager in #8146 reads this.
fc47044 to
76008ed
Compare
There was a problem hiding this comment.
Approval waits on an engineering-owner decision: the measured paging here departs from current spec:AST-094 FR3 (a 300 pt/s threshold rather than a projected rest point), FR6 (the settle under 225 pt/s and the animated page scroll start from rest) and FR4 (no motion role and no pager record), and the branch carries new FR11, FR19 and INV13 text from #8360. An engineering owner (the author included) can record that decision by commenting /approve-spec followed by the full head SHA, then amend AST-094 or add the pager's record, or say which part must change. Independently of that ruling, please:
- Land #8360 and #8676 and rebase, so this diff is the 8-file paging change instead of 20,230 lines.
- Have
useFingerScrollcalluseAttachmentinstead of repeating its Effects, and walk the flat tree (parentAcrossShadow) inpresentedOffsetOf. - Fix the INV13 sentence, which omits the gesture host's document
touchmoveandwheellisteners, move the two FR37 test claims to #8146's description, and restore thescroll.pagingmeasurement row, the motion README row and thescroll.paging.snapspring check.
[Automated review]
| for ( | ||
| let node: Node | null = element; | ||
| node != null; | ||
| node = node instanceof ShadowRoot ? node.host : node.parentNode |
There was a problem hiding this comment.
This walks parentNode and ShadowRoot.host, while the gesture host matches movers along the flat tree (containsAlongFlatTree in utils/flatTree.ts, which follows assignedSlot). For slotted content inside a moving shadow-tree element, presentedTarget retargets the touch but presentedRect adds no offset for that mover, so a press tracker judges the finger against a box the content has left. Walking parentAcrossShadow here keeps one flat-tree walk; a slotted case in presentedHitTesting.test.ts would pin it.
meta#8360's change on the internal touch foundation: one click guard shared by the menu press and the gesture host (its click listener stays once the menu press arms it, as its record has it, and lasts only while armed for the gesture host, FR24), the host as the one touch owner (finger scroll, page holds, attachments, the press tracker hook and a held control's kept touch, the long press's single source, presented hit testing on iOS WebKit), its cases in the gesture browser proof (Chromium and WebKit) and the README. It carries meta#8074, meta#8270, meta#8293, meta#8315, meta#8320 and meta#8356, and folds in meta#8358 (the long press's tolerance comment and an unused import). Rebased onto main after section 2 as one commit; the touch-action rule came with meta#8071. Where fingers come from their pointer events, a claimed finger's touchend is cancelled from its element (FR21), under a fine primary pointer wherever the browser sends Touch Events and inside an open shadow root too; at most eight such lifts wait for their touchend, and each goes once taken or after 500 ms, kept across the host's uninstall, which can run in the microtask after the pointerup, before the touchend that comes later in the same task (FR24). An armed press's page listeners go once the arena has decided its last touch, a tap that waits on a double tap included, and after its last detach the host holds no listener, note, frame or timer (FR24). The animator's busy proof is judged against the harness's compositor-only control (meta#8906), with a main-thread mutant as its negative, and the spring's curve is placed against the control's own clock, so the late frames a loaded runner sends fail nothing. With the host serving every pointer (meta#8071), this change's own pieces follow it: a press tracker takes a mouse press as a press, painting at once and braking no scroll (FR11); a finger's or a pen's hold that does something keeps text unselected under any primary pointer, while a mouse keeps its released behavior (FR41); and `perEventPointer`, from before FR43, changes nothing. These come from meta#8665, which keeps only the undecided rule for text selection during a mouse or pen drag. The hover recognizer and the host's hover routing land with hover's first user. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
…finger landed On iOS WebKit the host resolves moving content at touch-down and asks the engine what is under the finger. When a busy main thread hands it the touch later, the content has moved on: the host now asks at the point where what was under the finger at the touch's timestamp is drawn, so a tap on a moving sheet's Done still clicks Done (spec:AST-094 FR11). A presented source reports its offset at a given time for this. The presented browser proof drives the sheet with Core's animator and a critically damped spring, up into place and down off the screen over the scrim: a tap on the drawn Done clicks it once, and a drag on the drawn sheet catches it where it is drawn, in both directions.
…the finger (FR11, FR19); INV13 names the shared input listener Where the engine hit-tests an element a compositor animation moves at the motion's start (iOS WebKit), a touch on that content goes to what was drawn under the finger at touch-down, and a tap there activates that element with one click while the browser's own click is cancelled or swallowed (FR11). While such content around a recognizer that claims moving touches is moving, the document may carry a non-passive touchmove listener that cancels only owned moves and comes off when the motion ends; while it is on, a native scroll waits for a free main thread (FR19). The gesture host, press feedback, page hold, finger-scroll watch and click guard hear document-wide input through one capture-phase listener per event type on the window (architecture:react-component-runtime INV13).
…view fixes A touch the host handles late now also leaves alone content that slid under the finger after it landed: the touch goes to what was beneath it then (spec:AST-094 FR11). The input-stream changeset says what moved: the gesture host's listeners, and the press feedback's scroll and visibility listeners, since its pointer listeners were already on the window's capture phase. The coarse-pointer gate reads TOUCH_QUERY from touchQueries.ts instead of spelling it again. The 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.
76008ed to
44bce34
Compare
…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.
There was a problem hiding this comment.
Approval waits on an engineering-owner decision: the measured paging here departs from current spec:AST-094 FR3 (a 300 pt/s threshold rather than a projected rest point), FR6 (the settle under 225 pt/s and the animated page scroll start from rest) and FR4 (no motion role and no pager record), and the branch carries new FR11 and FR19 text and an INV13 sentence from #8360. An engineering owner can record the decision by commenting /approve-spec followed by the full head SHA, then amend AST-094 or add the pager's record to match; the author may do so as an engineering owner. Independently of that ruling, please address these before merge:
- Land #8360 and #8676 and rebase, so the diff against
mainis this pull request's 8-file change. - Have
useFingerScrollcalluseAttachmentinstead of repeating its two Effects. - Move the two page-rule tests named under Testing, and the FR37 sentence, to #8146's description.
- Make the INV13 sentence match the gesture host's document
touchmoveandwheellisteners, and fix thescroll.pagingleftovers, theusePagerpointer and the new verification row.
[Automated review]
| | FR42 | Default-theme geometry tests under a coarse and a fine pointer, and hit-testing between adjacent targets | every component at phone width with a finger and with a mouse; a dense toolbar; a chip and its remove button; a Switch beside a label link | a default-theme box that grows under touch or at phone width other than with its text, or, where no component or module record sets the hit area, a touch target under 44 px where neighbors allow it or a hit area that overlaps a neighbor's target, fails | | ||
| | FR43 | Browser tests with a mouse, a trackpad, a pen, a finger, and the keyboard, and resource counts | a pan by drag and by wheel; a slider's thumb under a wheel and a trackpad scroll; a ctrl-wheel pinch; a long press, a right click, and Shift+F10; a mouse press; a handle drag; a row swipe with a mouse and a trackpad; a component with no gesture | a gesture missing under a mouse or pen, a mouse-drag swipe action, a pen pinch, an edge pan under a mouse, a trackpad, or a pen, a press delay under a fine pointer, a wheel or trackpad scroll that changes a thumb's value or does not reach the scroll under it, an idle listener in a component with no gesture, or added stylesheet bytes, fails | | ||
| | FR43 trackpad | Unit tests on timed wheel streams; WebKit browser tests with trackpad streams (Chromium's driver wheels read as a mouse wheel) | a form and a required sheet pulled 30 to 240 px by a finger and by a trackpad; mouse lines, notches, ticks and accelerated deltas over a sheet, a pager and a row; ten swipes each starting mid-momentum, ten whose momentum ends, and a slow drag with 150 ms pauses; a trackpad flick and a slow stopped drag on an info sheet; a fling, a slow 60 px swipe, a 42 px spike and back-to-back swipes on a pager; a row opened and swiped back within one gesture, and a long fast swipe; a pinch ending | a trackpad stretch that differs from a finger's for the same travel (120 px gives 54 px on the reference sheet), a mouse wheel that moves a sheet, a pager or a row, splitter counts other than ten, ten and one, momentum that moves a surface, a flick released with no velocity, a pager that travels past one page or pages on a spike, an info sheet a flick leaves open or a slow stopped drag closes, an open row that reveals its other side within one gesture, or a pinch that ends after 100 ms fails | | ||
| | FR11, FR19 iOS | Presented tap/drag proof: CI WebKit (`pr-interaction-webkit`) and the iOS Simulator | owner phone check of the preview on 10/31 | evidence gap: no recorded physical-iPhone take yet (#9310). | |
There was a problem hiding this comment.
This row puts a dated owner check in the states column and an evidence gap with an issue number in the failure column, so it states no failure condition and keeps history as standing content (docs/contributing/spec-writing.md R3, R5). Name the scenarios and the failure (for example, a tap on a sliding sheet that activates what sat under the motion's start fails), and keep the missing physical-iPhone take in the pull request.
44bce34 to
5018b58
Compare
There was a problem hiding this comment.
Approval waits on an engineering-owner decision: the measured paging here departs from current spec:AST-094 FR3 (a 300 pt/s threshold rather than a projected rest point), FR6 (the settle under 225 pt/s and the animated page scroll start from rest) and FR4 (no motion role and no pager record), and the branch carries new FR11 and FR19 text and an INV13 sentence from #8360. An engineering owner can record the decision by commenting /approve-spec followed by the full head SHA, then amend AST-094 or add the pager's record to match; the author may do so as an engineering owner. Independently of that ruling, please address these before merge:
- Land #8360 and #8676 and rebase, so the diff against
mainis this pull request's 8-file change. - Have
useFingerScrollcalluseAttachment, and have the action-group tracker arm the shared click guard, instead of keeping their own copies. - Move the two page-rule tests named under Testing, and the FR37 sentence, to #8146's description.
- Make the INV13 sentence match the listeners kept outside the window stream, and fix the
scroll.pagingleftovers and theusePagerpointer.
[Automated review]
| 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(() => { | ||
| const node = isDocument(scroller) ? scroller : scroller.current; | ||
| if (attachedRef.current?.node === node) { | ||
| return; | ||
| } | ||
| attachedRef.current?.detach(); | ||
| attachedRef.current = | ||
| node == null | ||
| ? null | ||
| : { | ||
| node, | ||
| detach: watchFingerScrolls( | ||
| getGestureHost(isDocument(node) ? node : node.ownerDocument), | ||
| node, | ||
| ), | ||
| }; | ||
| }); | ||
| // Synchronizes with the gesture host's finger-scroll watch: unmounting | ||
| // releases the watch this hook attached. | ||
| useIsomorphicLayoutEffect( | ||
| () => () => { | ||
| attachedRef.current?.detach(); | ||
| attachedRef.current = null; | ||
| }, | ||
| [], | ||
| ); |
There was a problem hiding this comment.
These two Effects repeat useAttachment (gestures/useAttachment.ts lines 80–98): attach after each commit, release a replaced node, release at unmount. A fix to node replacement or StrictMode replay in one will miss the other. Let useAttachment take a Document target (or a node getter) and hand back the attached node, then call it here and delete these Effects.
| * @output Exports armClickGuard (and its ClickGuardOptions), | ||
| * disarmClickGuard, isClickGuardArmed, withoutClickGuard and | ||
| * CLICK_GUARD_WINDOW_MS | ||
| * @position The one guard for the browser's click after a gesture that |
There was a problem hiding this comment.
hooks/actionGroupTracker.ts swallowNextClick (lines 180–197) still keeps its own window-capture click, pointerdown and keydown swallower for the same click, with different rules (no 400 ms bound, a key disarms it, untrusted clicks pass). Have it arm this guard (with a disarm-on-key option if needed), or say here why action groups keep their own.
| small glue in its own build over these modules. The gesture host attaches for | ||
| every pointer, with no touch-only query; under a fine pointer only a component that | ||
| uses a gesture listens (`spec:AST-094` FR19, FR24, FR43). The modules are not public exports and are not | ||
| uses a gesture listens (`spec:AST-094` FR19, FR24, FR43). 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, present while a subscriber needs it. They run in subscription order, before any document or element listener; it is passive unless a subscriber cancels the event. The modules are not public exports and are not |
There was a problem hiding this comment.
The gesture host also adds a non-passive document touchmove while a claiming element moves (GestureHost.ts lines 1623–1625) and a non-passive document-capture wheel while a wheel gesture is latched (lines 2461–2464), and press feedback keeps its window keydown on the bubble phase (pressFeedback.ts line 709). Route the two host listeners through subscribeInput, or name all three as exceptions here.
| * and `solveAnimatedPageScroll` (`setContentOffset(_:animated:)`). The page | ||
| * a release settles on is the pager's own rule, on the motion foundation's | ||
| * release policy (resolveSnapPoint.ts) | ||
| * @position Pure math; no DOM, no React. Read by hooks/usePager.ts |
There was a problem hiding this comment.
hooks/usePager.ts does not exist at this head; say the pager in #8146 reads this.
| | M-S3 | Bounce-back spring | Same list: drag 120 pt past the top, hold still, release; a flick into the top edge at 3000 pt/s | contentOffset(t) after release and after edge contact; spring response and damping ratio; velocity hand-off | C | `scroll.bounce` | | ||
| | M-S5 | Rubber band | Same list on an 852 pt view: drag 100, 300, 600 pt past the top; a horizontal pager | displayed overflow against finger overflow; confirm c = 0.55 and d = the view's size on the axis | C | `scroll.rubberBand` | | ||
| | M-PG1 | Paging | Photos full-screen swipes; a probe paging scroll view | page chosen per release position × velocity (thresholds; at most one page?); contentOffset(t) onto the page | B, C | `scroll.paging` | | ||
| | M-PG1 | Paging | Photos full-screen swipes; a probe paging scroll view | page chosen per release position × velocity (thresholds; at most one page?); contentOffset(t) onto the page | B, C | `paging` (measured; see M-PG12) | |
There was a problem hiding this comment.
scroll.paging.projectionRate and scroll.paging.snap still cite M-PG1 (uikit/scroll.ts lines 91 and 97), and selectPage.ts reads the first; keep scroll.paging in this row.
On #8676 (on #8360). Review only this PR's change: vjeux/astryx@05b9a84...5018b58
User need
Paging under a finger should match iOS: the same page choice at the lift, the same settle. This adds UIKit's paging values and math as pure functions for the pager.
Problem and solution fit
motion/uikit/paging.ts: UIScrollView paging on iOS 26.2, each value with its measurement:v = (v + (target − x) · 0.000284858) · 0.970815; x += v, whole steps per frame;spec:AST-094FR37; UIKit takes the nearest);@measureM-PG13);setContentOffset(_:animated:)'s 0.3 s sinusoid, the rubber band's dimension, the pan's axis ratio.motion/paging.ts:pagingSettleVelocity(the floor),solvePaging(the integrator as a curve the animator plays) andsolveAnimatedPageScroll; the pager (chore(core): a pager that pages like a UIKit paging scroll view #8146) picks the page with these thresholds on the release policy,resolveSnapPoint.motion/selectPage.tsprojects withscroll.paging.projectionRate, a GUESS, while these thresholds are measured on iOS 26.2.Current specification
spec:AST-094FR1–FR3: measured, sourced UIKit values, kept as internal constants (no theme role); FR37. Nothing here changes Carousel.Public delta and caller burden
None: internal, not exported, unused so far. The changeset is
[chore].Design and alternatives
Evidence
No UI change: values and math only. Recorded from UIKit on the iOS 26.2 simulator, frame by frame: releases at rest, an overscroll return, flicks and animated scrolls, plus the advance speed bracketed at 298.6–310.7 pt/s. The tests replay them: settles within 0.35 pt RMS, animated scrolls within 0.5.
Scope
Testing
uikit.test.tschecks every new entry's source and that M-PG13 is listed in MEASUREMENTS.md.test:ui17,840 passed, Storybook build, the node project (2 failures incache.test.mjs, the same on feat(core): a pan takes wheel and trackpad scrolling, and a pinch a ctrl-wheel (spec:AST-094 FR43) #8676's head), changesets, check:repo, Prettier.Stack: #8360 → #8676 → this PR → #8146, one commit each, no merges.