Nav block: handle overflowing items using wrap on Block selection #18429
Conversation
Now detects hoz scroll and adds suitable class which can be used to hook CSS styles for when there is hoz scrolling.
Previously there were many issues with the editing experience and UI when allow for overflowing editing. Experiment with allow items to wrap when the Block is selected. See #18336 (comment)
| .wp-block { | ||
| display: flex; | ||
| flex: 1; // grow to own content width only | ||
| // margin-top: $block-padding*2; |
There was a problem hiding this comment.
are we going to keep these lines here?
There was a problem hiding this comment.
Yeh these are the key to the PR 😁
| if ( scrollWidth > clientWidth ) { | ||
| setHasScrollX( true ); | ||
| } else { | ||
| setHasScrollX( false ); |
There was a problem hiding this comment.
minor suggestion
what do you think about
useLayoutEffect( () => setHasScrollX( scrollWidth > clientWidth ), [ clientWidth, scrollWidth ] );Feel free to ignore it 👍
| @@ -79,6 +95,7 @@ function NavigationMenu( { | |||
| 'wp-block-navigation-menu', { | |||
| 'has-text-color': textColor.color, | |||
| 'has-background-color': backgroundColor.color, | |||
| 'has-scroll-x': hasScrollX, | |||
There was a problem hiding this comment.
It seems that the has-scroll-x' is not being used in any place of the CSS. Should we remove entirely the js code which handles it?
There was a problem hiding this comment.
We should if we decide this wrapping approach is valid.
|
I think this also touches #18337. |
|
Thank you for tackling this head-on. I'm seeing a few issues with scrollbars. This one probably shouldn't be present:
There's also a shift. There's also some sizing stuff going on:
It is very possible and even likely that I'm missing some of the complexities of the navigation menu, but just to be absolutely sure it's not a failure of my explanation, I created #18431 as a PR that simply allows wrapping using
It's simple, yes, but IMO it feels like a good and very basic baseline that unblocks us, and then we can explore separate PRs that keep all menu items on a single line and handle overflow in a dropdown menu or hamburger or something in that vein. Did I miss anything obvious? |
|
To second @jasmussen why would we wrap only on edit and not all the time? |
|
A scrollbar seems very clunky. Perhaps we collapse overflowing menu items within a "more" button. This is a fairly common pattern across the web, and would closely match how I'd expect this to work on the frontend as well. Here's a quick GIF showing how this could look/work:
|
|
@shaunandrews that is a creative approach! I like it :) I wonder what happens next:
|
Nice. It echoes my thoughts. But I wonder if the interface for editing the menu should just let items wrap, as this 3 line PR does: #18431. Picture this — you select the block to edit it, and items wrap. You deselect it and overflowing items are moved into a dropdown menu. The benefit of that approach is that we can merge #18431 as is to fix #18298, and then we can followup with a PR to enhance that further. |
@jasmussen I'm confused as to how #18431 improves upon this PR? Isn't it a duplicate? This discussion feels like we need a decision. I feel this might be best to happen on the Issue so it's more easily traceable. Do we agree? |
|
It is a duplicate, yes, it's a different approach to accomplish wrapping. What I'm suggesting with followup improvements, is something along the lines of what Shaun has suggested with a dropdown menu. I don't necessarily think that one blocks anything.
Agree on keeping it on the issue, this was spread across three PRs so a bit hard to follow. What needs deciding, though, scrollbar or wrap? |
|
Closed in favour of Wrap solution. See Closing in favour of Wrap solution. See #18298 (comment) |
An alternative attempt at #18298.
See also #18336 (comment)
This approach avoids retaining the overflow scroll when the Block is in edit mode. Instead the items are allowed to wrap thereby improving the editing experience.
The major issue is that there is quite a big change in layout when the Block is selected.
How has this been tested?
Manual testing.
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue).
Checklist: