-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Navigation Block / Page List: Unify menu item styles. #29975
Conversation
Size Change: -178 B (0%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
In Twenty Twenty One (not TT1 Blocks): The text size and appearance settings don't seem to apply to everything. I created a navigation block, added the all pages block, then added a Category link block. In the Typography settings, when I change the text size on the navigation block to 10 and the appearance to Extra Light, it looks like this: "Uncategorized" is the category link block. This does match the published view: In Twenty Seventeen everything seems to work correctly. |
The existence of the pages list block in the navigation screen is accidental today. It only appears if you click create from all pages in the placeholder of an empty menu. However, without the theme opt-in no blocks other than the link block should be used in the navigation block, by default. Just thought I'd mention that (and should probably open an issue asap :) ) |
Just to be sure, you mean the navigation screen, don't you? |
Yes, oops! Fixing its styling is good but I thought it's a good note. |
Yep makes sense! |
3d8a875
to
4ad0952
Compare
Note that this PR already includes the fix that was created in #30009. |
4ad0952
to
aa98ccb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me:
- Typography.
-
- Font size.
-
- Appearance.
-
- Letter Case.
- Color settings.
- Custom classes.
- Show submenu indicator icons on-off
Text decorations work for me on the parent, and the style is not applied to submenu items — from your own gifs it seems to be the expected behavior.
t19.mov
Tested on TT1 and TT1 blocks and achieved the same result.
This LGTM!
Thank you! |
Description
This PR unifies the CSS for menu items.
There is an argument for having CSS be scoped to each block, even if it means duplication. However in this case, the Page List block is a separate block that works outside of the navigation block. Therefore I would argue that it is the purview of the Navigation block to handle the styling of Page List items, when it is used inside, therefore it's appropriate to group styles for both inside navigation.
How has this been tested?
As a preface, please create and publish a number of pages, some that include at least 2 levels of subpages.
Now insert a navigation block, and in that, include both the Page List block, as well as some custom/manually inserted menu items. Please also include some custom submenu items.
Please test that menu items look identical whether they are from the Page List or custom.
Ideally, test in both the post editor and the site editor. It's good to test first with the Empty Theme from https://github.com/WordPress/theme-experiments so you see the default block rules on their own. Then you can test in other themes. For example, TwentyTwentyOne includes some blanket margin rules for blocks that make things look a bit off, but that's a pre-existing issue. TT1 blocks does not have this issue.
Please test the navigation screen, but with a non-FSE theme active (see #28874)
Screenshots
Before, editor, site editor, frontend in "empty theme":
After, editor, site editor, frontend:
GIF showing how the style properties now work identically for both types of menu items:
GIF showing the same, but for TT1 Blocks:
GIF showing the navigation screen looking slightly more uniform:
Note that the navigation screen could benefit from a separate look, happy to do so as a followup. But for now I wanted to primarily touch the block, and not regress the screen.
Checklist: