Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
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 | +51.1KB |
+463B |
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.9KB | 35.1KB | +197B |
Accessibility Audit
Status: No accessibility violations detected.
Visual Regression
57 of 504 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 37 more.
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report
5c55d14 to
a930927
Compare
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request needs an owner decision before it can be approved: pinch to close and the touch presentation it builds on rest on the draft component:Lightbox record and the new draft spec:AST-094, which also amends the current theme-token, scroll-chaining and pressed-state records, so an engineering owner needs to approve or reject that direction by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). Independently of that decision, these need changes:
- The diff against
mainis 35,647 authored lines because it carries the whole #8242 stack; land those pull requests first so this one is only its 874-line pinch-close commit. - Lightbox's open and close effect has no unmount cleanup: after a drag or pinch close, the thumbnail the photo landed on stays hidden if the Lightbox unmounts on close, and the 300 ms decode-wait timer is never cleared.
sourceForis marked "Proposed; pending owner review" in prose only, which would still release it as a stable prop.- AST-094 FR7 cites a
spec:AST-090that does not exist, and theonOpenChangedocs do not mention the pinch.
[Automated review]
| source, | ||
| visibility: source.style.visibility, | ||
| }; | ||
| source.style.visibility = 'hidden'; |
There was a problem hiding this comment.
Only the open and close layout effect restores this (restore() in that effect), and it has no cleanup: a caller that unmounts the Lightbox when onOpenChange(false) fires never gets its thumbnail back after a drag or pinch close. An unmount cleanup that restores landedRef (and clears the open timer below) would cover it.
| // until it can zoom, at most a moment. | ||
| img.style.opacity = '0'; | ||
| img.decode().then(start, start); | ||
| openTimerRef.current = setTimeout(start, OPEN_DECODE_WAIT_MS); |
There was a problem hiding this comment.
This timer, and the decode() promise above it, outlive the component: the effect returns no cleanup, so a Lightbox unmounted within 300 ms of opening on an undecoded photo still runs start() and zoomFromSource against its detached container (react-component-runtime INV5). Returning a cleanup that clears openTimerRef and advances openTokenRef and closeTokenRef would release them.
| * opened the viewer is the opened item's source when it shows a picture | ||
| * (an image, or a button or link around one). | ||
| * | ||
| * Proposed; pending owner review. |
There was a problem hiding this comment.
"Proposed; pending owner review" in prose does not make sourceFor provisional: AST-017 FR21 recognizes only an experimental prop object with @experimental and stability: 'experimental' docs, so as written the prop releases as stable API. Drop the wording once an owner approves it, or move it behind that boundary.
| follows. | ||
| - **FR7 — Exits animate as UIKit's do.** A touch presentation MUST play the | ||
| exit its UIKit counterpart plays. **Proposed; pending owner review:** this | ||
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 |
There was a problem hiding this comment.
No spec:AST-090 exists in the repository, at this head or on main, so the owners asked to rule on this amendment have no FR1 or FR10 to compare. Cite the record that actually states exits off by default and the instant reduced-motion close, or drop the clause (it recurs in the current-state impact).
| type: '(isOpen: boolean) => void', | ||
| description: 'Callback when the lightbox open state changes.', | ||
| description: | ||
| 'Callback when the lightbox open state changes: the Close button, Escape, a tap beside the media, or (on a touch screen) dragging a photo down.', |
There was a problem hiding this comment.
This list of close paths now misses the pinch in; the Chinese description and the dense onOpenChange text say the same and need it too.
a930927 to
9f5a29e
Compare
There was a problem hiding this comment.
This pull request needs an owner decision before it can be approved: pinch to close and the touch presentation it builds on rest on the draft component:Lightbox record and the new draft spec:AST-094, which also amends the current theme-token, pressed-state and scroll-chaining records, so an engineering owner needs to approve or reject that direction by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). Independently of that decision, these need changes:
- The diff against
mainis 36,068 authored lines because it carries the whole #8242 stack; land those pull requests first and rebase, so this one is only its 874-line pinch-close commit (the rebase also clears the unrelatedpr-a11yfailure). - Lightbox's open and close effect has no unmount cleanup: after a drag or pinch close, the thumbnail the photo landed on stays hidden if the Lightbox unmounts on close, and the decode-wait timer is never cleared.
- Gallery Previous/Next buttons and arrow keys now slide on UIKit's paging curve for mouse and keyboard users too, and keep travelling under reduced motion, although AST-094 itself leaves pointer presentations on the theme's motion tokens.
sourceForis marked "Proposed; pending owner review" in prose only, which would still release it as a stable prop.- AST-094 FR7 cites a
spec:AST-090that does not exist, and theonOpenChangedocs do not mention the pinch.
[Automated review]
|
|
||
| useEffect(() => { | ||
| if (index != null && index !== pager.index) { | ||
| pager.goTo(index, {reason: 'controlled'}); |
There was a problem hiding this comment.
This runs for every controlled index change, so Lightbox's Previous and Next buttons and its arrow keys now slide on UIKit's setContentOffset(animated:) curve for mouse and keyboard users too, and play keeps the travel under reduced motion (reducedMotion: 'keep' at line 350). The base swaps the item in place, architecture:theme-tokens INV3 has components consume the theme's motion roles, and AST-094 itself keeps pointer presentations on the theme's motion tokens and turns travel into a cross-fade under Reduce Motion (FR8). Keeping non-touch navigation on the swap (or a theme motion token) and dropping the travel under reduced motion would match both.
9f5a29e to
2cebd19
Compare
There was a problem hiding this comment.
This pull request needs an owner decision before it can be approved: pinch to close and the touch presentation it builds on rest on the draft component:Lightbox record and the new draft spec:AST-094, which also amends the current theme-token, pressed-state and scroll-chaining records, so an engineering owner needs to approve or reject that direction by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner). Independently of that decision, these need changes:
- The diff against
mainis 36,428 authored lines because it carries the whole #8242 stack; land those pull requests first and rebase, so this one is only its 1,079-line pinch-close commit. - Lightbox's open and close effect has no unmount cleanup: after a drag or pinch close, the thumbnail the photo landed on stays hidden if the Lightbox unmounts on close, and the decode-wait timer is never cleared.
- Gallery Previous/Next buttons and arrow keys now slide on UIKit's paging curve for mouse and keyboard users too, and keep travelling under reduced motion, although AST-094 itself leaves pointer presentations on the theme's motion tokens.
- The
hasZoomdocs and the changeset say a pinch "released below about 93.5 %" closes, but the code decides on the smallest scale and the reopening (a pinch in to 0.6 and out to 0.7 springs back), and theonOpenChangedocs still omit the pinch. sourceForis marked "Proposed; pending owner review" in prose only, which would still release it as a stable prop, and AST-094 FR7 cites aspec:AST-090that does not exist.
[Automated review]
| type: 'boolean', | ||
| description: | ||
| 'Enable zoom on double-click, or Enter/Space/+/- via keyboard (images only). When zoomed, drag or use arrow keys to pan.', | ||
| 'Enable zoom (images only): pinch, or double-tap or double-click to zoom in at that point and back, or Enter/Space/+/- via keyboard. When zoomed, drag (with momentum) or use arrow keys to pan. On a touch screen a tap on the photo hides the controls and turns the background black, as a zoom in does; the next tap, a return to the fitted size, or a keyboard focus on a control brings them back. A pinch in from the fitted size shrinks the photo and, released below about 93.5 %, closes the viewer into its thumbnail.', |
There was a problem hiding this comment.
This says a pinch "released below about 93.5 %" closes, but pinchCloses (zoomStage.ts:582-585) decides on the smallest scale and the reopening: in to 0.6 and out to 0.7 springs back although it is released far below 93.5 % (the PR's own proof, LightboxPinchClose.interaction.browser.spec.ts:423-436), and in to 0.92, released at 0.95, closes. "Goes below about 93.5 % and is not opened again by 5 % before the lift" would match the code; the Chinese text at line 223 needs the same.
|
|
||
| [feat] Pinching in on a zoomable Lightbox photo at its fitted size now closes the viewer into the photo's thumbnail, as in the iOS Photos viewer. | ||
|
|
||
| While the fingers pinch in, the photo shrinks with them about the point between them and follows it. The backdrop fades on Photos' measured curve and the controls fade faster. On release, a pinch below about 93.5 % closes: the photo flies into its thumbnail, or shrinks and fades in place when it has none. A pinch released above that, or reopened by 5 % before the lift, springs back to the fitted size. |
There was a problem hiding this comment.
"A pinch released above that ... springs back" is not what the code does: a pinch in to 0.92 and released at 0.95 (reopened by 0.03) closes, because pinchCloses only asks whether the smallest scale went below 0.9347 and the reopening stayed under 0.0502.
2cebd19 to
402cc0b
Compare
There was a problem hiding this comment.
This pull request needs an engineering owner's decision before it can be approved. Pinch to close and the touch presentation under it rest on the draft component:Lightbox record and the new draft spec:AST-094, and the stacked theme motion roles conflict with AST-094's own DEC-1. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these need changes:
- Reduce the diff to the pinch-to-close commit; it currently carries 40,755 authored lines from the open stack.
- Restore the caller's thumbnail and clear the open timer when Lightbox unmounts on close, since a pinch close currently leaves the thumbnail hidden.
- Regenerate
packages/cli/test/__golden__/no-icon-set/docs-topics.json, whose motion topic now failsexperimental-isolation.test.mjsin CI. - Give the touch defaults a restoring
presentationvalue as the currentarchitecture:public-component-apiINV16 asks, and report Lightbox's size delta with the touch modules loaded lazily or justified. - Correct the release-rule wording in
Lightbox.doc.mjsand the changeset, and add the pinch to theonOpenChangeclose paths.
[Automated review]
| source, | ||
| visibility: source.style.visibility, | ||
| }; | ||
| source.style.visibility = 'hidden'; |
There was a problem hiding this comment.
This hides the caller's thumbnail, and only restore() in the open/close layout effect below brings it back. That effect returns no cleanup, so a caller that renders {isOpen && <Lightbox …/>} keeps an invisible thumbnail after a pinch close or a drag to dismiss. The open timer and the decode() continuation from that effect also outlive an unmount. A cleanup that calls restore(), clears openTimerRef and advances openTokenRef and closeTokenRef would cover all three.
| "best-practices" | ||
| ], | ||
| "skeleton": "e648d618f9141bbd7f15bf970ba9e0d41b59ff9ae89e0cbbb2042e98bd5f0109", | ||
| "skeleton": "98c9196254cdf2ebb6fb700b0395137fd5d1128e0ebc41c257c577e8cf6c0ee8", |
There was a problem hiding this comment.
This skeleton is stale: experimental-isolation.test.mjs › "every stable docs topic" now produces a different skeleton hash for the motion topic, because the role table in motion.doc.mjs gained rows after this golden was last refreshed. Regenerating it fixes the failing test.
| type: 'boolean', | ||
| description: | ||
| 'Enable zoom on double-click, or Enter/Space/+/- via keyboard (images only). When zoomed, drag or use arrow keys to pan.', | ||
| 'Enable zoom (images only): pinch, or double-tap or double-click to zoom in at that point and back, or Enter/Space/+/- via keyboard. When zoomed, drag (with momentum) or use arrow keys to pan. On a touch screen a tap on the photo hides the controls and turns the background black, as a zoom in does; the next tap, a return to the fitted size, or a keyboard focus on a control brings them back. A pinch in from the fitted size shrinks the photo and, released below about 93.5 %, closes the viewer into its thumbnail.', |
There was a problem hiding this comment.
pinchCloses decides on the smallest scale and the reopening, not the scale at release. In to 0.92 and released at 0.95 closes; in to 0.6 and out to 0.7 springs back. "Goes below about 93.5 % and is not opened again by 5 % before the lift" would match the code. The Chinese text at line 223 needs the same change.
There was a problem hiding this comment.
Right, the rule reads the smallest scale and the reopening, not the scale at the lift. Fixed in f7cf46a: 'goes below about 93.5 % … unless it is opened again by 5 % before the lift', in English and Chinese.
|
|
||
| [feat] On iOS, pinching in on a zoomable Lightbox photo at its fitted size now closes the viewer into the photo's thumbnail, as in the iOS Photos viewer. Elsewhere a pinch in stops at the fitted size. | ||
|
|
||
| While the fingers pinch in, the photo shrinks with them about the point between them and follows it. The backdrop fades on Photos' measured curve and the controls fade faster. On release, a pinch below about 93.5 % closes: the photo flies into its thumbnail, or shrinks and fades in place when it has none. A pinch released above that, or reopened by 5 % before the lift, springs back to the fitted size. |
There was a problem hiding this comment.
"A pinch released above that ... springs back" does not match pinchCloses. A pinch in to 0.92 and released at 0.95 closes, because only the smallest scale and a reopening under 0.0502 count.
| type: '(isOpen: boolean) => void', | ||
| description: 'Callback when the lightbox open state changes.', | ||
| description: | ||
| 'Callback when the lightbox open state changes: the Close button, Escape, a tap beside the media, or (on a touch screen) dragging a photo down.', |
There was a problem hiding this comment.
The close paths now include a pinch in from the fitted size. The Chinese description (line 199) and the dense text (line 310) need it too.
There was a problem hiding this comment.
Right. Fixed in f7cf46a: the close paths name the pinch in, in English, Chinese and the dense description.
|
|
||
| useEffect(() => { | ||
| if (index != null && index !== pager.index) { | ||
| pager.goTo(index, {reason: 'controlled'}); |
There was a problem hiding this comment.
Every controlled index change animates here, so Lightbox's Previous and Next buttons and arrow keys now slide for mouse and keyboard users. play keeps that travel under prefers-reduced-motion: reduce (reducedMotion: 'keep'). The base swaps the item in place, and AST-094 FR8 turns travel into a cross-fade. Swapping or cross-fading for non-touch navigation, or under Reduce Motion, would keep that.
There was a problem hiding this comment.
| follows. | ||
| - **FR7 — Exits animate as UIKit's do.** A touch presentation MUST play the | ||
| exit its UIKit counterpart plays. **Proposed; pending owner review:** this | ||
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 |
There was a problem hiding this comment.
There is no spec:AST-090 in the repository, at this head or on main, so the owners asked to rule here have no FR1 or FR10 to compare against. Cite the record that states those rules, or drop the clause; it recurs at line 235.
| // Photos' pinch to close fades them by the pinch's scale instead. It | ||
| // is the iOS idiom (unverified on Android): elsewhere a pinch in | ||
| // stops at the fitted size. | ||
| onPinch: |
There was a problem hiding this comment.
Pinch to close is on by default on iOS with no way for a caller to keep the previous behavior. The current architecture:public-component-api INV16 and spec:AST-017 FR3 expect a presentation value that restores the fine-pointer mode, named in the changeset, unless an approved Lightbox record defines this as touch-only behavior.
| // and large touch displays stay anchored, phones in either orientation get | ||
| // the sheet. The query is the shared touch presentation's | ||
| // (utils/touchPresentation.ts). | ||
| export const COMPACT_TOUCH_PRESENTATION_QUERY = COMPACT_TOUCH_QUERY; |
There was a problem hiding this comment.
This moves presentation="adaptive" for DropdownMenu, ContextMenu, MoreMenu, Selector and MultiSelector from 768 px wide to under 744 px wide or under 500 px tall. That is unrelated to pinch to close and would review better in its own pull request.
|
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.
|
402cc0b to
f7cf46a
Compare
|
Pushed in f7cf46a: the pinch-close release passes |
… 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.
…croll (spec:AST-094 FR43) usePager attaches its pan and catch for every pointer instead of only under a coarse primary pointer, and its pan takes a trackpad's or a wheel's horizontal scroll through the gesture core. Lightbox's gallery lets the mouse page too. A click beside a gallery photo with zoom on is the stage's tap for every pointer, after the double-tap interval. Tests: LightboxEveryPointer's paging proofs (a mouse drag sideways and a trackpad's horizontal scroll page a gallery); the paging mode and fine-pointer proofs and usePager's React test that asserted no input under a fine pointer flip to FR43's. Docs in English and Chinese; the every-pointer changeset gains paging, and the paging changeset no longer says a mouse pages only with the buttons. Red: under a mouse a sideways drag and a trackpad's horizontal scroll did not page, the pager attached nothing on a fine pointer and the gallery claimed no touches; green after.
meta#8197 brings the zoom's and the pager's gestures for every pointer. The drag to dismiss keeps its finger and pen pointer list (the zoom's shared list is now every pointer's) and its touch-presentation gating until its own change; its imports drop meta#8197's coarse-pointer read, and the fine-pointer proofs keep meta#8197's gallery proof beside this PR's.
…ose (spec:AST-094 FR43) The drag to dismiss attaches for every pointer (a video keeps its own controls' drags), so a mouse or a pen drag down, or a wheel or trackpad scroll that moves a fitted photo down, closes the viewer. Its release follows the pointer that began it: the stage tells the owner whether a finger released it, and under reduced motion a finger's release cross-fades (DR2) while a mouse's, a trackpad's or a pen's completes at once on a fine primary pointer (DR3), for the photo, the backdrop and the controls alike. Tests: LightboxEveryPointer's dismiss proofs (a mouse drag down closes, one brought back springs back, a pen drag down closes, a wheel notch over a fitted photo closes, a mouse's release under Reduce Motion closes at once); the recognizer unit test and the fine-pointer proof that asserted no drag to dismiss under a fine pointer flip to FR43's. Docs in English and Chinese; the every-pointer changeset gains the drag down, and the drag-to-dismiss changeset no longer says the mouse is unchanged. Red: under a mouse, a pen or a wheel the drag down did not move or close the photo, nothing attached on a fine pointer and a fitted photo claimed no one-finger drags; green after.
…er released it (spec:AST-094 FR43) A drag down that lands the photo in its thumbnail, and the thumbnail's picture that fades in over it, follow the pointer that began the drag, as the straight-down dismissal does: under reduced motion a finger's release cross-fades (DR2) and a mouse's, a trackpad's or a pen's lands at once on a fine primary pointer (DR3). The touch presentation's readers are named where it is read. Tests: the zoom transition's proof that a mouse's drag down under Reduce Motion lands in the thumbnail at once. Red: under Reduce Motion a mouse's drag down into the thumbnail cross-faded for 300 ms before the viewer closed; green after.
…nothing calls The zoom transition plays on the stage's rect morph, so the stage's own play (and firstValues, which only it used) and the zoom transition's element curves (zoomElementCurves, with mapCurve) have no caller left. They go with their test and mock entry, and the comments that named them say what they mean without them.
…erimental keeps the stable viewer (spec:AST-017 FR21) Without experimental.sourceFor, the element that opened the viewer was the opened item's source whenever it showed a picture, so callers who omit experimental got the zoom transition, the hidden opener and the held close on a touch screen. That fallback now lives inside experimental: without the object no item has a source and the viewer opens and closes plainly, and the opener record holds the shared press controller only then. The prop and component docs, the zoom transition changeset (now experimental) and the opener record's header say so; the zoom transition's plain-image proof and the listener test pass experimental where they rely on the fallback, or prove the stable opening without it. Red: without experimental a plain image opener still zoomed the photo out of it on a touch screen; green after.
…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.
… pooled, against a compositor-only control Photos' fade lasts 0.2 s, so one blocked window held only 4 to 6 frames of the interaction harness's control animation in WebKit, and 3 on a loaded machine: too few to judge. The controls now hide six times, blocked inside each fade and shown again between, and pooledBusyProof judges the Close button's spot over the runs pooled (each run's stall against that run's control): 7 to 8 control frames a run in WebKit, 47 pooled.
…er's zoom hides them (spec:AST-094 FR43) The stage's tap toggles the controls for every pointer, so a mouse's or a pen's click on the photo hides them once a double-click can no longer follow, the next shows them, and with them hidden a click beside the photo brings them back. A gesture's zoom hides them for every pointer, a double-click's, a ctrl-wheel's and a pinch's alike; the keyboard's zoom leaves them. A tap recognized after a page change belongs to the photo it landed on and toggles nothing on the next one. Under a cancelled drag the returning controls and the black follow the pointer that released it. The paging layout effect reads getAnimations only where Web Animations exist, as the motion system does. Tests: the chrome proofs (a mouse click hides and shows the controls and a double-click zooms in and hides them; a click followed at once by a page change leaves the next photo its controls), the gallery's click-beside proof, the fine-pointer double-click proof and the unit test of a mouse's and a pen's zoom flip to FR43's. Docs in English and Chinese; the every-pointer changeset gains the pointer's zoom hiding the controls, and the chrome changeset no longer says a mouse click leaves them. Red: under a mouse a click on the photo and a double-click's zoom left the controls, and a click beside the photo closed the gallery; green after (and without the tap guard a click followed at once by a page change hid the next photo's controls).
…8252 The dismissal now leaves through this PR's leave(), which takes whether a finger released it: the drag down passes its pointer and the pinch to close a finger's, as before. The release callback keeps this PR's pinch and meta#8215's pointer. The docs and the pinch-close changeset put this PR's close beside the drag down for every pointer and say the thumbnail comes from experimental, after meta#8231.
…er, as a pinch does (spec:AST-094 FR43) Photos' pinch to close takes any pinch, so a ctrl-wheel (or a trackpad's pinch) out from the fitted size fades the backdrop and the controls and closes the viewer, as a finger's pinch in does, on any primary pointer. Its release follows the pointer that began it: under reduced motion a finger's close and spring-back cross-fade (DR2) and a ctrl-wheel's complete at once on a fine primary pointer (DR3). Tests: LightboxEveryPointer's ctrl-wheel proof also closes the viewer by a ctrl-wheel out. Docs in English and Chinese; the every-pointer changeset gains the close, and the pinch-close changeset no longer limits it to a coarse primary pointer. Red: under a mouse a ctrl-wheel out from the fitted size left the viewer open; 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.
…ter) into the base of meta#8197
…very pointer (spec:AST-094 FR43) The track's style still said the stage's touches went to it under the touch layer only, the zoom stage's catch still spoke of a touch, and the gallery's navigation comment tied the pager's slide to a finger, where it follows the touch presentation. They now say what the code does. Comments only.
…nter (spec:AST-094 FR43) The zoom stage's header still described a one-finger pan the photo follows, its dismiss flag still said an image on a touch screen, the photo-without-zoom style still said the touch layer took its drags, and the dismissal geometry spoke of a finger. They now say what the code does: a finger's, a mouse's or a pen's drag, or a scroll, drags a photo down to close. Comments only.
…nto its thumbnail when it has one Since this PR a dismissed photo shrinks into its thumbnail when the stage's target gives one, but the header still said there was no thumbnail to return to yet. Comment only.
…pinch too (spec:AST-094 FR43) A ctrl-wheel pinches as fingers do, and this PR's controls hide as either passes the fitted size, but the callback's doc still spoke of fingers still down. Comment only.
…ch too (spec:AST-094 FR43) A ctrl-wheel out from the fitted size closes the viewer as a finger's pinch in does, but the zoom stage's header and the pinch's comments still said the photo follows the fingers, and that the release always closes into the thumbnail. They now say what the code does. Comments only.
There was a problem hiding this comment.
This pull request needs an engineering owner's decision before it can be approved: spec:AST-094 leaves each component's touch presentation to its own record, the draft component:Lightbox record does not define pinch to close (now also a mouse's or trackpad's ctrl-wheel out) or the stacked touch modes it builds on, and this head adds new normative text to architecture:layer-runtime (INV13). An engineering owner can provide or identify that governing spec and record it by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. That decision needs to settle whether AST-094 FR36–FR37 govern a pinch's release, the way back architecture:public-component-api INV16 asks for, the fine-pointer decision spec:AST-017 FR59 asks for, the specification decision experimental.sourceFor needs, and whether the INV13 text stands. Whatever that decision is, these need changes:
- Reduce the diff to the pinch-to-close change; today it carries about 60,700 authored lines from the open stack.
- Fix CI's touch-module bundle check (
scripts/touch-support-bundle.test.mjs), which now finds 16 gesture modules in the root entry. - Restore the backdrop blur when a pinch that dipped below the fitted size ends at or above it, and play the pinch-close springs through motion roles as AST-094 FR4 requires.
- Add real-device iOS Safari and Android Chrome evidence, fine-pointer before-and-after evidence for the ctrl-wheel close, and the Lightbox import size against this pull request's base.
[Automated review]
| pinchRef.current = null; | ||
| const shown = current(); | ||
| // (Reopened past the fitted size, it is a zoom like any other.) | ||
| if (session.closable && session.smallest < 1 && pinch.scale < 1) { |
There was a problem hiding this comment.
A closable pinch that dipped below the fitted size and ends at or above it skips this branch, so dismiss.onRelease never runs, yet the onPinch(1) call at line 498 has already switched the backdrop blur off through fadeForDismissal (Lightbox.tsx:1095-1096). Nothing restores it until the viewer reopens or another dismissal is released, and at this head a ctrl-wheel out and back in does the same for a mouse. Could the pinch restore the blur when it ends after fading, with a browser case for a lift at or above 1?
| * 3000 × 2000 photo opened from the Library grid, two-finger pinches | ||
| * synthesized in-process (60 Hz), CASpringAnimation on position and bounds.size at the lift: mass 1, damping ratio 1, stiffness 924 … 969 (response 0.202 … 0.207 s); opacities stiffness 947.29, damping 61.556, Oct 24 2026 | ||
| */ | ||
| closeSpring: {response: 0.204, dampingRatio: 1}, |
There was a problem hiding this comment.
This table never calls registerMotionTable (the navigation, scroll, springs and timing tables do), so motionRoleOf finds no role for closeSpring or returnSpring and a theme's motion knobs never reach them. Current spec:AST-094 FR4: "A component MUST read touch motion through these roles and MUST NOT play a literal"; DEC-3 says touch motion is themeable, against this file's "Internal constants" header.
| * it went below `closeScale` and was not reopened by `reopenScale` from its | ||
| * smallest scale. Its speed does not count. | ||
| */ | ||
| export function pinchCloses(smallest: number, released: number): boolean { |
There was a problem hiding this comment.
The rule reads no velocity, so fingers still opening at the lift close the viewer when the reopening is under 0.0502. Current spec:AST-094 FR37: "A release whose velocity points back toward the start MUST cancel, whatever the travel so far." Whether FR36–FR37 govern a pinch is part of the owner decision this pull request waits on.
| }, | ||
| // Photos' pinch to close fades them by the pinch's scale instead, | ||
| // under any pinch, a ctrl-wheel's included (spec:AST-094 FR43). | ||
| onPinch: scale => { |
There was a problem hiding this comment.
With no pointer gate, a mouse's ctrl-wheel reaches this pinch: the gesture host's span 100 × exp(−deltaY / 100) and the recognizer's 8 px hysteresis make one 100 px notch read as scale 0.40, so Ctrl plus one notch down, the browsers' page zoom-out, closes the viewer. spec:AST-017 FR59 asks for fine-pointer evidence before and after, and an owner decision under the component record, for that change.
| */ | ||
|
|
||
| import {useMemo, useRef, useState, type RefObject} from 'react'; | ||
| import {LongPressGestureRecognizer} from '../gestures/LongPressGestureRecognizer'; |
There was a problem hiding this comment.
These static imports bring 16 gesture modules (67,201 minified bytes) into the root entry's bundle, and CI's test-node (2/4) fails scripts/touch-support-bundle.test.mjs, "is absent from the root entry", which pins architecture:react-component-runtime INV13's bundle claims. The pull request needs to leave that check green.
Depends on #8242, and on #8665 and #8676 (gestures for every pointer), which this stack carries. Review only this PR's change: vjeux/astryx@b826a45...7ceff88
Why
In Photos, pinching a photo in from its fitted size shrinks it and closes the viewer back into its thumbnail. In Lightbox, the same pinch zooms the whole page out instead.
What led to it
The UIKit-parity work for Astryx (
spec:AST-094): the last of Photos' viewer gestures, after the tap (#8242).Options and tradeoffs
Why this choice
It behaves like Photos and keeps a way back for a pinch that went too far. The cost is 0.8 KB of gzip JavaScript (104,082 → 104,909 bytes).
How to check it
In the "Gallery with zoom" story, on a phone:
Evidence
Safari on the iOS 26.2 simulator:
The pinch close's busy proof now judges against a compositor-only control animation: 7 to 10 frames in both engines.