Navigation List view: scroll horizontally when table overflows #45966
Conversation
|
|
MaggieCabrera
changed the title
packages/block-editor/src/components/off-canvas-editor/index.js
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
MaggieCabrera
requested review from
draganescu,
getdave and
scruffian
and removed request for
ellatrix
packages/block-editor/src/components/off-canvas-editor/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/off-canvas-editor/style.scss
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/off-canvas-editor/style.scss
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/off-canvas-editor/style.scss
Outdated
Show resolved
Hide resolved
|
|
||
| .block-editor-list-view-leaf { | ||
| display: block; | ||
| max-width: $sidebar-width - (2 * $grid-unit-20); |
There was a problem hiding this comment.
What does 2 * $grid-unit-20 represent? Sidebar padding? If so shall we make a Sass var to represent that?
There was a problem hiding this comment.
yes, it's sidebar padding, I just used the same var that the actual sidebar was using, since it's not defining one itself!
There was a problem hiding this comment.
Oh that's weird. I just see this and think "oh that represents something but I'm not sure what".
If it was a var $sidebar-padding I wouldn't have to interpret the code and it would be more readable.
I think we should do it but it's up to you 🙇
packages/block-editor/src/components/off-canvas-editor/style.scss
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/off-canvas-editor/index.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/off-canvas-editor/style.scss
Outdated
Show resolved
Hide resolved
|
Feel free to ping me again when you're ready for a re-review. |
|
I still need the extra div because the class needs to go outside the table element, but I'm only using the new classes to style it. I don't think we need to remove or change the spacing for the icons now, so I removed that as well:
|
MaggieCabrera
force-pushed
the
offcanvas-wide-sidebar
branch
from
77769e8 to
5780809
Compare
|
|
||
| .offcanvas-editor-list-view-leaf { | ||
| display: block; | ||
| // sidebar width - tab panel padding |
|
@MaggieCabrera Looks like we have some random static analysis errors. I re-ran the tests but if it doesn't pass you might want to rebase first. |
t need to scroll to see edit buttons every time
Co-authored-by: Dave Smith <[email protected]>
Co-authored-by: Dave Smith <[email protected]>
Co-authored-by: Dave Smith <[email protected]>
Co-authored-by: Dave Smith <[email protected]>
Co-authored-by: Dave Smith <[email protected]>
MaggieCabrera
force-pushed
the
offcanvas-wide-sidebar
branch
from
5780809 to
86cf823
Compare
|
Tests passed, I'm merging this, but we can revisit if there are any other design/UX concerns @SaxonF |
What?
When we have menu sub menus the whole sidebar will have a scrollbar. This PR makes the list view the only part of the sidebar that will overflow because of this and makes some adjustments to keep spacing tight and improve UX like forcing the max width of each menu item to be the same as the sidebar width. That allows for us to see the edit buttons on the items you don't need to scroll to see without actually having to scroll :D
Closes #45446
Why?
It was a bad user experience to force the user to scroll the whole sidebar in these cases.
How?
I'm adding an extra class to target the table for the overflow and forcing a max width on the rows so the edit icon doesn't always appear at the far end of however width the table is.
Testing Instructions
Create a Menu with multiple sub menus nested and hover over them. Check that the scroll only affects the list view.
Screenshots or screencast