-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add qty rules client side validation to cart #3471
Conversation
@@ -42,13 +42,43 @@ class CartItems extends HTMLElement { | |||
} | |||
} | |||
|
|||
resetQuantityInput(id) { |
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.
This is a similar function to what we have in BulkAdd
in global.js
. However, it doesnt make sense to extend that component yet since we dont have all the functionalities yet
When we do (this ticket https://github.com/orgs/Shopify/projects/6397/views/28?pane=issue&itemId=61489062) , let's do that
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.
Makes sense to me. I think we can live with the duplication for now, especially since we have a chance to revisit it shortly.
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.
@@ -42,13 +42,43 @@ class CartItems extends HTMLElement { | |||
} | |||
} | |||
|
|||
resetQuantityInput(id) { |
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.
Makes sense to me. I think we can live with the duplication for now, especially since we have a chance to revisit it shortly.
@dan-menard The client side validation is for qty rules only. I dont see the |
PR Summary:
Ensuring the qty rules validation happens before the cart API is called (this way the error doesnt need to come back from the API)
Before:
After:
Why are these changes introduced?
Fixes #3399
What approach did you take?
I am pulling the same functions I use in QOL and Quick Add Bulk
Testing steps/scenarios
Demo links
Checklist