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

Masterbar: Add recovery mode menu item. #12467

Merged
merged 1 commit into from
May 27, 2019
Merged

Conversation

jmdodd
Copy link
Member

@jmdodd jmdodd commented May 24, 2019

Allow users with the masterbar module active to exit recovery mode without needing to log out of their site.

  • This adds the core wp_admin_bar_recovery_mode_menu() menu item to the masterbar, improving compatibility of Jetpack with the new core WordPress Site Health feature.

Testing instructions:

  • Temporarily set wp_is_recovery_mode() in wp-includes/load.php to return an early true.
  • Load wp-admin and observe that the Exit Recovery Mode menu item is present in the masterbar.
  • Temporarily set wp_is_recovery_mode() to return an early false.
  • Load wp-admin and observe that the Exit Recovery Mode menu item is not present in the masterbar.

Proposed changelog entry for your changes:

  • Masterbar: Improve Site Health compatibility by adding recovery mode exit option.

Allow users with the masterbar module active to exit recovery mode without needing to log out of their site.
@jmdodd jmdodd added this to the 7.4 milestone May 24, 2019
@jmdodd jmdodd requested a review from a team as a code owner May 24, 2019 23:13
@jmdodd jmdodd added the [Feature] Masterbar WordPress.com Toolbar and Dashboard customizations label May 24, 2019
@jetpackbot
Copy link

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against d42d653

@jmdodd jmdodd added the [Status] Needs Review To request a review from Crew. Label will be renamed soon. label May 24, 2019
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 works well for me. Not blocking, but I think that maybe we could add some styling to this button in the future, just like for the original menu item?

Something like

diff --git a/modules/masterbar/overrides.css b/modules/masterbar/overrides.css
index aadbe9abf..d6a09be0d 100644
--- a/modules/masterbar/overrides.css
+++ b/modules/masterbar/overrides.css
@@ -75,6 +75,12 @@
 	font-size: 28px;
 }
 
+.jetpack-masterbar #wpadminbar #wp-admin-bar-recovery-mode {
+	background-color: #ca4a1f;
+	color: #fff;
+	margin-right: 1em;
+}
+
 @media screen and (max-width: 480px) {
 	.jetpack-masterbar.post-new-php.block-editor-page #wp-toolbar ul li {
 		flex: 1;
@@ -119,4 +125,8 @@
 	.jetpack-masterbar.post-new-php.block-editor-page #wpadminbar li#wp-admin-bar-newdash {
 		order: 3;
 	}
+
+	.jetpack-masterbar #wpadminbar #wp-admin-bar-recovery-mode {
+		display: none;
+	}
 }

What do you think?

@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 May 27, 2019
@jeherve
Copy link
Member

jeherve commented May 27, 2019

Merging this one for now. Styling can come in a future PR if needed.

@jeherve jeherve merged commit 3253238 into master May 27, 2019
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels May 27, 2019
@jeherve jeherve deleted the fix/masterbar-recovery-mode branch May 27, 2019 10:56
jeherve added a commit that referenced this pull request May 27, 2019
@jeherve
Copy link
Member

jeherve commented May 27, 2019

@Automattic/jetpack-design What's your take on the above?

image

vs.

image

Noting that I used WordPress' color here, #ca4a1f. I'm not sure what Muriel's color would be here. $muriel-hot-orange-500, #f67c00?

image

@pablohoneyhoney
Copy link

pablohoneyhoney commented May 27, 2019 via email

@jeherve
Copy link
Member

jeherve commented May 27, 2019

To give you more context, this is an element that usually appears in the admin bar on self-hosted WordPress sites.

We want to implement the same button in the WordPress.com admin bar, on Jetpack sites (self-hosted, but mostly Atomic). The button will only appear when something is wrong on your site, and you need to take action.

Here is how the button appears in WordPress core today:
image

jeherve added a commit that referenced this pull request May 27, 2019
* Kick off the changelog

* Add 7.3.1

* Update date and post link

* changelog: add #12219

* changelog: add #12170

* changelog: add #12184

* Changelog: add #12268

* Changelog: add #12081

* Changelog: add #12323

* Changelog: add #12204

* Changelog: add #12269

* Changelog: add #12332

* changelog: add #12339

* changelog: add #12209

* Changelog: add #12319

* Changelog: add #12357

* Changelog: add #12124

* Changelog: add #12373

* Changelog: add #12252

* Changelog: add #12383

* Changelog: add #12372

* changelog: add #12337

* Changelog: add #12290

* Changelog: add #12301

* Changelog: add #12061

* Testing list: add instructions for #12061

* Changelog: add #12393

* Update minimum supported version

See #12287

* Changelog: add #12406

* Testing list: add #12406

* Changelog: add #12277

* Changelog: add #12412

* Changelog: add #11318

* Changelog: add #12328

* Changelog: add #12425

* Changelog: add #12380

* Changelog: add #12428

* Changelog: add #12414

* Changelog: add #12395

* Changelog & Testing list: add #12416, #12417, #12418, and #12348

* changelog: add #12379

* Changelog: add #12341

* changelog: add #12444

* Changelog: add #12434

* Changelog: add #12454

* Changelog: add #12460

* Changelog: add #12463

* Changelog: add #12457

* Changelog / testing list: add #10333

* Changelog: add #12467


Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
@scottsweb
Copy link
Contributor

Because this button originally comes from core, I think we can stick with core colours in this instance.

If a user has the calypsoified theme enabled on wp-admin though, then I think using the orange would make more sense - that indicates to me they are probably an Atomic user.

jeherve added a commit that referenced this pull request May 29, 2019
The button was added in #12467.
This commit styles it so:
- It looks similar to the core Recovery mode button when seeing the default Masterbar.
- It uses the Calypso colors when the Masterbar is loaded by a user using Calypsoify
@pablohoneyhoney
Copy link

pablohoneyhoney commented May 31, 2019 via email

@jeherve
Copy link
Member

jeherve commented May 31, 2019

I understand the user won’t be interacting in both environments, right?

Not on Atomic, no, but on Jetpack one can freely activate and deactivate the WordPress.com toolbar feature, thus going from the core admin bar to the WordPress.com toolbar as they please.

I’d color the text in red/orange (the usual color for errors?) rather than a box/background.

Like so?

image

If so, it seems we would have to make that text bold as well:

https://webaim.org/resources/contrastchecker/?fcolor=CA4A1F&bcolor=23282D

image

Would that work?

dereksmart pushed a commit that referenced this pull request Jun 19, 2019
The button was added in #12467.
This commit styles it so:
- It looks similar to the core Recovery mode button when seeing the default Masterbar.
- It uses the Calypso colors when the Masterbar is loaded by a user using Calypsoify
jeherve added a commit that referenced this pull request Jun 27, 2019
The button was added in #12467.
This commit styles it so:
- It looks similar to the core Recovery mode button when seeing the default Masterbar.
- It uses the Calypso colors when the Masterbar is loaded by a user using Calypsoify
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Masterbar WordPress.com Toolbar and Dashboard customizations [Pri] BLOCKER
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants