Repository navigation
Components: Migrate remaining Notice consumers to @wordpress/ui - #48173
Merged
Merged
Conversation
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Package] My Jetpack
[Plugin] Jetpack
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
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:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. 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. |
Code Coverage SummaryCoverage changed in 1 file.
Full summary · PHP report · JS report Coverage check overridden by
I don't care about code coverage for this PR
|
vianasw
force-pushed
the
update/components-notice-remove-source
branch
from
April 18, 2026 19:59
e7195cc to
4cabe1f
Compare
vianasw
added
the
I don't care about code coverage for this PR
vianasw
force-pushed
the
update/components-notice-remove-source
branch
from
April 23, 2026 14:56
fb96aea to
d729d1e
Compare
simison
reviewed
Apr 28, 2026
simison
reviewed
Apr 28, 2026
simison
reviewed
Apr 28, 2026
CGastrell
pushed a commit
that referenced
this pull request
Apr 28, 2026
- Jetpack changelog entry: Type `changed` isn't valid for plugins/jetpack (allowed: major, enhancement, compat, bugfix, other). Switch to `other`. - tools/webpack.config.js: fix prettier/prettier indentation inside the DependencyExtractionPlugin requestMap override (the eslint --fix reindent). - connection-banner test: the old test used getByLabelText looking for ActionButton's `label` prop. The @wordpress/ui Notice.ActionButton puts the text in children; switch to getByRole with accessible name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CGastrell
added a commit
that referenced
this pull request
Apr 28, 2026
The original migration added `@wordpress/theme: { external: false }` to
DependencyExtractionPlugin requestMaps in protect/webpack.config.js and in
two call sites in plugins/jetpack/tools/webpack.config.js, on the theory
that @wordpress/theme is a transitive dep of @wordpress/ui that isn't
registered as a WP script handle in plain admin contexts.
Verified locally: with both files restored to trunk, `jetpack build
plugins/protect` succeeds and the migrated Notices render cleanly on
Protect Settings and Firewall — no console errors, no enqueue failures.
@wordpress/theme is shipped as a script module, not a classic script
handle, so the externalization concern doesn't apply here.
Per @simison's review feedback on #48173.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CGastrell
force-pushed
the
update/components-notice-remove-source
branch
from
April 28, 2026 21:44
d729d1e to
a538a42
Compare
CGastrell
added a commit
that referenced
this pull request
Apr 28, 2026
- firewall/index.jsx: wrap "Jetpack Firewall is currently disabled." in __() so it gets translated (regression introduced when migrating away from the old <Notice title=...> prop, which was already passing a translated string). - settings/index.jsx: replace `target="_blank" rel="noreferrer"` on both Notice.ActionLink instances with the `openInNewTab` shorthand. It's the @wordpress/ui Link API: sets target=_blank under the hood and appends the design system's external-link arrow indicator. Per @simison's review feedback on #48173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to #48171 (Boost migration). Migrates the three non-Boost call sites of the Jetpack-internal Notice component (from @automattic/jetpack-components) to the compound-component Notice from @wordpress/ui: - Jetpack plugin: _inc/client/components/connection-banner/index.jsx - Protect: src/js/routes/settings/index.jsx (2 notices) - Protect: src/js/routes/firewall/index.jsx (1 notice, previously aliased as JetpackNotice) Each call site decomposes title/actions/children into Notice.Root / Notice.Title / Notice.Description / Notice.Actions / Notice.CloseIcon, and maps `level` -> `intent` (same vocabulary). Adds @wordpress/ui 0.11.0 to projects/plugins/protect/package.json (the Jetpack plugin already had it). Webpack: forces @wordpress/theme to bundle inline in protect's webpack config and in the Jetpack plugin's tools/webpack.config.js DependencyExtractionPlugin() call sites. @wordpress/theme is a transitive dep of @wordpress/ui that isn't registered as a WP script handle in plain admin contexts, so externalizing it breaks enqueue. Leaving @wordpress/private-apis externalized: core's private-apis copy rejects lock() from non-core modules, but other externalized consumers (@wordpress/dataviews) depend on sharing the same instance. The source Notice component in js-packages/components and the my-jetpack package's three call sites are intentionally left unchanged here. my-jetpack pulls in @wordpress/dataviews which fights over the private-apis consent map when @wordpress/theme is bundled inline; that migration needs dedicated webpack surgery and is out of scope for this PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Jetpack changelog entry: Type `changed` isn't valid for plugins/jetpack (allowed: major, enhancement, compat, bugfix, other). Switch to `other`. - tools/webpack.config.js: fix prettier/prettier indentation inside the DependencyExtractionPlugin requestMap override (the eslint --fix reindent). - connection-banner test: the old test used getByLabelText looking for ActionButton's `label` prop. The @wordpress/ui Notice.ActionButton puts the text in children; switch to getByRole with accessible name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The original migration added `@wordpress/theme: { external: false }` to
DependencyExtractionPlugin requestMaps in protect/webpack.config.js and in
two call sites in plugins/jetpack/tools/webpack.config.js, on the theory
that @wordpress/theme is a transitive dep of @wordpress/ui that isn't
registered as a WP script handle in plain admin contexts.
Verified locally: with both files restored to trunk, `jetpack build
plugins/protect` succeeds and the migrated Notices render cleanly on
Protect Settings and Firewall — no console errors, no enqueue failures.
@wordpress/theme is shipped as a script module, not a classic script
handle, so the externalization concern doesn't apply here.
Per @simison's review feedback on #48173.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- firewall/index.jsx: wrap "Jetpack Firewall is currently disabled." in __() so it gets translated (regression introduced when migrating away from the old <Notice title=...> prop, which was already passing a translated string). - settings/index.jsx: replace `target="_blank" rel="noreferrer"` on both Notice.ActionLink instances with the `openInNewTab` shorthand. It's the @wordpress/ui Link API: sets target=_blank under the hood and appends the design system's external-link arrow indicator. Per @simison's review feedback on #48173. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CGastrell
force-pushed
the
update/components-notice-remove-source
branch
from
April 30, 2026 14:56
a538a42 to
fc25aa9
Compare
github-actions
Bot
removed
the
[Status] Needs Review
CGastrell
added a commit
that referenced
this pull request
May 8, 2026
…shboard build The Notice migration introduced a transitive dep on `@wordpress/theme` via `@wordpress/ui`. The dependency-extraction-webpack-plugin externalized it to the `wp-theme` script handle, which is not reliably registered as a WP script in plain admin contexts (e.g., Jurassic Ninja test sites). When `wp-theme` was missing from `$wp_scripts->registered`, WordPress silently skipped printing `jp-search-dashboard` -- no script tag, no inline initial state, blank page. Bundle both `@wordpress/theme` and `@wordpress/private-apis` together so the module-init `lock()` call lands on the same private-apis consent map. Same fix as #48173 (Components Notice migration) and #48171 (Boost). The search dashboard does not pull in `@wordpress/dataviews`, so this does not hit the my-jetpack collision case described there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CGastrell
added a commit
that referenced
this pull request
May 11, 2026
…48537) (#48550) * Search: migrate dashboard notices to @wordpress/ui Notice Replace the local SimpleNotice + NoticeAction wrappers with Notice.* from @wordpress/ui across the search dashboard: - record-meter/notice-box: tier notices use Notice.Root/Title/Description + Notice.Actions/ActionLink (openInNewTab); the legacy jp-search-notice-box__important class becomes intent="error". - pages/sections/first-run-section: indexing notice uses Notice.Root/Title/ Description. - global-notices: snackbar notices use Notice.Root + Notice.Description + Notice.CloseIcon, with auto-dismiss preserved via a useEffect timer. Status strings remain in the redux store; mapped to wp-ui intents at render. Drops vestigial fields the redux notice creators never set anyway (isCompact, isPersistent, displayOnNextPage, notice.button/href). Deletes the SimpleNotice/NoticeAction sources, their stylesheets, and notice-box.scss; strips the dead .dops-notice* overrides from global-notices/style.scss while keeping the snackbar positioning rules. Removes one site of @automattic/jetpack-components Gridicon usage (SimpleNotice's icon-wrapper) — helps the broader modernization sweep. Follow-up to simison review feedback on #48537. * Search: scope notice-box test queries to render container `@wordpress/ui` Notice uses `@wordpress/a11y` to announce notice text via `<div class="a11y-speak-region">` regions appended to `document.body`. After this PR's migration, the notice copy is in the DOM twice (in the rendered Notice and in the body-level a11y-speak live region), so `screen.getByText` throws "Found multiple elements". Capture the `container` from `render()` and use `within( container ).getByText(...)` so queries only see the rendered Notice and ignore the screen-reader announcement region that lives outside the container. Verified locally: all 5 tests in `notice-box.test.jsx` pass. * Search: bundle @wordpress/theme and @wordpress/private-apis in the dashboard build The Notice migration introduced a transitive dep on `@wordpress/theme` via `@wordpress/ui`. The dependency-extraction-webpack-plugin externalized it to the `wp-theme` script handle, which is not reliably registered as a WP script in plain admin contexts (e.g., Jurassic Ninja test sites). When `wp-theme` was missing from `$wp_scripts->registered`, WordPress silently skipped printing `jp-search-dashboard` -- no script tag, no inline initial state, blank page. Bundle both `@wordpress/theme` and `@wordpress/private-apis` together so the module-init `lock()` call lands on the same private-apis consent map. Same fix as #48173 (Components Notice migration) and #48171 (Boost). The search dashboard does not pull in `@wordpress/dataviews`, so this does not hit the my-jetpack collision case described there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Search: fix changelogger Type for jetpack plugin entry The jetpack plugin's changelogger uses bugfix/enhancement/compat/major/other taxonomy, not fixed/changed/added/removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CGastrell
added a commit
that referenced
this pull request
Sep 10, 2026
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
CGastrell
added a commit
that referenced
this pull request
Sep 10, 2026
…ice (#52160) * 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: 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 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
dhasilva
added a commit
that referenced
this pull request
Sep 11, 2026
* 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>
CGastrell
added a commit
that referenced
this pull request
Sep 14, 2026
The comments, README and Jetpack call site said the wp-theme and wp-private-apis handles are missing on many pages, and cited #48173. Neither holds at the WP 7.0 minimum: core registers both handles, and #48173 merged no webpack change. State the lock rule instead: a bundled @wordpress/private-apis unlocks only what its own bundle locked. State the current reason to use the option: @wordpress/ui is built against a newer @wordpress/theme than core's wp-theme. Record the new option as an added changelog entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnmaaEamjHo2MmDpnrwiNa
CGastrell
added a commit
that referenced
this pull request
Sep 17, 2026
…ith an opt-in option (#52207) * Webpack config: move the @wordpress/theme bundling rule into the shared default Three webpack configs each carried the same `DependencyExtractionPlugin` override for `@wordpress/theme` and `@wordpress/private-apis`, with the rationale written out four times. The rule now lives once in `defaultRequestMap`, which merges under every caller's own `requestMap`. The Jetpack plugin's AI admin and email design editor entries opt `@wordpress/private-apis` back out with an empty entry: both unlock private APIs exposed by external WordPress packages, so their consent map has to stay external. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Webpack config: replace the default flip with an opt-in bundleWpUiDeps option The earlier approach added @wordpress/theme and @wordpress/private-apis to defaultRequestMap. That changed every consumer: it broke the packages/jetpack-mu-wpcom build, broke three packages/wp-build-polyfills tests, needed two opt-outs inside the Jetpack plugin config, and grew two bundles. DependencyExtractionPlugin now takes bundleWpUiDeps, off by default, so nothing changes unless a call site asks. The four call sites that carried the nine-line requestMap block verbatim now set the flag instead. jetpack-ai-admin keeps its own requestMap, because it bundles @wordpress/theme alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SWVs5oUDimen1FeGJhJAkx * Webpack config: correct the bundleWpUiDeps rationale The comments, README and Jetpack call site said the wp-theme and wp-private-apis handles are missing on many pages, and cited #48173. Neither holds at the WP 7.0 minimum: core registers both handles, and #48173 merged no webpack change. State the lock rule instead: a bundled @wordpress/private-apis unlocks only what its own bundle locked. State the current reason to use the option: @wordpress/ui is built against a newer @wordpress/theme than core's wp-theme. Record the new option as an added changelog entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnmaaEamjHo2MmDpnrwiNa --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #48160
Proposed changes
Follow-up to #48171 (Boost migration). Migrates three non-Boost call sites of the Jetpack-internal
Noticecomponent (from@automattic/jetpack-components) to the compound-componentNoticefrom@wordpress/ui:_inc/client/components/connection-banner/index.jsxsrc/js/routes/settings/index.jsx(2 notices) andsrc/js/routes/firewall/index.jsx(was aliased asJetpackNotice)Each call site decomposes the old flat
title/actions/onCloseprops into sub-components:Notice.Rootwithintent→Notice.Title+Notice.Description+ optionalNotice.Actions(Notice.ActionButton/Notice.ActionLink) + optionalNotice.CloseIcon.levelmaps 1:1 tointent."@wordpress/ui": "0.11.0"toprojects/plugins/protect/package.json. The Jetpack plugin already had it.@wordpress/themeto bundle inline (external: false) in Protect'swebpack.config.jsand in bothDependencyExtractionPlugin()call sites inprojects/plugins/jetpack/tools/webpack.config.js.@wordpress/themeis a transitive dep of@wordpress/uibut isn't registered as a WP script handle in plain admin contexts, so externalizing it breaks the bundle enqueue (same fix as Boost: Migrate Notice to @wordpress/ui #48171).@wordpress/private-apisstays externalized in this PR: bundling it inline causeddataviewsto collide over the consent map in bundles that also pull dataviews in.The source component in
projects/js-packages/components/components/notice/and its barrel export stay in place. Boost still uses the internal Notice on trunk until #48171 lands.What's NOT in this PR
my-jetpack's three Notice consumers (my-jetpack-screen,product-detail-table,product-interstitial/jetpack-ai/product-page) are intentionally excluded.my-jetpack's bundle includes@wordpress/dataviews, which uses@wordpress/private-apis' consent map. When@wordpress/themeis bundled inline and@wordpress/private-apisis externalized,@wordpress/theme's module-initlock()against core's private-apis gets rejected — core only permits lock from modules shipped with WP core itself. Bundling@wordpress/private-apisinline too causes a different conflict:dataviews' bundledunlock()calls against what it thinks is the same consent map fail, because webpack dedupe isn't aligning the copies across packages.Fixing this properly needs either (a) a second entry point or (b) a targeted dependency-extraction-plugin change that groups
@wordpress/theme+@wordpress/private-apis+@wordpress/dataviewsinto a single bundled unit. Out of scope here — will land as a follow-up.Related product discussion/links
@wordpress/ui)Does this pull request change what data or activity we track or use?
No.
Testing instructions
jetpack build plugins/protect plugins/jetpack --deps.wp-admin → Jetpack. The "Connect your WordPress.com account" banner should render with@wordpress/uistyling — info intent, title, description, and a Connect your WordPress.com account action button. Clicking the action should still trigger the connection flow.wp-admin → Jetpack → Protect → Settings): if Account Protection is unsupported on your environment, a warning Notice renders with the new styling and a "Learn more" action link.wp-admin → Jetpack → Protect → Firewall): deactivate the firewall module (e.g.,wp jetpack module deactivate waf) to surface the "Jetpack Firewall is currently disabled." error Notice with the "Enable Firewall" action button — this was visually validated on a connected dev site and matches the pre-migration look, with@wordpress/uistyling.pnpm eslinton the touched files — should be clean.Follow-up
Once this PR and #48171 both merge, a further PR will bundle
my-jetpack's Notice migration together with the webpack surgery needed to reconcile@wordpress/ui's private-apis usage with@wordpress/dataviews. After that, the source Notice component injs-packages/componentscan be deleted.