Full Site Editing: Add Support for Templates Default and Custom Titles and Descriptions (Second Attempt)#26636
Full Site Editing: Add Support for Templates Default and Custom Titles and Descriptions (Second Attempt)#26636
Conversation
|
Size Change: -18.1 kB (1%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
d1ae8dc to
b1e0525
Compare
noahtallen
left a comment
There was a problem hiding this comment.
This tests well for me! No major blockers from my point of view. I like it!
My only concern is tying more stuff to PHP, which makes it more difficult to re-use outside of the wp-admin context. But I don't think we can get away from that here.
+1 on following up to refresh the copy. It seems weird how so many of the template names begin with default!
|
The part that seems odd to me is how the post immediately gets saved as draft when the user selects from the dropdown, which doesn't really align with the usual behaviour of WP. If they do that and then abandon the editor, the blank draft will still be visible next time they return. Perhaps use auto-draft instead, and then hide any templates from the site editor if their status is auto-draft and post content is empty. It also shouldn't be assumed that auto-draft templates are always triggered by themes, because I notice that if you go to Appearance > Templates > Add New and abandon the editor, the auto-draft will later show up in the site editor. The suggestion above would also fix this. |
@carlomanf good point. Currently we use We also need to communicate in the UI that these So we need something to determine incontrovertibly that a template is "theme-provided and untouched by user" (I did that with a custom status in the previous PR). By forcing As an aside: currently templates created in the Site Editor are not empty by default. EDIT:
Ah now I see what you mean here. |
a3aa198 to
2b20f5a
Compare
Wouldn't this still be handy if we wanted to search templates provided by a specific theme? Or if we only wanted to show templates for the currently active theme? |
@Addison-Stavlo This is all quite complicated, so let's see. Ideally we only want to show (and use!) custom and current theme's templates: we don't really need a theme slug, since the origin is "clearly" implied. When we switch theme, custom templates won't be affected: they'll still be the first choice for their corresponding content. What happens to the old templates, though? Should we wipe |
I was thinking add the theme meta on creation so we can only use auto-drafts that correspond to the current theme. But wiping all auto-drafts on theme change/update is another option. 🤔 |
gziolo
left a comment
There was a problem hiding this comment.
I left a few comments. I'm learning the codebase so I'm sure I made some mistakes but I hope to catch up as soon as possible to be able to help with integration with other parts of Gutenberg.
packages/edit-site/src/components/navigation-sidebar/navigation-panel/new-template-dropdown.js
Outdated
Show resolved
Hide resolved
I think the best solution is to rewrite Theme meta would not work because there is also the possibility of a template file being deleted from the current theme after the auto-draft was already generated. |
Having a theme meta doesn't prevent us from being able to delete the auto-draft if there is no longer a corresponding file in the theme. But yeah, if there is no need for an auto-draft after switching themes then wiping them may make the most sense. I can't really think of a case where one could switch themes and still be using an auto-draft template? Aside from that, template parts seems a bit different. I think you could add an auto-draft template part to a custom template, and as long as it isn't ever edited it will still remain an auto draft. If we took a similar wiping approach for template parts, that scenario could create a loss of content on the custom template. 🤔 |
| return templateTitle; | ||
| }, | ||
| [ name, kind, title, key ] | ||
| ); |
There was a problem hiding this comment.
This is an awkward place to use the core/edit-site store, as it might be used outside of the Site Editor, reason why we check for the existence of select( 'core/edit-site' ) right at the beginning of it.
The intention is that when we edit templates, we want to see its correct title (as in: the one expected by the user, consistent with other places in the UI) in the edits review panel.
Since auto-draft entities don't have a title, editing for the first time a theme-provided template in the Site Editor might result in displaying an incorrect title (undefined, or the slug, etc.).
There was a problem hiding this comment.
This is an awkward place to use the core/edit-site store
Would it make more sense to have the templateInfo store in core/editor instead of core/edit-site? While it is most relevant to the site editor, it seems like valuable information regardless. Similarly, it seems ok to access core/editor store in edit-site a bit more than it does to access core/edit-site in editor. 🤔
There was a problem hiding this comment.
This sounds like a great idea actually!
core/editor's editorSettings is already full of very specific stuff, I guess adding default template types in there too wouldn't do much harm. 😅
There was a problem hiding this comment.
@Addison-Stavlo After looking into it, I'm deciding against it.
The reason is that edit-post and edit-site initialize the editor in wildly different ways, and they add their PHP-provided settings to different stores.
I don't know the reason behind this, and what kind of inconsistencies it is currently creating.
Either way, I'm not going to attempt a major refactor of the editors initialization in this PR. 😄
The select( 'core/edit-site' ) is slightly smelly, but it works for our purposes.
As far as I could test, the entity review seems to work as expected in edit-post, displaying the correct template title.
By checking for core/edit-site, we are also preventing regressions in other editors that we might not catch.
…when strictly needed
d7bbf29 to
dbd2395
Compare
|
Will split this PR to make it more reviewable! |
Description
This is a simpler alternative to #26394, after an interesting comment by @carlomanf made me realize that I was looking at this from the wrong side, and needlessly overcomplicating the approach.
Fixes #25755
Update the templates handling to allow for custom titles and descriptions.
default_template_typesfilter, already used to temporarily remove theembeddefinition which it's currently not supported.getDefaultTemplateTypes(),getDefaultTemplateType(), andgetTemplateInfo()selectors ofcore/edit-site.draft. They will becomepublishedonce saved the first time.excerptsto thewp_templateCPT, used to store the template description.auto-drafttemplates, at least temporarily.getTemplateInfoutility into thegetTemplateInfoselector, since it relies on thecore/edit-sitestate.Main Differences with #26546
No more
theme-providedstatus. My original assumption that it was getting in the way of titles was proven incorrect after a deeper investigation. The status itself doesn't do anything to titles; it's the editor's entity retrieval that clears the title. In this PR there are some changes to prevent it.No more
themepost meta to indicate the source of a template.We shouldn't have duplicate templates, so a template can only ever be: either custom (
publishordraftstatus), or provided by the current theme (auto-draftstatus).Follow up
How has this been tested?
wp_templateitems of statusauto-draft; there might be duplicates for unrelated reasons (see Auto-draft template is created when draft already exists #26621, but not limited to it)./edit.php?post_type=wp_template).auto-drafttemplates. Feel free to wipe this list clean by moving all templates to trash, and permanently delete them.+in the Templates menu).Screenshots
The new Templates wp-admin list:
The Site Editor sidebar using the new default definitions, and including a draft template:
The Site Editor documents settings using the new default definitions:
Types of changes
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Checklist: