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 | +50.3KB |
+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
![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 until an engineering owner decides the direction: the tap-to-hide behavior and the other Lightbox touch features in this stack rest on the new draft spec:AST-094 (FR1–FR23, DEC-1 and DEC-2, including FR4's narrowing of the theme-tokens architecture) and on the draft Lightbox record, and any engineering owner, including you, can record that decision by commenting /approve-spec <full head SHA> on this pull request. Independently of that decision, please:
- land #8231 and the pull requests beneath it, then rebase onto main, so this diff is its own 941-line commit rather than 34,941 authored lines (the rebase also fixes
pr-a11y, which cannot find a spec file that main added); - restore the controls whenever the current item is not a zoom target, because in a gallery of photos and videos the controls hidden on a photo stay invisible and untappable on the video page, and a tap beside the video closes the viewer;
- attach the dialog's input listeners through a callback ref, because a Lightbox whose media arrive after it mounts never hides its controls;
- paint the black layer with a background colour role instead of
--color-on-light, which the theme-tokens architecture defines as a content colour.
[Automated review]
| // changes), so mirror them in the polite live region, including a hint that | ||
| // arrow keys pan while zoomed. A tap on the dark area around the photo | ||
| // closes, as a click on the backdrop does. | ||
| const isZoomTarget = hasZoom && !isVideo; |
There was a problem hiding this comment.
Controls hidden on a photo stay hidden when the gallery pages to a video: isZoomTarget turns false here, the stage that reports taps is disabled (line 820), and nothing clears data-chrome-hidden, so Close and the arrows stay invisible with pointer-events: none while a tap beside the video closes the viewer. Restoring the controls (restChrome() or setChrome({hidden: false, immersive: false})) whenever the current item is not a zoom target would fix it, with a mixed photo and video case in LightboxChrome.interaction.browser.spec.ts.
| dialog.removeEventListener('keydown', onKeyDown, true); | ||
| dialog.removeEventListener('focusin', onFocusIn); | ||
| }; | ||
| }, [setChrome]); |
There was a problem hiding this comment.
This effect reads dialogRef.current once and its only dependency, setChrome, is stable, so when Lightbox first renders without a current item (it returns null) the listeners never attach and a tap never hides the controls; with media set 300 ms after mount, every control stayed at opacity 1 after a tap on the photo. architecture:react-component-runtime INV6 asks the node owner to handle late attachment and replacement; a callback ref on the <dialog> would.
| position: 'absolute', | ||
| inset: 0, | ||
| zIndex: -1, | ||
| backgroundColor: colorVars['--color-on-light'], |
There was a problem hiding this comment.
--color-on-light is a content colour (architecture:theme-tokens INV10, shape 4: "content placed on a fill"), so this background follows any theme that retints content on light fills; a background role, or one the token owners admit for this surface, fits better.
6fae6a0 to
9db3e71
Compare
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides the direction: the tap-to-hide behavior and the other Lightbox touch features in this stack rest on the draft spec:AST-094 (FR1–FR23, DEC-1 and DEC-2, including FR4's narrowing of the theme-tokens architecture) and on the draft Lightbox record, and any engineering owner, including you, can record that decision by commenting /approve-spec <full head SHA> on this pull request, or say which part is approved. Independently of that decision, please:
- land #8231 and the pull requests beneath it, then rebase, so this diff is its own 941-line commit rather than 35,362 authored lines;
- restore the controls whenever the current item is not a zoom target, and keep the backdrop click from closing behind hidden controls, because in a gallery of photos and videos the controls hidden on a photo stay invisible and untappable on the video page and the next tap there closes the viewer;
- attach the dialog's input listeners through a callback ref, because a Lightbox whose media arrive after it mounts never hides its controls;
- paint the black layer with a background colour role instead of
--color-on-light, which the theme-tokens architecture defines as a content colour.
[Automated review]
| @@ -469,9 +624,24 @@ export function Lightbox({ | |||
| } | |||
| if (e.target === e.currentTarget || e.target === containerRef.current) { | |||
There was a problem hiding this comment.
This close ignores chromeHiddenRef, so it runs behind hidden controls wherever the stage does not take the tap: on a gallery's video page (the stage is off there) and, on a touchscreen whose primary pointer is fine, beside a single photo whose stage only hugs it (the stretch styles at lines 193-211 apply under pointer: coarse). Showing hidden controls here instead of closing, as onTapOutside does, would cover both.
There was a problem hiding this comment.
Fixed in 550d872 with the same change: with the controls hidden, the dialog's click shows them instead of closing.
9db3e71 to
1e440dc
Compare
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides the direction: the tap-to-hide behavior and the other Lightbox touch features in this stack rest on the draft spec:AST-094 (FR1–FR23, DEC-1 and DEC-2, including FR4's narrowing of the theme-tokens architecture) and on the draft Lightbox record, and any engineering owner, including you, can record that decision by commenting /approve-spec <full head SHA> on this pull request, or say which part is approved. Independently of that decision, please:
- land #8231 and the pull requests beneath it, then rebase, so this diff is its own 1,096-line commit rather than 35,517 authored lines;
- restore the controls whenever the current item is not a zoom target, and make the backdrop click show hidden controls instead of closing, because in a gallery of photos and videos the controls hidden on a photo stay invisible on the video page and the next tap there closes the viewer;
- attach the dialog's input listeners and the zoom stage's resize observer through callback refs, because a Lightbox whose media arrive after it mounts never hides its controls and a zoomed photo two or more pages into a gallery does not re-fit when the phone rotates;
- paint the black layer with a background colour role instead of
--color-on-light, which the theme-tokens architecture defines as a content colour.
[Automated review]
…control has focus)
1e440dc to
b1448e5
Compare
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides the direction: the tap-to-hide behavior and the other Lightbox touch features rest on the draft spec:AST-094 (FR1–FR23, DEC-1 and DEC-2) and the draft Lightbox record, and the theme motion roles and the new compact-touch query for presentation="adaptive" have no current spec; any engineering owner, including you, can record that decision by commenting /approve-spec <full head SHA> on this pull request, or say which part is approved. Independently of that decision, please:
- land #8231 and the pull requests beneath it, then rebase, so this diff is its own 1,102-line commit rather than 40,147 authored lines;
- restore the controls whenever the current item is not a zoom target, and make the backdrop click show hidden controls instead of closing, because in a gallery of photos and videos the controls hidden on a photo stay invisible on the video page and the next tap there closes the viewer;
- attach the dialog's input listeners and the zoom stage's resize observer through callback refs, because a Lightbox whose media arrive after it mounts never hides its controls and a zoomed photo two pages from the opened one does not re-fit when the phone rotates;
- reconcile the theme motion roles with FR4 and DEC-1, which reject per-theme touch motion, fix FR7's citation of a
spec:AST-090that is not in the repository, and drop theplatformadaptation condition and the module-level experimental subpaths or havespec:AST-012FR2 andspec:AST-017FR21 amended first; - paint the black layer with a background colour role instead of
--color-on-light, which the theme-tokens architecture defines as a content colour.
[Automated review]
| // changes), so mirror them in the polite live region, including a hint that | ||
| // arrow keys pan while zoomed. A tap on the dark area around the photo | ||
| // closes, as a click on the backdrop does. | ||
| const isZoomTarget = hasZoom && !isVideo; |
There was a problem hiding this comment.
On a video isZoomTarget is false, so the stage that reports taps is off (line 824), but nothing restores controls that a photo tap hid: after a swipe to a video, Close, the arrows, the counter and the caption stay invisible, and the dialog's click handler (lines 620-646) closes on the next tap beside the video. Restore the controls whenever the current item is not a zoom target, and have the click handler show hidden controls instead of closing, as onTapOutside does.
There was a problem hiding this comment.
Right, controls hidden on a photo stayed hidden on a video page, and a tap there closed the viewer behind them. Fixed in 550d872: an item that is not a zoom target brings the controls back, and the dialog's click shows hidden controls instead of closing. The new GalleryWithVideo story and LightboxChrome cover it.
| useEffect(() => { | ||
| const dialog = dialogRef.current; | ||
| if (dialog == null) { | ||
| return undefined; | ||
| } | ||
| const onPointerDown = (event: PointerEvent) => { | ||
| lastInputRef.current = event.pointerType; | ||
| }; | ||
| const onKeyDown = () => { | ||
| lastInputRef.current = 'keyboard'; | ||
| }; | ||
| const onFocusIn = (event: FocusEvent) => { | ||
| const target = event.target instanceof Element ? event.target : null; | ||
| if ( | ||
| chromeHiddenRef.current && | ||
| lastInputRef.current === 'keyboard' && | ||
| target?.closest('[data-lightbox-chrome]') != null | ||
| ) { | ||
| setChrome({hidden: false, immersive: immersiveRef.current}); | ||
| } | ||
| }; | ||
| dialog.addEventListener('pointerdown', onPointerDown, true); | ||
| dialog.addEventListener('keydown', onKeyDown, true); | ||
| dialog.addEventListener('focusin', onFocusIn); | ||
| return () => { | ||
| dialog.removeEventListener('pointerdown', onPointerDown, true); | ||
| dialog.removeEventListener('keydown', onKeyDown, true); | ||
| dialog.removeEventListener('focusin', onFocusIn); | ||
| }; | ||
| }, [setChrome]); |
There was a problem hiding this comment.
This effect reads dialogRef.current once and depends only on the stable setChrome. When Lightbox first renders with no current item it returns null, so the dialog that mounts later never gets these listeners, lastInputRef stays 'mouse', and a touch tap never hides the controls. A callback ref would attach and detach with the node (architecture:react-component-runtime INV6).
There was a problem hiding this comment.
Right, a dialog that mounted after the first render never got the listeners. Fixed in 550d872: they attach through a callback ref on the dialog. The new MediaArrivesLater story and LightboxChrome cover it.
| useEffect(() => { | ||
| const stage = options.stageRef.current; | ||
| if (stage == null || typeof ResizeObserver === 'undefined') { | ||
| return undefined; | ||
| } | ||
| const observer = new ResizeObserver(() => { | ||
| const before = geometryRef.current; | ||
| if (before == null || !zoomedRef.current) { | ||
| return; | ||
| } | ||
| const after = measure(); | ||
| if (after == null) { | ||
| return; | ||
| } | ||
| animator.stop(); | ||
| const next = transformForResize(before, after, targetRef.current); | ||
| write(next); | ||
| setZoomed(isZoomed(after, next)); | ||
| }); | ||
| observer.observe(stage); | ||
| return () => observer.disconnect(); | ||
| // The stage element is stable for the dialog's lifetime. | ||
| }, [animator, measure, options.stageRef, setZoomed, write]); |
There was a problem hiding this comment.
The observer watches the first stage element and never re-attaches, but a gallery renders only the current page and its neighbours and moves stageRef to the current page. Two pages on, the observed node is gone and a zoomed photo no longer re-fits when the phone rotates. Observe through a callback ref instead.
| position: 'absolute', | ||
| inset: 0, | ||
| zIndex: -1, | ||
| backgroundColor: colorVars['--color-on-light'], |
There was a problem hiding this comment.
--color-on-light is a content colour (architecture:theme-tokens INV10, shape 4: "content placed on a fill"), so the viewer's background follows a content role. A background role, or one the token owners admit for this surface, fits better.
| * 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 closes the when vocabulary to width, pointer, contrast and motion and says "Future environment fields require explicit additions to this closed vocabulary"; this adds platform without amending AST-012.
| "./motion/experimental": { | ||
| "source": "./src/motion/experimental.ts", | ||
| "types": "./dist/motion/experimental.d.ts", | ||
| "default": "./dist/motion/experimental.js" | ||
| }, | ||
| "./gestures/experimental": { | ||
| "source": "./src/gestures/experimental.ts", | ||
| "types": "./dist/gestures/experimental.d.ts", | ||
| "default": "./dist/gestures/experimental.js" | ||
| }, |
There was a problem hiding this comment.
spec:AST-017 FR21 puts experimental hooks, functions and types on "the owning component's /experimental import subpath". These module subpaths have no owning component, so under FR21 they would not be outside the stable promise the changeset describes.
| * ``` | ||
| */ | ||
| motion?: MotionScaleConfig; | ||
| motion?: MotionConfig; |
There was a problem hiding this comment.
Retuning touch springs, durations and curves per theme is what the draft AST-094 FR4 forbids ("A touch presentation MUST read its motion from the table on every theme") and DEC-1 rejects; the record or this field has to change before an owner can approve both.
| 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 at this head or on main, so this departure, and the matching current-state impact line, points at a record no owner can rule on.
|
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.
|
b1448e5 to
550d872
Compare
|
Pushed in 550d872: the controls' fades take the animator's default. |
…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.
… 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).
… 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).
…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.
There was a problem hiding this comment.
Approval is on hold until an engineering owner decides the direction: component:Lightbox is still draft and spec:AST-094 leaves each component's touch presentation to its own record. So the tap that hides the controls, the gestures that now reach mouse and trackpad users (spec:AST-017 FR59), experimental.sourceFor (FR21) and the new architecture:layer-runtime INV13 all need that decision. Any engineering owner, including you, can record it by commenting /approve-spec <full head SHA> on this pull request, or say which part is approved. Independently of that decision, please:
- land #8231 and the pull requests beneath it, then rebase, so this diff is its own change rather than 59,670 authored lines, and move the exit copy with INV13, the press and menu fixes and the build fix to their own pull requests;
- fix the failing root-entry check in
scripts/touch-support-bundle.test.mjs, which now finds the gesture modules in Core's root entry; - play the chrome fade, the black's turn and the dismiss and transition springs through motion roles (
spec:AST-094FR4); - size the gallery's caption row without hidden caption elements, and report element counts before and after (
architecture:component-style-authoringINV7); - add fine-pointer before-and-after evidence for each mouse-visible change, and real-device iOS Safari and Android Chrome recordings.
[Automated review]
| * The chrome's opacity as it hides or shows, from where it is: Photos' | ||
| * 0.2 s easeInEaseOut fade. | ||
| */ | ||
| export function chromeFadeCurve(from: number, isHidden: boolean): MotionCurve { |
There was a problem hiding this comment.
chromeFadeCurve and chromeBackgroundCurve play zoomMedia.chrome's 0.2 s and 0.1 s literals directly, so a theme's tempo never reaches them. spec:AST-094 FR4 asks every spring, duration and curve a touch presentation plays to be a motion role named for its use, with a component's own roles joining through the generated motion role table (DEC-5). The dismiss and transition springs in zoomMedia.ts need the same.
| <div | ||
| aria-hidden | ||
| inert | ||
| {...stylex.props( | ||
| styles.captionStacked, | ||
| styles.captionSizer, | ||
| )}> | ||
| {unrenderedCaptions.map(j => ( | ||
| <div | ||
| key={j} | ||
| {...stylex.props( | ||
| styles.caption, | ||
| styles.captionStacked, | ||
| )}> | ||
| {mediaArray[j].caption} | ||
| </div> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
This sizer mounts a hidden, inert element for every caption no rendered page shows, only to reserve the caption row's height: 97 extra elements on an interior page of a 100-photo captioned gallery. architecture:component-style-authoring INV7 rules out a hidden twin to reserve space and asks for measured element counts before and after any added element. Size the row on the element that shows the caption instead.
| import {LongPressGestureRecognizer} from '../gestures/LongPressGestureRecognizer'; | ||
| import {PanGestureRecognizer} from '../gestures/PanGestureRecognizer'; | ||
| import {PinchGestureRecognizer} from '../gestures/PinchGestureRecognizer'; | ||
| import {TapGestureRecognizer} from '../gestures/TapGestureRecognizer'; | ||
| import type {Point} from '../gestures/types'; | ||
| import {useGestureRecognizer} from '../gestures/useGestureRecognizer'; |
There was a problem hiding this comment.
Through these imports, and usePager.ts's, 16 gesture modules now reach Core's root entry (index.ts re-exports Lightbox). CI's scripts/touch-support-bundle.test.mjs root-entry check, which pins architecture:react-component-runtime INV13, fails as a result. Either keep the gesture code out of the root entry's bundle, or update that test to the bundle INV13 allows once a root-exported component imports the modules.
| correct, and the runtime polls nothing: an anchor that a scrolled | ||
| container hides is followed again when an intersection observer reports it | ||
| back in view. | ||
| - **INV13 — One exit copy per closing layer.** A layer's close plays on at most |
There was a problem hiding this comment.
INV13 is a new normative invariant on a current record, so an engineering owner needs to approve it. Its only caller is createLiftFromTrigger, which no component uses yet. It would review more cleanly in its own pull request, landing with its first adopter.
Depends on #8231, and on #8665 and #8676 (gestures for every pointer), which this stack carries. Review only this PR's change: vjeux/astryx@b428e07...b826a45
Why
In Photos, a tap on the photo hides the buttons and caption and turns the background black, so the photo has the screen to itself; the next tap brings them back. On a phone, Lightbox's controls always cover part of the photo.
What led to it
The UIKit-parity work for Astryx (
spec:AST-094), after zoom, paging, dismissal and the zoom transition (#8148 to #8231).Options and tradeoffs
--color-on-light(rgb 17, 17, 17); owners may prefer a dedicated token.Why this choice
It matches Photos and keeps the photo still. A double tap still zooms, because the tap waits out the double-tap interval, and a keyboard focus on a control brings the controls back. The cost is 1.3 KB of gzip JavaScript (102,742 → 104,082 bytes) and 0.1 KB of CSS.
How to check it
In the "Gallery with zoom" story, on a phone:
Evidence
Safari on the iOS 26.2 simulator:
Android 15 emulator, Chromium:
The fade's busy proof judged one blocked fade, and Photos' 0.2 s fade left a window too short to judge in WebKit: 4 to 6 frames of a compositor-only control animation, 3 on a busy machine. It now blocks inside six fades and judges them pooled: 47 control frames, 7 to 8 a run.