-
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
Adds "Template Parts" command to site editor #61287
Adds "Template Parts" command to site editor #61287
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
@annezazu @colorful-tones Do you know who I would @ to get labels and feedback on this PR? (Commands / Core Commands) |
I've assigned a few folks for PR review that I feel like could help move this forward. I think this would be a nice addition for WP 6.6 and would be happy to triage it onto the board, but I don't want contributors to feel like I'm adding more work. For now, I'll leave it off. |
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 the PR!
This PR makes sense to me, but I'd like to pass on a few points to move forward.
- I think this command should be added to
site-editor-navigation-commands.js
instead ofadmin-navigation-commands
. Then you may be wondering why the "Patterns" command exists inadmin-navigation-commands
, but I think the Patterns command should also be moved tosite-editor-navigation-commands.js
. Please refer to the explanation of #54133 for the reason why it currently exists inadmin-navigation-commands
. - This command should only be enabled if the template part actually exists. You will probably need to get the template parts via the
getEntityRecords
selector and check their number. - You could reuse the useIsTemplatesAccessible() hook to check if the site editor is accessible.
- Finally, I think here is where this command should be added. The Patterns command mentioned above should also be moved here via a separate PR.
If there is anything you do not understand in these explanations, please feel free to ask 👍
Thanks for the help on this!
I originally wrote it this way, but then the command is displayed in the initial blank state when you open the command palette in the site editor. I'm wondering if that's the ideal behavior and if we want that? (I'm not personally against it, but I assume since it's not in the main navigation on the left-hand sidebar, maybe it's not wanted in the initial command palette state?)
Can you clarify? I believe that a user can create new template parts regardless of whether any exist in the theme already, as long as it's a block theme.
From what I can tell, that hook only tells me if the current user can I'm wondering if these types of commands should rely on a slightly different hook that checks
I've opened this as a separate PR so we can move these forward together #61416 Thanks again for the feedback. |
As stated in this comment, in order to proceed with this PR, it seems necessary to resolve #61419 first. |
This is probably because you tried adding it to a command that has a 'site-editor' context? By default, a command should not be displayed if it has no context.
After running this command, I expect the "All template part" menu on the Patterns page to be focused. However, if the theme does not have any template parts and no new template parts have been created yet, this menu itself will not be displayed. This is why I suggested checking if the template part actually exists. I would like to suggest using the const getNavigationCommandLoaderPerTemplate = ( templateType ) =>
function useNavigationCommandLoader( { search } ) {
// ...
const commands = useMemo( () => {
// ...
}, [ isBlockBasedTheme, orderedRecords, history ] );
if ( records.length > 0 && templateType === 'wp_template_part' ) {
commands.push( {
name: 'core/edit-site/open-template-parts',
label: __( 'Template parts' ),
// ...
} );
}
return {
commands,
isLoading,
};
}; |
Thank you for the feedback! A few updates:
There is still a separate permissions issue that these template parts now load in the command palette for "Editor" level users, but if they select one, it send them to an error page. If it makes sense, I can open a separate PR for that. |
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.
LGTM!
- ✅ Works both inside and outside of the site editor.
- ✅ It also works with classic themes that support template parts.
There is still a separate permissions issue that these template parts now load in the command palette for "Editor" level users, but if they select one, it send them to an error page. If it makes sense, I can open a separate PR for that.
Yes, I also noticed this issue in #61444.
While creating this PR, I also noticed that individual pages, individual templates, and individual template parts were exposed to non-admin users via the command palette. These are caused by the fact that there is no permission check in the first place, so I would like to fix it in a follow-up PR.
However, I haven't started creating a PR yet, so if you're interested in working on it, I'd be happy to review it.
What?
Adds command to directly access your Template Parts while in the site editor.
Why?
This was actually a command that existed but was removed in #52817 as the Patterns and Template Parts UI were somewhat merged. However, since Template Parts are still a distinct entity and not always discoverable as something you'd find under "Patterns", this PR adds the direct link via command palette.
Closes #60443
How?
New command was added mostly matching the original but deep linking directly to template parts instead linking to Patterns and requiring an additional click to see.
Testing Instructions
cmd-k
to trigger the Command PaletteScreenshots or screencast
Needs feedback on: