Connection: Component updates - replace Modal with Dialog#50735
Connection: Component updates - replace Modal with Dialog#50735coder-karen wants to merge 18 commits into
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
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: 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. Backup 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. Boost 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. Search 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. Social 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. Starter Plugin 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. 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. Videopress 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. Mu Wpcom plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Inspect 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. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Automattic For agencies client 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. Classic Theme helper plugin 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. Paypal Payment buttons 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. Wpcloud Sso 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. Premium Analytics 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. Agents Manager 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 3 files.
1 file is newly checked for coverage.
|
There was a problem hiding this comment.
Pull request overview
This PR migrates the Connection UI dialogs in the projects/js-packages/connection package from @wordpress/components Modal to @wordpress/ui Dialog by introducing shared dialog scaffolding and updating the existing disconnect/manage/owner-disconnect flows. It also prevents the Jetpack plugin’s Owner Disconnect Dialog from inheriting connection package dialog styles by namespacing its styles to the plugin.
Changes:
- Added a shared
ConnectionDialog+ConnectionDialogTitlewrapper around@wordpress/uiDialogto standardize dismissal and accessible naming. - Updated Connection package dialogs, steps, styles, and tests to use the new shared dialog/title.
- Updated Jetpack plugin Owner Disconnect Dialog to use its own namespaced classes/styles and adjusted ARIA labelling.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/plugins/jetpack/changelog/update-connection-js-package-ui-pt5 | Changelog entry for plugin-side style namespacing. |
| projects/plugins/jetpack/_inc/client/components/owner-disconnect-dialog/style.scss | Adds namespaced, self-contained styles for the plugin’s owner disconnect dialog. |
| projects/plugins/jetpack/_inc/client/components/owner-disconnect-dialog/index.jsx | Switches plugin dialog classnames/labelledby to the new namespaced selectors/IDs. |
| projects/js-packages/connection/components/shared/connection-dialog/index.tsx | Introduces shared ConnectionDialog wrapper and ConnectionDialogTitle for accessible naming. |
| projects/js-packages/connection/components/owner-disconnect-dialog/test/component.tsx | Updates tests to validate dialog accessible naming and heading semantics. |
| projects/js-packages/connection/components/owner-disconnect-dialog/style.scss | Removes styles that were specific to the old Modal/button styling and relies on shared disconnect styles. |
| projects/js-packages/connection/components/owner-disconnect-dialog/index.tsx | Migrates owner disconnect to ConnectionDialog and uses ConnectionDialogTitle + Text. |
| projects/js-packages/connection/components/manage-connection-dialog/test/component.tsx | Updates wording and continues asserting dialog accessibility behavior. |
| projects/js-packages/connection/components/manage-connection-dialog/style.scss | Adjusts layout/styling to work with the Dialog popup sizing/scrolling behavior. |
| projects/js-packages/connection/components/manage-connection-dialog/index.tsx | Migrates manage connection dialog to ConnectionDialog and ConnectionDialogTitle. |
| projects/js-packages/connection/components/disconnect-dialog/test/step-thank-you.tsx | Wraps isolated step test in a minimal Dialog context for Dialog.Title. |
| projects/js-packages/connection/components/disconnect-dialog/test/step-disconnect.tsx | Same as above for the disconnect step. |
| projects/js-packages/connection/components/disconnect-dialog/test/step-disconnect-confirm.tsx | Same as above for the confirm step. |
| projects/js-packages/connection/components/disconnect-dialog/test/component.tsx | Updates tests to assert dialog + heading semantics with step-driven accessible naming. |
| projects/js-packages/connection/components/disconnect-dialog/style.scss | Updates dialog layout/typography/styles for the new Dialog-based structure. |
| projects/js-packages/connection/components/disconnect-dialog/steps/step-thank-you.tsx | Switches the step heading to ConnectionDialogTitle for correct accessible naming. |
| projects/js-packages/connection/components/disconnect-dialog/steps/step-survey.tsx | Switches the step heading to ConnectionDialogTitle. |
| projects/js-packages/connection/components/disconnect-dialog/steps/step-disconnect.tsx | Switches the step heading to ConnectionDialogTitle. |
| projects/js-packages/connection/components/disconnect-dialog/steps/step-disconnect-confirm.tsx | Switches the step heading to ConnectionDialogTitle. |
| projects/js-packages/connection/components/disconnect-dialog/index.tsx | Migrates disconnect dialog to ConnectionDialog. |
| projects/js-packages/connection/changelog/update-connection-js-package-ui-pt5 | Changelog entry for the Dialog migration in the Connection package. |
bindlegirl
left a comment
There was a problem hiding this comment.
LGTM 👍 I tested on self-hosted and WoW.
While testing I did notice one thing that I'm not sure if it's deliberate or not. The Esc works on Disconnect account dialog but not the first one (Disconnect site). It's not a new behavior. I just noticed it now. I'll let you decide if it's worth aligning these two or not 🙂
|
Thanks for the review! Are you referring to the Manage Connection Dialog (the first dialog that appears when clicking 'Site and account connected' from My Jetpack?). The dialogs that open from the two options within are both esc closable, so there is consistency based on the type of dialog as I saw it. But I dug more based on your comment. There was a deliberate decision to only focus on those Disconnect modals here - #27643. But the reasoning for leaving the Manage Connection Dialog isn't clear. There was a risk with including it there - if there were an in-flight unlink request. But this can be covered by a So I opted to add this in. Tested a few flows, and everything looks ok to me. |
bindlegirl
left a comment
There was a problem hiding this comment.
Thank you for adding that fix. Tested it again and it all looks good.
…tpack-connection.js
…fresh WP_Scripts singleton
…ore our consumers run, without depending on real WP_Scripts construction
Fixes CONNECT-390
Proposed changes
ConnectionDialogcomponent, which makes of use of@wordpress/ui'sDialogcomponent, replacing theModalthat was in place.disconnect-dialog/index.tsx,manage-connection-dialog/index.tsx, andowner-disconnect-dialog/index.tsx.Textcomponent from@wordpress/uias well (for heading./ sub-heading), in line with it's sibling components.ConnectionDialogTitleto help prevent an accessibility issue. The accessible name tracks the step — eg. success screen now announces "successfully disconnected", not the stale question.Additionally:
Dialogcomponent (from@wordpress/ui) pulls in@wordpress/themeand@wordpress/private-apisunder the hood. Older WP core (eg. 6.9) doesn't always ship those, so script fails to load. The fix here was to polyfill them viawp-build-polyfills, only registered when core's version is missing/incomplete — so most sites load nothing extra, and old sites get just the two small polyfills instead of Webpack bundling duplicate copies into every site's JS regardless of need.@wordpress/bootmodule to be dropped on WP < 7.0. Fix: register on every call, guarded only against duplicate hooks.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
On a Jetpack-connected site with this PR applied (locally or using the Jetpack Beta tester plugin):
setIsProvidingFeedback( true );withinhandleDisconnectinDisconnectDialogto get this to work, so testing locally and re-building for those changes (rebuild My Jetpack and dependencies) is what worked for me.To test that the Jetpack plugin
OwnerDisconnectDialogcomponent styles still look as expected:/wp-admin/admin.php?page=jetpack#/settings.siteConnectionModeto be returning true here:jetpack/projects/plugins/jetpack/_inc/client/searchable-modules/index.jsx
Line 119 in a72efdf
port.Connect your WordPress.com accountlinks in several cards (eg Forms). Click on this.Additionally, to confirm the Webpack bundling change, with WordPress 6.9 installed (can use a rollback plugin):