#27762 closed defect (bug) (fixed)
Remove whitespace between menu items
Reported by: | obenland | Owned by: | obenland |
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | minor | Version: | 3.0 |
Component: | Menus | Keywords: | commit has-patch |
Focuses: | Cc: |
Description
To position nav menus horizontally, developers have to resort to the floating hack to avoid spacing between menu items. The spacing is caused by line breaks after each menu element (li
s). They were introduced in r14248 to create readable markup (I assume), which is very inconsistent today.
This might break visual backwards compatibility for themes that are using inline-block and count on the whitespace to provide spacing between menu items. Which should be an extreme edge case, since there is no whitespace in the default fallback, wp_page_menu()
.
Attachments (1)
Change History (15)
#2
@
11 years ago
This also removes the closing line break for the menus in the admin output by the Walker_Nav_Menu_Checklist
and Walker_Nav_Menu_Edit
walkers. They don't appear to be affected by the change but we could override end_el
in those sub classes if we needed to.
#3
@
11 years ago
Removing the whitespace is just a really, really sucky solution.
If you don’t want the space, then just use a negative margin or set the font-size of the parent element to 0. You can always reset that and it has the same effect without having to worry about trivial things like where you put your line breaks.
We shouldn’t forget that people do all kinds of crazy with navigation menus from filtering to using Custom Walkers. So depending on having no line breaks is not a good idea.
Also the commit in question is 4 years old, why change that now? Plenty of people don’t use a wp_nav_menu() fallback, so we cannot be sure that this is an “extreme edge case”.
#4
@
10 years ago
Removing whitespace is the only current solution that is 100% guaranteed to work. I agree, it's not perfect, but every other solution is problematic. See http://davidwalsh.name/remove-whitespace-inline-block for more details.
Setting the font-size of the parent element to 0 doesn't work on many Android devices (it's fixed in later versions, but there are millions of android devices out there).
The negative margin solution is a bad idea, since you can't always be certain of the width of a single space character. It's very hit-and-miss.
#5
@
10 years ago
I'm very much in favor of removing the whitespace for the reasons listed by @emzo.
Also the commit in question is 4 years old, why change that now?
I'm not sure the age of a commit should have any bearing on whether or not it's a good commit. If anything an old commit might need more scrutiny, not less. :)
The David Walsh article is quite nice.
#6
@
10 years ago
some more background about this issue and why this ticket was created. IMO the point here is consistency: having whitespace for both or have not *for both*. I agree with @emzo: removing white space it's the only known current solution 100% guaranteed to work. Then, it's up to theme authors CSS-fu to be aware of inline-block rendering, if they count on the whitespace to provide spacing between menu items, they're doing it wrong.
#8
@
10 years ago
Another solution is to omit the closing list item "</li>" tags entirely. While it may feel 'hacky' to some, I believe it's perfectly valid HTML5.
#9
@
9 years ago
Not suggesting to actually use it, just an update: found a new (well, new for me!) technique to fight the white-space between inline elements using Flexbox. Setting this on the container:
display: flex; align-items: center; flex-wrap: wrap;
Actually, just the first line is necessary and the other two ones are there to center children vertically and allow them to wrap. Flexbox actually changes the layout model, so it should be used wisely :) Of course, it works just on modern browsers.
Source: https://css-tricks.com/fighting-the-space-between-inline-block-elements/
See examples of all the known techniques (the Flexbox one is the last one):
http://codepen.io/chriscoyier/pen/hmlqF
#10
@
9 years ago
- Keywords commit added; good-first-bug removed
- Milestone changed from Awaiting Review to 4.4
- Owner changed from jjeaton to obenland
#13
@
9 years ago
- Keywords has-patch added; needs-refresh needs-patch removed
#14
@
6 years ago
For reference, the correct way to do this is in wp_nav_menu()
by setting 'item_spacing' => 'discard'
https://developer.wordpress.org/reference/functions/wp_nav_menu/
This seems way too easy, but the attached patch removes the line break after the closing
li
inWalker_Nav_Menu
'send_el
method.