-
Notifications
You must be signed in to change notification settings - Fork 219
Add to Cart with Options block > Update the block to rely on dedicated render methods instead of WooCommerce core templates. #9494
base: trunk
Are you sure you want to change the base?
Conversation
…'s native templates for the simple and variable products.
…ion-add-to-cart-button directly.
…variable_product_to_cart method for enqueing 'wc-add-to-cart-variation'.
The release ZIP for this PR is accessible via:
Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
Size Change: 0 B Total Size: 1.09 MB ℹ️ View Unchanged
|
…e new variable_product_form method.
…the grouped_product_form method.
…rm, woocommerce_after_add_to_cart_form, woocommerce_before_add_to_cart_button, woocommerce_after_add_to_cart_button actions and consolidate the form render on a new method called add_to_cart_form. Add docblocks to all actions.
Hey @nefeline, would you mind converting this pull request to a draft until it's ready for review? 🙂 |
…r outputing the quantity.
Hey @imanish003 ! I'm so sorry: I forgot to create it as a draft, my bad =/ This is now ready for review. |
Thanks for the review @nerrad ! Regarding:
I made a series of changes to remove those Regarding the one added within the With that said, unless folks have any urgent concerns, I'd like to propose addressing the pre-existing exceptions that are still pending for grouped products and the quantity input in separate PRs as those are all in production already (in the core of Woo) and we have some urgency on clearing out the Fatal error and unblocking release Edit: See #9494 (comment) for additional change made following up this msg. |
Additional note regarding my previous comment over here:
Regarding the pre-existing global variable override: it was ditched as I just noticed it's not a requirement for rendering the block. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the one added within the add_to_cart_form protected method (which was explicitly introduced on this PR), instead of echoing the form, this method is now returning it. > But there's a minor caveat: we are now repeating the 'woocommerce_before_add_to_cart_form' and 'woocommerce_after_add_to_cart_form' actions across all methods for each one of the product types since we are not buffering the output anymore.
What implications are there. I'm assuming the actions will still only fire once depending on the product type?
Also, I go thinking, given product types are an extensible concept, one thing I'm wary about with this change is that any overrides that might exist for the templates in a theme (or injected via an extension) will no longer work.
Any refactor of this nature will need to also address the extensibility behaviour (beyond preserving actions) given that product types are an extensible concept. So while there's nothing really wrong with the code itself, I think it's premature to release it as is until we've figured out architecturally how things like the Add to Cart Block will be rendered on the server (will form elements be abstracted for instance) and the path to extensibility for that.
/** | ||
* Hook: woocommerce_after_single_variation. | ||
* | ||
* @since TBD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use the version placeholder, the release script will automatically add the correct version string.
* @since TBD | |
* @since $:VID:$ |
This could be updated in multiple places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I wasn't aware of this script, TIL!
Thanks @nerrad !
Yes, they will still fire once per product type; the only downside is that we are breaking the DRY principle by repeating those actions across different methods, for each one of the product types.
That's an excellent point; we definitely need a fallback for rendering custom product types and allowing extensibility, although it is not super clear to me what level of extensibility we should be providing as well: I'm thinking about a few possible paths, will share more details over here as soon as I have something more concrete to discuss. |
Bumping it to the 10.5.0 release. |
Thanks @nefeline, for your work! I will close this PR since that, at least for now, we don't see any major benefit with this approach. |
Thanks @gigitux, but this PR is not ready to be closed.
On the contrary: this is the basis for an upcoming discussion regarding block extensibility, and ultimately where we will be heading at some point in the near future. |
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
I'm removing the assigned milestone since before proceeding with this approach, it is necessary to discuss it |
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
Screen.Recording.2023-05-18.at.14.13.51.mov
With the removal of the global
$product
variable from this block on #9457 , follow-up changes are required to ensure it works as expected.Initially, this block relied on the native WooCommerce's templates for rendering the add-to-cart form button. The problem is those templates all rely exclusively on the global
$product
variable for rendering content, and in the absence of it, a Fatal error is thrown:Based on the outcome of the discussion on #9457 , on this PR an altered and consolidated version of all add-to-cart form templates is used for rendering the Add to Cart with Options block.
The templates consolidated here include:
woocommerce/templates/single-product/add-to-cart/external.php
woocommerce/templates/single-product/add-to-cart/grouped.php
woocommerce/templates/single-product/add-to-cart/simple.php
woocommerce/templates/single-product/add-to-cart/variable.php
woocommerce/templates/single-product/add-to-cart/variation-add-to-cart-button.php
Important notes
'woocommerce_single_variation'
, which was removed as it has thewoocommerce_single_variation_add_to_cart_button
function hooked to it and invokingwc_get_template( 'single-product/add-to-cart/variation-add-to-cart-button.php' );
, resulting in the same Fatal error aforementioned as a consequence).Fixes #9502
Other Checks
Testing
User Facing Testing
Add to Cart button with Options
works as expected and the product can be added to the cart.Add to Cart button with Options
is properly displayed for all of them, including: simple, variable, grouped, and external. Make sure all of these products can be added to cart.Add to Cart button with Options
block also works as expected within theSingle Product
block.Hooks Testing
woocommerce-gutenberg-products-block.php
file:WooCommerce Visibility
Changelog