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
needs:design-review
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 is on hold for an engineering owner's spec decision: the branch adds architecture:layer-runtime INV13 and its Exit copy section, rewrites the gesture-tuning ruling in spec:AST-057, and ships a press-feedback exception to spec:AST-094 FR19 that its own README calls pending. An engineering owner can record approval of the head's records by commenting /approve-spec <full head SHA> on this pull request (the author may do this as an engineering owner), while the FR19 exception needs its amendment on main or should come out. Separately, please land the stacked dependencies and rebase so this pull request carries only the lift commit, since the branch now has 39,463 authored changed lines.
.changeset/touch-selection-lock.mdannounces a[fix]for sliders, switches and segmented controls that no component is wired to here.liftScaleinpackages/core/src/lift/liftGeometry.tsbounds only the width, so a trigger taller than the viewport lifts past the bottom gutter andsurfacePlacementputs its menu above the top.
[Automated review]
There was a problem hiding this comment.
Approval is on hold for an engineering owner's spec decision: the branch adds architecture:layer-runtime INV13 and its Exit copy section, rewrites the gesture-tuning ruling in spec:AST-057, and ships a press-feedback exception to spec:AST-094 FR19 that its own README calls pending. An engineering owner can approve the head's records by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), while the FR19 exception needs its amendment on main or should come out. Separately, please land the stacked dependencies and rebase so this pull request carries only the lift (31,587 authored changed lines today), and fix these:
createLayerinpackages/core/src/lift/lift.tsgives the viewport-covering lift layertouch-action: none, so a pinch cannot zoom the page while a trigger is lifted (spec:AST-094FR10, FR35);touch-action: pinch-zoomstill keeps one-finger pans off.liftScaleinpackages/core/src/lift/liftGeometry.tsbounds only the width, so a trigger taller than the viewport less its gutters lifts past the bottom gutter, andsurfacePlacementthen answersabovewith no room there.- The
spec:AST-057edit calls the row's motion roles theme motion roles and drops durations from its Not public list, whichspec:AST-094FR4 and DEC-11 do not allow yet.
[Automated review]
|
Posted by Cindy's agent on her behalf. A naming change to match the records: please rename the module and its exports from lift to liftFromTrigger, for example |
|
On it. This pull request's next push renames the module and its exports to match
ContextMenu's imports follow in #8201. That push also folds the new foundation heads. |
|
Renamed in 6004a1e: |
Under main's rule (themes do not set motion roles; the touch tables register FR4's roles only), a lift's springs are its caller's own values. lift.ts's header, LiftMotion, LiftState and resolvedMotion docs and the module's README no longer say a theme retunes them. Comments only.
…t hold The layer runtime's presentation hold is now the input hold (Layer/inputHold, holdInput). The lift's import, its unit test's spy and the module's docs follow the new names. Behavior is unchanged.
…ord's name
architecture:layer-runtime INV15 calls it the lift-from-trigger module, and in spec:AST-094 a bare "lift" is the finger leaving the screen. The module moves to packages/core/src/liftFromTrigger/ and its exports take the name: createLiftFromTrigger, useLiftFromTrigger, LiftFromTrigger{,Motion,Options,Result}, and the geometry's LiftFromTrigger{Gutter,Viewport,ScaleRule}, liftFromTriggerScale and liftFromTriggerRect. Files, headers, SYNC paths and tests follow. Behavior is unchanged.
…he numbers after it as they were A lifted row's spot was held by a span, so a list held a span among its items and an ordered list numbered the items after it one higher while the row was lifted. Two browser proofs, red on the span: every child of the list is an li while lifted, at the row's size whatever the page styles an li with, and a later item's marker paints the same.
…mber In a ul, ol or menu the placeholder is an li, so the list keeps only li children, as a row menu attached to its list item needs. It is laid out as a block, never a list item, so the items after it keep their numbers, and its own minimum and maximum sizes, padding and border are reset, so the page's rules for an li never change the spot it holds.
…module (INV15) liftCopy places one copy of the source's picture in the caller's host at the source's box (aspect fill, its corners, inert, hidden from assistive technology), against the host's offset, borders and scroll, while the source hides in place with no placeholder; an image source gives its own picture, another the caller's src; a drop shows the source as it was and leaves nothing, twice is once, and with two copies of one source it shows only when the last drops; nothing changes for a source with no box. Red: liftCopy does not exist.
…the lift module (INV15) A zoom from a thumbnail into a viewer needs the thumbnail's picture between the viewer's backdrop and its photo, which a trigger in the top layer cannot be (it would cover the viewer, or sit under its backdrop). liftCopy is INV15's clone: one image of the source's picture (aspect fill, its corners) in the caller's host at the source's box, inert and hidden from assistive technology, while the source hides in place and keeps its layout. The caller moves the copy on the rect morph, which turns an interrupted motion around from the rect on screen; drop() takes the copy away and shows the source as it was once the last copy of it drops. Internal, not exported.
On the fixture, in Chromium and WebKit: the copy lifts into a viewer at the thumbnail's box, one element more and not in the top layer, so the viewer's stacking puts it under the photo, while the thumbnail hides and keeps its layout; it grows toward the photo on the rect morph, a close once it is well on its way turns the same element around from the rect on screen with no frame jumping to either end, and it lands in the thumbnail; the drop shows the thumbnail and leaves the page as it was.
The gaps paragraph said no lift module exists. With the module in packages/core/src/liftFromTrigger/, it names it under INV15 and keeps the gap that remains: no runtime backdrop tint.
The rect morph takes a motion, `{spring}` or a duration with a curve,
where it took a bare spring. The lift wraps each of its four springs,
and the zoom proof its one.
A private data attribute is written only in its owner module, so the lift writes no name of its own. A lift takes the attribute its caller owns for the layer and its parts, and a zoom's copy is marked only when its caller passes one.
The header keeps to its inputs, outputs and place, and the README holds what a lift adds; it gains the placeholder's list rule and the lifted source taking no hits. Inline comments drop their record citations and keep the reason.
The layer runtime's input hold judges each event by its own pointer, so a mouse keeps its clicks during a presentation. The lift is a touch presentation, so its hold proof taps with a finger, before and after the growth settles. The proofs import Playwright through the interaction harness, as the lint rule asks.
Each comment sentence in the module, its tests and its fixture keeps to 25 words, one topic each. A long sentence is split, or cut where the README says it already. The header keeps to 12 lines. No code changes.
A source taller than the viewport less its top and bottom gutters landed past the bottom gutter, and a surface with no room above or below was still placed above. Red: the scale ignores the usable height, and placement never reports that neither side fits.
A lifted source is never taller than the viewport less its top and bottom gutters, as it is never wider. A surface goes above only when it fits under the top gutter; with room on neither side, placement answers `none` and the layer runtime's own fallback places it.
While a row is lifted, a two-finger pinch on the lift layer must zoom the page, tap nothing and reach nothing beneath. Red in Chromium: the layer's `touch-action: none` keeps the page at scale 1. WebKit skips, as the input hold's pinch case does: it pinch-zooms from injected touches only in Mobile Safari on the simulator.
The layer takes `touch-action: pinch-zoom`, as the input hold's shield does: a one-finger pan still scrolls nothing under the lift, and a pinch zooms the page.
A drop while the tint still rises must keep the tint's velocity, so the tint rises a little past where the drop found it before it clears. Red in both engines: the clear restarts from rest at the opacity shown, and the tint falls from the first frame after the drop.
The tint is the opacity of the layer's `::backdrop` on its own motion engine in `pseudoElement` mode, in place of keyframes the lift sampled itself. A drop while the tint rises now turns it with its velocity, and Reduce Motion is the animator's, as for every other motion of the lift. The backdrop's color stays held by its own animation. The tint-clear proof reads what the backdrop shows: a risen tint holds no animation.
The shadow played keyframes the lift sampled itself, where the README says it rides a rect morph. It now plays the source's path on `createRectMorph`, on the source's clock, with its fade on its own spring as the morph's `fade` timing. Its drop is the morph's dismissal from the rect on screen, with its velocity. Reduce Motion is the animator's. The frame-for-frame proof passes unchanged in both engines.
The lift zeroes a row's own margin, and the drop must put it back at once, with none of the row's transitions running. The proof guards the style flush that ends the restore: with the flush removed, the margin's transition runs, as that mutation shows. Page script cannot count layouts, so no proof shows the extra layout itself.
`settleSource` read the source's computed `width`, a geometry value, between its restore writes and the next lift's reads. So a lift that replaced another laid the page out twice in one task. It reads `transitionProperty` instead: the restored values still apply before the transitions return, with no layout.
|
/approve-spec 106ec92 |
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. This is still a draft from a fork, so once it is marked ready for review, a maintainer adds the ci:run label so CI runs.
#9764 (the rect morph's paths): approved, no blocking findings.
#9769 (the rect morph and one eased progress): approved, no blocking findings.
#8485 (the lift-from-trigger module): approved, no blocking findings.
Whole stack: passes. No pull request rewrites code another one adds, and nothing new is exported from the package.
A pull request in this stack lands only when it and every pull request below it are approved.
[Automated review]
The spec owner asked to drop the lift's shadow, since it made the lift look worse. The lift keeps its zoom, tint, surface placement and backdrop tap, and drops the shadow box with its rect morph, springs and option. The page gains three elements while lifted, not four: the layer, the anchor and the placeholder. The proofs assert no shadow element, and the frame-for-frame shadow proof goes.
The proof that the menu grows out of the press read the menu two frames after it began. One WebKit run under the CI profile drew it at 144 px then, past the 125 px bound, because a late frame let the growth run on. The sample now pauses the menu's own animations 30 ms in, reads the box, and ends the growth, so frame timing cannot move it.
There was a problem hiding this comment.
Changes requested for the stack: each pull request's own changes hold up, but the stack needs a rebase onto main and a one-line fix before it can land.
#9764
Verdict: approve (head unchanged since the last review; the stack result below includes a fix in its morphPath.ts).
#9769
Verdict: approve (head unchanged since the last review; the conflict below sits in its import move).
#8485
Verdict: approve on its own changes (the shadow drop keeps the lift at three added elements, and its proofs pass; two optional notes are inline).
Whole stack: changes requested
- Regression: #9769's
easedProgressimport move inpackages/core/src/Popover/popoverSurfaceMotion.tsnow conflicts withmain's newENTRANCE_OPACITY_FLOORimport, so GitHub cannot merge #9769 or #8485 and no tests have run at this head; mergemaininto #9769 keeping both imports, then restack #8485. - Regression: merged with
main, #9764's proof inpackages/core/src/motion/morphPath.test.ts("a popover (#8187) continues an interrupted motion in its own law") fails, becausemain'ssurfaceCurvesnow clamps opacity at 0 whilegrowPath'sopacityAt(morphPath.ts:146) runs to −0.025 on the overshoot; clamp it withMath.max(0, …).
A pull request lands only when it and every pull request below it are approved.
[Automated review]
| height: box.height * scaleY, | ||
| }; | ||
| }, | ||
| opacityAt: p => stateAt(p).opacity, |
There was a problem hiding this comment.
Regression: merged with today's main, morphPath.test.ts:368 fails here ("expected -0.025000000000000022 to be close to +0"). main's surfaceCurves now clamps with Math.max(opacityFloor, opacity(p)), and this opacityAt runs below 0 on the 1.05 overshoot. opacityAt: p => Math.max(0, stateAt(p).opacity) matches Popover's live law again.
| readThemeTiming, | ||
| type ThemeTimingTokens, | ||
| } from '../motion/themeTiming'; | ||
| import {easedProgress, mapProgress, mapRange} from '../motion/progress'; |
There was a problem hiding this comment.
| sheet and anchored presentations, and the one lift-from-trigger module exists | ||
| (INV15, `packages/core/src/liftFromTrigger/`); no runtime backdrop tint |
There was a problem hiding this comment.
Optional: "the one lift-from-trigger module exists" is build status inside a gaps paragraph. Stating INV15's gap as its observable mismatch keeps the record free of temporary status, for example "INV15 gap: ContextMenu and Lightbox do not use the lift-from-trigger module yet".
The lift pinned the source at `offsetWidth` and `offsetHeight`, which round. A source laid out a fraction of a pixel wider than its rounded width, such as a short label in a card, lost that fraction when lifted, and its text wrapped onto a second line past its box. The lift now pins the border box's size from the computed style, which keeps the fraction, and falls back to the offset size for a box with no used size in px.
There was a problem hiding this comment.
Changes requested for the stack: each pull request's own changes hold up, but the stack needs main merged in and a one-line fix before it can land.
#9764
Verdict: approve (head unchanged since the last review; the whole-stack fix below is in its morphPath.ts).
#9769
Verdict: approve (head unchanged since the last review; the conflict below sits in its import move).
#8485
Verdict: approve on its own changes: pinning the trigger at its unrounded layout width keeps a fractional-width trigger's text on its lines, and the lift's browser proofs pass.
Whole stack: changes requested
- Regression: #9769's
easedProgressimport move inpackages/core/src/Popover/popoverSurfaceMotion.tsconflicts withmain's newENTRANCE_OPACITY_FLOORimport, so GitHub cannot merge #9769 or #8485 and no CI has run at this head; mergemaininto #9769 keeping both imports, then restack #8485. - Regression: merged with
main, #9764's proof inpackages/core/src/motion/morphPath.test.ts("a popover (#8187) continues an interrupted motion in its own law") fails, becausemain'ssurfaceCurvesnow clamps opacity at 0 whilegrowPath'sopacityAt(morphPath.ts:146) runs to −0.025 on the overshoot; clamp it withMath.max(0, …).
A pull request lands only when it and every pull request below it are approved.
[Automated review]
Stack: #9764 → #9769 → #8485
Maintainer impact
The UIKit-parity program brings iOS 26's touch presentations to Astryx, and two of them lift their trigger: ContextMenu's long-press preview (#8201) and Lightbox's zoom (#8231). Each was building its own copy. ContextMenu's clone mounted the trigger's content twice, as a second React tree. Lightbox kept its own zoom copy. The two motions would drift apart, and the page paid for both.
Intended invariant
architecture:layer-runtimeINV15: one opt-in lift module, apart from the runtime's hold and exit copy.architecture:component-style-authoringINV7 andspec:AST-017FR60: nothing at rest, at most five elements lifted.spec:AST-107FR7: a listed public-API gap, so it stays internal.Change
There were three options. Each surface could keep its own copy, which doubles ContextMenu's tree. A clone of the trigger's DOM loses a React trigger's state and listeners. Or the trigger lifts itself. This takes the third, with an empty placeholder holding its spot:
packages/core/src/liftFromTrigger/exposescreateLiftFromTrigger()anduseLiftFromTrigger(). Lightbox's copy must sit between the viewer's backdrop and its photo. SoliftCopy(thumbnail, {host})makes one picture in the viewer and hides the thumbnail in place. Everything moves on the animator: the trigger on #9769's rect morph, and the tint on the layer's::backdropchannel. A drop while either rises turns it with its velocity. The lifted trigger casts no shadow. The lift keeps the page's pinch-zoom (touch-action: pinch-zoomon its layer) and fits between the viewport's gutters on both axes.surfacePlacementreports when a surface fits on neither side (spec:AST-094DEC-19); the layer runtime's fallback is then the caller's path. The caller names the lift's marks with a private attribute it owns, so the module writes none of its own. Nothing imports the module yet.Evidence
Scope
Testing
Restacked onto #9769's head 33a3ee8, which holds main's input hold, exit copy, presentation hold and one eased progress. This pull request carries only its own commits:
{spring}motion. The caller names the lift's marks:attributeis required for a lift, and a zoom's copy is marked only when given one. Comments state reasons, not records, in sentences of at most 25 words. The hold proof taps with a finger.The pull requests it carried are merged (#8342, #8186, #8076) or superseded (#8329, by #9764 and #9769).
Main's input hold judges each event by its own pointer, so a mouse keeps its clicks during a presentation. The lift is a touch presentation, so its hold proof taps with a finger. The proofs import Playwright through the interaction harness.
The review rounds add twelve commits, each test ahead of its fix and red before it:
none(3 units red).transitionProperty, notwidth, so a lift that replaces another lays the page out once. No proof can count layouts from page script.Local checks, one worker:
CI=true pnpm lint(0 errors), check:knowledge, check:changesets and test:node (17,544 passed).In the lift fixture (
pnpm test:interaction packages/core/src/liftFromTrigger, Chromium and WebKit):lichildren.liftCopy: one picture appears in the viewer at the thumbnail's box, and the drop brings the thumbnail back.The unit tests (
pnpm vitest run packages/core/src/liftFromTrigger) cover the same and the geometry.Writing rubric results
R1–R6: no narration, research, evidence-cell changes, rejected alternatives, change references or open questions; the layer-runtime gaps paragraph only states which gaps remain.