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

Override masterbar to show quick links on Atomic sites #18791

Merged
merged 7 commits into from
Feb 16, 2021
Merged

Override masterbar to show quick links on Atomic sites #18791

merged 7 commits into from
Feb 16, 2021

Conversation

tjcafferkey
Copy link
Contributor

@tjcafferkey tjcafferkey commented Feb 11, 2021 •
edited
Loading

Fixes Automattic/wp-calypso#49793

Changes proposed in this Pull Request:

  • Add specificity to admin menu CSS so that they get applied on Atomic sites.
  • Additional specificity will display masterbar icons on mobile

Jetpack product discussion

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

No

Testing instructions:

  • Create an Atomic site on your A8C account so that nav unification is enabled.
  • Install Jetpack Beta
  • Checkout this branch via Jetpack Beta plugin
  • Reduce viewport to mobile view and observe changes below

Repeat steps on a non-A8C account to ensure this works as expected for non-nav unified users.

Before After

Proposed changelog entry for your changes:

@jetpackbot
Copy link

jetpackbot commented Feb 11, 2021 •
edited
Loading

Scheduled Jetpack release: March 2, 2021.
Scheduled code freeze: February 22, 2021

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.

Generated by 🚫 dangerJS against 23582d3

@tjcafferkey tjcafferkey marked this pull request as ready for review February 11, 2021 12:38
@tjcafferkey tjcafferkey self-assigned this Feb 11, 2021
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello tjcafferkey! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D56893-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

@github-actions github-actions bot added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Feature] Masterbar WordPress.com Toolbar and Dashboard customizations labels Feb 11, 2021
@tjcafferkey tjcafferkey requested a review from a team February 11, 2021 16:31
@tjcafferkey tjcafferkey added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Feb 11, 2021
@mmtr mmtr added [Status] Needs Team Review and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Feb 12, 2021
mmtr
mmtr previously requested changes Feb 12, 2021
View reviewed changes
Copy link
Member

@mmtr mmtr left a comment

Choose a reason for hiding this comment

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

Pretty close, @tjcafferkey! I do see the full masterbar now, but I noted there is a weird orange box (probably related to the orange Debug menu item on the masterbar) and the avatar looks like it's misplaced.

Screen Shot 2021-02-12 at 11 37 48

Is that something that can be fixed?

@tjcafferkey
Copy link
Contributor Author

Hmm I encountered that avatar issue myself and thought I had fixed it. I'll continue to take a look at this thanks @mmtr! 👍🏻

cpapazoglou
cpapazoglou requested changes Feb 15, 2021
View reviewed changes
Copy link
Contributor

@cpapazoglou cpapazoglou 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 with and without a8c account and works well for me too. Can we just hide the "Debug" button for (max-width: 782px)

@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [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 Team Review labels Feb 15, 2021
@jeherve jeherve added this to the 9.4.1 milestone Feb 15, 2021
@jeherve jeherve modified the milestones: 9.4.1, 9.5 Feb 16, 2021
cpapazoglou
cpapazoglou approved these changes Feb 16, 2021
View reviewed changes
Copy link
Contributor

@cpapazoglou cpapazoglou left a comment

Choose a reason for hiding this comment

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

Thanks for fixing that last minor! LGTM 🚢 !

@cpapazoglou
Copy link
Contributor

I have also applied the patch in wpcom and inspected that we haven't broken something.

@cpapazoglou cpapazoglou 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 Feb 16, 2021
@cpapazoglou
Copy link
Contributor

cpapazoglou commented Feb 16, 2021 •
edited
Loading

I have restarted the failing wpcom tests

Edit: They have now passed ✅!

@jeherve jeherve 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 Feb 16, 2021
jeherve
jeherve approved these changes Feb 16, 2021
View reviewed changes
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.

This tests well for me.

@tjcafferkey tjcafferkey merged commit 9031fab into master Feb 16, 2021
@tjcafferkey tjcafferkey deleted the fix/wp-admin-bar-on-small-devices branch February 16, 2021 14:07
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 16, 2021
@tjcafferkey
Copy link
Contributor Author

tjcafferkey commented Feb 16, 2021 •
edited
Loading

Deployed to WPCOM in r221126-wpcom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve approved these changes

@mmtr mmtr mmtr approved these changes

@cpapazoglou cpapazoglou cpapazoglou approved these changes

Assignees

@tjcafferkey tjcafferkey

Labels
[Feature] Masterbar WordPress.com Toolbar and Dashboard customizations [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
9.5
Development

Successfully merging this pull request may close these issues.

Nav Unification: wp-admin bar not displayed on small devices
6 participants
@tjcafferkey @jetpackbot @matticbot @cpapazoglou @jeherve @mmtr