[feat]: Checkout Page Order Summary #2271

sirugh · 2020-03-19T20:10:09Z

Description

Adds the order summary to the checkout page. I reused the price summary component from cart page so we can discuss how to properly store/structure the directories here if we want.

That said, in re-using the pre-existing component I was able to skip over a lot of repetition. The only difference between cart and checkout summaries are that on cart we have the word "Estimated" in front of shipping, tax and total as well as a "proceed to checkout" button. On the checkout page there is no button and we remove the word "Estimated".

I also added a Link functionality to move us from cart to checkout by clicking the button :)

Related Issue

Closes PWA-184.

Acceptance

Verification Stakeholders

Specification

Verification Steps

Add an item to your cart.
Go to the cart page, estimate your shipping, add some other things like coupon/gift card. Note how the price summary looks as well as the totals therein.
Click to proceed to checkout. The totals should mirror what you saw on cart. The text should not say "Estimated" and there should be no "Proceed to checkout" button.
Regress in mobile.

Screenshots / Screen Captures (if appropriate)

Video

Desktop

Mobile

Checklist

I have updated the documentation accordingly, if necessary.
I have added tests to cover my changes, if necessary.

Signed-off-by: sirugh <rugh@adobe.com>

…sCheckout prop used to determine some string values Signed-off-by: sirugh <rugh@adobe.com>

Signed-off-by: sirugh <rugh@adobe.com>

PWAStudioBot · 2020-03-19T20:20:33Z

	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).
📖	Associated JIRA tickets: PWA-184.

Generated by 🚫 dangerJS against 03dd7ba

Signed-off-by: sirugh <rugh@adobe.com>

supernova-at · 2020-03-20T16:52:54Z

packages/peregrine/lib/talons/CartPage/PriceSummary/usePriceSummary.js

@@ -28,6 +29,10 @@ export const usePriceSummary = props => {

    const [{ cartId }] = useCartContext();

+    const location = useLocation();
+    // We don't want to display "Estimated" or the "Proceed" button in checkout.
+    const isCheckout = location.pathname === '/checkout';


Nice. An alternate approach would be passing isCheckout in as a prop.

I believe React Router has a hook for this that handles edge cases properly. Could be wrong.

supernova-at · 2020-03-20T16:59:33Z

packages/venia-ui/lib/components/CartPage/PriceSummary/priceSummary.js

+                <Link to={'/checkout'} className={classes.images}>
+                    {'Proceed to Checkout'}
+                </Link>
+            </Button>


I like this but I wonder if we don't miss out on potential analytics event hooks or similar by not having a handleProceedToCheckout in the talon.

But I suppose these JSX files are in the domain of the merchants writing their own storefronts, so they could easily add their own onClick if they wanted.

I think if people wanted to they could extend the Link to trigger some analytic handler. Certainly something to be aware of.

This is invalid HTML. You can't put an <a> element inside a <button>.

Permitted content: Phrasing content but there must be no Interactive content

If this is a submit button for a form, handle the navigation in the form's onSubmit handler; otherwise, either handle the navigation in the button's onClick handler or style the Link to match the desired appearance.

packages/venia-ui/lib/components/CheckoutPage/checkoutPage.css

revanth0212 · 2020-03-20T18:19:09Z

packages/venia-ui/lib/components/CartPage/PriceSummary/priceSummary.js

@@ -59,11 +54,21 @@ const PriceSummary = props => {

    const { subtotal, total, discounts, giftCards, taxes, shipping } = flatData;

-    const priceClass = isCartUpdating ? classes.priceUpdating : classes.price;
-    const totalPriceClass = isCartUpdating
+    const priceClass = isUpdating ? classes.priceUpdating : classes.price;


I understand this is not on you, but can you change the style of css class names to underscore casing from camel casing? I am not sure if we have a standard on that, but we should make a choice here for consistency.

There are many places where this is happening, and we have to make changes incrementally once we decide on what is the way to go.

I tend to write classes with camel case. Most of the underscore names you see are from the original scaffolding of the checkout page where we didn't call out the naming convention of css classes.

We can discuss as a team and add something to the wiki.

Correct. Classnames should, preferably, be camelcased.

revanth0212 · 2020-03-20T18:19:32Z

packages/venia-ui/lib/components/CartPage/PriceSummary/priceSummary.js

        ? classes.priceUpdating
        : classes.totalPrice;

+    const proceedToCheckoutButton = !isCheckout ? (
+        <div className={classes.checkoutButton_container}>


It is camel and underscore here.

packages/venia-ui/lib/components/CheckoutPage/checkoutPage.css

…-summary

revanth0212

As per our conversations over slack, I am fine with the CSS naming here. Approved 👍

Signed-off-by: sirugh <rugh@adobe.com>

sirugh · 2020-03-23T19:46:37Z

packages/peregrine/lib/talons/CheckoutPage/useCheckoutPage.js

@@ -122,6 +123,15 @@ export const useCheckoutPage = props => {
        }
    }, [cartId, getCheckoutDetails, getCheckoutStep, setCheckoutStep]);

+    const windowSize = useWindowSize();


It feels odd to me to use this hook inside a talon since window size is presentational.

Is there any other solution for this?

What do you mean?

Ok I moved this to the component. It did not feel like logic that belonged to a talon. We can discuss in general.

Signed-off-by: sirugh <rugh@adobe.com>

packages/venia-ui/lib/components/CheckoutPage/OrderSummary/orderSummary.css

Signed-off-by: sirugh <rugh@adobe.com>

jimbo · 2020-03-23T22:12:14Z

packages/venia-ui/lib/components/CheckoutPage/checkoutPage.js

@@ -121,26 +138,18 @@ const CheckoutPage = props => {
                            : 'Review and Place Order'}


I understand that this expression was already here, not introduced by this PR, but when you see a multiline expression in a component you're working in, might as well fix it. 🙂

Didn't see it, can fix though.

jimbo · 2020-03-23T22:13:51Z

packages/venia-ui/lib/components/CheckoutPage/checkoutPage.js

        setShippingInformationDone,
        setShippingMethodDone,
        setPaymentInformationDone
    } = talonProps;

    const classes = mergeClasses(defaultClasses, propClasses);

+    const windowSize = useWindowSize();
+    const isMobile = windowSize.innerWidth <= 960;


Hiding a bit of UI should be done in CSS with display: none, not in JS with useWindowSize. There are exceptions, but this isn't one of them.

The only way out of this that I see is applying a class based on the step, and then handling the condition with css media query. Sound good?

jimbo · 2020-03-23T22:14:36Z

packages/venia-ui/lib/components/CheckoutPage/checkoutPage.css

+    .payment_information_heading {
+        composes: stepper_heading;
+        /* On mobile, order summary has a top border, so avoid doubling up. */
+        border-bottom: unset;


When you're overriding one aspect of a shorthand property, just go for the minimum necessary delta. Helps clarify the intent and in some cases (not this one) helps with transitions.

border-bottom-width: 0;

Fair :) idk why but I thought unset applied the default

jimbo · 2020-03-23T22:16:57Z

packages/venia-ui/lib/components/CheckoutPage/PaymentInformation/paymentInformation.css

+
+@media (max-width: 960px) {
+    .heading {
+        border: unset;


Likewise, just override the border width.

jimbo · 2020-03-23T22:24:18Z

packages/venia-ui/lib/components/CartPage/PriceSummary/priceSummary.js

+                <Link to={'/checkout'} className={classes.images}>
+                    {'Proceed to Checkout'}
+                </Link>
+            </Button>


This is invalid HTML. You can't put an <a> element inside a <button>.

Permitted content: Phrasing content but there must be no Interactive content

If this is a submit button for a form, handle the navigation in the form's onSubmit handler; otherwise, either handle the navigation in the button's onClick handler or style the Link to match the desired appearance.

jimbo · 2020-03-23T22:26:12Z

packages/venia-ui/lib/components/CartPage/PriceSummary/priceSummary.js

@@ -59,11 +54,21 @@ const PriceSummary = props => {

    const { subtotal, total, discounts, giftCards, taxes, shipping } = flatData;

-    const priceClass = isCartUpdating ? classes.priceUpdating : classes.price;
-    const totalPriceClass = isCartUpdating
+    const priceClass = isUpdating ? classes.priceUpdating : classes.price;


Correct. Classnames should, preferably, be camelcased.

jimbo · 2020-03-23T22:28:01Z

packages/peregrine/lib/talons/CartPage/PriceSummary/usePriceSummary.js

@@ -28,6 +29,10 @@ export const usePriceSummary = props => {

    const [{ cartId }] = useCartContext();

+    const location = useLocation();
+    // We don't want to display "Estimated" or the "Proceed" button in checkout.
+    const isCheckout = location.pathname === '/checkout';


I believe React Router has a hook for this that handles edge cases properly. Could be wrong.

sirugh added 5 commits March 19, 2020 11:54

Links to checkout page from within cart price summary

86c8509

Signed-off-by: sirugh <rugh@adobe.com>

Renames to OrderSummary and uses cart price summary

6add2ea

Signed-off-by: sirugh <rugh@adobe.com>

Reuses price summary component with a minor refactor to allow a new i…

36dc35e

…sCheckout prop used to determine some string values Signed-off-by: sirugh <rugh@adobe.com>

Removes border per ux guidance. Minor css tidying.

d26acdb

Fixes tests

29e5e0d

sirugh requested a review from jimbo March 19, 2020 20:10

Add isUpdating state to checkout for eventual use

f714116

Signed-off-by: sirugh <rugh@adobe.com>

PWAStudioBot added pkg:peregrine pkg:venia-ui labels Mar 19, 2020

sirugh added 2 commits March 19, 2020 15:25

Disable proceed button while mutations are in flight

1f2590e

Signed-off-by: sirugh <rugh@adobe.com>

Updates snapshots

f2fb2bd

Signed-off-by: sirugh <rugh@adobe.com>

supernova-at reviewed Mar 20, 2020

View reviewed changes

m2-community-project bot assigned supernova-at Mar 20, 2020

m2-community-project bot added the Progress: review label Mar 20, 2020

supernova-at previously approved these changes Mar 20, 2020

View reviewed changes

m2-community-project bot assigned sirugh Mar 20, 2020

revanth0212 requested changes Mar 20, 2020

View reviewed changes

m2-community-project bot assigned revanth0212 Mar 20, 2020

m2-community-project bot added Progress: changes requested and removed Progress: review labels Mar 20, 2020

Merge remote-tracking branch 'origin/develop' into rugh/pwa-184-order…

1ec72da

…-summary

sirugh dismissed supernova-at’s stale review via 1ec72da March 20, 2020 18:36

revanth0212 previously approved these changes Mar 20, 2020

View reviewed changes

CSS tweaks after merge

e5ad429

Signed-off-by: sirugh <rugh@adobe.com>

sirugh dismissed revanth0212’s stale review via e5ad429 March 20, 2020 19:01

revanth0212 previously approved these changes Mar 20, 2020

View reviewed changes

Comment out prop for now

07e6efb

Signed-off-by: sirugh <rugh@adobe.com>

sirugh dismissed revanth0212’s stale review via 07e6efb March 20, 2020 19:38

revanth0212 previously approved these changes Mar 20, 2020

View reviewed changes

On mobile, hide order summary unless in review step

48a0c7c

Signed-off-by: sirugh <rugh@adobe.com>

sirugh dismissed revanth0212’s stale review via 48a0c7c March 23, 2020 19:45

sirugh commented Mar 23, 2020

View reviewed changes

simpler conditional

ff4b73b

Signed-off-by: sirugh <rugh@adobe.com>

revanth0212 approved these changes Mar 23, 2020

View reviewed changes

revanth0212 requested changes Mar 23, 2020

View reviewed changes

packages/venia-ui/lib/components/CheckoutPage/OrderSummary/orderSummary.css Outdated Show resolved Hide resolved

Move conditional to component since it is presentational

befd532

Signed-off-by: sirugh <rugh@adobe.com>

revanth0212 previously approved these changes Mar 23, 2020

View reviewed changes

tjwiebell added the version: Minor This changeset includes functionality added in a backwards compatible manner. label Mar 23, 2020

Remove talon prop

03dd7ba

Signed-off-by: sirugh <rugh@adobe.com>

sirugh dismissed revanth0212’s stale review via 03dd7ba March 23, 2020 21:01

tjwiebell approved these changes Mar 23, 2020

View reviewed changes

m2-community-project bot assigned tjwiebell Mar 23, 2020

m2-community-project bot added Progress: approved and removed Progress: changes requested labels Mar 23, 2020

tjwiebell merged commit 932b3fc into develop Mar 23, 2020

tjwiebell deleted the rugh/pwa-184-order-summary branch March 23, 2020 21:14

m2-community-project bot removed the Progress: approved label Mar 23, 2020

tjwiebell mentioned this pull request Mar 23, 2020

Order Summary - Fix font size unit #2275

Merged

jimbo reviewed Mar 23, 2020

View reviewed changes

m2-community-project bot assigned jimbo Mar 23, 2020

sirugh mentioned this pull request Mar 24, 2020

[chore]: Order Summary fast follow #2278

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feat]: Checkout Page Order Summary #2271

[feat]: Checkout Page Order Summary #2271

sirugh commented Mar 19, 2020 •

edited

Loading

PWAStudioBot commented Mar 19, 2020 •

edited

Loading

supernova-at Mar 20, 2020

jimbo Mar 23, 2020

supernova-at Mar 20, 2020

sirugh Mar 20, 2020

jimbo Mar 23, 2020

revanth0212 Mar 20, 2020

revanth0212 Mar 20, 2020 •

edited

Loading

sirugh Mar 20, 2020

jimbo Mar 23, 2020

revanth0212 Mar 20, 2020

revanth0212 left a comment

sirugh Mar 23, 2020

revanth0212 Mar 23, 2020

sirugh Mar 23, 2020

sirugh Mar 23, 2020

jimbo Mar 23, 2020

sirugh Mar 23, 2020

jimbo Mar 23, 2020

sirugh Mar 23, 2020

jimbo Mar 23, 2020

sirugh Mar 23, 2020 •

edited

Loading

jimbo Mar 23, 2020

jimbo Mar 23, 2020

jimbo Mar 23, 2020

jimbo Mar 23, 2020

		@@ -121,26 +138,18 @@ const CheckoutPage = props => {
		: 'Review and Place Order'}

[feat]: Checkout Page Order Summary #2271

[feat]: Checkout Page Order Summary #2271

Conversation

sirugh commented Mar 19, 2020 • edited Loading

Description

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented Mar 19, 2020 • edited Loading

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

revanth0212 Mar 20, 2020 • edited Loading

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

revanth0212 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sirugh Mar 23, 2020 • edited Loading

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

sirugh commented Mar 19, 2020 •

edited

Loading

PWAStudioBot commented Mar 19, 2020 •

edited

Loading

revanth0212 Mar 20, 2020 •

edited

Loading

sirugh Mar 23, 2020 •

edited

Loading