"Automated update failed" notice breaks Site Health styling

[johnjamesjacoby]

When an automated update fails, a notice is shown in WordPress Admin.

This notice appears in such a place and in a way as to make the "Site Health" page header area appear unintentionally offset by it.

Screenshot imminent.

[costdev]

A few notes on this:

• This also affects the Settings > Privacy screen, as this shares the same design language as Site Health.
• Looking at the Settings > Privacy screen, all notices that don't have the inline class are placed below the header.
  • The notices generated by maintenance_nag(), update_nag() and for multisite, site_admin_notice() all have the inline class. A solution could be to remove this class to create a consistent display.
• When the currently selected Privacy page does not exist, the resulting notice on the Settings > Privacy screen is full width.
• To reproduce this issue and test patches, you can display the maintenance nag by adding the following to a new plugin:

<?php

/**
 * Plugin Name: Show maintenance nag.
 * Description: Shows the maintenance nag.
 * Author:      WordPress Core Contributors
 * Author URI:  https://make.wordpress.org/core
 * License:     GPLv2 or later
 * Version:     1.0.0
 */

add_action(
        'admin_init',
        function () {
                global $upgrading;
                $upgrading = true;
        }
);

[costdev]

Patch 54624.diff

[costdev]

Testing Instructions

These steps define how to reproduce the issue, and indicate the expected behavior.

Steps to Reproduce

1. Create and activate the plugin shown in this comment.
2. 🐞 Navigate to Tools > Site Health. The notice appears above the header.
3. 🐞 Navigate to Settings > Privacy. The notice appears above the header.

Expected Results

• ❌ Tools > Site Health: The notice appears above the header.
• ❌ Settings > Privacy: The notice appears above the header.

Steps to Test patch 54624.diff

1. Run grunt patch:54624.
2. Run npm run build:dev (or whichever build script your workflow uses).
3. Create and activate the plugin shown in this comment.
4. Navigate to Dashboard. Ensure that the notice has its typical location and appearance.
5. Navigate to Tools > Site Health. The notice should appear below the header and be full width.
   • If it does not, perform a hard refresh to clear your cache before posting your test results.
6. Navigate to Settings > Privacy. The notice should appear below the header and be full width.
   • If it does not, perform a hard refresh to clear your cache before posting your test results.
7. Feel free to test in responsive mode for additional test data.

Expected Results

• ✅ Dashboard: The notice has its typical location and appearance.
• ✅ Tools > Site Health: The notice should appear below the header, and be full width.
• ✅ Settings > Privacy: The notice should appear below the header, and be full width.
• ✅ Responsive mode: The notice should appear below the header, and be full width.

Test Report Icons:
🐞 <= Indicates where issue ("bug") occurs.
✅ <= Behavior is expected.
❌ <= Behavior is NOT expected.

[costdev]

@Clorith, if you have time, could you take a look at this video showing patch 54624.diff, patch 54624.diff and let me know your thoughts on this solution?

As an aside, when speaking with @TimothyBlynJacobs, we thought it might make more sense for all notices on these screens to be constrained to the max content width. This would be another ticket, but food for thought.

[Clorith]

This is actually the case for any notice displayed using the admin_notice hook, including those from plugins or themes.

It was brought up during the implementation of the Site Health Checker module that these dashboard nags would push it down, I was trying to find that discussion for reference without much luck, but I seem to recall the consensus, at that time, was that it should be left alone until a proper notification system was in place in core instead.

I can't remember the rationale for it without finding the ticket, but I can say that there is a case to be made that anything that goes under the heading becomes associated with that page, and not the WordPress install as a whole, if that's a "good" reason, as opposed to maintaining a nice visual appearance, is another topic :)

[SergeyBiryukov]

Replying to Clorith:

It was brought up during the implementation of the Site Health Checker module that these dashboard nags would push it down, I was trying to find that discussion for reference without much luck, but I seem to recall the consensus, at that time, was that it should be left alone until a proper notification system was in place in core instead.

#46651 seems to have a related discussion.

[costdev]

My reading of #46651 is that while it wasn't ideal to put notices below the header, after accessibility and design feedback, it was the most suitable available solution.

comment 23 says that the update nag "can't be moved". The current patch provides a way for us to do that.

The Notifications feature project is going out for feedback/testing and not sure when it'll land unfortunately. Until then, I think we can make this change to improve accessibility and have a consistent location for notices on the Site Health and Privacy screens.

[costdev]

Pinging @afercia and @joedolson for their input as the order of markup pertains to accessibility and thoughts on the patch, if they have time. 🙂

[audrasjb]

Before patch: Site Health

[audrasjb]

Before patch: Privacy

[audrasjb]

After patch: Site Health

[audrasjb]

After patch: Privacy

[audrasjb]

@costdev while it does look better with your patch, I think the update nag is misplaced. It should ideally be displayed on the top of the screen, and if possible in the white section of the header banner. For consistency with other screens but also because semantically it is not the content of the screen, so it shouldn't appear after the screen heading.

[costdev]

Alternative: Wrap the notices in a white container and adjust margin for consistency with other screens.

[costdev]

Thanks for the feedback @audrasjb! Can you take a look at 54624.2.diff and see if you think this works better?

FYI: Including the notices directly in the header div means changing more of the CSS due to text-align: center on the header and display: inline-grid on the nav element. Simply wrapping the notices in the container seemed the cleaner option.

[SergeyBiryukov]

54624.2.diff​ looks OK to me visually, but adding a hardcoded list of screens in wp-admin/admin-header.php does not seem ideal for future maintenance.

Personally, I would like to correct the margins, but keep the grey backgroud, I think that makes the notices more readable and more in line with other admin screens. We should also test this with multiple admin notices displayed at the same time.

54624.3.diff​ would be my suggestion here, see 54624.3.png​.

[audrasjb]

I also think it's easier to keep the notices on the very top of admin screen, before the white banner, and it's also easier to maintain since it's just a CSS tweak.

[joedolson]

Consistency is pretty important from an accessibility perspective, so I think it would be better if the order isn't changed for design reasons on a few isolated pages.

[johnjamesjacoby]

it's easier to keep the notices on the very top of admin screen, before the white banner, and it's also easier to maintain since it's just a CSS tweak.
Consistency is pretty important from an accessibility perspective

+1 to both, from me!

Visually, what @SergeyBiryukov suggested is not pleasing 😅 but I don't dislike it for the here & now 💗

---

This is me yearning for an official style-guide (cc @helen & @bueltge 💪) for the various ways that admin-area UI elements are intended to be used – and potentially how they should not be. UI like Chonky Header were not designed with the intention of anything appearing above it – not even Notices – and a semi-formal collision assessment on it would've likely revealed Notices as a blocker (cc @afercia 💪)

[costdev]

Just tested 54624.3.diff and it looks the same as shown in 54624.3.png and looks as expected on mobile.

As we have four committers in favour of 54624.3.diff, it looks like this one's ready for commit consideration.

Any takers? 🙂

[audrasjb]

Alright! I'll ship this :)
Thanks!

Self assigning for commit.

[joedolson]

Can do.

[joedolson]

Hah, we stomped on top of each other. All yours.

[audrasjb]

oh!
ok :D

[audrasjb]

In 54220:

General: Ensure admin notices are properly displayed on Site Health layout.

This changeset adds some CSS tweaks to ensure admin notices like the "Automated update failed" notice don't break the layout of the Site Health and Privacy Settings screens.

Props johnjamesjacoby, costdev, Clorith, audrasjb, SergeyBiryukov, joedolson.
Fixes #54624.