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 the block theme folders to templates and parts #36647

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

youknowriad
Copy link
Contributor

See #36548

This PR updates the block themes to use templates and parts instead of block-templates and block-template-parts.
It also keeps backward compatibility, that said a theme can not have "templates" and "block-template-parts" or "block-templates" and "parts". I also updated all the documentation we have.

Notes

The issue also mentions the future of patterns and styles, I think these are not requirements for 5.9

Testing instructions

  • Make sure themes with "block-templates" and "block-template-parts" folders work properly.
  • Make sure themes with "templates" and "parts" folders work properly.

@youknowriad youknowriad added Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. labels Nov 19, 2021
@youknowriad youknowriad requested review from mtias and a team November 19, 2021 12:01
@youknowriad youknowriad self-assigned this Nov 19, 2021
@carolinan
Copy link
Contributor

Theme Check also needs to be updated, otherwise themes will not be able to be uploaded to WordPress.org
https://github.com/WordPress/theme-check/blob/master/checks/class-fse-required-files-check.php

@youknowriad
Copy link
Contributor Author

@ockham Do you think you can help with the php unit failures here? I'm not sure I understand the results I'm having and the previous assertions.

*
* @return array Folder names used by block themes.
*/
function get_block_theme_folders( $theme_stylesheet = null ) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not accept theme_dir directly and remove calculation logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From an API point of view, I felt that accepting a them stylesheet (or id) is a bit better.

Copy link
Member

Choose a reason for hiding this comment

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

No strong preference. I was just curious :)

@ockham
Copy link
Contributor

ockham commented Nov 19, 2021

@ockham Do you think you can help with the php unit failures here? I'm not sure I understand the results I'm having and the previous assertions.

@youknowriad I'll have a look!

@ockham
Copy link
Contributor

ockham commented Nov 19, 2021

All of the *_block_template_takes_precedence_over_* tests are failing, which means that the template resolution algorithm thinks that the PHP templates always take precedence -- probably because it can't locate the block templates. So something in template resolution seems to be a bit off still...

@ockham
Copy link
Contributor

ockham commented Nov 19, 2021

Is it possible that the reason for this issue is that the test (end dev) env is using Core trunk, and thus the original versions of the template resolution algorithm that we already backported (since it finds those functions and doesn't need GB's 5.9 compat layer)?

The reason why I think this might be the case is as follows: If I move the text fixture files back from phpunit/fixtures/themes/test-theme/templates/ to phpunit/fixtures/themes/test-theme/block-templates/, the lines that assert that the template resolution algorithm finds the block template no longer fail. In other words, the template resolution algorithm expects to find the templates in block-templates/ (rather than templates/).

@ockham
Copy link
Contributor

ockham commented Nov 19, 2021

Specifically, I think that GB is picking up Core's versions of _get_block_template_file and _get_block_templates_files, respectively, which are hard-wired to block-templates and block-template-parts.

@youknowriad
Copy link
Contributor Author

@ockham that makes sense, I guess we need to make the fix in both places at the same time.

@youknowriad
Copy link
Contributor Author

I think actually that we should remove the unit test from here, now that these functions are just in Gutenberg to support old versions.

@ockham
Copy link
Contributor

ockham commented Nov 19, 2021

I think actually that we should remove the unit test from here, now that these functions are just in Gutenberg to support old versions.

Let's just make sure that we've carried them over to wordpress-develop so we don't lose test coverage for some of these cases 👍

@ockham
Copy link
Contributor

ockham commented Nov 19, 2021

I think actually that we should remove the unit test from here, now that these functions are just in Gutenberg to support old versions.

Let's just make sure that we've carried them over to wordpress-develop so we don't lose test coverage for some of these cases 👍

Looks like we still have to backport them: https://github.com/WordPress/wordpress-develop/blob/d87196b560203345b385bef6bef9666e7147bb46/tests/phpunit/tests/block-template.php

@ockham
Copy link
Contributor

ockham commented Nov 19, 2021

I think actually that we should remove the unit test from here, now that these functions are just in Gutenberg to support old versions.

Let's just make sure that we've carried them over to wordpress-develop so we don't lose test coverage for some of these cases 👍

Looks like we still have to backport them: https://github.com/WordPress/wordpress-develop/blob/d87196b560203345b385bef6bef9666e7147bb46/tests/phpunit/tests/block-template.php

I've started a PR: WordPress/wordpress-develop#1920

@ockham
Copy link
Contributor

ockham commented Nov 25, 2021

I think actually that we should remove the unit test from here, now that these functions are just in Gutenberg to support old versions.

#36855

@youknowriad
Copy link
Contributor Author

The tests here are passing, so this should be good to land now.

@youknowriad youknowriad merged commit c6b83c1 into trunk Nov 25, 2021
@youknowriad youknowriad deleted the update/block-theme-folders branch November 25, 2021 11:39
@github-actions github-actions bot added this to the Gutenberg 12.1 milestone Nov 25, 2021
@noisysocks noisysocks removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants