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

Add support for Gutenberg 12.1.0 block template naming convention #31522

Conversation

sunyatasattva
Copy link
Contributor

@sunyatasattva sunyatasattva commented Dec 28, 2021

All Submissions:

Changes proposed in this Pull Request:

Gutenberg 12.1.0 has changed the convention for the directory paths from block-templates and block-template-parts to templates and parts respectively.

Previously, WooCommerce was checking whether block templates were available, only in the block-templates directory.

This commit adds support for both older and newer naming conventions, as well as parents and child themes.

Closes #31518

How to test the changes in this Pull Request:

Using WP 5.8

  1. Install Gutenberg plugin and a block theme which is using the old template directory names such as TT1 Blocks.
  2. Check that the WC block templates render/are available in the Site Editor (under Appearance), and on the frontend.
  3. Activate a block theme which is using the new template directory names such as the latest version of Tove)
  4. Check that the WC block templates still render/are available in the Site Editor (under Appearance), and on the frontend.

Using WP 5.9, replicate the steps above with the Gutenberg plugin deactivated.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Add support for Gutenberg 12.1.0 block template naming convention in themes

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

Gutenberg 12.1.0 has changed the convention for the directory paths from
`block-templates` and `block-template-parts` to `templates` and `parts`
respectively.

Previously, WooCommerce was checking whether block templates were
available, only in the `block-templates` directory.

This commit adds support for both older and newer naming conventions,
as well as parents and child themes.

Fixes #31518
@sunyatasattva sunyatasattva added focus: template Issue related to WooCommerce templates. changelog added labels Dec 28, 2021
@sunyatasattva sunyatasattva self-assigned this Dec 28, 2021
sunyatasattva added a commit to woocommerce/woocommerce-blocks that referenced this pull request Dec 28, 2021
…nvention (#5455)

Gutenberg 12.1.0 has changed the convention for the directory paths from
`block-templates` and `block-template-parts` to `templates` and `parts` respectively.

Allow compatibility with themes which follow both conventions and also make sure that
we remain backwards-compatible.

Fixes #5450

Some of this fix has a dependency on WooCore

See: woocommerce/woocommerce#31522
Aljullu
Aljullu previously approved these changes Dec 29, 2021
Copy link
Contributor

@Aljullu Aljullu left a comment

Choose a reason for hiding this comment

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

Thanks for upstreaming this change @sunyatasattva! I left a couple of suggestions, but pre-approving from the WC Blocks side.

Just to clarify, do you think this needs to be included in a future patch release? My understanding is that we need either this fix or woocommerce/woocommerce-blocks#5455 to be included in WC core 6.1 if possible. Am I understanding that right?

plugins/woocommerce/includes/class-wc-template-loader.php Outdated Show resolved Hide resolved
plugins/woocommerce/includes/class-wc-template-loader.php Outdated Show resolved Hide resolved
@sunyatasattva
Copy link
Contributor Author

Just to clarify, do you think this needs to be included in a future patch release? My understanding is that we need either this fix or woocommerce/woocommerce-gutenberg-products-block#5455 to be included in WC core 6.1 if possible. Am I understanding that right?

I think we'd need both to be included for full functionality. Either won't be enough :/

So yea, if possible, this should be included ASAP.

@Aljullu
Copy link
Contributor

Aljullu commented Dec 30, 2021

Thanks for updating the PR @sunyatasattva!

I think we'd need both to be included for full functionality. Either won't be enough :/

I'm unable to reproduce the original issue (#31518). From my investigation, it seems to be because we add a filter to woocommerce_has_block_template:

https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/2635445c5d303eba4dfef12efbde0f29667f2673/src/BlockTemplatesController.php#L406-L439

Which in woocommerce/woocommerce-blocks#5455 was already updated to account for the new directory structure.

Don't get me wrong, I agree this should be upstreamed to WC core, but I would like to make sure if this needs to go into WC core 6.1 or it can wait for WC core 6.2.

In any case, cc'ing @jonathansadowski as the release lead of WC 6.1 for awareness.

@jonathansadowski
Copy link
Contributor

Don't get me wrong, I agree this should be upstreamed to WC core, but I would like to make sure if this needs to go into WC core 6.1 or it can wait for WC core 6.2.

Thanks for the ping. If I'm understanding the issue correctly, it seems reasonable to me that this support could wait until 6.2, as it wouldn't break any existing theme, and that a work-around for supporting the newer convention until 6.2 would be to install a version of blocks that includes woocommerce/woocommerce-blocks#5455. Does that seem reasonable or is there something that I'm missing?

Copy link
Contributor

@tjcafferkey tjcafferkey left a comment

Choose a reason for hiding this comment

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

Works great! LGTM

@tjcafferkey
Copy link
Contributor

Does that seem reasonable or is there something that I'm missing?

@jonathansadowski we could fix this in Woo Blocks temporarily however this means that WooCommerce Block Templates will not be fully compatible with WordPress 5.9 unless users have WooCommerce Blocks installed as well.

I realise this request is beyond the current code freeze but I think it's important to include this fix to ensure compatibility with WP 5.9.

Copy link
Contributor

@vedanshujain vedanshujain left a comment

Choose a reason for hiding this comment

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

LGTM as well. I am going to merge to trunk, and leaving the decision for when to include to release lead.

@vedanshujain vedanshujain merged commit aee1f08 into trunk Jan 4, 2022
@vedanshujain vedanshujain deleted the fix/31518-support-for-new-block-templates-naming-conventions branch January 4, 2022 08:23
@vedanshujain vedanshujain added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] and removed changelog added labels Jan 4, 2022
@github-actions github-actions bot added this to the 6.2.0 milestone Jan 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jan 4, 2022

Hi @vedanshujain, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the status: needs changelog label
  • Add the status: needs testing instructions label

@jonathansadowski jonathansadowski removed this from the 6.2.0 milestone Jan 4, 2022
@jonathansadowski jonathansadowski added this to the 6.1.0 milestone Jan 4, 2022
@jonathansadowski jonathansadowski added the release: cherry-pick Commits from this PR also needs to be added to current release branch. label Jan 4, 2022
@jonathansadowski
Copy link
Contributor

Yep, I agree with the importance. Thanks for flagging. I'll make sure this gets cherry-picked into 6.1

@jonathansadowski jonathansadowski added cherry picked and removed release: cherry-pick Commits from this PR also needs to be added to current release branch. labels Jan 4, 2022
jonathansadowski added a commit that referenced this pull request Jan 4, 2022
Update the readme file to include changes from #31522
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: template Issue related to WooCommerce templates. release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing support for latest Gutenberg blocks directory naming conventions
5 participants