Skip to content

docs(Lightbox): caption type comparison story for review (not for merge) - #10084

Closed
vjeux wants to merge 8 commits into
meta:mainfrom
vjeux:uikit/c13-lightbox-caption-compare
Closed

vjeux wants to merge 8 commits into
meta:mainfrom
vjeux:uikit/c13-lightbox-caption-compare

Conversation

@vjeux

@vjeux vjeux commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Closed, not for merge: the design owner picked the body type for gallery captions on card 46D, and #9810 builds it in (its caption is body Text). This preview's job is done.

A preview for #9810's caption type pick, on #9810's head 053f321: the same captions in the body type, in the supporting type, and in the supporting type with the primary color. The story-only captionType and captionColor props exist only for this comparison and never ship. Not for merge; it closes after the pick.

vjeux added 3 commits November 4, 2026 01:30
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.
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.
@vercel

vercel Bot commented Nov 5, 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 5, 2026 7:44pm UTC

Request Review

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

astracat-bot Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

AI review status for this pull request.

Review status Updated
❓ Needs maintainer input (for maintainers only) Nov 5, 2026, 7:55 PM UTC

@github-actions github-actions Bot added the needs:design-review Affects visuals — Design should review label Nov 5, 2026
@github-actions github-actions Bot added the needs:spec-owner-review Current knowledge records await owner approval label Nov 5, 2026
@vjeux
vjeux force-pushed the uikit/c13-lightbox-caption-compare branch from 1a940f7 to 743e48d Compare November 5, 2026 13:45

@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.

This preview needs an engineering owner's decision before the review can finish. It is marked not for merge, but it carries #9810's whole branch plus two story-only props, captionType and captionColor, on the exported LightboxProps.

If it closes after the caption type pick, as the description says, nothing more is needed here. If it is meant to merge, an engineering owner records the decision by commenting /approve-spec followed by the full head SHA. The author may do this as an engineering owner. The approval covers the record changes it carries: component:Lightbox FR1, FR5 and its Motion roles table, FR11, the new FR17 and AR9, spec:AST-038's Lightbox row, and the three zoomMedia.settle motion-role rows. The owner also rules on whether the two props may ship.

Blocking, whatever the decision:

  • The pull request changes 10,495 authored lines, of which the preview's own change is 81; closing it after the pick, or basing it on #9810's branch, keeps it to its purpose.
  • useZoomStage.ts adds a capture pointerdown listener that rewrites the shared pan recognizer's start threshold on every press, so the fix belongs in PanGestureRecognizer (a per-press threshold) with the listener removed.

To settle with the spec decision:

  • After a pinch past a zoom limit, the zoom settle eases from rest (settleCurves in zoomStage.ts), while spec:AST-094 FR45 requires a spring that starts at the gesture's velocity.
  • A zoomed photo's mouse, pen or trackpad pan stops dead at its edges and at the release, while spec:AST-094 FR43 gives fine pointers the same rubber band and spring release as a finger.

[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 listener sets recognizers.pan.hysteresis on every press, from outside the shared PanGestureRecognizer. No other component writes a recognizer's threshold, and component:Lightbox FR14 asks for no Lightbox-owned gesture listener. A per-press threshold in PanGestureRecognizer (read from the first touch's pointerType in touchesBegan, as ScreenEdgePanGestureRecognizer does for its zone) would let this listener go.

Comment thread packages/core/src/Lightbox/zoomStage.ts Outdated
readonly scale: MotionCurve;
} {
const along = (a: number, b: number): MotionCurve =>
solveAnimatedPageScroll({

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.

No velocity is passed here, so the settle after a pinch past a zoom limit starts from rest. spec:AST-094 FR45 says the settle after a pinch "MUST play a spring … that starts at the gesture's velocity", and the new FR5 cites FR45 for this curve. Either amend FR45 in the same change or start the settle at the release velocity.

* Not for merge: the caption's Text type, for the caption type comparison
* preview only. This prop exists only in that preview and never ships.
*/
captionType?: 'body' | 'supporting';

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.

captionType, and captionColor below it, are on the exported LightboxProps, so every caller (and UseLightboxOptions) gets them, though the JSDoc says they never ship. That holds only if this pull request closes unmerged. Otherwise an engineering owner needs to rule on them.

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.
@vjeux
vjeux force-pushed the uikit/c13-lightbox-caption-compare branch from 743e48d to 4c07f5c Compare November 5, 2026 16:40
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

Current run: The current CI run concluded skipped. Current analysis is unavailable.


Generated by PR Enrichment workflow | View current CI run

@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.

This preview needs an engineering owner's decision before the review can finish: it is marked not for merge, yet it carries #9810's whole branch plus two story-only props, captionType and captionColor, on the exported LightboxProps. If it closes after the caption type pick, as the description says, nothing more is needed. If it is meant to merge, an engineering owner records the decision by commenting /approve-spec followed by the full head SHA (the author may do this as an engineering owner), covering component:Lightbox FR1, FR11, the new FR17 and AR9, and spec:AST-038's Lightbox row, and rules on whether the two props may ship.

Blocking, whatever the decision:

  • The pull request changes 10,190 authored lines, of which the preview's own change is 73, so closing it after the pick, or basing it on #9810's branch, keeps it to its purpose.
  • useZoomStage.ts adds a capture pointerdown listener that rewrites the shared pan's start threshold on every press, although PanGestureRecognizer already takes hysteresis as a per-press function, so pass that function and remove the listener.
  • The proposed FR17 allows no Lightbox-owned colour beyond AR9's backing for the controls, yet every captioned gallery's band and open caption sit on a Lightbox-owned backing (CAPTION_BACKING in Lightbox.tsx), so the record or the code has to change.

To settle with the spec decision:

  • A zoomed photo's mouse, pen or trackpad pan stops dead at its edges and at the release, while spec:AST-094 FR43 gives fine pointers the same rubber band and release spring as a finger.

[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 listener writes recognizers.pan.hysteresis on every press from outside the shared recognizer. PanGestureRecognizer now takes hysteresis as a function it reads from each press's first touch, so hysteresis: touch => (touch.pointerType !== 'touch' && isZoomed(...) ? 0 : UIKIT_PAN.hysteresis.value) on Lightbox.zoomPan would let this listener and setBeginThreshold go (architecture:react-component-runtime INV16, component:Lightbox FR14).


// The caption's backing: the controls' backing colour, darker, so the caption
// keeps 4.5:1 over any photo or backdrop behind it (WCAG 2.2 1.4.3).
const CAPTION_BACKING = `color-mix(in srgb, ${colorVars['--color-on-light']} 90%, transparent)`;

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 backing paints every captioned gallery's band and open caption, with or without hasZoom, but the proposed FR17 allows "no Lightbox-owned colors beyond AR9's backing and on-dark color", and AR9 covers only Close, Previous, Next and the Counter. Either name the caption's backing in FR17 (or FR1, which already says "on a scrim") or drop it.

write(draggedTransform(geometry, raw));
return;
}
const range = restRange(geometry, raw.scale);

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.

Without the touch presentation, the pan clamps here and the release below coasts at zero velocity, so a mouse, pen or trackpad pan stops dead at the photo's edges. spec:AST-094 FR43 gives those pointers the same rubber band and release spring as a finger, and component:Lightbox DEC-2 adopts FR43. Add them, or record the gap and amend FR43.

* 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 (lines 366-385), so this sentence is stale. Suggested: "Past a limit, the return springs from the pinch's scale velocity, anchored at the last centroid."

@vjeux
vjeux force-pushed the uikit/c13-lightbox-caption-compare branch from 4c07f5c to 8929f6c Compare November 5, 2026 18:11

@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.

This preview needs an engineering owner's decision before the review can finish: it is marked not for merge, yet it carries #9810's whole branch plus two story-only props, captionType and captionColor, on the exported LightboxProps. If it closes after the caption type pick, as the description says, nothing here needs action on this pull request. If it is meant to merge, an engineering owner (the author may, as one) records the decision by commenting /approve-spec <full head SHA> for component:Lightbox FR1, FR11, the new FR17 and AR9 with their verification rows and Known gaps, and spec:AST-038's Lightbox row, and rules on whether the two props may ship and on the spec:AST-094 FR43 conflict below.

Blocking before it could merge:

  • In a hasZoom gallery, an arrow key pressed while the photo or "more" has focus shows the next item's photo while the counter and announcement name the current one, because the refocus at Lightbox.tsx:1106 scrolls the viewport a page before the pager moves; focusForLastInput(target, {preventScroll: true}) fixes it.
  • At 10,303 authored lines, 73 of them the preview's own, merging would land #8147, #8148, #8197 and #9810 under a preview's title, so close it after the pick or base it on #9810's branch.
  • The capture pointerdown listener in useZoomStage.ts (lines 289-311) sets the shared pan's start threshold from outside PanGestureRecognizer, which already takes hysteresis as a per-press function, so pass that function and delete the listener.
  • A mouse, pen or trackpad pan of a zoomed photo stops dead at the edges and at the release, where spec:AST-094 FR43 gives every pointer the same rubber band and release spring; add them or amend FR43 (this one waits on the spec decision).

[Automated review]

Comment thread packages/core/src/Lightbox/Lightbox.tsx Outdated
active.closest('[inert]') != null;
const target = isZoomTarget ? pageMediaRef.current : closeButtonRef.current;
if (isFocusDropped && target != null) {
focusForLastInput(target);

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 hasZoom gallery this focuses the new page's photo box before usePager's layout effect moves the track, so focus() scrolls the clipped viewport a page (1,320 px at 1280 px wide) and the viewer shows the item after the current one while the counter and announcement name the current one. focusForLastInput(target, {preventScroll: true}) keeps the current photo centred; the new keyboard test could also assert that the current page sits in the viewport after the key.

@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.

This preview needs an engineering owner's decision before the review can finish: it is marked not for merge, yet it carries #9810's whole branch plus two story-only props, captionType and captionColor, on the exported LightboxProps. If it closes after the caption type pick, as the description says, nothing here needs action on this pull request. If it is meant to merge, an engineering owner (the author may, as one) records the decision by commenting /approve-spec <full head SHA> for component:Lightbox FR1, FR11, the new FR17 and AR9 with their verification rows and Known gaps, and spec:AST-038's Lightbox row, and rules on whether the two props may ship and on the spec:AST-094 FR43 conflict below.

Blocking before it could merge:

  • In a hasZoom gallery, an arrow key pressed while the photo or "more" has focus shows the next item's photo while the counter and announcement name the current one, because the refocus at Lightbox.tsx:1098 goes through moveFocusLeftBehind, which focuses without preventScroll and scrolls the viewport a page before the pager moves; letting moveFocusLeftBehind pass focus options and calling it with {preventScroll: true} fixes it.
  • At 10,298 authored lines, 73 of them the preview's own, merging would land #8147, #8148, #8197 and #9810 under a preview's title, so close it after the pick or base it on #9810's branch.
  • The capture pointerdown listener in useZoomStage.ts (lines 292-311) sets the shared pan's start threshold from outside PanGestureRecognizer, which already takes hysteresis as a per-press function, so pass that function and delete the listener.
  • A mouse, pen or trackpad pan of a zoomed photo stops dead at the edges and at the release, where spec:AST-094 FR43 gives every pointer the same rubber band and release spring; add them or amend FR43 (this one waits on the spec decision).

[Automated review]

useIsomorphicLayoutEffect(() => {
const note = pagingFocusRef.current;
pagingFocusRef.current = null;
moveFocusLeftBehind(

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 hasZoom gallery this still scrolls the viewer a page: moveFocusLeftBehind focuses the new page's photo box with focusForLastInput(target) and no options, before usePager's layout effect below moves the track. ArrowRight from the focused photo or "more" then shows the third photo under a "2 / 3" counter (viewport scrollLeft 1,320 at 1280 px wide, 442 on a 402 px phone). Letting moveFocusLeftBehind forward FocusOptions to focusForLastInput and passing {preventScroll: true} here keeps the current photo centred.

vjeux added 2 commits November 5, 2026 11:27
Each page of a Lightbox gallery laid out its own photo and caption, so
a neighbour with a longer caption arrived with a smaller photo. Every
page now has a caption band two caption lines tall under its photo,
whatever the caption, so the photo area is the same on every page and
paging never resizes it.

A longer caption shows two lines and a "more" button. The button opens
the whole caption over the bottom of the photo, on the scrim the
controls use, and its "less" closes it. Paging closes it too. The
whole caption is always the viewer's accessible description, clamped
or open. The viewer lays out no hidden copies of other captions.

The viewer renders inside MediaTheme, dark, so its controls, caption and
counter take the theme's dark surface tokens and its rules for them,
with no Lightbox-owned colors beyond the controls' and the caption's
backing. The caption is body Text, both of its lines start at one edge,
and "more" and "less" are a ghost Button. The band clears the screen's
bottom edge and its safe area. Focus the viewer moves follows the last
input, so a tap that opens it paints no ring.

The caption sits on its own backing, the controls' backing colour at
90 %, so it keeps 4.5:1 over any photo or backdrop behind it. With
zoom, the controls' icons and the Counter's text keep the on-dark
colour, whatever a theme does to buttons or Text. An open caption is
the clamped caption itself, unclamped over the photo's bottom, so the
caption renders once. In a gallery the keyboard's zoom button is the
current page's photo box, so the caption band and its button sit
outside it, and Enter or Space on "more" toggles the caption. Paging
from a focused "more", "less" or photo moves focus to the new page's
photo, or to Close without zoom, so the arrow keys keep paging. "Less"
keeps its focus.
A preview for meta#9810's caption type pick. The same captions open in
the body type, in the supporting type, and in the supporting type in
the primary color, each in a band two lines of its own type tall.

The story-only captionType and captionColor props exist only for this
comparison and never ship. This change closes after the pick.

@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.

This preview needs an engineering owner's decision before the review can finish: it is marked not for merge, yet it carries #9810's whole branch plus two story-only props, captionType and captionColor, on the exported LightboxProps. If it closes after the caption type pick, as the description says, nothing here needs action on this pull request. If it is meant to merge, an engineering owner (the author may, as one) records the decision by commenting /approve-spec <full head SHA> for component:Lightbox FR1, FR11, the new FR17 and AR9 with their verification rows and Known gaps, and spec:AST-038's Lightbox row, and rules on whether the two props may ship and on the spec:AST-094 FR43 conflict below.

Blocking before it could merge:

  • In a hasZoom gallery, an arrow key pressed while the photo or "more" has focus shows the next item's photo while the counter and the dialog's name give the current one, because the refocus at Lightbox.tsx:1097 goes through moveFocusLeftBehind, which focuses without preventScroll and scrolls the viewport a page before the pager moves; letting moveFocusLeftBehind pass focus options and calling it with {preventScroll: true} fixes it.
  • At 10,328 authored lines, 73 of them the preview's own, merging would land #8147, #8148, #8197 and #9810 under a preview's title, so close it after the pick or base it on #9810's branch.
  • The capture pointerdown listener in useZoomStage.ts (lines 292-311) sets the shared pan's start threshold from outside PanGestureRecognizer, which already takes hysteresis as a per-press function, so pass that function and delete the listener.
  • A mouse, pen or trackpad pan of a zoomed photo stops dead at the edges and at the release, where spec:AST-094 FR43 gives every pointer the same rubber band and release spring; add them or amend FR43 (this one waits on the spec decision).

The close path's new focus return without scrolling matches the overlay-dismissal family's FR9.

[Automated review]

@vjeux vjeux closed this Nov 6, 2026
@vjeux
vjeux deleted the uikit/c13-lightbox-caption-compare branch November 6, 2026 05:29
@vjeux
vjeux restored the uikit/c13-lightbox-caption-compare branch November 6, 2026 10:42

This branch was successfully deployed

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

Labels

CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review needs:spec-owner-review Current knowledge records await owner approval

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants