[PWA-272] [Feature]: "Edit Item" From Cart Page Kebab Menu #2191

tjwiebell · 2020-02-25T18:48:59Z

Description

As a shopper, I want to edit my product from within the cart page workflow in order to change configurable options (size, color, quantity, etc.) prior to purchasing.

Today, edit exists within the mini-cart experience but during the build out of the new Cart page we realized that reusing this same experience requires additional design. This is a follow up story to PWA-238.

Need to decide on design treatment for edit item experience within the full cart.

Related Issue

[PWA-272] [Feature]: "Edit Item" From Cart Page Kebab Menu

Acceptance

Verification Stakeholders

Specification

Verification Steps

Add Simple and Configurable Product to cart and go to /cart page
Verify no edit option available for simple product
Edit Config Product, verify that all selected options are correct
Change some configurable options and quantity and update
Verify cart is updated with selections and totals are updated

Screenshots / Screen Captures (if appropriate)

Checklist

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

- Reuse product details from parent and render first half of modal content

…it-item

- Update some option props so they don't submit my form - Drill down saving state and sync with mutations

tjwiebell · 2020-02-25T18:51:35Z

packages/venia-ui/lib/components/ProductOptions/swatch.js

            style={finalStyle}
+            title={label}
+            type="button"


Swatch and tile were missing type="button" prop, and were triggering form submission on selection.

I was wondering why you re-ordered the props, now I get it Alphabetical order.

Is that OCD or our standard? I ask because, I have OCD too 😆

PWAStudioBot · 2020-02-25T18:55:27Z

	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-238.

Generated by 🚫 dangerJS against aef4e4e

revanth0212

I did not review the changes completely. Heard you are refactoring the code. Try to think about my comments while doing that.

packages/peregrine/lib/talons/CartPage/ProductListing/EditModal/useEditModal.js

revanth0212 · 2020-02-25T21:08:28Z

packages/peregrine/lib/talons/CartPage/ProductListing/EditModal/useProductDetail.js

+    ['OUT_OF_STOCK', 'Out of stock']
+]);
+
+export const useProductDetail = props => {


Does this needs to be a talon? It looks like a props mapper instead.

I would still consider destructuring or flattening of the graph logic, which I do want to contain in a talon. Open to extracting somewhere if you have a proposal.

We don't have anything formally defined for such use cases but it looks like the mapStateToProps we used to have in the olden golden days. Let's ask @jimbo

I found a place where we have done something similar but it is in a peregine talon. Not sure if we can just have the flatten/mapper functions in the component file.

pwa-studio/packages/peregrine/lib/talons/CartPage/ProductListing/useProduct.js

Line 86 in fce67e6

const flattenProduct = item => {

Don't look at the git blame 😉 that was also me. The intention was once we started to see overlap, that could be broken out into a utility; but these components differ slightly.

packages/venia-ui/lib/components/CartPage/ProductListing/EditModal/editModal.css

revanth0212 · 2020-02-27T19:02:31Z

packages/venia-ui/lib/components/CartPage/ProductListing/EditModal/productForm.js

+
+    if (isLoading) {
+        return (
+            <LoadingIndicator


I believe we can achieve some optimization by moving the loading indicator component instance to a useMemo and returning the result if isLoading. Since only 2 states exist for the component either loading or not potentially only creating 2 instances.

.... const loadingIndicator = useMemo(() => { if (isLoading) { return ( <LoadingIndicator classes={{ root: classes.loading }} >{`Fetching Product Options...`}</LoadingIndicator> ); } else { return null; } }, [isLoading, classes.loading]); if (isLoading) { return loadingIndicator } ....

I remember @jimbo left some comments on my PR about useMemo. Don't do this yet. Let me check with Jimmy first.

Very interested in this discussion. I have not been a huge fan of our overuse of useMemo and useCallback, so would be nice to define some usage criteria. Relevant article:

https://kentcdodds.com/blog/usememo-and-usecallback

packages/venia-ui/lib/components/CartPage/ProductListing/EditModal/productForm.js

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

revanth0212 · 2020-02-27T19:10:08Z

packages/venia-ui/lib/components/ProductOptions/swatch.js

            style={finalStyle}
+            title={label}
+            type="button"


I was wondering why you re-ordered the props, now I get it Alphabetical order.

Is that OCD or our standard? I ask because, I have OCD too 😆

- Write tests to cover all functionality

revanth0212 · 2020-02-28T19:08:04Z

packages/peregrine/lib/talons/CartPage/ProductListing/EditModal/__tests__/useEditModal.spec.js

+    useAppContext: jest.fn()
+}));
+
+describe('return correct open status', () => {


I personally like using describe but I remember I was asked not to use it unless there is no way and you want to mock/perform something for only certain tests. Check with @jimbo or other files.

I did find other usages that use it to group tests, so I'm going to leave this for now.

revanth0212

Just a couple of minor things. Let me know what you think about those.

revanth0212 · 2020-02-28T19:39:41Z

...ages/peregrine/lib/talons/CartPage/ProductListing/EditModal/__tests__/useProductForm.spec.js

+    return <i talonProps={talonProps} />;
+};
+
+const configItemResponse = {


What do you think about moving this to __fixtures__/configResponses.js?

Moved in ec2be48

revanth0212 · 2020-02-28T19:40:12Z

...ages/peregrine/lib/talons/CartPage/ProductListing/EditModal/__tests__/useProductForm.spec.js

+    loading: false
+};
+
+const cartItem = {


Even this guy to a __fixtures__ file.

Moved in ec2be48

revanth0212 · 2020-02-28T19:41:59Z

packages/venia-ui/lib/components/CartPage/ProductListing/EditModal/__tests__/editModal.spec.js

+});
+
+test('renders open drawer with active item', () => {
+    const mockItem = {


Since thig guy is small, it can stay here.

schensley

UX approved.

NOTE: there is some buggy-ness in that the option selection should reset (to the original config) if the edit drawer is closed without tapping the "Update" button.

fooman · 2020-03-02T09:09:37Z

Noted a minor issue with this PR on the new cart page. When trying to update the quantity when on the Cart page via the Mini Cart, the items are removed from the Cart page but not updated:

Totals are also still the old value.

dpatil-magento · 2020-03-02T16:00:32Z

@schensley Please check below observations related to UX.

Update button is enabled by default on Edit Cart Panel.
On Edit Cart Panel - Whats the reason of having product image when it does not get updated after changing configurations, also it consumes lots of space in mobile pushing buttons down .

schensley · 2020-03-02T16:12:17Z

Thank you @dpatil-magento for posting the comment.
I am providing some UX opinions below...

@schensley Please check below observations related to UX.

Update button is enabled by default on Edit Cart Panel.

On Edit Cart Panel - Whats the reason of having product image when it does not get updated after changing configurations, also it consumes lots of space in mobile pushing buttons down .

Yes. Disabled buttons can be confusing for shoppers and reduce confidence in the integrity of the system. In a case where a shopper were to open the edit panel, but make no edits and tapped the "Update" button, (ideally) the edit panel would close and nothing would change in the cart (essentially equivalent to tapping the close control).
The image should change to match the color selected - this is being looked into. But, the image is included in the edit panel to provide context for the edit, and the real estate is being taken advantage of to allow a more detailed view to the shopper, but this image area can be modified or not included if that is a decision the Merchant arrives at.

Thnx. Scott

- Ensure that item state is being cleared when the modal is closed

…udio into tommy/cart-page-edit-item

…ld not trigger a save

…it-item

- Fixup tests after mainline merge

dpatil-magento · 2020-03-03T16:28:08Z

QA Pass, good to merge.

@fooman Thanks for checking but Cart/Mini-Cart sync work is not done yet, so that issue is not addressed in scope of this PR.

tjwiebell added 5 commits February 21, 2020 12:19

- Create skeleton of components needed to render new edit modal

fd2dc7e

- Reuse product details from parent and render first half of modal content

Merge remote-tracking branch 'origin/develop' into tommy/cart-page-ed…

2cb795c

…it-item

Finish up UI work for edit modal

0f9d675

- Hook everything up to the proper mutations to update the backend

007f620

- Update some option props so they don't submit my form - Drill down saving state and sync with mutations

Address feedback from first pass of UX review

d57a961

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

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

tjwiebell commented Feb 25, 2020

View reviewed changes

m2-community-project bot assigned tjwiebell Feb 25, 2020

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

m2-community-project bot added the Progress: review label Feb 25, 2020

PWAStudioBot added pkg:peregrine pkg:venia-ui labels Feb 25, 2020

revanth0212 requested changes Feb 25, 2020

View reviewed changes

m2-community-project bot assigned revanth0212 Feb 25, 2020

m2-community-project bot moved this from Review in Progress to Changes Requested in Pull Request Progress Feb 25, 2020

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

tjwiebell added 2 commits February 26, 2020 14:51

Fix loading indicator

6eaba52

Address some early PR feedback

1522b8d

tjwiebell requested a review from revanth0212 February 26, 2020 21:10

Fix existing failing tests

e78bdb1

revanth0212 requested changes Feb 27, 2020

View reviewed changes

tjwiebell added 3 commits February 27, 2020 16:34

- Always render the empty modal so the inital animation looks correct

40ffdaa

- Write tests to cover all functionality

Resolve issue with test that was rendering twice

c4e523b

Address PR feedback

fbe5519

tjwiebell requested a review from revanth0212 February 28, 2020 18:23

revanth0212 reviewed Feb 28, 2020

View reviewed changes

tjwiebell requested a review from revanth0212 February 28, 2020 20:15

revanth0212 previously approved these changes Feb 28, 2020

View reviewed changes

Center quantity stepper based on article about usability

ebe9e36

tjwiebell dismissed revanth0212’s stale review via ebe9e36 February 28, 2020 21:04

revanth0212 previously approved these changes Feb 28, 2020

View reviewed changes

Reduce image size to get update button above fold on desktop

5346f97

tjwiebell dismissed revanth0212’s stale review via 5346f97 February 28, 2020 21:49

schensley previously approved these changes Feb 28, 2020

View reviewed changes

m2-community-project bot assigned schensley Feb 28, 2020

m2-community-project bot moved this from Changes Requested to Reviewer Approved in Pull Request Progress Feb 28, 2020

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

Merge branch 'develop' into tommy/cart-page-edit-item

4fe503a

tjwiebell added 2 commits March 2, 2020 11:57

- Fix Chrome complaining about inputType prop

b8a5e6b

- Ensure that item state is being cleared when the modal is closed

Merge branch 'tommy/cart-page-edit-item' of github.com:magento/pwa-st…

6d5b680

…udio into tommy/cart-page-edit-item

tjwiebell dismissed schensley’s stale review via 6d5b680 March 2, 2020 17:58

tjwiebell added 3 commits March 2, 2020 12:35

Cover edge case where an option being set back to original value shou…

8efb78b

…ld not trigger a save

Merge remote-tracking branch 'origin/develop' into tommy/cart-page-ed…

a7f51fb

…it-item

- Flip the bulk mutation so coupon codes are retained

d74f356

- Fixup tests after mainline merge

Merge branch 'develop' into tommy/cart-page-edit-item

aef4e4e

supernova-at approved these changes Mar 3, 2020

View reviewed changes

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

dpatil-magento merged commit 847878f into develop Mar 3, 2020

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

dpatil-magento deleted the tommy/cart-page-edit-item branch March 3, 2020 19:15

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PWA-272] [Feature]: "Edit Item" From Cart Page Kebab Menu #2191

[PWA-272] [Feature]: "Edit Item" From Cart Page Kebab Menu #2191

tjwiebell commented Feb 25, 2020

tjwiebell Feb 25, 2020

revanth0212 Feb 27, 2020

PWAStudioBot commented Feb 25, 2020 •

edited

revanth0212 left a comment

revanth0212 Feb 25, 2020

tjwiebell Feb 26, 2020

revanth0212 Feb 27, 2020

revanth0212 Feb 27, 2020

tjwiebell Feb 28, 2020

revanth0212 Feb 27, 2020

revanth0212 Feb 27, 2020

tjwiebell Feb 28, 2020

revanth0212 Feb 27, 2020

revanth0212 Feb 28, 2020 •

edited

tjwiebell Feb 28, 2020

revanth0212 left a comment

revanth0212 Feb 28, 2020

tjwiebell Feb 28, 2020

revanth0212 Feb 28, 2020

tjwiebell Feb 28, 2020

revanth0212 Feb 28, 2020

schensley left a comment

fooman commented Mar 2, 2020

dpatil-magento commented Mar 2, 2020

schensley commented Mar 2, 2020

dpatil-magento commented Mar 3, 2020

[PWA-272] [Feature]: "Edit Item" From Cart Page Kebab Menu #2191

[PWA-272] [Feature]: "Edit Item" From Cart Page Kebab Menu #2191

Conversation

tjwiebell commented Feb 25, 2020

Description

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PWAStudioBot commented Feb 25, 2020 • edited

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

revanth0212 Feb 28, 2020 • edited

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

schensley left a comment

Choose a reason for hiding this comment

fooman commented Mar 2, 2020

dpatil-magento commented Mar 2, 2020

schensley commented Mar 2, 2020

dpatil-magento commented Mar 3, 2020

PWAStudioBot commented Feb 25, 2020 •

edited

revanth0212 Feb 28, 2020 •

edited