Repository navigation
Conversation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
1a940f7 to
743e48d
Compare
There was a problem hiding this comment.
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.tsadds a capturepointerdownlistener that rewrites the shared pan recognizer's start threshold on every press, so the fix belongs inPanGestureRecognizer(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 (
settleCurvesinzoomStage.ts), whilespec:AST-094FR45 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-094FR43 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}); |
There was a problem hiding this comment.
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.
| readonly scale: MotionCurve; | ||
| } { | ||
| const along = (a: number, b: number): MotionCurve => | ||
| solveAnimatedPageScroll({ |
There was a problem hiding this comment.
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'; |
There was a problem hiding this comment.
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.
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.
…'s reference resolution)
743e48d to
4c07f5c
Compare
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
There was a problem hiding this comment.
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.tsadds a capturepointerdownlistener that rewrites the shared pan's start threshold on every press, althoughPanGestureRecognizeralready takeshysteresisas 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_BACKINGinLightbox.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-094FR43 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}); |
There was a problem hiding this comment.
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)`; |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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."
4c07f5c to
8929f6c
Compare
There was a problem hiding this comment.
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
hasZoomgallery, 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 atLightbox.tsx:1106scrolls 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
pointerdownlistener inuseZoomStage.ts(lines 289-311) sets the shared pan's start threshold from outsidePanGestureRecognizer, which already takeshysteresisas 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-094FR43 gives every pointer the same rubber band and release spring; add them or amend FR43 (this one waits on the spec decision).
[Automated review]
| active.closest('[inert]') != null; | ||
| const target = isZoomTarget ? pageMediaRef.current : closeButtonRef.current; | ||
| if (isFocusDropped && target != null) { | ||
| focusForLastInput(target); |
There was a problem hiding this comment.
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.
8929f6c to
a372279
Compare
There was a problem hiding this comment.
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
hasZoomgallery, 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 atLightbox.tsx:1098goes throughmoveFocusLeftBehind, which focuses withoutpreventScrolland scrolls the viewport a page before the pager moves; lettingmoveFocusLeftBehindpass 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
pointerdownlistener inuseZoomStage.ts(lines 292-311) sets the shared pan's start threshold from outsidePanGestureRecognizer, which already takeshysteresisas 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-094FR43 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( |
There was a problem hiding this comment.
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.
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.
a372279 to
0705074
Compare
There was a problem hiding this comment.
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
hasZoomgallery, 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 atLightbox.tsx:1097goes throughmoveFocusLeftBehind, which focuses withoutpreventScrolland scrolls the viewport a page before the pager moves; lettingmoveFocusLeftBehindpass 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
pointerdownlistener inuseZoomStage.ts(lines 292-311) sets the shared pan's start threshold from outsidePanGestureRecognizer, which already takeshysteresisas 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-094FR43 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]
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
captionTypeandcaptionColorprops exist only for this comparison and never ship. Not for merge; it closes after the pick.