Skip to content

Templates: Search for old template names in the parent theme too #36910

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.

Sign up for GitHub

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

Merged
scruffian merged 3 commits into from
Dec 13, 2021
Merged

Templates: Search for old template names in the parent theme too #36910

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5d3869f
Templates: Search for old template names in the parent theme too
scruffian Nov 26, 2021
12b017b
check for the directory rather than the file
scruffian Dec 8, 2021
004cba5
Fix template part file lookup
youknowriad Dec 13, 2021
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
Prev Previous commit
Next Next commit
check for the directory rather than the file
  • Loading branch information
@scruffian @youknowriad
scruffian authored and youknowriad committed Dec 13, 2021
commit 12b017b342c403244e05344d6774635f735161ba
2 changes: 1 addition & 1 deletion lib/compat/wordpress-5.9/block-template-utils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function get_block_theme_folders( $theme_stylesheet = null ) {
$parent_root_dir = get_theme_root( $parent_theme_name );
$parent_theme_dir = "$parent_root_dir/$parent_theme_name";

if ( is_readable( $theme_dir . '/block-templates/index.html' ) || is_readable( $parent_theme_dir . '/block-templates/index.html' ) ) {
if ( is_readable( $theme_dir . '/block-templates/' ) || is_readable( $parent_theme_dir . '/block-templates/' ) ) {
Copy link
Contributor

@youknowriad youknowriad Dec 9, 2021

Choose a reason for hiding this comment

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

Can't we just remove || is_readable( $parent_theme_dir . '/block-templates/' ), since we're calling the function for both themes (parent and child), this should work right?

Copy link
Contributor Author

@scruffian scruffian Dec 10, 2021

Choose a reason for hiding this comment

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

I don't think so. If we did that then we'd be back to the changes that are already in trunk, since the other lines don't actually do anything except assign variables.

Copy link
Contributor

@youknowriad youknowriad Dec 10, 2021

Choose a reason for hiding this comment

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

I thought the issue was about themes that are child themes, override some templates but not "index.html" so it's different than trunk? Are there any other use-cases here?

Copy link
Contributor

@pbking pbking Dec 10, 2021

Choose a reason for hiding this comment

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

They might not override any templates at all.

Copy link
Contributor

@youknowriad youknowriad Dec 10, 2021

Choose a reason for hiding this comment

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

They might not override any templates at all.

In that case that function is useless and doesn't matter what it returns.

Copy link
Contributor

@youknowriad youknowriad Dec 13, 2021

Choose a reason for hiding this comment

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

I don't think so. Child themes can simply provide a different theme.json file and no other changes.

In that case it doesn't matter what this function returns.

Copy link
Contributor Author

@scruffian scruffian Dec 13, 2021

Choose a reason for hiding this comment

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

If I create a child theme with no templates or template parts I see this message:

Screenshot 2021-12-13 at 11 08 48

Copy link
Contributor

@youknowriad youknowriad Dec 13, 2021

Choose a reason for hiding this comment

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

Indeed, I think we had an issue with the template part block's code where the child theme was not handled properly and it should be fixed in 004cba5

The root issue is that get_theme_file_path function tries to fallback to the parent theme automatically but it doesn't account with the fact that the path could change slightly between parent and child themes (folder names)

Copy link
Contributor

@youknowriad youknowriad Dec 13, 2021

Choose a reason for hiding this comment

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

Feel free to ship this if the fix in that commit is good enough for you.

Copy link
Contributor Author

@scruffian scruffian Dec 13, 2021

Choose a reason for hiding this comment

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

Looks good thanks

return array(
'wp_template' => 'block-templates',
'wp_template_part' => 'block-template-parts',
Expand Down