[Site Editor]: Add fallback template content on creation #42520
Conversation
ntsekouras
added
[Type] Enhancement
ntsekouras
requested review from
ockham,
mcsf,
spacedmonkey,
mtias,
jameskoster,
TimothyBJacobs and
kevin940726
|
Size Change: +537 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
lib/compat/wordpress-6.1/class-gutenberg-rest-templates-controller.php
Outdated
Show resolved
Hide resolved
I'm finding this happens when creating a Everything else seems to be working well. |
|
@TimothyBJacobs had a few suggestions over at #42007 that might make sense to apply to this PR. I'm not quite familiar enough with the intricacies of the REST API to know what exactly they would look like when applied, but maybe @TimothyBJacobs you can leave a few notes here? 😄 |
lib/compat/wordpress-6.1/class-gutenberg-rest-templates-controller.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Okay, I guess we can keep gutenberg_get_template_hierarchy as-is if you prefer.
Overall, this is testing well. I've added a few more suggestions; I'll recap them here for easier readability:
- Let's consider using the existing endpoint route, and replying with a 302 redirect to the fallback template if the requested template doesn't exist (see).
- Let's maybe add unit tests for
gutenberg_get_template_hierarchyitself (rather than only for the endpoint) -- IMO, it's good to cover the lower-level function as well, in case we'd like to e.g. reuse it elsewhere. - Let's at least add a TODO about special cases with hyphenated post types (see).
- @jameskoster suggested having Custom templates fall back to Single, which IMO makes sense (see).
|
Thanks again for the thorough review Bernie! I haven't addressed all your comments now, but I will, at the start of next week and will follow up on this PR.
Agreed on the above.
I'll try adding this in this PR. |
ntsekouras
force-pushed
the
add/fallback-template-content-on-creation
branch
from
c137cfa
to
3a035bd
Compare
|
This is ready for another round of testing and reviews. I moved the template hierarchy calculation in client side as the template resolution in core is quite different than our use case. In our case we have a template related comment from @ockham above:
Other updates:
|
|
I can't comment on the code, but everything is working really well for me 🎉 |
|
Thanks a lot for all your work, @ntsekouras! TBH I liked it better before fc72112 😅 Can we maybe revert that? (13563e2 is fine.) I swear I came back to this PR fully expecting to sign off on it 😅 But I feel that fc72112 just adds a bunch of complexity, and the
The only information we get from the endpoint is which of these templates exists. Aside from the fact that the Other than that, fc72112 adds a lot of complexity and introduces new concepts (such as the Again, I really appreciate your work; I'm sorry if my earlier suggestions maybe nudged you into a direction you didn't really wanted this PR to go. Anyway, I'd hate for this to result in a compromise that neither of us are happy with. So let's revert fc72112, and I'll give approval -- sound good? 😊 |
|
Thanks for the feedback Bernie! It's great to discuss things to try to find the best solution possible, so no worries at all if the process takes a bit longer 😄
Actually complexity was the reason I decided to move this client side.. It's not enough to revert as the initial implementation had the issue of multiple words slugs in post types or taxonomies, etc.. So in order to implement that I think the complexity explodes, because we would need to Do you think we can do the above in php simpler than it is client side now? I think the bigger part of the js util will be the same in php, with the exception of replacing the available meta with the above extraction new logic. |
Ah right, I hadn't seen that! Thanks for clarifying 👍
I'm not 100% sure it can be done more simply on the server, but I'd say that since the server is the ultimate source of truth that knows which post types and taxonomies exist, it makes sense to do it there -- anything on the client side would have to either fetch the same info from the server via the REST API, or duplicate it via something like In terms of code, I'd vaguely do the following, given a
Does that make sense? It shouldn't introduce too much overhead in practice, since most slugs won't have that many Even if the code ends up slightly more complex, I think it's well worth it to avoid having the consumer on the client side do more legwork 🙂 (I have to admit I didn't quite get the part about "aliases for some of them like |
There was a problem hiding this comment.
Left a little remark, but otherwise LGTM!
I have one parting question, though: why compute the hierarchy on the client before requesting the .../lookup endpoint with { slug, hierarchy }? Why not just request the endpoint with { slug, isCustom, templatePrefix } and let the server do the work?
ntsekouras
force-pushed
the
add/fallback-template-content-on-creation
branch
from
fc72112
to
d1eb0d0
Compare
I moved the logic server side again with the extra args Miguel suggested. All this heavy lifting for Thank you all for the reviews and testing and especially @ockham 😄 |
There was a problem hiding this comment.
I tested a whole bunch of flows and everything is working as expected for me. Very excicted to get this merged 😁
One thing did trip me up, but it's a bit of an edge case: I had an empty category template, so when I created a template for my 'Uncategorized' post category, that was also empty. As a follow-up I wonder if it would be good to skip empty templates?
| */ | ||
| function get_template_hierarchy( $slug, $is_custom = false, $template_prefix = '' ) { |
There was a problem hiding this comment.
One concern with moving this logic to the server is accidentally committing to supporting a function forever. What do you think of:
| */ | |
| function get_template_hierarchy( $slug, $is_custom = false, $template_prefix = '' ) { | |
| * @access private | |
| * @internal | |
| */ | |
| function get_template_hierarchy( $slug, $is_custom = false, $template_prefix = '' ) { |
| */ | |
| function get_template_hierarchy( $slug, $is_custom = false, $template_prefix = '' ) { | |
| */ | |
| function _gutenberg_get_template_hierarchy( $slug, $is_custom = false, $template_prefix = '' ) { |
There was a problem hiding this comment.
I don't think we should have a gutenberg prefix in core, also the logic in this function would need to change if the template hierarchy logic changes, and I don't imagine this will happen any time soon.
There was a problem hiding this comment.
I wish we had something like https://github.com/Brain-WP/Hierarchy in the core.
|
Thank you all for landing this! |
…e content. This commit improves site editor templates by: * Adds a post meta `is_wp_suggestion` to templates created from the site editor. Why? To differentiate the templates created from the post editor in the Template panel in inspector controls and the templates suggested in site editor. See [WordPress/gutenberg#41387 Gutenberg PR 41387] for more details. * Expands the template types that can be added to the site editor to include single custom post type and specific posts templates. See [WordPress/gutenberg#41189 Gutenberg PR 41189] for more details. * Adds fallback template content on creation in site editor: * Introduces `get_template_hierarchy()` to get the template hierarchy for a given template slug to be created. * Adds a `lookup` route to `WP_REST_Templates_Controller` to get the fallback template content. See [WordPress/gutenberg#42520 Gutenberg PR 42520] for more details. * Fixes a typo in default category template's description within `get_default_block_template_types()`. See [WordPress/gutenberg#42586 Gutenberg PR 42586] for more details. * Changes field checks from `in_array()` to `rest_is_field_included()` in `WP_REST_Post_Types_Controller`. * Adds an `icon` field to `WP_REST_Post_Types_Controller` Follow-up to [53129], [52331], [52275], [52062], [51962], [43087]. Props ntsekouras, spacedmonkey, mamaduka, mburridge, jameskoster, bernhard-reiter, mcsf, hellofromTonya. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54269 602fd350-edb4-49c9-b593-d223f7449a82
…e content. This commit improves site editor templates by: * Adds a post meta `is_wp_suggestion` to templates created from the site editor. Why? To differentiate the templates created from the post editor in the Template panel in inspector controls and the templates suggested in site editor. See [WordPress/gutenberg#41387 Gutenberg PR 41387] for more details. * Expands the template types that can be added to the site editor to include single custom post type and specific posts templates. See [WordPress/gutenberg#41189 Gutenberg PR 41189] for more details. * Adds fallback template content on creation in site editor: * Introduces `get_template_hierarchy()` to get the template hierarchy for a given template slug to be created. * Adds a `lookup` route to `WP_REST_Templates_Controller` to get the fallback template content. See [WordPress/gutenberg#42520 Gutenberg PR 42520] for more details. * Fixes a typo in default category template's description within `get_default_block_template_types()`. See [WordPress/gutenberg#42586 Gutenberg PR 42586] for more details. * Changes field checks from `in_array()` to `rest_is_field_included()` in `WP_REST_Post_Types_Controller`. * Adds an `icon` field to `WP_REST_Post_Types_Controller` Follow-up to [53129], [52331], [52275], [52062], [51962], [43087]. Props ntsekouras, spacedmonkey, mamaduka, mburridge, jameskoster, bernhard-reiter, mcsf, hellofromTonya. See #56467. Built from https://develop.svn.wordpress.org/trunk@54269 git-svn-id: http://core.svn.wordpress.org/trunk@53828 1a063a9b-81f0-0310-95a4-ce76da25c4cd
…e content. This commit improves site editor templates by: * Adds a post meta `is_wp_suggestion` to templates created from the site editor. Why? To differentiate the templates created from the post editor in the Template panel in inspector controls and the templates suggested in site editor. See [WordPress/gutenberg#41387 Gutenberg PR 41387] for more details. * Expands the template types that can be added to the site editor to include single custom post type and specific posts templates. See [WordPress/gutenberg#41189 Gutenberg PR 41189] for more details. * Adds fallback template content on creation in site editor: * Introduces `get_template_hierarchy()` to get the template hierarchy for a given template slug to be created. * Adds a `lookup` route to `WP_REST_Templates_Controller` to get the fallback template content. See [WordPress/gutenberg#42520 Gutenberg PR 42520] for more details. * Fixes a typo in default category template's description within `get_default_block_template_types()`. See [WordPress/gutenberg#42586 Gutenberg PR 42586] for more details. * Changes field checks from `in_array()` to `rest_is_field_included()` in `WP_REST_Post_Types_Controller`. * Adds an `icon` field to `WP_REST_Post_Types_Controller` Follow-up to [53129], [52331], [52275], [52062], [51962], [43087]. Props ntsekouras, spacedmonkey, mamaduka, mburridge, jameskoster, bernhard-reiter, mcsf, hellofromTonya. See #56467. Built from https://develop.svn.wordpress.org/trunk@54269 git-svn-id: https://core.svn.wordpress.org/trunk@53828 1a063a9b-81f0-0310-95a4-ce76da25c4cd
…e content. This commit improves site editor templates by: * Adds a post meta `is_wp_suggestion` to templates created from the site editor. Why? To differentiate the templates created from the post editor in the Template panel in inspector controls and the templates suggested in site editor. See [WordPress/gutenberg#41387 Gutenberg PR 41387] for more details. * Expands the template types that can be added to the site editor to include single custom post type and specific posts templates. See [WordPress/gutenberg#41189 Gutenberg PR 41189] for more details. * Adds fallback template content on creation in site editor: * Introduces `get_template_hierarchy()` to get the template hierarchy for a given template slug to be created. * Adds a `lookup` route to `WP_REST_Templates_Controller` to get the fallback template content. See [WordPress/gutenberg#42520 Gutenberg PR 42520] for more details. * Fixes a typo in default category template's description within `get_default_block_template_types()`. See [WordPress/gutenberg#42586 Gutenberg PR 42586] for more details. * Changes field checks from `in_array()` to `rest_is_field_included()` in `WP_REST_Post_Types_Controller`. * Adds an `icon` field to `WP_REST_Post_Types_Controller` Follow-up to [53129], [52331], [52275], [52062], [51962], [43087]. Props ntsekouras, spacedmonkey, mamaduka, mburridge, jameskoster, bernhard-reiter, mcsf, hellofromTonya. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54269 602fd350-edb4-49c9-b593-d223f7449a82
What?
Resolves: #36648
Alternative of: #42007, #41848, #37054
Why?
This PR finds and adds fallback template content on creation. It does so in server-side - without a strong opinion about this..
Testing Instructions
.
Examples:
categorytemplate it will be the fallback content for any specific category templatearchivetemplate(notcategory) it will be the fallback content for any specific category templatesingulartemplate it will be the fallback content for any specific postType post(single-book-hello, single-post, single-post-hi) and for page, single and attachment templateshomeNotes
indextemplate content, if no better match is found.404in a follow up.