WordPress.org

Make WordPress Core

Opened 7 years ago

Last modified 2 weeks ago

#28983 assigned defect (bug)

Admin bar length or Admin bar on two line

Reported by: korobochkin Owned by: KZeni
Milestone: Future Release Priority: normal
Severity: normal Version: 4.3
Component: Toolbar Keywords: has-patch needs-testing needs-design-feedback
Focuses: ui, css Cc:

Description

On some sites I see the bad admin bar.

http://f.cl.ly/items/2Y372s072P0k2t1H142q/Screen-Shot-2014-07-22-at-14.58.10.png

And this without any external plugins stuff (Jetpack Statistic for example), except that buddy press notification counter.

The possible solutions, if screen too small:

  1. Leave only icons (like on mobile screens).
  2. Crop user name:
    • Hello, Kolya Korobochkin [avatar]
    • Hello, Kolya [avatar]
    • Hello [avatar]
    • [avatar]
  3. Admin bar on two lines (maybe hard to do, because #wpadminbar have static height and position: fixed).
  4. Add horizontal scroll without visible scrollbar (scrolling with mouse press and pull).

All options required JS and I'm sorry.

Attachments (4)

responsive-admin-bar.diff (2.7 KB) - added by terminalpixel 5 years ago.
Patch to use flexbox within the admin bar
admin-bar-wrap-fix.diff (1.9 KB) - added by KZeni 6 weeks ago.
Updated implementation that was downloaded as a diff from https://github.com/WordPress/wordpress-develop/pull/352/
updated-admin-bar-wrap-fix.diff (1.8 KB) - added by KZeni 6 weeks ago.
Updated implementation that was downloaded as a diff from https://github.com/WordPress/wordpress-develop/pull/1082/ (migrated the change into a new PR that's now associated with this ticket)
1082.diff (1.3 KB) - added by KZeni 4 weeks ago.
Updated implementation that was downloaded from https://github.com/WordPress/wordpress-develop/pull/1082.diff (Removed styles that were more problematic than helpful after more thorough testing.)

Download all attachments as: .zip

Change History (29)

#1 @SergeyBiryukov
7 years ago

  • Component changed from Menus to Toolbar

#2 @morganestes
6 years ago

  • Keywords needs-patch reporter-feedback added

Thanks for the ticket @korobochkin. Have you found that this happens at a consistent width with all plugins disabled, or does it depend on plugins that add menu items of different lengths?

#3 @afercia
5 years ago

  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 3.9.1 to 4.3

There's no need for additional menu items to reproduce this, some languages have longer strings than the English ones, not to mention users may choose to display their (potentially very long) full name so there's just not enough space before the responsive media query kicks in.

https://cldup.com/qU-s0Bo2q4.png

Probably there's not a complete solution to this but maybe setting a max-width and using text-overflow: ellipsis; on the username link could help?
Not sure when this was introduced, setting version to 4.3.

@terminalpixel
5 years ago

Patch to use flexbox within the admin bar

#4 @terminalpixel
5 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

I've created a fix for this that uses flexbox.

It is far from ideal but I think it is less buggy than the current issue. I cannot see a simple way to get a better result without either using JavaScript or reworking the admin bar html.

It should probably be tested with older browsers and plugins that make use of the admin bar.

https://i.imgur.com/Zjz9gMs.jpg

#5 @sabernhardt
7 weeks ago

  • Focuses css added

#6 @KZeni
7 weeks ago

Also see #44438 as this other ticket with its own patch (and accompanying plugin to implement the fixed behavior in the meantime without needing a core file edit via https://wordpress.org/plugins/admin-bar-wrap-fix/) was created before noticing this ticket was already out there. The plugin made back in 2018 (still maintained) does fix things in the meantime for those that need it, but I do still think this should really be officially fixed so that a plugin like that is no longer needed (plugin as a patch a-la the plugin as a feature approach [can be phased out once WP itself officially adopts it], I guess.)

As @sabernhardt mentioned in the other ticket:

I think we'll want to take parts from each patch.

Last edited 6 weeks ago by SergeyBiryukov (previous) (diff)

#7 @sabernhardt
6 weeks ago

#44438 was marked as a duplicate.

#8 @KZeni
6 weeks ago

While #44438 has been closed as being considered a duplicate & this being the ticket that preceded it. It did have a different means of addressing the issue (it uses flexbox still, but has some differentiation on other details.)

You can see the diff of the newer patch from the other ticket as a pull request (still open) on GitHub at https://github.com/WordPress/wordpress-develop/pull/352. Also, https://wordpress.org/plugins/admin-bar-wrap-fix/ effectively has that code as a standalone plugin for additional testing/development without editing the WP core.

Either way, one of the two (or a mixture of both) should be implemented to avoid this issue that's been part of WordPress for too long now.

Last edited 6 weeks ago by SergeyBiryukov (previous) (diff)

#9 @KZeni
6 weeks ago

What's the best approach to decide the best code to move forward with for the final implementation?

Do we simply go with the newer patch (https://github.com/WordPress/wordpress-develop/pull/352)? I mean, it's fully addressing things with how it is now that then doesn't need any further modifications, really. At that point, it's been more thoroughly tested in comparison per it being available as a plugin for years now (which people have submitted updates to as items were found) as well as being more recent so it's less likely to have out-of-date styling/approaches coming into play. That'd also make the implementation a bit more straightforward.

Also, should I close https://github.com/WordPress/wordpress-develop/pull/352/ in favor of re-creating a pull request under this ticket?

Open to thoughts for those more familiar/involved with the WordPress core & patch approval process than myself here.

Last edited 6 weeks ago by KZeni (previous) (diff)

@KZeni
6 weeks ago

Updated implementation that was downloaded as a diff from https://github.com/WordPress/wordpress-develop/pull/352/

#10 @sabernhardt
6 weeks ago

  • Owner set to KZeni
  • Status changed from new to assigned

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


6 weeks ago

Trac ticket: https://core.trac.wordpress.org/ticket/28983 (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; more recent releases haven't actually changed the CSS involved here) 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).

#12 @prbot
6 weeks ago

KZeni commented on PR #1082:

I have closed https://github.com/WordPress/wordpress-develop/pull/352/ per the ticket associated with that (https://core.trac.wordpress.org/ticket/44438) being seen as a duplicate of an older ticket (https://core.trac.wordpress.org/ticket/28983; what's referenced above), and so this is just migrating the PR over to the active trac ticket.

@KZeni
6 weeks ago

Updated implementation that was downloaded as a diff from https://github.com/WordPress/wordpress-develop/pull/1082/ (migrated the change into a new PR that's now associated with this ticket)

#13 @KZeni
6 weeks ago

Okay, should be all set with the new diff and new PR on GitHub with the latest code all under this one active trac ticket (myself having been added as the owner as well).

It seems the next step is getting this pushed to a core code maintainer's/reviewer's hands so it can be officially implemented so stuff like https://wordpress.org/plugins/admin-bar-wrap-fix/ being used to prevent a problematic admin bar can be phased out.

@sabernhardt Can you assist with that in any way?

Last edited 6 weeks ago by KZeni (previous) (diff)

#14 @sabernhardt
6 weeks ago

Sorry I haven't looked closely at the patch yet to see whether it already meets this, but I should share my ideas for the direction of the ticket sooner rather than later.

For a core change, I think I'd prefer:

  • preventing the links from dropping to the next line when there is no plugin-added content
  • trying to accommodate an additional 2 or 3 small links well
  • requiring a plugin to fix any problem caused by multiple and/or large plugin links
Last edited 6 weeks ago by sabernhardt (previous) (diff)

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


6 weeks ago

#16 @KZeni
6 weeks ago

@sabernhardt I guess I'm a bit lost when it comes to what "links" are being referred to in that bulleted list.

You can see in https://github.com/WordPress/wordpress-develop/pull/1082/files (and the most recent .diff file attached to the ticket; though the GitHub PR has some additional dev notes) that it's really just adding 2 code blocks to the src/wp-includes/css/admin-bar.css file that makes the admin bar take advantage of flexbox and ellipses-based text-overflow when applicable so the admin bar doesn't allow items to wrap onto a new line outside of the admin bar, if/when that happens due to any number of plugins potentially adding items to the admin bar (at that point, WordPress should be flexible in not having the admin bar break its layout at that point since it allows plugins to add items to the admin bar).

This update then doesn't rely on or otherwise involve any plugin and shouldn't conflict with any plugins that I'm aware of that any update to the admin bar's styling might have (while addressing an active issue due to the inflexibility of the admin bar as it is now [while being perfectly seamless for those not affected by this issue] & being straightforward enough that plugins that are changing the admin bar's styling should be able to accommodate reasonably quickly & easily in an update.)

Last edited 6 weeks ago by KZeni (previous) (diff)

This ticket was mentioned in Slack in #core-css by sabernhardt. View the logs.


6 weeks ago

#18 @prbot
4 weeks ago

ryelle commented on PR #1082:

I'm attaching some screenshots of the admin & frontend, with a fake plugin that adds 3 menu items. I also changed my user's display name & site title to be longer, so you can see on the frontend, that even without any extra menu items the menu wraps. These screenshots were taken on Mac/Firefox at 783px wide, which is the narrowest before the menu drops to icons-only.

I think the 100px sizing introduced here is a bit too small for items to be clearly understood, so I also tried testing with only the code in the @media screen and (min-width: 782px) { … } media query.

| | Current (trunk) | With PR applied | With only media query |

|

| WP Dashboard (no plugins) | <img width="804" alt="before-be-no-plugin" src="https://user-images.githubusercontent.com/541093/112508060-97078d80-8d65-11eb-95ee-4cc254864150.png"> | <img width="822" alt="pr-be-no-plugin" src="https://user-images.githubusercontent.com/541093/112508042-94a53380-8d65-11eb-87c9-16bb0c90e2d8.png"> | <img width="807" alt="tweaked-be-no-plugin" src="https://user-images.githubusercontent.com/541093/112508028-91aa4300-8d65-11eb-8513-970b8418f51c.png"> |
| WP Dashboard (with plugin-added items) | <img width="811" alt="before-be-plugin" src="https://user-images.githubusercontent.com/541093/112508059-97078d80-8d65-11eb-8c22-0fb28197fc88.png"> | <img width="809" alt="pr-be-plugin" src="https://user-images.githubusercontent.com/541093/112508039-940c9d00-8d65-11eb-8314-3356803097de.png"> | <img width="808" alt="tweaked-be-plugin" src="https://user-images.githubusercontent.com/541093/112508027-91aa4300-8d65-11eb-8ef8-470504e3cc88.png"> |
| Frontend | <img width="816" alt="before-fe" src="https://user-images.githubusercontent.com/541093/112508051-95d66080-8d65-11eb-9ae7-c3489cfd5b97.png"> | <img width="809" alt="pr-fe" src="https://user-images.githubusercontent.com/541093/112508035-940c9d00-8d65-11eb-88e1-b12e560e468f.png"> | <img width="807" alt="tweaked-fe" src="https://user-images.githubusercontent.com/541093/112508023-9111ac80-8d65-11eb-8f90-52a29f906b96.png"> |

#19 @ryelle
4 weeks ago

  • Keywords needs-design-feedback added

I've attached some screenshots on github (since it's easier to organize them there) - they don't embed nicely here, but you can click through here.

I'm also tagging this for design feedback - I think this is a good start, but could use some more thought about which items truncate, when should they truncate, etc.

#20 @prbot
4 weeks ago

KZeni commented on PR #1082:

That's some really nice testing there, thanks! Do you happen to have that fake plugin available for download so I can give it a shot? I can see that being helpful. 🙂

I can definitely see excluding the 2 other code blocks that aren't the primary @media screen and (min-width: 782px) { block.

Those 2 additions were from a user of the plugin having said they had used that made things nicer for them (as noted with the inline comments for the previous commit). However, I can see that there are negatives to what they've implemented in common situations & probably isn't the best option for everyone. Honestly, that code update might've been something subjective that was just a refinement (didn't address an active _issue_ or anything serious like that) for their specific site so I'm going to consider updating the PR (and my plugin-as-a-patch plugin) to exclude that.

At that point, if we just go with the code from the @media screen and (min-width: 782px) { block, does that actually just take care of it? Does anyone see it needing any adjustments after that point, or is it just a matter of testing at various sizes & with a varying number of items in the admin bar and acting accordingly (moving forward with official adoption if no issues remain)?

#21 @KZeni
4 weeks ago

Thanks again, @ryelle. I'll converse on the GitHub PR considering it appears this is syncing the conversation from there reasonably well (outside of the images/layout, possible edits then not getting synced, and a few other things where GitHub is the ideal reference.)

#22 @prbot
4 weeks ago

KZeni commented on PR #1082:

Alright, I've updated the PR since it really seems those other 2 code blocks were more problematic than they were helpful.

@KZeni
4 weeks ago

Updated implementation that was downloaded from https://github.com/WordPress/wordpress-develop/pull/1082.diff (Removed styles that were more problematic than helpful after more thorough testing.)

#23 @prbot
4 weeks ago

ryelle commented on PR #1082:

Do you happen to have that fake plugin available for download so I can give it a shot?

Sure - it's just something I put together to test this, but you can find it here.

At that point, if we just go with the code from the @media screen and (min-width: 782px) { block, does that actually just take care of it?

I also noticed a few alignment issues: the "1" here is right at the edge, and the w in New is cut off. Also, the dropdown menu is much more narrow - it looks like it matches the width of the parent item, so I'm not sure what it would do with a long submenu item.

<img width="268" alt="Screen Shot 2021-03-25 at 12 16 54 PM" src="https://user-images.githubusercontent.com/541093/112524734-8ad7fc00-8d76-11eb-9338-379f797f47b5.png">

That said, I'd like the design team to chime in too, in case they have some other specific suggestions.

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


2 weeks ago

#25 @lukecarbis
2 weeks ago

#38968 was marked as a duplicate.

Note: See TracTickets for help on using tickets.