-
Notifications
You must be signed in to change notification settings - Fork 142
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
[Hooks] Fix the traversal of wishlist data #1045
[Hooks] Fix the traversal of wishlist data #1045
Conversation
@@ -31,7 +31,7 @@ export const useWishList = ({listId = ''} = {}) => { | |||
} | |||
) | |||
const wishLists = productLists?.data?.filter((list) => list.type === 'wish_list') || [] | |||
const currentWishlist = wishLists?.data?.find((list) => list.id === listId) || wishLists[0] | |||
const currentWishlist = wishLists.find((list) => list.id === listId) || wishLists[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do we use wishLists[0]
as a fallback, when we know that it doesn't have the correct ID?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, that's a good point.. we wanted to use the first wishlist only when the caller does not pass in the list id. Let me fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH, in the context of retail react app, you never need to pass in list id. The template always use the first wishlist that has type wish_list
. We should remove that listId
argument.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. We can either mimic the existing logic (of using a name and a type):
pwa-kit/packages/template-retail-react-app/app/hooks/use-wishlist.js
Lines 12 to 13 in 0ef7c22
const useWishlist = () => | |
useCustomerProductList(PWA_DEFAULT_WISHLIST_NAME, PWA_DEFAULT_WISHLIST_TYPE) |
Or use the "first wishlist that has type wish_list". Whichever we decide to do, let's do it in a separate PR.
A small bug fix, which is extracted from #1042.
FYI @alexvuong since you're working on wishlist-related stuffs too.
Types of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization