Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
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 owner decision: the new draft spec:AST-094 (FR1-FR23, DEC-1/DEC-2) is the only authority for the Lightbox touch zoom and its UIKit gesture and motion foundations. An engineering owner records the decision by commenting /approve-spec ccbbc4f4e1c93024e4e6a337053cb2204a42f906 on this pull request; the author may do so if they are an engineering owner. Whatever that decision, these need changes:
- Split the 27,757 authored lines (gesture recognizers, motion foundation, interaction harness, Lightbox zoom) into reviewable pull requests.
- Under Reduce Motion, a second zoom or pan key during the cross-fade leaves the photo dimmed (to 28% opacity with
+then-150 ms apart), even after reopening, becausesettleToinuseZoomStage.tsalso stops the opacity fade. - On touch, a Lightbox with
hasZoomoff no longer closes on a backdrop tap: the stage now stretches for every item, though the changeset says nothing changes there. - The new
pr-interactionjob adds a separately routed lane for component tests, whichspec:AST-030FR11 rules out.
[Automated review]
| reducedMotion: AnimateOptions['reducedMotion'], | ||
| ) => { | ||
| const from = current(); | ||
| animator.stop(); |
There was a problem hiding this comment.
Under Reduce Motion this stop() also freezes the opacity of the cross-fade dip animator.play started, and the next dip returns to that frozen value. Pressing + then - within the 500 ms fade leaves the photo dimmed (0.277 opacity with the keys 150 ms apart), and it survives closing and reopening. Settling to an explicit opacity of 1 (and resetting opacity with the item) would fix it.
| alignSelf: {default: null, '@media (pointer: coarse)': 'stretch'}, | ||
| flexGrow: {default: null, '@media (pointer: coarse)': 1}, | ||
| minHeight: {default: null, '@media (pointer: coarse)': 0}, |
There was a problem hiding this comment.
This coarse-pointer stretch (and the one on imageWrapper) applies to every item, so with hasZoom off or a video a tap on the dark area lands on the stage instead of the container, and handleBackdropClick no longer closes the Lightbox. Gating it to the zoom stage keeps the changeset's "With hasZoom off, nothing changes." true.
| ## Touch presentation | ||
|
|
||
| Whether a component presents itself the touch (UIKit) way is answered once, | ||
| in `../utils/touchPresentation.ts`: |
There was a problem hiding this comment.
../utils/touchPresentation.ts and the test said to enforce it do not exist at this head; Lightbox writes its own @media (pointer: coarse) instead.
There was a problem hiding this comment.
At this head ../utils/touchPresentation.ts is there, with touchPresentation.test.ts beside it: both come with #8076, which this stack carries through #8320. Lightbox reads the touch presentation through useTouchPresentation() from that module, one model on every touch device, and the stage's layout keys on its isTouch, with no media query of its own.
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsLightbox (@astryxdesign/core) · View in Storybook
Component 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.
|
| Component | JavaScript | CSS |
|---|---|---|
| core/Lightbox | +35.3KB |
+43B |
Bundle Size Summary
Gzip figures measure each package's entry file, not the code an import pulls in.
| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |
|---|---|---|---|---|
| @astryxdesign/core | 4.9KB | 1.2KB | 1.2KB | 0B |
| Package stylesheet | Gzip before | Gzip after | Delta |
|---|---|---|---|
| @astryxdesign/core | 34.6KB | 34.6KB | +6B |
Accessibility Audit
Status: No accessibility violations detected.
Visual Regression
55 of 502 shot(s) changed.
A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.
| component | story | theme | mode | pixels |
|---|---|---|---|---|
| Selector | Indicator Space Bottom Sheet Narrow End RTL | neutral | light | 753,933 |
| Selector | Indicator Space Bottom Sheet Wide Start | neutral | light | 740,072 |
| RichTextEditor | Markdown parity: side by side | probe | dark | 558,840 |
| RichTextEditor | Markdown parity: side by side | probe | light | 540,756 |
| Chat | Full AI Chat | probe | light | 230,823 |
| Chat | Full AI Chat | probe | dark | 229,805 |
| AppShell | With Mobile Nav | probe | light | 217,010 |
| AppShell | With Mobile Nav | probe | dark | 216,960 |
| Collapsible | Dividers | probe | dark | 66,445 |
| Collapsible | Dividers | probe | light | 66,445 |
| AppShell | Playground | neutral | dark | 65,198 |
| AppShell | Playground | neutral | light | 63,788 |
| Chat | Default | neutral | dark | 36,304 |
| Chat | Default | neutral | light | 35,767 |
| Chat | Empty State | probe | light | 23,531 |
| Chat | Empty State | probe | dark | 22,622 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | dark | 15,378 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | light | 14,107 |
| Chat | With Attachments | probe | light | 13,406 |
| Chat | With Attachments | probe | dark | 11,232 |
and 35 more.
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report
ccbbc4f to
54ebf13
Compare
There was a problem hiding this comment.
This pull request needs an engineering owner's decision on the new draft spec:AST-094 (FR1–FR23, DEC-1 and DEC-2, including its proposed amendments to other records) and on the Lightbox zoom built on it before it can be approved. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, or by saying which requirements should change; the author may do this if they are an engineering owner. Independently of that decision, these need fixing:
- Under Reduce Motion, pressing
+then-quickly leaves the photo at about 25% opacity, even after closing and reopening, becausesettleTostops the opacity fade along with the zoom (useZoomStage.ts:259). - On touch screens a Lightbox without
hasZoomno longer closes on a backdrop tap, because the coarse-pointer stage stretch applies to every item (Lightbox.tsx:162-180). - The new
pr-interactionjob is a separately routed lane, whichspec:AST-030FR11 does not allow, so the interaction proofs should join the existing browser test owner. - The diff carries about 27,900 authored lines from the unmerged stack, so please land or rebase the predecessor pull requests until this one holds only the Lightbox change, and drop the README and MEASUREMENTS passages that describe files not in the pull request.
[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 interaction coverage of existing surfaces (core source and Storybook) through its own scope check, which spec:AST-030 FR11 does not allow: "New coverage inside an existing surface MUST join those owners and MUST NOT create a separately routed lane or required context." Could the proofs run as a step of the existing browser test owner, as the Lightbox arrow-key contract already does, or be admitted through an AST-030 amendment (FR10)?
| where a switch outside any scroll view is needed. | ||
|
|
||
| Done (iOS 26.2 simulator, Oct 23 2026; in the switch's own table, | ||
| `uikit/switch.ts`): M-SW02, M-SW03, M-SW04, M-SW05, M-SW06, M-SW09 (UISwitch |
There was a problem hiding this comment.
uikit/switch.ts (here) and uikit/contextMenu.ts (line 254) are neither in this pull request nor on main; please drop these references until those tables land.
54ebf13 to
a467b57
Compare
There was a problem hiding this comment.
This pull request needs an engineering owner's decision on the new draft spec:AST-094 (FR1–FR23, DEC-1 and DEC-2, with its proposed amendments to other records) and on the public changes built on it: the Lightbox zoom, the two experimental subpaths, the new adaptive breakpoint, and the theme motion roles, which the record's own FR4 and DEC-1 currently rule out. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, or by saying which requirements should change; the author may do so if they are an engineering owner. Independently of that decision, these need fixing:
- Under Reduce Motion, pressing
+then-quickly leaves the photo at about 30% opacity, even after closing and reopening, becausesettleTostops the opacity fade with the zoom (useZoomStage.ts:269). - On touch screens a Lightbox without
hasZoomno longer closes on a backdrop tap, because the coarse-pointer stretch applies to every item (Lightbox.tsx:166-181). experimental-isolation.test.mjsfails on a staledocs-topics.jsongolden, and the motion docs, theme template and changesets describe motion roles and foundation usage that no component has yet.- The
platformadaptation condition needs aspec:AST-012FR2 amendment, and thepr-interactionjob should join the existing browser test owner (spec:AST-030FR11). - Please land or rebase the stacked predecessors so this diff (about 31,800 authored lines) holds only the Lightbox change, and ship the theme, adaptive-presentation and experimental-export changes separately.
[Automated review]
| reducedMotion: AnimateOptions['reducedMotion'], | ||
| ) => { | ||
| const from = current(); | ||
| animator.stop(); |
There was a problem hiding this comment.
Under Reduce Motion this stop() also freezes the opacity of a running fade-through, and when the frozen transform already equals the target nothing replays, so the photo stays dim. + then - 150 ms apart leaves it at opacity 0.299, still 0.299 after closing and reopening (the item reset below does not touch opacity either). Ending every settle at an explicit opacity of 1, and resetting opacity with the item, fixes it.
There was a problem hiding this comment.
Right, a stopped cross-fade froze the photo at about 0.3 opacity, and it survived reopening. Fixed in e282e3c: every settle starts from full opacity, and the item reset writes opacity 1 too. LightboxZoomModes presses + then − 150 ms apart under Reduce Motion, then closes and reopens: 0.285 before the fix, 1 after.
| alignSelf: {default: null, '@media (pointer: coarse)': 'stretch'}, | ||
| flexGrow: {default: null, '@media (pointer: coarse)': 1}, | ||
| flexShrink: {default: null, '@media (pointer: coarse)': 1}, | ||
| flexBasis: {default: null, '@media (pointer: coarse)': 0}, |
There was a problem hiding this comment.
This coarse-pointer stretch (and the one on mediaGroup above) applies to every item, so with hasZoom off or a video a tap on the dark area lands on the stage, not the container, and handleBackdropClick no longer closes the viewer. Moving it into imageWrapperZoomTarget and a zoom-only media-group style keeps the changeset's "With hasZoom off, nothing changes." true.
There was a problem hiding this comment.
Right, the stretched stage swallowed taps meant for the backdrop when zoom is off. Fixed in e282e3c: only a zoom target's stage fills the viewer on a coarse pointer. LightboxZoomModes taps the dark area beside a photo with zoom off, and the viewer closes (it stayed open before).
| * 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 makes the when vocabulary closed (width, pointer, contrast, motion) and says "Future environment fields require explicit additions to this closed vocabulary." Could the FR2 amendment for platform come with this, for the AST-012 owners to approve, or the condition wait for it?
| # 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 interaction coverage of existing surfaces through its own scope check, which spec:AST-030 FR11 rules out: "New coverage inside an existing surface MUST join those owners and MUST NOT create a separately routed lane or required context." Could the proofs run as a step of the existing browser test owner, as the harness typecheck already joins typecheck?
| content: [ | ||
| { | ||
| type: 'prose', | ||
| text: 'On a touch screen, sheets, alerts, menus, navigation and scrolling play the springs, durations and curves measured from iOS. 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.', |
There was a problem hiding this comment.
At this head only Lightbox's zoom reads the motion system; sheets, alerts, menus, navigation and scrolling are unchanged, so a theme that retunes springs.sheetPresent (the example here, in the defineTheme JSDoc and in the template) changes nothing. Could this list only the roles a shipped component plays?
| "best-practices" | ||
| ], | ||
| "skeleton": "e648d618f9141bbd7f15bf970ba9e0d41b59ff9ae89e0cbbb2042e98bd5f0109", | ||
| "skeleton": "a86303e70a6cb63931e8e29c3b470b94f8edcc0b15576a907680a376213faddc", |
There was a problem hiding this comment.
This motion skeleton is stale: the zoom-geometry commit adds the zoomMedia.settle.curve row to astryx docs motion afterwards, so experimental-isolation.test.mjs now fails on this file (and test-node (1/4) with it). Rerunning the golden update and reviewing its diff fixes it.
There was a problem hiding this comment.
Regenerated in e282e3c: the motion topic's skeleton matches again.
| where a switch outside any scroll view is needed. | ||
|
|
||
| Done (iOS 26.2 simulator, Oct 23 2026; in the switch's own table, | ||
| `uikit/switch.ts`): M-SW02, M-SW03, M-SW04, M-SW05, M-SW06, M-SW09 (UISwitch |
There was a problem hiding this comment.
uikit/switch.ts (here) and uikit/contextMenu.ts (line 254) are neither in this pull request nor on main; please drop these references until those tables land.
|
Posted by Cindy's agent on her behalf. Parts of this PR re-implement behavior that already exists, or is being specified, as a shared part of core. Converging keeps one implementation per behavior:
This is about where the behavior lives, not about the PR's intent. |
|
Agreed: one implementation.
|
a467b57 to
e282e3c
Compare
github-actions
Bot
added
the
needs:design-review
|
Pushed in e282e3c, on #8320 @ 41b46ea (with #8076 @ b496085 underneath). Lightbox reads touch through |
e282e3c to
3a3acec
Compare
…roll with its speed A wheel gesture records its scroll type. At its end a trackpad's releases at its last wheel, keeping its speed (its momentum arrives as more wheel), and a mouse wheel's notches release WHEEL_END_MS later, past the velocity's stillness interval, so velocity() and scrollVelocity() read 0 and position alone decides where a pan settles or a pager pages: the coordinator's ruling where FR43 is silent.
…ifts the drag's selection hold at once) into the wheel input
…ill) into the wheel helper
…against a compositor-only control On a two-core runner the software compositor and the screencast share the core the block spins on, so a compositor animation presents fewer frames there than a display-rate bound allows, though it moves on every frame the compositor presents. startBusyControl shows a small box (a manual popover, in the top layer) whose transform moves it for as long as the page runs; tracked from the same screencast and passed to busyWindowProof as control, it is what the compositor presented in that window. The component passes when it moved on the control's frames (90%, one miss allowed) and stalled at most one interval longer than the control; a window where the control moved on fewer than four frames fails. A main-thread animation still fails. Red first: four unit cases (a stall the compositor made for both, which the verdict without the control fails; a frozen component; one the control outpaces; a control that did not move) fail on the old busy.ts. Lightbox's double-tap zoom proof pinned to two cores: its current bound failed 7 of 10 runs, the control judgment 0 of 10; on all cores both pass 10 of 10. The rAF mutant fails against the control in both engines.
…interaction harness's busy-window control) into meta#8148
…base of meta#8147 A busy-window proof can judge a component against a compositor-only control animation recorded in the same window (startBusyControl, CONTROL_RGB, busyWindowProof's control option), instead of a frame-rate bound that a two-core runner's software compositor cannot meet.
…positor-only control On a two-core runner the software compositor and the screencast share the core the blocked main thread spins on, so every compositor animation presents fewer frames there: the proof's frame-rate bound failed 7 runs in 10 pinned to two cores (the pr-interaction red on meta#8215). It now records the interaction harness's control animation in the same window and passes when the photo keeps pace with it (busyWindowProof's verdict); a main-thread animation still fails.
github-actions
Bot
added
the
needs:spec-owner-review
…ver pooled runs A fade that lasts a fraction of a second leaves one blocked window only a few frames: in WebKit, or on a loaded machine, the control can present fewer than the four one window needs, and the proof fails as too little to judge, saying nothing about the animation. pooledBusyProof(runs) judges several runs of the same short animation as one long window is judged against the control: the control must move on at least four frames over all the runs, the component on 90% of them (one miss allowed), and in each run its longest stall may be at most one interval longer than that run's control's, since a stall belongs to its run. Red first: a 30 ms judged window holds at most three frames and one run fails as too little to judge; pooled over six runs, a compositor animation passes and a main-thread one fails, in Chromium and WebKit.
There was a problem hiding this comment.
Approval is on hold for a spec decision: no current record covers Lightbox's touch zoom (component:Lightbox is a draft that states no zoom, and spec:AST-094 leaves each component's touch presentation to that component's own record while its FR43 says gestures attach for every pointer), and this head adds a new architecture:layer-runtime INV13 for the exit copy. An engineering owner can record that decision by commenting /approve-spec followed by the full head SHA on this pull request, or by saying what should change; the author may do so as an engineering owner. Whatever the decision, these still need changes:
- Please land the gesture, motion, harness, lift-module and exit-copy foundations through their own pull requests so this one carries only the zoom; it is 39,987 authored lines today.
- The gesture host this branch adds takes input only under a coarse primary pointer, which current AST-094 FR19 and FR43 and
react-component-runtimeINV13 rule out. - Once merged with
main,useZoomCorenever adds its mouse double-click and drag listeners to a viewer that opens before its photos arrive. - The Chinese
hasZoomdocs and the component JSDoc misdescribe the zoom, the grabbing cursor now waits for movement without being named (AST-017 FR59), and the iOS and Android evidence should come from real devices.
[Automated review]
| // recognizers'. | ||
| const isEnabled = options.isEnabled; | ||
| useEffect(() => { | ||
| const stage = optionsRef.current.stageRef.current; |
There was a problem hiding this comment.
Once this merges with main, where #8741 opens a viewer when its first item arrives, this effect has already run with no stage, and none of its dependencies change when the stage mounts: the mouse double-click and drag listeners are never added until the viewer closes and reopens. Following the element after every render, as the resize subscription below does (:428-441), would fix it, and a test that opens before the media arrives would keep it fixed.
| * coarse, or with `perEventPointer` (FR19; `spec:AST-057`). | ||
| */ | ||
| private isActive(recognizer: GestureRecognizer): boolean { | ||
| return this.isCoarse || this.perEventRecognizers.has(recognizer); |
There was a problem hiding this comment.
Current react-component-runtime INV13 says “The gesture host attaches for every pointer, with no touch-only query”, and AST-094 FR19 “Recognizers attach for every pointer, with FR43's exceptions”; this gate lets a recognizer take input only under a coarse primary pointer, and pointerChanged cancels everything when it turns fine. Could the host follow FR43, with its listed exceptions, in the foundation's own pull request?
…ckpad (spec:AST-094 FR43) The zoom stage's pan, double tap, tap and catch attach for every pointer; the pinch stays a finger's, and a ctrl-wheel reaches it through the gesture core (meta#8676). A double-click is the stage's double tap (the zoom core's own mouse path goes), a wheel pans a zoomed photo, a zoomed drag rubber-bands at the edges and a released one coasts. The stage fills the viewer for every pointer. A release follows the pointer that began it: under reduced motion a finger's cross-fades (DR2) and a mouse's, a trackpad's or a pen's completes at once on a fine primary pointer (DR3). Tests: LightboxEveryPointer's zoom proofs (a ctrl-wheel zooms, a wheel pans a zoomed photo, a released drag coasts); the fine-pointer and zoom mode proofs that asserted the old mouse zoom flip to FR43's; the unit tests compare a mouse's double-click with a finger's double tap. Docs in English and Chinese; a patch changeset, and the touch zoom changeset no longer says a mouse keeps the old zoom. Red: under a mouse the double-click zoomed 2x about the centre, a ctrl-wheel did not zoom, a wheel did not pan a zoomed photo, a released drag did not coast and the stage did not fill the viewer; green after.
…er (spec:AST-094 FR43) The zoom stage's header, its enabled flag and the drag geometry still spoke of fingers and pens only, of a mouse whose limits do not give, and of a double-click on the released zoom; the hasZoom JSDoc still limited the pinch and the double tap to a coarse pointer; the stage's styles still said the touch layer ran on a touch screen only (FR19); and the fine-pointer proofs' header still claimed touch work leaves that zoom alone. They now say what the code does: every pointer reaches the recognizers (the pinch is the fingers' and a ctrl-wheel's), the released zoom is the keyboard's, and under Reduce Motion a finger's release cross-fades while the others complete at once on a fine pointer. Comments only.
… pointer) into meta#8197 The zoom stage keeps this PR's pan and catch conditions on meta#8148's pointer lists, and the zoom core its surface and edge checks while its mouse path goes. The stage's options and layout take meta#8148's ungated forms; the touch presentation stays for the gallery's buttons and arrow keys. The zoom target's fine-pointer proof lists the stage's claimed touches.
…sed it; the opener's zoom inside experimental) into meta#8242 Only the landing into the thumbnail follows the releasing pointer: the zoom out of the thumbnail on open and back into it on close stays touch-only, and without experimental the opener is no source. The stage's ref for early callbacks stays beside meta#8148's zoom wording. Under a cancelled drag the controls that return on their own fade keep this PR's release, and the rest follow meta#8215's pointer; the docs keep this PR's tap sentences after meta#8148's zoom wording, and the mouse and pen unit tests keep this PR's controls proof in meta#8148's form.
… of meta#8197 meta#8148 now carries the gesture core of meta#8676 and meta#8665, which attaches recognizers for every pointer and reads the selection lock from the pointer alone. Two adaptations follow: usePager calls locksSelectionFor with the pointer only, and the bare pager's fine-pointer proof expects a mouse drag to page (spec:AST-094 FR43).
There was a problem hiding this comment.
Approval is on hold for a spec decision: no current record covers Lightbox's new every-pointer zoom (component:Lightbox is a draft that states no zoom, and spec:AST-094 leaves each component's presentation to its own record), the mouse double-click changes from 2× to aspect fill with no way back (spec:AST-017 FR59 and FR3), and this head adds a new architecture:layer-runtime INV13. An engineering owner can record that decision by commenting /approve-spec with the full head SHA on this pull request, or by saying what should change; the author may do so as an engineering owner. Whatever the decision, these still need changes:
- Land the gesture, motion, harness, lift-module and exit-copy foundations through their own pull requests; this one is 45,670 authored lines.
scripts/touch-support-bundle.test.mjsfails on this head because Lightbox now brings the gesture modules into Core's root entry; update that pin to one shared copy and no public entry.- The description says the mouse zoom is unchanged, but a double-click now zooms 1.07× for a 3:2 photo in a 1280 × 800 viewer; describe the zoom for every pointer, add mouse and trackpad evidence before and after, and correct both changesets and the JSDoc.
[Automated review]
| return filledDoubleTapScale; | ||
| } | ||
| const fill = Math.max(width / fitWidth, height / fitHeight); | ||
| return fill > 1 + ZOOMED_EPSILON ? fill : filledDoubleTapScale; |
There was a problem hiding this comment.
For a mouse on a landscape window this returns a zoom barely above 1: a 3:2 photo in a 1280 × 800 viewer fits at 1200 × 800, so fill is 1.067 and a double-click zooms 1.07× where main zooms 2× (1.2× for a 4:3 photo). Only a fill at or below 1.001 falls back to filledDoubleTapScale. Whether the desktop double-click should follow Photos' aspect fill is part of the open owner decision; the description and changeset should state the result either way.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Lightbox's zoom now works under a finger the way the iOS Photos viewer does, on phones and tablets (a coarse primary pointer). Pinch the photo to zoom around your fingers, and move them to pan while you zoom. Past the zoom limits the photo gives way, then eases back when you let go. Double-tap zooms until the photo fills the screen, bringing the tapped point to the centre; double-tap again to go back. A zoomed photo pans with momentum, stretches past its edges and settles back. The keyboard keeps the zoom it had: 2× about the centre, over the same 200 ms ease-out, and keyboard panning now stops at the photo's edges instead of carrying it out of view. |
There was a problem hiding this comment.
At this head every gesture attaches for every pointer (EVERY_POINTER in useZoomStage.ts), so "on phones and tablets (a coarse primary pointer)" no longer bounds who gets this zoom: a finger on a touchscreen laptop gets it too.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Lightbox: a mouse, a pen and a trackpad now use the same gestures as a finger. Ctrl-wheel to zoom; drag or scroll to pan a zoomed photo, which coasts on when released moving. A double-click zooms in at the click, as a double tap does; the keyboard keeps its 2× zoom about the centre. |
There was a problem hiding this comment.
This replaces the released double-click, 2× about the centre, with aspect fill at the click (1.07× for a 3:2 photo in a 1280 × 800 viewer) and makes a zoomed drag start after 10 px. spec:AST-017 FR3 asks a functional default change's changeset to name the old default and how to keep it; neither is named here.
| * zoom like a photo viewer: under a finger, pinch about the fingers (with the | ||
| * give past the limits and the eased return); double-tap or double-click to | ||
| * zoom in at that point and back, Enter/Space on the image or `+`/`-` from | ||
| * the keyboard; and pan (drag, under a finger with momentum and the rubber | ||
| * band at the edges, or arrow keys when zoomed; arrows navigate the gallery |
There was a problem hiding this comment.
At this head a ctrl-wheel pinches and a mouse drag coasts and rubber-bands as well, so describing the pinch, momentum and rubber band as "under a finger" undersells the every-pointer behavior.
Depends on #8147, and on #8665 and #8676 (gestures for every pointer), which this stack carries. Review only this PR's change: vjeux/astryx@b4d3b13...bfe994f
Why
On a phone, a pinch on a Lightbox photo zooms the whole page, and a double tap zooms 2× about the centre wherever it lands. People compare it with Photos on their own phone, where the photo zooms about the fingers, gives at its limits and eases back, and a double tap fills the screen at the tapped point.
hasZoomshould work with fingers.What led to it
The UIKit-parity work for Astryx brings measured iOS behavior to touch screens (
spec:AST-094). This is Lightbox's zoom, on #8147's values and geometry.Options and tradeoffs
import {Lightbox}(52,679 → 88,555 bytes), mostly the gesture host and animator the later PRs share.Why this choice
Only this matches Photos, and it stays smooth when the page is busy. The controls and caption keep the page's own pinch zoom, so text stays zoomable (WCAG 1.4.4). A mouse, a pen and a trackpad zoom the same way (
spec:AST-094FR43): a double-click zooms in, a ctrl-wheel or a trackpad pinch zooms, and a wheel pans a zoomed photo. The keyboard keeps its 2× zoom about the centre.How to check it
In the "With zoom" story, on a phone:
Evidence
Safari on the iOS 26.2 simulator:
Android 15 emulator, Chromium:
