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
[feature]: Replace ProductQuantity on Product Page with QuantityFields stepper #2690
Changes from 6 commits
d708938
7d4b3a5
6ff4984
39b4ce7
82a356b
c2408d7
df0141a
becf0cc
d00884a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,5 +119,6 @@ exports[`renders quantity correctly 1`] = ` | |
</span> | ||
</button> | ||
</div> | ||
|
||
</form> | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ import Button from '../Button'; | |
import Carousel from '../ProductImageCarousel'; | ||
import FormError from '../FormError'; | ||
import { fullPageLoadingIndicator } from '../LoadingIndicator'; | ||
import Quantity from '../ProductQuantity'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh also, can we throw a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There were two things I wanted to do as a followup, prefer to just merge this: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created PWA-960, thanks for the reminder; would have missed actually creating that in the backlog. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tjwiebell please mention to me when you create that issue. I hope I can resolve that :D |
||
import { QuantityFields } from '../CartPage/ProductListing/quantity'; | ||
import RichText from '../RichText'; | ||
|
||
import defaultClasses from './productFullDetail.css'; | ||
|
@@ -145,9 +145,11 @@ const ProductFullDetail = props => { | |
<section className={classes.options}>{options}</section> | ||
<section className={classes.quantity}> | ||
<h2 className={classes.quantityTitle}>Quantity</h2> | ||
<Quantity | ||
<QuantityFields | ||
classes={{ root: classes.quantityRoot }} | ||
initialValue={quantity} | ||
onValueChange={handleSetQuantity} | ||
onChange={handleSetQuantity} | ||
min={1} | ||
message={errors.get('quantity')} | ||
/> | ||
</section> | ||
|
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 looks good - my only concern is that the message is a bit close to the field and not red.
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.
We stuck with the same UX from the dropdown treatment, which you recently implemented 😉 it looks pretty identical to me.
#2690 (comment)