Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
30ce9c7 to
ffa0821
Compare
|
Posted by Cindy's agent on her behalf. Thanks for the public snap-points hook. One structure ask: this PR has its own release rule ( |
meta-cla
Bot
added
the
CLA Signed
|
Done in a671989: |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This snap-point drag can't be approved yet: createDragToSnapPoints and useDragToSnapPoints rely on spec:AST-107, which isn't on main (it is proposed in #8600), and the current specs still keep the gesture and motion layer and release thresholds internal. An engineering owner needs to provide or identify the governing spec for these exports, for example by landing #8600, and record that decision here by commenting /approve-spec followed by the full head SHA; the author may do so as an engineering owner. Whatever that decision, these changes are still needed:
- Rebase once the stacked pull requests land, so this diff holds only the snap-point change (about 1,500 changed lines instead of 20,493).
- Make
useDragToSnapPointsattach to an element that mounts or is replaced after the first render, asusePanGesturedoes. - Start a re-created factory from the element's current offset, so changing
snapPoints,axisorflickThresholddoesn't make the next drag jump. - Reject an empty
snapPointsin the hook as the factory does, and add a test that fails if the settle ignores the release velocity.
[Automated review]
| useEffect(() => { | ||
| const element = ref.current; | ||
| if (element == null) { | ||
| return undefined; | ||
| } | ||
| const created = createDragToSnapPoints(element, { | ||
| axis, | ||
| snapPoints: points.split(',').map(Number), | ||
| flickThreshold, | ||
| onSettle: index => onSettleRef.current(index), | ||
| }); | ||
| snapRef.current = created; | ||
| return () => { | ||
| created.destroy(); | ||
| if (snapRef.current === created) { | ||
| snapRef.current = null; | ||
| } | ||
| }; | ||
| }, [ref, axis, points, flickThreshold]); |
There was a problem hiding this comment.
This effect only re-runs when ref, axis, points or flickThreshold change, so an element that mounts after the first commit (for example {open && <div ref={ref} />}) or a replaced node never gets the drag, and settleTo stays a no-op. usePanGesture in this stack re-checks the node on every commit; doing the same here, or taking a callback ref as useAnimator does, would handle attachment and replacement as architecture:react-component-runtime INV6 asks.
| } | ||
| const created = createDragToSnapPoints(element, { | ||
| axis, | ||
| snapPoints: points.split(',').map(Number), |
There was a problem hiding this comment.
[].join(',') is '', and ''.split(',').map(Number) is [0], so an empty snapPoints passed to the hook becomes one snap point at 0 instead of throwing as createDragToSnapPoints does.
| const animator = createMotionAnimator(element); | ||
| // The snap point the element rests on, or is settling toward. | ||
| let index = resolveSnapPoint({ | ||
| points: snapPoints, | ||
| position: 0, | ||
| velocity: 0, | ||
| flickThreshold, | ||
| }).index; |
There was a problem hiding this comment.
When the hook re-creates the factory after snapPoints, axis or flickThreshold change, this starts from offset 0 and the snap point nearest 0, but the previous animator's inline translate stays on the element, so the next drag jumps from where the element is drawn to the finger's travel from 0. Seeding the animator's initial and the starting index from the element's current offset would keep them in step.
There was a problem hiding this comment.
This snap-point drag can't be approved yet: createDragToSnapPoints and useDragToSnapPoints rely on spec:AST-107, which isn't on main (it is proposed in #8600), and the current specs still keep the gesture and motion layer and release thresholds internal. An engineering owner needs to provide or identify the governing spec for these exports, for example by landing #8600, and record that decision here by commenting /approve-spec followed by the full head SHA; the author may do so as an engineering owner. Whatever that decision, these changes are still needed:
- Rebase once the stacked pull requests land, so this diff holds only the snap-point change (roughly 1,550 changed lines instead of 20,574).
- Make
useDragToSnapPointsattach to an element that mounts or is replaced after the first render, asusePanGesturedoes. - Start a re-created factory from the element's current offset, so changing
snapPoints,axisorflickThresholddoesn't send the next drag back toward the first snap point. - Mirror the templates' snap points in right-to-left layouts, where the card now moves away from its next place and off-screen.
- Add a test that fails if the settle ignores the release velocity.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner deciding which spec governs the new public createDragToSnapPoints and useDragToSnapPoints (with flickThreshold): spec:AST-107 exists only in open #8600, while current spec:AST-017 FR21, architecture:react-component-runtime INV13 and spec:AST-100 FR3 keep gesture modules and release thresholds internal. Any engineering owner, the author included if they are one, can record that decision by commenting /approve-spec 21f071817cf2ef629e0e2cfa8713f02377a0a57e on this pull request, or can point to another current record that governs these exports. Whatever the decision, these still need changes:
- The diff carries 20,168 authored lines from eleven other open pull requests, so rebase onto the landed dependencies until it holds only the snap-point drag.
- A mouse or pen drag never starts where the primary pointer is fine (
GestureHost.ts:580-583), though both doc pages and the templates' grab cursor promise one. useDragToSnapPointsnever attaches to an element that mounts after its first render, and aftersnapPointschange the next drag starts from 0 (useDragToSnapPoints.ts:67-85,createDragToSnapPoints.ts:124-129).- The templates' card moves the wrong way in right-to-left layouts (
DragToSnapPointsCard.tsx:15-24), and no test fails when the settle ignores the release velocity (spec:AST-094FR38).
[Automated review]
| }); | ||
| }; | ||
|
|
||
| const pan = createPanGesture(element, { |
There was a problem hiding this comment.
On a desktop whose primary pointer is fine, the document's gesture host installs no listener (GestureHost.ts:580-583), so no mouse or pen drag reaches this pan and only settleTo moves the element, while both doc pages describe the settle "after a mouse or pen drag" and the templates show a grab cursor. Either give the public pan a mouse and pen path on its own element under a fine pointer (the host already does this for action groups, GestureHost.ts:476-487), or document that the drag needs a coarse primary pointer and drop the grab cursor.
|
|
||
| const PLACES = ['Hand', 'Table', 'Discard pile']; | ||
| /** Where each place sits, in px from the card's resting position. */ | ||
| const SNAP_POINTS = [0, 110, 220]; |
There was a problem hiding this comment.
These offsets are physical, but the card and the place buttons sit in rows that mirror: in a right-to-left layout Table is to the card's left, a drag toward it settles back on Hand, and Table's button slides the card 110 px right, past the inline-start edge (design:bidirectionality DR13). Mirror the offsets when the layout is right-to-left, mapping onSettle's sorted index back to its place; DragToSnapPointsCardVanilla.tsx needs the same change.
| snap.destroy(); | ||
| }); | ||
|
|
||
| it('a quick flick carries to the next snap point', async () => { |
There was a problem hiding this comment.
This flick test asserts only onSettle, so it still passes when the settle drops the release velocity (createDragToSnapPoints.ts:147); spec:AST-094 FR38's verification row needs a test that fails then. Assert the recorded settle's opening keyframes, or the animator's velocity right after the release, against the release velocity.
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
github-actions
Bot
added
the
needs:spec-owner-review
There was a problem hiding this comment.
Approval waits on an engineering owner deciding the architecture:theme-authoring-contract INV15 that this branch carries from #8661: its clause that "an explicit token anywhere in the lineage still wins" over a child theme's tempo and character is not stated by spec:AST-108 FR5 and changes the record's own base-then-generated precedence. Any engineering owner, the author included if they are one, can record that decision by commenting /approve-spec d504154424d4488fde7f96499f099505c5a2ab66 on this pull request or ask for the clause to change, and rebasing the theme work out of this branch also removes it. Whatever the decision, these still need changes:
- A mouse or pen drag never starts where the primary pointer is fine (
GestureHost.ts:580-583), thoughspec:AST-107FR1 and FR2 require every pointer and both doc pages and templates promise one. - The diff carries 22,498 authored lines from thirteen other open pull requests, so rebase onto the landed dependencies until it holds only the snap-point drag.
useDragToSnapPointsnever attaches to an element that mounts after its first render, and after an option change the next drag starts from 0 (useDragToSnapPoints.ts:67-85,createDragToSnapPoints.ts:122-129).- The templates' card moves the wrong way in right-to-left layouts (
DragToSnapPointsCard.tsx:15), and no test fails when the settle ignores the release velocity (spec:AST-094FR38).
[Automated review]
| are the defaults. A duration scale's durations win over `tempo`, and | ||
| explicit tokens win over both. An extending theme inherits motion field by | ||
| field: a duration-scale field or knob the child leaves unset keeps its | ||
| base's value, and an explicit token anywhere in the lineage still wins. Each duration's |
There was a problem hiding this comment.
This clause makes a base theme's explicit motion token outrank a child theme's tempo and character. spec:AST-108 FR5 says explicit tokens win over the knobs, but not that a base theme's do, and this record's own order (resolved base, then generated values, then explicit tokens) lets a child's generated durations replace its base's tokens. Either an engineering owner approves this as the reading, or the clause follows the existing order.
There was a problem hiding this comment.
This needs changes before it can merge. The snap-point drag follows spec:AST-107 apart from the hook point below, but the branch still carries about 34,900 lines from other open pull requests, including the FR41 sentence whose decision belongs to #8665, so please land those and rebase until the diff holds only the drag. After that:
- Call
animator.stop()beforeanimator.dispose()indestroy, so that a drag destroyed during a settle no longer snaps the element back to where the settle began (createDragToSnapPoints.ts:259-269). useDragToSnapPointsupdates options in place through an internal path the public factory lacks, whichspec:AST-107FR6 rules out. Either re-create the drag instead, or have an engineering owner amend FR6.- Add FR6's server-import, listener-count and mid-drag
destroytests, and correct both docs pages' Reduce Motion text for a coarse primary pointer.
[Automated review]
| capture: true, | ||
| }); | ||
| pan.destroy(); | ||
| animator.dispose(); |
There was a problem hiding this comment.
dispose() cancels a running settle without freezing it, and the animator keeps a curve's start inline until the curve retires (animator.ts:1203-1206). So a drag destroyed mid-settle snaps back to where the settle began, which is what a factory caller hits when it re-creates the drag to change snapPoints. Calling animator.stop() first, as began does at line 205, leaves the element where it is, as the docs promise.
| return; | ||
| } | ||
| if ( | ||
| updateDragToSnapPoints(attached.snap, { |
There was a problem hiding this comment.
This internal path keeps a settle in flight across an option change, which a createDragToSnapPoints caller cannot do through the public handle. spec:AST-107 FR6 says the hook "has no behavior of its own", and its verification row fails "a hook with behavior its factory lacks". Now that re-creation starts where the element is drawn, re-creating here (once destroy stops the animator first) would match the factory.
| category: 'Content', | ||
| criterion: '2.3.3 Animation from Interactions', | ||
| description: | ||
| 'When the person asks for reduced motion, the settle after a touch drag becomes a short cross-fade into the snap point, and after a mouse or pen drag it lands at once. `settleTo` lands at once with a mouse and fades on a touch screen. While the finger is down, the element still follows it.', |
There was a problem hiding this comment.
On a coarse primary pointer, such as a tablet's pen, the animator cross-fades a mouse or pen release under Reduce Motion (animator.ts:2978), as spec:AST-094 FR43 asks. This text says the release lands at once. createDragToSnapPoints.doc.mjs:105 has the same sentence.
| snap.destroy(); | ||
| }); | ||
|
|
||
| it('after destroy no drag moves the element and nothing reports', async () => { |
There was a problem hiding this comment.
spec:AST-107 FR6's verification row asks for a server import, listener counts after destroy, and destroy mid-drag. This test checks only that nothing moves. createAnimator.server.test.ts shows the server-import pattern, and a destroy mid-settle would also cover the snap-back.
|
Thanks.
|
There was a problem hiding this comment.
This needs changes before it can merge. The branch still carries about 35,000 lines from other open pull requests, including the FR41 sentence whose decision belongs to #8665, so please land those and rebase until the diff holds only the snap-point drag. After that, useDragToSnapPoints needs one fix: when its snap points or flick speed change, it decides on its own where the element goes, which spec:AST-107 FR6 rules out, and it re-aims a settle that a drag's release started, or drops its onSettle, although the hook's page says the settle carries on (useDragToSnapPoints.ts:144-160); have the hook only create the drag again, or have an engineering owner amend FR6 so the factory owns that behaviour.
[Automated review]
| // Where it was going: the point a settle in flight heads for, else where | ||
| // it rests, else, moved since (a drag or its settle), where it is now. | ||
| const aim = aimRef.current; | ||
| const drawn = drawnOffset(element, axis); | ||
| const goal = aim?.moving | ||
| ? aim.offset | ||
| : aim != null && near(drawn, aim.offset) | ||
| ? aim.offset | ||
| : drawn; | ||
| const nearest = nearestSnapPoint(offsets, goal); | ||
| if ( | ||
| nearest != null && | ||
| (aim?.moving === true || !near(nearest.offset, goal)) | ||
| ) { | ||
| aimRef.current = {offset: nearest.offset, moving: true}; | ||
| snap.settleTo(nearest.index); | ||
| } |
There was a problem hiding this comment.
After new snap points or a new flickThreshold, the hook knows a target only from its own settleTo (:181) or an arrival (:128-131). A settle that a drag's release started is therefore re-aimed at the point nearest where destroy stopped it: a flick from 260 toward 480, interrupted at 330, lands on 240 and reports 1. When the element is already within half a pixel of that point, nothing reports at all. This is also behaviour the factory lacks, which spec:AST-107 FR6 rules out ("has no behavior of its own"). Creating the drag again and nothing more would match a factory caller; if the carry-on is wanted, the factory's handle would need to own it.
| name: 'options.snapPoints', | ||
| type: 'number[]', | ||
| description: | ||
| "Offsets in px along `axis` from the element's resting position, such as `[0, 240, 480]`, sorted before use (`onSettle` and `settleTo` count in that order); an empty list makes the drag do nothing. Unlike BottomSheet's `snapPoints`, they are not fractions of the viewport. New ones create the drag again where the element is: a settle in flight carries on to its point, and a resting element whose point is gone moves to the nearest new one.", |
There was a problem hiding this comment.
"A settle in flight carries on to its point" holds only for a settle this hook's own settleTo started. A release's settle is re-aimed at the nearest point, or never reported, when new snap points arrive (useDragToSnapPoints.ts:144-160).
|
Design question on |
|
Thanks.
|
There was a problem hiding this comment.
This needs changes before it can merge, though the snap-point drag itself now follows spec:AST-107: the hook only creates the drag again when its options change, which fixes the re-aimed settle from the last round. The branch still carries about 35,000 lines from other open pull requests (the pan, the animator, the gesture host, the theme motion knobs and the harness), including the FR41 sentence whose decision belongs to #8665, so please land those and rebase until the diff holds only the drag. The carried theme knobs also leave out the --ease-exit token that spec:AST-108 FR2 names, which belongs in #8661, and the description's line that new options go to the running drag no longer matches what the hook does.
[Automated review]
|
Thanks.
|
…nothing else does (#8671) ## Why Every pull request that gives a component a gesture fails `test-node` once merged with main, and the failure names no component: `scripts/touch-support-bundle.test.mjs` requires Core's root and hooks entries to carry no gesture code. At least six open pull requests (#8142, #8190, #8337, #8159, #8098, #8658) fail it this way. ## What led to it This comes from the work to make Astryx's touch components behave as native iOS ones do. Since #8465, a component that uses a gesture imports the touch modules directly (`architecture:react-component-runtime` INV13), and the root entry re-exports every component, so it carries gesture code as soon as one component does. ## Options and tradeoffs - Exempt the root entry: unblocks everyone, but stops catching a component that pulls gesture code it does not use. - Check each public import on its own: catches exactly that, at the cost of a list of the imports allowed to carry gesture code. ## Why this choice The per-import check keeps INV13's promise, a component without gestures ships none of their code, while letting gesture users through by name. It was offered on #8562 and aimed at main's test. ## What changes - Each public import of Core is built the way the size report builds it, and may carry gesture code only if `GESTURE_USERS` names it. A failure lists the import, its gesture modules with their sizes, and the chain of imports that brings them. - The root entry may carry only what a named import carries; the utils entry stays gesture-free. - A strict case keeps Button and TextInput free of gesture code whatever the list says. - `GESTURE_USERS` starts empty, so every import stays as strict as on main. The pull request that gives a component a gesture adds that component's import, and each import that reaches it, with a comment naming the gesture, so the new gesture user shows up in review. ## How to check it 1. Run `pnpm exec vitest run --project node scripts/touch-support-bundle.test.mjs` on main with this branch merged: it passes. 2. On a tree whose component uses a gesture, run it without naming that import in `GESTURE_USERS`: the test fails and names that import and the chain that brings the gesture code. Name it, and the test passes. 3. Import a gesture module from Button: the strict case fails. ## Evidence - With the list empty, the test passes on main: no public import carries gesture code today. - On open trees that give a component a gesture (AppShell's scroll-edge bar, Slider, the List reorder), the per-import case fails naming exactly those imports and their chains, and passes once each is named. - No product code changes. ## Scope - [x] No intended public API, product behavior, visual direction, or policy change. - [x] Product changes discovered during the work were removed or split. - [x] Public text and artifacts contain no internal Meta context.
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 WebKit interaction job's scope covers every module the gesture host now bundles: hooks/isRtlElement and utils' flatTree, hapticSwitch, inputStream, clickGuard, selectionLock and coarsePointer. The scope test's example of a util outside it moves from clickGuard, which the gesture fixture now bundles, to focusReturn. 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, the haptics module's baseline entry goes with its raw name, and the WebKit interaction job's scope covers the new module. No behavior changes.
…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.
The public animator (spec:AST-107 FR3, FR6): createAnimator(element) over the internal animator, with animate, decay, set, get, getVelocity, stop and destroy, from the package root; and useAnimator(ref), which takes the element's ref like the gesture hooks, so a gesture hook can share the element, and returns the animator. After every commit the hook follows the element behind the ref. The internal factory is renamed createMotionAnimator, so no internal factory shares a public factory's name. Docs, a changeset, and playing-cards templates in React and plain JavaScript, with their registrations.
…s (spec:AST-107 FR1, FR6)
The public pan: `createPanGesture(element, {axis?, onPan}) → {destroy}` and
`usePanGesture(ref, {axis?, onPan})` over it, reporting
`{state, translation, velocity}` as a drag begins, changes, ends or is
cancelled, under a finger, a mouse and a pen; the translation leaves out the
travel the drag needed to begin, so an element that follows it does not
jump. Docs pages for both, with a playing-card example in plain JavaScript
and in React, the route lock and the sandbox's routes.
…element on snap points
`createDragToSnapPoints(element, {axis, snapPoints, flickThreshold?,
onSettle}) → {settleTo, destroy}` and `useDragToSnapPoints(ref, …)` over it
(spec:AST-107 FR2, FR6): a drag moves the element along its axis under every
pointer, and the release settles it where Core's one release policy says
(the nearest point; the next one past the flick speed, 1008 px/s by default;
back to where the drag started when the release moves back toward it), on
the settle spring from the release velocity. Under Reduce Motion a touch
release cross-fades to its point and any other lands at once. Docs pages and
playing-card examples for both, the route lock and the sandbox's routes.
7cf3bfe to
8ec02b0
Compare
There was a problem hiding this comment.
This needs changes before it can merge, though the snap-point drag itself follows spec:AST-107 FR2, FR5 and FR6. The branch still carries about 23,000 lines from #8360, #8676, #8653 and #8655, so please land those and rebase until the diff holds only the drag's own commit. One test gap remains in the drag: nothing passes a non-default flickThreshold through the factory or the hook, which AST-107 FR2's verification asks for, so a test where the same release settles differently with flickThreshold: 600 than with the default would close it.
[Automated review]
| points, | ||
| position: offset, | ||
| velocity: release, | ||
| flickThreshold, |
There was a problem hiding this comment.
No test passes a non-default flickThreshold through the drag: the one custom value in the tests goes straight to resolveSnapPoint (createDragToSnapPoints.test.ts:78-88), so passing DEFAULT_FLICK_THRESHOLD here instead would fail nothing. spec:AST-107 FR2's verification row asks for "a flick past flickThreshold and the default flick speed". A factory test with flickThreshold: 600, where a release of about 800 px/s from 0 at 30 px settles on index 1 while the default keeps it on 0, would cover it.
Stacked on #8655 (and #8653, #8676 and #8360 below it), one commit each on
main, with no merge commits: review only this pull request's commit, vjeux@8ec02b0a29.Why
A product's drawer, card or panel that rests on a few positions wants iOS's release behavior: the nearest position after a slow release, the next one after a flick, and back where it started when the finger moves back. Without a public API, each product rebuilds this from raw pointer events.
What led to it
This comes from the work to make Astryx's touch components behave as native iOS ones do.
spec:AST-107FR2 asks for it over the public pan (#8655) and animator (#8653), with a thin hook (FR6) and Core's release policy (resolveSnapPoint), so drawers and BottomSheet release alike.Options and tradeoffs
snapPointsare, or pixel offsets from the resting position: fractions suit sheets, offsets suit any element.Why this choice
The spec picks pixel offsets and a public
flickThreshold(default 1008 px/s, UIKit's measured switch). New options create the drag again where the element is, which stops a settle in flight there: the hook adds no behavior of its own (FR6).settleTo(index)gives keyboard users the same positions.What changes
createDragToSnapPoints(element, {axis, snapPoints, flickThreshold?, onSettle})from the package root only, anduseDragToSnapPoints(ref, options)from@astryxdesign/core/hooks, which follows the element behind its ref after every commit.spec:AST-094FR43). The settle playssprings.settlefrom the release velocity (FR38); under Reduce Motion a touch release cross-fades, a mouse or pen release lands at once.destroyremoves every listener.How to check it
useDragToSnapPointsexample (a card on Hand, Table and Discard pile), drag past halfway toward Table and release: it settles there. Flick: it carries one place further. Drag toward Table and back: it returns to Hand.Evidence
settleTo.pnpm test:ui(17,804),check:repo, lint,check:changesets, and the snap interaction spec ×10 in Chromium pinned to 2 cores (40) and under the CI WebKit job's command (40); the node suites fail only the twocache.test.mjslock cases that fail on main.Scope