Navigation: Fix space-between #36441
Conversation
jasmussen
added
[Block] Navigation
|
Size Change: +70 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
jasmussen
force-pushed
the
try/fix-space-between
branch
from
2bcfec7
to
6742a8d
Compare
There was a problem hiding this comment.
It would be good to avoid using the legacy justification classes, because ideally they should be removed altogether once we've sorted out a smart solution for submenu alignment.
I can only reproduce this issue in the editor, not in the front end, and the culprit seems to be the extra div wrapper around the block contents.
In the front end, the content wrapper is wp-block-navigation__responsive-container-content, and wp-block-navigation__container is the navigation link wrapper. Both these classes have the flex layout styles attached. But in the editor, we have an extra nested wrapper with wp-block-navigation__container which is the only child of wp-block-navigation__responsive-container-content. Being a child of a flex container, it gets squeezed to its minimum width, so the space-between doesn't work.
I think the most harmless fix for this would be to set wp-block-navigation__responsive-container-content to display: block in the editor only, because in the editor it only ever has the one child, which is the inner block wrapper. What do you think?
|
For me in trunk, space between is broken both in the editor, and on the frontend: The idea of not using the alignment classes is a noble goal, I hope and generally do believe we can achieve it. But just to satisfy my curiosity, what is the main problem with having those Note that the key bit to solve here is to allow the inner item to grow, flex grow to be specific: I'm fairly sure that we used to have flex-grow applied by default to that container, regardless of justification. But I'm also pretty sure we removed that again as it caused problems elsewhere. Just to revisit, it does appear to be working fine with letting everything flex-grow: There's a case inside the burger where it needs to be unset again:
But that should be easy. I'm just scratching my head why I scoped it to only apply to space between. I'll have to dig a bit, but that's probably the cleaner solution, just applying it to all. |
|
Yep, here's the issue: #35722 Editor:
Frontend: This is because the navigation container is allowed to expand: Any thoughts? |
Oooh, I see it now. It's most noticeable in the front end whenever
The only problem is the longer they stay, the more likely it is that we'll hook custom behaviour onto them and thus be unable to remove them 😅 Layout removes the need for these classes in all other blocks where it's used, so ideally the same would happen here. Having said that, Navigation presents a unique set of challenges due to its complex markup structure and the variety of blocks it can house, so it may be that we're just hitting the limitations of layout here. And if this is a problem unique to Navigation (which it seems to be, at least so far) we need a custom solution. If we want to scope these styles to when Two links and Social: Two links and Search: Two links and a Page List with two items (assuming both link container and page list container have
The best overall visual solution for this would be Alternatively, what if we scoped the
And this wouldn't look too bad (it won't look perfect either way 🤷 )
And in that case we needn't use the classnames 😅 |
I suppose with no utility classes, the markup is slightly more minimal and clean — this is the primary benefit, correct? Yes, as you dove in here, space-between is a bit of a tricky one to apply to a navigation container in order to meet user expectations. Social links mixed in is a good example, and yes, it'd be nice if we could use The thing is: even if this PR doesn't get it perfectly right at this point (we might need rules to target social links, etc) — if we keep the justification class around, we should be able to fix anything the markup can throw at us. Right? I suppose we could always re-introduce or add back the classes if we encounter markup in the core blocks that won't work correctly with inheriting just a single property. But the thing is, these utility classes, just like That said, I do like the sound of this:
How would we do that in a way that worked in both editor and frontend, and was decently understandable CSS? |
Good call, thank you. Fixed: |
The justification class won't help with the problem of having two containers side by side, both set to
I was thinking something like |
jasmussen
force-pushed
the
try/fix-space-between
branch
from
9be0b9e
to
6aeb8cf
Compare
|
Wild, I had forgotten about I still do think we might want to restore/embrace the classes at some point, I feel like there are some theme and block use cases that need them. But it's a noble goal to try and absorb that complexity, so let's definitely try without them for starters. |
This is something we should definitely gather more data on! Echoing my comment on #36525, it would be good to look at use cases and see what we can solve with global styles vs what we need classes for. |
|
Thanks for your extensive help on this one 👌 |
noisysocks
removed
the
Backport to WP 6.7 Beta/RC
* Navigation: Fix space-between * Apply to page list. * Try only child.
Description
Fixes #36440.
Restores a justificiation rule to the navigation block that was removed in a refactor. This fixes the justification:
How has this been tested?
Insert a horizontal navigation block with 3 items then apply space-between on it. The stretching of the space between items should be visible in editor and frontend.
Checklist:
*.native.jsfiles for terms that need renaming or removal).