Repository navigation
feat(core): UIScrollView zoom constants and the zoom stage geometry - #8147
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 | 55.0 kB (0) · 9.1 kB CSS (0) | 1,126 (+241) | ✓ · ✓ · ✓ |
Package size: @astryxdesign/core 6.59 MB packed (+9.9 kB); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 9 rows, core 8 rows; 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? |
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]
|
/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]
|
/approve-spec e62fa50 |
There was a problem hiding this comment.
Changes requested. The measured zoom table and the zoom-stage geometry match their iOS 26.2 numbers and component:Lightbox FR6, and the Lightbox record change carries an engineering owner's approval at this head. One blocking finding remains:
- Regression: the zoom settle after a pinch released past a limit plays
timing.curves.movefrom rest for 100 ms + 300 ms × |ln ratio| (zoomMedia.ts:65,zoomStage.ts:336-363), but currentspec:AST-094FR45, FR4 and DEC-26 make a pinch's settle a spring that starts at the release velocity and say no motion playstiming.curves.move, and this pull request leaves that record unamended beside the approved Lightbox FR5 that says the opposite.
To fix it, add the matching spec:AST-094 amendment to this pull request (FR45's list of exceptions, FR4's "no motion plays them" sentence and the FR45 verification row) so an engineering owner can approve it at the new head, or play the settle as a spring from the release velocity and restore FR5's list. The earlier optional notes on the zoomStage.ts comments and the lnRatioMs row wording still apply.
[Automated review]
| * iPhone 17 Pro, CAAnimation read (timing function control points), | ||
| * Oct 23 2026 | ||
| */ | ||
| curve: timing.curves.move, |
There was a problem hiding this comment.
Regression (blocking): this makes the zoom settle after a pinch play timing.curves.move as a timed ease-in-out from rest. Current spec:AST-094 FR45 says motion a gesture drives, including "the settle … after" a pinch, "MUST play a spring … that starts at the gesture's velocity", and FR4 says timing.curves.move stays the measured reference "and no motion plays them". The approved Lightbox FR5 sentence does not amend AST-094. Either add the AST-094 amendment here (FR45's exceptions, FR4's sentence, the FR45 verification row) for an exact-head approval, or play the settle as a spring from the release velocity.
| | FR2 | Viewer overlay carries the current `lightbox` target. Media, Caption, and Counter carry no Lightbox public target. | Current source and public docs | settled | | ||
| | 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 a gesture drives in them (the page settle, the dismiss and the pinch close) is a named motion role read through the motion role table, never a literal, and starts at the gesture's velocity; motion that a tap, a double tap, a key or the program starts (the controls fade and a double tap's zoom transition) plays the theme's duration and easing tokens (`spec:AST-094` FR45). The zoom settle plays `timing.curves.move` for `zoomMedia.settle.baseMs` plus `zoomMedia.settle.lnRatioMs` times \|ln(zoom ratio)\|, 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.
Regression (blocking): this row now says the zoom settle plays timing.curves.move for a timed length, while spec:AST-094 FR45, unchanged here, still makes "the settle … after" a pinch a spring from the gesture's velocity, and its verification row fails a Lightbox photo pinch whose settle "starts from rest". The two current records would disagree; amend AST-094 in this pull request as well.
|
/approve-spec e463256 |
There was a problem hiding this comment.
Changes requested. The measured zoom table and the zoom-stage geometry match their iOS 26.2 numbers and component:Lightbox FR6, and the Lightbox record change carries an engineering owner's approval at this head, but one blocking finding remains:
- Regression: the zoom settle after a pinch released past a limit plays
timing.curves.movefrom rest for 100 ms + 300 ms × |ln ratio| (zoomMedia.ts:65,zoomStage.ts:317-363), while currentspec:AST-094FR45, FR4 and DEC-26 still make a pinch's settle a spring that starts at the release velocity and the FR45 curve arm the new FR5 sentence cites is only proposed in #9915, so merging now leaves the two records disagreeing.
To fix it, merge after that FR45 amendment is on main and rebase onto it, or carry the amendment here for an engineering owner's approval at the new head, or play the settle as a spring from the release velocity and restore FR5's list. The earlier optional notes on the zoomStage.ts comments and the lnRatioMs row wording still apply.
[Automated review]
| | FR2 | Viewer overlay carries the current `lightbox` target. Media, Caption, and Counter carry no Lightbox public target. | Current source and public docs | settled | | ||
| | 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 a gesture drives in them (the page settle, the dismiss and the pinch close) is a named motion role read through the motion role table, never a literal, and starts at the gesture's velocity; motion that a tap, a double tap, a key or the program starts (the controls fade and a double tap's zoom transition) plays the theme's duration and easing tokens (`spec:AST-094` FR45). The zoom settle plays `timing.curves.move` for `zoomMedia.settle.baseMs` plus `zoomMedia.settle.lnRatioMs` times \|ln(zoom ratio)\|, at most `zoomMedia.settle.maxMs`, per `spec:AST-094` FR45; iOS measures this settle keyed by zoom ratio. Under reduced motion they follow AR6. | `spec:AST-094` FR4, FR13 | settled; implementation gap | |
There was a problem hiding this comment.
Blocking: "per spec:AST-094 FR45" cites a curve arm that FR45 on main does not have yet; current FR45 makes the settle after a pinch a spring that "starts at the gesture's velocity", FR4 says no motion plays timing.curves.move, and DEC-26 says any gesture uses a spring. This row can merge once the FR45 amendment proposed in #9915 is on main (rebased onto it), or with that amendment carried here.
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, and Photos' double tap and maximum zoom. UIKit's eased zoom settle stays a measurement only. Lightbox's main interaction is a gesture, so its zoom settles on springs.settle (spec:AST-094 FR45). 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 70a405d |
There was a problem hiding this comment.
Recommending approval, pending CI: one required check is still running on this head.
The measured zoom values and the zoom-stage geometry match the current touch-presentation spec (spec:AST-094 FR1–FR3: measured, sourced values and UIKit's rubber band) and component:Lightbox FR6 (a double tap to the larger of filling the viewer and 2×, about the tapped point), and the unit tests reproduce the iOS 26.2 numbers. The zoom settle that conflicted with spec:AST-094 FR45 is no longer part of this pull request.
Optional: the two earlier line comments on zoomStage.ts still apply (the unit in the comment at lines 343–344, and naming u and v at line 208).
[Automated review]
|
/approve-spec 70a405d |
Stack: #8147 → #8148 → #8197 → #8215, #9810 (both on #8197)
One commit on main 1dce318, which carries #9915's motion classes and #10068's shared theme-timing reader. This round rebases onto it and follows #9915's class for Lightbox: its main interaction is a gesture, so its zoom settles on the shared
springs.settlerole (spec:AST-094FR45, DEC-26;component:LightboxFR5 on main), not on UIKit's eased curve. Specs decision, Nov 4 2026: "Lightbox: shared springs.settle, no new zoom role". SozoomMediadrops the settle's curve and its length law, the zoom stage geometry drops the settle curves it built, and the record keeps main's text, without the Motion roles table this pull request had added. The measured settle stays inmotion/MEASUREMENTS.mdas a reference.Why
Lightbox's touch zoom (#8148) has to feel like the iOS Photos viewer, which people compare it with on their own phones. This pull request adds the measured values and the pure geometry #8148 builds on. Nothing renders differently yet.
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). Photos' aspect-fill double tap and a maximum zoom of 6 times the photo's pixel size are data, each with its measured source.Lightbox/zoomStage.ts: where a zoomed photo rests, the transform that keeps a point in place, the drag's rubber band, and the double-tap and release targets.ZOOM_STEP(2) is Lightbox's own zoom step.motion/MEASUREMENTS.mdandmotion/README.mdlist the zoom measurements. UIKit's eased zoom settle (100 ms plus 300 ms per unit of the zoom ratio's log, at most 400 ms) stays there as a reference only.component:LightboxFR5 already settles the zoom on a spring.How to check it
Nothing changes on screen in this pull request. With #8148 on top, in the "Gallery with zoom" story on a phone:
Writing rubric results
No record line changes in this pull request.
Testing
At this head (70a405d) on main 1dce318:
pnpm install --frozen-lockfileexits 0; coretscis clean; the motion andzoomStageunit files pass 891/891;pnpm check:knowledgepasses. With #8148 on top (1b08e4d), the full Lightbox battery passes (that pull request lists it).