-
Notifications
You must be signed in to change notification settings - Fork 219
Add minimum quantity, maximum quantity, and step (multiple_of) to the Cart Block and Store API #5406
Conversation
044d6b1
to
ec4fe27
Compare
Size Change: +1.51 kB (0%) Total Size: 815 kB
ℹ️ View Unchanged
|
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.
Thank you for working on this Mike! absolutely great effort. I have some comments, UX wise only, @vivialice is also going to add a comment here later regarding design. Feel free to merge after you handle my review, even if don't come to the same conclusion, I trust you have a better vision than me.
- When an item quantity doesn't match its step, you get this weird edge case (limit is 4, quantity was 2):
.
Once I clicked 6, the new quantity was 4, bound by 0 and 8. 0 didn't make a lot of sense to me
It's hard to know if the number is disabled or enabled.
Should we try to correct quantity to multipleOf so in the case above, the item quantity is increased to 4 on load.
I know you mentioned that
If the initial value isn't a multiplier of 4, after changing the quantity, your new quantity would be a multiplier of 4.
But I think we should correct it nevertheless? I managed to checkout no problem with that quantity.
The same is for limits, should the quantity be corrected to min or max in store API before returning? or would this complicate things?
For adding to cart, for items that already existed in cart and their quantity was less the minimum, I got an error:
Min is 5. I'd assume adding to cart would correct that to 5?
I know that most of those limitation are only showing given I'm adding filters to the fly while I have items in cart. I believe we should support this nevertheless, given a lot of limits are going to be conditional which mean they would kick in even if an item is already in cart.
const QuantityInput = ( { disabled, min, max, step = 1, value, onChange } ) => { | ||
const hasMaximum = typeof max !== 'undefined'; | ||
|
||
/** | ||
* The goal of this function is to normalize what was inserted, | ||
* but after the customer has stopped typing. | ||
* | ||
* It's important to wait before normalizing or we end up with | ||
* a frustrating experience, for example, if the minimum is 2 and | ||
* the customer is trying to type "10", premature normalizing would | ||
* always kick in at "1" and turn that into 2. | ||
* | ||
* Copied from <QuantitySelector> | ||
*/ | ||
const normalizeQuantity = useDebouncedCallback( ( initialValue ) => { | ||
// We copy the starting value. | ||
let newValue = initialValue; | ||
|
||
// We check if we have a maximum value, and select the lowest between what was inserted and the maximum. | ||
if ( hasMaximum ) { | ||
newValue = Math.min( | ||
newValue, | ||
// the maximum possible value in step increments. | ||
Math.floor( max / step ) * step | ||
); | ||
} | ||
|
||
// Select the biggest between what's inserted, the the minimum value in steps. | ||
newValue = Math.max( newValue, Math.ceil( min / step ) * step ); | ||
|
||
// We round off the value to our steps. | ||
newValue = Math.floor( newValue / step ) * step; | ||
|
||
// Only commit if the value has changed | ||
if ( newValue !== initialValue ) { | ||
onChange( newValue ); | ||
} | ||
}, 300 ); | ||
|
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.
I think we used this in a lot of places that it might be worth abstracting up?
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.
I think we should use the same component in both places (follow up) rather than abstracting this one function.
assets/js/atomic/blocks/product-elements/add-to-cart/product-types/simple.js
Show resolved
Hide resolved
assets/js/blocks/cart-checkout/cart/cart-line-items-table/cart-line-item-row.tsx
Show resolved
Hide resolved
Since this has some follow up work Im punting to next release. |
Heya @mikejolley ! I just have a comment about the stepper UI. It seems like it should be a selector instead to me, like this: Is this stepped solution displayed on the product page as well? |
@vivialice no, it's just on the cart right now but I think they will be unified eventually. A dropdown would force us to populate all possible options. That's fine for small numbers, but flawed for lots of steps or when there is no upper limit. The step attribute we're using comes from number inputs ref. I'm fine with putting the + / - back, but this will need a design solution because it's not clear visually what the + and - do when step is greater than 1. |
Ahh yes that's true. The original + / - is probably the better solution overall.
Do you mean it might be unclear it increases in multiples? |
@vivialice yes, because the UI for all + - buttons look the same, but the qty increment or decrement may differ. |
098e5aa
to
d51aad8
Compare
I've removed the custom step indicator so now all increments use + / - with no indication of qty.
I've mitigated this by running normalize on mount.
More difficult server side because we'd have to update cart items, but I think it's unlikely to change randomly like we're doing in testing, so we can leave for the normalization function in the client.
I didn't account for the case of min changing unexpectedly. For items not in the cart, 5 would be the inital number added. I think it might be useful to support but as it's edge case I won't work on this yet. |
does |
@ralucaStan Yes, I just confirmed so. The Mini Cart Block uses the same qty component, and same endpoints, so yes, these new props are respected there too. |
1fae9a9
to
27315f9
Compare
Closes #5037
Implements the following changes:
quantity_limits
returned from the Store APIcart
endpints, which containsminimum
,maximum
, andmultiple_of
. There is also aneditable
prop which can be set to false to disable inputs.minimum
,maximum
, andmultiple_of
as part of theadd_to_cart
param. I've updated theQuantityInput
component to match and support step, in the same way @senadir added support toQuantitySelector
.quantity_limits
are validated by the cart controller when adding an item to the cart, or when updating an item already in the cart. The API returns error 400 if the provided value is out of bounds.Styles the quantity input for stepped (multiple_of
) quantities so the +/- buttons clearly show what value comes next.editable
is false.Fixes #3888
Caveats:
This functionality is implemented in the Store API. Therefore, minimum, maximum, and step attributes can be circumvented via core functionality e.g. Shop page. Core offers its own filters to control this.
List of hooks
woocommerce_store_api_product_quantity_multiple_of
woocommerce_store_api_product_quantity_minimum
woocommerce_store_api_product_quantity_maximum
woocommerce_store_api_product_quantity_editable
(cart only)Testing steps
Developer testing steps:
Smoke test adding items to cart and changing quantity, either via the + and - buttons, or directly via the input.
Disable Selectors
Add the following code to disable quantity inputs for cart items:
Test that no selectors are shown on the cart page. If you attempt to update a qty via the API for an existing cart item, you will get a 400 error.
Step Property
Add the following code to change the multiple/step property for all cart items:
Min/Max for Cart Items
Add the following code to change the minimum and maximum for all cart items:
Min/Max for Cart Products
Add the following code to change the minimum qty for products (this affects add to cart events):
User facing testing steps:
Screenshots
Qty vs qty disabled:
Changelog