Update: Show default template name on template picker list.#45865
Update: Show default template name on template picker list.#45865jorgefilipecosta wants to merge 1 commit into
Conversation
|
|
| if ( $post_type === 'page' ) { | ||
| $templates = array(); | ||
| if ( ! empty( $post->post_name ) ) { | ||
| $templates[] = "page-{$post->post_name}.php"; |
There was a problem hiding this comment.
| $templates[] = "page-{$post->post_name}.php"; | |
| $templates[] = "page-{$post->post_name}"; |
I think we should drop the php suffix for block templates.
There was a problem hiding this comment.
The feedback was addressed 👍
7b7776c to
a295b3e
Compare
|
I can't comment on the code, but just leaving a note to confirm that this is working as expected for me for block templates. |
a295b3e to
aa287a9
Compare
aa287a9 to
5837b7a
Compare
|
| } | ||
| return $label; | ||
| } | ||
| add_filter( 'default_page_template_title', 'gutenberg_add_template_to_default_template_title' ); |
There was a problem hiding this comment.
I've been checking, Core code, and this filter is used in a two other places: some kind of meta box, and the post list. So I'm guessing this change will impact these places as well. Do we want to do that?
There was a problem hiding this comment.
I think so. We want to do that. This only affects block themes, and we want in post list and other places to show the same template title as in the post editor.
| * @return string The new display value for the default page template title. | ||
| */ | ||
| function gutenberg_add_template_to_default_template_title( $label, $context = '' ) { | ||
| global $post_type, $post; |
There was a problem hiding this comment.
Basically, if I'm understanding properly, we're trying to run WordPress's template resolution algorithm for the current global $post and $post_type. I feel this is not the first time I make this comment but it would be great if this algorithm could be abstracted in a dependency-less class/function. Is there a way to share code? (Probably not for this PR anyway)
Also, I want to ping @ockham here as I believe he worked on this kind of things previously for block templates resolution.
Also, this will run the template resolution for each editor page load, I'm assuming that's fast enough?
There was a problem hiding this comment.
it would be great if this algorithm could be abstracted in a dependency-less class/function.
100%. It would be great to have something similar to Hierarchy in the core. A class that can resolve a template based on the current post or query context.
There was a problem hiding this comment.
Basically, if I'm understanding properly, we're trying to run WordPress's template resolution algorithm for the current global $post and $post_type. I feel this is not the first time I make this comment but it would be great if this algorithm could be abstracted in a dependency-less class/function. Is there a way to share code? (Probably not for this PR anyway)
Sure, I agree that abstracting template resolution is a common need. However, it's not straightforward to implement. For example, in this case, we want template resolution to use the default/fallback template even if a custom template has been assigned. In other cases, we may need template resolution to work differently.
I don't think this should be a blocker for us. We can explore options for abstraction while also addressing the specific needs we have, and later consolidate.
Also, this will run the template resolution for each editor page load, I'm assuming that's fast enough?
Yes, I think it is something fast enough we also call these functions on the site front end anyway, where performance is even more critical.
|
I'm not sure I like this PR personally, I think it's adding complexity and a bit of fragility (for instance the template name doesn't change after you save or make changes to a post which could impact its template, like assigning an id...) |
|
I'd be more ok with this change if we had the right abstraction in place and my thinking is that maybe it's not worth it with a hacky implementation. I know that we can't do better than this for now though. |
Fixes: #41449
This PR updates the default template label to say what the default template name is. This makes the user aware of what the default template is.
What?
The implementation was a little complex as we needed to compute the default template for the post if it had no template assigned to it. I followed the logic in get_single_template, get_page_template, and get_singular_template.
Screenshots or screencast