Protect notices: back the notice component with the @wordpress/ui Notice

[CGastrell]

Fixes JETPACK-2554

Proposed changes

• Backs Protect's hand-rolled Notice with the @wordpress/ui Notice, so Protect's notices move to the design system without touching a call site. @wordpress/ui is already a direct dependency at 0.21.0.
• type maps to intent, message to Notice.Description, dismissable to Notice.CloseIcon. The duration auto-clear timer is unchanged. The hand-rolled @wordpress/icons switch, the aria-label string and the chassis CSS (background, colours, icon box, message typography, close-button chrome) all go — 90 lines out, 30 in.
• The stylesheet keeps only what the design system does not provide: the floating toast's positioning, its z-index, and the margin-bottom under .notice--info that user-connection-needed-modal depends on.

Two things worth a reviewer's eye:

• type="warning" now looks like a warning. fix-threat-modal/index.jsx:79 passes it for an inactive extension. The old switch had no warning case, so it fell through to the info icon on the default gray chassis. @wordpress/ui has a real warning intent, so that notice is now amber. Intentional, but it is a visible change.
• container-type is reset on the floating toast. Notice.Root sets container-type: inline-size; inside a position: fixed container that shrinks to fit, a size-contained child reports no width. Measured in a headless browser: 26×120px without the override, 249×48px with it. Same trap as the Jetpack plugin's toasts in Jetpack notices: back SimpleNotice with the @wordpress/ui Notice #52015.

Four things the later commits added

• The announcer never gets a JSX message. Notice.Root renders spokenMessage to a string during its own render. Every error notice interpolates a Link, whose hooks then land in Notice.Root's hook list, and the useEffect on the next line throws — React unmounted the whole Protect dashboard on any failed request. Only a plain string is announced now, so the saved and saving toasts speak and errors stay silent. Announcing errors needs a spoken string carried alongside the JSX, which is worth doing separately.
• @wordpress/theme and @wordpress/private-apis are bundled, not externalized. Notice.CloseIcon reaches both through IconButton and its tooltip, so the build started emitting 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. webpack.config.js now bundles them, as the Jetpack plugin's and Boost's admin builds already do. That costs about 27KB gzip on top of the ~29KB the design system Notice itself adds.
• The dismiss control keeps Protect's own label. Notice.CloseIcon defaults to "Dismiss", which would have renamed the control the e2e suite clicks.
• The e2e "Changes saved" assertion is scoped to the app root, because speak() leaves its text in the live region after the notice goes and the helper runs six times.

Related product discussion/links

• Jetpack admin UI: standardize every notice and smooth the frame transitions
• Jetpack notices: back SimpleNotice with the @wordpress/ui Notice #52015 did the same swap for the Jetpack plugin's SimpleNotice.
• This changes the notice chassis only. Protect keeps its own useNotices React context rather than sharing a toast surface, which is JETPACK-2556's decision, not this PR's.

Does this pull request change what data or activity we track or use?

No.

Testing instructions

On a connected site with Jetpack Protect active, running this branch.

• The toast, which is the main path. Go to Jetpack Protect → Settings and toggle Account protection. A green success toast reading "Changes saved." should appear top right — sized to its message, not collapsed to a narrow sliver, and dismissable via its close icon. It should clear itself after a few seconds. The collapse is the regression risk; if the toast renders as a tall thin strip, the container-type reset is not applying.
• The saving state. Throttle the network and toggle again: an info toast reads "Saving Changes…" while the request is in flight, with no close icon.
• The error state. Ignore a threat (or save a Firewall setting) with the REST API failing — an error toast appears with a support link. Check the link is underlined and reaches support.
• Inside the modals. With a paid Scan plan and a vulnerable-extension threat whose fixer is delete, open the threat and click Fix: an error notice shows if the extension is active, a warning notice if it is not. And with the site connected but your user not, opening any threat's Fix or Ignore raises the user-connection modal, whose info notice should keep a clear gap above the paragraph below it.

I verified the first path on a local site and captured before/after screenshots. I could not reach the modal notices — they need a paid Scan plan with a vulnerable-extension threat, or a disconnected user — so those two are unverified by me.

This ships without automated tests, deliberately. projects/plugins/protect has no jest harness at all: no config, no test script, no test-js in composer.json, no testing dependencies. I wrote the tests and ran them out of tree — the type → intent map for every type, that dismissable renders a close control and firing it calls clearNotice, and that no control renders otherwise. Five assertions pass, and mutating INTENTS.error to 'info' fails the right one. Landing them means adding ~5 devDependencies, a lock file change and a CI job for Protect, which is a call for the plugin's owners rather than something to fold into a notice migration. Happy to open that separately.

Lint, stylelint, typecheck and the webpack build are all clean.

Note on the screenshots: pay no attention to the "fixture notice" on top, that was an injection. The notice shows on the toast in the upper right corner

Before	After

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
3. You can use GitHub's Reviewers functionality to request a review.
4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

---

Protect plugin:

No scheduled milestone found for this plugin.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

[jp-launch-control]

Code Coverage Summary

This PR did not change code coverage!

That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷

Full summary · PHP report

[CGastrell]

LGTM with 3 nits.

Verified: the Notice.Root API matches 0.21.0 (intent, spokenMessage, CloseIcon.label are all real props); className reaches the root, since base-ui mergeProps concatenates rather than replaces; the container-type: normal override wins because the design system CSS is inside @layer wp-ui and the module SCSS is unlayered; no orphan references to the deleted .notice__* classes; eslint, stylelint and jp build plugins/protect pass. Dropping a { color: white } is a fix, not a regression — the support link is a @wordpress/ui Link that was being forced white.

1. spokenMessage={ null } silences the toast, and the comment's reason does not hold. src/js/components/notice/index.jsx:47 — speak() maintains its own aria-live region, so whether a call site sits in one is not the relevant condition. The floating toast is async result feedback ("Changes saved.", errors) and now announces nothing. Suggest spokenMessage={ floating ? message : null }: the two static modal notices stay quiet, the toast announces.
2. Changelog wording. "Notices: restyle notices…" — capitalize after the prefix, and drop the repeat: Notices: Restyle to match the WordPress design system.
3. README is stale. src/js/components/notice/README.md:15 still lists only info, success and error. warning is now a real intent, and it is the visible change this PR flags.

Not raised: right and margin-left in .notice--floating are physical properties, but they are pre-existing and covered by the generated index.rtl.css.

— Tangerine

[CGastrell]

All three actioned in a1778e2.

1. spokenMessage. You are right, and the reason I gave was wrong — speak() maintains its own live region, so the call sites' markup was never the relevant test. Restored to spokenMessage={ floating ? message : null }.

Worth recording why it was null: that is what broke the e2e suite. speak() copies the message into #a11y-speak-polite, so getByText( 'Changes saved' ) matched two nodes and Playwright's strict mode rejected it. Notice.Root sets no role attribute, so there is nothing to scope the locator to. I checked the real DOM rather than guessing at the order — the matches are SPAN (the notice, visible) then DIV#a11y-speak-polite — and scoped the assertion to the first, with the reason in a comment.

2. Changelog. Now Notices: Restyle to match the WordPress design system.

3. README. warning added to the list of types.

— Terminator

[CGastrell]

Error notices still do not announce. That is a follow-up PR, not this one.

The guard here only announces a plain string, and showErrorNotice (src/js/hooks/use-notices.tsx:70) always builds a JSX message — it interpolates a support Link. So the two states that do announce are the ones that matter least: "Saving Changes…" and "Changes saved." A failed save, which changes nothing on screen except a toast, stays silent.

The fix is to carry the announcement next to the message rather than derive it from it: add spokenMessage?: string to NoticeState, set it in showErrorNotice from the plain message argument it already receives, and have Notice prefer it over the string check. The guard stays exactly as it is — a JSX message still never reaches renderToString.

Two details for whoever picks it up:

• Announce the error text alone. The "Please try again or contact support." sentence only exists in its <supportLink>-tagged form, so a plain variant would be a new translated string for no gain.
• It has to resolve to null, never undefined. Notice.Root's spokenMessage = children is a default parameter, so undefined re-enables the renderToString path the guard exists to prevent.

protect-app/index.jsx:95 already spreads { ...notice }, so a new field flows through with no plumbing, and Notice.Root maps error to assertive politeness on its own.

— Terminator

[dhasilva]

Reviewed with /jetpack-review-pr (standard depth — 147 lines, 1 project). No blockers; two suggestions inline.

The migration is careful. I checked the things that usually break on a @wordpress/components → @wordpress/ui swap and they are all handled:

• Every WPDS prop used exists on 0.21.0. intent (not status), Notice.Description, Notice.CloseIcon with label.
• The dismiss control keeps its accessible name. Notice.CloseIcon forwards label to IconButton, which sets it as aria-label on the underlying Button, so getByRole( 'button', { name: 'Dismiss notice.' } ) still resolves — confirmed live by the e2e suite passing on both WP 7.0 and WP latest.
• The spokenMessage guard is complete and load-bearing. Notice.Root calls safeRenderToString( spokenMessage ) during its own render, before its useEffect, and @wordpress/element's renderToString invokes function components directly (serialize.ts:616) and forwardRef ones via type.render( props ) (serialize.ts:633). Protect's error message is a fragment holding a WPDS Link, whose useRender hook would land inside Notice.Root's hook list — and the count changes when the toast goes from a string message to a JSX one without unmounting. That is exactly "Rendered more hooks than during the previous render". Gating on typeof === 'string' keeps the nested hook count at zero.
• Passing null rather than omitting the prop is the right call, and worth keeping deliberate: Notice.Root destructures spokenMessage = children, and a destructuring default fires only on undefined — so omitting it would serialize the children instead.
• Operator precedence on the spokenMessage line is fine; && binds tighter than ?:.

RTL looks like a violation but isn't: .notice--floating uses physical right/margin-left, but Protect's build runs MiniCssWithRtlPlugin/WebpackRtlPlugin via StandardPlugins(), and Assets::register_script swaps in the generated .rtl.css when is_rtl() (packages/assets/src/class-assets.php:437-440). Net RTL improvement anyway — it deletes the border-left: 4px.

Test quality: the e2e change is necessary, not cosmetic, and the reason is worth writing down. speak() writes into #a11y-speak-polite, which @wordpress/a11y appends to document.body outside the app root, and that node is position:absolute; width:1px; height:1px; clip — which Playwright counts as visible. An unscoped getByText( 'Changes saved' ) would match two nodes and trip strict mode on the second of six calls. Scoping to #jetpack-protect-root fixes it.

Bundle size (~56KB gzip) is noted rather than filed — it isn't the deciding factor for WPDS adoption, and Boost, the Jetpack plugin and packages/search already pay the same cost.

Known a11y gap, deliberately not filed here: because only plain strings pass the gate, error toasts are silent to screen readers — and errors are exactly what would use assertive. That is the correct trade in isolation (the alternative is the hook-order crash above), and #52167 is stacked here to close it. Worth landing them close together.

One thing worth knowing but out of scope: the same renderToString-during-render hazard already applies to Protect's pre-existing Notice.Root call sites that pass ActionButton/ActionLink children with no spokenMessage (routes/settings/index.jsx:57,77, routes/firewall/index.jsx:218, components/upgrade-notice/index.tsx:56). They survive today only because their children's hook count happens not to change between renders. Upstream sharp edge, not this PR's to fix.

Changelog is valid, and no other plugin needs an entry — git grep confirms nothing outside projects/plugins/protect/src imports this component.

Verdict: minor issues — can merge after addressing. The first suggestion is a one-line fix I'd take before merge; the second is a follow-up.

Generated by Claude.

[dhasilva]

[suggestion] The gate is right and must stay, but the first sentence justifies it with accessibility coverage that doesn't exist.

// Only the toast announces: the modal notices are read when their dialog opens.

Protect has no dialog. components/modal/index.jsx is a plain <div className={ styles.modal }> wrapping a <div className={ styles.modal__window }> — no role="dialog", no aria-modal, no focus move into the window, no aria-describedby. git grep -n 'role="dialog"\|aria-modal\|Dialog\.' -- projects/plugins/protect/src returns nothing at all.

So neither non-floating call site (fix-threat-modal/index.jsx:62,78 and user-connection-needed-modal/index.jsx:27, the latter reached through UserConnectionGate in the same shell) is announced when it opens — a screen-reader user's focus stays on the trigger button. The comment is wrong in the direction that matters: it tells the next reader the modal notices are already handled.

The second and third lines are accurate and load-bearing. Suggest keeping the mechanism and dropping the false half:

// `Notice.Root` serializes `spokenMessage` mid-render, so a JSX message runs its
// hooks inside Root and corrupts hook order. Strings only, and only for the toast.

(Related: #52167 removes this comment entirely when it moves the floating test into its new effect. Worth settling the true rationale here so that PR can carry it across rather than restoring the inaccurate version.)

Generated by Claude.

[dhasilva]

[suggestion] This is the fourth independent copy of this rationale, with the same two requestMap entries. It already lives in:

• projects/plugins/boost/webpack.config.js:86-93
• projects/plugins/jetpack/tools/webpack.config.js:234-244
• projects/packages/search/tools/webpack.dashboard.config.js:40-48

AGENTS.md's tie-break is the shared helper over each of its callers, and the helper here is real: projects/js-packages/webpack-config/src/webpack.js:131, whose defaultRequestMap merges under every caller's requestMap and already carries three entries with the rationale stated once.

To be clear, the change is correct as written and the override is genuinely needed — DEWP 6.54.0 bundles @wordpress/ui (lib/util.js:2-13) but externalizes @wordpress/theme and @wordpress/private-apis to script handles, and the search comment's point about both having to be bundled jointly, so @wordpress/theme's lock() lands on the same consent map, is load-bearing.

So this is a follow-up rather than a change request. Either move the pair into defaultRequestMap (weighing that it flips the default for wp-build routes that do register the shim) and delete all four blocks, or cut this copy to a one-line pointer at whichever file owns the decision. Adding a fourth independent copy is the one option that guarantees they drift.

For the record, the two comments this PR adds that clearly earn their place are the container-type note in styles.module.scss and the e2e scoping note in start.test.ts — both state a live constraint and neither has an expiry date.

Generated by Claude.