Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
There was a problem hiding this comment.
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
inertandaria-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
usePagerfrom reporting controlled index changes from an Effect (architecture:react-component-runtimeINV2 and INV3) and return the track when a controlledindexdoes 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]
| <div | ||
| key={i} | ||
| data-lightbox-page="" | ||
| data-current={i === index ? '' : undefined} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
|
||
| useEffect(() => { | ||
| if (index != null && index !== pager.index) { | ||
| pager.goTo(index, {reason: 'controlled'}); |
There was a problem hiding this comment.
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.
411109c to
d593122
Compare
There was a problem hiding this comment.
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.
usePagerreports 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') { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
d593122 to
b0ebc5a
Compare
There was a problem hiding this comment.
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
indexwhen a controlled parent refuses a swipe, and stop callingonIndexChangefrom 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]
|
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.
|
b0ebc5a to
2834675
Compare
There was a problem hiding this comment.
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
platformadaptation axis from the theme template, sincedefineThemerejects it. - Rebase onto the stacked pull requests so this one carries only the paging commit, and split out the
astryxStylexundefinedVariablesoption.
[Automated review]
| autoPlay={isCurrent && hasAutoPlay} | ||
| preload={isCurrent ? 'auto' : 'metadata'} |
There was a problem hiding this comment.
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.
| * are primary-pointer precision, contrast preference, motion preference, and | ||
| * platform (`ios` for the browsers of iPhone and iPad, `other` for the rest). |
There was a problem hiding this comment.
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 = { |
There was a problem hiding this comment.
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.
2834675 to
739d31d
Compare
There was a problem hiding this comment.
Approval is waiting on an engineering owner's decision. Paging, touch zoom and the mouse and trackpad gestures follow the current Lightbox record (FR6, FR7, FR11), but this branch also adds new text to that record (FR5's zoom-settle length law and a Motion roles section) and gives every gallery, mouse included, a caption row at the viewer's bottom edge that no current spec states and that spec:AST-017 FR59 reserves for its own owner decision. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner. Whatever the decision, please address these:
- Remove the hidden copies of other items' captions that reserve the caption row (
architecture:component-style-authoringINV7) and size the row with CSS instead. - With a mouse, keep controlled
indexchanges instant: today an app-driven move leaves the stage empty for about 140 ms, and a Next the parent refuses flashes the next photo. - Keep gallery captions out of the zoom surface, since a pinch on a zoomable gallery's caption no longer zooms the page (FR6, AR8), and correct the touch-zoom changeset that says it still does.
- Drop Lightbox's own refused-swipe return (
swipeReportRef,renderAfterSwipeand its Effect), which repeats whatusePageralready does. - Keep the FR5 and FR6 gaps in the record's Current-state list and name the stage's own press tracker under FR14, since the page settle is not a motion role and the caption pinch is unmet.
- Add fine-pointer before-and-after evidence (FR59), element counts before and after (INV7), and Lightbox's import size against
main(INV6). - Land #8147 and #8148 or rebase onto them so this pull request carries only its paging commit, and remove the unused
screenSignaltest helper.
[Automated review]
| <div | ||
| aria-hidden | ||
| inert | ||
| {...stylex.props( | ||
| styles.captionStacked, | ||
| styles.captionSizer, | ||
| )}> | ||
| {unrenderedCaptions.map(j => ( |
There was a problem hiding this comment.
These hidden copies of the captions no rendered page shows exist only to reserve the caption row. architecture:component-style-authoring INV7 rules that out: "Content renders once: no hidden twin to measure or reserve space." In an open 4-item gallery the DOM grows from 28 to 37 elements. Could the row be sized with CSS on the captions that are shown?
| if (!isTouch) { | ||
| pager.goTo(next, {animated: false}); |
There was a problem hiding this comment.
This pre-move covers only Lightbox's own buttons and keys. Any other controlled index change goes through usePager's controlled sync, which animates by default, so with a mouse an app-driven move leaves the stage empty for about 140 ms and a refused Next flashes the next photo before it returns. spec:AST-017 FR59 keeps fine-pointer behavior as it was. Could the controlled sync be instant where the primary pointer is fine?
| )} | ||
| /> | ||
| styles.track, | ||
| isZoomTarget && styles.trackZoomTarget, |
There was a problem hiding this comment.
In a gallery the captions live inside this track, so with hasZoom a pinch on a caption resolves to touch-action: none and never zooms the page. component:Lightbox FR6 and AR8 say a pinch on the caption still zooms the page. Keeping the caption row outside the zoom surface would restore that.
| useEffect(() => { | ||
| const reported = swipeReportRef.current; | ||
| swipeReportRef.current = null; | ||
| if (reported != null && index !== reported && pager.index !== index) { | ||
| pager.goTo(index, {reason: 'controlled'}); | ||
| } | ||
| }, [canNext, index, setIndex]); | ||
| }); |
There was a problem hiding this comment.
usePager already returns the pager to a controlled index the owner keeps after any reported page (its reported counter and controlled sync), and Lightbox always passes index. This Effect, swipeReportRef and renderAfterSwipe repeat that path and can be removed.
|
|
||
| [feat] Lightbox's zoom now works under a finger the way the iOS Photos viewer does, on a phone, a tablet or a touchscreen laptop. Pinch the photo to zoom around your fingers, and move them to pan while you zoom. Past the zoom limits the photo gives way, then eases back when you let go. Double-tap zooms until the photo fills the screen, at least 2×, and keeps the tapped point under your finger; double-tap again to go back. A zoomed photo pans with momentum, stretches past its edges and settles back. The keyboard keeps the zoom it had: 2× about the centre, over the same 200 ms ease-out, and keyboard panning now stops at the photo's edges instead of carrying it out of view. | ||
|
|
||
| The photo's stage fills the viewer. A pinch on the photo no longer zooms the page, and a pinch on the controls or caption still does. With `hasZoom` on, a tap on the dark area beside the photo closes the viewer after the double-tap wait instead of at once; a click still closes it at once. The zoom animations run on the compositor, so they stay smooth while the page is busy. With `hasZoom` off, nothing changes. |
There was a problem hiding this comment.
"a pinch on the controls or caption still does" holds for a single photo only: in a gallery with hasZoom the caption sits inside the zoom track, whose touch-action is none.
| page instead of the photo, a double tap zooms 2× about the centre, and there | ||
| is no swipe paging, drag or pinch to close, tap to hide the controls, or | ||
| thumbnail transition. | ||
| - **Touch presentation (FR8–FR10, FR12).** On a touch screen there is no drag |
There was a problem hiding this comment.
This drops FR5 and FR6 from the gaps, but the page settle and the button slide still play paging values that registerPagingTable lists as no motion role, and a pinch on a zoomable gallery's caption does not zoom the page. Keeping both gaps listed would keep the record accurate.
| // Gallery navigation. Under the touch presentation, the buttons and the | ||
| // arrow keys slide the pager to the new page, as a paging scroll view's | ||
| // `setContentOffset` does. Where the primary pointer is fine, they show the | ||
| // new item at once, as they always have. The pager moves there first, so | ||
| // the controlled sync finds nothing to animate. It returns to `index` if | ||
| // the owner refuses the change. |
There was a problem hiding this comment.
Non-blocking (INV29): "as they always have" narrates history, and the last sentence misdescribes a refused change with a mouse. Suggested: "Under the touch presentation the buttons and arrow keys slide the pager. Under a fine pointer they move it at once, before the owner answers."
There was a problem hiding this comment.
Paging and zoom follow the current Lightbox spec (FR6, FR7, FR11), but approval waits on an engineering owner's decision: this branch adds new requirement text to that current record (FR5's zoom-settle law, a Motion roles section and AR9) and ships an every-pointer gallery caption row and a control backing that no current spec states, which spec:AST-017 FR59 reserves for their own owner decision. 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 changes:
- With a mouse, pen or trackpad, a swipe the parent refuses snaps back in one frame instead of springing back as FR7 requires, because
isControlledChangeAnimated: isTouchalso covers the return after a reported drag; animate that return and add a mouse case to the paging spec. - The hidden copies of unrendered captions that reserve the caption row contradict INV7 ("Content renders once: no hidden twin to measure or reserve space"); size the row from the captions that are shown, or keep each caption under its photo.
- The stage's own
onPointerDownpress tracker repeats what the shared pan, pinch and tap recognizers already know, against FR14 and the one-shared-path rule; hand the pan's start point and the pinch's first touch through the shared hooks and drop the tracker, or record why they cannot serve. - The description still needs fine-pointer before-and-after evidence (AST-017 FR59), element counts before and after (INV7), and the Lightbox import's gzip and CSS size against
mainfor this head (INV6). - At 7,999 authored lines the range carries #8147 and #8148 beneath the paging commit; land or rebase onto them so this pull request carries only its own commit.
[Automated review]
| // Under the touch presentation a change of `index` slides the pager. Under | ||
| // a fine pointer it moves the pager at once, so a refused change shows | ||
| // nothing. | ||
| isControlledChangeAnimated: isTouch, |
There was a problem hiding this comment.
This flag also governs usePager's return after a reported drag the parent refuses (usePager.ts:1445-1452), so with a mouse, pen or trackpad the refused swipe jumps back in one frame (measured: −877 px to 0) instead of springing back as component:Lightbox FR7 requires. Animate the post-report return whatever the flag, and add a mouse-drag case beside the touch one in LightboxPaging.interaction.browser.spec.ts.
| src={currentItem.src} | ||
| alt={currentItem.alt} | ||
| draggable={false} | ||
| onPointerDown={e => { |
There was a problem hiding this comment.
This press tracker records the point, pointer type and caption hit beside the shared pan, pinch and tap recognizers, and the click path repeats the tap's UIKIT_TAP.allowableMovement (:716-723). That is a second press path (FR14: "adds no gesture listener outside them"). Hand the pan's start point to shouldBeginPan and read the pinch's first tracked touch in shouldBeginPinch, then drop this listener, or record why the shared modules cannot serve.
| {mediaArray[i].caption} | ||
| </div> | ||
| )} | ||
| <div |
There was a problem hiding this comment.
These hidden copies of the captions no rendered page shows exist only to reserve the caption row, which architecture:component-style-authoring INV7 rules out ("Content renders once: no hidden twin to measure or reserve space"); an open 4-item gallery goes from 28 to 37 elements. Size the row from the shown captions, or keep each caption under its photo.
There was a problem hiding this comment.
Paging and zoom follow the current Lightbox spec (FR6, FR7, FR11), and a refused mouse drag now springs back, but approval waits on an engineering owner's decision: this branch adds new requirement text to that current record (FR5's zoom-settle law, a Motion roles section and AR9) and ships looks no current spec states, for mouse users too (one bottom caption row in every gallery, a dark control backing with hasZoom, and a zoom-target focus ring that traces the whole viewer), which spec:AST-017 FR59 reserves for their own owner decision. 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 changes:
- The hidden copies of unrendered captions that reserve the caption row contradict INV7 ("Content renders once: no hidden twin to measure or reserve space"); size the row from the captions that are shown, or keep each caption with its photo.
- The description still needs fine-pointer before-and-after evidence (AST-017 FR59) for the bottom caption row, the control backing and the focus ring.
- At 8,102 authored lines the range carries #8147 and #8148 beneath the paging commit; land that stack first and qualify the rest as an owner-directed batch once every delta is settled, or split it further.
[Automated review]
| styles.captionStacked, | ||
| styles.captionSizer, | ||
| )}> | ||
| {unrenderedCaptions.map(j => ( |
There was a problem hiding this comment.
These copies of other items' captions mount caller content a second time only to reserve the caption row (an open 4-photo gallery goes from 28 to 37 elements). architecture:component-style-authoring INV7: "Content renders once: no hidden twin to measure or reserve space, because CSS can size and reserve it on the element that shows it." Size the row from the captions that are shown, or keep each caption with its photo so no row needs reserving, and delete the sizer.
| }); | ||
| // A gallery pages like Photos. A drag or a flick moves between items on a | ||
| // UIKit paging scroll view's release: one page at most, with Photos' 40 pt | ||
| // gap. The buttons and keys move there on `setContentOffset(animated:)`. A |
There was a problem hiding this comment.
Non-blocking (INV29): the buttons and keys slide only under the touch presentation (isControlledChangeAnimated: isTouch below); with a mouse they move at once. Suggested: "A drag or a flick moves one page at most, with Photos' 40 pt gap. A zoomed photo keeps the drag until its edge."
|
/approve-spec e670f4b |
There was a problem hiding this comment.
Changes requested. Paging and zoom follow the current Lightbox spec (FR6, FR7, FR11), each gallery page now keeps its caption under its photo, and the engineering owner's /approve-spec for this head settles the record's new text (FR5's zoom-settle law, the Motion roles section, AR9) and the new looks. Two things still need changes:
- In a zoomable gallery whose current item has a caption, a click or tap in the strip the caption leaves empty at the viewer's bottom edge no longer closes the viewer (it does on
mainand in a gallery without zoom); count a press on the current page outside its photo and caption as the backdrop, and add a test for it. - At 7,974 authored lines the range needs its owner-directed batch named in the description by its row identifier (it otherwise qualifies at this size), or the zoom constants and touch zoom beneath should land first.
[Automated review]
| if ( | ||
| e.target === e.currentTarget || | ||
| e.target === containerRef.current || | ||
| (hasZoom && !isVideo && e.target === mediaGroupRef.current) |
There was a problem hiding this comment.
In a zoomable gallery whose current item has a caption, the caption is laid out at the page's bottom edge and moved up under the photo (placeCaption), so its slot is left empty. A click there lands on the [data-lightbox-page] element, which this check does not count as the backdrop, and the zoom stage's tap ignores it because the point is outside the stage. At 1280 × 1100 a click at (640, 1086) closes the viewer on main and leaves it open here. Counting the current page itself as the backdrop here (or widening the stage tap's check to the page) fixes it; a test that clicks the empty slot would pin it.
| placeCaption(); | ||
| }, | ||
| }); | ||
| // A touch that began on a gallery page's caption. |
There was a problem hiding this comment.
Non-blocking (INV29): this restates the name startsOnCaption; it can go.
|
/approve-spec b7c9690 |
There was a problem hiding this comment.
Changes requested. Touch paging and zoom follow the current Lightbox spec, the record's new text is owner-approved for this head, and a click in the strip a raised caption leaves now closes the viewer again. Two things need changes:
- Desktop behavior change needs an owner decision: a sideways mouse drag or a trackpad's sideways scroll now pages a gallery with default props (
Lightbox.tsx:840, together with the pager's default trackpad-scroll pan), so either gate the pager's pan to touch and pen or cite the owner decision that covers it. - At 8,030 authored lines the range is over the owner-directed batch limit of 8,000 and declares no stack, so land #8147 and #8148 first or add one identical
Stack: #8147 → #8148 → #8197line to all three descriptions.
[Automated review]
| // Every pointer pages, a mouse's drag and a trackpad's scroll included. | ||
| // A drag that starts on a video's controls stays the video's. | ||
| panOptions: { | ||
| allowedPointerTypes: ['touch', 'pen', 'mouse'], |
There was a problem hiding this comment.
Desktop behavior change needs an owner decision: with 'mouse' here, and the pager's default trackpad-scroll pan, a sideways mouse drag or trackpad scroll now pages a gallery with default props. Gate the pan to touch and pen (allowedPointerTypes: ['touch', 'pen'], allowedScrollTypes: []), or cite the owner decision that covers mouse and trackpad paging.
There was a problem hiding this comment.
The owner decision is the current record: component:Lightbox FR11 on main says "A mouse, a pen, and a trackpad also use FR7's drag paging…", and the engineering owner stamped this head (22:31Z).
| readonly translation: Point; | ||
| } | ||
|
|
||
| /** Every pointer: a mouse, a pen and a finger. */ |
There was a problem hiding this comment.
Non-blocking (INV29): this comment restates the constant's name; delete it.
| ); | ||
| return; | ||
| } | ||
| // Ended or cancelled. |
There was a problem hiding this comment.
Non-blocking (INV29): // Ended or cancelled. restates the branch; delete it.
|
/approve-spec d585c45 |
There was a problem hiding this comment.
Changes requested. Touch paging and zoom follow the current Lightbox spec, and the record's new text is owner-approved for this head. Two things need changes:
- Desktop behavior change needs an owner decision: a sideways mouse drag or trackpad scroll now pages a gallery with default props (
Lightbox.tsx:841, plus the pager's default trackpad-scroll pan), so either gate the pager's pan to touch and pen or cite the owner decision that covers this desktop change. - Scope: the range carries 8,030 authored lines with no exception, because the
Stack:line does not hold (#8215 and #9810 carry different lines and do not contain this head) and the batch is over its size ceiling, so declare one identical stack line in #8147, #8148 and this pull request, or land #8147 and #8148 first.
Non-blocking: the comments at useZoomStage.ts:90 and :392 restate the code and can go.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: this branch rewrites the current Lightbox spec, and no spec approval covers this revision. The change drops mouse, pen and trackpad drag paging from FR11, says a mouse wheel's notch leaves a zoomed photo in place, adds FR5's zoom-settle length law with a Motion roles section, and adds AR9's dark control backing. An engineering owner records the decision by commenting /approve-spec <full head SHA> on this pull request; the author may do so as an engineering owner.
These need changes whatever that decision is:
- Desktop behavior change needs an owner decision: a default gallery's stage now fills the viewer, so a mouse press beside the photo that moves more than 45 px or is held past 1.5 s no longer closes the viewer (
Lightbox.tsx:867-905, pinned byLightbox.test.tsx:363-383); close a mouse click there through the dialog's backdrop click and keep the tap recognizer for touch and pen, or cite the decision that covers it. - The props docs say "a swipe, a mouse drag or a trackpad's sideways scroll moves between items too" (
Lightbox.doc.mjs:83and the Chinese copy at:198), and the changeset says "On a touch screen", but drag paging runs only under a coarse primary pointer; say "on a phone or tablet" instead. - The range is 8,143 authored lines with no scope exemption: #8215 and #9810 sit on this pull request's previous revision, so the
Stack:line does not hold; rebase them onto this revision, or land #8147 first.
[Automated review]
| () => | ||
| new TapGestureRecognizer({ | ||
| name: 'lightbox-backdrop-tap', | ||
| allowedPointerTypes: ['touch', 'pen', 'mouse'], |
There was a problem hiding this comment.
Desktop behavior change needs an owner decision: with 'mouse' here and the gallery stage filling the viewer, a mouse press beside the photo that moves more than 45 px or is held past 1.5 s no longer closes the viewer, where main closes on any click on the dark area. Close a mouse click there through handleBackdropClick and keep this recognizer for touch and pen, or cite the decision that covers it.
| type: 'LightboxMedia | LightboxMedia[]', | ||
| description: | ||
| "Media to display. Pass a single object for one item, or an array for gallery mode with prev/next navigation. Each item is {src: string, alt: string, caption?: ReactNode, type?: 'image' | 'video'}; type defaults to 'image', and zoom/pan is disabled for 'video'.", | ||
| "Media to display. Pass a single object for one item, or an array for gallery mode with prev/next navigation (a swipe, a mouse drag or a trackpad's sideways scroll moves between items too). Each item is {src: string, alt: string, caption?: ReactNode, type?: 'image' | 'video'}; type defaults to 'image', and zoom/pan is disabled for 'video'.", |
There was a problem hiding this comment.
A mouse drag and a trackpad's sideways scroll do not page on this revision (Lightbox.tsx:835 enables the pan only under a coarse primary pointer). Suggest "(on a phone or tablet, a swipe moves between items too)", here and in the Chinese copy at line 198.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] A Lightbox gallery now pages the way the iOS Photos viewer does. On a touch screen, drag or flick between items: the row follows your finger, and the release lands on the next or previous item, one at most. A gap of 40 px separates the items while they move. A zoomed photo pans with your finger until it reaches its edge; a new drag from that edge moves to the next item. A photo you leave zoomed returns to its fitted size once the gallery rests. Under a finger, the previous and next buttons and the arrow keys slide to the item instead of swapping it in place. |
There was a problem hiding this comment.
"On a touch screen" also covers a touchscreen laptop, whose fine primary pointer gets no drag paging. Suggest "On a phone or tablet, drag or flick between items".
|
/approve-spec 48f02af |
There was a problem hiding this comment.
Changes requested. Touch paging and zoom follow the current Lightbox spec (FR6, FR7), the spec text this branch adds is owner-approved for this revision, and the docs and changeset now say swipe paging is for phones and tablets. These need changes:
- Desktop behavior change needs an owner decision: in a default gallery, a mouse press beside the photo that is released on the photo no longer closes the viewer, where
maincloses it, because the new click handler atLightbox.tsx:1146-1160decides by the release point alone; close when the press or the release falls outside the current photo-and-caption box, asmain's backdrop target did, or cite the decision that covers it. - Miss: in a default gallery, a mouse click on the dark area beside a photo that is narrower than its caption now closes the viewer, where
mainkeeps it open, and the same press-and-release rule fixes it. - The range is 8,300 authored lines with no scope exemption: #8215 and #9810 sit on this pull request's earlier revision, so the
Stack:line does not hold; rebase them onto this revision, or land #8147 first.
Smaller, non-blocking: the every-pointer changeset should say "With hasZoom", and two test comments still describe the earlier stage.
[Automated review]
| onClick={ | ||
| isGallery && !isZoomTarget | ||
| ? e => { | ||
| // A keyboard's click (detail 0) on a caption's | ||
| // link is the link's. | ||
| if ( | ||
| e.detail > 0 && | ||
| pointerTypeRef.current === 'mouse' && | ||
| !isOnCurrentMedia(e.clientX, e.clientY) | ||
| ) { | ||
| handleClose(); | ||
| } | ||
| } | ||
| : undefined | ||
| } |
There was a problem hiding this comment.
Desktop behavior change needs an owner decision: this decides by the release point alone, so in a default gallery a mouse press beside the photo released on it no longer closes the viewer (main closes it, since the click lands on the container), and a click beside a photo narrower than its caption now closes it (main keeps it open, since the click lands on the media group). Measured in the Gallery story at 1280 × 800 with a 200 × 1200 photo. To match main, record the press point in onPointerDown and close when the press or the release falls outside the union box of the current photo (or video) and caption.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Lightbox: a ctrl-wheel or a trackpad pinch now zooms a photo, and a trackpad's scroll pans a zoomed photo. |
There was a problem hiding this comment.
Non-blocking: these gestures need hasZoom; without it a ctrl-wheel still zooms the page. Suggested: "[feat] Lightbox: with hasZoom, a ctrl-wheel or a trackpad pinch now zooms a photo, and a trackpad's scroll pans a zoomed photo."
| // Without zoom, a click on the dark area lands on the container or the | ||
| // dialog, and both close. A gallery's stage closes on its own tap. With |
There was a problem hiding this comment.
Non-blocking (INV29): under a mouse the gallery's stage closes on its click (line 1146), not its tap. Suggested: "A gallery's stage closes on its own tap, or a mouse's click."
|
/approve-spec 21b6f31 |
There was a problem hiding this comment.
Changes requested. Touch paging and zoom follow the current Lightbox spec (FR6, FR7), the spec text this branch adds is owner-approved for this revision, and a mouse click beside a default gallery's photo now closes exactly as on main. Two things need changes:
- The range is 8,480 authored lines with no scope exemption: #8215 and #9810 still sit on this pull request's earlier revision, so the
Stack:line does not hold; rebase them onto this revision, or land #8147 first, which leaves about 7,500 lines here. .changeset/lightbox-every-pointer.mdsays a ctrl-wheel or a trackpad pinch now zooms a photo, but only a Lightbox withhasZoomdoes; write "Lightbox: withhasZoom, a ctrl-wheel or a trackpad pinch now zooms a photo, …" so the release note does not promise zoom to every Lightbox.
Smaller, non-blocking: reflow the comment at Lightbox.tsx:673-677, and two test comments still describe an earlier version of the stage.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Lightbox: a ctrl-wheel or a trackpad pinch now zooms a photo, and a trackpad's scroll pans a zoomed photo. |
There was a problem hiding this comment.
These gestures attach only with hasZoom (the zoom stage runs with isEnabled: isOpen && isZoomTarget), and without it a ctrl-wheel still zooms the page, so this release note promises photo zoom to every Lightbox. Suggested:
[feat] Lightbox: with `hasZoom`, a ctrl-wheel or a trackpad pinch now zooms a photo, and a trackpad's scroll pans a zoomed photo.
|
/approve-spec 2b36122 |
There was a problem hiding this comment.
Changes requested. Touch paging and zoom follow the current Lightbox spec (FR6, FR7), the spec text this branch adds is approved by an engineering owner for this revision, and a default gallery looks and behaves with a mouse as on main. The changeset finding from the last review is fixed. One thing still needs a change:
- The range is 8,512 authored lines with no scope exemption: #8215 and #9810 still sit on this pull request's previous revision, so the
Stack:line does not hold; rebase them onto this revision, or land #8147 first, which leaves about 7,560 lines here.
Smaller, non-blocking: two test comments still describe an earlier version of the stage, and the paging changeset gives the import as 99.7 KB where the description measures 99.6 KB.
[Automated review]
| page, | ||
| }) => { | ||
| // A window taller than the photo's fitted height leaves the dark area | ||
| // above it; the stage stretches only under the touch layer. |
There was a problem hiding this comment.
Non-blocking: with hasZoom the single photo's stage stretches on every pointer (mediaGroupZoom and imageWrapperFill are not gated on the touch presentation), so this click lands on the stage and closes through its tap. Suggested: "A window taller than the photo's fitted height leaves the dark area above it; the stage's tap there closes the viewer."
| const y = before.stage.y + before.stage.height / 2; | ||
| const left = before.stage.x + 100; | ||
| // 2x about the centre from the keyboard (a double-click is the stage's | ||
| // double tap, the aspect fill), with room to drag past an edge. |
There was a problem hiding this comment.
Non-blocking: a mouse double-click zooms 2× about the clicked point (doubleClickAt, useZoomCore.ts:454-463), not the aspect fill. Suggested: "(a double-click zooms 2× about the clicked point)".
|
|
||
| Each item's caption moves with its item, right under its photo. `onIndexChange` also reports a swipe. | ||
|
|
||
| Lightbox now loads its touch code with it, as Toast and Slider do. Its import grows from 51.4 KB to 99.7 KB gzip with the zoom and paging beneath, mostly shared touch modules that a page loads once; Lightbox's own code adds 14.1 KB minified. |
There was a problem hiding this comment.
Non-blocking: the description measures 99,585 bytes gzip, which is 99.6 KB, not 99.7 KB.
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, the eased settle and its length law, and Photos' double tap and maximum zoom. The settle's three lengths are internal motion roles. component:Lightbox names them in a new Motion roles table, and the generated motion role table carries them. Lightbox/zoomStage.ts is the pure geometry of a zooming scroll view in a photo viewer, checked against those numbers. A double tap zooms to the larger of filling the stage and 2x, about the tapped point, as component:Lightbox FR6 asks. Nothing renders the stage yet: Lightbox's touch zoom plays it in the next change.
|
/approve-spec 878f6b8 |
There was a problem hiding this comment.
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-311sets the zoom pan's start threshold from Lightbox's own capturepointerdownlistener, againstarchitecture:react-component-runtimeINV16 and Lightbox FR14; givePanGestureRecognizera 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}); |
There was a problem hiding this comment.
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.
|
/approve-spec 7c96bcc |
There was a problem hiding this comment.
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-311sets the zoom pan's start threshold from Lightbox's own capturepointerdownlistener, againstarchitecture:react-component-runtimeINV16 and Lightbox FR14; givePanGestureRecognizera per-press threshold and delete the listener.- Miss: the zoom settle after a pinch released past a limit plays the eased
timing.curves.movecurve (useZoomCore.ts:384-392), while currentspec:AST-094FR45 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]
| 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, | ||
| ); |
There was a problem hiding this comment.
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), |
There was a problem hiding this comment.
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.
| /** 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'; |
There was a problem hiding this comment.
Non-blocking (INV29): these two comments restate the constants' names; they can go.
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.
There was a problem hiding this comment.
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-311sets the zoom pan's start threshold from Lightbox's own capturepointerdownlistener, againstarchitecture:react-component-runtimeINV16 and Lightbox FR14; givePanGestureRecognizera per-press threshold and delete the listener.- Miss: the zoom settle after a pinch released past a limit plays the eased
timing.curves.movecurve (useZoomCore.ts:389-393), while currentspec:AST-094FR45 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-58addseasedProgress, a second eased-curve builder besidesolveAnimatedPageScrolland the zoom's ownsettleCurves, so the double tap no longer shares the curve code every other zoom settle uses; pass the theme's duration and curve tosettleCurves, or buildeasedProgressonsolveAnimatedPageScroll.
This pull request is reviewed on its own because its stack line does not hold yet.
[Automated review]
| 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, | ||
| ); |
There was a problem hiding this comment.
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.
| : settleCurves( | ||
| from, | ||
| target, | ||
| resolveMotion(stageElement, zoomMedia.settle.curve), | ||
| ); |
There was a problem hiding this comment.
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.
| 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); | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
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.
|
/approve-spec dc3a360 |
Batch: B7 Lightbox
Stack: #8147 → #8148 → #8197 → #8215, #9810 (both on #8197)
Stacked on #8148 (and #8147 below it). Review this pull request's one commit: vjeux/astryx@14f14d7...dc3a360
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-094FR13), 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:
@playwright/testa lint error in browser specs, so the Paging and PagingModes specs importtestandexpectfrom@astryxdesign/a11y-spec/playwright, as main's do.pointerdownhandler and the zoom stage's capture listener for the start threshold.Rebased on #8148's 33e0568 (main 5a373df). Against d585c45, the head the spec owners stamped:
useZoomStagereads its options directly in the tap callback, as feat(Lightbox): touch zoom like the iOS Photos viewer #8148 now does, sinceuseGestureRecognizercalls the newest callback.usePagerkeeps a refused drag's spring-back withisControlledChangeAnimatedoff. 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.hasZoom, since a Lightbox without it leaves them to the page.Against 39ad02a, where the iPhone check passed, this head answers the reviews of that head and of 5a34859:
usePagerhook gainsisControlledChangeAnimated(defaulttrue, so Carousel is unchanged), and Lightbox passesisControlledChangeAnimated: isTouch.isControlledChangeAnimatedsays; only a newindexfollows it.pointerdownlistener now only records the pointer type the browser'sdblclickdoes not carry.usePagercall 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:LightboxFR7 (current) decides it.Options and tradeoffs
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.
spec:AST-094FR43).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 ownpointerdownlistener records each press's pointer type, which the browser'sdblclickdoes 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:How to check it
In the "Gallery with zoom" story, on a phone:
Swipe left: the next photo slides in, and the counter reads 2 / 4.
Drag a little and let go: the photo springs back.
Zoom in, drag to the photo's edge, then drag again: the next photo comes.
Pinch with a sideways drift: the photo zooms; nothing pages.
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.
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, drag sideways, or scroll sideways on a trackpad: the row follows, as a finger's does. A click on Next shows the next photo at once. With Reduce Motion on, a swipe fades instead of sliding.
Evidence
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
usePagerreverted: 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
usePagerinteraction 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:
this pull request:
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:
this pull request: 
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:
this pull request: 
Safari on the iOS 26.2 simulator, from the previous head (the paging is unchanged here), a swipe left:
Android 15 emulator, Chromium:
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 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-lockfile0; coretsc0; strict ESLint on every changed file 0;CI=true pnpm lint0 errors; the Lightbox, motion, gestures and hooks unit files 2,650/2,650;pnpm check:knowledgeandpnpm check:changesetspass; 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-lockfileexits 0; coretscis clean, and so is strict ESLint on every changed file;CI=true pnpm lintshows no error; INV29 finds no long sentence;pnpm check:knowledgepasses;usePager, motion/uikit and gestures unit files pass 1302/1302;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
usePagerunit 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-lockfileexits 0; coretscis clean, and so is strict ESLint on every changed file;pnpm lintshows the same 171 warnings as the previous head and no error; INV29 finds no long sentence;usePager, motion/uikit and gestures unit files pass 1290/1290;pnpm check:knowledgepasses;pan-y pinch-zoom);detailcheck (5641fb2), the unit "leaves the viewer open when the keyboard activates a caption link after a mouse press";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";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.