Navigation block: update fallback nav creation to most recently created menu #46286
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: -18 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
scruffian
reviewed
Dec 5, 2022
| @@ -277,6 +277,7 @@ function block_core_navigation_get_classic_menu_fallback() { | |||
| } | |||
|
|
|||
| // Otherwise return the most recently created classic menu. | |||
| usort( $classic_nav_menus, fn( $a, $b) => $b->term_id - $a->term_id ); | |||
There was a problem hiding this comment.
I think this syntax was introduced in PHP 7.4. WordPress says it requires PHP 7.4, but I don't see any use of arrow functions in WordPress or Gutenberg, so maybe it's better to avoid them?
There was a problem hiding this comment.
Fixed in 3871774f423eed36338a57633418c3e8be4a3f6f
scruffian
reviewed
Dec 5, 2022
| @@ -305,7 +305,8 @@ function Navigation( { | |||
| if ( a.locations < b.locations ) { | |||
| return 1; | |||
| } | |||
| if ( a.locations > b.locations ) { | |||
| // If no locations are assigned, we compare the term_id to determine and sort by the most recently created. | |||
| else if ( a.locations > b.locations || a.term_id < b.term_id ) { | |||
There was a problem hiding this comment.
Why are locations in this part of the calculation?
There was a problem hiding this comment.
Good question, I realized the logic was convoluted. Cleaned this up in 9abfcd5
scruffian
force-pushed
the
fix/nav-fallback-menu-order
branch
from
3871774
to
6dc44f0
Compare
|
This wasn't working for me in the editor as navigation menus have an |
|
Thanks for the fixes, this is testing correctly for me. Any other feedback to address? |
mpkelly
pushed a commit
to mpkelly/gutenberg
that referenced
this pull request
Dec 7, 2022
…ed menu (WordPress#46286) * Sort fallback classic menu conversion by most recently created. * Separate sorting calc from location calc. * Use standard anonymous function syntax. * Format php. * change the property name and simplify the functions Co-authored-by: Ben Dwyer <ben@scruffian.com>
82 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 sorts the classic menus so the most recently created menu is used in the series of fallback nav creation.
If there is a menu named primary or has a primary location, that menu still takes precedence.
Why?
Before #45976, a navigation would only be created if the site had a single classic menu. Now that the fallback system can handle multiple cases, we need to consider which menu should be prioritized in the fallback order.
The WP function that get classic menus returns an array of menus sorted by name, alphabetically by default. So we need to instead sort by their
term_idif we want the most recently created.How?
See above.
Testing Instructions
wp_navigations previouslywp_navigation, repeat step 4 but visiting the front-end first, then the editor.Testing Instructions for Keyboard
Screenshots or screencast