[feature]: Replace ProductQuantity on Product Page with QuantityFields stepper #2690

huykon · 2020-09-09T13:23:37Z

Description

The quantity selector on PDP and cart don't currently match either in the way they look or in the way they behave.

The cart stepper is the latest design, and the quantity selector on PDP should be updated to match this.

Related Issue

Closes #2679.

Acceptance

Verification Stakeholders

@tjwiebell

Specification

Verification Steps

Go to the PDP.
Verify the quantity field is QuantityFields.

Screenshots / Screen Captures (if appropriate)

Checklist

I have added tests to cover my changes, if necessary.

I have updated the documentation accordingly, if necessary.

PWAStudioBot · 2020-09-09T13:37:07Z

	Fails
🚫	A version label is required. A maintainer must add one.

	Messages
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).

Generated by 🚫 dangerJS against d00884a

schensley · 2020-09-21T18:53:44Z

@huykon while this work does make the "stepper" quantity control more consistent with the control as it exists in cart, that its size (width) seems to only be restricted by the with of the area it exists in can present a bit of a problem in terms of recognition of the control or user expectations (see screen shot).
At some sizes it appears that that quantity control may expand to such a degree that

it's hard to recognize it as a means for setting the quantity (recognition) and
separates the "increase" and "decrease" buttons to a degree that can make for a less efficient UI.
I would recommend placing some restrictions to the width of this such that it will appear to accommodate a typical and reasonable quantity (if a shopper is likely to purchase quantities in the hundreds then it should look like a 3-digit number would fit). The "right" size might be a matter of opinion or aesthetics, but generally speaking there is a "shape" that most users recognize.
Also, just a question about the values that appear in the drop down when you click into the quantity field, I am assuming these are meant to be configurable, but some thought might be given to the height of the drop down container that it would fit within the viewport should it provide an long (infinite?) list of numbers.
Thanks.

huykon · 2020-09-22T01:14:28Z

@schensley Thank for your comment so what should I do now:

Reduce the width of quantity box
limit the max number to quantity input
Please give me your feedback!

tjwiebell · 2020-09-22T21:40:27Z

packages/venia-ui/lib/components/ProductFullDetail/productFullDetail.css

+    grid-template-columns: auto 1fr auto;
+    padding: 0 1rem;


@huykon - I would suggest this change given @schensley's feedback on sizing. This is no longer a dropdown, so I don't think anything else needs changed other than the width.

Suggested change

grid-template-columns: auto 1fr auto;

padding: 0 1rem;

grid-template-columns: auto 4rem auto;

justify-content: start;

I have just updated code as your recommend.

schensley · 2020-09-24T15:55:39Z

HI @huykon sorry I didn't get back to you, but it looks like you've done an excellent job. Thanks. UX approved.

huykon · 2020-09-25T01:00:00Z

@schensley thank you

tjwiebell

Change looks good 👍 ProductQuantity can safely be deprecated now, but I'll create a followup to make QuantityStepper @api and deprecate that old guy. Nice work!

dpatil-magento · 2020-09-30T05:26:45Z

@huykon @tjwiebell Let me know if we need to create follow up tickets or these needs to be handled here -

When user enter qty greater than available, no error is shown to user even though graphql returns it

Might just be a QA flow :) if user enters 0 in qty field then good to enable + stepper and disable Add to Cart button?

huykon · 2020-09-30T06:44:41Z

@dpatil-magento I will wait the confirmation from @tjwiebell to decide whether we should update or not. Thanks for your reporting

tjwiebell · 2020-09-30T13:41:27Z

@dpatil-magento I will wait the confirmation from @tjwiebell to decide whether we should update or not. Thanks for your reporting

Great catch @dpatil-magento - these errors are not getting passed to the QuantityStepper, which doesn't appear to have error handling even if we were. This should be resolved in this scope.

@huykon - Let's add an optional message prop to QuantityFields and dependently render a message element if truthy. It should auto flow to the next row given the grid, so you should just need grid-column-end: span 3; (and maybe justify-self: start so it's left aligned). Use previous rendering as a guide.

This is simple and should be fixed, just pass min={1} prop to QuantityFields

huykon · 2020-09-30T14:48:22Z

I have just updated. Please check again @tjwiebell @dpatil-magento

tjwiebell

Tiny change to make this more consistent with the code base then this looks good to go 🚀

packages/venia-ui/lib/components/CartPage/ProductListing/quantity.js

dpatil-magento · 2020-09-30T15:36:07Z

Thanks @huykon @tjwiebell , there are still some breaking flows -

2 (continuing) - Make qty blank and click outside (I think at this step only good to make qty display back to 1) and enter 01 which turns to 11 and now click add to cart button, user see the error.

3 - Add an item to cart and then increase the qty and directly (keeping cursor in qty field only) hit Add to cart again - Previous qty being added agian and not newly entered one

…lues

dpatil-magento · 2020-10-01T00:36:58Z

QA Approved.

sirugh

One comment but I won't hold the PR up on it as I don't have context around discussions with UX on this change.

sirugh · 2020-10-01T15:08:08Z

packages/venia-ui/lib/components/CartPage/ProductListing/quantity.js

@@ -29,41 +30,46 @@ export const QuantityFields = props => {
        maskInput
    } = talonProps;

+    const errorMessage = message ? <Message>{message}</Message> : null;


This looks good - my only concern is that the message is a bit close to the field and not red.

We stuck with the same UX from the dropdown treatment, which you recently implemented 😉 it looks pretty identical to me.

#2690 (comment)

sirugh · 2020-10-01T15:12:13Z

packages/venia-ui/lib/components/ProductFullDetail/productFullDetail.js

@@ -12,7 +12,7 @@ import Button from '../Button';
 import Carousel from '../ProductImageCarousel';
 import FormError from '../FormError';
 import { fullPageLoadingIndicator } from '../LoadingIndicator';
-import Quantity from '../ProductQuantity';


Oh also, can we throw a @deprecated label on the old ProductQuantity component? After this PR it will only be used in the legacy mini cart.

There were two things I wanted to do as a followup, prefer to just merge this:

#2690 (review)

Created PWA-960, thanks for the reminder; would have missed actually creating that in the backlog.

@tjwiebell please mention to me when you create that issue. I hope I can resolve that :D

tjwiebell

Do I have write access to approve?

change product quantity field to QuantityFields component

d708938

m2-community-project bot added this to Ready for Review in Pull Request Progress Sep 9, 2020

PWAStudioBot added the pkg:venia-ui label Sep 9, 2020

prettier code

7d4b3a5

tjwiebell reviewed Sep 22, 2020

View reviewed changes

m2-community-project bot assigned tjwiebell Sep 22, 2020

m2-community-project bot moved this from Ready for Review to Review in Progress in Pull Request Progress Sep 22, 2020

m2-community-project bot added the Progress: review label Sep 22, 2020

quantity field beauty

6ff4984

huykon requested a review from tjwiebell September 25, 2020 12:51

tjwiebell previously approved these changes Sep 28, 2020

View reviewed changes

Merge branch 'develop' into product/quantity-input

39b4ce7

update quantity stepper

82a356b

huykon dismissed tjwiebell’s stale review via 82a356b September 30, 2020 14:32

tjwiebell suggested changes Sep 30, 2020

View reviewed changes

packages/venia-ui/lib/components/CartPage/ProductListing/quantity.js Outdated Show resolved Hide resolved

add variable for expressions

c2408d7

tjwiebell previously approved these changes Sep 30, 2020

View reviewed changes

Move addToCart to onSubmit handler so mutation receives up to date va…

df0141a

…lues

tjwiebell dismissed their stale review via df0141a September 30, 2020 18:12

PWAStudioBot added the pkg:peregrine label Sep 30, 2020

Merge branch 'develop' into product/quantity-input

becf0cc

re-create snapshot for test case

d00884a

sirugh approved these changes Oct 1, 2020

View reviewed changes

m2-community-project bot assigned sirugh Oct 1, 2020

m2-community-project bot moved this from Review in Progress to Reviewer Approved in Pull Request Progress Oct 1, 2020

m2-community-project bot added Progress: approved and removed Progress: review labels Oct 1, 2020

sirugh reviewed Oct 1, 2020

View reviewed changes

sirugh approved these changes Oct 1, 2020

View reviewed changes

tjwiebell added the version: Major This changeset includes incompatible API changes and its release necessitates a Major version bump. label Oct 1, 2020

tjwiebell approved these changes Oct 1, 2020

View reviewed changes

dpatil-magento merged commit 0a3ec3e into magento:develop Oct 1, 2020

m2-community-project bot moved this from Reviewer Approved to Done in Pull Request Progress Oct 1, 2020

m2-community-project bot added Progress: done and removed Progress: approved labels Oct 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature]: Replace ProductQuantity on Product Page with QuantityFields stepper #2690

[feature]: Replace ProductQuantity on Product Page with QuantityFields stepper #2690

huykon commented Sep 9, 2020 •

edited

PWAStudioBot commented Sep 9, 2020 •

edited

schensley commented Sep 21, 2020

huykon commented Sep 22, 2020

tjwiebell Sep 22, 2020

huykon Sep 24, 2020

schensley commented Sep 24, 2020

huykon commented Sep 25, 2020

tjwiebell left a comment

dpatil-magento commented Sep 30, 2020

huykon commented Sep 30, 2020

tjwiebell commented Sep 30, 2020 •

edited

huykon commented Sep 30, 2020

tjwiebell left a comment

dpatil-magento commented Sep 30, 2020

dpatil-magento commented Oct 1, 2020

sirugh left a comment

sirugh Oct 1, 2020

tjwiebell Oct 1, 2020

sirugh Oct 1, 2020

tjwiebell Oct 1, 2020

tjwiebell Oct 1, 2020

huykon Oct 1, 2020

tjwiebell left a comment

[feature]: Replace ProductQuantity on Product Page with QuantityFields stepper #2690

[feature]: Replace ProductQuantity on Product Page with QuantityFields stepper #2690

Conversation

huykon commented Sep 9, 2020 • edited

Description

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented Sep 9, 2020 • edited

schensley commented Sep 21, 2020

huykon commented Sep 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schensley commented Sep 24, 2020

huykon commented Sep 25, 2020

tjwiebell left a comment

Choose a reason for hiding this comment

dpatil-magento commented Sep 30, 2020

huykon commented Sep 30, 2020

tjwiebell commented Sep 30, 2020 • edited

huykon commented Sep 30, 2020

tjwiebell left a comment

Choose a reason for hiding this comment

dpatil-magento commented Sep 30, 2020

dpatil-magento commented Oct 1, 2020

sirugh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjwiebell left a comment

Choose a reason for hiding this comment

huykon commented Sep 9, 2020 •

edited

PWAStudioBot commented Sep 9, 2020 •

edited

tjwiebell commented Sep 30, 2020 •

edited