Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Fix custom templates with fallback being incorrectly attributed #5447

Merged
merged 5 commits into from
Dec 24, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 5 additions & 18 deletions src/BlockTemplatesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,16 +175,10 @@ public function add_block_templates( $query_result, $query, $template_type ) {
// @todo: Add apply_filters to _gutenberg_get_template_files() in Gutenberg to prevent duplication of logic.
foreach ( $template_files as $template_file ) {

// Avoid adding the same template if it's already in the array of $query_result.
if (
array_filter(
$query_result,
function( $query_result_template ) use ( $template_file ) {
return $query_result_template->slug === $template_file->slug &&
$query_result_template->theme === $template_file->theme;
}
)
) {
// If we have a template which is eligible for a fallback, we need to explicitly tell Gutenberg that
// it has a theme file (because it is using the fallback template file). And then `continue` to avoid
// adding duplicates.
if ( BlockTemplateUtils::confirm_theme_file_when_fallback_is_available( $query_result, $template_file ) ) {
continue;
}

Expand Down Expand Up @@ -348,14 +342,7 @@ function ( $template ) use ( $template_slug ) {
}

// If the theme has an archive-product.html template, but not a taxonomy-product_cat.html template let's use the themes archive-product.html template.
if ( 'taxonomy-product_cat' === $template_slug && ! BlockTemplateUtils::theme_has_template( 'taxonomy-product_cat' ) && BlockTemplateUtils::theme_has_template( 'archive-product' ) ) {
$template_file = get_stylesheet_directory() . '/' . self::TEMPLATES_DIR_NAME . '/archive-product.html';
$templates[] = BlockTemplateUtils::create_new_block_template_object( $template_file, $template_type, $template_slug, true );
continue;
}

// If the theme has an archive-product.html template, but not a taxonomy-product_tag.html template let's use the themes archive-product.html template.
if ( 'taxonomy-product_tag' === $template_slug && ! BlockTemplateUtils::theme_has_template( 'taxonomy-product_tag' ) && BlockTemplateUtils::theme_has_template( 'archive-product' ) ) {
if ( BlockTemplateUtils::template_is_eligible_for_product_archive_fallback( $template_slug ) ) {
$template_file = get_stylesheet_directory() . '/' . self::TEMPLATES_DIR_NAME . '/archive-product.html';
$templates[] = BlockTemplateUtils::create_new_block_template_object( $template_file, $template_type, $template_slug, true );
continue;
Expand Down
71 changes: 71 additions & 0 deletions src/Utils/BlockTemplateUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -281,4 +281,75 @@ public static function supports_block_templates() {

return true;
}

/**
* Checks if we can fallback to the `archive-product` template for a given slug
*
* `taxonomy-product_cat` and `taxonomy-product_tag` templates can generally use the
* `archive-product` as a fallback if there are no specific overrides.
*
* @param string $template_slug Slug to check for fallbacks.
* @return boolean
*/
public static function template_is_eligible_for_product_archive_fallback( $template_slug ) {
$eligible_for_fallbacks = array( 'taxonomy-product_cat', 'taxonomy-product_tag' );

if (
in_array( $template_slug, $eligible_for_fallbacks, true )
&& ! self::theme_has_template( $template_slug )
&& self::theme_has_template( 'archive-product' )
) {
return true;
}

return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we just return the result of the evaluation instead of using an if here?

}

/**
* Sets the `has_theme_file` to `true` for templates with fallbacks
*
* There are cases (such as tags and categories) in which fallback templates
* can be used; so, while *technically* the theme doesn't have a specific file
* for them, it is important that we tell Gutenberg that we do, in fact,
* have a theme file (i.e. the fallback one).
*
* **Note:** this function changes the array that has been passed.
*
* It returns `true` if anything was changed, `false` otherwise.
*
* @param array $query_result Array of template objects.
* @param array $template A specific template object which could have a fallback.
*
* @return boolean
*/
public static function confirm_theme_file_when_fallback_is_available( $query_result, $template ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think something like set_has_theme_file_if_fallback_is_available would be a slightly better name? At a glance it is hard to tell what this function does imo.

$template_with_fallback_idx = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is idx? If it means index can we just use the full word instead?

$is_duplicate = false;

array_walk(
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why you chose to use this over a foreach? foreach is quicker :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason but my massive PHP ignorance. I didn't know you could get the index with foreach but it looks like you can! Thanks for pointing that out.

$query_result,
function( $query_result_template, $idx ) use ( $template, &$template_with_fallback_idx, &$is_duplicate ) {
if (
$query_result_template->slug === $template->slug
&& $query_result_template->theme === $template->theme
) {
$is_duplicate = true;

if ( self::template_is_eligible_for_product_archive_fallback( $template->slug ) ) {
$template_with_fallback_idx = $idx;
}
}
}
);

if ( $is_duplicate ) {
if ( is_int( $template_with_fallback_idx ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add a comment here explaining why we need to check is_int? This will stop someone "refactoring" it in future 🤓

$query_result[ $template_with_fallback_idx ]->has_theme_file = true;
}

return true;
}

return false;
}
}