This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Add to Cart with Options Block: Remove global variable overwrite. #9457
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
woocommercebot
requested review from
a team and
kmanijak
and removed request for
a team
|
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.08 MB ℹ️ View Unchanged
|
4 tasks
kmanijak
approved these changes
May 15, 2023
There was a problem hiding this comment.
Heads up "Add to Cart Form" was renamed to "Add to Cart Options" in #9238, so please update the testing steps 🙏
Otherwise, it's testing well! Checked with different types of products: simple, variable, and grouped.
Thanks for your fast reaction to this issue! 🚀
nerrad
reviewed
May 15, 2023
thealexandrelara
approved these changes
May 15, 2023
There was a problem hiding this comment.
Thank you for working on this and fixing the issue so quickly, I can confirm that the following scenarios are working as expected:
✅ Works when we have just one Single Product block is added to a page/post
✅ Works when we multiple Single Product block are added to a page/post
✅ Works on the Single Product page
✅ Works on the Single Product page inside a Single Product block
✅ Works with Simple, Grouped, Variable, and External products
Regarding the code, I agree with Darren that we could leave the variable name as $product instead of $single_product. I would also change the title of this PR and the testing notes to Add to Cart with Options (as pointed by Karol).
Since there is nothing I see as being a blocker, I'm approving the PR 🚀
nefeline
changed the title
|
Thanks all for the reviews! 🙌
Good point, updated!
I did this change 100% on purpose, as I wanted to make it even more evident that this is a local variable (even though, in practice, the name doesn't impact the end result at all). Since folks here are more fond of |
Important NoteThe Add to Cart with Options block might not function as expected anymore within the Single Product block after this PR + the equivalent change to the Product Gallery block are merged: this is a temporary regression for this experimental block. Since no concerns were raised with the approach of relying on our altered version of those templates for rendering the Add to Cart with Options and the Product Gallery blocks opened for discussion on this PR, I'm opening two separate PRs to introduce our version of those and restore functionality for this block. cc: @nerrad , @gigitux , @thealexandrelara |
4 tasks
This was referenced May 18, 2023
3 tasks
tarunvijwani
changed the title
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ensure the
$productglobal variable is not overwritten within theAdd to Cart with Optionsblock.Note about the usage of the Add to Cart with Options block within the Single Product block.
While this PR fixes the problem when the Add to Cart with Options block is used within the Single Product template, additional changes will be required to fully support its functionality within the (currently experimental) Single Product block, as all WooCommerce's relevant templates currently rely exclusively on the global
$productvariable for rendering content (e.g. https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce/templates/single-product/add-to-cart/simple.php#L20-L24 ). With that said, I see two alternatives:$productvariable to do so.Between these two options, I'm more inclined to stick with the second one; as for the core of Woo, this change might not be reasonable considering the nature of the templates and their intended use-case: opening this topic for discussion in case folks have different ideas/suggestions/concerns. As soon as we make a decision, a new PR will be opened to address this use case.
Edit: I've opened a draft PR (#9458) with the fix for the Product Gallery Block, relying on this second approach as suggested here.
cc: @nerrad @danielwrobert @gigitux @thealexandrelara
Fixes #9453
User Facing Testing
WooCommerce Visibility
Performance Impact
Changelog