Fix navigation block classname issues #34344
Merged
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] Regression
jasmussen
requested review from
adamziel,
ajitbohra,
chrisvanpatten,
draganescu,
getdave,
kevin940726 and
tellthemachines
as code owners
jasmussen
commented
Aug 27, 2021
| @@ -44,6 +44,15 @@ const MyNavigation = () => ( | |||
| </Navigation> | |||
| ); | |||
| ``` | |||
| ## CSS Classes leveraged | |||
There was a problem hiding this comment.
Let me know if this is the right place and/or format for these.
There was a problem hiding this comment.
Looks good, thanks for doing that. 👍
7 tasks
|
Size Change: +30 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
|
@jasmussen I'm seeing a wider range of styling issues for the core navigation block editor styles. For this I'm turning off the 'Use theme styles' option in the editor to see what the base styles are like:
|
|
Thank you for the thorough testing! I'll get those fixed up 👌 |
|
Thanks again for the excellent bug report, it helped me trace the issue to #34258. I believe the change I just pushed fixes all the issues you mention:
|
I wanted to expand on this one, this lack of margin is intentional so as to enable navigation designs that are meant to be "lists of links". Some more details in #30805. You can still adjust the line height, or add background color, which both affect the vertical spacing:
Those margins also have reduced specificity so that you can set them via global styles. |
talldan
approved these changes
Aug 27, 2021
|
Thanks for the review. There are some issues that have to be fixed in TT1 itself, and which we can't fix in the navigation block lest we break global styles. |
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
Followup to issues caused by #34171. Classnames were changed and made generic, this PR catches a few that I missed the first time around.
How has this been tested?
Please test the navigation block empty, empty unselected, with menu items including dropdowns, with the Page List that also includes submenu items.
Please test the navigation scren.
Note that while this PR slightly improves the navigation screen, it doesn't go as far as #34281 does, which includes some further CSS changes to make the chevron metrics and rotation work better. Happy to include this here if need be, but wanted to keep it to just classname fixes for a start.
Screenshots
Before:
After:
Checklist:
*.native.jsfiles for terms that need renaming or removal).