Skip to content

Commit d121e06

Browse files
CGastrellclaudedhasilva
authored
Protect notices: announce errors to screen readers (#52167)
* Protect notices: back the notice component with the @wordpress/ui Notice The hand-rolled chassis — an icon switch, a message div and a dismiss button over a dark banner — becomes `Notice.Root` with a `Notice.Description` and a `Notice.CloseIcon`. The public props are unchanged, so all three call sites keep working: `type` picks the intent, `message` fills the description, `dismissable` draws the close control, and `duration` keeps its own timeout. `type="warning"` now reaches an intent of its own. The old switch had no case for it, so the fix-threat modal's inactive-extension warning fell through to the info icon over the default chassis. Only the floating toast passes a `spokenMessage`. It appears without a focus change and no call site wraps it in a live region; the other two sit inside a modal that is announced when it opens. The stylesheet keeps the floating placement, the info notice's bottom margin and the stacking context, and loses everything that painted the old chassis. `container-type: normal` is new: `Notice.Root` sets `container-type: inline-size`, and a size-contained child reports no width to the shrink-to-fit fixed box, which collapsed the toast to 26px. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: keep the notice silent and its dismiss label The Protect e2e suite failed on both counts, and both are this PR's doing. `spokenMessage` defaulted the toast into `speak()`, which copies the message into `#a11y-speak-polite` — so `getByText( 'Changes saved' )` matched two nodes and Playwright's strict mode rejected it. The legacy notice never announced and no call site sits in a live region, so this restores that. Announcing a toast is a real improvement, but it belongs in its own change, applied consistently. `Notice.CloseIcon`'s default label is "Dismiss", which renamed the control the suite clicks. The existing translated string is passed through instead, so the accessible name users already have survives the chassis swap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: let the toast announce, and follow the three review notes `speak()` keeps its own live region, so whether a call site sits in one was never the right test — the reason the previous commit gave was wrong. The toast reports an async result with no focus change, which is what `speak()` is for, so it announces again; the two modal notices stay quiet, since their dialog is read when it opens. That is what broke the e2e suite: `speak()` copies the message into `#a11y-speak-polite`, so `getByText( 'Changes saved' )` matches twice. Verified in a browser that the notice is the earlier of the two, and scoped the assertion to it. Also: capitalise the changelog entry after its prefix and drop the repeated word, and add `warning` to the component README's list of types, which this change makes a real intent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: never hand a JSX message to the announcer `Notice.Root` renders `spokenMessage` to a string during its own render. When the message holds a component with hooks — every error notice interpolates a `Link` — that component's hooks land in `Notice.Root`'s hook list, and the `useEffect` on the next line reads a mismatched slot and throws. React then unmounts the whole Protect dashboard, so any failed request blanked the page. `safeRenderToString`'s try/catch does not help: the throw comes from the hook after it. Only a plain string is announced now, which covers the saved and saving toasts. Announcing errors needs a spoken string carried alongside the JSX; that is worth doing separately. The same trap is recorded in `packages/newsletter/_inc/subscribers/components/modals/add-subscribers-modal.tsx`. Two follow-ups from the same review: the e2e assertion is scoped to the app root, because `speak()` leaves its text in the live region after the notice goes and the helper runs seven times; and `NoticeState.type` gains `warning`, which the component and README already offer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: bundle the two packages the close control pulls in `Notice.CloseIcon` reaches `@wordpress/theme` and `@wordpress/private-apis` through `IconButton` and its tooltip, so the build started externalising them to the `wp-theme` and `wp-private-apis` script handles. Protect registers no shim for those, and WordPress refuses to enqueue a bundle with an unmet dependency — the admin page would render empty wherever core does not supply them. They are bundled instead, which is what the Jetpack plugin's own admin bundles do for the same reason. Verified by rebuilding: both handles are gone from `build/index.asset.php`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: correct a comment the e2e scoping outdated The Firewall block still said the "Changes saved" assertion stays page-scoped on purpose. It does not any more. The changelog entry becomes `minor`, matching the same migration for the Jetpack plugin in #52015: every Protect notice is restyled, one turns amber, and the toasts start announcing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: announce error notices to screen readers The announcement was derived from the message, and only a plain string was ever handed to `Notice.Root` — anything else corrupts its hook order. `showErrorNotice` always builds JSX, because it interpolates a support link, so failed saves stayed silent while "Changes saved." did not. Carry the announcement in the notice state instead: `showErrorNotice` sets `spokenMessage` from the plain string it already receives, and the component prefers it over the message. It still resolves to null rather than undefined, so `Notice.Root` never falls back to rendering children. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: speak the whole error, and speak it every time The announcement dropped the "Please try again or contact support." sentence because it only exists with interpolation tags. Stripping the tags off the translated string gives the plain sentence at no translation cost, so the announcement now carries what the notice says. Each notice also gets an identity, and the toast is keyed on it. Without that a repeat of the same error re-renders with identical props, the announcing effect never re-runs, and the second failure is silent. `use-fixers-query` is the path that reaches it: its polling error fires the same message with no other notice in between. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: re-announce without stealing focus Keying the toast on the notice id remounted it, which destroyed the node focus was in: a keyboard user reading the error, with focus on its support link or dismiss button, lost focus to the body when the next notice arrived. In an accessibility change that is not a fair trade. The announcement now varies instead of the element. A repeat alternates a trailing non-breaking space — the same device `@wordpress/a11y` uses for this — so the announcing effect re-runs while the element stays mounted. Two identical errors in a row take consecutive ids, so the parity always flips between them; anything that arrives in between announces on its own text. Two more from the same review: the provider stamps the id, so no future writer can forget it and silently break the repeat, and the tag strip no longer names `supportLink`, so renaming the tag cannot silently stop it working. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: announce per notice, not per distinct message The parity nonce was wrong. `Notice.Root` announces when the value it is handed changes, so the previous revision alternated a trailing space keyed on the notice id — but the id advances once per stored notice while the announcement happens once per committed render. Any event that stores an even number of notices in one commit leaves the parity where it was, and the repeat is silent again. `use-fixers-query` is exactly that shape: one instance per threat row plus the modals, all sharing a query, all raising the same error in one commit. So the announcement moves here, keyed on the notice, and `Notice.Root` is handed `null` unconditionally — it never renders a message to a string now, which is what the crash came from. `speak()` handles an identical repeat itself. Two more from the same review: the id is stamped outside the state updater, and the component README documents `spokenMessage` and `id`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: tie the dismiss timer to the notice as well `duration` keyed its timer on the message, so a second identical success inherited whatever was left of the first one's 7.5 seconds instead of starting again. The notice's id is what says "this is a different notice", and the announcement already uses it. Also from review: the mid-render rationale is stated once, on the prop that carries it, and the new dependency is filed in alphabetical order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: drop a comment's claim of a nonexistent dialog The comment above `spokenMessage` said the modal notices are read when their dialog opens. Protect has no dialog: `components/modal/index.jsx` renders a plain `<div>` with no `role="dialog"`, no `aria-modal` and no focus move into the window, so nothing reads those notices. Keep the half that is true and load-bearing -- why the prop takes strings only -- and drop the rest. Also cut the `requestMap` comment in the plugin's webpack config to the live constraint. The mechanism is already spelled out in three other configs, and a fourth copy drifts. The code is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: keep the joint-bundling constraint on the requestMap The pair has to be bundled together: `@wordpress/theme` opts in and calls `lock()` at module scope, and `@wordpress/private-apis` keeps its consent map per module instance. Bundle one and externalize the other and the two maps diverge, which surfaces at runtime as "Cannot unlock an object that was not locked before" or a duplicate opt-in, with nothing pointing back at the build config. The full account lives in #48173. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: point at the requestMap rationale instead of restating it Three other webpack configs already carry this reasoning in full. A fourth copy is the one outcome that guarantees they drift, so this leaves the constraint and the reference and nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: name the tag the spoken message strips CodeQL reads a `<[^>]+>` strip as HTML sanitization and files it high severity (js/incomplete-multi-character-sanitization). Nothing here reaches innerHTML — the result goes to `speak()`, which writes textContent — but the generic form buys nothing over naming the one placeholder the string actually carries, and `createInterpolateElement` breaks just as loudly if that name ever changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Protect notices: answer the review on the announcement Rename the per-notice stamp from `id` to `noticeId`. It is a sequence number rather than an identity, and `id` would have become a real DOM attribute the day `Notice.Root` is handed rest props. Say why only the toast announces, and drop the comparison with `Notice.Root`'s own announcement: WordPress/gutenberg#82737 proposes removing that, so the comparison would describe a mechanism a reader cannot reach. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lo29yrft9KqShYK3p1wQe6 * Protect notices: announce a failing fixers poll once per streak Every failed poll produces a new Error object, so keying the effect on it re-announced the same message assertively every five seconds for as long as the poll kept failing. Key on `isError` instead: one announcement per streak, and another only once a success has come in between. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lo29yrft9KqShYK3p1wQe6 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Douglas <douglas.henri@automattic.com>
1 parent c38909e commit d121e06

7 files changed

Lines changed: 66 additions & 28 deletions

File tree

‎pnpm-lock.yaml‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: patch
2+
Type: fixed
3+
4+
Notices: Announce errors to screen readers.

‎projects/plugins/protect/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
"@automattic/jetpack-connection": "workspace:*",
3333
"@automattic/jetpack-scan": "workspace:*",
3434
"@tanstack/react-query": "5.102.8",
35+
"@wordpress/a11y": "4.54.0",
3536
"@wordpress/api-fetch": "7.54.0",
3637
"@wordpress/components": "40.0.0",
3738
"@wordpress/compose": "8.7.0",

‎projects/plugins/protect/src/js/components/notice/README.md‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,11 @@ A simple notice component for displaying alerts and messages to the user.
55
## Usage
66

77
```jsx
8-
<Notice
9-
type="info"
10-
message="Code is poetry."
11-
/>
8+
<Notice type="info" message="Code is poetry." />
129
```
1310

1411
## Props
1512

16-
Supported `type` values are `info`, `success`, `warning`, and `error`.
13+
Supported `type` values are `info`, `success`, `warning`, and `error`.
14+
15+
`spokenMessage` is the plain-text announcement for screen readers, used when `message` carries markup. Only `floating` notices are announced. `noticeId` changes on every notice, so an identical repeat announces again.

‎projects/plugins/protect/src/js/components/notice/index.jsx‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { speak } from '@wordpress/a11y';
12
import { __ } from '@wordpress/i18n';
23
import { Notice as WPNotice } from '@wordpress/ui';
34
import clsx from 'clsx';
@@ -17,6 +18,8 @@ const Notice = ( {
1718
duration = null,
1819
floating = false,
1920
message,
21+
noticeId = 0,
22+
spokenMessage,
2023
type = 'success',
2124
} ) => {
2225
const { clearNotice } = useNotices();
@@ -25,6 +28,16 @@ const Notice = ( {
2528
clearNotice();
2629
}, [ clearNotice ] );
2730

31+
const spoken = spokenMessage ?? ( 'string' === typeof message ? message : null );
32+
33+
// Keyed on `noticeId`: effect deps compare by value, so an identical repeat would not re-fire.
34+
useEffect( () => {
35+
// Toast only: notices rendered inside a modal are not announced today.
36+
if ( floating && 'string' === typeof spoken ) {
37+
speak( spoken, 'error' === type ? 'assertive' : 'polite' );
38+
}
39+
}, [ noticeId, floating, spoken, type ] );
40+
2841
/**
2942
* Clears the notice automatically after {duration} milliseconds.
3043
*/
@@ -36,7 +49,7 @@ const Notice = ( {
3649
}
3750

3851
return () => clearTimeout( timeout );
39-
}, [ clearNotice, duration, message ] );
52+
}, [ clearNotice, duration, noticeId ] );
4053

4154
return (
4255
<WPNotice.Root
@@ -46,9 +59,9 @@ const Notice = ( {
4659
styles[ `notice--${ type }` ],
4760
floating && styles[ 'notice--floating' ]
4861
) }
49-
// `Notice.Root` serializes `spokenMessage` mid-render, so a JSX message runs its
50-
// hooks inside Root and corrupts hook order. Strings only, and only for the toast.
51-
spokenMessage={ floating && 'string' === typeof message ? message : null }
62+
// Null, not omitted: the default is the children, which `Notice.Root` serializes
63+
// mid-render, corrupting hook order. Drop with the prop (WordPress/gutenberg#82737).
64+
spokenMessage={ null }
5265
>
5366
<WPNotice.Description>{ message }</WPNotice.Description>
5467
{ dismissable && (

‎projects/plugins/protect/src/js/data/scan/use-fixers-query.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,14 +144,15 @@ export default function useFixersQuery( {
144144
enabled: isRegistered,
145145
} );
146146

147-
// Handle error if present in the query result
147+
// Once per error streak, not per failed poll: each failure is a new Error object, and
148+
// polling one every 5s would interrupt the screen reader with the same message each time.
148149
useEffect( () => {
149-
if ( fixersQuery.isError && fixersQuery.error ) {
150+
if ( fixersQuery.isError ) {
150151
// Reset the query data to the initial state
151152
queryClient.setQueryData( [ QUERY_FIXERS_KEY ], initialData );
152153
showErrorNotice( __( 'An error occurred while fetching fixers status.', 'jetpack-protect' ) );
153154
}
154-
}, [ fixersQuery.isError, fixersQuery.error, queryClient, showErrorNotice ] );
155+
}, [ fixersQuery.isError, queryClient, showErrorNotice ] );
155156

156157
return fixersQuery;
157158
}

‎projects/plugins/protect/src/js/hooks/use-notices.tsx‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import type { Dispatch, FC, ReactNode, SetStateAction } from 'react';
88

99
interface NoticeState {
1010
message?: string | JSX.Element;
11+
spokenMessage?: string;
12+
// Stamped by the provider on every notice, so an identical repeat still announces.
13+
noticeId?: number;
1114
dismissable?: boolean;
1215
duration?: number;
1316
type?: 'success' | 'info' | 'warning' | 'error';
@@ -18,10 +21,21 @@ interface NoticeContextValue {
1821
setNotice: Dispatch< SetStateAction< NoticeState > >;
1922
}
2023

24+
let lastNoticeId = 0;
25+
2126
const NoticeContext = createContext< NoticeContextValue | undefined >( undefined );
2227

2328
export const NoticeProvider: FC< { children: ReactNode } > = ( { children } ) => {
24-
const [ notice, setNotice ] = useState< NoticeState >( null );
29+
const [ notice, setNoticeState ] = useState< NoticeState >( null );
30+
31+
const setNotice: Dispatch< SetStateAction< NoticeState > > = useCallback( value => {
32+
const noticeId = ++lastNoticeId;
33+
34+
setNoticeState( previous => {
35+
const next = 'function' === typeof value ? value( previous ) : value;
36+
return next ? { ...next, noticeId } : next;
37+
} );
38+
}, [] );
2539

2640
return (
2741
<NoticeContext.Provider value={ { notice, setNotice } }>{ children }</NoticeContext.Provider>
@@ -66,27 +80,30 @@ export default function useNotices() {
6680

6781
const showErrorNotice = useCallback(
6882
( message: string ) => {
83+
const error = message || __( 'An error occurred.', 'jetpack-protect' );
84+
const advice = __(
85+
'Please try again or <supportLink>contact support</supportLink>.',
86+
'jetpack-protect'
87+
);
88+
6989
setNotice( {
7090
type: 'error',
7191
dismissable: true,
92+
// The same translated string, minus the one interpolation tag it carries.
93+
// Strip it by name: a generic tag strip trips CodeQL's sanitization rule.
94+
spokenMessage: `${ error } ${ advice.replace( /<\/?supportLink>/g, '' ) }`,
7295
message: (
7396
<>
74-
{ message || __( 'An error occurred.', 'jetpack-protect' ) }{ ' ' }
75-
{ createInterpolateElement(
76-
__(
77-
'Please try again or <supportLink>contact support</supportLink>.',
78-
'jetpack-protect'
97+
{ error }{ ' ' }
98+
{ createInterpolateElement( advice, {
99+
supportLink: (
100+
<Link
101+
openInNewTab
102+
href={ hasPlan ? PAID_PLUGIN_SUPPORT_URL : FREE_PLUGIN_SUPPORT_URL }
103+
children={ null }
104+
/>
79105
),
80-
{
81-
supportLink: (
82-
<Link
83-
openInNewTab
84-
href={ hasPlan ? PAID_PLUGIN_SUPPORT_URL : FREE_PLUGIN_SUPPORT_URL }
85-
children={ null }
86-
/>
87-
),
88-
}
89-
) }
106+
} ) }
90107
</>
91108
),
92109
} );

0 commit comments

Comments
 (0)