Update connect buttons in banner#8544
Conversation
|
Thanks @johnHackworth, this looks great! I see the changes in the small banner on both wp-admin Dashboard and in the Plugins directory after the full-screen banner there is dismissed. Some comments:
|
|
Visually, looks great! Nice work! |
|
I also added another small change to the colors variables and connect button colors. other than that, LGTM! |
|
Joan:
yup, sorry, I messed up the text in the PR, that's the actual param :) |
|
I've just added the full-screen banner button I was still missing to the PR:
|
…er/focus states to all green buttons.
|
Just added some copy changes to the full-screen banner (as described in my kickoff post), updated the buttons on the Jetpack page (when not yet connected), and added hover styles to all green buttons. Screenshots of those changes here. Don't worry about the color variable names—I'll be standardizing them to be aligned with the Calypso color naming conventions in a separate PR (they will be $green-jetpack, $green-medium, and $green-dark). I've already defined and used $green-dark in my latest commit though, since that's the new hover/focus button border color. cc @MichaelArestad @johnHackworth for review. |
zinigor
left a comment
There was a problem hiding this comment.
Reviewed, found one place with minor indentation problems. Tested, everything looks good, except I have one question in the following comment.
| .jp-jetpack-connect__button { | ||
| background: $green-primary; | ||
| border-color: $green-secondary; | ||
| color: $white; |
There was a problem hiding this comment.
Indentation seems to be using different styles here. Let's use tabs please.
There was a problem hiding this comment.
That is my mistake. Thanks for catching!
|
Now I'm seeing buttons with new colors in different UI, are these buttons supposed to be like this? One is green, one is blue on the disconnected dashboard view: |
@zinigor This is intentional. |
|
@zinigor thank you for the review, Igor! I fixed the spaces issue in the scss file |
…g customer know they've already connected the site. `auth_approved` is a connection URL param that let's a connecting customer skip the screen that shows the terms-of-service-acceptance button. In #8544 we meant to use `auth_approved`
* `already_authorized` is a connection URL param that let's a connecting customer know they've already connected the site. `auth_approved` is a connection URL param that let's a connecting customer skip the screen that shows the terms-of-service-acceptance button. In #8544 we meant to use `auth_approved` * Keep auth_approved param from WP Admin connect redir This banners may include `auth_approved` which is intended to be sent to the connection API endpoint. Ensure relevant `auth_approved` params are included if provided when redirecting to the connection endpoint.
Changes proposed in this Pull Request:
Testing instructions:
ping @joanrho and @MichaelArestad for design review