Product sets: wishlist (@W-12301966@) #917

vmarta · 2023-01-20T01:39:47Z

Updating the wishlist functionality to work with product sets.

For PDP page:

Add-to-wishlist callback is more dynamic. It's now accepting a given product (so no longer assume to know which product on the page)

For Account's Wishlist page:

Call-to-action button for a product set: show Add All button if the children are not master/variant types. Or else show a button that navigate you to the PDP page.
Quantity for product set: when clicking Add All button, the quantity means the number in quantity selector multiplied by the number of items in the set
The logic requires setProducts data to exist but it was not available from the getProducts endpoint. So had to fetch the individual products for its setProducts data, but doing it in parallel for better performance.

Types of Changes

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Documentation update
Breaking change (could cause existing functionality to not work as expected)
Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

Removing a public function or component or prop

Adding a required argument to a function

Changing the data type of a function parameter or return value

Adding a new peer dependency to package.json

Changes

(change1)

How to Test-Drive This PR

(step1)

Checklists

General

Changes are covered by test cases
CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

There are no changes to UI

or...

Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
Changes comply with WCAG 2.0 guidelines levels A and AA
Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

Changes include a UI text update in the Retail React App (which requires translation)

I'll revert this commit later

bfeister · 2023-01-23T18:42:01Z

...es/template-retail-react-app/app/pages/account/wishlist/partials/wishlist-primary-action.jsx

+                {buttonText.viewOptions}
+            </Button>
+        )
+        // TODO: create a new set in BM to test this scenario


It's fine if our main demo instance doesn't have this change yet (we can create a ticket for that) but has this been tested against real data coming back from the API?

Yes, it's been tested. I've created the product set in my sandbox and tested it against the API.

bfeister · 2023-01-23T18:46:09Z

...es/template-retail-react-app/app/pages/account/wishlist/partials/wishlist-primary-action.jsx

+                {isOpen && (
+                    <ProductViewModal
+                        isOpen={isOpen}
+                        onOpen={onOpen}
+                        onClose={onClose}
+                        product={variant}
+                        addToCart={(variant, quantity) => handleAddToCart(variant, quantity)}
+                    />
+                )}
+            </>
+        ) : (
+            <Button
+                variant={'solid'}
+                onClick={() => handleAddToCart(variant, variant.quantity)}
+                w={'full'}
+                isLoading={isLoading}
+            >
+                {buttonText.addToCart}
+            </Button>
+        )
+    }
+
+    return button


Personally I like to avoid using let if at all possible. If you're open to it, I would consider just moving these to immediate returns like this

if (isProductASet) { return <>{something}</> } else if (condition) { return <>{somethingElse}</> } else { return <>{default}</> }

instead of doing let button and then declaring it in if / else block scope

bfeister · 2023-01-23T18:47:12Z

packages/template-retail-react-app/config/default.js

-                clientId: 'c9c45bfd-0ed3-4aa2-9971-40f88962b836',
-                organizationId: 'f_ecom_zzrf_001',
+                clientId: '199c814e-ce47-4af6-a699-b16e2c7b91e7',
+                organizationId: 'f_ecom_zzrf_009',


Is this change intentional? Maybe I'm missing something but isn't it moving the default to no longer be our demo zzrf_001 instance?

Yeah, it was intentional to make it easier for the code reviewer to test the changes. I plan to remove it before the PR is merged.

bfeister · 2023-01-23T18:47:30Z

packages/template-retail-react-app/config/default.js

@@ -55,7 +55,7 @@ module.exports = {
                path: 'api'
            },
            {
-                host: 'zzrf-001.dx.commercecloud.salesforce.com',
+                host: 'zzrf-009.dx.commercecloud.salesforce.com',


Same as https://github.com/SalesforceCommerceCloud/pwa-kit/pull/917/files#r1084422300

bfeister

Minor comments. No blockers or major concerns. Looks great @vmarta 🎉

vmarta · 2023-01-24T20:29:54Z

FYI I plan to do the following:

On the PDP, make sure the Add To Wishlist button for the individual children works with the child (not the parent)
Revert some api config change that was used for debugging/testing

bfeister

Looks good 👌

This reverts commit 484c8d8.

wjhsf · 2023-01-25T17:28:17Z

packages/template-retail-react-app/app/commerce-api/hooks/useCustomerProductLists.js

+                                product.setProducts = data.setProducts
+                            })
+
+                        productSetFetches.push(promise)


If you use .map instead of .forEach, you don't need .push 😉

wjhsf · 2023-01-25T17:28:48Z

packages/template-retail-react-app/app/commerce-api/hooks/useCustomerProductLists.js

+                        productSetFetches.push(promise)
+                    })
+
+                await Promise.allSettled(productSetFetches)


This ignores fetches that fail. What happens when some products have setProducts but others don't?

Good question.. it should be Promise.all.

I was thinking of this extending the data with fetched setProducts as something "optional". But actually, that was not accurate.

wjhsf · 2023-01-25T17:30:38Z

...es/template-retail-react-app/app/pages/account/wishlist/partials/wishlist-primary-action.jsx

-            }
-        ]
+
+        const isAddingASet = Boolean(item.setProducts)


Boolean() > !! 🙌

wjhsf · 2023-01-25T17:33:09Z

packages/template-retail-react-app/app/pages/product-detail/index.jsx

        try {
            await wishlist.createListItem({
-                id: product.id,
+                id: product.id || product.productId,


What's the difference between product.id and product.productId? Maybe add a comment explaining why we check both.

This product is actually either a product or a variant. With variant, its id is called productId. I think I need to be more explicit with the wording here.. have both product and variant.

* Product sets: basic PLP (#878) * Show product sets on PLP (alongside other products) For now, the product sets would look the same as any other kinds of products. * Parse the product type * Add todo * Ignore linting of this line for now * Add "Starting at" label * Product sets: basic pdp (@W-12301851@) (#897) * Product set: render the children products with minimal UI * No longer showing breadcrumbs in the child items * Individual swatches now have correct `value` There was a subtle bug where the `value` of the SwatchGroup was passed down to override all the values of the children Swatch components. * 1st attempt at saving swatches state in the page url * Avoid null in the url search params * Some refactoring * Add-to-cart now works for child items * Show accordion for each child item * Add todo * Rename variable for accuracy The child product itself is actually not a set, but is considered a set product. * Optional argument * Some refactoring and fix linting * Helper hook for url search params * Clean up * Rename module for a specific use case (PDP) * Show/hide add-to-cart buttons accordingly * Modal shows product set data * Parent product * Variant's product image shows up in add-to-cart modal * Rename variables for clarity * Fix linting error * Add horizontal rules to separate the parent and child items * Tweak whitespace * Localize the Starting At label * Add todo * Product sets: wishlist (@W-12301966@) (#917) * Starting At label * 1st attempt at updating wishlist for product sets * "Select Options" → "View Options" * Some refactoring * Render add-all-to-cart button if necessary * Better way to fetch `setProducts` data * Switch to zzrf-009 temporarily * For debugging purpose I'll revert this commit later * Simpler requirement * Update what quantity meant for product set * Some refactoring * Add comment * Add-to-wishlist for the individual child items * Rename variable * Update button text on PDP * Update button text on Wishlist page * Revert "Switch to zzrf-009 temporarily" This reverts commit 484c8d8. * PDP: show wishlist button on mobile * Fix formatting issues (prettier) * PR feedback * Clearer as to which is product or variant * Make it consistent with addToWishlist signature * [Feature] Add `Add Set to Cart` Button for Product Sets (#931) * Product sets: basic PLP (#878) * Variant's product image shows up in add-to-cart modal * Get modal showing multiple items added to cart * Add scroll to after validation Co-authored-by: Vincent Marta <[email protected]> * Einstein API to support product sets (#962) * verify viewcategory and viewsearch is working * viewproduct event is sent for every child product * addtocart event fires for all children when add set to cart * parent set only sends id * Design enhancements for product sets (#975) * remove complete the set carousel * remove complete the set translations * sticky buttons on mobile view * fix modal vertical margins on desktop * move recos to modalbody and cleanup margins * add dividers between product set items in mobile view * fix margins so sticky buttons do not overlap with content * add lazy loading of product set children product images * remove height - does nothing * bump build size to 55 * add conditional for complete the set and change loading var name * lint * revert translations * Cleanup * added to cart modal is scrollable and 90% of viewport * remove dialogprops * lint * Testing product sets - @W-12414116@ (#1003) * Product tile: test product set's price label * Test product set rendering * Test callbacks are called properly * itemAccumulatedCount is redundant * Test that modal can render multiple products * Finally the add-to-cart modal shows up in testing * Clean up * Test error messages * Test lazy loading of the child products * Split a test into smaller ones * Use `describe` to group all the related tests * Update comment * Test the wishlist and its primary action buttons * Clean up * Some refactoring * Some refactoring * Linting fix * Update comment * Test `getDisplayVariationValues` * Test the usePDPSearchParams * Tweak another test to pass Not related to product sets, but needing CI to pass. * Move mocked data into their own files * Moving it to within `beforeEach` --------- Co-authored-by: Ben Chypak <[email protected]> Co-authored-by: yunakim714 <[email protected]>

vmarta added 10 commits January 18, 2023 18:46

Starting At label

b0e3b11

1st attempt at updating wishlist for product sets

fba1f7b

"Select Options" → "View Options"

d86479b

Some refactoring

1143550

Render add-all-to-cart button if necessary

f9d7f2c

Better way to fetch setProducts data

0c9fe68

Switch to zzrf-009 temporarily

484c8d8

For debugging purpose

304ef2c

I'll revert this commit later

Simpler requirement

eb29f64

Update what quantity meant for product set

85d44d9

vmarta changed the title ~~Product sets: wishlist~~ Product sets: wishlist (@W-12301966@) Jan 21, 2023

bfeister reviewed Jan 23, 2023

View reviewed changes

bfeister approved these changes Jan 23, 2023

View reviewed changes

Some refactoring

aefeebb

vmarta added 3 commits January 24, 2023 13:47

Add comment

d3336ca

Add-to-wishlist for the individual child items

f0c7258

Rename variable

6598a4b

vmarta marked this pull request as ready for review January 24, 2023 23:34

vmarta requested a review from a team as a code owner January 24, 2023 23:34

bfeister approved these changes Jan 24, 2023

View reviewed changes

vmarta added 4 commits January 24, 2023 16:00

Merge branch 'feature/product-sets' into product-sets-wishlist

51ddcd6

Update button text on PDP

2fd92d3

Update button text on Wishlist page

de6acf4

Revert "Switch to zzrf-009 temporarily"

a6a51b9

This reverts commit 484c8d8.

wjhsf approved these changes Jan 25, 2023

View reviewed changes

PDP: show wishlist button on mobile

eed9ddd

vmarta added 4 commits January 25, 2023 17:05

Fix formatting issues (prettier)

810bbcf

PR feedback

88450e5

Clearer as to which is product or variant

87cd8cb

Make it consistent with addToWishlist signature

b3c2a2a

vmarta merged commit f3a4e7d into feature/product-sets Jan 26, 2023

vmarta deleted the product-sets-wishlist branch January 26, 2023 02:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Product sets: wishlist (@W-12301966@) #917

Product sets: wishlist (@W-12301966@) #917

vmarta commented Jan 20, 2023 •

edited

Loading

bfeister Jan 23, 2023

vmarta Jan 24, 2023

bfeister Jan 23, 2023

bfeister Jan 23, 2023

vmarta Jan 24, 2023

bfeister Jan 23, 2023

bfeister left a comment

vmarta commented Jan 24, 2023

bfeister left a comment

wjhsf Jan 25, 2023

wjhsf Jan 25, 2023

vmarta Jan 26, 2023

wjhsf Jan 25, 2023

wjhsf Jan 25, 2023

vmarta Jan 26, 2023

Product sets: wishlist (@W-12301966@) #917

Product sets: wishlist (@W-12301966@) #917

Conversation

vmarta commented Jan 20, 2023 • edited Loading

Types of Changes

Changes

How to Test-Drive This PR

Checklists

General

Accessibility Compliance

Localization

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

bfeister left a comment

Choose a reason for hiding this comment

vmarta commented Jan 24, 2023

bfeister 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

vmarta commented Jan 20, 2023 •

edited

Loading