Repository navigation
fix(lab): MobilePowerSearch names its trigger by what it shows and announces inside its sheets - #8007
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📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsMobilePowerSearch (@astryxdesign/lab) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: the trigger now carries its visible text in its name and announcements reach the open sheet, but the add confirmation is lost whenever resultCount changes in the same commit as the add. In announce (MobilePowerSearch.tsx:569-571), the count's call cancels the add's pending frame before it writes, so a consumer that derives resultCount from filters hears only the new count, never Added "…" (spec:AST-086 FR6, once per occasion). Please append a message that arrives while a frame is pending to the pending text instead of cancelling it, and add a regression test with a wrapper whose resultCount follows filters. Separately, the header note's AST-086 FR10 citation covers only owners rehosted above native modals; Platform support and FR3 are the claims that apply here.
[Automated review]
| if (sheetAnnouncementFrameRef.current != null) { | ||
| cancelAnimationFrame(sheetAnnouncementFrameRef.current); | ||
| } |
There was a problem hiding this comment.
Cancelling the pending frame drops a message that has not been written yet. That happens on a content-search add whenever the consumer derives resultCount from filters: onChange re-renders the parent, the result-count Effect calls announce in the same commit, and only the count is ever written, so Added "…" is never announced (spec:AST-086 FR6, once per occasion). Consider appending to the pending text instead (the join FR9 uses for the shared channel), for example by keeping pending messages in a ref and writing messages.join(' ') when the frame fires, plus a regression test with a wrapper whose resultCount follows filters.
f55f90e to
b8fd1b6
Compare
There was a problem hiding this comment.
Requesting changes: the trigger now carries its visible text in its accessible name, the result count no longer announces on a StrictMode mount, and the add confirmation and a same-commit count change are both heard inside the open sheet. One thing remains: the new in-sheet status region is named by concatenating the label with a literal (aria-label={`${triggerLabel} status`} in MobilePowerSearch.tsx), which the internationalization architecture's INV1a rules out for Astryx-owned assistive text. Dropping the aria-label (a polite status region needs no name) and finding the region by its existing data-testid in the tests would resolve it, as would a catalog message.
[Automated review]
| role="status" | ||
| aria-live="polite" | ||
| aria-atomic="true" | ||
| aria-label={`${triggerLabel} status`} |
There was a problem hiding this comment.
This name is Astryx-owned assistive text assembled from the label and a literal, which architecture:internationalization INV1a rules out (“it MUST NOT be assembled at the call site from literals, concatenation”); in a localized app the region would be named by the translated label plus an English “status”. A polite status region needs no name to announce, so dropping aria-label and finding the region by its existing data-testid in the tests would resolve it; a catalog message would also work.
b8fd1b6 to
f84eb42
Compare
There was a problem hiding this comment.
Recommending approval: the trigger now carries its visible text in its accessible name, the result count stays silent on a StrictMode mount, and the add confirmation and a same-commit count change are announced together inside the open sheet, whose status region no longer carries an assembled name. One optional hardening: the unmount-only cleanup in MobilePowerSearch.tsx cancels the sheet announcer's frame and timer but leaves their refs set, so a React <Activity> hide during a pending write would silence later in-sheet announcements; resetting both refs and the pending queue there covers it. The description's Evidence section still calls the region “Manage filters status”, a name it no longer has.
[Automated review]
| useEffect( | ||
| () => () => { | ||
| if (sheetAnnouncementFrameRef.current != null) { | ||
| cancelAnimationFrame(sheetAnnouncementFrameRef.current); | ||
| } | ||
| if (sheetAnnouncementClearRef.current != null) { | ||
| clearTimeout(sheetAnnouncementClearRef.current); | ||
| } | ||
| }, | ||
| [], | ||
| ); |
There was a problem hiding this comment.
Non-blocking: this cleanup cancels the frame and the clear timer but leaves both refs set. When React tears Effects down without unmounting (a <Activity mode="hidden"> boundary) while a write is pending, sheetAnnouncementFrameRef.current keeps the cancelled id, so after the component is shown again every in-sheet announce returns at the pending-frame check and nothing is written. Setting both refs to null and emptying pendingSheetMessagesRef here covers it.
| // sheet is open. A count present on mount is not announced unprompted, and | ||
| // the guard compares against the last announced text rather than a mounted | ||
| // flag, so StrictMode's effect replay announces nothing either (FR6). The | ||
| // announcement acquires no resource, so there is no cleanup. |
There was a problem hiding this comment.
Nit: on the open-sheet path announce starts a frame and a 2 s timer, released by the unmount-only Effect and the next write, so “acquires no resource” is not quite accurate; naming that owner here would keep the comment true.
…nounces inside its sheets Three accessibility defects in the Lab MobilePowerSearch, all on the sheet flow's trigger and its announcements. The trigger's accessible name is now the field label followed by its visible content, the placeholder or the selected tokens, through aria-labelledby on the label and the content section; with a custom label or placeholder the name and the visible text no longer differ (WCAG 2.5.3). The result count announcement compares against the last announced text instead of a mounted flag, so the count present on mount is not announced and StrictMode's effect replay announces nothing; a real change announces once. Announcements made while a sheet is open went to the shared page-body live region, which assistive technology does not expose under a native modal dialog. Each sheet now carries a persistent, named polite status region, created empty and kept mounted, and the add announcement and a result count change that happen while a sheet is open are written there; the closed field keeps using the shared channel. Messages that arrive while a write is pending are joined in order into that write, the way the shared channel joins them, so an add and the result count it changes in the same commit are both heard.
f84eb42 to
91cd9be
Compare
fix(lab): MobilePowerSearch names its trigger by what it shows and announces inside its sheets
User impact
Someone using the Lab MobilePowerSearch on a touch layout with assistive technology:
labelorplaceholder(the content-search story: label "Manage filters", placeholder "Search issues…"), the trigger button's accessible name was the label only, so "click Search issues" did nothing (WCAG 2.5.3 Label in Name).showModal()).Problem and solution fit
Field's<label for>named the button, and the button's visible text (placeholder or tokens) was outside the name. The fix is the onecomponent:PowerSearchFR11 uses on the sheet port:FieldgetslabelID, the content section gets an id, and the button'saria-labelledbylists both, so the name is "label + what is visible". Nothing visible changes.hasMountedRefflips on the first effect run; StrictMode runs the effect, its cleanup, then the effect again, and the second run sees the flag set and announces. The guard now compares against the last announced text, initialised to the mount value (announcedResultCountRef = useRef(resultCountText)), so neither the mount nor the replay is a new occasion (spec:AST-086FR6), and a same-value update does not repeat. Same shape as core PowerSearch onfeat/powersearch-experimental-bottom-sheet.spec:AST-086Platform support: the shared channel on the page body is not exposed under a native modal, and a modal-hosted owner uses a persistent region inside that modal; FR3 (persistent, created empty), FR5 (polite for status) and FR6 (once per occasion) apply to it, with FR10's rehosted-owner containers as the analogous shape. Each sheet step now renders oneVisuallyHiddenrole="status" aria-live="polite" aria-atomicregion with no accessible name (a polite status region needs none, and a name built from the label plus a literal would not localize), mounted with the sheet and empty until a message arrives.announce()routes by the current step: a sheet open → clear the region, write on the next frame (so the same words announce again for a new occasion), clear it after 2 s (so a sheet that opens later does not hold stale text); closed → the shareduseAnnouncechannel as before. A message that arrives while that frame is pending is appended and written with it, joined in order (spec:AST-086FR9's rule for the shared channel), never cancelled: a consumer whoseresultCountfollowsfiltersre-renders in the add's commit, and both "Added "…"" and the new count are heard. (First review round: the count's call cancelled the add's pending frame, so only the count was ever written.) Only the active step's dialog is exposed (the retained one isinert/aria-hidden, closed ones aredisplay: none), so the message is heard once. Lab uses public core only (VisuallyHidden,useAnnounce);useAnnounceRenderedTextanduseBusyAnnouncementare not exported, and the core-internal helper on feat(core): Tokenizer touch sheet behind experimental.presentation #7977 is not depended on.Audit of announcements made while a sheet is open (MobilePowerSearch and Lab MobileTokenizer):
@astryx.tokenizer.tokenAdded("Added "…"")@astryx.powersearch.resultCountresultCountprop changesrole="checkbox"buttons whosearia-checkedtoggles, which the row itself conveys; its Clear all goes through anAlertDialog; nouseAnnouncecall exists in the component)MobileTokenizer's free-text "Add" row removes itself and clears the query without a spoken confirmation — that is a missing announcement, not an inaudible one, and is outside this fix; noted for the owner.
Expected behavior and authority
component:PowerSearchFR11 (sheet presentation: the trigger's name is the label followed by its visible text) on the port branch.spec:AST-086Platform support (the shared channel is not exposed under a native modal) and FR3 (a persistent container, created empty) are the claims that admit the in-sheet region; FR5 (polite), FR6 (once per occasion) and FR9's join rule govern what it says. FR10 is the analogous pattern for owners rehosted above modals, cited for shape only.component:PowerSearchFR19 (result count announced on change, not on mount).component:BottomSheetSwitcherDEC-1 (current): each step is its own native modal dialog, which is why the page-body channel is hidden from the moment the flow opens.Smallest restoration
Two files.
MobilePowerSearch.tsx: twouseIds andaria-labelledby/labelID/contentid(fix 1); the ref-compare guard (fix 2); a routedannouncewith its frame/timer refs and cleanup, onesheetStatusnode rendered in each of the three steps, a header note (fix 3). No new props, no visual change, no change toMobileTokenizer. The test file updates two exact-name assertions that encoded the old (wrong) name and adds the cases below.Evidence
setup({label: 'Search issues', placeholder: 'Search content…'})→ trigger name "Search issues", visible text "Search content…" (name ≠ visible). StrictMode render withresultCount={12}→ the page region reads "12 results" with no change (the new test fails against the old guard with exactly that message). Add a content suggestion with a sheet open → the message lands inbody > [aria-live=polite], which the open modal hides; nothing inside the sheet.aria-labelledbyof exactly two ids; StrictMode mount announces nothing, a change to 1 announces "1 result" once, the same value again announces nothing; the add is written to the unnamedrole="status"region inside the open dialog, and the page region stays empty; aresultCountchange while the sheet is open likewise lands in the sheet's region.resultCount12 → 1 still announces through the shared channel and renders "1 result"; the 80 other MobilePowerSearch tests (flow, steps, values, content search, timezone) are untouched and pass; MobileTokenizer 10/10.MobilePowerSearch.test.tsx): "announces both the add and a result count that follows the filters in the same commit, joined in the sheet region" — a wrapper derivesresultCountfromfilters; the region readsAdded "release notes" 20 results(fails on the cancelling version with20 results); "keeps the trigger's visible text inside its accessible name with a custom label and placeholder, and with tokens"; "announces a result count change through the page channel while closed, and nothing for the count present on mount, under StrictMode" (fails on the previous guard); "routes a result count change to the live region inside the open sheet, which stays mounted and empty until then"; the content-search add test now asserts the message in the sheet's region and an empty page region.No real-browser run: the three checks are DOM/accessibility-tree facts (
aria-labelledbyresolution, effect replay, which element holds the text) that jsdom proves;spec:AST-086lists DOM checks as the routing evidence and leaves spoken timing to the recommendedspec:AST-009follow-up.Scope
@astryxdesign/labis private; no prop added).Testing
The sheet announcer's teardown Effect also resets its frame and timer refs and the pending queue, so an Effect teardown that is not an unmount (a React
<Activity>hide during a pending write) cannot leave a cancelled id that would block later in-sheet announcements.