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

Evaluate if we can remove do_action( 'woocommerce_sidebar' ); from Legacy Template block #5077

Closed
Aljullu opened this issue Nov 5, 2021 · 2 comments · Fixed by #5097
Closed
Assignees
Labels
focus: FSE Work related to prepare WooCommerce for FSE. type: bug The issue/PR concerns a confirmed bug.

Comments

@Aljullu
Copy link
Contributor

Aljullu commented Nov 5, 2021

A couple of templates rendered by the Legacy Template block run do_action( 'woocommerce_sidebar' );, that causes an error to appear in debug mode:

imatge

and renders an unstyled sidebar below the block given that the styles are not loaded.

This issue is about investigating what are the pros and cons of removing the call to that action.

@Aljullu Aljullu added type: bug The issue/PR concerns a confirmed bug. focus: FSE Work related to prepare WooCommerce for FSE. labels Nov 5, 2021
@Aljullu
Copy link
Contributor Author

Aljullu commented Nov 8, 2021

Another option is to keep the woocommerce_sidebar action but unregister the woocommerce_get_sidebar function associated with it. This is what WC does for the WP core themes without sidebar:

https://github.com/woocommerce/woocommerce/blob/3611d4643791bad87a0d3e6e73e031bb80447417/plugins/woocommerce/includes/theme-support/class-wc-twenty-twenty-one.php#L28

However, IMO we should remove the do_action( 'woocommerce_sidebar' ); call altogether from the Legacy Template block. Block templates won't be rendering the WooCommerce sidebar, so I don't think it makes sense calling that action at all. In addition, block themes seem like a good opportunity to not carry over this action. I took a quick (and non-exhaustive) look at how it's being used by extensions and all instances I found are about hiding and replacing the sidebar. But those things can now be done via FSE technologies.

So my proposal would be to remove do_action( 'woocommerce_sidebar' ); from all block templates rendered by the Legacy Template block. In the unlikely case that we get reports that it broke extensions, we can reconsider adding it back (maybe with remove_action( 'woocommerce_sidebar', 'woocommerce_get_sidebar', 10 );), but I would only do that if really necessary. In fact, I expect the possibility to break extensions to be higher if we keep the action than if we remove it.

Thoughts @frontdevde @tjcafferkey?

@frontdevde
Copy link
Contributor

Agree with your assessment here. Let's remove it for block templates rendered by the Legacy Template block. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
focus: FSE Work related to prepare WooCommerce for FSE. type: bug The issue/PR concerns a confirmed bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants