The admin menu remains sticky when displaying warning or error messages on the Site Health page.

[dhrumilk]

Improper Sticky Behavior of Admin Menu when displaying warning or error messages on the Site Health page.

Refrance: ​https://www.loom.com/share/3565aaf2d04e4b0b87dee361ee8335bb

[audrasjb]

Can't reproduce the issue

[audrasjb]

Hello and thanks for the ticket,

Unfortunately, I can't reproduce the issue. I tried to reproduce it by throwing a warning using a division by zero in the Hello Dolly plugin. See the above screenshot.

Would you mind providing exact reproduction steps, please? Thanks!

[dhrumilk]

@audrasjb I have attached the screenshot below for the details of my wordpress.
and I am getting issues in the wordpress trunk version, not in beta. Thanks!

[audrasjb]

Ok I wasn't able to reproduce the issue because I wasn't triggering the warning on the good place.

For those interested, I can reproduce the exact issue using this example snippet:

add_action( 'init', function() {
	0 / 0;
} );

[mikinc860]

I have verified with the wordpress-develop trunk branch and I am able to produce this issue.

Ref video:
​https://www.loom.com/share/01260392010f4c409ecf35b572ec73b6

[chiragrathod103]

Replying to audrasjb: I think that issue will occur using the trunk branch.

Hello and thanks for the ticket,

Unfortunately, I can't reproduce the issue. I tried to reproduce it by throwing a warning using a division by zero in the Hello Dolly plugin. See the above screenshot.

Would you mind providing exact reproduction steps, please? Thanks!

[audrasjb]

As a matter of fact, this issue can be reproduced on older versions, and not only on trunk.

Removing trunk version.

[audrasjb]

By adding a margin-top to .health-check-header, I was able to fix the visual issue with the white Site Health banner not aligned to the admin menu.

At the first place, I thought this what was the ticket was about.
But it looks like it's more about the positioning "issue" on the admin menu when scrolling down.

In that case, I wanted to mention that this happens on every screens of the admin and not only on Site Health.

[audrasjb]

A margin-top on .health-check-header fixes the banner alignement issue

[audrasjb]

The menu positioning "issue" happens on each admin screen, not only on Health check

[dhrumilk]

Replying to audrasjb: #58449 here someone already created a ticket to solve that margin issue.

By adding a margin-top to .health-check-header, I was able to fix the visual issue with the white Site Health banner not aligned to the admin menu.

At the first place, I thought this what was the ticket was about.
But it looks like it's more about the positioning "issue" on the admin menu when scrolling down.

In that case, I wanted to mention that this happens on every screens of the admin and not only on Site Health.

Fixes https://core.trac.wordpress.org/ticket/58455
And also https://core.trac.wordpress.org/ticket/58449

[audrasjb]

Hello,

I didn't notice the other ticket, but I think ​PR 4561 should fix both #58455 and #58449.

Testing steps:

0. Ensure you have WP_DEBUG / WP_DEBUG_DISPLAY set to true

1. Copy the following sample code in your functions.php file (or in a custom mu-plugin for example) to generate a warning on the admin.

add_action( 'init', function() {
	0 / 0;
} );

2. Go to any admin screen and ensure the Admin Menu is not fixed/sticky anymore.

3. Bonus (fixes #58449): Go to Privacy Settings screen and to Site Health screen and ensure the white background header is aligned to the menu.

[Dharm1025]

a margin in admin menu on admin pages other than Privacy and Site Health pages.

[Dharm1025]

Hi @audrasjb,

Thanks for the testing steps, I have tested ​https://github.com/WordPress/wordpress-develop/pull/4561 and it fixes the fixed/sticky menu issue. After applying the patch Admin Menu is not fixed/sticky anymore.

However, I have noticed a margin in the admin menu on admin pages other than Privacy and Site Health pages. Please check the attachment I have added above.
(Attachment Screenshot 2023-06-08 at 10.15.41 AM.png)

Thanks

[oglekler]

Right now The Block editor after loading is hiding errors completely, with the patch they are becoming visible but not fully readable.

I am removing needs-testing until patch will be refreshed.

And in PHP 8+ dividing by zero is a fatal error, so, we need another notice/warning providing function to test with.

[oglekler]

I just got this white space on the Profile page in admin, but I didn't manage to see the error on top of the page and in the debug log as well. In my case, the error was: Failed opening '/var/www/html/wp-content/wp-cache-config.php'...
I activated caching with WP Super Cache and then removed this file.

/wp-admin/admin-header.php:212 (WP 6.2.2)

$error_get_last = error_get_last();

// Print a CSS class to make PHP errors visible.
if ( $error_get_last && WP_DEBUG && WP_DEBUG_DISPLAY && ini_get( 'display_errors' )
	// Don't print the class for PHP notices in wp-config.php, as they happen before WP_DEBUG takes effect,
	// and should not be displayed with the `error_reporting` level previously set in wp-load.php.
	&& ( E_NOTICE !== $error_get_last['type'] || 'wp-config.php' !== wp_basename( $error_get_last['file'] ) )
) {
	$admin_body_class .= ' php-error';
}

So, this is not so much about white space as it is about why this error was whisked out at some point and not shown anywhere.

[audrasjb]

Unfortunately, due to a few vacation days this week, I won’t be able to put together a new patch. Anyone is welcome to jump in!

[chiragrathod103]

I have created diff with fixing all additional bugs. for mobile, desktop and block editor page

[mukesh27]

Thanks @chiragrathod103 for 58455.diff​.

Quick feedback. The margin-top: 32; should be margin-top: 32px;

[oglekler]

@audrasjb will try to follow up on this.

I am wondering if the case when body class with error is appearing and the error itself is not is worth further (separate) investigation.

[chiragrathod103]

Thanks @mukesh27, here updated new diff with minor change.

[audrasjb]

Testing 58455.1.diff

[audrasjb]

Thanks for the patch!
I tested 58455.1.diff and it appears there's still a small issue on very small screens, because the notice is not located at the same place.

Also, there are a couple coding standards issues in this patch (lack of new lines in CSS files) and I'm wondering whether the two !important can be removed or not.

Let's move this ticket to 6.4!
Thanks :)

[Ankit K Gupta]

Hi everyone,

I am testing the latest patch file but getting errors when using this patch on my local

patch unexpectedly ends in middle of line
patch: **** Only garbage was found in the patch input.

Does this patch file https://core.trac.wordpress.org/attachment/ticket/58455/58455.1.diff work for you?

[wasiur195]

Replying to dhrumilk:

Improper Sticky Behavior of Admin Menu when displaying warning or error messages on the Site Health page.

Refrance: ​https://www.loom.com/share/3565aaf2d04e4b0b87dee361ee8335bb

[wasiur195]

@mukesh27 it looks like merging issue and patch needs to be refreshed again

[wasiur195]

@mukesh27 it looks like the merging issue and patch need to be refreshed again. Also, it is missing several and also needs to be refreshed a bit.
​https://prnt.sc/cpgUmPWTOVRJ

[marybaum]

Looks like this might need a new person to do a new patch. Any volunteers? And as @joemcgill points out, ideally the patch would fix what's causing the behavior. But a tweak to the UI to make it more resilient is a good thing

[oglekler]

Because we already have a patch, but it needs more work, I am adjusting keywords.

[marybaum]

#58449 was marked as a duplicate.

PR fixes the alignment of #body-content and sticky admin menu when there is PHP notice.

Note: For the block editor page(add/edit post) php notices are partially hidden by the block editor. Not sure if that falls into the scope of this ticket, any suggestions?

Trac ticket: https://core.trac.wordpress.org/ticket/58455

[ugyensupport]

Waiting for the patches to test.

[audrasjb]

Testing PR5168 - dashboard on large screen

[audrasjb]

Testing PR5168 - dashboard on small screen

[audrasjb]

Testing PR5168 - site health on large screen

[audrasjb]

Testing PR5168 - site health on small screen

[audrasjb]

Thanks for the PR.

It fixes the issue on the Site Health screen reported in this ticket and doesn't introduce any issue on other screens.

While it doesn't fix the Block Editor issue, I tend to think that this could also be another ticket.
Waiting for another testing before commit.

[Ankit K Gupta]

Test Report. ✅

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/5168

Env

• WordPress - 6.4-alpha-20230920.161551
• Web Server: Nginx
• Chrome Version - 116
• OS - macOS
• Theme: TT2
• PHP - 7.4.0
• Active Plugin - Custom plugin (to generate errors)

Actual Results

• ✅ Issue resolved with PR.

Test result

Tested the issue and confirmed that it has been resolved through the use of PR-5168.
Tested the Site Health page as well as other admin pages from the WordPress Dashboard.
Additionally, conducted testing on the sidebar navigation, including its Expand/Collapse mode for regression. Found work as expected.

Video Demonstration, after the patch:

​https://www.loom.com/share/95d4f58a96aa47aa92a254043a3d9834

[devmuhib]

Test Report

I test patch using the grunt command. According to grunt, 58455.1.diff is the latest patch. So, I tested this one.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/58455/58455.1.diff

Environment

• OS: Window 10
• Web Server: nginx/1.25.2
• PHP: 7.4.33
• WordPress: 6.4-alpha-56267-src
• Browser: Google Chrome
• Theme: Twenty-Twenty-Four

Actual Results

• ✅ Issue resolved with patch.

Supplemental Artifacts

Screenshot:

[nicolefurlan]

@audrasjb it looks like we have some passing test reports here. Does this one feel ready for commit? Do we want to split the Block Editor issue to another ticket or address it in this one?

[marybaum]

Time to commit! (According to the Tuesday 20:00 scrubbers)

[hellofromTonya]

@audrasjb is this ready for commit? And if yes, are you planning to commit it before 6.4 RC1 (Oct 17th) or even the possible unscheduled Beta 5 (Oct 16th)?

[nicolefurlan]

This ticket was discussed in today's bug scrub.

@hellofromTonya is reviewing this. Will commit or punt depending on review.

[hellofromTonya]

Patch for commit consideration: ​https://github.com/WordPress/wordpress-develop/pull/5168/

Replying to audrasjb:

Thanks for the PR.

It fixes the issue on the Site Health screen reported in this ticket and doesn't introduce any issue on other screens.

While it doesn't fix the Block Editor issue, I tend to think that this could also be another ticket.
Waiting for another testing before commit.

Following @audrasjb recommendations:

• Additional testing: comment:42 test report confirms it works as expected on the Site Health page. Reached out to @ironprogrammer for one more test report.

• Block Editor issue: I agree with @audrasjb - this issue is out-of-scope for this ticket, as it likely will need different considerations. A new ticket can be opened for it.

Pending additional testing, the patch seems to resolve the reported issue in this ticket. Thus, the patch is a commit candidate.

As I know Jb has less capacity right now, reassigning commit review to me.

[azaozz]

Replying to hellofromTonya:

Patch for commit consideration: ​https://github.com/WordPress/wordpress-develop/pull/5168/

Looking at the PR, one thing I'm wondering about it this bit:

.php-error #wpadminbar {
	position: fixed;
}

As far as I see #wpadminbar already has position: fixed, see: https://core.trac.wordpress.org/browser/trunk/src/wp-includes/css/admin-bar.css#L85.

Not sure if the above bit is needed. It is only for screens < 600px width (older smartphones?) where the admin bar is specifically set to position: absolute. On the other hand I can't test this natively at the moment.

[ironprogrammer]

Test Report

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/5168

tl;dr: This patch introduces a regression in how the admin menu scrolls for other admin pages when an error is displayed. IMHO this needs some more time.

Expected Results

Taking a quick step back, I wanted to look at how users might expect to see the admin menu without a displayed error. I believe that the sidebar menu should scroll only as far as needed to display the start and end of the menu items. Here is how this looks in 6.3:

And when an error is displayed, the menu should remain visible, where you can't "scroll past it":

This behavior is the baseline considered in this report.

Environment

• Hardware: MacBook Pro Apple M1 Pro
• OS: macOS 13.6
• Browser: Safari 16.6
• Server: nginx/1.25.2
• PHP: 8.2.11
• WordPress: 6.4-alpha-56267-src
• Active Plugins:
  • test-trac-58455 -- an mu-plugin with the following code:

    <?php
    add_action( 'init', function() { trigger_error( 'Testing Trac #58455.' ); } );
    

Actual Results

• ⚠️ On the Site Health screen, the admin menu scrolls similar to existing behavior, though the padding around the error message is improved.
• ❌ When an error is displayed on other pages, the admin menu scrolls inconsistently; sometimes fixed, other times absolute. See Figure 2.

Supplemental Artifacts

Figure 1: Comparing WordPress trunk (patched) with 6.3, the menu behaves the same.

Figure 2: Showing how Site Health scrolling "issue" now affects other pages. E.g. regression to Dashboard admin menu. Menu should not scroll away.

[hellofromTonya]

Thank you @ironprogrammer for your thorough test report.

Given the latest findings:

tl;dr: This patch introduces a regression in how the admin menu scrolls for other admin pages when an error is displayed. IMHO this needs some more time.

Moving this to 6.5 and adding changes-requested keyword to indicate additional work is needed to resolve the regression.

[hellofromTonya]

Whoops forgot to reassign the ticket back to @audrasjb, the original owner.

[swissspidy]

@audrasjb Is this on your radar still for 6.5? Looks like a punt candidate given the lack of activity.

[audrasjb]

Yes it is a punt candidate. I will even move it to Future Release given it is still not ready.