Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: Remove need for template prefix in gutenberg_get_template_hierarchy. #46257

Conversation

jorgefilipecosta
Copy link
Member

This PR removes the need to pass a template prefix in gutenberg_get_template_hierarchy.

In #46248 when we open a template that is empty, we show the non-empty fallbacks, and we don't know the template prefix. The template prefix is part of the slug so the function should be able to do its work even if it is not passed.
This PR updates the function to work even without template_prefix (it computes it based on the slug). This is required, so the fallback templates displayed in #46248 are correct (currently, for some template types they are wrong),

Testing

Verify the PHP unit tests pass.

@jorgefilipecosta jorgefilipecosta added the [Type] Enhancement A suggestion for improvement. label Dec 1, 2022
@jorgefilipecosta jorgefilipecosta force-pushed the update/remove-need-for-template-prefix-in-gutenberg_get_template_hierarchy branch from eff8577 to dce4af1 Compare December 2, 2022 14:31
$hierarchy = get_template_hierarchy( 'taxonomy-books-action-adventure', false, 'taxonomy-books' );

$hierarchy = gutenberg_get_template_hierarchy( 'taxonomy-book_type-adventure', false );
$this->assertEquals( array( 'taxonomy-book_type-adventure', 'taxonomy-book_type', 'taxonomy', 'archive', 'index' ), $hierarchy );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why in this case it returned both 'taxonomy-book_type' and taxonomy as parents? What's the logic for extracting the "prefix" from the slug, are there any risk of ambiguity?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a template for taxonomy-book_type-adventure. It is a template for the term adventure of the taxonomy book type. The hierarchy of the templates is to try to use the term specific term template taxonomy-book_type-adventure. If it does not exist, WordPress tries to use the general template for the specific taxonomy in this case, taxonomy-book_type and if it does not exists WordPress tries to use taxonomy. WordPress tries to use both taxonomy-book_type and taxonomy so both should be included.
The logic is available at https://github.com/WordPress/gutenberg/pull/46257/files#diff-02603047171e2b2aa5174d1b541451ed8245519310dde37fda28fd8fe8b7480aR68-R85.
We check all the available taxonomies and verify if the prefixes match.

are there any risk of ambiguity

Yes, we have this risk, but so has the WordPress template resolution engine. Imagine we have the following template taxonomy-a-b-c.
And we have taxonomy a-b with term c.
And we have taxonomy a with term b-c.
The WordPress template resolution engine is going to apply the template to both term c of taxonomy a-b and term b-c of taxonomy a. In practice is not big deal normally developers would not create an a-b taxonomy for post types and taxonomies normally "-" is not used and "_" is used instead. I don't think that's a case we should optimize for.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming, there's no perf impact on these lookups (available taxonomies...), let's ship this.

@jorgefilipecosta jorgefilipecosta merged commit a33c214 into trunk Dec 20, 2022
@jorgefilipecosta jorgefilipecosta deleted the update/remove-need-for-template-prefix-in-gutenberg_get_template_hierarchy branch December 20, 2022 10:50
@github-actions github-actions bot added this to the Gutenberg 14.9 milestone Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants