Jetpack: Extend disconnection dialog component to handle multiple steps and accept more props #21048

jboland88 · 2021-09-10T21:38:04Z

This PR extends the disconnect dialog component so that it can show more information before disconnecting and then provide a feedback survey following disconnection. The dialog prop set is expanded with the intent of re-using the component in different contexts.

Changes proposed in this Pull Request:

Update the disconnect dialog component to have a multi-step disconnect flow:
1 : Disconnect screen - show optional information passed via props and allow user to disconnect
2: Disconnect confirmation - shows following successful disconnection
3: Optional Feedback survey - allow user to provide optional feedback after disconnecting
4: Thank you step - shows after the user elects to submit feedback
Add several additional props to make the disconnect modal more flexible in multiple contexts
Notably, props added include connectedPlugins that will show a list of other plugins that are using the Jetpack connection, and disconnectComponent which allows for a component to be passed that will render on the first screen of the disconnect flow.

Jetpack product discussion

pbNhbs-1n2-p2
p9dueE-3NP-p2

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

This PR allows the user to submit an optional feedback survey. The survey response is connected to the user's ID.
Analytics events are recorded for how a user interacts with the disconnect flow ( opening the flow, what steps are viewed, submitting the survey ) - see a full list of these events at the bottom of the test plan on this PR.

Testing instructions:

Check out this branch on your Jetpack development environment
The updates in this PR are most readily observed when disconnecting via the Jetpack Backups plugin, make sure you have the Jetpack Backups plugin installed and that Jetpack is connected.
If you have not already, build the scripts for the Jetpack Backup plugin with jetpack build
In the wp-admin, navigate to the admin page for the Jetpack Backup plugin and click the "Disconnect" link in the Connection section
You should be presented with the first step of the disconnect flow, which will present a general warning message about disconnecting Jetpack:
Click on "Disconnect" to disconnect the site. The button should show "Disconnecting" while disconnection is in progress.
After a successful disconnect, you will see the disconnect confirmation screen, which will show a "Back to my website" button. Clicking "Back to my website" will exit the disconnect flow and you should see the Jetpack connection screen for the Jetpack Backups plugin.
This PR does include changes to show an additional survey step and a thank you step as part of the disconnection flow. However, it relies on the consumer of the JS connection package to provide some of the information required to collect the survey. Please see the test plan for Backup: provide additional disconnect info in backup plugin #21421 to test the full disconnection flow, including the survey and thank you screens.
This PR collects analytics events, but it does not have the correct scripts loaded to send them to tracks. Check that the window._tkq array contains events related to the disconnect flow. If you complete the test plan, you should see the following event names in window.tkq when on the "Jetpack Disconnected" step of the flow:

jetpack_disconnect_dialog_open
jetpack_disconnect_dialog_step with prop step of disconnect
jetpack_disconnect_dialog_step with prop step of disconnect_confirm

Additional events that are collected in subsequent steps of the flow ( see test plan for #21421 ) are:

jetpack_disconnect_dialog_step with prop step of survey
jetpack_disconnect_survey_submit
jetpack_disconnect_dialog_step with prop step of thank_you

… multiple steps and accept more props

test-case-reminder

Here are some suggested test cases for this PR.

Connection

In-place connection with free plan
In-place connection with paid plan
In-place connection with product purchase
Classic connection. Use Safari, or set a constant JETPACK_SHOULD_NOT_USE_CONNECTION_IFRAME to true
Disconnect/reconnect connection
Secondary user connection
Connection on multisite

Verify that the changes are compatible with the plugins that use the connection package.

WooCommerce Payments
Jetpack Boost
Previous versions of Jetpack

If you think that suggestions should be improved please edit the configuration file here. You can also modify/add test-suites to be used in the configuration file.

github-actions · 2021-09-10T21:41:07Z

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.
⚠️ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team 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 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: December 7, 2021.
Scheduled code freeze: November 30, 2021.

Backup plugin:

Next scheduled release: December 7, 2021.
Scheduled code freeze: November 29, 2021.

Boost plugin:

Next scheduled release: November 30, 2021.
Scheduled code freeze: November 23, 2021.

…nt to show connected plugins and Jetpack benefit information

matticbot · 2021-10-11T18:57:08Z

Caution: This PR has changes that must be merged to WordPress.com
Hello jboland88! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D68232-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

…yle fix

fgiannar

Hi Josh,

Thank you so much for working on this.

Some initial thoughts:

I think we can create a new DisconnectSurvey component and make use of that in DisconnectDialog.
I'd suggest the above for ConnectedPlugins. Aka introduce a new RNA component that will render the connected plugins (in the same spirit you created DisconnectCard) outside of the DisconnectDialog context

Caveats! What we need to pay extra attention about the DisconnectDialog component is that even though we make the REST API call to disconnect (and get a successful response) we need to be careful not to update the state immediately (or call the onDisconnected cb) because if we did that the component itself would not render anymore.
Therefore, informing the state and calling onDisconnected would happen upon clicking the "Back to Wordpress" button. This was carefully designed to be the only way to close the modal, so now we need to ensure that any button that closes / completes the survey will do the same.
For this reason, I'd propose the suggested DisconnectSurvey component to accept callbacks onSubmit and onDismiss in which we will make sure to pass the disconnect handler.

Let me know your thoughts!

jboland88 · 2021-10-18T14:08:36Z

Thanks for the feedback @fgiannar !
I think that all sounds good, I'll work on separating the components DisconnectSurvey and ConnectedPlugins to make things more modular.

leogermani · 2021-10-18T14:51:35Z

Hi Josh, thanks for the PR!

What exactly is the DecorativeCard component. Do you think this could be reused in other contexts? If so, I think we could create a component of its own, out of the DisconnectDialog component.

When this disconnect dialog is used in the Plugins page, upon plugin deactivation, we don't need to make the API request to disconnect the site because it will get disconnected during the plugin deactivation hook. Do you think it's possible to add a prop there to skip the actual disconnection in this case?

…alog-component

jboland88 · 2021-10-20T21:48:33Z

Thanks for the feedback @leogermani

What exactly is the DecorativeCard component. Do you think this could be reused in other contexts?

This is the decorative element that shows on the 2nd and 4th steps of the disconnect flow, something like this:

I do think this has some potential for re-use, I've broken it out into a separate component.

When this disconnect dialog is used in the Plugins page, upon plugin deactivation ... Do you think it's possible to add a prop there to skip the actual disconnection in this case?

Sure thing, I have a context prop that is checked in the handleDisconnect callback before the actual disconnection occurs. My current thought is to use the pluginScreenDisconnectCallback to handle a redirect to the regular plugin deactivation URL.

…e the component

…nnect-dialog-component

…m:Automattic/jetpack into update/extend-disconnect-dialog-component

jeherve

It tests well for me, I think it's mostly ready to merge at this point. I'll approve. I have a remark, but that's something that can be addressed in a follow-up PR, so you don't have to go through the whole dependency update again :)

I should note that I did not review the accessibility of the modal / survey steps; I am not very knowledgeable on that topic. Is that something you've tested on your end, try to go through the flow with the keyboard for example?

Also if note, I got a 404 on the survey API endpoint at the end of the survey once, but could not reproduce afterwards.

projects/plugins/jetpack/class.jetpack.php

jeherve · 2021-11-29T12:19:12Z

projects/plugins/jetpack/class.jetpack.php

-		unset( $active_plugins_using_connection['jetpack'] );
-		$this->load_view( 'admin/deactivation-dialog.php', $active_plugins_using_connection );
+	public function jetpack_plugin_portal_containers() {
+		$this->load_view( 'admin/jetpack-plugin-portal-containers.php' );


At this point and as I'm looking at this again, I wonder if it's worth loading an extra file vs. just adding the 2 div containers right here in the function. If there was more in that template file, extracting would be worth it, but here I'm not sure. What do you think?

Hmm 🤔
I think I could go either way. I tend towards a preference to not output HTML from functions where possible, seems like a nice separation of logic vs output. But you are correct in that there's not much to output in the template file. Would the benefit of including the HTML inline here be the elimination of an extra file?

Would the benefit of including the HTML inline here be the elimination of an extra file?

Yeah, exactly. The more I look at it the more I think that extra file may be a bit overkill. That said, I'm not feeling strongly about this, so happy to merge this as it is. I'll approve this PR again and let you decide. 👍

Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>

jboland88 · 2021-11-29T17:28:32Z

I should note that I did not review the accessibility of the modal / survey steps; I am not very knowledgeable on that topic. Is that something you've tested on your end, try to go through the flow with the keyboard for example?

@jeherve - I have not tested for this specifically. I'll make a note/ task to follow up and make any related updates.

feedback addressed

github-actions · 2021-11-29T17:51:05Z

Great news! One last step: head over to your WordPress.com diff, D68232-code, and commit it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

jboland88 · 2021-11-29T22:04:41Z

r235765-wpcom

* master: (22 commits) Update storybook monorepo to v6.4.0 (#21897) Refresh the site's modules and settings after successful product activation (#21886) Plugin Install: Bump MC stat on success or fail. (#21893) Nav Unification: Use correct user capability for the Inbox menu item (#21882) Jetpack: Extend disconnection dialog component to handle multiple steps and accept more props (#21048) Disable CSSTidy property shorthand optimization in the editor or headstart (#21891) Connection UI: Update Composer instructions in README.md (#21890) Partner Compatibility: Adding a unique connection screen for customers who receive a coupon from a Jetpack partner (#21813) Search package: move search dashboard to package - the base (#21865) JITM: wrap CTA below text on small viewports (#21688) Licensing JS Package – fix icon positioning and width (#21878) JITM: new JS and CSS builder (#21874) Support site_intent for `/me/sites` endpoint (#21880) Don't render Critical CSS while generating CritCSS. (#21879) ConnectScreen: make button full width on small screens (#21683) Improve the copy of the license activation banner (#21876) Rename Webpack-built files back from `.min.js` and remove hashes (#21748) Search: Migrate more helper classes to package (#21751) Search: migrate/add search rest API (#21584) Add initial state to the connection package (#21864) ...

Did a pass over _inc/client/components looking for things that weren't used anywhere, and found a bunch: * components/data/query-connect-url - Last use removed in #8014 * components/data/query-connection-status - Last use removed in 62e9ab0 * components/data/query-modules - Last use removed in bfc40ad * components/data/query-plugin-updates - Last use removed in #17003 * components/data/query-site-products - Last use removed in #21594 * components/form/* - Didn't check for last use, too many bits. But it looks like the `formsy-react` package many of these depended on wasn't even installed since #8208. * components/inline-expand - Last use removed in #6550 * components/jetpack-dialogue - Last use removed in #16518 * components/jetpack-logo - Last use removed in #20148 * components/jetpack-termination-dialog - Last use removed in #21048 * components/module-settings/index.jsx - Last use removed in #10644 * components/module-settings/inline-module-toggle.jsx - Last use removed in #12118 * components/screen-reader-text - Last use removed in #18843 * components/settings - Last use removed in 26315e1, I think * components/tags-input - Last use removed in #11772 Then there were a few more that were only used from some of the above: * components/data/query-connected-plugins * components/module-settings/form-components.jsx * components/multiple-choice-question * components/setting-toggle

Did a pass over _inc/client/components looking for things that weren't used anywhere, and found a bunch: * components/data/query-connect-url - Last use removed in #8014 * components/data/query-connection-status - Last use removed in 62e9ab0 * components/data/query-modules - Last use removed in bfc40ad * components/data/query-plugin-updates - Last use removed in #17003 * components/data/query-site-products - Last use removed in #21594 * components/form/* - Didn't check for last use, too many bits. But it looks like the `formsy-react` package many of these depended on wasn't even installed since #8208. * components/inline-expand - Last use removed in #6550 * components/jetpack-dialogue - Last use removed in #16518 * components/jetpack-logo - Last use removed in #20148 * components/jetpack-termination-dialog - Last use removed in #21048 * components/module-settings/index.jsx - Last use removed in #10644 * components/module-settings/inline-module-toggle.jsx - Last use removed in #12118 * components/screen-reader-text - Last use removed in #18843 * components/settings - Last use removed in 26315e1, I think * components/tags-input - Last use removed in #11772 Then there were a few more that were only used from some of the above: * components/data/query-connected-plugins * components/module-settings/form-components.jsx * components/multiple-choice-question * components/setting-toggle Co-authored-by: Brandon Kraft <public@brandonkraft.com>

Notable changes: * Remove `Jetpack_User_Agent_Info::is_OperaMobile()`. #16434 removed the jetpack-device-detect function this was calling in October 2021, but missed this caller. * Remove `jetpack_server_sandbox()` and `jetpack_server_sandbox_request_parameters()`. Due to a wrong namespace in #21128, they'd have thrown fatals since October 2021. * Use `$skin` instead of `$upgrader->skin`, it's the same object but not typed as a parent class. Should be no change to functionality. * Add some missing methods to `WPCOM_JSON_API`. * Add some missing abstract methods to SAL_Site, and implement in Jetpack_Site. * Fix `SAL_Token::is_global()`. * Remove `views/admin/deactivation-dialog.php`, unused since #21048 in November 2021. * Add `@phan-var-force` in a lot of "template" files. I'm not entirely happy with this, it pollutes the global scope for any subsequent files processed, but short of writing a Phan plugin that somehow adjusts the scope for only the file I can't find a better idea.

Add initial structure to the disconnection dialog component to handle…

7ed751f

… multiple steps and accept more props

jboland88 added [Status] In Progress [JS Package] Connection labels Sep 10, 2021

test-case-reminder bot reviewed Sep 10, 2021

View reviewed changes

jboland88 added 3 commits September 30, 2021 08:38

Merge branch 'master' into update/extend-disconnect-dialog-component

7c1b3c9

Merge branch 'master' into update/extend-disconnect-dialog-component

f28233c

Update disconnect step with output for connected plugins, add compone…

364609f

…nt to show connected plugins and Jetpack benefit information

matticbot added the Touches WP.com Files label Oct 11, 2021

jboland88 added 2 commits October 14, 2021 11:22

Add content and styling to disconnect flow, intial handling for survey

ec669fe

Updated condition for when no connectedPlugins are provided, small st…

5eb49f8

…yle fix

jboland88 mentioned this pull request Oct 14, 2021

Backup: provide additional disconnect info in backup plugin #21421

Merged

jboland88 requested review from fgiannar and leogermani October 14, 2021 21:15

fgiannar previously requested changes Oct 18, 2021

View reviewed changes

jboland88 added 4 commits October 20, 2021 11:08

Break out smaller components, add initialization for tracks

bfc591b

[not verified] Merge branch 'master' into update/extend-disconnect-di…

0c806cd

…alog-component

Update lock file after merging master

e149231

Fix filtering for connected plugins output, update comments

dec8af5

jboland88 requested a review from IanRamosC October 20, 2021 21:49

jboland88 added 2 commits October 22, 2021 15:31

Design improvements, move control for opening DisconnectDialog outsid…

4fa0743

…e the component

Merge branch 'master' into update/extend-disconnect-dialog-component

82bcf52

jboland88 mentioned this pull request Oct 22, 2021

Update: allow for multiple steps in disconnection flow #20994

Closed

github-actions bot added the RNA label Oct 22, 2021

jboland88 mentioned this pull request Oct 22, 2021

Jetpack: improve plugin disconnection flow #20914

Merged

jboland88 marked this pull request as ready for review October 22, 2021 21:52

IanRamosC and others added 4 commits November 28, 2021 23:46

Merge remote-tracking branch 'origin/master' into update/extend-disco…

afac99c

…nnect-dialog-component

Merge branch 'update/extend-disconnect-dialog-component' of github.co…

a35ffef

…m:Automattic/jetpack into update/extend-disconnect-dialog-component

Merge branch 'master' into update/extend-disconnect-dialog-component

eeb3036

Version bump for partner coupon package

aa1bac9

github-actions bot added the [JS Package] Partner Coupon label Nov 29, 2021

jboland88 requested a review from jeherve November 29, 2021 14:37

jeherve previously approved these changes Nov 29, 2021

View reviewed changes

jeherve added [Status] Needs Testing We need to add this change to the testing call for this month's release [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Nov 29, 2021

kraftbj dismissed jeherve’s stale review via aa1bac9 November 29, 2021 17:07

Update projects/plugins/jetpack/class.jetpack.php

5f86825

Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>

jeherve approved these changes Nov 29, 2021

View reviewed changes

jboland88 merged commit 96351c0 into master Nov 29, 2021

jboland88 deleted the update/extend-disconnect-dialog-component branch November 29, 2021 17:49

github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Nov 29, 2021

jeherve removed the [Status] Needs Testing We need to add this change to the testing call for this month's release label Nov 30, 2021

jboland88 added the [Status] Needs Privacy Updates Our support docs will need to be updated to take this change into account label Dec 6, 2021

coder-karen removed the [Status] Needs Privacy Updates Our support docs will need to be updated to take this change into account label Dec 29, 2021

anomiex mentioned this pull request May 10, 2022

jetpack: Remove unused JS client components #24319

Merged

anomiex mentioned this pull request May 10, 2024

Jetpack: Fix various PhanUndefined issues #37344

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jetpack: Extend disconnection dialog component to handle multiple steps and accept more props #21048

Jetpack: Extend disconnection dialog component to handle multiple steps and accept more props #21048

jboland88 commented Sep 10, 2021 •

edited

Loading

test-case-reminder bot left a comment

github-actions bot commented Sep 10, 2021 •

edited

Loading

matticbot commented Oct 11, 2021

fgiannar left a comment

jboland88 commented Oct 18, 2021

leogermani commented Oct 18, 2021

jboland88 commented Oct 20, 2021

jeherve left a comment

jeherve Nov 29, 2021

jboland88 Nov 29, 2021

jeherve Nov 29, 2021

jboland88 commented Nov 29, 2021

github-actions bot commented Nov 29, 2021

jboland88 commented Nov 29, 2021

Jetpack: Extend disconnection dialog component to handle multiple steps and accept more props #21048

Jetpack: Extend disconnection dialog component to handle multiple steps and accept more props #21048

Conversation

jboland88 commented Sep 10, 2021 • edited Loading

Changes proposed in this Pull Request:

Jetpack product discussion

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

Related:

Testing instructions:

test-case-reminder bot left a comment

Choose a reason for hiding this comment

Connection

github-actions bot commented Sep 10, 2021 • edited Loading

matticbot commented Oct 11, 2021

fgiannar left a comment

Choose a reason for hiding this comment

jboland88 commented Oct 18, 2021

leogermani commented Oct 18, 2021

jboland88 commented Oct 20, 2021

jeherve left a comment

Choose a reason for hiding this comment

jeherve Nov 29, 2021

Choose a reason for hiding this comment

jboland88 Nov 29, 2021

Choose a reason for hiding this comment

jeherve Nov 29, 2021

Choose a reason for hiding this comment

jboland88 commented Nov 29, 2021

github-actions bot commented Nov 29, 2021

jboland88 commented Nov 29, 2021

jboland88 commented Sep 10, 2021 •

edited

Loading

github-actions bot commented Sep 10, 2021 •

edited

Loading