Navigation: Fix page list issues in overlay #37444
Merged
+8
−9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jasmussen
added
[Type] Bug
|
Size Change: -90 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
kjellr
reviewed
Dec 16, 2021
| padding-left: 32px; | ||
| padding-right: 32px; | ||
| padding-left: 2em; | ||
| padding-right: 2em; |
There was a problem hiding this comment.
Does it make sense to use em-based values for the nav padding in general? It gets a little extra-spacious if you specify a larger text size:
There was a problem hiding this comment.
I guess that change looks a bit slipstreamed in. When I made that change, I actually made it to match up with the change made in #37323. But I'm happy to revisit both of these? What would be ideal in your mind?
There was a problem hiding this comment.
I think this can be handled separately. Ideally there's some other measurement we can harness for the default (maybe blockGap?). Or else, switching to rem would be a little more stable.
There was a problem hiding this comment.
I'll follow up with rems
jasmussen
changed the title
kjellr
approved these changes
Dec 16, 2021
|
Also, we should backport this one for 5.9. 👍 |
jasmussen
added
the
Backport to WP 6.7 Beta/RC
tellthemachines
removed
the
Backport to WP 6.7 Beta/RC
tellthemachines
pushed a commit
that referenced
this pull request
Dec 21, 2021
* Fix padding for page list in overlay menu. * Fix vertical stacking of menu containers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #37413.
If you mix manual/custom navigation items with a Page List, the two containers would wrap inappropriately:
This PR fixes that so it wraps correctly:
It also fixes an issue with padding, both for indentation of page list items, and bottom padding for long overlays that scroll:
How has this been tested?
Insert a navigation menu that features lots of items, and notably including both manually added items, and a Page List. Set it to always open as an overlay, then verify the menu items stack vertically on a small screen.
Checklist:
*.native.jsfiles for terms that need renaming or removal).