-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Site Editor: Improve the Navigation panel's menu query #48908
Conversation
order: 'desc', | ||
orderby: 'date', |
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.
These two arguments aren't required since they are defaults, but I think it makes it easier to scan what query the component makes.
Size Change: +514 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
@@ -66,17 +71,7 @@ export default function SidebarNavigationScreenNavigationMenus() { | |||
}; | |||
}, [] ); | |||
|
|||
// Sort navigation menus by date. | |||
const orderedNavigationMenus = useMemo( |
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.
Ì think we have similar code in the navigation menu block. Is it possible to replace that one too?
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.
Is it for the same post type? Because I couldn't find one where we order on the client-side.
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.
gutenberg/packages/block-library/src/navigation/edit/index.js
Lines 204 to 214 in 8a81086
// Only autofallback to published menus. | |
const fallbackNavigationMenus = useMemo( | |
() => | |
navigationMenus | |
?.filter( ( menu ) => menu.status === 'publish' ) | |
?.sort( ( menuA, menuB ) => { | |
const menuADate = new Date( menuA.date ); | |
const menuBDate = new Date( menuB.date ); | |
return menuADate.getTime() < menuBDate.getTime(); | |
} ), | |
[ navigationMenus ] |
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.
I think the request there includes "draft" menus but the ordering is probably something we can remove?
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.
Do you mind if I do that separately? Would be easier to revert if it affects anything in the Navigation block.
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.
What?
This is a follow-up to #48689.
PR improves REST API query for the Site Editor's navigation panel.
Why?
The component was fetching all published navigation menus and sorting them by date on the client side - this isn't necessary. The REST API can handle this directly.
How?
I've updated
NAVIGATION_MENUS_QUERY
to fetch the last published navigation menu.Testing Instructions