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
2648906 to
3162635
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request can't be approved yet: the experimental rect morph has no approved spec behind it, because it rests on draft spec:AST-094 (FR5 and FR6, with FR8 for Reduce Motion), which this branch also adds. An engineering owner needs to decide whether those requirements govern the morph, with FR8 first brought in line with design:reduced-motion DR2's 200 ms opacity-only cross-fade, and record it by commenting /approve-spec <full head SHA>; the author is an engineering owner and may do so. Separately, the diff against main carries the whole predecessor stack (193 files, about 31,000 changed lines), so once #8076 and its predecessors land, please rebase so this pull request holds only its own commit, and split or justify that commit's 2,045 lines.
spec:AST-094FR4 and DEC-1 say touch motion is not themable, while the stack'sdefineTheme({motion})makes it themable; one of them needs to change.- The note in
rectMorph.tsthat a present fades in and a dismissal fades out under Reduce Motion holds only for paths with an opacity; documentcrossfadeAsfor opacity-less entrances and exits and add a test for one.
[Automated review]
| - **FR8 — Reduce Motion substitutes, never jumps.** Under | ||
| `prefers-reduced-motion: reduce`, a touch presentation MUST behave as its | ||
| UIKit counterpart does with Reduce Motion and Prefer Cross-Fade Transitions | ||
| on: a transition that travels becomes a cross-fade carrying the same change | ||
| of state. An entrance lands in place and fades in, an exit fades out in | ||
| 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 | ||
| owner review:** this is the minimally moving form the draft `design:motion` | ||
| DR5 asks for, for touch presentations (owners: ernestt, cixzhang); the | ||
| alternative is UIKit's Reduce Motion alone, which keeps the slides of | ||
| navigation, sheets and modals. |
There was a problem hiding this comment.
FR8 contradicts current design:reduced-motion DR2, which says each Reduce Motion transition "MUST change opacity only and MUST finish in 200 ms or less" (and DR1 forbids springing content). The 0.5 s navigation cross-fade and the spring-timed sheet and menu fades exceed it, and the animator in this branch already follows DR2 with a 200 ms fade. Suggest having FR8 defer to design:reduced-motion DR1-DR3 for touch presentations.
| - **FR4 — Physics are not themable in touch presentations.** A touch | ||
| presentation MUST read its motion from the table on every theme. Themes keep | ||
| color, shape, type and spacing; the theme's duration and easing tokens keep | ||
| governing pointer presentations. **Proposed; pending owner review:** this | ||
| narrows `architecture:theme-tokens` (owners: cixzhang, imdreamrunner) to | ||
| pointer presentations, and departs from the draft `design:motion` DR3 | ||
| (owners: ernestt, cixzhang) wherever UIKit settles with an overshoot or | ||
| stretches with a rubber band. |
There was a problem hiding this comment.
FR4 (and DEC-1) say touch motion reads the table on every theme, but this branch also adds defineTheme({motion}) so themes can retune touch springs, durations and curves, and the animator plays the theme's value for any table spring. One of the two needs to change.
| * Reduced Motion is the animator's (design:reduced-motion): on a coarse | ||
| * pointer a present lands at rest and fades in, a dismissal fades out where | ||
| * it stands, opacity only, in 200 ms; on any other pointer the change | ||
| * completes at once. |
There was a problem hiding this comment.
This holds only for a path with an opacityAt. Without one (and without crossfadeAs), present first writes the source rect at full opacity and the animator then dips: fade out, land, fade back in (animator.ts:1591-1601). Suggest saying here and on RectMorphPlayOptions.crossfadeAs that an entrance or exit along an opacity-less path passes crossfadeAs, and adding a Reduced Motion test with lerpPath(SQUARE, BOX).
3162635 to
da398d0
Compare
There was a problem hiding this comment.
This pull request can't be approved yet: the experimental rect morph rests only on draft spec:AST-094 (FR5 and FR6, with FR8 for Reduce Motion), which this branch also adds, so an engineering owner needs to decide whether those requirements govern the morph, with FR8 first brought within design:reduced-motion DR2's 200 ms opacity-only cross-fade, and record it by commenting /approve-spec <full head SHA>; the author is an engineering owner and may do so. Separately, the diff against main carries the whole predecessor stack (197 files, about 32,000 changed lines), so once #8076 and its predecessors land, please rebase so this pull request holds only its own commit, and split or justify that commit's 2,047 lines. These also need fixing before the stack lands:
- The adaptation
platformcondition adds a field thatspec:AST-012FR2's closed condition vocabulary does not allow. @astryxdesign/core/motion/reducedMotion.stylexis published as an ordinary, non-experimental subpath under a[fix]changeset, and its comment cites an AST-094 OQ4 that the record does not have.- The adaptive presentation switch reads the default
mdbreakpoint rather than the theme's, although its changeset says it follows the theme. - A theme's motion roles never reach
createRectMorph, which plays the springs it is given, whilespec:AST-094FR4 says touch motion is not themable at all; one direction needs to win. - The rect morph's Reduce Motion note holds only for paths with an opacity; document
crossfadeAsfor opacity-less entrances and exits and test one.
[Automated review]
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
There was a problem hiding this comment.
spec:AST-012 FR2 says a when object "may contain width, height, pointer, contrast, and motion" and that "Future environment fields require explicit additions to this closed vocabulary." This adds a sixth field without amending FR2, so it contradicts the current record. Suggest dropping platform from this stack, or landing an owner-approved FR2 amendment first.
| @@ -177,6 +190,13 @@ const STATIC_EXPORTS = { | |||
| types: './dist/utils/interactionOverlay.stylex.d.ts', | |||
| default: './dist/utils/interactionOverlay.stylex.js', | |||
| }, | |||
| // Proposed; pending owner review (spec:AST-094 OQ4): the Reduce Motion | |||
There was a problem hiding this comment.
spec:AST-094 has only OQ1 and OQ2, so this owner question is not recorded anywhere. And because ./motion/reducedMotion.stylex is an ordinary subpath whose constants carry no @experimental, spec:AST-017 FR21 makes them stable API on release, under a [fix] changeset that never names the import path. Suggest moving them behind motion/experimental, or recording the question in AST-094 and shipping them with a changeset that names the subpath.
| export const COMPACT_QUERY = DEFAULT_COMPACT_DEFINITION.query; | ||
|
|
||
| /** Both: presentations such as a sheet in place of a popover. */ | ||
| export const COMPACT_TOUCH_QUERY = |
There was a problem hiding this comment.
This is the default definition (compactDefinition() with no theme points), so presentation="adaptive" switches at 768 px even when the theme overrides md. .changeset/touch-presentation.md says the components follow "the theme's md point", and spec:AST-012 FR9 says "below the effective md point", with "overridden md" a required verification state. Suggest building the query from the nearest theme's breakpoints and adding that test, or narrowing the changeset to the default.
| play: RectMorphPlayOptions, | ||
| velocity: number, | ||
| ) => { | ||
| const progress = progressCurve(spring, {velocity}); |
There was a problem hiding this comment.
progressCurve solves the spring as given (progress.ts:64) and the animator only applies a theme's role value to a spring it is handed (animator.ts:1529-1530), so a theme that retunes springs.menuOpen never reaches the morph. That matches draft AST-094 FR4 but not the stack's theme motion roles and astryx docs motion; once FR4 is settled, either resolve the spring here through resolveMotion(element, spring) or drop the roles.
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 |
da398d0 to
752ecfb
Compare
There was a problem hiding this comment.
Changes are needed before this can be approved: the diff against main still carries the whole predecessor stack (186 files, about 31,000 authored lines), so once #8076 and its predecessors land, please rebase so this pull request holds only its own commit, and split or justify that commit's 2,019 lines. With the morph now internal it needs no new spec decision, but two fixes remain in it:
createRectMorphturns its spring into a finished curve withprogressCurve, so a theme's motion role such assprings.menuOpennever reaches it, although the motion README says table springs take the theme's value; resolve the spring withresolveMotion(or document that it plays as given) and add a test.- The Reduce Motion note in
rectMorph.tsholds only for paths with an opacity, since an opacity-less present first shows the source rect and then dips, so documentcrossfadeAsfor such entrances and exits and test one.
[Automated review]
Popover's grow out of its trigger and shrink back into it now run on the motion foundation's rect morph (meta#8329). The popover's path is an edge grow (edgeGrowPath) from the point where it meets its source, with the collapsed scale and length from motion/uikit/popover.ts, on UIKit's present and dismiss springs. The morph plays it as translate and scale about the box's corner and presents the same rects as before (the unit tests compare the visual rect at each time with the previous model), interrupts from the rect on screen with its velocity, and retires its inline state once it settles. The dismissal still plays on the layer runtime's exit copy, and the hold during the presentation is unchanged. The layer runtime's keyboard cap reads the layer's offset box, which the morph's translate and scale do not move, so it needs no change. No change in behavior.
…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.
…task ended at once, however late the first read WebKit under load from other tasks: the call's task ends at once, another task holds the main thread, and the browser commits the animation after it, dated at the call. Nothing in the call's task delayed it, but the first read still moved its start to the read, about 95 ms late on a loaded box, so whatever chains on its end (the second half of a Reduce Motion cross-fade, a cancel fading on the previous fade's end) ran that much late and the cross-fade missed design:reduced-motion DR2's 200 ms. Fails now: the start moves to 1100.
e7b0676 moved a call's start from rest to its first read whenever WebKit had dated it more than two frames before that read. Under load from other tasks the first read comes late with nothing in the call's task delaying it, so the start moved about 95 ms on a loaded box, and whatever chained on the call's end ran that late: the second half of a Reduce Motion cross-fade, so SearchField's cancel missed DR2's 200 ms. The call's task now records when it ended, at its first microtask (or a read in those microtasks is that time), and only a call whose own task ran past the call by more than two frames, the block e7b0676 fixed, moves its start; a read other tasks delay keeps the browser's start.
…s its start past the block Core Animation dates an animation at the commit, when the calling task ends, however short the block before it. A 20 ms block in the call's task, with WebKit's start dated at the call, left the start at the call: the two-frame bound skipped it. Fails now.
There was a problem hiding this comment.
Requesting changes: the rect morph itself rests on current specs (architecture:layer-runtime INV15 and spec:AST-094 FR5, FR6, FR8 and FR31), but this branch still carries #8076, #8360, #8340 and #8497, so the diff is 29,966 authored lines while the morph alone is 2,121. Please land those first, rebase so only the morph remains, and bring it under 2,000 lines (for example, the path laws with their geometry tests first) or give concrete evidence that it cannot be split. Separately, architecture:react-component-runtime INV12 has the size-change glide play through this morph once Core has it, while packages/core/src/motion/animatedLayout.ts keeps its own translate, so please land #8721 together with this pull request or have an engineering owner amend INV12 first. The carried gesture host also installs nothing under a fine pointer, which current spec:AST-094 FR19 and FR43 no longer allow; that belongs to #8076's review.
[Automated review]
… task, with no threshold Core Animation dates an animation at the commit, when the calling task ends, whatever the block's length. The call's task already records when it ended, at its first microtask, so a start the browser dated before that (WebKit's last frame time before the task) now moves to it: a task that ended at once leaves the start at the call, a blocked one moves it past the block, however short, and a read other tasks delay changes nothing. The two-frame bound, which told a read's own frame from a block before the task's end was known, goes, and the start no longer moves to the read.
…shows at each progress Menus, Popover, the lifted preview, Lightbox's zoom and a sub-menu's parent card grow a surface out of what the finger touched and shrink it back, and each needs the same geometry. morphPath.ts names it: lerpPath, a straight line between two rects with an optional companion opacity; edgeGrowPath, UIKit's popover growth out of a point on its edge (iOS 26.2); each path's restartFrom, how an interrupted motion continues in its own law; and morphCurves, the translate, scale and opacity curves that put an element laid out in the path's box on its rect. travelAt is exported for the rect morph's player. Internal to core; the tests check both laws, their restarts and the curves at every 1/120 s, and that each presentation's model is the same rect at every progress within 0.5 px.
… keeps only its player The paths (lerpPath, edgeGrowPath, morphCurves and their geometry) now live in morphPath.ts, their own pull request, which this one merges. The morph imports them and re-exports them where its callers import them, so no caller changes; its file keeps createRectMorph, present and dismiss, and its tests keep the player's cases (the popover's model stays for the present that turns around on its edge growth). The README row and the changeset say what is left.
…the clock's own resolution 'a dismissal mid-present starts from the box: reads either side of the switch' read the box and switched in a timer task, with the morph on the timeline's clock. In a timer task WebKit can hand out a timeline time a frame stale (11-15 ms on a loaded box) while it samples the running box fresher, so the morph's start sat behind what was on screen: steps of 1.9-4.5 px against the 0.5 px bound (4 in 48 runs). The reads and the switch now run inside a frame callback, where the timeline's time is the frame's. What is left is the clock's resolution: WebKit counts it in whole milliseconds and samples the box between them, up to 0.54 px at this point of the travel. The bound adds the box's travel in one tick of that clock, measured over the frame before the switch, to the subpixel; the restart the proof has teeth against still steps tens of px. 40 of 40 in WebKit at load 95, 10 of 10 in Chromium.
…sent's first frame on the path 'on screen, the box keeps moving through the switch without a jump' failed 2 to 5 in 32 in WebKit on a loaded box with a 235-238 px worst step, always between the first two tracked frames: the screencast opened on the last frame composited before the present's first, the box at rest, then showed the present at its start 240 px below. That step onto the path is not the switch, as the proof's notes already say. The proof now tracks from the first frame off the rest position, where the present starts. On both the published head and this one the same frame showed up, so it is the screencast's, not the morph's. 64 of 64 in WebKit at load 88, with the container's process cap lifted (its hang is the harness's).
…this head The headers of morphPath.ts and its test named rectMorph.ts and rectMorph.test.ts, which arrive with the pull request stacked on this one, and cited 'law M1', which no document defines. They now describe the player without naming its file and cite spec:AST-094 FR5, which keeps touch motion to translate, scale and opacity on the compositor.
The header cited 'law M1' and 'laws M2, M3', which no document defines. It now cites spec:AST-094 FR5 for compositor-only motion and FR6 for an interruption that starts from the presented rect with its velocity, and its paragraph wraps within the line width.
There was a problem hiding this comment.
This can't be approved yet: the rect morph's player itself rests on current specs (architecture:layer-runtime INV15 and spec:AST-094 FR4–FR6, FR8 and FR31), but landing it while the size-change glide keeps its own translate contradicts architecture:react-component-runtime INV12, and the proposal on #8721 is to drop that clause rather than move the glide. An engineering owner needs to settle whether INV12's rect-morph clause stands, so the glide's move lands with this pull request, or is amended on main first, and record that decision on this pull request by commenting /approve-spec <full head SHA>; as an engineering owner, you may record it yourself. Independently, the branch still carries #8909, #8340, #8497, #8360 and #8076 (30,188 authored lines against the player's 1,597), including a copy of #8360 without its latest dip-landing fix, so please land those first and rebase so only the player remains.
[Automated review]
Depends on #8909 (the paths), #8340 (the animator) and #8497 (the progress module), on #8360. Review only this PR's change: vjeux/astryx@d1b883e...e874b14.
Why
Five touch presentations grow a surface out of what the finger touched and shrink it back: menus (#8080), Popover (#8187), the lifted preview (#8201), Lightbox's zoom (#8231) and a sub-menu's parent card (#8220). Each was writing its own version, and each would have to get interruptions and Reduce Motion right on its own.
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). On iOS, a popover or menu grows from its source and, when interrupted, turns around from where it is on screen.
Options and tradeoffs
width,heightandtop: simple paths, but layout and paint every frame on a phone.Why this choice
A transform about the top-left corner fits every path; a computed origin (the menu's) fits only paths with a fixed point. Restarting an interrupted motion in the path's own law keeps the popover's edge growth, where a straight restart would bend it.
What changes
lerpPath,edgeGrowPath, or a caller's own), which it re-exports.createRectMorphpresents along a path and dismisses back onto a target the caller gives (chore(core): the layer runtime's exit copy, for a touch presentation's close #8186's exit copy, or the element). Each interrupts the other from the rect on screen, keeping its velocity. Nothing stays inline once a present settles.How to check it
pnpm test:interaction rectMorphin Chromium and WebKit: the presentations do no layout or paint per frame.Evidence
Scope