Template Parts: Improve resolution and saving behavior. #21766
Conversation
epiqueras
requested review from
ajitbohra,
jorgefilipecosta,
Soean,
talldan and
TimothyBJacobs
as code owners
epiqueras
added
[Feature] Full Site Editing
[Type] Bug
|
Size Change: +3.07 kB (0%) Total Size: 845 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Testing these instructions on:
- 2020 (w/ demo templates) - works as expected.
- 2020-blocks (w/ demo templates) seems to have some issues, but that may be expected given conflictions between what is supplied by the demo templates and what is supplied by the block based theme?
- 2020-blocks (w/o demo templates) seems to have issues inconsistently. Half the time I try from a clean slate it seems to work as expected, the other half of the time (see below)....
Editing the header in isolation prompts saving 2 copies of the header template part:
Once saved, the changes are not persistent to the template it is used in nor the front-end. Loading the site-editor and editing the template part in isolation again then ends up giving the expected result. I have had this happen ~4 times or so, but have also had it work perfectly a couple times... 🤷♀️ so I'm not really sure what to say about that other than noting what I have seen.
P.S. It's good to see you around these parts again! 🥳
| @@ -26,20 +26,25 @@ export default function useTemplatePartPost( postId, slug, theme ) { | |||
| 'postType', | |||
| 'wp_template_part', | |||
| { | |||
| status: 'auto-draft', | |||
| status: [ 'publish', 'auto-draft' ], | |||
There was a problem hiding this comment.
This now allows the TP placeholder block to find matches for CPT TPs. 🎉
I have a very small PR open (#21623) where I removed this status field from the query as well as a css rule noted below in order to get CPT TPs previewable/insertable through the placeholder. It may make sense for me to close that out in favor of what you are doing here, but in the process of looking into that I found a few other things that may be worth noting while we are in this area (but not necessarily anything blocking this PR):
-
Inserting 'broke' recently. In the PR I mentioned inserting currently works as expected. As soon as I rebase it with master (or try it on this branch since it is more recent), inserting now inserts a blank version of the TP instead (which may load properly if the editor is reloaded after saving its parent entity). I looked around a little to see what change caused this but did not really have any luck.
-
As far as the placeholder/inserter is concerned, I can't seem to get this to find any matches for theme supplied template parts. After testing a little, it seems
getEntityRecordwith the provided Id will retrieve the theme supplied TP whilegetEntityRecordswith a query will not. I could find the twentytwenty header in 'auto-draft' status on the singular selector after knowing the Id for it, but the results from the plural selector with this query seem to ignore it. 😞 -
It seems the preview of the template parts from the placeholder/inserter is cut off at the bottom. You can really notice it if you create one that is only a couple words in a paragraph block and then try to insert it with the placeholder component. Removing
seems to fix the issue for me. Maybe we could consider removing that here (if that change makes sense to you) if Im closing out that other PR.gutenberg/packages/block-library/src/template-part/editor.scss
Lines 19 to 21 in c6fb7f9
There was a problem hiding this comment.
Inserting 'broke' recently. In the PR I mentioned inserting currently works as expected. As soon as I rebase it with master (or try it on this branch since it is more recent), inserting now inserts a blank version of the TP instead (which may load properly if the editor is reloaded after saving its parent entity). I looked around a little to see what change caused this but did not really have any luck.
This appears to be an inner blocks regression. #21804
As far as the placeholder/inserter is concerned, I can't seem to get this to find any matches for theme supplied template parts. After testing a little, it seems getEntityRecord with the provided Id will retrieve the theme supplied TP while getEntityRecords with a query will not. I could find the twentytwenty header in 'auto-draft' status on the singular selector after knowing the Id for it, but the results from the plural selector with this query seem to ignore it. 😞
I just tried header and twentytwenty-blocks in this PR and it worked as expected.
It seems the preview of the template parts from the placeholder/inserter is cut off at the bottom. You can really notice it if you create one that is only a couple words in a paragraph block and then try to insert it with the placeholder component...
I think it makes more sense to update your PR to be just that; it's unrelated to this PR.
There was a problem hiding this comment.
Thanks for the clarification!
I just tried header and twentytwenty-blocks in this PR and it worked as expected.
Oh it is great to see this working! I may have been testing it wrong, I saw a record for 'header' and 'twentytwenty' when I was looking before that I got though the Id, so I had been trying to get that one to show up (i was probably trying to get the demo template header 🤣 ). But testing on this PR with the block based themes works! 🎉
There was a problem hiding this comment.
Nice!
I just found the cause of the insertion regression. I'll ping you on the write-up :)
|
Thanks, @Addison-Stavlo, it's nice to be back 😄 🎉
🚀
You can't test this on the Twenty Twenty Blocks theme with demo templates enabled, because the <!-- wp:template-part {"slug":"header","theme":"twentytwenty"} /-->In short, demo templates are meant to be used with the regular Twenty Twenty for testing.
This was a separate issue, but I worked on a fix because it touches the same logic. What was happening was that sometimes when preloading, all the template loading code would run a second time, creating auto-drafts for non-customized template parts again. The template part IDs sent back to the site editor were from the first run, but when loading the main template, which includes the template parts, the blocks would resolve the IDs from the second run. Then, when using the template switcher, you would switch to the IDs from the first run, which resulted in having edits for two different auto-drafts. Thanks for catching this. Now it will reuse posts across runs, giving priority to published posts over auto-drafts in case the template part has been customized elsewhere just like the block does. |
There was a problem hiding this comment.
This was a separate issue, but I worked on a fix because it touches the same logic.
Nice! Thanks for fixing that as well, this all works very well for me now and looks great.
I don't know if you'l want some other eyes on this or not, but you got an approval from my end!
Closes #21512
Description
Template part resolution works by looking for a
postIdattribute on the block and falling back to looking for the theme's provided template part.htmlfile using thethemeandslugattributes. ThepostIdis only added to a template part instance after it is customized and saved. This meant that after customizing a template part, templates that referenced it only byslugandthemewould not resolve to the customized template part.This was fixed by adding an intermediate check between the
postIdand.htmlfile lookup where we check for a published template part for the appropriatethemeandslug.Additionally, there was an issue wherein customizing a template part sometimes the API appended counters (
-2) to slugs. This was fixed by using thewp_unique_post_slugfilter, just like for templates.How to test this?
headertemplate part in isolation.front-pagetemplate now renders the editedheaderboth in the editor and front end.Types of Changes
Bug Fix: Template part resolution now works as expected in templates referencing customized template parts.
Checklist: