Repository navigation
[Component Audit] Lightbox: screen readers hear the open viewer, and 0 resets the zoom - #10775
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| Lightbox · Storybook | 122.6 kB (+66 B) · 10.6 kB CSS (0) | 3,412 (+35) | ✓ · ✓ · ✓ |
| lab/CommentThread | 344.5 kB (+40 B) · 2.4 kB CSS (0) | — | — · — · — |
| lab/Comments | 351.3 kB (+35 B) · 2.5 kB CSS (0) | — | — · — · — |
Package size: @astryxdesign/core 7.21 MB packed (+388 B); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 29 rows, core 36 rows, theme-butter 1 row, theme-y2k 1 row; run pnpm api:report.
Touched components checked; add ci:run-visual to run the full suite.
Generated by PR Enrichment workflow | View full report
cixzhang
added
ci:run-a11y
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes. Both fixes work: the position and zoom messages now land in the open viewer's live region instead of the page's, and 0 returns a zoomed photo to its fitted size. The new tests fail on the base and pass on this head. One thing to change first:
ViewerAnnouncerand its ref wrapper (Lightbox.tsx:479-482,:617-620) repeat the announcer handoffPowerSearchBottomSheet.tsxalready has (SheetAnnouncer,:589-604,:993-999), so please call the sharedannounceInModal(dialogRef.current, message)fromhooks/useAnnounce.ts, asToastViewportanduseBusyAnnouncementdo, or move one shared announcer intohooks/and use it from both components.
[Automated review]
3e0666b to
b565892
Compare
There was a problem hiding this comment.
Requesting changes. The fix itself works: the position and zoom messages now land in the open viewer's live region instead of the page's, and 0 returns a zoomed photo to its fitted size; the new tests for both fixes fail on the base and pass on this head. One change is still needed:
ViewerAnnouncerand its ref wrapper (Lightbox.tsx:479-482,:617-620) repeat the announcer handoffPowerSearchBottomSheet.tsxalready has (SheetAnnouncer,:589-604,:993-999), so please call the sharedannounceInModal(dialogRef.current, message)fromhooks/useAnnounce.ts, asToastViewportanduseBusyAnnouncementdo, or move one shared announcer intohooks/and use it from both components.
[Automated review]
There was a problem hiding this comment.
Approved pending CI: on this push the required test, build and docsite-test checks are deferred, and they run in the pre-merge re-run.
Both fixes now do what the Lightbox contract asks: paging and zoom messages land in the open viewer's own live region instead of the page behind the modal, and 0 returns a zoomed photo to its fitted size. The new unit tests for both fail before the change and pass after it. Routing the messages through the shared announceInModal resolves the earlier request to avoid a second private announcer.
[Automated review]
User impact
A screen-reader user now hears the viewer while it is open:
0key returns a zoomed photo to its fitted size, as the viewer's keyboard contract promises. Before, it did nothing. Ctrl+0 and Cmd+0 still reset the browser's page zoom.Nothing changes on screen: frames before and after match, except the frame taken after pressing
0, which now shows the fitted photo instead of the zoomed one (and the browser's own loading spinner in the empty video player, which moves between runs).Builders get docs that match the component:
hasZoomlists the0key, and the guidance no longer says the lightbox is only for images (it shows images and videos).Audit result
core/Lightbox(released in@astryxdesign/coreunder./Lightbox)1.16.6fe174dd1213(rebased after the LayoutPanel audit landed; the change set is unchanged)ec3f47f3206e0af20dbfb26eeb18704b9b44f907Lightbox.spec.md(authority: current), unchanged. AR3 requires0to return to the fitted size, and AR4 withspec:AST-086FR14 requires the position and zoom messages to be delivered inside the open viewer; the record lists both as implementation gaps (Current-state impact). This pull request closes both in code. No observational backfill was prepared.announceInModal) with its own dialog, as other modal owners do, and0without Ctrl, Cmd, or Alt zooms out like-. No prop, default, target, or visual change.Sensitivity: fix the open BLOCK (Needs Review 1) and change nothing else → projected 92.0 / A.
spec:AST-086FR14), and0did nothing (AR3). Open FIX: the zoom message does not name the zoom level (AR4; Needs Review 2). Modal dialog named by the active item, focus inside, Tab contained (FR4), Escape closes and focus returns to the opener; controls 32px; the audit's axe pass over the open viewer reports no violations (axe does not measure the caption over the translucent backdrop; see Needs Review 1)lightboxtarget on the dialog; controls delegate to Button'sbuttontarget; colours, spacing, radius, and type from tokens; the zoomable controls sit on Lightbox's own on-light backing with on-dark icons (AR9); Media, Caption, and Counter have no target, as the record statesisOpen/onOpenChange,index/defaultIndex/onIndexChange,hasZoom,hasAutoPlay; rest andrefreach the dialog0and for in-viewer delivery were missing downstream of the §1 defects; they now exist as unit cases (position and zoom routing,0, Ctrl/Cmd+0) and in the keyboard browser spec that runs on every pull requestdblclicklistener and the viewer'spointerdownpress tracker sit outside the shared touch modules. The position message's Effect fires once per index the viewer actually shows, which a controlledindexdecides, so it is not handler workhasZoomnow lists0§5b is scored before from frames of the fixture commit (component unchanged from base), so the open contrast BLOCK counts on both sides. §11 is not run for an existing core component.
Why the score moved: the two §1 BLOCKs close, and the docs FIX closes. The grade stays capped at C by the open contrast BLOCK, which needs an owner decision.
Findings fixed
useAnnounceabove its own dialog's announcement provider, so its position and zoom messages went to the page's shared channel, which a screen reader does not read while the modal viewer is opencomponent:LightboxAR4;spec:AST-086FR14announceInModal. Red, then green: the new unit case fails at the fixture commit (no message in the dialog) and passes after; in real Chromium the audit receipts at the fixture commit find "Mountain lake, 2 of 4" and "Zoomed in, use arrow keys to pan" in the page's region and none in the viewer's, and after the fix find both in the viewer's region and neither on the page0did not return a zoomed photo to its fitted sizecomponent:LightboxAR30without a modifier zooms out like-; Ctrl/Cmd+0 stays the browser's page-zoom reset. Red, then green: the new unit case and the browser receipts (zoom still pressed after0) fail at the fixture commit and pass after; the keyboard browser spec now checks+, the in-viewer message, and0hasZoomdocs and JSDoc did not list0Open Viewerstory, open on load;.github/a11y-audit-axe.jsonpoints the audit's axe pass at it and waits fordialog[open]Needs Review
--color-on-darkdirectly on the--color-overlaybackdrop. Over a white page in light mode, the measured composite is 4.00:1 in Neutral and 3.33:1 in Butter for 17px caption text, under WCAG 2.2 1.4.3's 4.5:1 (D7); the core default overlay with no theme computes to 2.63:1. Dark mode measures 20.5:1. A gallery withouthasZoomputs its Counter on the same pair (not measured here). The remedy is a visual decision for the owner: for example a darker viewer backdrop, or the Counter's backing behind the caption.Lightbox.spec.mdCurrent-state impact still lists the0key and the delivery outside the viewer, which this pull request fixes, and Tab containment (FR4), whichuseFocusRegionContainmentalready provides and the keyboard browser spec checks. A later owner-approved record change can remove them.dblclicklistener and press tracker, as the record lists.-, and now0, pressed while a drag pans a zoomed photo zoom out, and the drag's next move can restore the zoomed scale. Not scored here.+or-(observation, pre-existing). While a zoomable photo is open these zoom the photo instead of the page. No current record decides this; Ctrl/Cmd+0 is left to the browser.Evidence
Open Viewerstory for the axe pass. The audit spec drives the existingGallery with Zoom,Default, andVideostories with real keys, replaces the remote photos with one local picture so frames repeat exactly, and aborts the remote video.5d39f03(component unchanged from base), fails only the fiveGallery with Zoomcases (Neutral light and dark, Butter, RTL, 320px), each on the same four checks (spec as of the fixture run): the position message is on the page and not in the viewer, the zoom message is not in the viewer, and the zoom stays pressed after0. TheDefaultandVideocases pass.ci:run-a11yandci:run-rtl: all 8 audit cases pass in artifactpr-a11y-evidence-ec3f47f-1(sha256:7192aebf7720ae6adc67a613f8790620ac7a24e259cb3c9cc74f7b683096ccfa), each receipt bound to buildec3f47f; the audit's axe pass over the open viewer reports no violations; pr-rtl, Stable visual regression, and the keyboard browser spec green. Locally all 8 cases pass with receipts bound to the head. Against the red-before build, the frames match except the fivefittedframes (now the fitted photo, as0intends), the Video frame, whose only changed pixels are the browser's own loading spinner over the empty player, and the first frame in Butter and at 320px, where the photo's edge lands one CSS pixel apart; every geometry and state check passes in all of them.astryx-lightboxtarget, in the page direction, named by the active item, with focus inside and a Close button; Escape closes it and focus returns to the opener;Gallery with Zoom: counter "1 / 4"; Previous disabled at the start; Next at the inline end (the right in LTR, the left in RTL); Close, Previous, and Next at 32 × 32px; paging announces "Mountain lake, 2 of 4" inside the viewer and nothing on the page;+zooms, keeps focus inside, and announces inside the viewer and not on the page;0returns to the fitted size;Default: no gallery buttons or counter, caption shown, no zoom target;Video: native controls, no zoom target.0now reaches, shown in thefittedframes.Closed evidence inventory
Lightbox,LightboxProps,useLightboxLightbox/index.tsLightbox.doc.mjs,useLightbox.doc.mjsarchitecture:public-component-apilightboxtarget, Media, CloseLightbox.tsxLightbox.tsxmedia,indexGallery with ZoomcasesLightbox.tsxmedia.captionDefault, gallery first frameLightbox.tsxmedia.typeVideocaseLightbox.tsx,useZoomCore.ts,useZoomStage.ts0case (new); zoom browser specshasZoomLightbox.tsxspec:AST-086FR14Lightbox.tsxfamily:overlay-dismissalLightbox.tsx, touch moduleshasZoomLightbox.tsxLightbox.tsxdesign:bidirectionalityLightbox.tsxFR10 eligibility report
{ "schema": "astryx.audit-eligibility/v1", "component": "Lightbox", "package": "core", "mode": "night-watch", "rubricVersion": "1.16.6", "repository": { "base": "fe174dd1213", "head": "ec3f47f3206e0af20dbfb26eeb18704b9b44f907" }, "contract": { "id": "component:Lightbox", "authority": "current", "changed": false }, "inventory": { "closed": true, "rows": 12, "open": ["touch presentation partial"] }, "unresolved": { "objective": [ "zoom message does not name the zoom level (AR4)" ], "manual": [ "D7: caption text on the light-mode backdrop under 4.5:1; remedy is a visual decision", "record follow-up: Current-state impact lists gaps now closed" ], "ownedElsewhere": [ "touch presentation FR5, FR8, FR9, FR10, FR12, FR16 under spec:AST-094", "FR14 listeners outside the shared touch modules" ] }, "remediations": [ {"id": "A6/AR4", "kind": "bug-fix", "evidence": "in-viewer unit case and Chromium receipts red at the fixture commit, green at head"}, {"id": "A4/AR3", "kind": "bug-fix", "evidence": "0-key unit case and Chromium receipts red at the fixture commit, green at head"}, {"id": "X3/X21", "kind": "doc-drift", "evidence": "guidance names images and videos; hasZoom lists 0"} ], "approvals": { "specOwner": "success (no record change)" }, "checks": "green at ec3f47f (CI run 42631047655, attempt 2)", "eligible": false, "reasons": [ "spec:AST-029 is phase accepted; FR11 activation has not shipped", "an open BLOCK remains (D7)" ] }