Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Disconnect Dialog #13517

Merged
merged 37 commits into from
Oct 24, 2019
Merged

New Disconnect Dialog #13517

merged 37 commits into from
Oct 24, 2019

Conversation

cbauerman
Copy link
Contributor

@cbauerman cbauerman commented Sep 23, 2019

Changes proposed in this Pull Request:

  • Adds new, redesigned disconnect dialog
  • Disconnect dialog now dynamically pulls in usage stats of Jetpack on the site
  • Prep work for Survey Step in disconnect dialog
  • Prep work for Plugins Install mobile

Two Site Benefits - Mobile
Screen Shot 2019-09-24 at 3 39 04 PM

Three Site Benefits - Mobile
Screen Shot 2019-09-24 at 3 39 04 PM

Single Site Benefit - Wide
Single Site Benefit - Wide

Two Site Benefits - Wide
Two Site Benefits - Wide

Three Site Benefits - Wide
Three Site Benefits - Wide

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • Internal reference: parBdM-hs-p2

Redesign of Jetpack Disconnect Dialog

Testing instructions:

  1. Navigate to /wp-admin/admin.php?page=jetpack#/dashboard
  2. Click "Manage Site Connection"

Screen Shot 2019-09-24 at 3 53 46 PM

3. Verify that your dialog roughly matches one of the above screenshot. 4. Modify the following code snippet and insert into `_inc/client/components/jetpack-termination-dialog/features.jsx` before the line `const siteBenefitCount = siteBenefits.length;` with varying amounts of additional site benefits to test the various cases.
	siteBenefits = siteBenefits.concat( [
		{
			title: 'Brute Force Protection',
			description: 'The number of malicious login attempts blocked by Jetpack.',
			amount: 32030,
			gridIcon: 'lock',
		},
		{
			title: 'Contact Forms',
			description: 'The number of live Jetpack forms on your site right now.',
			amount: 31,
			gridIcon: 'align-image-center',
		},
		{
			title: 'Publicize',
			description: 'The number of live social media connections, powered by Jetpack.',
			amount: 3,
			gridIcon: 'share',
		},
		{
			title: 'Subscribers',
			description: 'The number of people subscribed to your updates through Jetpack.',
			amount: 4200,
			gridIcon: 'user',
		},
	] );

Proposed changelog entry for your changes:

Redesign of Jetpack Disconnect Dialog

@cbauerman cbauerman added this to the 7.8 milestone Sep 23, 2019
@cbauerman cbauerman self-assigned this Sep 23, 2019
@jetpackbot
Copy link

jetpackbot commented Sep 23, 2019

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: November 5, 2019.
Scheduled code freeze: October 29, 2019

Generated by 🚫 dangerJS against 3a750b4

@jeherve jeherve removed this from the 7.8 milestone Sep 23, 2019
@jeherve
Copy link
Member

jeherve commented Sep 23, 2019

Removing the milestone since we discussed that this would be postponed to the next release:
p1568997915079200-slack-jetpack-crew

@jeherve jeherve added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it Admin Page React-powered dashboard under the Jetpack menu labels Sep 23, 2019
@cbauerman
Copy link
Contributor Author

Removing the milestone since we discussed that this would be postponed to the next release:
p1568997915079200-slack-jetpack-crew

I managed to both flip the code freeze and release dates and move Sep 23 to Oct 23 in my mind 🤦‍♂️ Thanks for the catch!

@cbauerman cbauerman added this to the 7.9 milestone Sep 23, 2019
@cbauerman cbauerman added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Sep 24, 2019
@cbauerman cbauerman marked this pull request as ready for review September 24, 2019 22:56
@cbauerman cbauerman requested a review from a team as a code owner September 24, 2019 22:56
@cbauerman cbauerman added the [Status] Needs Design Review Design has been added. Needs a review! label Sep 24, 2019
@scottsweb
Copy link
Contributor

Thanks for the ping on this. Some initial thoughts:

  • The close icon at the top right of the modal doesn't appear to be clickable (no cursor change) and it cannot be tabbed to either. So perhaps we just need to warp it in an <a>?
  • We do not provide this modal when removing the plugin from the plugins.php screen yet. The same goes for the disconnect link on admin.php?page=jetpack-debugger. Guessing that might be coming soon along with the disconnect survey.
  • On first load the modal was a little narrow, but it cleared up after subsequent refreshes, I think it might have been a build issue:

Screenshot 2019-09-25 at 15 22 37

  • The responsiveness of the modal is great 👍
  • There were a couple of things in the original mockup (towards the bottom of the modal) that I think worked a little better:

Screenshot 2019-09-25 at 15 59 34

  • The first CTA was changed to blue and "continue" to make it clear there is a second step
  • The support link had a little more room around it
  • The links were not italicised
  • We use a mixture of log out and disconnect, I am not sure where we landed on this in the end but it was certainly a point of discussion. On that same note it might be worth a copy review from @robertbpugh

If I can help with anything then please let me know!

@jeherve jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Sep 26, 2019
@cbauerman
Copy link
Contributor Author

cbauerman commented Sep 30, 2019

So, to wrap this whole conversation I need to add that this is designed to be PR 1 of 3 which explains some changes:

  1. ( This PR ) Redesigned Disconnect Dialog with 1-step, only on Jetpack Dashboard
  2. 2-Step Disconnect Dialog with Survey
  3. Add to Plugins Screen

So the following changes are intentional

  • Blue Continue Button -> Red Disconnect Button
  • No Survey Step
  • Missing from Plugins Screen
    Are definitely missing by design.

I will be addressing/looking into:

  • Cursor on modal close
  • Link italics -> underlines
  • Support link space

and I am looking for feedback on the copy. My plan is to use "Disconnect Jetpack" on the dashboard and "Disable Jetpack" on the upcoming plugins screen. ( cc @robertbpugh )

let me know how that sounds @scottsweb!

@cbauerman cbauerman closed this Sep 30, 2019
@cbauerman cbauerman reopened this Sep 30, 2019
@cbauerman
Copy link
Contributor Author

cbauerman commented Sep 30, 2019

Did not mean to close, pressed the button without looking too close 🤦‍♂️

@cbauerman cbauerman added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Sep 30, 2019
@scottsweb
Copy link
Contributor

That all sounds good to me, it is good to break this down into separate PRs. Your copy proposal makes sense too given that the outcomes are slightly different based on the where the modal is initiated from.

@scottsweb
Copy link
Contributor

scottsweb commented Oct 2, 2019

Did you want me to test this again @cbauerman now the styles have been updated ?

@cbauerman
Copy link
Contributor Author

Did you want me to test this again @cbauerman now the styles have been updated ?

@scottsweb yes please!

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving after a rebase. I'll merge this now to avoid any more conflicts with master in the future.

@jeherve jeherve merged commit e09460b into master Oct 24, 2019
@jeherve jeherve deleted the add/new-aag-disconnect-dialog branch October 24, 2019 10:07
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 24, 2019
@jeherve jeherve added the [Status] Needs Testing We need to add this change to the testing call for this month's release label Oct 24, 2019
@jeherve jeherve added [Status] Has Changelog and removed [Status] Needs Changelog [Status] Needs Testing We need to add this change to the testing call for this month's release labels Oct 28, 2019
jeherve added a commit that referenced this pull request Oct 28, 2019
jeherve added a commit that referenced this pull request Oct 29, 2019
* 7.9: Changelog

* Update version number

* Update stable tag and tested up to

* Changelog: add #13530

* changelog: add #13578

* Changelog: add #13598

* Changelog: add entry for numerous block preview changes

* Changelog: add #13599

* changelog: add #13541

* Changelog: add #13542

* Changelog: add #13331

* Changelog: add #13558

* Changelog: add #13409

* Changelog: add #13582

* Changelog: add #13600

* Changelog: add #13601

* Changelog: add #13595

* Changelog: add #12695

* Changelog: add #13009

* Changelog: add #13649

* Changelog: add #13450

* Changelog: add #13507

* Changelog: add #13658

* Changelog: add #13687

* changelog: add #13683

* Changelog: add #9323

* Changelog: add #13681

* Fix typos in readme

* Add link to WordPress Beta Tester plugin

* Changelog: add #13630

* Changelog: add #13695

* Changelog: add #13659

* Changelog: add #13716

* Changelog: add #13664

* Changelog: add #13682

* Changelog: add #13362

* Changelog: add #13563

* Add testing list for #13563

* Changelog: add #13735

* Changelog: add #13752

* Changelog: add #13624

* Changelog: add #13756

* Changelog: add #13745

* Changelog: add #13728

* Changelog: add #13779

* Changelog: add #13699

* Changelog: add #13804

* Changelog: add #13761

* Changelog: add #13637

* Changelog: add #13517

* Changelog: add #13521

* Changelog: add #13729

* Testing list: add testing instructions for #13729

* Changelog: add sync changes

* Changelog: add #13807

* Changelog: add #13654

* Changelog: add #13795

* Changelog: add #13801

* Changelog: add #13818

* Changelog: add #13725

* Changelog: add #13831

* Changelog: add #13516

* Testing list: add Twenty Twenty instructions

* Changelog: add #13799

* Changelog: add #13805

* Changelog: add #13688

* Changelog: add #13830
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin Page React-powered dashboard under the Jetpack menu [Pri] High [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants