Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
the
needs:design-review
|
AI review status for this pull request.
|
![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 is on hold for a spec decision: the UIKit motion foundation and the zoom geometry rest only on draft spec:AST-094, so an engineering owner (the author qualifies, as an engineering owner) needs to approve or amend its DEC-1, DEC-2, FR1–FR3 and FR5–FR6, together with the amendments it proposes to current records, by commenting /approve-spec <full head SHA> on this pull request. Independently of that decision, two changes are needed:
- Run the new interaction proofs inside an existing CI owner job or the required
testjoin instead of the separately routedpr-interactionjob, whichspec:AST-030FR11 and FR7 do not allow. - Rebase or retarget so this pull request carries only its own zoom commit once its predecessors land; as issued it carries 13,842 authored lines of the stack.
[Automated review]
| # them, so a change to an instrument is proven by its negative controls | ||
| # before a component spec relies on it. Chromium only on these runners; | ||
| # see the harness README for running WebKit. | ||
| pr-interaction: |
There was a problem hiding this comment.
This job routes itself (Check interaction scope) and no join requires it: the test join and .github/scripts/ci-test-join.mjs still list only test-ui, test-node, test-build, registry-contract and test-native-swift. spec:AST-030 FR11 says new coverage inside an existing surface must join those owners and must not create a separately routed lane or required context, and FR7 says an owned lane's failure must fail its join. Hosting these steps in an existing owner job (as pr-a11y hosts the other Chromium guards) or requiring this job from the test join would conform.
| * it rests with no gap at either edge. All constants come from | ||
| * motion/uikit/zoomMedia.ts and motion/uikit/scroll.ts. | ||
| * | ||
| * SYNC: When modified, update zoomStage.test.ts and useZoomStage.ts. |
There was a problem hiding this comment.
useZoomStage.ts is not in the tree at this head (nor is the useZoomStage named on line 12), so this SYNC line points at nothing; point it at a file that exists, or add the hook together with its consumer.
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| Lightbox · Storybook | 52.8 kB (0) · 10.2 kB CSS (0) | 1,117 (+284) | ✓ · ✓ · ✓ |
Package size: @astryxdesign/core 6.31 MB packed (+12.4 kB); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: core 1 row; run pnpm api:report.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
8bdfcde to
64a8d76
Compare
github-actions
Bot
removed
the
needs:design-review
There was a problem hiding this comment.
This pull request is on hold for a spec decision: the zoom geometry and the motion and gesture foundation it carries rest only on draft spec:AST-094, whose FR4 and DEC-1 keep touch motion out of themes, while this head also ships theme motion roles and a platform adaptation condition outside the closed vocabulary of spec:AST-012 FR2. An engineering owner (the author qualifies, as one) records the decision by commenting /approve-spec <full head SHA> once the record and the code agree, or asks for the record to be amended first. Independently of that decision, these changes are needed:
- Rebase or retarget so this pull request carries only its own zoom commit once its predecessors land; as issued it carries 29,365 authored lines of the stack.
- Run the interaction proofs inside an existing CI owner job or the required
testjoin instead of the separately routedpr-interactionjob, whichspec:AST-030FR11 and FR7 do not allow. - Mark
zoomMedia@experimental, since it is exported asuikit.zoomMediafrom@astryxdesign/core/motion/experimentalandspec:AST-017FR21 requires the marker on every declaration there.
[Automated review]
|
|
||
| import {registerMotionTable} from '../roles'; | ||
|
|
||
| export const zoomMedia = { |
There was a problem hiding this comment.
zoomMedia is reachable as uikit.zoomMedia from @astryxdesign/core/motion/experimental (export * as uikit in motion/experimental.ts), but unlike the other declarations there it has no @experimental doc block, which spec:AST-017 FR21 requires ("Declarations carry @experimental"). Adding the doc block, and naming it in the uikit comment and in motion/experimental.test.ts, would conform.
There was a problem hiding this comment.
Right, FR21 asks for the marker on every declaration the experimental subpath exposes. Fixed in ff5cf88: zoomMedia carries a doc block with @experimental, the uikit comment names it, and motion/experimental.test.ts checks both (the doc-block check failed before).
There was a problem hiding this comment.
| ); | ||
| } | ||
|
|
||
| const ease = cubicBezier(zoomMedia.settle.curve); |
There was a problem hiding this comment.
This fixes zoomMedia.settle.curve when the module loads, but the same curve is registered as the motion role zoomMedia.settle.curve and listed by astryx docs motion as something a theme can retune, so no theme value can reach the settle. Either keep the curve out of the role registry or let settleCurves take the curve resolved for the element, whichever the spec:AST-094 FR4 decision calls for.
There was a problem hiding this comment.
Right, the role was listed but no theme value could reach the settle. Fixed in ff5cf88: settleCurves takes the curve the element's theme resolves for zoomMedia.settle.curve (from #8148 on, the shared zoom stage, useZoomCore.ts, passes resolveMotion(stage, zoomMedia.settle.curve)). zoomStage.test.ts runs a linear theme value through it and failed before.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] `presentation="adaptive"` on DropdownMenu, ContextMenu, MoreMenu, Selector and MultiSelector chooses the bottom sheet on a compact touch screen: a coarse primary pointer and a viewport narrower than 744px or shorter than 500px (it was 768px wide and below). An iPad mini in portrait keeps the anchored popover; a phone in landscape now gets the sheet. |
There was a problem hiding this comment.
The same query also decides sub-menu drill-in (useMenuDrillIn), and a sub-menu's presentation defaults to adaptive, so with no opt-in sub-menus now drill in on a phone in landscape and fly out on 744–768 px touch tablets. Worth naming here beside the root presentation.
64a8d76 to
ff5cf88
Compare
github-actions
Bot
added
the
needs:design-review
ff5cf88 to
b4db6a7
Compare
There was a problem hiding this comment.
This pull request is on hold for its governing spec: the zoom geometry and the motion, gesture and theme-motion changes it carries rest on spec:AST-094, which is still proposed in #8378 and not on main, and no record yet covers the press-feedback selection lock or the new undefinedVariables build option. An engineering owner (the author is one) needs to land that record on main, or identify other current spec for these additions. Independently of that decision, these changes are needed:
- Rebase or retarget so this pull request carries only its own zoom commit once its predecessors land; as issued it carries 34,536 authored lines of the stack.
- Run the interaction proofs inside an existing CI owner job or the required
testjoin instead of the separately routedpr-interactionjob, whichspec:AST-030FR11 and FR7 do not allow.
[Automated review]
| @@ -2,7 +2,15 @@ | |||
| "extends": "../../tsconfig.json", | |||
| "compilerOptions": { | |||
| "outDir": "./dist", | |||
| "rootDir": "./src" | |||
| "rootDir": "./src", | |||
| "paths": { | |||
There was a problem hiding this comment.
These paths let Lab typecheck imports of core modules that core's exports do not list (@astryxdesign/core/gestures/* includes the velocity tracker). A Lab file using them would fail for every Lab consumer, and architecture:interaction-modality INV10 says Lab cannot import the tracker. No Lab file uses them at this head; dropping the mappings here and in tsconfig.build.json would conform.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] Under a finger or pen, holding a control or a cell that shows the touch press (the press feedback surfaces) no longer starts text selection or the iOS callout, as on UIKit controls. Selection and the callout come back when the finger lifts. Text fields keep their selection, a link keeps its native menu and preview, and a mouse keeps desktop selection. |
There was a problem hiding this comment.
spec:AST-017 FR7 reserves [fix] for corrections; turning off selection and the callout on every touch press surface is a new behaviour no current record asks for, so it reads as a capability ([feat]) once its authority is settled.
| '@astryxdesign/build': patch | ||
| --- | ||
|
|
||
| [fix] `astryxStylex` resolves `defineConsts` values read from another file. With the `{stylexOptions}` form (the one Storybook uses), a file that read only constants from a `.stylex.ts` module lost that import once StyleX compiled it, so the constants never reached the CSS: a constant used as a media-query key failed the build ("Invalid empty selector") and one used as a value shipped as a custom property nothing declares. A constant defined on the library side of the layer split and read on the product side stayed unresolved in either form. A production build now also warns, naming the variable and the rule that reads it, when its stylesheet reads a StyleX-generated custom property that nothing declares; set `undefinedVariables: 'error'` to fail the build instead. |
There was a problem hiding this comment.
Besides the defineConsts correction, this [fix] adds a capability: the undefinedVariables option and a default production warning. spec:AST-017 FR7 tags compatible capabilities [feat], so splitting it into the fix and a [feat] for the option would conform.
| /** | ||
| * Registers the table's springs and curves as motion roles a theme may | ||
| * retune; the limits, scales and duration law are geometry. Idempotent: | ||
| * Lightbox's touch module (Lightbox/useZoomStage.ts) calls it before its |
There was a problem hiding this comment.
Lightbox/useZoomStage.ts does not exist at this head and nothing calls registerZoomMediaTable() yet; zoomStage.ts:12 has the same useZoomStage reference. Naming the module that will call it once it lands keeps the pointer true.
b4db6a7 to
a33800d
Compare
|
On the review's points:
|
There was a problem hiding this comment.
This pull request is on hold for its governing spec: the zoom geometry and the motion, gesture and theme-motion changes it carries rest on spec:AST-094, which is still proposed in #8378 and not on main, and no record covers the press-feedback selection lock or the new undefinedVariables build option. An engineering owner (the author is one) needs to land that record on main, recording the decision with /approve-spec <full head SHA> on the pull request that carries it, or identify other current spec for these additions, and settle the selection lock and the build option too. Independently of that decision, these changes are needed:
- Rebase or retarget so this pull request carries only its own zoom commit once its predecessors land; as issued it carries 37,668 authored lines of the stack.
- Run the interaction proofs inside an existing CI owner job instead of their own routed
interaction-scopeandpr-interactionlane, whichspec:AST-030FR11 does not allow. - Drop Lab's
tsconfigpaths and the docsite and Sandbox resolvers for core modules core does not export: no Lab file uses them, and the gestures mapping reaches the velocity tracker Lab may not import. - Add real-browser evidence for the moved adaptive edges (852 × 393 and 768 × 1024 with a coarse pointer), as
spec:AST-013FR10 requires.
[Automated review]
| echo 'applies=true' >> "$GITHUB_OUTPUT" | ||
| fi | ||
|
|
||
| pr-interaction: |
There was a problem hiding this comment.
The test join now requires this job, which meets spec:AST-030 FR7, but the proofs still run in a lane of their own with their own routing job (interaction-scope, line 3759). FR11 says new coverage inside an existing surface "MUST join those owners and MUST NOT create a separately routed lane or required context", and the same record names pr-a11y as the pull-request owner that keeps fast interaction guards. Running these steps inside that owner, or inside test-ui, would conform.
| 'springs.alertDismiss': 'spring', | ||
| 'springs.alertPresent': 'spring', | ||
| 'springs.banner': 'spring', | ||
| 'springs.bouncy': 'spring', |
There was a problem hiding this comment.
Once released, a role id is theme input a theme pins, and this one (like springs.swiftUIDefault, springs.uikitDefault and timing.curves.caDefault) names a platform preset rather than a use. architecture:theme-tokens INV3 asks for purpose-led names, and #8378's FR4 renames these presets (springs.emphasis for .bouncy), so a theme that sets them now would break when that record lands.
| content: [ | ||
| { | ||
| type: 'prose', | ||
| text: 'Touch motion has a table of springs, durations and curves measured on iPhone and iPad. Each value is a motion role named by its table path, such as `springs.sheetPresent` or `timing.curves.easeInOut`. A theme retunes a role through `motion.springs`, `motion.durations` or `motion.curves`, at its root or in an adaptation rule; a role it leaves alone keeps its measured default. A motion reads its roles as it starts, so a theme change applies from the next motion.', |
There was a problem hiding this comment.
No shipped component calls resolveMotion or the animator at this head, so a theme that follows this section writes variables nothing reads yet. Saying so here and in the [feat] changeset, or shipping the fields with the first component that reads them, would keep the docs true.
|
Posted by Cindy's agent on her behalf. Cindy decided touch text selection (recorded in #8454, This PR's selection lock should follow that rule: suppress selection only on hold-driven elements, not on every pressable. vjeux, could you update the PR to match? |
|
/approve-spec 1f12539 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested: one fix remains before approval. The measured zoom values and the geometry match component:Lightbox FR6 and spec:AST-094 FR1–FR4, the tests reproduce the iOS 26.2 numbers, and the Lightbox record change has an engineering owner's approval.
Blocking
settleCurvesinpackages/core/src/Lightbox/zoomStage.ts(lines 333–372) re-implements the eased cubic-bézierMotionCurvethatsolveAnimatedPageScrollinpackages/core/src/motion/paging.ts(lines 121–161) already builds, so please have both call one shared helper (or call the existing solver once per axis) and delete the copy.
Optional
ZOOM_STEP's doc says a double-click zooms this many times, butLightbox.tsxstill uses a literal2for the double-click and the+key.- The
zoomMedia.tsheader says touch motion has no theme surface, which no longer matches its three registered roles or the themetempoknob. - Lightbox FR5's "per unit of the zoom ratio's log" leaves out the absolute value and natural log the code uses (
|ln ratio|).
[Automated review]
| const SLOPE_STEP = 1e-4; | ||
|
|
||
| function slopeOf(ease: (p: number) => number) { | ||
| return (p: number): number => { | ||
| const lo = Math.max(0, p - SLOPE_STEP); | ||
| const hi = Math.min(1, p + SLOPE_STEP); | ||
| return hi > lo ? (ease(hi) - ease(lo)) / (hi - lo) : 0; | ||
| }; | ||
| } | ||
|
|
||
| const defaultEase = cubicBezier(zoomMedia.settle.curve); | ||
|
|
||
| /** | ||
| * UIScrollView's zoom settle as curves for the animator. The offset and the | ||
| * scale move together on one eased curve, from `from` to `to`, over | ||
| * `settleSeconds`. The curve is `zoomMedia.settle.curve` by default. | ||
| */ | ||
| export function settleCurves( | ||
| from: ZoomTransform, | ||
| to: ZoomTransform, | ||
| curve: readonly [number, number, number, number] = resolveMotion( | ||
| null, | ||
| zoomMedia.settle.curve, | ||
| ), | ||
| ): { | ||
| readonly duration: number; | ||
| readonly x: MotionCurve; | ||
| readonly y: MotionCurve; | ||
| readonly scale: MotionCurve; | ||
| } { | ||
| const duration = settleSeconds(from.scale, to.scale); | ||
| const ease = | ||
| curve === zoomMedia.settle.curve ? defaultEase : cubicBezier(curve); | ||
| const easeSlope = slopeOf(ease); | ||
| const along = (a: number, b: number): MotionCurve => ({ | ||
| duration, | ||
| at: t => a + (b - a) * ease(Math.min(1, Math.max(0, t / duration))), | ||
| velocity: t => | ||
| ((b - a) * easeSlope(Math.min(1, Math.max(0, t / duration)))) / duration, | ||
| }); |
There was a problem hiding this comment.
This builds each axis as a + (b − a) · ease(t ÷ duration) with its own numeric slope. solveAnimatedPageScroll in motion/paging.ts (lines 121–161) already returns that MotionCurve from {from, to, durationMs, curve}, and the two already disagree on the velocity at the start for a non-ease-in curve. Please have both call one shared eased-move helper (or call the existing solver once per axis) and delete SLOPE_STEP, slopeOf, defaultEase and along.
| export const ZOOMED_EPSILON = 0.001; | ||
|
|
||
| /** | ||
| * Lightbox's zoom step. A double-click zooms this many times, and a double |
There was a problem hiding this comment.
Optional: Lightbox's double-click and + key still use their own literal 2 (Lightbox.tsx:537, :553), so nothing ties them to ZOOM_STEP. Read ZOOM_STEP there, or drop the double-click clause.
| * is the give past the zoom limits, the settle's curve and duration law, | ||
| * and Photos' double tap and maximum zoom | ||
| * @position A domain of the UIKit constants table (motion/uikit). Lightbox's | ||
| * zoom stage reads it. Internal constants: touch motion has no theme |
There was a problem hiding this comment.
Optional: three of these values are now registered motion roles, and spec:AST-108 FR3 lets a theme's tempo scale role durations. Suggested: "The three settle lengths are motion roles. The other values are internal constants."
| | FR3 | Close, Previous, and Next button surfaces use Button's `button` target through IconButton rather than Lightbox-owned button targets. | Current source and target docs | settled | | ||
| | FR4 | The open viewer is a focus region through Core's shared focus-region module (`component:Dialog/DEC-2`): Tab from its last control moves focus to its first and Shift+Tab from its first moves focus to its last, so focus never leaves the viewer for the browser's own interface. | `component:Dialog/DEC-2` | settled; implementation gap | | ||
| | FR5 | Under a coarse primary pointer, after hydration, Lightbox presents FR6–FR10 with no opt-in. Every spring, duration, and curve they play is a named motion role (zoom settle, page settle, dismiss, pinch close, controls fade, zoom transition) read through the motion role table, never a literal. Under reduced motion they follow AR6. | `spec:AST-094` FR4, FR13 | settled; implementation gap | | ||
| | FR5 | Under a coarse primary pointer, after hydration, Lightbox presents FR6–FR10 with no opt-in. Every spring, duration, and curve they play is a named motion role (zoom settle, page settle, dismiss, pinch close, controls fade, zoom transition) read through the motion role table, never a literal. The zoom settle plays `timing.curves.move` for `zoomMedia.settle.baseMs` plus `zoomMedia.settle.lnRatioMs` per unit of the zoom ratio's log, at most `zoomMedia.settle.maxMs`. Under reduced motion they follow AR6. | `spec:AST-094` FR4, FR13 | settled; implementation gap | |
There was a problem hiding this comment.
Optional: the code uses |ln(zoom ratio)|. "per unit of the zoom ratio's log" leaves out the absolute value and the natural log, so read literally a zoom out would shorten the settle below 100 ms.
1f12539 to
3f30514
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 3f30514 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Recommending approval. The measured zoom values and the zoom-stage geometry match spec:AST-094 FR1–FR4 and component:Lightbox FR6, each settle axis now plays through the shared solveAnimatedPageScroll, and the FR5 settle sentence and Motion roles table have an engineering owner's approval for this commit. Three optional touch-ups are in the inline comments: a comment that calls fractions of the fitted box "px", a comment a clearer name would replace, and the lnRatioMs row's wording, which leaves out the absolute value FR5 now states.
[Automated review]
| // The photo point (in fitted-box px of `before`) at the old centre, then | ||
| // the same point in the new fitted box. |
There was a problem hiding this comment.
Optional (INV29): u and v are divided by before.fit.width and before.fit.height, so they are fractions of the fitted box, not px. Suggested: "The photo point at the old centre, as a fraction of the fitted box. The new transform puts it at the new centre."
| scale: number, | ||
| ): ZoomTransform { | ||
| const {fit} = geometry; | ||
| // The fitted-box point under `anchor`. |
There was a problem hiding this comment.
Optional (INV29): this comment explains what u and v hold. Naming them for it (for example photoX, photoY) makes the comment unnecessary.
| | Role | Default | Use | | ||
| | ---------------------------- | ------- | ------------------------------------------------------ | | ||
| | `zoomMedia.settle.baseMs` | 100 ms | The zoom settle's length before its zoom-ratio term | | ||
| | `zoomMedia.settle.lnRatioMs` | 300 ms | The length added for each unit of the zoom ratio's log | |
There was a problem hiding this comment.
Optional: FR5 now says |ln(zoom ratio)|; "each unit of the zoom ratio's log" drops the absolute value, so read alone a zoom out would shorten the settle. "The length added for each unit of |ln(zoom ratio)|" matches FR5.
3f30514 to
8c1e3e6
Compare
There was a problem hiding this comment.
Recommending approval. The measured zoom values and the zoom-stage geometry match spec:AST-094 FR1–FR4 and component:Lightbox FR6, the settle reuses the shared animated-page-scroll curve and rubber band, and the component:Lightbox record change (the FR5 settle sentence and the Motion roles table) carries an engineering owner's approval for this head. The zoom-stage, UIKit-table, motion-role and paging tests, the motion-role generator check, strict lint, the core typecheck and the knowledge and changeset checks pass.
Optional, non-blocking:
zoomStage.tsline 398:uandvare fractions of the fitted box, not px; the comment can say so.zoomStage.tsline 211: naminguandvfor what they hold (for examplephotoX,photoY) makes the comment unnecessary.Lightbox.spec.mdline 162: "each unit of the zoom ratio's log" drops the absolute value FR5 states; "each unit of |ln(zoom ratio)|" matches it.
[Automated review]
|
/approve-spec 8c1e3e6 |
8c1e3e6 to
366123e
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 366123e |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Recommending approval. The measured zoom values and the zoom-stage geometry match spec:AST-094 FR1–FR4 and component:Lightbox FR6, and the component:Lightbox FR5 settle sentence and Motion roles table carry an engineering owner's spec approval for this head. The unit tests reproduce the measured iOS numbers, and the generated motion-role table is in step with the registered roles.
Optional, non-blocking:
zoomStage.ts:398-399:uandvare fractions of the fitted box, not px, so the comment's unit is wrong.zoomStage.ts:211: naminguandvfor what they hold would replace the comment.Lightbox.spec.md:162: "each unit of |ln(zoom ratio)|" would match FR5, where "the zoom ratio's log" drops the absolute value.
[Automated review]
366123e to
79ab09d
Compare
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 79ab09d |
There was a problem hiding this comment.
Recommending approval. The measured zoom values and the zoom-stage geometry match spec:AST-094 FR1–FR4 and component:Lightbox FR6: the settle curve is the timing.curves.move role, each settle axis reuses the shared animated page-scroll curve, the drag reuses the shared rubber band, and the tests reproduce the iOS 26.2 numbers. The component:Lightbox FR5 sentence and Motion roles table carry an engineering owner's /approve-spec for this exact head, and the generated motion role table lists the three new roles.
No blocking findings. The three optional line comments already on the pull request still apply: the unit in the comment at zoomStage.ts:398-399 (fractions of the fitted box, not px), naming u and v at zoomStage.ts:211, and "each unit of |ln(zoom ratio)|" for the lnRatioMs row in Lightbox.spec.md.
[Automated review]
The UIKit constants table gains zoomMedia, measured on iOS 26.2 in a UIKit lab app and in Photos: the give past the zoom limits, the eased settle and its length law, and Photos' double tap and maximum zoom. The settle's three lengths are internal motion roles. component:Lightbox names them in a new Motion roles table, and the generated motion role table carries them. Lightbox/zoomStage.ts is the pure geometry of a zooming scroll view in a photo viewer, checked against those numbers. A double tap zooms to the larger of filling the stage and 2x, about the tapped point, as component:Lightbox FR6 asks. Nothing renders the stage yet: Lightbox's touch zoom plays it in the next change.
|
/approve-spec 1053544 |
There was a problem hiding this comment.
Recommending approval, pending CI: the required checks are still running on this head.
The measured zoom values and the zoom-stage geometry match spec:AST-094 FR1–FR4 and component:Lightbox FR6: the settle curve is the timing.curves.move role, each settle axis reuses the shared animated page-scroll curve, the drag reuses the shared rubber band, and the tests reproduce the iOS 26.2 numbers. The component:Lightbox FR5 sentence and Motion roles table carry an engineering owner's /approve-spec for this exact head, and the generated motion role table lists the three new roles.
No blocking findings. The three optional line comments already on the pull request still apply: the unit in the comment at zoomStage.ts:398-399 (fractions of the fitted box, not px), naming u and v at zoomStage.ts:211, and "each unit of |ln(zoom ratio)|" for the lnRatioMs row in Lightbox.spec.md.
[Automated review]
Stack: #8147 → #8148 → #8197 → #8215, #9810 (both on #8197)
Rebased onto main ed33727, where #8178 (UIDatePicker) added to two of the same files. Both conflicts were additive and keep both sides:
motion/README.mdkeeps main'suikit/datePicker.tsrow, then this pull request'suikit/zoomMedia.tsrow;motion/uikit/index.ts's file header listsdatePickerbesidezoomMedia. Nothing else changed: the own patch differs from 79ab09d's only in that header. At 1053544:pnpm install --frozen-lockfileexits 0; coretscis clean, and so are strict ESLint andpnpm linton the changed files; the motion units pass 861/861 andzoomStage.test.ts31/31;pnpm check:knowledgeandpnpm check:changesetspass.Earlier: rebased onto main 5a373df, where #8159 (Wheel) added to the same two files. Both conflicts were additive and keep both sides:
motion/MEASUREMENTS.mdkeeps main's "Picker (UIPickerView)" section, then this pull request's "Zoom media" section;motion/uikit/index.ts's file header listspickerbesidezoomMedia. Nothing else changed.First pull request of Lightbox's touch stack. It is one commit on
main. First user: #8148, which stacks on it; the two leave draft together.Rebased onto main fc68123 with no content change, so #8148 merges with today's main:
git range-diff 559b0054f9..3f305147c5 fc681230d5..366123e954reads=.Why
Lightbox's touch zoom (#8148) has to feel like the iOS Photos viewer, which people compare it with on their own phones. A pinch past the maximum gives and eases back. A hard stop at the limit, or a bouncy spring, shows at once. This pull request adds the measured values and the geometry #8148 builds on. Nothing renders differently yet.
What led to it
The UIKit-parity work for Astryx measures each touch behavior in a small UIKit test app and in Photos (iOS 26.2 simulator), and it records every number with its source (
spec:AST-094FR1–FR4).component:LightboxFR5–FR13 (current) decide the zoom.Options and tradeoffs
component:LightboxFR5).component:LightboxFR6 asks for the larger of filling the viewer and 2×, about the tapped point, so the stage does that. The table keeps Photos' measured rule as data.Why this choice
Measured values with their source can be checked: a test enforces a source on every entry. Pure geometry is testable without a browser.
What changes
motion/uikit/zoomMedia.ts: past a maximum M the zoom reads M·(2 − M/s), with s the pinch's scale; past a minimum m it reads m ÷ (2 − s/m). It also holds the settle's curve (timing.curves.move) and its three lengths, and a maximum zoom of 6 times the photo's pixel size.registerZoomMediaTableregisters the three lengths as motion roles.Lightbox/zoomStage.ts: where a zoomed photo rests, the transform that keeps a point in place, the drag's rubber band, the double-tap and release targets, and the settle as animator curves, each axis on the eased movesolveAnimatedPageScrollplays.ZOOM_STEP(2) is Lightbox's own zoom step.component:Lightbox: FR5 names the zoom settle's roles, and a Motion roles table gives their defaults (spec:AST-094DEC-5). The generatedmotion-roles.mdcarries them.How to check it
Nothing changes on screen in this pull request. With #8148 on top, in the "With zoom" story on a phone:
Evidence
[chore]changeset.Writing rubric results
component:Lightbox(FR5's added sentence and the new Motion roles section):#followed by digits,/pull/or/issues/.docs/specs/AST-094-touch-presentations/motion-roles.mdis generated, with no hand edits:node scripts/generate-motion-roles.mjs --checkpasses.Testing
At 3f30514 on main 559b005 (the same patch as this head):
tscare clean;pnpm check:knowledgeandnode scripts/generate-motion-roles.mjs --checkpass;resolveMotionand pager unit files pass 332/332;pnpm review:prelists items b, d, f and g (g: the spec owner approves the record change).At 3f30514 the root build, package size, lint, typecheck and both unit suites (test:ui 19,038 tests; test:node 16,071) passed; they run again at this head on main fc68123 before this leaves draft.
Checklist at 79ab09d
pnpm install --frozen-lockfile,pnpm build,pnpm lint(0 errors),pnpm typecheckandpnpm test:ui(19,687/19,687) pass. test:node: 86packages/clifiles fail the same way on main at the same time on the same box (main's known CLI load reds, fixed at their cause in a separate series); 2 more CLI files hit timeouts in this run at a higher box load (45 vs 27). This pull request touches nopackages/clifile.