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
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: No accessibility violations detected. Visual Regression55 of 502 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 35 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested: against main, this pull request carries #8076's whole unmerged chain, 22,516 changed lines across 127 files, while the progress module itself is 334 lines and fits spec:AST-094 FR3 and FR6. Please land the chain first, or rebase once it has landed, so the diff is progress.ts, its test and its changeset alone.
- Add a
progress.tsrow to the motion README's Files table, which the module's SYNC rule names, and replace the header's references torectMorph.tsand "laws M2, M3", neither of which exists in the repository. - Update the description:
presentedProgressVelocitytakes(animator, channels, now?), not(animator, property, from, to).
[Automated review]
| * progress per second; each channel's own velocity stays in the channel's | ||
| * units per second. | ||
| * | ||
| * An interrupted motion continues from what is on screen (laws M2, M3): the |
There was a problem hiding this comment.
"laws M2, M3" are not defined anywhere in the repository, and rectMorph.ts (line 12) does not exist yet. Citing spec:AST-094 FR6 ("A new target MUST keep the current velocity") names the rule this paragraph implements; the rect morph can be named once it lands.
| * (or one shared `at` for both calls); two calls from rest otherwise each | ||
| * start at their own commit. | ||
| * | ||
| * SYNC: When modified, update /packages/core/src/motion/progress.test.ts and |
There was a problem hiding this comment.
This SYNC rule names the motion README, but its Files table has no row for progress.ts; add one so the folder's map lists the module.
There was a problem hiding this comment.
Changes requested: against main, this pull request still carries #8076's unmerged chain, 21,772 changed lines across 120 files, while the progress module itself is 335 lines and fits spec:AST-094 FR3 and FR6. Please land #8076's chain first, or rebase once it has landed, so the diff is progress.ts, its test, its changeset and the README row alone. The README row, the header's references and the description's signature raised on the earlier head are fixed.
[Automated review]
There was a problem hiding this comment.
Changes requested: against main this pull request still carries #8076's unmerged chain, 21,781 changed lines across 117 files, while the progress module itself is 335 lines and fits spec:AST-094 FR3 and FR6. The chain's gesture host also predates DEC-8, now current on main: GestureHost.onScroll lets any scroll take an unclaimed touch without the 10 px take distance FR20 requires, and applyStyle writes the derived touch-action over the author's value instead of intersecting it as FR18 requires. Please bring the chain in line with DEC-8 and land it through #8076, then rebase so this diff is progress.ts, its test, its changeset and the README row alone.
[Automated review]
| }); | ||
| } | ||
|
|
||
| private readonly onScroll = (event: Event): void => { |
There was a problem hiding this comment.
spec:AST-094 FR20 (DEC-8, current on main) now counts a native scroll as the browser taking a touch only once that finger has moved the 10 px scroll take distance (FR2). This handler hands every unclaimed touch inside the scrolling container to browserTookOver on the first scroll event, so a still finger whose scroller moves loses its tap and click; the test at __tests__/GestureHost.test.ts lines 209–215 pins that. Gate it on the finger's own travel from where it went down, in #8076's chain.
| } | ||
| } | ||
|
|
||
| private applyStyle(element: HTMLElement, registration: Registration): void { |
There was a problem hiding this comment.
spec:AST-094 FR18 (DEC-8, current on main) now requires the derived touch-action to be intersected with the element's own author value and never wider than it. applyStyle writes gestureHostStyle(...) inline over the author's value, so a tap recognizer turns an author's pan-y into manipulation and hands back the pinch-zoom the author removed. Intersect with the computed author touch-action before writing, in #8076's chain.
There was a problem hiding this comment.
Changes requested: against main this pull request still carries #8076's unmerged chain, 20,447 changed lines across 119 files, while the progress module itself is 335 lines and fits spec:AST-094 FR3 and FR6. The chain also contradicts current specs: GestureHost.onScroll lets any scroll take an unclaimed touch without the 10 px take distance FR20 requires, applyStyle writes the derived touch-action over the author's value instead of intersecting it as FR18 requires, and useGestureRecognizer calls onGesture from an Effect when a node replacement or unmount cancels a gesture, against architecture:react-component-runtime INV2. Please fix these in #8076's chain and land it there, then rebase so this diff is progress.ts, its test, its changeset and the README row alone. The description also lists mapRange(value, from, to); the function is mapRange(input, output, {clamp}) and returns the map.
[Automated review]
There was a problem hiding this comment.
Changes requested: against main this pull request still carries #8076's unmerged chain, 20,170 changed lines across 120 files, while the progress module itself is 335 lines and fits spec:AST-094 FR3 and FR6. That chain also contradicts current specs, so please fix these in #8076 and land it there, then rebase so this diff is progress.ts, its test, its changeset and the README row:
GestureHost.onScrolllets any scroll take an unclaimed touch without the 10 px take distance FR20 (DEC-8) requires.applyStylewrites the derivedtouch-actionover the author's value instead of intersecting it, as FR18 requires.useGestureRecognizercallsonGesturefrom an Effect when a node replacement or unmount cancels a gesture, againstarchitecture:react-component-runtimeINV2.
[Automated review]
| ) { | ||
| return; | ||
| } | ||
| current?.detach?.(); |
There was a problem hiding this comment.
architecture:react-component-runtime INV2: this detach, like the unmount cleanup at line 247, runs the host's detach, which cancels a recognizer that is mid-gesture (GestureHost.ts line 342). cancelled reaches the listener at lines 75–79 synchronously and isSyncingRef is not set here, so the caller's onGesture fires from this Effect. Run the detach under isSyncingRef, as setEnabled is (lines 86–93), and add a mid-gesture unmount and replacement test, in #8076's chain.
…he press attribute The release note said every press painter writes data-astryx-press through one internal writer, but the action-group tracker (hooks/actionGroupTracker.ts) writes its groups' presses itself, as the gestures README and utils/pressFeedback.ts already say. The note now says paintPress writes every press it tracks and the action-group tracker writes its own.
…llowance under every primary pointer The useMenuPress record's FR9 puts the trigger's held finger under spec:AST-094 FR29 under every primary pointer (FR43), so a finger on a touchscreen laptop may drift 13 pt before the open, as on a phone. The trigger read (pointer: coarse) as its press started and took the long press's 10 px where the pointer was fine. Three cases fail until it does not: a 12 pt drift opens under a fine pointer too, a (12, 12) settle opens there, and nothing reads a pointer query, the trigger's press included.
…lowance under every primary pointer The useMenuPress record's FR9 governs the trigger's held finger by spec:AST-094 FR29 under every primary pointer (FR43). The trigger now hands the long press FR29's 13 px drift allowance whatever the primary pointer, so it no longer reads (pointer: coarse) as its press starts, and the menus drop their coarsePointer import. ContextMenu keeps the long press's own allowable movement. The changeset says the same.
Strict ESLint reported six unused bindings in files this pull request carries: the host's HAPTIC_SWITCH_ATTRIBUTE import, two unused imports in the pan's tests (scrollProbe, ScreenEdgePanGestureRecognizer), the swipe test's drift, the presented-hit-testing test's gestures, and a GestureHost test's before, which now asserts what it was read for: a second finger's moves leave the pan where it was.
…eal angular speed The release velocity of a rotation whose listener sets rotation = 0 on each change, at 2 degrees every 16 ms, read 4.363 rad/s against the 2.182 rad/s of the same stream without a re-base: the test asserted only more than 1.5. It now asserts the 2.18 rad/s (spec:AST-100 FR2), and fails.
The rotation's velocity tracker samples the raw unwrapped angle, which setting rotation does not move, so the setter's shift of the samples read each re-base as extra turn and doubled the release velocity of a listener that re-bases on every change. The setter no longer touches the tracker.
…mates while it runs The proof read the element's animations 230 ms in, after the dip had ended, and asserted only that every animated key is opacity, which an empty list passes: with the cross-fade removed from the plan (the move still landing), the proof still passed. It now reads the keyframe keys right after the call starts and asserts they include opacity and nothing else; the same mutant fails it. Chromium and WebKit pass. (F1's in-flight read from c648034.)
…roller, and its claimed moves are cancelled under pointer events Two comparisons stop at the shadow boundary, each failing a case: - once the host's listener on a recognizer's inner node has seen the finger, isFingerDownIn reads it as outside a light-DOM scroller around that root, so a scroll under it is not the finger's (FR20); - under inputMode 'pointer-events', a claimed pan on a shadow host cancels none of its finger's moves over the inner content, since the finger's target is the inner node and its touches reach the host retargeted (FR19).
…the flat tree isFingerDownIn and the pointer-events finger match compare with containsAlongFlatTree, as the scroll watch already does: a finger on content inside an open shadow root is down in the scroller around that root, and its touches, retargeted to the shadow host, match its pointer finger, so a claimed pan cancels its moves there (spec:AST-094 FR19, FR20).
…achments asks, and only then The host let the last attachment decide whether an element is an action group (useGestureRecognizer always passes the option), and never recomputed it at a detach: a plain attachment after the group's turned the group off, and a group's attachment released left a recognizer that never asked taking presses under a fine pointer. Fails until the host counts the attachments that ask.
…ents asks The host counts the attachments that pass actionGroup: true and keeps the element a group while one remains, recomputed at each detach. An attachment that passes false, as useGestureRecognizer does by default, no longer turns a group off, and a group's last asking attachment taking its leave ends the group.
… that content The presented hit test takes the document's elementFromPoint, which answers a shadow host for anything inside its open root, and compares with Node.contains, which stops at the boundary: a recognizer on a control inside a moving sheet's shadow root gets no finger, and the tap's click goes to the host. Fails until the hit test descends into open roots and compares along the flat tree.
Where a moving element is drawn away from its layout box, the host hit-tests what is drawn under the finger with the document's elementFromPoint, which answers a shadow host for anything inside its open root. The hit test now descends through each open root's own elementFromPoint and compares with the movers along the flat tree, so a recognizer on a control inside a moving sheet's shadow root gets its finger and the tap clicks that control.
…nt lets go The host's touch-action rules sit in an adopted sheet or a style element per document or shadow root, and were never removed, so a page whose last recognizer detached kept them, against spec:AST-094 FR24 (nothing touch-related remains after the last instance unmounts). Fails until a root's rules go with its last marked element.
…th its last marked element The touch-action writer records which elements it marked in each document or shadow root, and when the last one is unmarked (its last recognizer detached) it removes that root's adopted sheet or style element, so nothing touch-related remains after the last instance unmounts (spec:AST-094 FR24). A later mark writes the rules again; an element marked in another root first lets go of the old one.
… the inline style it reads - The motion registry, the host's engine gate and the README name the presented hit test as an engine workaround (spec:AST-013 FR6) with its review condition (re-measure on each iOS release; remove it once iOS WebKit hit-tests a compositor-moved element where it is drawn) and its regression fixture. - While a curve runs the animator holds the value it starts from inline, so presentation.ts and the README no longer say the inline style holds the end.
…core/internal/touch Core ships the internal entry @astryxdesign/core/internal/touch, which re-exports the gesture primitives for Astryx's own packages, and the Lab-import guard fails a Lab import of core outside core's exports (architecture:react-component-runtime INV13). The README and the barrel's header named internal workspace imports and said nothing here is exported; they now name that entry.
…ill needs the element, and the listeners that cancel The release note, the stream's header, the host's comment and the README said the stream is one passive listener per event type and that a document-capture stopPropagation no longer starves the recognizers. Such a stop never hid a listener on the same node, and some subscriptions cancel. They now say what changed: an app's window-capture stopPropagation, or a document-capture stopImmediatePropagation registered before core, no longer hides a touch; a recognizer that must claim still needs the touch to reach its element; the gesture host's touchend, click and contextmenu cancel while a claimer is attached and the page hold's touchmove while it holds; and the stream stays ahead of an app's window-capture listener only until a type's listener is re-added. The README's list of blocking touchmove listeners gains the claimed touch's target and the document while a claiming element moves.
…default it moves
The {stylexOptions} form of astryxStylex now passes StyleX
treeshakeCompensation: true unless the caller sets it. spec:AST-017 FR3
asks the release note to name the old and new default and the option
that restores the old one; it now does.
…er and for the two stops the input stream fixes - A still finger with sub-pixel jitter over a scroll view keeps its tap and scrolls nothing (spec:AST-094 FR18–FR22). - A tap is still recognized under an app's window-capture stopPropagation, and under a document-capture stopImmediatePropagation registered before core's: the recognizers hear touches on the window's capture phase. Chromium and WebKit pass.
…s once Where nothing that claims is attached, the host's touchend listener is passive and cannot cancel the engine's own click after a retargeted tap clicks what is drawn. The shared click guard swallows that late click whatever is attached; this case pins it, as asked on meta#8293.
…esture host's subscriptions The gesture host subscribes no click: the one cancelling click subscriber is the click guard's, added at a window's first arm and kept. The release note, the stream's header and the gestures README listed click with the host's touchend and contextmenu; they now name the guard's own.
Folds this pull request onto the canonical gesture foundation, meta#8360's head, which carries meta#8076, and onto main, in place of the old meta#8076 and harness copies it carried. The one conflict is the motion README's Files table: meta#8360's table, with this pull request's progress.ts row added after presentation.ts.
There was a problem hiding this comment.
Changes requested: the progress module itself looks right and fits spec:AST-094 FR3 and FR6, but against main this branch now carries #8360's whole open chain, 27,237 of its 27,572 changed lines. Please land #8360 first, then rebase so this diff is only progress.ts, its test, the changeset and the README row, and update the description, which still names #8076 and an earlier push. The gesture-host issues raised on earlier revisions (touch-action intersection, a scroll under a still finger, the teardown callback and the screen-edge band) are fixed in the carried code.
[Automated review]
Depends on #8360 (and its predecessors). Review only this PR's change: vjeux/astryx@cba0ad9...3c42849; the merges after take #8360's head and main.
Why
Motions that move several properties along one path, such as a popover growing out of its button while it fades in, all need the same four pieces of math. Each presentation had its own copy of them.
What led to it
This comes from the work to make Astryx's touch components behave as native iOS ones do (sheets, menus, gestures and their motion). The shared rect morph (#8329) needed these pieces; splitting them out keeps that pull request to the morph itself.
Options and tradeoffs
Why this choice
One unit everywhere is what lets an interruption continue instead of restart, and a separate module lets the presentations that follow a spring (a scrim's dim behind a sheet) use the same curve.
What changes
motion/progress.ts, internal, with:progressCurve(spring, {velocity, epsilon}): a spring's progress from 0 to 1 as a closed-form curve, exactly 1 once it settles;mapProgress(driver, f): a curve that follows another throughf;mapRange(input, output, {clamp}): the clamped linear mapfusually is;presentedProgressVelocity(animator, channels, now?): the velocity on screen, so an interruption continues rather than restarts.No component imports it yet; #8329's rect morph is its first caller.
How to check it
pnpm exec vitest run packages/core/src/motion/progress.test.ts.Evidence
Scope