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.

Sign up for GitHub

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

Jump to bottom

Connection: show full screen connection banner modal in JP dash #10888

Merged
merged 9 commits into from
Dec 14, 2018
Merged

Connection: show full screen connection banner modal in JP dash #10888

merged 9 commits into from
Dec 14, 2018

Conversation

jeherve
Copy link
Member

@jeherve jeherve commented Dec 7, 2018 •
edited
Loading

Fixes #10563
Fixes #8618

Changes proposed in this Pull Request:

That modal was only loaded in the Plugins screen; let's load it in the Jetpack menu as well.

Notes

Since we are getting rid of the old connection component that used to be displayed on the Jetpack dashboard page, we now have to reload the page when you disconnect your site from WordPress.com, so the new modal can appear again.

By doing so, we do not see the notice that appears at the top of the Jetpack dashboard after a disconnection, inviting you to take a survey to tell us why you disconnected.

screenshot 2018-12-10 at 16 49 07

Testing instructions:

  • Start from a site where Jetpack is not active.
  • Activate the plugin.
  • Check the connection modal appearing after the plugin is activated.
  • Go to the Jetpack menu. You should see the same modal.

Proposed changelog entry for your changes:

  • Improve the connection flow for new Jetpack users.

@jeherve jeherve added [Status] In Progress Admin Page React-powered dashboard under the Jetpack menu Connect Flow Connection banners, buttons, ... labels Dec 7, 2018
@jeherve jeherve added this to the 6.9 milestone Dec 7, 2018
@jeherve jeherve self-assigned this Dec 7, 2018
@jeherve jeherve requested a review from a team December 7, 2018 18:39
@jetpackbot
Copy link

jetpackbot commented Dec 7, 2018 •
edited
Loading

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: January 10, 2019.
Scheduled code freeze: January 3, 2019

Generated by 🚫 dangerJS against ab976fb

@joanrho
Copy link
Contributor

joanrho commented Dec 7, 2018

@jeherve Thank you! This looks wonderful and works as expected.

The Plugins page modal is just perfect and the Jetpack Dashboard page looks great as well (I saw your note above about the layout issue at the bottom of the connection modal on this page—that was my only comment while testing).

@jeherve
Copy link
Member Author

jeherve commented Dec 7, 2018

Note that we'll also need to test what happens when you disconnect from WordPress.com, as I removed elements from the dashboard that are usually displayed there once you disconnect.

@joanrho
Copy link
Contributor

joanrho commented Dec 8, 2018

Props to @MichaelArestad for the commit that fixed the CSS! 🌟🎉

@jeherve jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Dec 10, 2018
@jeherve jeherve requested a review from a team December 11, 2018 16:46
@jeherve jeherve added the [Status] Needs Design Review Design has been added. Needs a review! label Dec 11, 2018
keoshi
keoshi requested changes Dec 11, 2018
View reviewed changes
Copy link
Contributor

@keoshi keoshi left a comment

Choose a reason for hiding this comment

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

Go to the Jetpack menu. You should see the same modal.

I did not see the modal appear on this page after activating the plugin. Am I doing something wrong?

@jeherve
Copy link
Member Author

jeherve commented Dec 11, 2018

Was Jetpack previously connected to WordPress.com on your site? I would recommend that you try after disconnecting from WordPress.com in the Jetpack menu, then deactivating Jetpack, then activating it back.

Let me know how that goes!

@keoshi
Copy link
Contributor

keoshi commented Dec 11, 2018

@jeherve Nope, fresh site with no previous connection.

keoshi
keoshi previously approved these changes Dec 11, 2018
View reviewed changes
Copy link
Contributor

@keoshi keoshi left a comment

Choose a reason for hiding this comment

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

Finally was able to test this properly. Jetpack beta seems to lose the branch association every time I deactivate JP, but this worked for me:

Deactivated JP > Opened JP Beta > Reselected the branch (nothing seemed to be selected) > JP was auto-activated > Opened Jetpack and saw the modal

Looks good, let's 🚢

@jeherve jeherve mentioned this pull request Dec 11, 2018
Closed
@keoshi keoshi removed the [Status] Needs Design Review Design has been added. Needs a review! label Dec 11, 2018
@oskosk
Copy link
Contributor

oskosk commented Dec 13, 2018

Was about to rebase but I don't know what's happening here with jetpack-speed.svg. Seems like you're introducing it here but also @jeffgolenski already introduced it in #10867

jeherve and others added 9 commits December 14, 2018 09:46
@jeherve
Connection: show full screen connection banner modal in JP dash
e55c8b3 
That modal was only loaded in the Plugins screen; let's load it in the Jetpack menu as well.
@jeherve
Remove JetpackConnect component
5ce67a4
@jeherve
Full connection banner: update content.
8d24ed0 
Based on mockup here:
#10563 (comment)
@jeherve
Change from parameter for connect URLs to match current page
cbc1a43
@jeherve
Remove call to deleted scss file
24060e3
@jeherve
Always display the new modal as long as you are not connected
a081432
@MichaelArestad @jeherve
Connection modal: fixed visual bugs
809ed75 
* fixed top and bottom margin
* now mobile friendly
* including flexbox
* no longer on top of menus or colliding with sidebar
@jeherve
Only display modal on Jetpack admin page.
b666f29
@jeherve
Force refresh of the page when you disconnect from WordPress.com
ab976fb 
This forces the connection modal to be displayed again.
@jeherve jeherve force-pushed the update/disconnected-jetpack-dashboard branch from f195d52 to ab976fb Compare December 14, 2018 08:58
@jeherve
Copy link
Member Author

jeherve commented Dec 14, 2018

I have now rebased.

oskosk
oskosk approved these changes Dec 14, 2018
View reviewed changes
Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

LGTM!

@oskosk oskosk added [Status] Ready to Merge Go ahead, you can push that green button! [Status] Design Review Complete and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Dec 14, 2018
@oskosk oskosk merged commit 63049e4 into master Dec 14, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Dec 14, 2018
@oskosk oskosk deleted the update/disconnected-jetpack-dashboard branch December 14, 2018 15:35
@jeherve jeherve added [Status] Needs Changelog [Status] Needs Testing We need to add this change to the testing call for this month's release and removed [Status] Needs Changelog [Status] Needs Testing We need to add this change to the testing call for this month's release labels Dec 14, 2018
jeherve added a commit that referenced this pull request Dec 19, 2018
@jeherve
Changelog: add #10888
6bf3c5a
jeherve added a commit that referenced this pull request Jan 3, 2019
@jeherve
Changelog: add #10888
99e3257
@jeherve jeherve mentioned this pull request Jan 3, 2019
Closed
jeherve added a commit that referenced this pull request Jan 3, 2019
@jeherve
Jetpack 6.9: Changelog and testing list (#10722)
8d91603 
* Add first version of the Changelog and testing list for 6.9

* Changelog: add #10710

* changelog: add #10538

* changelog: add #10741

* changelog: add #10749

* changelog: add #10664

* changelog: add #10224

* changelog: add #10788

* Changelog: add #10560

* Chanegelog: add #10812

* changelog: add #10556

* Changelog: add #10668

* Changelog: add #10846

* Changelog: add #10947

* Changelog: add #10962

* Changelog: add #10956

* Changelog: add #10940

* Changelog: add #10934

* Changelog: add #10912

* changelog: add #10866

* changelog: add #10924

* Changelog: add #10936

* Changelog: add #10833

* changelog: add #10867

* Changelog: add #10960

* Changelog: add #10888

* changelog: add #10840

* changelog: add #10972

* Changelog: add #10979

* changelog: add #10909

* Changelog: add #10958

* Changelog: add #10981

* Changelog: add #10564

* Changelog: add #10809

* Changelog: add #10982

* Changelog: add #10706

* Changelog: add #10978

* Changelog: add #10132

* Changelog: add #11022

* Changelog: add #11024

* Changelog: add #10875

* Changelog: add #11030

* Changelog: add #11053

* Changelog: add #10880

* Changelog: add #9359

* Changelog: add #11037

* Update block list

* Changelog: add #11060

* Changelog: add #10755

* changelog: add #11000

* Changelog: add #10786

* Changelog: add #10945

* Changelog: add #10597
@jeherve jeherve mentioned this pull request May 23, 2019
Closed
jeherve added a commit that referenced this pull request May 23, 2019
@jeherve
Connect flow: remove post-disconnect survey
972841e 
Fixes #12452

This survey is not displayed to users since #10888, so there is really no need to keep this around.
@jeherve jeherve mentioned this pull request May 23, 2019
Merged
jeherve added a commit that referenced this pull request May 24, 2019
@jeherve
Connect flow: remove post-disconnect survey (#12458)
69b4b41 
Fixes #12452

This survey is not displayed to users since #10888, so there is really no need to keep this around.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oskosk oskosk oskosk approved these changes

@keoshi keoshi keoshi left review comments

Assignees

@jeherve jeherve

Labels
Admin Page React-powered dashboard under the Jetpack menu Connect Flow Connection banners, buttons, ... [Status] Design Review Complete
Projects
None yet
Milestone
6.9
Development

Successfully merging this pull request may close these issues.
7 participants
@jeherve @jetpackbot @joanrho @keoshi @oskosk @matticbot @MichaelArestad