Enable always on burger menu for responsive navigation menus. #35568
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
|
Size Change: +523 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
vcanales
added
[Block] Navigation
|
Cool! How far are we from being able to implement the proposed design?
|
Closer now (b758795) 😉 The new design also proposes moving these settings to the root level panel instead of being under "Display settings", but that will require a bigger refactor; I think this is a good first step.
|
jasmussen
reviewed
Oct 13, 2021
jasmussen
reviewed
Oct 13, 2021
|
I pushed a small tweak to the inspector control, to make it look like this: I'm going to explore how we can get the Overlay Menu subheading closer. |
|
I added a separate subheading: This particular subheading style is likely one we will want to revisit and unify. In this case, the change I'm making affects all |
jasmussen
reviewed
Oct 13, 2021
| .wp-block-navigation__responsive-container-close { | ||
| display: none; | ||
| &:not(.hidden-by-default) { | ||
| &:not(.is-menu-open) { |
There was a problem hiding this comment.
The rules here are a bit hard to parse, but I think they make sense. But it would be good if a themer, perhaps @kjellr could take this PR for a spin with a theme that uses the existing responsive feature, to see that it works as it should?
There was a problem hiding this comment.
I tested this with a few existing block themes that had the responsive menu set to (what is now) "Mobile" and found no issues. @jeffikus and @scruffian are also giving it a quick spin too just to be sure but I feel pretty solid on this so far.
|
I tweaked the wording a bit: |
|
Pushed a change to simplify the two existing toggles as well, now that they have a subheading: |
|
Alright, I've pushed some polish to this one, tested it thoroughly, and it feels like it's in both a pretty solid place featurewise, and in my testing, it works as intended with no ill side effects. The only missing thing is to add some back compat to existing themes that toggled on the previous Here's some test content for an existing menu that is set to "responsive": |
I'm not sure if we should do this as there are labels from other controls inside |
For the time being, it's not touching labels, I agree that needs a more holistic look. It only touches h3's inside the inspector — and those appear to have been mostly unused for the inspector, so it feels safe enough to me. Also CC @ciampo related to the components/labels conversation around headings we had recently. |
|
This is excellent, thank you Nik for the deprecation. I'm now seeing classic menus map seamlessly to the new: I'm happy to give this PR a green check, because to me this one is solid, safe, and great. But I'd love a few more eyes on this one just to increase the confidence level. Thanks everyone, and thanks Vicente for the fast work on this one! |
ntsekouras
reviewed
Oct 13, 2021
ntsekouras
approved these changes
Oct 13, 2021
|
Thanks for the quick work on this one! |
|
Tested on Blockbase, Quadrat, Geologist, Seedlet Blocks and Mayland Blocks. Looks good on all of them. |
Tested, and it looks good to me too! Thanks everyone for contributing, fast iteration, and testing! It was beautiful waking up to all this activity 🙂 |
|
💯 |
Closed
6 tasks
4 tasks
iandunn
added a commit
to WordPress/wporg-mu-plugins
that referenced
this pull request
Oct 26, 2021
`isResponsive` was removed in WordPress/gutenberg#35568, which broke several parts of the design. See WordPress/wporg-news-2021#53
iandunn
added a commit
to WordPress/wporg-news-2021
that referenced
this pull request
Oct 26, 2021
`isResponsive` was removed in WordPress/gutenberg#35568, which broke several parts of the design. Similar changes were made in a corresponding commit to the global header/footer. Fixes #53
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 #34783
Introduces an option to keep a Navigation Menu always hidden behind a burger menu.
Classnames and text are WIP.
How has this been tested?
Screenshots
Individual Switches
Toggle Group