-
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
Templates: Only include templates for the current post types #35802
Conversation
lib/full-site-editing/class-gutenberg-rest-templates-controller.php
Outdated
Show resolved
Hide resolved
Size Change: +8 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
The other related thing that would be good is to exclude the hierarchy templates from the dropdown (things like index, single, archive..., we should only load the custom templates that don't have any "hierarchy" meaning like we do in classic themes) |
174829c
to
be7686b
Compare
@youknowriad added exclusion logic in be7686b. |
@jasmussen, that's correct. @youknowriad updated logic as you suggested. |
I haven't tested this in action yet, but the code looks good 👍 |
@youknowriad, re-filtering: I think parts of Question: When do we usually start moving PHP parts into WP Core PRs/patches from the plugin? |
I'm worried about splitting filtering logic into two places, so I'd prefer if we move the filtering there (even without refactoring) if possible.
It seems we're 20 days before "alpha", I think that's a good time to start thinking about these things. Ideally we should have an editor lead defined though to coordinate with them and make decisions about what to include... I'm not sure when we'll get that information. |
I agree with hiding index, archive, 404, but not single post type templates. Only because having them there makes it easier for users to edit the default for post/page when they are editing that content. |
The "default template" is already added automatically in the frontend, so I think this covers your point here. |
I mean I agree that if there is a custom template assigned to page in theme.json, it should only show for page. |
Isn't default the "default" that you use when you create a new template? that the theme can define? |
No, it's the template inherited from the template hierarchy, so |
Can we change that label to what it actually is? (in a different PR) So if I am editing a page it says page automatically, not "default". |
Tested with a few different themes and with custom post types, works well. |
I updated the filtering logic, and now it's all done inside Thanks, everyone, for the feedback and testing ❤️ |
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 👍
22a4f61
to
c680bb1
Compare
Introduces 'WP_Block_Template::$default' property
c680bb1
to
9af8ab4
Compare
I pushed the required changes.
* Templates: Only include templates for the current post types * Exclude default templates when requesting templates for post type * Introduces `WP_Block_Template::$is_custom` property * Move filtering logic inside `gutenberg_get_block_templates` * Update collections params
if ( ! gutenberg_supports_block_templates() ) { | ||
return $templates; | ||
} | ||
|
||
$block_templates = gutenberg_get_block_templates( array(), 'wp_template' ); | ||
$block_templates = gutenberg_get_block_templates( array( 'post_type' => $post_type ), 'wp_template' ); |
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 this change hasn't been backported properly into Core yet.
Description
Updates block templates loading logic only to include templates for the current post type. These post types can be defined using the
customTemplates
setting in the theme.json.Fixes #31704
How has this been tested?
update/templates
branch.Screenshots
Types of changes
Bugfix
Checklist:
*.native.js
files for terms that need renaming or removal).