Templates: add postTypes and isCustom fields & filters
#62075
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
oandregal
added
[Type] Enhancement
oandregal
requested review from
mcsf,
youknowriad,
jorgefilipecosta and
ntsekouras
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/compat/wordpress-6.6/rest-api.php |
| }, | ||
| }, | ||
| { | ||
| header: __( 'Type' ), |
There was a problem hiding this comment.
I've made the following decisions about how to present the isCustom field to the user:
- Called it
Type. Alternatives:Template type,Is custom?. - Displayed it as a badge field in the grid layout, as we do for other boolean-type fields (sync status in patterns).
- In any layout, it'd only render "Custom" if the template is custom. If it is not, it'll render an empty value. Alternatively, it could say "Default".
This is a concept that requires a certain knowledge of WordPress templates, so I was unsure about how to simplify it further. Happy to hear thoughts.
There was a problem hiding this comment.
Yeah this is very tricky.
- I'm not sure about displaying the type as a badge; it might not be obvious what this refers to without the label?
- 'Type' suggests multiple options... It's odd that the column is either empty or 'custom'. Would it be feasible to add other types? E.g. Archive, Single, Utility? Combined with the post type filter this could potentially work quite well.
- I'd probably hide this field by default.
There was a problem hiding this comment.
It's odd that the column is either empty or 'custom'. Would it be feasible to add other types? E.g. Archive, Single, Utility? Combined with the post type filter this could potentially work quite well.
To be honest, I'm struggling to see the value of this field/filter in isolation: having the postTypes one enables users to search for templates bound that those post types — which are considered custom already, so this Type is a redundant. Is there any other value to it I'm not seeing?
For adding more types: is there any stablished categorization of templates I can look at? That could be useful, though I wouldn't want to introduce a new concept to templates in this PR.
If this field doesn't provide much value right now, I'd rather start with postTypes only.
There was a problem hiding this comment.
I think the main motivator was to provide a way to replicate the 'Custom' section in 6.5's Templates sidebar:
Without this filter it wouldn't be possible to view custom templates easily which could be seen as a regression? Possibly a bad idea, but could the filter be hidden in the UI but still function as part of a 'Custom' view? I think pattern categories work a similar way.
The template hierarchy resolves by page type, essentially; "Archive", "Singular", "Front Page", "Posts Page", "Search Results", "404".
Grouping by "Archive", "Singular", and "Utility" seems reasonable, but I don't think it's defined in the code. Agree it seems late to be adding this complexity.
There was a problem hiding this comment.
The template hierarchy resolves by page type, essentially; "Archive", "Singular", "Front Page", "Posts Page", "Search Results", "404".
I feel this is the best solution, and I'm also wary to introduce it this close to the beta.
If we agree exposing those types would be best long-term, a plan for 6.6 can be:
- naming the field "Type", so we can reuse it later
- its values for now are "Custom" / "Not custom" — we can add more granularity in 6.7 ("Archive", etc.)
Thoughts?
Pushed this approach at 895214a. Made the field hidden by default and not a badge as well.
There was a problem hiding this comment.
How do you feel about implementing the filter with custom / not custom as values, but hiding it in the UI for 6.6?
We can then use it to add a 'Custom' view, including a description about what custom templates are. This would be similar (I think) to categories in the Patterns page.
This would be dependent on updating the frame titles, but there's a PR for that ready to go.
|
Size Change: +320 B (+0.02%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
oandregal
force-pushed
the
update/sidebar-templates
branch
from
c821e44 to
d48ddab
Compare
|
Unsure how it works, but the post type filter turned up some unexpected results for me:
When the post types field is visible, it's a little strange that some templates show the field, but others do not, especially in table layout. Should a |
I need to verify this logic to be the same we use when the user switches templates: Gravacao.do.ecra.2024-05-28.as.18.00.48.movIt seems that:
Does that sound good or am I missing something? |
|
Edit: I've updated this comment with the latest I've learned about how the I'll look into this tomorrow morning, but at the implementation level this is a bit tricky. It seems the next steps are:
|
| elements: [ | ||
| { value: 'post', label: POST_TYPES.post }, | ||
| { value: 'page', label: POST_TYPES.page }, | ||
| ], |
There was a problem hiding this comment.
Are these the only possible post types?
There was a problem hiding this comment.
No, they are not. There's a TODO note over there to look at this later, when the custom filtering was addressed.
oandregal
force-pushed
the
update/sidebar-templates
branch
from
16518e7 to
5345174
Compare
|
I've implemented the logic above and this is how it looks like:
The results make sense to me, though it's a complex topic and I'm not sure the information display helps to make sense of it if you're not familiar with all kind of templates.
|
| { | ||
| header: __( 'Post types' ), | ||
| id: 'postTypes', | ||
| getValue: ( { item } ) => |
There was a problem hiding this comment.
This getValue function is used by the filters. Note how the values returned here are different from the values returned by render, which is not what we've been doing so far.
This enables us to implement this logic, in line with the server logic. For example, when querying the templates endpoint with post_type: post the server returns:
- Templates whose
templates->post_typesvalues containpost. - User templates that are custom but don't have any post type information.
Additionally, there's a few default templates with special meaning that are not returned by the endpoint in that scenario because they're the default and we should probably display here when the user filters by post type:
- The
single&singulartemplates, as per the template hierarchy.
There was a problem hiding this comment.
An alternative to this approach would be allowing the field to provide a custom filter callback. I'm investigating this next, though I wanted to share as early as possible in case anyone has different ideas.
There was a problem hiding this comment.
I've extracted a getPostTypesFromItem method that centralizes all this logic. I feel better about this now. Still need to review/add support for CPTs to assess whether this is a good enough approach.
I think this looks good. But shouldn't the For user-generated custom templates, should the post types value be We'll need to think about how this works for other template types like archives. E.g. Finally, should templates not associated with any particular post type still show the field but with an |
oandregal
force-pushed
the
update/sidebar-templates
branch
from
5345174 to
1a776f2
Compare
|
James, I've pushed some changes that implement your suggestions for post types. So far, only primary and secondary templates are mapped to a post type, the rest will display "n/a". Do we consider support for variable templates ( I'm looking at "Custom" field/filter & CPTs support next. Gravacao.do.ecra.2024-05-30.as.11.24.42.mov |
For these templates I fear "Post types: n/a" is probably too disingenuous: 
It's unclear to users why There's one other quirk we should probably consider here; the |
I've done some research and find this is plugin territory and has to do with how they provide template metadata. The gist of the issue is that core & plugins have a different understanding of what
This PR is working as expected for the templates coming from the theme. However, given the different interpretations plugins have for For me, the next steps here are:
Pinging people that has been involved on this for thoughts and good measure:
|
I'm a bit confused by the Single product template having an empty array as the
Thanks for raising this! My guess is that most plugins have set
This would work for us. 👍 Heads-up that we are also working on introducing a template registration API for plugins (#61577), so we have the opportunity to unify how this is handled across different plugins. |
Oh, nice, I'll review that! I'm focused on backports today, so I'll take a look in a day or two. |
There was a problem hiding this comment.
Thanks for the research into what "custom" and "post types" actually mean in this context!
I suppose that, as far as "custom" is concerned, those two different definitions ("is not a default template" / "lives in the database") overlapped until recently. It was only through the improvement of templating infrastructure (including the get_block_templates hook) that it became gradually easier to produce new realities for "custom" templates.
Anyway, I don't really have an answer for you on that, and I think it'll take a few discussions and competing opinions before we can untangle this.
| if ( null === $template_metadata ) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Super minor, but this check isn't needed before isset.
| if ( null === $template_metadata ) { | |
| return null; | |
| } |
| @@ -256,6 +290,27 @@ export default function PageTemplates() { | |||
| } ) ); | |||
| }, [ records ] ); | |||
|
|
|||
| const getPostTypesFromItem = ( item ) => { | |||
| // This logic replicates querying the REST templates endpoint with a post_type parameter. | |||
| // https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/block-template-utils.php#L1077 | |||
There was a problem hiding this comment.
This needs to be a permalink and not just relative to trunk, otherwise it will go (and already has gone) stale.
| @@ -256,6 +290,27 @@ export default function PageTemplates() { | |||
| } ) ); | |||
| }, [ records ] ); | |||
|
|
|||
| const getPostTypesFromItem = ( item ) => { | |||
There was a problem hiding this comment.
This should be a useCallback, no?
Part of #59659
What?
Adds two new fields to the templates (
postTypes&isCustom).postTypesfield)isCustomfield)Why?
This is a preliminary step to use this to provide a new sidebar filters that include "Posts", "Pages", and "Custom", as per #59659
How?
isCustomfield & filter in the client. Displays it as a badge in the grid layout, and only renders theCustomvalue.postTypefield & filter in the client.post_typesin the templates REST endpoint.Testing Instructions
Visit Site Editor > Templates and verify there's two new fields (hidden by default) and filters.
TODO
postTypelogic. The current logic only accounts for templates the theme intended to register for a given post type. However, when switching templates (Post editor > swap templates), the user-created templates are also listed.