FSE: Fall back to next best template in hierarchy when querying through REST API (Take Two) #41848
Conversation
|
Size Change: -24 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
|
👋 FYI @jameskoster @ntsekouras 🙂 |
adamziel
added
the
Backport to WP Minor Release
|
Woohoo! I'm very excited about this one. The mechanics seem to work very well – the correct fallback template was used in all of my testing. I did notice however that new templates are not immediately published upon creation (I believe they are on trunk), but it's also not possible to save when you land in the editor. This can lead to some confusing UX: 404.mp4I'm not sure that auto-publishing is the correct approach, but that's probably something to explore separately. For now I suppose it's more important to be consistent. I performed a number of additional tests by deleting all templates except Index in my current theme. Not wanting to add more complexity (😬), but we might consider some exceptions, or even some hard-coded fallbacks. A couple of situations felt a bit odd:
We can probably explore these in a follow-up since they're flows more likely to be encountered by advanced users, where the issues aren't so egregious. An odd quirk I observed: During the first creation, the fallback works as expected. But on subsequent creations of different templates I am met with a blank canvas, unless I return to the templates list and repeat the action. Better explained with a video: repeat.mp4 |
| return $template; | ||
| if ( count( $slug_parts ) > 2 ) { | ||
| // Now we look for the CPT with slug as defined in $slug_parts[2] | ||
| $post = get_page_by_path( $slug_parts[2], OBJECT, $slug_parts[1] ); |
There was a problem hiding this comment.
Why use this function over a WP_Query?
There was a problem hiding this comment.
WP_Query is probably the better option; get_page_by_path was the stop-gap to make it work 👍
| return $template; | ||
| if ( count( $slug_parts ) > 2 ) { | ||
| // Now we look for the CPT with slug as defined in $slug_parts[2] | ||
| $post = get_page_by_path( $slug_parts[2], OBJECT, $slug_parts[1] ); |
There was a problem hiding this comment.
Also can't we just do this?
| $post = get_page_by_path( $slug_parts[2], OBJECT, $slug_parts[1] ); | |
| $post = get_page_by_path( $slug_parts[2], OBJECT, $template_type ); |
There was a problem hiding this comment.
I'm afraid that won't work -- $template_type will be something like single, category, or even 404.
If it's single, then $slug_parts[1] will be the post type that the template is for.
($template_type OTOH will be reflected in $slug_parts[0].)
There was a problem hiding this comment.
This PR is pretty hard to follow and the problem it is trying to solve seems unclear.
In short, I don't like this solution at all. get_block_template is now a core function and is being used by developers. Out of the blue changing the behaviour of it, without warning and with no test coverage, feel reckless. Adding magic logic to this function to fix this problem, is not the place to do this. A solution that fallbacks back in javascript makes much more sense to me.
I would recommend adding a link to the REST API response or a default content field maybe.
|
Thank you for your candid feedback @spacedmonkey. I realize this approach is not without controversy — especially the part about using the template fallback even when requesting a template by a given ID (see prior discussion at #37258).
Well, it’s still a work in progress ;) FWIW, I plan to encapsulate some logic into functions to better organize the code.
FWIW, I definitely want to add some test coverage. That said, changing
I respectfully disagree here. What we currently have in Gutenberg (and WordPress’s REST API) is an incomplete emulation of the template hierarchy resolution as it works on the frontend that is, among other things, ignorant of per-ID and per-slug templates (such as IMO, adding JavaScript to the mix to fully capture the template hierarchy makes things worse: Developers would then have to look into both PHP and JS files to figure out how template lookup works; and since the delineation would be somewhat arbitrary, it’d be fairly non-obvious how to extend that logic. I'll continue to work a bit more on this PR and to provide more background and information how it's supposed to work etc. I'd be grateful if you could give it another look once it's ready for review! :) |
Hey @jameskoster! Happy to hear, and thank you for testing! 🙌
Yeah, so this behavior mostly results from the editor not enabling the Save button if the user hasn't made any changes to the previously "saved" state of the template -- and in this case, the notion of "saved" is extended to what the fallback template looked like. So the Save button should become enabled once you make any change to the template. OTOH, if you don't make any changes and instead return straight to the template list, the template isn't saved indeed -- for the same reason: WordPress will continue to use the fallback template to render whatever the more specific template would've been for. So, for example:
I think that this might be fairly intuitive in some cases (like the above one maybe), but since this behavior applies to all templates, it's arguable that it's less so for more "exotic" fallbacks. LMK if this makes sense. We can try to change some of this behavior; the complexity involved may vary greatly depending on what aspect(s) we'd like to change specifically, since some things tie pretty deeply into existing concepts and might be hard to change without big changes to the underlying functionality. So it'd be good to get a sense of what things are a dealbreaker UX-wise, and which ones we can maybe mitigate through messaging etc :) |
The thing is that this PR is seeking the template hierarchy as it would work on the frontend, and those are the actual fallbacks that would be used there 😕 (also see https://wphierarchy.com/) Now I agree that those don't feel right; they probably haven't been much of a problem prior to FSE since themes would pretty much always include a 404 and single post template, so the fallback was rarely needed. While I'd love to make the fallbacks fell more right, I also don't want to deviate from how it'd work on the frontend. Not totally sure how to best tackle this 🤔
Sounds good; definitely something we can work on later as a refinement on top of this PR 👍 |
Yeah, I've also run into this behavior. That's definitely a bug -- probably some sort of editor state that's kept around. I'll look into this. |
ockham
force-pushed
the
try/implicit-template-resolution-for-rest-api-take-two
branch
from
4f59e1b
to
e0ff8b5
Compare
Should be fixed now. There's still an error where when creating a CPT-specific template (Single item: Post) followed by a post-specific template (Post: Hello World), the latter will somehow override the former. I'll investigate tomorrow. |
It makes sense, but the UX is confusing because it is directly opposed to what happens when you create a template on trunk. There the template is published immediately, so for that not to happen in this PR feels broken to me. At the moment it is probably best to align with trunk, IE publish immediately. We can then explore whether that is the best flow separately.
The fallback is least successful in two key flows, creating a |
|
I've now started an alternative exploration over in #42007, based on feedback here and via DM. Its overall footprint is smaller, and templates are saved immediately after creating 🙂 |
|
Testing using the Gutenberg PR build. Part 1.
I can verify that the site editor opens, pre-populated with the generic "single" template, and that I see Single item: Post
This is what I see (The site is a local testing site I have been using for all things FSE).
Part 2.
I can verify 4, 5, and 6.
NB! I again click the Add New button -> Single item: Post and the Add template: Post modal is seen.
Upon making another Template for all single posts I see this: So there is some weirdness going on. I am starting over. I have now deleted all my earlier made custom templates. I made a template for Hello World but again it did not show up in the Templates List screen. |
|
Thank you very much for your testing, @paaljoachim! Unfortunately, I'm afraid we're not going to move forward with this PR since it's proven too controversial. I've filed a somewhat smaller change in #42007, but even that one looks like it's not everyone's preferred solution. We're probably going to go with @ntsekouras' #42520 😊 Anyway, I'm going to close this PR. Sorry -- should've done it sooner! |
ockham
deleted the
try/implicit-template-resolution-for-rest-api-take-two
branch
|
Thanks for working on it @ockham Bernie! |
gziolo
removed
the
Backport to WP Minor Release
What?
Take Two of #37258.
Purports to fix #36648, as an alternative approach to #37054. For more background, see #37054 (comment)
(More description to come)
Why?
See e.g. #37258 (comment) and #37258 (comment).
How?
By always using the full template hierarchy to look for potential fallback templates, even when queried for a template ID.
Testing Instructions
This testing routine will verify a number of things:
First, we create a "Single Post" template and verify that it comes pre-populated with the contents of the theme-supplied "Single" template:
Second, we create a "Single Post" template for a specific post and verify that it comes pre-populated with the generic "Single Post" template that we just created: This is still buggy, reload every time you go back to the template list
Ideally, give it some more smoke testing -- whatever you can think of makes sense: delete the template again; find a way to test it on the frontend; etc.
TODO
single-post-hello-world) being applied to existing CPT-specific templates (e.g.single-post).gutenberg_resolve_home_template/_resolve_home_block_template).Screenshots
TBD