Repository navigation
chore(core): UIKit's paging values and math, measured on iOS 26.2 - #8145
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.
76008ed to
44bce34
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
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.
5018b58 to
347debf
Compare
github-actions
Bot
removed
the
needs:design-review
There was a problem hiding this comment.
Approval waits on an engineering-owner decision: this pull request writes new paging text into the current spec:AST-094 (an FR3 speed threshold instead of a projection, an FR6 settle that starts from rest under 225 pt/s, an FR4 implementation gap and DEC-25), and no owner has approved it at this head. An engineering owner can record the decision by commenting /approve-spec followed by the full head SHA, or say which part must change; the author may do so as an engineering owner, and the ruling should also say whether FR43's tenth-of-a-page spike guard and FR36–FR38 still apply to paging. Independently of that ruling, please address these before merge:
- Land #8676 and rebase, so the diff against
mainis this change's 8 files. - Have the wheel pre-filter ask the pan's own direction rule instead of
panTakesMove, which already swallows the first wheels of a diagonal trackpad scroll on a pan withaxisRatio: 2. - Reword the changeset, since only the settle and
setContentOffset(animated:)are replayed against recorded frames. - Retire
scroll.paging.snapnow that the settle is measured, and record Carousel's impact and gap (or leave Carousel out of the FR3 paragraph).
[Automated review]
| } | ||
|
|
||
| /** Whether a pan's `direction` takes a contact moving by (cx, cy). */ | ||
| function panTakesMove( |
There was a problem hiding this comment.
This is a second copy of the pan's direction rule (PanGestureRecognizer.classify), without its axisRatio. For a horizontal pan with axisRatio: 2, a 35° trackpad scroll has its first two wheels prevented while nothing moves, then the pan fails and the rest of the stream scrolls the page. Could the host ask the pan's own classification through the recognizer internals instead, and drop this function?
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] Internal: UIKit's paging values and math for touch presentations. The thresholds that pick the page a release settles on, the settle that carries it there and `setContentOffset(animated:)` are 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.
Only the settle and setContentOffset(animated:) are replayed against recorded frames here; nothing in this change reads advanceSpeed, commitFraction or maxPagesFromStart. Suggested: "The settle and setContentOffset(animated:) replay UIKit's recorded frames to under half a pixel; the page thresholds are recorded values the pager reads."
| measures (0.55 for a scroll view and a sheet), and a release MUST pick its | ||
| rest point by projecting its velocity the way its UIKit counterpart does: | ||
| with a deceleration rate, or with a measured projection time. | ||
| A paging scroll view (the pager, and Carousel with `hasSnap` under a |
There was a problem hiding this comment.
Carousel with hasSnap pages with the browser's scroll-snap-type: x mandatory today, and the Current-state impact line for the shared pager names only a trackpad swipe. Please add the finger page rule and an implementation gap there, or leave Carousel out of this paragraph until the pager replaces its scroll-snap.
| | 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 | `scroll.paging`, `paging` | |
There was a problem hiding this comment.
M-PG1 now feeds both scroll.paging and paging, so the guessed scroll.paging.snap spring stays beside the measured settle for the same use. Consider retiring scroll.paging.snap (and its values.test.ts entry) in this change.
|
Rebased onto main: 13 files.
|
|
/approve-spec e4bb202 |
There was a problem hiding this comment.
Changes requested: an engineering owner has approved this head's paging text, but two sentences in the records now contradict text they leave unchanged. Please fix both; the fix commit needs a fresh /approve-spec.
- Name FR36 and FR38 in DEC-25's narrowing, or add a paging sentence to each. FR36 still decides by projected travel and FR38 still starts the settle at the release velocity, which contradicts the new FR3 and FR6 paging paragraphs that FR36–FR38 are said to govern.
- Limit "where the primary pointer is coarse" in the shared-pager impact line to Carousel.
component:LightboxFR11 and DEC-2 keep trackpad paging in Lightbox's viewer under a fine pointer.
[Automated review]
| of a page, not by a projection, and starts a slow release's settle from rest. | ||
| The owner tried this paging on the pager and the paging Carousel on | ||
| 2026-11-01, with a trackpad and on a phone: "trackpad is good", "phone good". | ||
| This narrows FR3 and FR6 for paging only; every other release keeps them. A |
There was a problem hiding this comment.
DEC-25 narrows only FR3 and FR6, but FR36 (a decision by projected travel) and FR38 ("The commit or the spring-back MUST start at the release velocity") stay as written, and the FR36–FR40 row still fails "a settle that does not start at the release velocity". If FR36–FR38 apply to paging, they contradict the new FR3 paragraph (a 310.7 pt/s release at 10 % advances while a 298.6 pt/s one at 40 % stays, so no projection fits) and the FR6 paragraph (a release under 225 pt/s starts from rest). Suggest naming FR36 and FR38 here: a paging release decides by FR3's paging rule, and its settle starts as FR6's paging paragraph says.
| moves it. Implementation gap: a finger drags it one for one below rest. | ||
| - The shared pager (Carousel's paging presentation, Lightbox's viewer): a | ||
| trackpad swipe follows FR43's pager rule. | ||
| trackpad swipe follows FR43's pager rule where the primary pointer is coarse. |
There was a problem hiding this comment.
This now limits Lightbox's viewer too, but component:Lightbox FR11 says "A mouse, a pen, and a trackpad also use FR7's drag paging" and DEC-2 gives fine pointers the gestures FR43 gives every pointer. Suggest scoping the limit to Carousel, for example: "a trackpad swipe follows FR43's pager rule in Lightbox's viewer under every pointer (component:Lightbox FR11), and in Carousel's paging presentation where the primary pointer is coarse."
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.
|
Fixed in 070b242, with the spec owners' wording:
|
|
/approve-spec 070b242 |
There was a problem hiding this comment.
Recommend approval. An engineering owner approved the new paging text in spec:AST-094 and component:Lightbox at this head, and both record fixes from the earlier review landed: DEC-25 now says how a paging release picks its page and starts its settle, and the shared-pager impact line keeps trackpad paging in Lightbox's viewer under every pointer. The paging math replays the recorded UIKit settles and page scrolls to under half a pixel, and nothing reaches users until the pager plays it. Nothing else needs to change here.
[Automated review]
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;@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.ts,scroll.paging) is deleted, so one paging rule remains.Current specification
spec:AST-094FR1–FR3: measured, sourced UIKit values, kept as internal constants (no theme role). This PR changes two records with the spec owners' text:spec:AST-094: FR3 and FR6 appends, FR4's implementation-gap sentence, DEC-25 (FR37 unchanged), the FR3 and FR6 pager rows, the FR36–FR40 row's paging note, and the shared pager's trackpad line (Lightbox's viewer under every pointer, Carousel where the primary pointer is coarse);component:LightboxFR7: a gallery release lands by FR3's paging rule (DEC-25).They need a spec owner's
/approve-spec <head sha>.Public delta and caller burden
No behavior change: the new math is internal and unused so far, and the deleted
selectPagehad no caller. 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.
Scope
Testing
paging.test.ts:solvePagingandsolveAnimatedPageScrollreplay UIKit's recorded frames (settles within 0.35 pt RMS, animated scrolls within 0.5);pagingSettleVelocityzeroes a release below the floor and keeps a steady 249 pt/s one; the animated scroll lasts 0.3 s whatever the distance.uikit.test.tschecks every new entry's source and its measurement date, and that M-PG12 and M-PG13 are listed in MEASUREMENTS.md.selectPage.test.tsgoes withselectPage, andvalues.test.tsdrops the retiredscroll.paging.snapentry..githubfiles:git diff --stat 5a455f5fdd55a07dac50ac661b6f53714839c0a5...HEAD -- .githubis empty against main at 5a455f5.spec:AST-094FR1–FR4, FR6, FR36–FR40, FR43 and DEC-25 (new here);component:LightboxFR7 and FR11;spec:AST-100FR2, FR3 and DEC-1 (one release filter, thresholds per component);architecture:knowledge-contractsINV29 (comments: no record ids, dates or quotes; short sentences; a header of about 12 lines); and the global recordspnpm specs:for --difflists for these paths. No public API change. The owner-gated paths aredocs/specs/AST-094-touch-presentations/spec.mdanddocs/specs/components/Lightbox/Lightbox.spec.md(change-scope.cjs --classify-paths);pnpm check:knowledgepasses.Writing rubric results
Run over the record text this PR adds to
spec:AST-094andcomponent:Lightbox(the spec owners' wording, pasted as given):Rejected:line in DEC-25.Stack: this PR → #8146 → #8153 (#8676 and #8360 have merged), one commit each, no merges.