Make WordPress Core

Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#44438 closed defect (bug) (duplicate)

Prevent Admin Bar Items From Wrapping if There's More Items Than the Available Adminbar Width

Reported by: kzeni's profile KZeni Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.9.6
Component: Toolbar Keywords: needs-testing has-patch has-screenshots
Focuses: ui, css, administration Cc:

Description

I have a few sites where the number of plugins, etc. being used on the site has lead to the admin toolbar on a desktop browser wrapping to a second line. This then makes it so the drop down menu for the first row of items can't be accessed since the user then has the hover be hijacked by the second row item which is overlaid on top of the resulting drop down. This seems like a rather serious usability bug that should be addressed.

I have a proposed patch which would make the desktop size of the admin bar use flex layout in conjunction with overflow & text-overflow to make it so this problem isn't encountered. Meanwhile, the tablet & mobile sizes for the admin bar are unaffected due to the @media query being used to isolate this to 782px or wider browsers).

  1. Screenshot of before (current WordPress behavior): https://cloudup.com/cXkSHQN1UbX
    • Notice that moving the mouse down on the My Sites drop down can be intercepted by the Forms menu & the same can be said for the Performance menu being intercepted by the secondary menu items; this becomes more of an issue with narrower browsers & with more items in the admin bar.)
  2. Screenshot after (using the exact CSS I'm proposing here): https://cloudup.com/cEO--F1sMsC
    • Everything is accessible, nothing is wrapping, and the icons are still visible where the full text cannot be shown; was better than before visually & in terms of usability.

Attachments (3)

Patch for admin-bar.css (808 bytes) - added by KZeni 6 years ago.
Simply what needs to be added to wp-includes/css/admin-bar.css and then also included in the resulting admin-bar.min.css (probably before the first @media query in the CSS)
Updated patch for admin-bar.css (1.2 KB) - added by KZeni 6 years ago.
Updated version of the original Patch for admin-bar.css to make it friendly for admin bars which don't have too many items shown.
admin-bar-wrap-fix.zip (2.8 KB) - added by KZeni 6 years ago.
Here's the patch packaged as a plugin for ease of testing/implementation as needed.

Download all attachments as: .zip

Change History (22)

@KZeni
6 years ago

Simply what needs to be added to wp-includes/css/admin-bar.css and then also included in the resulting admin-bar.min.css (probably before the first @media query in the CSS)

#1 follow-up: @joyously
6 years ago

It's not exactly usable if you don't know what those items are.

I agree that it's not very usable the other way either.

The problem is that the admin bar should be for admin items, but plugins put other stuff in there. That's why themes (in the .org repo) are not allowed to affect the admin bar. Plugins should have some restrictions also.

#2 in reply to: ↑ 1 @KZeni
6 years ago

Replying to joyously:

Saying it's not very usable means the mobile admin bar for WordPress isn't very usable since that's just icons whereas this is the icons in addition to having as much of the text shown as possible.

I don't see how this would be harmful when / if the restriction of plugins adding items to the admin bar takes place, and it does make it so the admin bar is no longer broken by adding a happy medium between the mobile & full admin bar. Also, I do see the reason for plugins being able to add items to the admin bar, and this just makes it so WordPress handles that better if/when there becomes too many items to fit normally.

Again, here's a screenshot of before:
https://cldup.com/WnDrBXEPNW.png

And here's a screenshot after:
https://cldup.com/TdiUj6eaYR.png

I checked this in the current version of Safari, Chrome, and Firefox so far without issue.

Aside from more thorough browser testing, I don't see any reason this shouldn't be moved into the next WordPress release... do you? What are the next steps here?

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

This ticket was mentioned in Slack in #design by kzeni. View the logs.


6 years ago

@KZeni
6 years ago

Updated version of the original Patch for admin-bar.css to make it friendly for admin bars which don't have too many items shown.

#4 @KZeni
6 years ago

Okay, I tested to see how my patch would affect admin bars when there weren't too many items shown, and it had the secondary menu (user menu, etc.) left aligned in the item list rather than right aligned like you'd expect. As such, you'll find https://core.trac.wordpress.org/attachment/ticket/44438/Updated%20patch%20for%20admin-bar.css is the updated patch to use which fixes this issue.

Here's screenshots of the results with this updated patch.

Again, here's what WordPress currently does (overlapping items hijack hover and makes things inaccessible, and the user experience is generally broken):
https://cldup.com/KFuta_EkKa.png

Too many items to show with patch (at least show the left icon & truncate text as needed):
https://cldup.com/YuwkAezcuG.png

Fewer items shown with patch (shows normally):
https://cldup.com/rgm8FakWZ4.png

Should be ready for testing & implementation now.

@KZeni
6 years ago

Here's the patch packaged as a plugin for ease of testing/implementation as needed.

#5 @KZeni
6 years ago

Okay, the plugin is now officially available via https://wordpress.org/plugins/admin-bar-wrap-fix/ (see this plugin for updates potentially being made to the CSS being used in the meantime if I then forget to update the provided code here.)

I would still like to see this (or similar) integrated into the WordPress core so there isn't an issue without it.

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

This ticket was mentioned in Slack in #core-php by johnbillion. View the logs.


6 years ago

This ticket was mentioned in PR #352 on WordPress/wordpress-develop by KZeni.


4 years ago
#7

Trac ticket: https://core.trac.wordpress.org/ticket/44438 (has been open for some time without being really followed up on so hopefully this PR breathes some life back into this.)

The Trac ticket has a bunch of additional details regarding the issue being resolved here, screenshots, etc.

Code here is from version 1.0.3 of https://wordpress.org/plugins/admin-bar-wrap-fix/ (June 22, 2020) per that plugin ideally not being necessary to prevent issues that this resolves (with WordPress just preventing it from the start.) Somewhat of a "patch plugin" equivalent to a "feature plugin" WordPress has for forthcoming features, and it might be good to include it in the WP core at some point (if not now).

#8 @Ottaviano
4 years ago

Sure would be nice if this was finally fixed. So freaking sloppy that the admin bar content exceeds its container and spills out into the website. It’s been like that for years too, a disgrace.

#9 @sabernhardt
4 years ago

  • Focuses css added

#10 follow-up: @sabernhardt
4 years ago

Another flexbox patch is available on #28983. I think we'll want to take parts from each patch.

#11 in reply to: ↑ 10 @KZeni
4 years ago

Replying to sabernhardt:

Another flexbox patch is available on #28983. I think we'll want to take parts from each patch.

Oh, I didn't come across that when I was searching around for a fix before I submitted this & everything. I appreciate the info now, nonetheless!

It's kinda crazy to think this has been around with a ticket since 2014 to then have that become stale enough for a new ticket in 2018 (with a plugin that helps people resolve the issue until it's fixed introduced at that time) with the ticket being on the verge of going totally stale again before these recent comments.

Is there anything further to be done to get this ticket some additional attention to be considered for official adoption in a future WP version so this doesn't become stale again? It doesn't seem like the most controversial/involved update to include in an upcoming version.

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

#12 follow-up: @sabernhardt
4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Because both tickets contain a patch, I'll close this ticket in favor of the older #28983. If you would like to add anything, please comment there.

@KZeni If you would like to guide that ticket toward completion, I could assign you as the owner (let me know if I should). You would not need to be the person to create the patch, etc. The main part of the role is project management.

#13 @KZeni
4 years ago

@sabernhardt Really, I think we need someone who has more direct involvement/management of WordPress core to be included here, if that someone isn't you.

I mean, I'm watching both tickets and can chime in as needed, but we've really just been here with two reasonable patches (this one also being on GitHub) & everything that has been available for years while it just hasn't been picked up by the core team for one reason or another. I guess I'm uncertain the best way to get it included beyond what's been done regardless of which ticket we continue with moving forward (that said, I'd lean towards the newer code in this ticket rather than the other as it seems to have had additional testing & evolution through it also being exposed as part of the plugin it was made available as that the other implementation seems to be missing.)

KZeni commented on PR #352:


4 years ago
#14

Also see https://core.trac.wordpress.org/ticket/28983 as this ticket may take precedent for being older than my ticket mentioned above while discussing the same issue.

KZeni commented on PR #352:


4 years ago
#15

Also see https://core.trac.wordpress.org/ticket/28983 as this ticket may take precedent for being older than my ticket mentioned above while discussing the same issue.

#16 in reply to: ↑ 12 @KZeni
4 years ago

Replying to sabernhardt:

@KZeni If you would like to guide that ticket toward completion, I could assign you as the owner (let me know if I should). You would not need to be the person to create the patch, etc. The main part of the role is project management.

I suppose I could be the owner of the other ticket (https://core.trac.wordpress.org/ticket/28983). It seems like the current owner (or the "reported by" user, I suppose) submitted the ticket 7 years ago and hasn't done anything with it since (seeing they weren't the one that provided the patch that's currently part of that ticket & haven't made any update/comment on the ticket in all this time.) I'm at least active, in comparison, and also know about the more recent patch.

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

#17 @sabernhardt
4 years ago

Ownership assigned. If you want someone else to take over later, I think you could either un-assign yourself or re-assign to me.

KZeni commented on PR #352:


4 years ago
#18

See https://github.com/WordPress/wordpress-develop/pull/1082 as the trac ticket this was attached to (https://core.trac.wordpress.org/ticket/44438) was seen as a duplicate of https://core.trac.wordpress.org/ticket/28983. As such, the PR has been moved over to the active ticket involved here.

#19 @KZeni
4 years ago

Got it. I've closed the GitHub PR for this ticket, and so I think this trac ticket & everything can be left closed in favor of using https://core.trac.wordpress.org/ticket/28983 and https://github.com/WordPress/wordpress-develop/pull/1082 moving forward now.

Note: See TracTickets for help on using tickets.