Make WordPress Core

Opened 6 years ago

Last modified 2 years ago

#45186 new defect (bug)

Admin notice jumps from above the H1 to below it when created.

Reported by: dschalk's profile dschalk Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 2.7
Component: Administration Keywords: needs-patch
Focuses: ui, administration Cc:

Description

Admin notices first appear above the H1 of a page, and then jump to just below it one second later. It is easy to reproduce by going to Settings->General-> Click "Save changes". You will see the "Settings Saved" notice jump from above the H1 to below it. You can see it in the gif below:
https://www.dropbox.com/s/438bwkztew9349s/admin_notice.gif
This also happens for custom notices, for example any class="notice notice-warning" will also jump.

Attachments (3)

admin_notice.gif (857.2 KB) - added by dschalk 6 years ago.
gif that shows how notification jumps
0001-Removing-notice-jumps-in-settings-pages.patch (4.4 KB) - added by elpanda13@… 2 years ago.
Patch of the commit mentioned before
45186.diff (3.6 KB) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (15)

@dschalk
6 years ago

gif that shows how notification jumps

#1 @swissspidy
6 years ago

Hi and welcome to WordPress Trac!

That's an interesting report, because notices are moved below the heading on purpose to make for a better and more accessible site structure. Since this happens using JavaScript, there's a short period of time where the notice is visible before the heading until the JavaScript is loaded and executed.

#2 @SergeyBiryukov
6 years ago

  • Component changed from General to Administration

#3 @dschalk
6 years ago

Hi @swissspidy, thank you for your quick response and explanation. It was my guess as well that it had something to do with the timing of elements being loaded on the page. I hope it does not mean it would be complicated to fix. I understand it is a very minor issue.

#4 @miyarakira
4 years ago

This issue still exists in WP 5.5.3. Admin notices "jump" upon page load, as they're moved from above H1 to below it.

It makes the initial impression of the admin screen feel clunky. This is classic "flash of unstyled content" (https://en.wikipedia.org/wiki/Flash_of_unstyled_content), which deserves a resolution for improved user experience.


The JavaScript that performs this move is in wp-admin/js/common.js (https://github.com/WordPress/wordpress-develop/blob/78f451030b75a5c55c6cc1a4dae5b833ce9e003e/src/js/_enqueues/admin/common.js#L1083).

There must be a historical reason why this was implemented in the first place - probably to solve the fact that the admin_notices action is performed earlier in wp-admin/admin-header.php, before any page-specific template is loaded.


To solve this properly, I believe there needs to be a new action after the H1 element, for users to opt-in to outputting the admin notices in the correct place.

That means every existing admin screen should implement the action after the title, even custom admin screens added by users. By making the new action opt-in, it should be possible to gradually transition everyone to the new convention, and eventually the existing JavaScript "hack" of moving notices after page load can be deprecated.


For example, in the admin dashboard screen wp-admin/index.php, it would be implemented here after the H1 element: https://github.com/WordPress/wordpress-develop/blob/78f451030b75a5c55c6cc1a4dae5b833ce9e003e/src/wp-admin/_index.php#L118-L120

Following the existing logic for the admin_notices action in admin-header.php, it would look like:

<?php

if ( is_network_admin() ) {
        do_action( 'network_admin_notices_after_title' );
} elseif ( is_user_admin() ) {
        do_action( 'user_admin_notices_after_title' );
} else {
        do_action( 'admin_notices_after_title' );
}

?>

Here the new action is tentatively named admin_notices_after_title.

Since every admin screen needs to opt-in to running the action, I suppose the above logic should be wrapped in a function, i.e., admin_notices_after_title().


Well, I can see why this issue has been left unsolved. It requires a long-term approach of adopting a new convention, for everyone including WP core to output admin notices in the correct place.

In my opinion the effort will be worth it for the peace of mind, to once and for all resolve this UX issue which happens on every page load in the admin.

Last edited 4 years ago by miyarakira (previous) (diff)

#5 @oglekler
3 years ago

This jump is really annoying. And if the notice has class 'inline' the message is staying put about the H1. I was trying to understand how one hook is writing messages in two different places before I paid attention to this freak jump. I didn't look at it closely because layout shift is sadly quite normal behaviour in the admin.

The layout shift is a big problem for WordPress in general and certain plugins are using JS too much to add stuff on the page, as well as the build of the Block Editor which also shakes the layout a lot.

There is supposed to be another solution. Right now it looks like a temporary fix that stuck forever.

I've checked the 5.8.2 version before writing the comment…

#6 follow-up: @elpanda13@…
2 years ago

First of all, i'm sorry if the format is not the best, i'm new and not quite used to the standards here.

I took a look at this issue and the real main problem is not that the notification is moved but that the notification is actually placed before the H1 tag in the first place (so the system is actually doing what is told).

Here's where the code is first called in the one of settings page: https://github.com/WordPress/wordpress-develop/blob/78f451030b75a5c55c6cc1a4dae5b833ce9e003e/src/wp-admin/options-general.php#L52

There are other settings page where the template is pretty much the same.

The actual call that prints the notice is found here: https://github.com/WordPress/wordpress-develop/blob/78f451030b75a5c55c6cc1a4dae5b833ce9e003e/src/wp-admin/admin-header.php#L302-L304

There might be a reason why the code is here (probably because it didn't require to modify the template's structure, maybe?) but it seems a bit out of place to me.

I searched everywhere and unless i'm mistaken, it all points to options-head.php being used only for the settings page (and plugins maybe? this file seems to be used for back compatibility only, which got me confused as to why core is still using it?)

At this point maybe we can fix it by requiring "options-head.php" directly below the H1 on each of the settings page, it seems harmless at this point.

You can see a possible patch here: https://github.com/alejandrosan3/wordpress/commit/468bcce1a2a717374f0bfdfa673683870c9926d7

If you can confirm it as a good solution, i can send a PR.


P.S: i left the code commented out in admin-header.php on purpose but it will of course be removed if everything else is ok.

Last edited 2 years ago by elpanda13@… (previous) (diff)

@elpanda13@…
2 years ago

Patch of the commit mentioned before

This ticket was mentioned in Slack in #slackhelp by elpanda13gmailcom. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by elpanda13gmailcom. View the logs.


2 years ago

@SergeyBiryukov
2 years ago

#9 in reply to: ↑ 6 @SergeyBiryukov
2 years ago

Replying to elpanda13@…:

At this point maybe we can fix it by requiring "options-head.php" directly below the H1 on each of the settings page, it seems harmless at this point.

Hi there, thanks for the investigation and the patch!

It looks like the associated JS code that moves the notice after the first heading was introduced for WordPress 2.7 in [9569], and further adjusted in [9626], [11837], [11858], [13446], [31023], [32974], [35238], [35516], [36134], [38983]. Setting the version accordingly.

Since options-head.php has some back-compat code for plugins not using the Settings API, instead of explicitly including that file on option screens, I wonder if a cleaner solution might be to call just settings_errors() directly, like we already do on Privacy screens:

See 45186.diff. For non-core files, I think we still need to call settings_errors() in options-head.php, hence the file_exists() check with a comment. It might also be a good idea to think how plugins or themes could opt-in to a later settings_errors() call. Perhaps a new action, admin_notices_after_title, as suggested in comment:4, would be the way to go?

#10 @SergeyBiryukov
2 years ago

  • Version changed from 4.9.8 to 2.7

#11 @johnbillion
2 years ago

One concern with a new action is it would be complicated for plugins to maintain backwards compatibility and avoid duplicate messages at the same time. If a plugin added support for admin_notices_after_title it would have to retain support for the existing admin_notices which would (could?) result in a duplicate message, especially as the new hook would fire after the old hook.

Aside from that I'm in favour of any attempt to prevent these headings from jumping around. As others have pointed out, it makes pages with a large amount of JavaScript very janky.

#12 @apedog
2 years ago

notices are moved below the heading on purpose to make for a better and more accessible site structure.

I'd just like to chime in to say that I have never found this to be accessible. I wrote a script that moves the notices above the title after common.js moves them below the title.

Having the page title way up, and then multiple notices (most of which are not related to the page I'm on - promotional stuff, long jQuery Migrate Enabler texts, etc.), and then the actual page content, is confusing. I do not consider this accessible in any way.

If there was an option to disable the script moving the notices below the title - I would use it. Always.

This structure makes a lot more sense to me:

  • Notices (mostly unrelated to content)
  • Title (very related to content)
  • Content
Note: See TracTickets for help on using tickets.