Use cleaned slug to query for template part post #25030
Conversation
noahtallen
added
[Type] Bug
noahtallen
requested review from
aristath,
jeyip,
carolinan and
Addison-Stavlo
noahtallen
requested review from
ajitbohra,
Soean and
talldan
as code owners
|
Size Change: +9 B (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
Looking through the code for auto-drafting, it seems like this would be the case. Im thinking we would just need to change the 'name' value in that query array you mentioned by adding the clean_for_slug there. |
|
Looking at the format of the autodraft: $post = [
"post_title" => "test-dir/new-template-part",
"post_status" => "auto-draft",
"post_name" => "test-dir-new-template-part"
"post_type" => "wp_template_part",
];When inserting the post, both title and name are set to the slug, so it appears that only the name field is being modified here 🤔 |
|
Looking at this code though: gutenberg/lib/template-loader.php Lines 159 to 162 in 73c234a It looks like we avoid referencing the ID for auto-draft, so it would only affect things after the template part is considered "published". (That said, doesn't that mean that we are creating tons of extra autodrafts??) |
In the case of template parts in nested folders, yes that seems like thats what it would mean. But if we clean_for_slug the name field in the first query that should prevent it.
Im not exactly sure what you mean here. If it is an auto draft it checks to see if that auto draft's content match the file contents to see if it needs to create a new one, but either way it still returns the auto-drafts ID. gutenberg/lib/template-loader.php Lines 180 to 183 in 73c234a |
Ah, this is the part I was missing, thanks.
I'm struggling to find a similar PHP function :/ I'm thinking we could search for title instead of name so that we don't have to do any conversion there. |
noahtallen
requested review from
spacedmonkey and
TimothyBJacobs
as code owners
The name is formatted differently than the slug
noahtallen
force-pushed
the
fix/template-part-resolution-from-nested-directories
branch
from
d56f59a to
93abd13
Compare
|
@Addison-Stavlo I added With 93abd13, I confirmed that it no longer does that. We use |
|
Hmmm...I think I may be misunderstanding something. Original Steps:
What I did:
The template part does not display the distinct content in the new file I made |
I don't think we ever try to find all template parts provided by the theme and then pull them into the site editor. We only ever show template parts from themes which are used by templates that the theme provides. Essentially, when loading the template, we look for all template part blocks in the template, and only then try to find block template part HTML files which match the slugs in the template part blocks. So I'm not sure if it would show up otherwise.
If you do this step from the template HTML file before loading it in the site editor, it should work.
I'm not sure if this would work because
Does it show up in the list as an option? At any rate, I think you're uncovering some more limitations with template part loading. I think they might possibly apply more broadly than just template parts with
Also, just a couple tips for this step:
|
There was a problem hiding this comment.
Tested and confirmed:
✅ Subdirectory template part is properly resolved.
✅ Subdirectory template parts no longer have duplicated auto-drafts created on each load.
✅ The rest of the flow seems to continue to work as expected.
The changes look good to me. 😁 Nice catch and fix!
Ahhhh I see. I should modify the template part slug in index.html. I tried again and referencing block templates in subdirectories works as expected 🎉
Thanks for the tips Noah 🙏 |
|
I'll go ahead and merge this. I think there may still be some weird things to work out when it comes to slugs & titles and the difference with cleanForSlug, but those can be done separately. This still fixes the root issue, which is allowing themes to include template parts from subdirectories, which is now working correctly. |
noahtallen
deleted the
fix/template-part-resolution-from-nested-directories
branch
Description
Fixes #20375. Apparently, WordPress changes the title of the template part CPT to remove a character like
/and replace it with-, which is obviously problematic for paths. We "clean for slug" in several other places in the template part, so I'm just adding that here. That said....What other code could be problematic because of this? Do we need to support the character
/more directly? I wonder if this code could be problematic when trying to figure out if we need to create a new template part autodraft:gutenberg/lib/template-loader.php
Lines 147 to 157 in 73c234a
it might not be able to find existing autodrafts because it does not convert the slug.
How has this been tested?
Locally in edit site. To test, you should do the following:
block-template-partscalledtemplate-part-subdirnew-template-part.html. Copy/paste some block markup into that file and make sure you can identify what the content is by changing some paragraph content or something like that.template-part-subdir/new-template-part. (e.g. change the slug fromheaderto essentially the path to the new template part under the subdirectory)Screenshots
Checklist: