-
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
Site Editor (Experiment): Automatically open the sidebar to the appropriate template sub-menu #30098
Conversation
Size Change: +39 B (0%) Total Size: 1.41 MB
ℹ️ View Unchanged
|
Thanks @Copons this is working as expected now :) I think we'll need to see the outcome of 29630 before merging, but this is in a good spot imo. |
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.
So far it looks good and works as expected! Tests are failing on the base branch. Once that PR is merged we can circle back to this.
import { | ||
getTemplatesLocationMap, | ||
getUnusedTemplates, | ||
} from '../template-hierarchy'; |
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.
Thanks for extracting these!
const templates = select( coreDataStore ).getEntityRecords( | ||
'postType', | ||
'wp_template', | ||
{ | ||
per_page: -1, | ||
} | ||
); | ||
const showOnFront = select( coreDataStore ).getEditedEntityRecord( | ||
'root', | ||
'site' | ||
).show_on_front; | ||
|
||
if ( | ||
isTemplateSuperseded( | ||
template.slug, | ||
map( templates, 'slug' ), | ||
showOnFront | ||
) | ||
) { | ||
return MENU_TEMPLATES_UNUSED; | ||
} | ||
|
||
return getTemplateLocation( template.slug ); |
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 really love this 😄 Great use of existing functions!
884913c
to
4829b76
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.
One small thing I've noticed code quality wise.
packages/edit-site/src/components/navigation-sidebar/navigation-panel/template-hierarchy.js
Outdated
Show resolved
Hide resolved
4829b76
to
14bfe0e
Compare
Description
Follow up to #26964
The previous PR only took care of Template Parts, as they are arguably easier to handle.
Their sub-menu location is explicitly contained by their post objects, so there's not much to figure out.
Templates locations, on the other hand, depend on other templates as well.
While most commonly templates will belong to a precise location as defined in the Navigation Panel constants, at times a template might be superseded by another (e.g.
front-page
supersedeshome
), and end up grouped in their own "Unused" sub-menu.Most of the groundwork was already laid down in #28291, by creating specific helper functions for the menu rendering.
This PR takes those functions and moves them into the
template-hierarchy.js
utilities file, to share them with thegetCurrentTemplateNavigationPanelSubMenu
selector introduced in the previous PR.NOTE: I'm not overly happy with how I've handled this move, and I think I can do better.
How has this been tested?
Screenshots
Types of changes
Checklist: