Auto set first Navigation Menu as current if only 1 exists #39880
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
getdave
requested review from
draganescu,
scruffian,
mtias,
talldan and
tellthemachines
|
Size Change: +412 B (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
|
Tests are failing because of this line Perhaps the Nav block is stealing focus when it automatically selects the first Navigation Menu? |
|
What's in the branch now should at least allow us to test how this feels as a user. |
mtias
reviewed
Apr 1, 2022
mtias
reviewed
Apr 1, 2022
|
This works as described. I can also confirm that if a theme uses an empty navigation block the first navigation is displayed |
|
Although this does create an issue. If I have two navigation menus, one is displayed on the front end, but not in the site editor: Frontend: Site Editor We should fix this in a different PR though... |
scruffian
previously approved these changes
Apr 1, 2022
How is this happening? It should be caught by the conditional. |
This PR doesn't touch the server side render, so I assume that's already the behaviour on the frontend. |
youknowriad
reviewed
Apr 1, 2022
@scruffian Good catch 👏 @mtias It's happening because of this line which tries to get an appropriate fallback if the block is empty: gutenberg/packages/block-library/src/navigation/index.php Lines 455 to 458 in 2bdb3a4 The fallback mechanic for the front end is (in order of precedence):
This is what was agreed prior to WP 5.9. Happy to amend that code as necessary. What result should we look to achieve? Just ignore Nav Menus and use list of Pages? |
mtias
reviewed
Apr 4, 2022
|
@getdave that fallback for the front-end makes sense, we should just ensure we replicate on the back-end for empty menus not initiated by the user. We can follow up on that to not hold this one further. |
adamziel
reviewed
Apr 4, 2022
getdave
force-pushed
the
try/auto-select-first-nav-menu-if-solo
branch
from
101f02e
to
c15dd88
Compare
getdave
dismissed
scruffian’s stale review
Stale. Too many changes have happened.
draganescu
reviewed
Apr 5, 2022
draganescu
reviewed
Apr 5, 2022
draganescu
approved these changes
Apr 5, 2022
There was a problem hiding this comment.
This looks good. I don't like that we need to wait for all the menus to load now before any interaction, maybe a better way is to preload menu data, the impact IMO should not be that big. However for the purposes of this PR the code looks good and it works as described.
Co-authored-by: Andrei Draganescu <me@andreidraganescu.info>
|
Thank you for working on this, I consider it an crucial step towards making menus more intuitive and reliable to use. |
You are welcome 🙇 |
38 tasks
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.
What?
This PR causes the Navigation block to auto select the first Navigation Menu as its current active menu if there is only a single
wp_navigation.What is NOT
This PR does not:
Why?
Currently an empty Nav block requires the user to select the Navigation Menu they want to use. However, most users will probably only have a single Navigation Menu -
Primary.On Theme switch currently the user is required to reselect their Menu each time which is unwanted overhead. The change in this PR means that if there is only a single Menu then the block will automatically use that Menu. This avoids the need to reselect the Menu.
Note that if there is more than 1 Menu then we don't pursue this behaviour as we cannot determine which Menu would be the most appropriate.
How?
Check for how many Navigation Menus are available and if there is only one then auto-select that as the current Nav block's active Menu.
Testing Instructions
Start empty.Select menudropdown and clickCreate empty menu.Start empty./wp-admin/edit.php?post_type=wp_navigation).Screenshots or screencast
Screen.Capture.on.2022-03-31.at.11-50-37.mov