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

Use dynamic Jetpack logos on JITMs #15096

Merged
merged 3 commits into from
Mar 27, 2020
Merged

Use dynamic Jetpack logos on JITMs #15096

merged 3 commits into from
Mar 27, 2020

Conversation

pidgeonman
Copy link
Contributor

@pidgeonman pidgeonman commented Mar 24, 2020 •
edited by jeherve
Loading

Changes proposed in this Pull Request:

Depending on the different areas of wp-admin, show just the green Jetpack logo in a JITM when a larger one is already displayed, like in the Jetpack dashboard. Everywhere else, a larger logo (with the "Jetpack" text) should be displayed, provided a Jetpack logo doesn't already exist in the area.

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

This modifies some graphical aspects of the Jetpack experience (JITMs) to improve brand placement. Please refer to this Asana task for more info:
1165973696403626-as-1165991880476585

Testing instructions:

  1. Create a test JITM that should be displayed everywhere.
  2. Refer to these screenshots:
    jetpack-logo-jitm-use

Proposed changelog entry for your changes:

Use dynamic Jetpack logos on JITMs

@pidgeonman pidgeonman added [Status] Needs Review To request a review from Crew. Label will be renamed soon. Admin Page React-powered dashboard under the Jetpack menu labels Mar 24, 2020
@pidgeonman pidgeonman requested a review from a team March 24, 2020 04:21
@pidgeonman pidgeonman self-assigned this Mar 24, 2020
@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Mar 24, 2020
@jetpackbot
Copy link

jetpackbot commented Mar 24, 2020 •
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: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against 8d6d1d5

@jeherve jeherve added [Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it and removed Admin Page React-powered dashboard under the Jetpack menu labels Mar 24, 2020
@jeherve jeherve added this to the 8.4 milestone Mar 24, 2020
robertf4
robertf4 reviewed Mar 24, 2020
View reviewed changes
Copy link
Contributor

@robertf4 robertf4 left a comment

Choose a reason for hiding this comment

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

I'm not sure why this is happening, but I see the full Jetpack logo being unusually small and the text is green, like this:

image

robertf4
robertf4 reviewed Mar 24, 2020
View reviewed changes
packages/logo/src/class-logo.php Outdated Show resolved Hide resolved
packages/logo/src/class-logo.php Outdated Show resolved Hide resolved
@robertf4 robertf4 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 Mar 24, 2020
@pidgeonman pidgeonman 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 Mar 26, 2020
robertf4
robertf4 approved these changes Mar 26, 2020
View reviewed changes
Copy link
Contributor

@robertf4 robertf4 left a comment

Choose a reason for hiding this comment

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

Looks great, tests great! 🚢

@jeffgolenski jeffgolenski added the [Status] Needs Design Review Design has been added. Needs a review! label Mar 26, 2020
@jeffgolenski
Copy link
Member

@pidgeonman @robertf4 As a designer (not too technical), what's a good way for me to test this? Can I just spin up this branch with the beta plugin or do I need to do anything with wp-calypso as well?

@pidgeonman
Copy link
Contributor Author

@jeffgolenski , using the beta plugin and applying the branch should be enough.

@jeffgolenski
Copy link
Member

Design tested and looks great on large screens and mobile. Design approved!

@jeffgolenski jeffgolenski added [Status] Design Review Complete and removed [Status] Needs Design Review Design has been added. Needs a review! labels Mar 26, 2020
leogermani
leogermani approved these changes Mar 27, 2020
View reviewed changes
Copy link
Contributor

@leogermani leogermani left a comment

Choose a reason for hiding this comment

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

Tested both on Jetpack Dashboard and other areas of admin and looks good.

Only comment I have is that on the JP Dashboard nothgin changed. I was already seeing the small logo.

Themes page BEFORE this PR:
logo-master

Themes page AFTER this PR
log-pr

Themes page AFTER this PR on mobile.
logo-pr-mobile

@leogermani leogermani added [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 Mar 27, 2020
@pidgeonman pidgeonman merged commit b5742dd into master Mar 27, 2020
@pidgeonman pidgeonman deleted the add/use-dynamic-jitm-logos branch March 27, 2020 21:57
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 27, 2020
jeherve added a commit that referenced this pull request Mar 31, 2020
@jeherve
Changelog:add #15096
02e8cca
jeherve added a commit that referenced this pull request Mar 31, 2020
@jeherve
8.4 Changelog (#15075)
0eb0b35 
* Initial changelog entry

* Changelog: add #14904

* Changelog: add #14910

* Changelog: add #14913

* Changelog: add #14916

* Changelog: add #14922

* Changelog: add #14924

* Changelog: add #14925

* Changelog: add #14928

* Changelog: add #14840

* Changelog: add #14841

* Changelog: add #14842

* Changelog: add #14826

* Changelog: add #14835

* Changelog: add #14859

* Changelog: add #14884

* Changelog: add #14888

* Changelog: add #14817

* Changelog: add #14814

* Changelog: add #14819

* Changelog;: add #14797

* Changelog: add #14798

* Changelog: add #14802

* Changelog: add #13676

* Changelog: add #13744

* Changelog: add #13777

* Changelog: add #14446

* Changelog: add #14739

* Changelog: add #14770

* Changelog: add #14784

* Changelog: add #14897

* Changelog: add #14898

* Changelog: add #14968

* Changelog: add #14985

* Changelog: add #15044

* Changelog: add #15052

* Update to remove Podcast since it remains in Beta

* Changelog: add #14803

* Changelog: add #15028

* Changelog: add #15065

* Changelog:add #14886

* Changelog: add #15118

* Changelog: add #14990

* Changelog: add #14528

* Changelog: add #15120

* Changelog: add #15126

* Changelog: add #15049

* Chanegelog: add #14852

* Changelog: add #15090

* Changelog: add #15138

* Changelog: add #15124

* Changelog:add #15055

* Changelog: add #15017

* Changelog: add #15109

* Changelog: add #15145

* Changelog:add #15096

* Changelog:add #15153

* Changelog: add #15133

* Changelog: add #14960

* Changelog: add #15127

* Changelog: add #15056

* Copy current changelog to changelog archive.

* Clarify changelog description
@pidgeonman pidgeonman mentioned this pull request May 14, 2020
Merged
@jeherve jeherve removed the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leogermani leogermani leogermani approved these changes

@robertf4 robertf4 robertf4 approved these changes

Assignees

@pidgeonman pidgeonman

Labels
[Feature] JITM Just In Time Messages - pop-up tips and suggestions that appear on the dashboard and sidebar. [Status] Design Review Complete [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
8.4
Development

Successfully merging this pull request may close these issues.
7 participants
@pidgeonman @jetpackbot @jeffgolenski @leogermani @robertf4 @jeherve @matticbot