Skip to content
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

Account's wishlist: integrate hooks #1042

Merged
merged 41 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
11b6261
1st attempt at integrating the hooks
vmarta Mar 7, 2023
c862ffe
Fix iterating the `wishLists`
vmarta Mar 7, 2023
3923dc8
1st attempt at handling the qty change
vmarta Mar 7, 2023
0b444d5
Attempt at selective update
vmarta Mar 7, 2023
d72419c
Simplify the UX by not doing this optimistically
vmarta Mar 7, 2023
a82ae6d
Better loading states, and consider empty list
vmarta Mar 7, 2023
fbf6cba
More typescript friendly
vmarta Mar 7, 2023
428c3a3
Add comment
vmarta Mar 7, 2023
6d8d9bb
Clean up
vmarta Mar 8, 2023
a35170d
Some refactoring
vmarta Mar 8, 2023
4dd1661
Merge branch 'feature/integrate-commerce-sdk-react' into hooks-accoun…
vmarta Mar 9, 2023
7fe8af0
Remove debug code
vmarta Mar 9, 2023
dff593a
Add to cart button
vmarta Mar 9, 2023
e51439c
More button click handler
vmarta Mar 10, 2023
5524928
More concise way to selectively update
vmarta Mar 10, 2023
da6129e
Attempt at passing the 1st test
vmarta Mar 10, 2023
7900f52
Merge branch 'feature/integrate-commerce-sdk-react' into hooks-accoun…
vmarta Mar 13, 2023
907401a
Update useCurrentBasket call after merge
vmarta Mar 13, 2023
c6ad3fc
Remove redundant test
vmarta Mar 13, 2023
3de2b35
Update test for the Remove button
vmarta Mar 14, 2023
49eb095
Fix the rest of the tests
vmarta Mar 14, 2023
fa90831
Extract the mocked data
vmarta Mar 14, 2023
20d0c2e
Merge branch 'feature/integrate-commerce-sdk-react' into hooks-accoun…
vmarta Mar 14, 2023
1f307c0
Fix more tests
vmarta Mar 14, 2023
20c2186
Merge branch 'feature/integrate-commerce-sdk-react' into hooks-accoun…
vmarta Mar 14, 2023
a7fad37
PDP: fix broken add-to-wishlist button
vmarta Mar 14, 2023
1bd4a5c
Clarify why we're calling `onClick` twice
vmarta Mar 14, 2023
f5159e2
Small refactoring
vmarta Mar 14, 2023
6bd9593
Consider being undefined
vmarta Mar 14, 2023
3c47cf8
PR feedback
vmarta Mar 14, 2023
487c775
No longer does selective update for the cache
vmarta Mar 14, 2023
438a45c
Decreasing quantity to 0 would auto-remove the item
vmarta Mar 15, 2023
df6e823
PDP: add-to-wishlist now works properly again for product sets
vmarta Mar 15, 2023
3792643
Clean up
vmarta Mar 15, 2023
67c339f
Refactor onClick into 2 separate, clearer events
vmarta Mar 15, 2023
1643906
Update test, after props were updated recently
vmarta Mar 15, 2023
60aecdf
Merge branch 'feature/integrate-commerce-sdk-react' into hooks-accoun…
vmarta Mar 15, 2023
b5c1db5
Back to a 1 prop now, but with a promise instead
vmarta Mar 15, 2023
9fd05e0
Increase jest timeout, since waitFor timeout was increased too
vmarta Mar 15, 2023
c73b2e5
Update the test with mock responses
vmarta Mar 15, 2023
7c6e45e
Merge branch 'feature/integrate-commerce-sdk-react' into hooks-accoun…
vmarta Mar 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const ItemPrice = ({currency, align = 'right', baseDirection = 'column', ...prop

<FormattedNumber
style="currency"
currency={currency || basket.currency || activeCurrency}
currency={currency || basket?.currency || activeCurrency}
value={displayPrice}
/>
{hasDiscount && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import React, {useEffect, useState} from 'react'
import React, {useState} from 'react'
import {Stack, Heading} from '@chakra-ui/layout'
import {FormattedMessage, useIntl} from 'react-intl'
import {Box, Flex, Skeleton} from '@chakra-ui/react'
import {useProducts, useShopperCustomersMutation} from 'commerce-sdk-react-preview'

import useCustomer from '../../../commerce-api/hooks/useCustomer'
import useNavigation from '../../../hooks/use-navigation'
import useWishlist from '../../../hooks/use-wishlist'
import {useToast} from '../../../hooks/use-toast'
import {useWishList} from '../../../hooks/use-wish-list'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On develop when the shopper clicks the "Add to Wishlist" button on one of the items of a product set, the individual item is added to the wishlist, whereas now the entire set is added to the wishlist instead of the individual product.

Example product set:
https://pwa-kit.mobify-storefront.com/global/en-GB/product/fall-lookM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm strange, that might be a regression after resolving merge conflicts.. let me investigate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I've fixed this issue.


import PageActionPlaceHolder from '../../../components/page-action-placeholder'
import {HeartIcon} from '../../../components/icons'
Expand All @@ -21,69 +21,93 @@ import WishlistPrimaryAction from './partials/wishlist-primary-action'
import WishlistSecondaryButtonGroup from './partials/wishlist-secondary-button-group'

import {API_ERROR_MESSAGE} from '../../../constants'
import {useCurrentCustomer} from '../../../hooks/use-current-customer'

const numberOfSkeletonItems = 3

const AccountWishlist = () => {
const customer = useCustomer()
const navigate = useNavigation()
const {formatMessage} = useIntl()
const toast = useToast()

const [selectedItem, setSelectedItem] = useState(undefined)
const [localQuantity, setLocalQuantity] = useState({})
const [isWishlistItemLoading, setWishlistItemLoading] = useState(false)
Copy link
Collaborator

@kevinxh kevinxh Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this state? can we replace this by the isLoading state from the mutation? (i might be missing some context)

Copy link
Contributor Author

@vmarta vmarta Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. That state is still necessary in 1 case (removing an item), but for another case (updating quantity), I've updated it so it used the mutation's loading state instead.

(The one case where it's still necessary is because the mutation lives in a different file/component)

const wishlist = useWishlist()

const {data: wishListData, isLoading: isWishListLoading} = useWishList()
const productIds = wishListData?.customerProductListItems?.map((item) => item.productId)

const {data: productsData, isLoading: isProductsLoading} = useProducts(
{parameters: {ids: productIds?.join(','), allImages: true}},
{enabled: productIds?.length > 0}
)

const wishListItems = wishListData?.customerProductListItems?.map((item, i) => {
return {
...item,
product: productsData?.data?.[i]
}
})

const updateCustomerProductListItem = useShopperCustomersMutation(
'updateCustomerProductListItem'
)
const deleteCustomerProductListItem = useShopperCustomersMutation(
'deleteCustomerProductListItem'
)
const {data: customer} = useCurrentCustomer()

const handleActionClicked = (itemId) => {
setWishlistItemLoading(!!itemId)
setWishlistItemLoading(Boolean(itemId))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

setSelectedItem(itemId)
}

const handleItemQuantityChanged = async (quantity, item) => {
// This local state allows the dropdown to show the desired quantity
// while the API call to update it is happening.
setLocalQuantity({...localQuantity, [item.productId]: quantity})
setWishlistItemLoading(true)
let isValidChange = false
setSelectedItem(item.productId)

const body = {
...item,
quantity: parseInt(quantity)
}
// To meet expected schema, remove the custom `product` we added
delete body.product

const parameters = {
customerId: customer.customerId,
itemId: item.id,
listId: wishListData?.id
}

const mutation =
parseInt(quantity) > 0
? updateCustomerProductListItem.mutateAsync({body, parameters})
: deleteCustomerProductListItem.mutateAsync({parameters})
Comment on lines +91 to +92
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you do update with 0? Feels like the back end should be capable of handling the deletion logic when quantity is 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would happen is the back end would accept 0 as the minimum quantity, but it does not remove the item from the list.


try {
await wishlist.updateListItem({
...item,
quantity: parseInt(quantity)
})
} catch {
await mutation
isValidChange = true
setSelectedItem(undefined)
} catch (err) {
toast({
title: formatMessage(API_ERROR_MESSAGE),
status: 'error'
})
}
setWishlistItemLoading(false)
setSelectedItem(undefined)
setLocalQuantity({...localQuantity, [item.productId]: undefined})

// If true, the quantity picker would immediately update its number
// without waiting for the invalidated lists data to finish refetching
return isValidChange
}

useEffect(() => {
if (customer.isRegistered) {
// We want to reset the wishlist here
// because it is possible that a user
// adds an item to the wishlist on another page
// and the wishlist page may not have enough
// data to render the page.
// Reset the wishlist will make sure the
// initialization state is correct.
if (wishlist.isInitialized) {
wishlist.reset()
}

wishlist.init({detail: true})
}
}, [customer.isRegistered])
const isPageLoading = wishListItems ? isProductsLoading : isWishListLoading

return (
<Stack spacing={4} data-testid="account-wishlist-page">
<Heading as="h1" fontSize="2xl">
<FormattedMessage defaultMessage="Wishlist" id="account_wishlist.title.wishlist" />
</Heading>
{!wishlist.hasDetail && (

{isPageLoading && (
<Box data-testid="sf-wishlist-skeleton">
{new Array(numberOfSkeletonItems).fill(0).map((i, idx) => (
<Box
Expand All @@ -108,7 +132,7 @@ const AccountWishlist = () => {
</Box>
)}

{wishlist.hasDetail && wishlist.isEmpty && (
{!isPageLoading && !wishListItems && (
<PageActionPlaceHolder
data-testid="empty-wishlist"
icon={<HeartIcon boxSize={8} />}
Expand All @@ -129,18 +153,21 @@ const AccountWishlist = () => {
/>
)}

{wishlist.hasDetail &&
!wishlist.isEmpty &&
wishlist.items.map((item) => (
{!isPageLoading &&
wishListItems &&
wishListItems.map((item) => (
<ProductItem
key={item.id}
product={{
...item.product,
quantity: localQuantity[item.productId]
? localQuantity[item.productId]
: item.quantity
quantity: item.quantity
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code in develop removes the item from the wishlist when the shopper decrements the product quantity < 1, when taking either of these two actions in the QuantityPicker:

  1. Typing the number '0' in the quantity Input field.
  2. Pressing the '-' button when the quantity is 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, the smart deletion logic originally comes from:

updateListItem: async (listId, item) => {
const {id, quantity} = item
if (quantity === 0) {
await removeListItem(listId, id)
actions.removeListItem(listId, id)
return
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I've implemented that.

}}
showLoading={isWishlistItemLoading && selectedItem === item.productId}
showLoading={
(updateCustomerProductListItem.isLoading ||
deleteCustomerProductListItem.isLoading ||
isWishlistItemLoading) &&
selectedItem === item.productId
}
primaryAction={<WishlistPrimaryAction />}
onItemQuantityChange={(quantity) =>
handleItemQuantityChanged(quantity, item)
Expand Down
Loading