-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Always output the min and max attributes for the quantity selector. #35767
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
Conversation
Even if these are set to the same value (and so the quantity cannot be changed--which results in a readonly quantity selector) we must output them for compatibility with code (example: Composite Products) that expects them to always be present.
|
💡 Re failing required check "highlight template changes", in this case our initial goal is to ship this in 7.2.0 (I've made a request for a code freeze exception) so we don't require a further bump. I assume we can just ignore and request that Atlas force merge later on. |
Test Results SummaryCommit SHA: 58ed07d
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
|
Thank you @barryhughes for fixing this so quickly! Really appreciate it! I tested the fix and confirmed that the issue we witnessed in Composite Products has been resolved. Here's a video before the fix (the total price is not updated): Screen.Capture.on.2022-11-30.at.10-24-07.mp4and here's a video after the fix (the total price is updated): Screen.Capture.on.2022-11-30.at.10-25-09.mp4 |
jorgeatorres
left a comment
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.
LGTM. Thanks @barryhughes!
…35767) Even if these are set to the same value (and so the quantity cannot be changed--which results in a readonly quantity selector) we must output them for compatibility with code (example: Composite Products) that expects them to always be present.
* Always output the min and max attributes for the quantity selector. (#35767) Even if these are set to the same value (and so the quantity cannot be changed--which results in a readonly quantity selector) we must output them for compatibility with code (example: Composite Products) that expects them to always be present. * Prep for cherry pick 35767 Co-authored-by: Barry Hughes <[email protected]> Co-authored-by: WooCommerce Bot <[email protected]>
|
Hello everyone ! |
|
Hi @Stas0238, please do create a fresh bug report if you see problems. That's the best way to ensure it is seen and triaged 👍 |
This change builds on #34282 and makes a small amendment to improve compatibility with extensions like Composite Products.
In the earlier PR, we made it so that the quantity selector is still visible even if the min and max allowed quantities are set to the same value (for example, if precisely 2 units must be purchased). Whereas, previously, it was still rendered but was invisible.
However, as part of that earlier change, we also stopped outputting the
minandmaxattributes in some cases ... unfortunately, this posed a compatibility concern for some extensions which expect and depend on theminandmaxattributes being available. So, in this PR, we restore them.All Submissions:
Changes proposed in this Pull Request:
Always render the
minandmaxattributes for product quantity selectors, even in readonly mode where the quantity is fixed.How to test the changes in this Pull Request:
Start by re-testing as per the original instructions contained in #34282:
Optionally, you can continue to test by installing and activating Composite Products. To learn more about how this extension works please see these docs but, in summary, all you need to do is:
With that done:
Other information:
pnpm --filter=<project> changelog add?FOR PR REVIEWER ONLY: