Skip to content

feat(Lightbox): swipe between gallery photos like Photos - #8197

Open
vjeux wants to merge 13 commits into
meta:mainfrom
vjeux:uikit/c24-pager
Open

vjeux wants to merge 13 commits into
meta:mainfrom
vjeux:uikit/c24-pager

Conversation

@vjeux

@vjeux vjeux commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Batch: B7 Lightbox
Stack: #8197 → #9810
Stack: #8197 → #8215

On main 1c142dc. This head adds three commits to the previous head 22da53d: the release-note fix vjeux@32e21eb (CodeBunny's 16:52Z item; see Evidence), a merge of main, and a frame floor for the instant-page check, vjeux@3ee5207 (see Evidence). The merge's one conflict was the comment above expectSpringZoom in LightboxFinePointer.a11y.browser.spec.ts: #10301 moved that check onto the shared interaction harness, and the merge keeps #10301's check and comment as they are on main. This pull request's own diff is otherwise unchanged. The two fixes from the round before stay: vjeux@91d4a1e and vjeux@22da53d

The previous head added the zoom pan's begin threshold on the merge worker's 0fc7062 (see Evidence): vjeux@b8266e6

Until card 46 decides the shared caption row, each page lays out its own photo and caption as main does; a neighbour with a longer caption can arrive with a slightly smaller photo.

Drag paging is part of the touch presentation, which a coarse primary pointer brings (spec:AST-094 FR13), as the spec owners ruled for drag to close (#8105). Under a fine pointer the Previous and Next buttons and the arrow keys page, and a mouse's drag or a trackpad's sideways scroll leaves a gallery on its photo, as on main. FR11's text is unchanged; the record's Current-state list keeps naming FR7's drag paging and FR8's drag to close as not built for a mouse, a pen or a trackpad. This changes this pull request's own product lines after its stamp at d585c45, so it needs the spec owners' re-check.

With zoom, a page's caption moves up under its photo. A click or tap in the strip it leaves at the page's bottom edge closes the viewer, as on main: the stage's tap, which sees the gallery's whole track, now closes for any tap off the photo except on the caption.

Rebased on #8148's 14f14d7 (#8147 e463256 on main f10bc91), with no change to this pull request's own lines. Against 2b36122, the head the spec owners stamped:

  • A mouse's or a pen's click in a gallery closes only outside main's media group, by the rule feat(Lightbox): touch zoom like the iOS Photos viewer #8148 now follows. That group runs from the photo's top to its caption's bottom. It is as wide as the photo's natural width or its caption, whichever is wider, up to the viewer. So a click beside a large photo, level with it, leaves the viewer open, as on main, and a click past a narrow photo's caption closes it. A finger keeps the union box below, and the stage's backdrop tap takes only a finger.
  • Every press in the viewer records its pointer and point at the viewer's container, as feat(Lightbox): touch zoom like the iOS Photos viewer #8148 now does, so the stage's own handler goes.
  • Main's test(browser): keep every Playwright browser context off remote hosts #9811 makes @playwright/test a lint error in browser specs, so the Paging and PagingModes specs import test and expect from @astryxdesign/a11y-spec/playwright, as main's do.
  • While zoomed, the photo's shown box counts as media for a mouse's click, as feat(Lightbox): touch zoom like the iOS Photos viewer #8148 now rules, so a click on a zoomed page photo never closes the viewer.
  • The record's Current-state list: the paging motion roles line now names the page settle only, by the spec owners' ruling, and the press tracker line names where presses are recorded now, the viewer's pointerdown handler and the zoom stage's capture listener for the start threshold.
  • Two tests follow feat(Lightbox): swipe between gallery photos like Photos #8197's paging: a finger's drag on a fitted photo starts no zoom pan here, so the strip's first tap after it closes the viewer, as at 2b36122; and a gallery page's caption is found on its own page.

Rebased on #8148's 33e0568 (main 5a373df). Against d585c45, the head the spec owners stamped:

  • Drag paging is the touch presentation's (above): the pager's pan is on only under a coarse primary pointer.
  • useZoomStage reads its options directly in the tap callback, as feat(Lightbox): touch zoom like the iOS Photos viewer #8148 now does, since useGestureRecognizer calls the newest callback.
  • usePager keeps a refused drag's spring-back with isControlledChangeAnimated off. Lightbox no longer reaches that path (its pan is touch-only, where the option is on), so a hook test pins it: with the option off, a drag the owner refuses springs back animated, and a new controlled index moves at once. Its red-first is a NOT-FOR-LANDING arm (9dd2b84) where a refused page follows the option.
  • Without zoom, a mouse's click on the stage closes the viewer as a click on main's backdrop does. It closes when its press or its release is off the current media group: the photo or video and its caption, as one box. So a press on the dark area released on the photo closes, and a click beside a photo narrower than its caption, within the caption's width, leaves the viewer open. How far the press moved or how long it was held does not matter. The stage's tap recognizer (a 45 px movement limit and a 1.5 s hold limit) takes only a finger, and tests the same box. A keyboard's click on a caption's link stays the link's.
  • The props doc and the changeset say that a swipe pages on a phone or tablet, not under every pointer.
  • feat(Lightbox): touch zoom like the iOS Photos viewer #8148's every-pointer changeset says the ctrl-wheel and trackpad gestures come with hasZoom, since a Lightbox without it leaves them to the page.
  • Two comments that restated the code go, and two long sentences are split.

Against 39ad02a, where the iPhone check passed, this head answers the reviews of that head and of 5a34859:

  • A mouse's or the keyboard's Next shows the next photo at once again, as before this pull request; only a finger's swipe slides. The shared usePager hook gains isControlledChangeAnimated (default true, so Carousel is unchanged), and Lightbox passes isControlledChangeAnimated: isTouch.
  • A drag the owner refuses springs back under every pointer (FR7). The return after a page the pager reported and the owner refused animates whatever isControlledChangeAnimated says; only a new index follows it.
  • The press tracker goes through the shared recognizers (FR14). The video controls band reads where the pan's first touch began, and the caption pinch reads the pinch's first touch. Without zoom, a tap beside a gallery's photo closes the viewer through a shared tap recognizer, which fails past the tap's movement limit. The stage's own pointerdown listener now only records the pointer type the browser's dblclick does not carry.
  • A pinch that starts on a caption zooms the page, not the photo. The photo row takes its own touches, the track leaves only the page's pinch-zoom, and the zoom stage's pinch does not begin when its first finger lands on a caption. A one-finger swipe from the caption still pages.
  • A video that ends during a drag on its frame leaves the drag to page; a guard test holds it.
  • The record names two more Current-state gaps (below), and an unused usePager call and a stale comment go.

Batch: Lightbox · paging. One intent, gallery paging across pointers; most of its authored lines are tests (about 1,550 of 2,350).

Why

On a phone, a Lightbox gallery moves only through two small arrow buttons. People expect to swipe between photos as in Photos, with a zoomed photo panning to its edge before the next one comes.

What led to it

The UIKit-parity work for Astryx (spec:AST-094): after the zoom (#8148), paging, on the shared pager main already has. component:Lightbox FR7 (current) decides it.

Options and tradeoffs

  • CSS scroll snapping: native and small, but it cannot share a gesture with the pinch (a drifting pinch pages) or apply UIKit's release rule.
  • A pager on the gesture host (this pull request): the row follows the finger, the release picks the page by UIKit's rule (one page at most) and settles on the compositor, and zoom and paging share the drag as nested scroll views do. The cost is JavaScript; the size line is under Evidence.

Why this choice

Only the pager keeps a pinch from paging and lets a zoomed photo keep the drag until its edge, as Photos does. Pages sit 40 pt apart, Photos' measured gap.

  • A video pages from its frame. Its native controls are taken as a 48 px band at its bottom edge, since browsers expose no control geometry, so a drag that starts there stays the video's: a scrubber or a volume drag does not page. A browser that places its controls elsewhere pages from them; the record says so.

Record change

component:Lightbox's Current-state impact list gains a note on FR7, in the spec owner's words: "Video controls (FR7). A video's native controls are taken as a 48 px band at its bottom edge, since browsers expose no control geometry; a browser that places controls elsewhere pages from them." FR7's text is unchanged. It also gains two lines: "Paging motion roles (FR5). The page settle and the buttons' slide play the pager's own UIKit values, which the motion role table does not list as roles." and "Press tracker (FR14). The stage's own pointerdown listener records each press's pointer type, which the browser's dblclick does not carry, and clears the flag a drag leaves for its click." The list also drops what this change closes: swipe paging from the touch gaps, and paging from the fine-pointer gap ("A mouse drag on a fitted photo does not close the viewer" remains). The record needs a spec owner's approval.

FR11's text is unchanged. Its Current-state gap line stays as #8148 has it: "Fine-pointer gestures (FR11). FR11's drag paging (FR7) and drag to close (FR8) are not built for a mouse, a pen or a trackpad: a drag on a fitted photo does nothing."

Writing rubric results

Run over docs/specs/components/Lightbox/Lightbox.spec.md:

  • R1 narration: none; the Current-state lines state what the shipped component does.
  • R2 research: none in the record.
  • R3 evidence: none.
  • R4 rejected alternatives: one, in DEC-2, one line (unchanged).
  • R5 change references: none.
  • R6 open questions: none open.

How to check it

In the "Gallery with zoom" story, on a phone:

  1. Swipe left: the next photo slides in, and the counter reads 2 / 4.

  2. Drag a little and let go: the photo springs back.

  3. Zoom in, drag to the photo's edge, then drag again: the next photo comes.

  4. Pinch with a sideways drift: the photo zooms; nothing pages.

  5. In "Gallery with video", page to the video. Drag its scrubber: the video seeks and the gallery stays. Drag on the video's frame: the gallery pages.

  6. Pinch on a photo's caption: the page zooms and the photo stays as it is. Swipe from the caption: the gallery pages.

With a mouse, a sideways drag leaves the gallery on its photo, as on main, and so does a trackpad's sideways scroll. A click on Next shows the next photo at once. With Reduce Motion on, a swipe fades instead of sliding.

Evidence

  • The release note names the snap paging Carousel (32e21eb), CodeBunny's 16:52Z item: a plain hasSnap keeps native CSS scroll snap, and only hasSnap={useSnapPaging()} runs the pager whose settle now follows tempo (Carousel.tsx's hasSnap docs), so the changeset names that value. The same commit drops tempo from the pager gap in spec:AST-094's FR45 verification row, which the tempo fix made stale.

  • The instant-page check judges at least 12 frames (3ee5207): the check that Next and the arrow key show the next photo at once sampled the track for 600 ms and needed more than 10 frames, and WebKit in a 4-CPU container painted 3 to 6 under load. It now samples for 600 ms and at least 12 frames, gives up at half the test's timeout with "too few frames to judge", and keeps the one-place check as it was.

  • A released page settles at the theme's tempo (22da53d): the spec owners ruled this a consistency fix: spec:AST-108 plays motion at the theme's tempo, and spec:AST-094 requires the touch presentations to follow it, but the pager's settle after a release ignored it. solvePaging now takes the tempo. Each step lasts tempo times as long and carries tempo times the release's distance, so a settle from rest keeps its shape and takes tempo times as long, and a release still starts at 96 % of the finger's speed. At tempo 1 the settle is unchanged. No motion role names the paging table, so usePager reads the tempo of the theme the track is in. Every pager follows it: a Lightbox gallery, and a Carousel given hasSnap={useSnapPaging()} (on main), which the new changeset names; a plain hasSnap is native CSS scroll snap and does not change. goTo's animated scroll stays spec:AST-094's separate gap.

    • A hard release overshoots a little more at a slower tempo, as a spring does: a 3,000 px/s release overshoots its page by 3.2 px at the default tempo and by 17.8 px at the slowest tempo a theme allows (1.5). Only the test's tempo 3 gives 104.6 px.
    • Red first: the paging.test.ts case for a settle's shape and length at tempo 0.75, 1.5 and 3 fails on the previous head's code, and so does usePager.test.ts "a release's settle plays at the theme's tempo 3". The two other new paging.test.ts cases are guards that pass on the old code: a release's first speed at any tempo (a first speed left unscaled fails it) and tempo 1 unchanged.
    • The busy-main-thread proof of the gallery's settle plays at tempo 3, as the zoom's does. The slowed settle runs first, on a fresh track, and the page starts a 600 ms block on it; then the same swipe runs at tempo 1. The slowed settle is three times as long (2.9 to 3.1), still runs when the block ends, and keeps pace with the compositor's control animation through the block. A frozen settle and a settle that the main thread drives both fail the proof. On the previous head's build the proof fails 5/5 in WebKit and 3/3 in Chromium, on the length ratio (1.00).
    • The proof's floor: under the CI profile (4 CPUs) the proof passes 30/30 in WebKit. With four more busy loops inside those four CPUs, the compositor presents a frame about every 200 ms, and the proof fails 10/10 on its own floor ("the control moved on 2 frames in the judged part, under 4"). At tempo 3 the settle's tail moves less than a screencast step per frame after about 550 ms, which leaves too few frames to judge there. This is the harness's documented starved-compositor limit.
    • "A drag the owner refuses springs back over frames" now reads the spring-back from its own keyframes instead of counting main-thread frames, which a loaded runner thins out. It failed 3/10 under the WebKit CI profile on the previous head; it passes 10/10 on both builds now.
  • A single zoomed photo gives past its edge again (91d4a1e), CodeBunny's 10:27Z item: the zoom stage's pan refused a finger's drag headed past a zoomed photo's edge in every Lightbox, so a single photo resting at its edge no longer rubber-banded there. The stage now yields at the edge only when Lightbox says an outer pager is there (hasOuterPager: a gallery under a finger). The new case "a single zoomed photo's drag outward from the edge it rests at rubber-bands and returns" passes on main and at this head, and fails on the previous head, 3 of 3 in Chromium and in WebKit. The record's press-tracker line no longer names the stage's own pointerdown listener.

  • The zoom pan's begin threshold (b8266e6): the zoom stage's pan takes its begin threshold per press from the shared PanGestureRecognizer's hysteresis function, and Lightbox's own capture pointerdown listener on the stage goes. A mouse's or a pen's drag on a zoomed photo still pans from its first move, and a finger keeps UIKit's threshold. New unit case: "takes the zoom pan's begin threshold from the shared pan, with no pointerdown listener of its own on the stage".

    • Pinch and pan start, against 0fc7062, same frames:
      • Chromium with touch emulation, 2 runs × 5 per head: the pinch starts at 9 px of spread and 150 ms on both heads; the pan on a 2× photo starts at 16 px of travel and 266.8 ms, with a first move of 6 px, on both heads.
      • Safari on the iOS 26.2 simulator, 4 takes per head, alternating: a two-finger pinch scales the photo on its first two-finger move (one simulator step, 20–24 pt of spread) on both heads, and the pan after it starts at the first move past 10 pt (11 pt in 1 pt steps) on both heads.
    • A fine-pointer test's comment splits one 26-word sentence in two (architecture:knowledge-contracts INV29). The same sentence in feat(Lightbox): touch zoom like the iOS Photos viewer #8148's LightboxZoom spec gets its split when this branch is restacked on feat(Lightbox): touch zoom like the iOS Photos viewer #8148's current head.
    • At b8266e6: Lightbox units 129/129; core tsc 0; strict ESLint 0 on the three files.
  • Red-first: "a drag that starts on a video's controls stays the video's; a drag on its frame pages" fails without the band: the drag on the controls pages the gallery. It passes in Chromium and WebKit.

  • At this head: New cases pin the mouse path through the pager's option: "with a mouse the Next button shows the next photo at once" and "with a mouse a Next the owner refuses never moves the photo", each sampling the row's offset every frame. "a pinch on a zoomable gallery's caption still zooms the page" (Chromium; WebKit's driver drops the first finger) fails without the zoom stage's caption check: the photo's scale read 1.72 beside the page's 1.72. "a video that ends during a drag on its frame leaves the drag to page" guards the drag through the video's ended.

  • A refused drag springs back: "with a mouse a drag the owner refuses springs back" samples the track every frame from the release. It fails with this head's usePager reverted: the photo jumps from past half a page to rest in one frame, as CodeBunny measured (−877 px to 0).

  • Browsers: Chromium runs the Lightbox and Carousel a11y specs 30/30 and the Lightbox and usePager interaction specs 84/84; WebKit runs the same interaction specs 78/78. Each engine skips the cases its driver cannot send (4 in Chromium, 10 in WebKit), and the other engine runs them. The video band runs in both. Without the band, WebKit's drag on a video's controls pages the gallery; Chromium's own control panel already keeps that drag.

  • Size (INV6): Lightbox loads its touch code with it, as Toast and Slider do, by the spec owners' decision that touch code stays eager. The Lightbox import, bundled with every module it reaches (React external) and gzipped at the default level, grows from 51,394 bytes on main (dc90b50) to 99,585 at this head, the zoom and paging beneath included. Most of it is the shared touch modules a page loads once: minified, the gestures grow by 77.6 KB, motion by 41.3 KB and the shared hooks by 10.1 KB, while Lightbox's own code grows by 14.1 KB. The package stylesheet grows by 45 bytes gzip (40,267 → 40,312). No dependency changes.

  • Elements (INV7): an open 4-photo "Gallery" at its first photo renders 28 elements on main and 34 at this head. The added six: the paging track; the current page's box and its media box (the zoom stage); and the neighbouring page the drag reveals (its box, media box and photo). Each page keeps its one caption, as main keeps its one.

  • Fine pointer, before and after (FR59): desktop Chromium, a mouse drag from x 900 to 300 across "Gallery" leaves it on 1 / 4 on main and on this head; a trackpad's sideways scroll leaves it there too (WebKit). The strips this item showed before were of the earlier, every-pointer paging.

    main: main: a mouse drag moves nothing

    this pull request: the row follows the mouse and pages

  • Fine pointer, the looks the range carries from feat(Lightbox): touch zoom like the iOS Photos viewer #8148 (FR59): desktop Chromium, "With zoom" over a light photo.

    • The control backing: on main the photo never lies under Close, so it has none; here a double-click zooms the photo under Close, which sits on its dark backing.

      main: main: Close over the backdrop, no backing this pull request: Close on its backing over the zoomed photo

    • The zoom target's focus ring, from the keyboard: on main it traces the photo's box; here the stage fills the viewer, so the ring traces the viewer's edge, inside it.

      main: main: the ring around the photo box this pull request: the ring along the viewer's edge

  • Fine pointer, the zoom settle the range carries from feat(Lightbox): touch zoom like the iOS Photos viewer #8148 (FR59): with hasZoom, Enter on the zoom target and a mouse double-click zoom on the shared springs.settle that feat(Lightbox): touch zoom like the iOS Photos viewer #8148 gives every zoom, where main plays a 200 ms ease-out. Measured at each exact head in desktop Chromium (1280 × 800, mouse and keyboard, no touch; default theme, tempo 1; "With zoom" with a 3000 × 2000 photo, 1× to 2×), by pausing the running animation and sampling it every 2 ms:

    Head 50 % 90 % 99 % Within 0.1 %
    main (1dce318), Enter or double-click 70 ms 150 ms 186 ms 196 ms (a 200 ms ease-out)
    this head (0fc7062), Enter or double-click 134 ms 310 ms 530 ms 736 ms

    Enter and the double-click measure the same at each head, two runs each.

Safari on the iOS 26.2 simulator, from the previous head (the paging is unchanged here), a swipe left:

main this pull request, with the main thread blocked 600 ms after the lift
main: nothing moves the next photo settles through the block

Android 15 emulator, Chromium: two swipes

The page settle's busy proof tracks the next photo as it slides in, against a compositor-only control animation, and passes in both engines.

Testing

At 3ee5207 on main 1c142dc: pnpm install --frozen-lockfile exits 0; the root build and the Storybook build pass; core tsc is clean; the Lightbox, usePager and paging unit files pass 283/283. The spring-zoom checks ("the keyboard’s zoom settles on springs.settle" and "a mouse double-click settles on springs.settle too") pass 30/30 in Chromium on 2 cores and 20/20 in WebKit in a 4-CPU container (the CI profile). The instant-page check passes 20/20 in each; its old sampler failed 10/10 in that WebKit container, and a track that shows one intermediate place fails it in both engines. The whole LightboxFinePointer a11y file passes 21/21 in Chromium and 19 (2 skips) in WebKit.

At 22da53d on main bef866e:

  • pnpm install --frozen-lockfile exits 0; core tsc is clean; the motion, hooks, Lightbox, SnapPaging, Carousel and gestures unit files pass 2,956/2,956; pnpm check:knowledge and pnpm check:changesets pass; comment sentence lengths 0 over.
  • Main's only change under these files since 2151c88 is two lines of the spec:AST-094 record, so the browser runs on the same two fix commits on 2151c88 (dc015a2) stand: the Lightbox, usePager, SnapPaging and Carousel interaction specs pass 118 in WebKit in a 4-CPU container (the CI profile) and 118 in Chromium (10 skips each); the Lightbox and Carousel a11y specs pass 42/42 in Chromium.
  • The tempo-3 busy proof of a page's settle and its two controls pass 30/30 in the CI profile and 9/9 in Chromium; on the previous head's build the proof fails 5/5 and 3/3, on the settle's length (ratio 1.00).
  • The proof's floor: with four more busy loops inside the CI profile's four CPUs, the proof fails 10/10 on its own rule ("the control moved on 2 frames in the judged part, under 4"), as the zoom's busy proof is documented to at that load. It is the harness's starved-compositor limit, not a stall, and the proof's comment states it.
  • "A drag the owner refuses springs back over frames" passes 10/10 in the CI profile on both builds (3/10 on the previous head before its fix).
  • "A single zoomed photo's drag outward from the edge it rests at rubber-bands and returns" passes on main and at this head, and fails 3/3 in Chromium and in the CI profile on the previous head's build.

Earlier, at dc3a360:

At dc3a360, this pull request's own lines are byte-identical to 878f6b8's (the head below moved). At 878f6b8, product code and tests are byte-identical to 7f5fcc1 and 42cb8bf apart from the record and two test files, and these ran on them:

At dc3a360 on box 6: pnpm install --frozen-lockfile 0; core tsc 0; strict ESLint on every changed file 0; CI=true pnpm lint 0 errors; the Lightbox, motion, gestures and hooks unit files 2,650/2,650; pnpm check:knowledge and pnpm check:changesets pass; a root build and a Storybook build of this head, then Chromium Lightbox a11y specs 24/24, Chromium Lightbox interaction specs 67/67 (3 WebKit-only skips), WebKit 63/63 (7 Chromium-only skips).

  • pnpm install --frozen-lockfile exits 0; core tsc is clean, and so is strict ESLint on every changed file; CI=true pnpm lint shows no error; INV29 finds no long sentence; pnpm check:knowledge passes;
  • the Lightbox, usePager, motion/uikit and gestures unit files pass 1302/1302;
  • a root build and a Storybook build of 42cb8bf, then: Chromium Lightbox a11y specs 24/24 (with this head's spec files); Chromium Lightbox and hook interaction specs 89/89 (5 WebKit-only skips); WebKit 84/84 (10 Chromium-only skips);
  • red first on 6a6505e's build: "in a gallery, a mouse click beside a large photo, level with it, leaves the viewer open, as on main" and "without zoom, a mouse click at the far edge beside a large gallery photo leaves the viewer open, as on main"; and the unit "stays open on a mouse click beside a large photo, level with it, since its media spans the viewer" with 2b36122's Lightbox.tsx.

Earlier, at 2b36122 on #8148's 33e0568 (main 5a373df): #9810's gate at 86d0dd1, which holds this tree plus #9810, passed: Chromium Lightbox a11y specs 17/17; Chromium Lightbox interaction specs 67/67 (3 WebKit-only skips); WebKit 63/63 (7 Chromium-only skips); the Lightbox and usePager unit files 254/254. The checks below ran at 99cfba1, this change before #8148's trackpad gate, a changeset line, a reflowed comment and two test titles.

  • pnpm install --frozen-lockfile exits 0; core tsc is clean, and so is strict ESLint on every changed file; pnpm lint shows the same 171 warnings as the previous head and no error; INV29 finds no long sentence;
  • the Lightbox, usePager, motion/uikit and gestures unit files pass 1290/1290; pnpm check:knowledge passes;
  • a root build and a Storybook build of this tree, then: Chromium Lightbox a11y specs 17/17; Chromium Lightbox interaction specs 65/65 (2 WebKit-only skips); WebKit 60/60 (7 Chromium-only skips);
  • red first on d585c45's build: "a mouse drag sideways leaves a gallery on its photo, as on main" (Chromium and WebKit), "a trackpad's horizontal scroll leaves a gallery on its photo" (WebKit), and "a gallery leaves its sideways touches to the page where the primary pointer is fine" (its viewport claimed pan-y pinch-zoom);
  • red first on c358801's build: "without zoom, a mouse press beside the photo that moves 60 px before its release / is held 1.6 s before its release closes the gallery, as on main"; on a NOT-FOR-LANDING arm without the click's detail check (5641fb2), the unit "leaves the viewer open when the keyboard activates a caption link after a mouse press";
  • red first with the previous head's stage click (48f02af): on a NOT-FOR-LANDING arm with its Lightbox.tsx (14548d2), the units "closes on a mouse press beside the photo that is released on it" and "stays open on a mouse click beside a narrow photo, within its caption's width"; on a build whose stage click code is that head's, the browser cases "…a mouse press beside the photo that is released on it closes the gallery, as on main" and "…a mouse click beside a photo narrower than its caption leaves the gallery open, as on main";
  • on main's Storybook and on this build alike, in the Gallery story: a press beside the photo released on it closes the viewer; a click beside a 128 × 768 photo, within its 273 px caption's width, leaves it open; a press on the photo released beside it closes it;
  • lint, typecheck and the full unit suites run before this leaves draft.

Before merge, the human check on a phone tries a scrubber drag on iOS Safari and Android Chrome: it stays with the video, and a drag on the frame pages. If either fails, the video clause becomes a named gap and this ships photo paging only.

@vercel

vercel Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
astryx Ready Ready Preview Nov 6, 2026 6:00pm UTC

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 24, 2026
@astracat-bot

astracat-bot Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Nov 6, 2026, 6:12 PM UTC

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

🔎 Preview · 3ee5207

Storybook · Sandbox · built from this PR's head

Modified components

Component Size LoC Visual · A11y · RTL
lab/Comments 🔴 313.8 kB (+7.3 kB) · 2.5 kB CSS (0) — — · — · —
lab/CommentThread 🔴 308.3 kB (+6.9 kB) · 2.4 kB CSS (0) — — · — · —
Lightbox · Storybook 🔴 109.4 kB (+6.4 kB) · 9.9 kB CSS (+66 B) 2,971 (+502) ✓ · ✓ · ✓
29 more components
Component Size LoC Visual · A11y · RTL
BottomSheet 59.4 kB (0) · 7.5 kB CSS (0) — — · — · —
ContextMenu 90.5 kB (0) · 16.6 kB CSS (0) — — · — · —
DateInput 173.7 kB (0) · 16.4 kB CSS (0) — — · — · —
DateTimeInput 196.1 kB (0) · 18.1 kB CSS (0) — — · — · —
DropdownMenu 86.1 kB (0) · 16.7 kB CSS (0) — — · — · —
Markdown 162.2 kB (0) · 19.5 kB CSS (0) — — · — · —
MoreMenu 86.3 kB (0) · 16.7 kB CSS (0) — — · — · —
MultiSelector 88.7 kB (0) · 16.3 kB CSS (0) — — · — · —
Pagination 103.3 kB (0) · 17.3 kB CSS (0) — — · — · —
Popover 78.5 kB (0) · 13.4 kB CSS (0) — — · — · —
PowerSearch 271.9 kB (0) · 23.1 kB CSS (0) — — · — · —
ProgressBar 11.5 kB (0) · 6.2 kB CSS (0) — — · — · —
SegmentedControl 89.5 kB (0) · 6.8 kB CSS (0) — — · — · —
Selector 93.2 kB (0) · 16.9 kB CSS (0) — — · — · —
Slider 101.8 kB (0) · 8.7 kB CSS (0) — — · — · —
Switch 96.3 kB (0) · 9.8 kB CSS (0) — — · — · —
Table 99.9 kB (0) · 17.3 kB CSS (0) — — · — · —
TableCell 91.7 kB (0) · 17.0 kB CSS (0) — — · — · —
TableHeaderCell 91.3 kB (0) · 16.8 kB CSS (0) — — · — · —
TimeInput 150.5 kB (0) · 15.5 kB CSS (0) — — · — · —
Tokenizer 84.8 kB (0) · 18.1 kB CSS (0) — — · — · —
lab/ChatEmojiPicker 96.5 kB (0) · 773 B CSS (0) — — · — · —
lab/ChatReactionBar 97.6 kB (0) · 1.1 kB CSS (0) — — · — · —
lab/CommentComposer 137.6 kB (0) · 1.9 kB CSS (0) — — · — · —
lab/NavigationScreen 78.4 kB (0) · 1.5 kB CSS (0) — — · — · —
lab/NavigationStack 78.4 kB (0) · 1.5 kB CSS (0) — — · — · —
lab/PageControl 71.7 kB (0) · 709 B CSS (0) — — · — · —
lab/TourStep 84.7 kB (0) · 590 B CSS (0) — — · — · —
richtext/RichTextEditorToolbar 244.1 kB (0) · 454 B CSS (0) — — · — · —

Package size: @astryxdesign/core 6.80 MB packed (+11.0 kB); no new dependencies.

API changes: none in this pull request

Committed API report behind this build: cli 19 rows, core 24 rows; run pnpm api:report.

Touched components checked; add ci:run-visual or ci:run-rtl to run the full suites.

Current run: CI concluded failure; parts of this report may be missing.


Generated by PR Enrichment workflow | View full report

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on an engineering-owner decision: swipe paging for Lightbox galleries rests on the draft spec:AST-094 (FR1–FR23, DEC-1, DEC-2) and on a draft component:Lightbox record that does not cover paging, so an engineering owner needs to approve or reject that direction, including its proposed amendments to architecture:theme-tokens, family:buttons, architecture:interaction-modality and spec:AST-025, by commenting /approve-spec <full head SHA> on this pull request (the author may do this as an engineering owner). Whatever that decision, these fixes are needed:

  • Make gallery pages that are not current inert and aria-hidden, since Tab can currently reach a link in the next item's caption, scrolling that item into view under the previous item's counter, and screen readers read the neighbours' captions.
  • Re-observe the zoom stage when the current page changes, because two items away from the opening one a rotation no longer re-fits a zoomed photo.
  • Stop usePager from reporting controlled index changes from an Effect (architecture:react-component-runtime INV2 and INV3) and return the track when a controlled index does not follow a swipe.
  • Land the stacked pull requests first so this one carries only its own change instead of about 31,000 lines.

[Automated review]

Comment thread packages/core/src/Lightbox/Lightbox.tsx Outdated
<div
key={i}
data-lightbox-page=""
data-current={i === index ? '' : undefined}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pages that are not current stay in the tab order and the accessibility tree; only their <img> is hidden. With a four-item gallery open at 2 / 4, Tab from Previous reached a link in item 3's caption and the stage (overflow: hidden) scrolled 442 px, so item 3 showed under a 2 / 4 counter. Consider inert and aria-hidden="true" on every page without data-current.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, off-screen pages stayed focusable and in the accessibility tree. Fixed in 2834675: every page but the current one is inert and aria-hidden. LightboxPaging checks the attributes and tabs through the dialog without ever entering an off-screen page.

});
observer.observe(stage);
return () => observer.disconnect();
// The stage element is stable for the dialog's lifetime.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a gallery stageRef is pageMediaRef, which moves to a new element on every page, so this no longer holds: the observer stays on the opening page's box, and once that page unmounts (two items away) a rotation no longer re-fits a zoomed photo. Re-observe when the stage element changes, or observe the track, whose size every page shares.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the observer stayed on the opening page's box. Fixed in 2834675: the stage watches the track, whose size every page shares, and re-attaches whenever it turns on. LightboxPaging rotates two pages on, and the zoomed photo keeps its centre point.

Comment thread packages/core/src/hooks/usePager.ts Outdated

useEffect(() => {
if (index != null && index !== pager.index) {
pager.goTo(index, {reason: 'controlled'});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Effect reports onIndexChange(index, 'controlled') through goTo and names no external system, which architecture:react-component-runtime INV2 and INV3 ask for. It also never returns the track when a controlled index refuses a swipe, because it runs only when index changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #8146 039d4e9, which e762b43 takes: a controlled move is the owner's own and is never reported, and a page the owner refuses returns the pager to index.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval waits on an engineering owner's decision on the draft touch-presentations spec (AST-094) that this pull request introduces as the authority for Lightbox swipe paging, including whether buttons and arrow keys keep sliding under Reduce Motion where its FR8 asks for a cross-fade. 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. Whatever the decision, these need fixing:

  • Pages beside the current photo stay focusable and exposed: Tab reaches photo 3's caption link and scrolls it into view under “2 / 4”.
  • A controlled parent that refuses a swipe is left with the swiped photo on screen.
  • Only the photo the dialog first rendered re-fits when a zoomed photo is rotated.
  • usePager reports controlled index changes from an Effect.
  • The diff carries #8045, #8146 and #8148 (31,552 authored lines); rebase or retarget it so it holds only the paging step.

[Automated review]

},
shouldBeginPan: direction => !stage.isZoomed() || stage.isAtEdge(direction),
onIndexChange: (next, reason) => {
if (reason === 'drag') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When index is controlled and the parent refuses the change, this drag report goes through setIndex, which only calls onIndexChange, and usePager re-syncs only when index changes, so the track stays on the swiped photo: after a swipe the viewer shows photo 3 under “2 / 4”. Return the pager to the controlled index when the parent keeps it, and add a test with a refusing parent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the track stayed on the swiped photo under the old counter. Fixed in 2834675: a swipe's report renders the viewer once more, and when the parent kept its index the pager goes back to it. The new GalleryKeepsIndex story and LightboxPaging cover it (it stayed on the swiped photo before).

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval is waiting on an engineering owner's decision about Lightbox swipe paging and the draft spec:AST-094 that would govern it: an engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request (the author may do so if they are an engineering owner), or paging waits for a current Lightbox spec. Whatever that decision, these need changes:

  • Make the off-screen gallery pages inert, so screen readers and Tab no longer reach captions and controls that are not on screen.
  • Move the pager back to index when a controlled parent refuses a swipe, and stop calling onIndexChange from the controlled-sync Effect.
  • Report Lightbox's base-against-head gzip size (the import grows by about 31 KB gzip) and load the touch-only code lazily or justify it.
  • Re-observe the zoom stage when the current page changes, regenerate the CLI docs golden that fails test-node (1/4), and rebase onto the stacked pull requests so this one carries only the paging commit.

[Automated review]

@cixzhang

Copy link
Copy Markdown
Contributor

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:

  • Reduced motion. core/src/hooks/usePager.ts:353 handles reduced motion locally. Per design:reduced-motion (DR1–DR4), the animator is the one place that resolves it (touch cross-fades in 200 ms or less with no travel, a fine pointer changes at once), and core has no opt-out, so reducedMotion: 'keep' should be removed.

This is about where the behavior lives, not about the PR's intent.

@vjeux

vjeux commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Agreed: one implementation.

@github-actions github-actions Bot added the needs:design-review Affects visuals — Design should review label Oct 25, 2026
@vjeux

vjeux commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Pushed in 2834675, on #8146 @ f6b1827, which has no 'keep', and #8320 @ 41b46ea. The gallery passes no reduced-motion option of its own.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval is waiting on an engineering owner's decision about Lightbox swipe paging and the draft touch-presentations spec (AST-094) proposed to govern it: an engineering owner either approves that spec and the paging direction by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or paging waits for a current Lightbox spec. That decision has to settle AST-094's requirements, including FR4's rule that touch motion is not themable while this branch adds theme motion roles the pager reads, and how callers keep the fine-pointer behaviour on a touch device. Whatever the decision, these need changes:

  • Pause a gallery video when the viewer moves away from it; today a video the person started keeps playing, unseen and unreachable, on the neighbouring page.
  • Split the UIKit gesture table so Lightbox ships only the rows it reads, and report base and head sizes with the CSS delta, loading the touch-only code lazily or justifying its cost.
  • Remove the platform adaptation axis from the theme template, since defineTheme rejects it.
  • Rebase onto the stacked pull requests so this one carries only the paging commit, and split out the astryxStylex undefinedVariables option.

[Automated review]

Comment on lines +754 to +755
autoPlay={isCurrent && hasAutoPlay}
preload={isCurrent ? 'auto' : 'metadata'}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When this page stops being current only autoPlay and preload change, and neither pauses a playing video. The page stays mounted under key i as the previous neighbour, inert, so a video the person started keeps playing with its sound until they move two items away and the page unmounts. Consider pausing every non-current page's video when the index changes, with a paging test that plays a video.

Comment thread packages/cli/assets/theme.template.ts Outdated
Comment on lines +361 to +362
* are primary-pointer precision, contrast preference, motion preference, and
* platform (`ios` for the browsers of iPhone and iPad, `other` for the rest).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defineTheme no longer accepts a platform condition: themeAdaptations.ts admits width, pointer, contrast and motion, and any other key throws is not supported. A theme author following this sentence gets a build error; consider restoring the base wording without platform.

* UIKit's gesture thresholds and velocity filter, each with its source.
* @experimental
*/
export const UIKIT_GESTURES = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lightbox imports recognizers that read this one object, so the swipe, rotation and screenEdgePan rows and every source string ship with every Lightbox import, which component-style-authoring INV6 rules out for tuning tables. Consider one export per recognizer's rows, with the provenance strings kept out of the runtime values.

@cixzhang

cixzhang commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 878f6b8

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. Touch paging and zoom follow the current Lightbox spec (FR6, FR7, FR11), the record text this branch adds is owner-approved for this head, and a default gallery under a mouse behaves as on main.

  • The range is 9,183 authored lines with no scope exemption: #8215 and #9810 still sit on this pull request's previous revision, so the declared stack does not hold; rebase them onto this head.
  • useZoomStage.ts:292-311 sets the zoom pan's start threshold from Lightbox's own capture pointerdown listener, against architecture:react-component-runtime INV16 and Lightbox FR14; give PanGestureRecognizer a per-press threshold and delete the listener.

This pull request is reviewed on its own because its stack line does not hold yet.

[Automated review]

: UIKIT_PAN.hysteresis.value,
);
};
element.addEventListener('pointerdown', onPress, {capture: true});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This capture listener writes recognizers.pan.hysteresis on every press, outside the shared recognizer that owns the begin threshold (PanGestureRecognizer.ts:135, :338); ScreenEdgePanGestureRecognizer.ts:199 already picks a per-touch threshold inside the recognizer. architecture:react-component-runtime INV16 and Lightbox FR14 ask for no component listener here. Suggest a per-press threshold option on PanGestureRecognizer (for example hysteresis: number | ((touch: GestureTouch) => number)) passed from useZoomStage, and delete this listener and its Current-state sentence.

@cixzhang

cixzhang commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 7c96bcc

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. Touch paging and zoom follow the current Lightbox spec (FR6, FR7, FR11), the record text this branch adds is owner-approved for this head, and a default gallery under a mouse behaves as on main.

  • The range is 9,307 authored lines with no scope exemption: #8215 and #9810 still sit on an earlier revision of this pull request, so the declared stack does not hold; rebase them onto this head.
  • useZoomStage.ts:289-311 sets the zoom pan's start threshold from Lightbox's own capture pointerdown listener, against architecture:react-component-runtime INV16 and Lightbox FR14; give PanGestureRecognizer a per-press threshold and delete the listener.
  • Miss: the zoom settle after a pinch released past a limit plays the eased timing.curves.move curve (useZoomCore.ts:384-392), while current spec:AST-094 FR45 and DEC-26 require a gesture's settle to play a spring from its velocity; play it on a spring, or have an engineering owner amend AST-094 to name this exception.

This pull request is reviewed on its own because its stack line does not hold yet.

[Automated review]

Comment on lines +292 to +311
useAttachment(
surfaceRef,
element => {
const onPress = (event: PointerEvent) => {
const geometry = geometryRef.current;
setBeginThreshold(
recognizers.pan,
event.pointerType !== 'touch' &&
geometry != null &&
isZoomed(geometry, current())
? 0
: UIKIT_PAN.hysteresis.value,
);
};
element.addEventListener('pointerdown', onPress, {capture: true});
return () =>
element.removeEventListener('pointerdown', onPress, {capture: true});
},
recognizers,
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This capture listener writes recognizers.pan.hysteresis on every press, outside the shared PanGestureRecognizer that owns the begin threshold, which architecture:react-component-runtime INV16 and component:Lightbox FR14 rule out. A per-press threshold on the recognizer (for example hysteresis: number | ((touch: GestureTouch) => number)) would let useZoomStage pass it and drop this listener and its Current-state sentence.

: settleCurves(
from,
target,
resolveMotion(stageElement, zoomMedia.settle.curve),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Miss: a pinch released past a zoom limit settles here on zoomMedia.settle.curve, the timing.curves.move bezier. Current spec:AST-094 FR45 says a gesture's settle and rubber-band return MUST play a spring from the gesture's velocity, DEC-26 says any interaction that involves a gesture uses a spring, and the FR4 role paragraph says no motion plays timing.curves.move. Play this return on a spring role, or have an engineering owner amend AST-094 to name the zoom settle as an exception.

Comment on lines +161 to +164
/** The theme's duration token a tap-started zoom plays. */
const TAP_ZOOM_DURATION: DurationVarName = '--duration-medium';
/** The theme's easing token a tap-started zoom plays. */
const TAP_ZOOM_EASE: EaseVarName = '--ease-move';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (INV29): these two comments restate the constants' names; they can go.

vjeux added 2 commits November 4, 2026 04:22
Lightbox's zoom moves to the gesture recognizers and the compositor
animator. A pinch zooms about the fingers with the pan riding along, and
it gives past the zoom limits and eases back about the last centroid. A
finger's double tap zooms to the larger of filling the viewer and 2x,
about the tapped point, over the theme's move easing and medium
duration. A mouse's or a pen's double-click zooms 2x about the clicked
point. In the touch presentation a zoomed pan has UIScrollView's rubber
band, coast and edge bounce. A mouse's or a pen's drag pans 1:1 and
stops at the edges and at the release, as before.

The zoom takes every pointer: a ctrl-wheel or a trackpad pinch zooms,
and a trackpad's scroll pans a zoomed photo; a mouse wheel's notches
never pan. The photo's stage fills the viewer and takes the pinch; the
controls and caption keep the page's pinch-zoom.
Since a zoomed photo can now lie under Close, Previous, Next and the
counter, a zoomable Lightbox sets them on its own translucent backing
and keeps their icons and text on the on-dark colour, so they keep
3:1, and 4.5:1 for the counter's text, over any photo in every theme.
The keyboard keeps its 2x zoom about the centre over the released 200 ms
ease-out, and keyboard panning stops at the photo's edges.

The double tap reads its tokens through the shared theme-timing
reader in motion/themeTiming.ts. A theme that leaves --duration-medium
unset gets Core's default duration there, not a zoom that lands at once.
A Lightbox gallery moved only through its two arrow buttons. A finger's
drag or flick now pages between items with UIKit's paging release: one
page at most, with Photos' measured 40 pt gap, settled on the
compositor. The drag paging is part of the touch presentation, which a
coarse primary pointer brings (spec:AST-094 FR13); under a fine pointer
the buttons and the arrow keys page, as before. A zoomed photo keeps
the drag until its edge, as nested scroll views do, and a pinch never
pages.

A video pages from its frame. Its native controls are taken as a 48 px
band at its bottom edge, since browsers expose no control geometry, so
a drag that starts there stays the video's (component:Lightbox FR7).

Only the current page takes focus and is in the accessibility tree.
Next, Previous and any other change of `index` slide the row under the
touch presentation; under a fine pointer the row moves at once, so a
button the owner refuses shows nothing. A drag the owner refuses
springs back. Under Reduce Motion they fade. On a
zoomable gallery the photo rows take the stage's touches while the
track allows the page's pinch-zoom, so a pinch on a caption still
zooms the page.

The pan and the pinch read where their touches began from the shared
recognizers, and without zoom a tap beside a gallery's photo closes
the viewer through a shared tap recognizer.

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. Touch paging and zoom follow the current Lightbox spec (FR6, FR7, FR11), the record text this branch adds is the text an engineering owner approved, and a default gallery under a mouse behaves as on main.

  • The range is 9,531 authored lines with no scope exemption: #8215 and #9810 still sit on earlier revisions of this pull request, so the declared stack does not hold; rebase them onto this head.
  • useZoomStage.ts:289-311 sets the zoom pan's start threshold from Lightbox's own capture pointerdown listener, against architecture:react-component-runtime INV16 and Lightbox FR14; give PanGestureRecognizer a per-press threshold and delete the listener.
  • Miss: the zoom settle after a pinch released past a limit plays the eased timing.curves.move curve (useZoomCore.ts:389-393), while current spec:AST-094 FR45 requires a pinch's settle to spring from its velocity; play it on a spring, or have an engineering owner amend AST-094 to name this exception.
  • motion/themeTiming.ts:38-58 adds easedProgress, a second eased-curve builder beside solveAnimatedPageScroll and the zoom's own settleCurves, so the double tap no longer shares the curve code every other zoom settle uses; pass the theme's duration and curve to settleCurves, or build easedProgress on solveAnimatedPageScroll.

This pull request is reviewed on its own because its stack line does not hold yet.

[Automated review]

Comment on lines +292 to +311
useAttachment(
surfaceRef,
element => {
const onPress = (event: PointerEvent) => {
const geometry = geometryRef.current;
setBeginThreshold(
recognizers.pan,
event.pointerType !== 'touch' &&
geometry != null &&
isZoomed(geometry, current())
? 0
: UIKIT_PAN.hysteresis.value,
);
};
element.addEventListener('pointerdown', onPress, {capture: true});
return () =>
element.removeEventListener('pointerdown', onPress, {capture: true});
},
recognizers,
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This capture pointerdown listener sets recognizers.pan.hysteresis per press, outside the shared recognizer (architecture:react-component-runtime INV16, Lightbox FR14). ScreenEdgePanGestureRecognizer already picks a per-touch threshold inside the recognizer; give PanGestureRecognizer a per-press threshold (for example hysteresis: number | ((touch: GestureTouch) => number)) and delete this listener.

Comment on lines +389 to +393
: settleCurves(
from,
target,
resolveMotion(stageElement, zoomMedia.settle.curve),
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A pinch released past a limit settles here on zoomMedia.settle.curve, the timing.curves.move bezier. Current spec:AST-094 FR45 says a pinch's settle and rubber-band return MUST play a spring that starts at the gesture's velocity, and its FR4 role paragraph says no motion plays timing.curves.move. Play this return on a spring role, or have an engineering owner amend AST-094 to name the exception.

Comment on lines +38 to +58
export function easedProgress(
seconds: number,
ease: (x: number) => number,
): MotionCurve {
const at = (t: number) =>
seconds <= 0 || t >= seconds ? 1 : ease(Math.max(0, t) / seconds);
// A central difference, one step inside the curve at either end.
const step = Math.min(1e-3, seconds / 100);
return {
duration: seconds,
at,
velocity: t => {
if (seconds <= 0 || t >= seconds) {
return 0;
}
const before = Math.max(0, t - step);
const after = Math.min(seconds, t + step);
return (at(after) - at(before)) / (after - before);
},
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

easedProgress is the eased bezier with a central-difference velocity that solveAnimatedPageScroll (motion/paging.ts:121) already provides, and the zoom's other settles reach it through settleCurves. Return the duration and curve from the reader and pass them to settleCurves, handling a zero length once in solveAnimatedPageScroll, so one eased-curve builder remains.

@cixzhang

cixzhang commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

/approve-spec dc3a360

vjeux and others added 3 commits November 5, 2026 07:14
The UIKit constants table gains zoomMedia, measured on iOS 26.2 in a
UIKit lab app and in Photos: the give past the zoom limits, and Photos'
double tap and maximum zoom. UIKit's eased zoom settle stays a
measurement only. Lightbox's main interaction is a gesture, so its zoom
settles on springs.settle (spec:AST-094 FR45).

Lightbox/zoomStage.ts is the pure geometry of a zooming scroll view in a
photo viewer, checked against those numbers. A double tap zooms to the
larger of filling the stage and 2x, about the tapped point, as
component:Lightbox FR6 asks. Nothing renders the stage yet: Lightbox's
touch zoom plays it in the next change.
Lightbox's zoom moves to the gesture recognizers and the compositor
animator. A pinch zooms about the fingers with the pan riding along, and
it gives past the zoom limits and springs back about the last centroid,
from the photo's scale velocity at the lift. A finger's double tap zooms
to the larger of filling the viewer and 2x, about the tapped point. A
mouse's or a pen's double-click zooms 2x about the clicked point. Every
zoom settles on springs.settle, since Lightbox's main interaction is a
gesture (spec:AST-094 FR45). In the touch presentation a zoomed pan has
UIScrollView's rubber band, coast and edge bounce. A mouse's or a pen's
drag pans 1:1 and stops at the edges and at the release, as before.

The zoom takes every pointer: a ctrl-wheel or a trackpad pinch zooms,
and a trackpad's scroll pans a zoomed photo; a mouse wheel's notches
never pan. The photo's stage fills the viewer and takes the pinch; the
controls and caption keep the page's pinch-zoom.
Since a zoomed photo can now lie under Close, Previous, Next and the
counter, a zoomable Lightbox sets them on its own translucent backing
and keeps their icons and text on the on-dark colour, so they keep
3:1, and 4.5:1 for the counter's text, over any photo in every theme.
The keyboard keeps its 2x zoom about the centre, and keyboard panning
stops at the photo's edges.
@cixzhang

cixzhang commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 0fc7062

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. Touch paging follows the current Lightbox spec (FR5, FR7), and the spec text this pull request adds is approved for this head; two fixes remain:

  • useZoomStage.ts sets the zoom pan's start threshold from Lightbox's own capture pointerdown listener, although PanGestureRecognizer's hysteresis option takes a per-press function, so the threshold belongs there and the listener should go (architecture:react-component-runtime INV16, component:Lightbox FR14).
  • Miss: the zoom pan now refuses a drag that starts at a zoomed photo's edge for every Lightbox, so a single zoomed photo dragged outward from its edge under a finger no longer moves or rubber-bands as component:Lightbox FR6 states; yield at the edge only when a gallery's pager can take the drag.

This pull request is reviewed as a member of its stack, and it lands only when it and #8147 and #8148 below it are approved.

[Automated review]

Comment on lines +292 to +310
useAttachment(
surfaceRef,
element => {
const onPress = (event: PointerEvent) => {
const geometry = geometryRef.current;
setBeginThreshold(
recognizers.pan,
event.pointerType !== 'touch' &&
geometry != null &&
isZoomed(geometry, current())
? 0
: UIKIT_PAN.hysteresis.value,
);
};
element.addEventListener('pointerdown', onPress, {capture: true});
return () =>
element.removeEventListener('pointerdown', onPress, {capture: true});
},
recognizers,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This listener writes recognizers.pan.hysteresis on every press, outside the shared recognizer (architecture:react-component-runtime INV16, component:Lightbox FR14). PanGestureRecognizer's hysteresis option now takes (touch: GestureTouch) => number and reads it when the press's first touch lands, so the threshold can move into the constructor below and this block and setBeginThreshold can go:

const pan = new PanGestureRecognizer({
  name: 'Lightbox.zoomPan',
  // ...
  hysteresis: touch =>
    touch.pointerType !== 'touch' &&
    geometryRef.current != null &&
    isZoomed(geometryRef.current, current())
      ? 0
      : UIKIT_PAN.hysteresis.value,
});

* It follows UIKit's measured behavior (motion/uikit/zoomMedia.ts). Inside the
* limits the zoom follows the pinch 1:1. The content point under the centroid
* stays under it, past the limits too. No scale momentum follows the lift.
* Past a limit, the return is one eased animation anchored at the last

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (INV29): the return past a limit now springs from the pinch's velocity (:367-384), not one eased animation. Suggested: "Past a limit, the return springs from the pinch's velocity, anchored at the last centroid."

The zoom pan's begin threshold was set by Lightbox's own capture
pointerdown listener on the stage. The shared PanGestureRecognizer
takes a per-press hysteresis function, so the threshold moves there
and the listener goes. A mouse's or a pen's drag on a zoomed photo
still pans from its first move, and a finger keeps UIKit's threshold.

The zoom stage's header now says that the return past a limit springs
from the pinch's velocity.

A fine-pointer test's comment splits one 26-word sentence in two, under
the 25-word limit.
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 0fc7062

@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec b8266e6

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested. Touch paging follows the current Lightbox spec (FR5, FR7), the zoom pan's start threshold now goes through the shared pan recognizer, and the spec text this pull request adds is approved for this head; four fixes remain:

  • The branch still carries #8147 and #8148, which have both merged into main, so the range is 8,955 authored lines and conflicts with main in 13 files; update it onto main, which leaves the pager increment, and restack #8215 and #9810 on the new head so the Stack: line holds.
  • useZoomStage.ts refuses a finger's drag outward from a zoomed photo's edge in every zoomable Lightbox, so a single zoomed photo no longer rubber-bands there as component:Lightbox FR6 states; yield at the edge only when a gallery's pager can take the drag.
  • The record's new press-tracker line still says the zoom stage's own capture pointerdown listener sets the pan's start threshold, but this head removes that listener, so that sentence should go.
  • Miss: the description's "Why this choice" and "How to check it" say a mouse drag and a trackpad's sideways scroll page a gallery, while this head pages only under touch and its own specs pin the opposite, so those two lines should go.

[Automated review]

Comment on lines +223 to +226
return !(
Math.abs(x) >= Math.abs(y) &&
isAtEdgeOf(geometry, x < 0 ? 'left' : 'right')
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This edge rule runs in every zoomable Lightbox, so on a single zoomed photo a finger's drag outward from the edge it rests at never begins: the photo neither moves nor rubber-bands, where main begins the pan for any finger. component:Lightbox FR6 says "A zoomed photo pans with momentum and rubber-band edges." Yielding at the edge only when a pager can take the drag (for example an option Lightbox sets to isGallery && isTouch) keeps the gallery handoff and the single photo's band.

Comment on lines +107 to +109
`click` do not carry, and clears the flag a drag leaves for its click. The
zoom stage's own capture `pointerdown` listener sets the pan's start
threshold for each press.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This head removes the zoom stage's capture pointerdown listener (the threshold now comes from PanGestureRecognizer's per-press hysteresis), so this sentence reports a gap that no longer exists. Suggested: end the line after "…and clears the flag a drag leaves for its click."

vjeux added 3 commits November 6, 2026 07:44
This merge takes main at bef866e, with the Popover group, and
this pull request's own diff is unchanged.
The zoom stage's pan refused a finger's drag that headed past a zoomed
photo's edge in every Lightbox, so a single photo resting at its edge
no longer rubber-banded there. Only a gallery's touch pager takes that
drag now: the stage yields at the edge only when Lightbox says an
outer pager is there. A single photo gives past its edge as on main.

The record's press-tracker line no longer names the stage's own
pointerdown listener, which the pan's per-press threshold replaced.
The spec owners ruled this a consistency fix: AST-108 plays motion at
the theme's tempo, and AST-094 requires the touch presentations to
follow it, but the pager's settle after a release ignored it.
solvePaging now takes the tempo. Each step lasts tempo times as long
and carries tempo times the release's distance, so a settle from rest
keeps its shape and takes tempo times as long, and a release still
starts at 96 % of the finger's speed. At tempo 1 the settle is
unchanged. No motion role names the paging table, so usePager reads
the tempo of the theme the track is in. Every pager follows it: a
Lightbox gallery and a Carousel with hasSnap.

The busy-main-thread proof of the gallery's settle now plays at tempo
3, as the zoom's does. A swipe at tempo 1 checks that the slowed settle
is three times as long, and a frozen settle and one that the main
thread drives both fail the proof. A refused drag's spring-back is now
read from its keyframes rather than counted in main-thread frames,
which a loaded runner thins out.
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 22da53d

@astracat-bot astracat-bot Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: touch paging follows the current Lightbox spec (FR6, FR7), mouse and keyboard users keep today's behaviour, and the spec text this pull request adds is approved for this revision. One fix remains:

  • .changeset/pager-settle-tempo.md says a released page settles at the theme's tempo "in a Carousel with hasSnap", but hasSnap on its own is native CSS scroll snap and does not change, so the release note should say "in a Carousel given hasSnap={useSnapPaging()}".
  • Before merge: the branch now conflicts with main in LightboxFinePointer.a11y.browser.spec.ts, where #10301 rewrote the expectSpringZoom comment; merging main and keeping its comment resolves it.

Non-blocking: the GalleryWithCaptions story's comment says the photo keeps one box whatever captions render, while each page now sizes its photo above its own caption. CI has not run on this revision yet; a maintainer adds the ci:run label so it runs.

[Automated review]

Comment thread .changeset/pager-settle-tempo.md Outdated
'@astryxdesign/core': patch
---

[fix] A page that a swipe releases now settles at the theme's `tempo`, as the rest of its motion does: in a `Carousel` with `hasSnap`, and in a `Lightbox` gallery. At the default tempo the settle is unchanged.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasSnap written alone is true, which Carousel documents as native CSS scroll snap (Carousel.tsx:151-158); only a useSnapPaging() value runs the pager this change retimes (Carousel.tsx:443). As written, the release note tells every hasSnap user that their snap follows tempo. Suggest: "in a Carousel given hasSnap={useSnapPaging()}".

};

/**
* A gallery whose captions differ in length: the photo keeps one box

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: each page now lays out its own photo above its own caption (Lightbox.tsx:250-258), so a page with a longer caption gets a smaller photo box, as the description notes. This comment says the photo keeps one box whatever captions render; rewording it to what the story shows keeps it accurate.

vjeux added 3 commits November 6, 2026 09:02
The release note named a Carousel with hasSnap, but a plain hasSnap is
native CSS scroll snap. Only hasSnap={useSnapPaging()} runs the pager
whose settle now follows tempo, so the note names that value. The
AST-094 FR45 verification row no longer lists tempo as a gap of the
pager's settle.
This merge takes main at 1c142dc. meta#10301 rewrote the spring-zoom
check in LightboxFinePointer.a11y.browser.spec.ts onto the shared
interaction harness, and this pull request had split a sentence of
that check's old comment. The merge keeps meta#10301's check and its
comment as they are on main.
The check that Next and the arrow key show the next photo at once
samples the track every frame for 600 ms after the press, and it needs
more than 10 frames. WebKit in a 4-CPU container painted 3 to 7 frames
in that time under load, so the check failed there. The sampler now
runs for 600 ms and at least 12 frames. At half the test's timeout it
gives up, and too few frames fail by name. The one-place check is the
same.
@cixzhang

cixzhang commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

/approve-spec 3ee5207

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending CI: CI has not run on this revision yet, so a maintainer adds the ci:run label so it runs.

Touch paging follows the current Lightbox spec (FR6, FR7), mouse and keyboard users keep today's behaviour, and the spec text this pull request adds is approved for this revision. The earlier changeset wording is fixed: the tempo note now names a Carousel given hasSnap={useSnapPaging()}.

Non-blocking: the GalleryWithCaptions story's comment still says the photo keeps one box whatever captions render, while each page now sizes its photo above its own caption.

[Automated review]

This branch was successfully deployed

1 active deployment
Preview — 3ee52070 Deployed Nov 6, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:run A maintainer lets this outside contributor's pull request run CI (AST-030 FR17) ci:run-a11y Run the accessibility suite on this PR; write access, next push (AST-030 FR12) CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants