@W-18407137 Trigger a modal if new bonus products exist in AddToCart response#2541
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
| // Compare existing bonus products with new bonus discount line items | ||
| const newBonusItems = | ||
| addToCartResponse?.bonusDiscountLineItems?.filter( | ||
| (newItem) => |
There was a problem hiding this comment.
change name to bonusItem?
There was a problem hiding this comment.
You can move this to within the "isValidResponse" check, right?
packages/template-retail-react-app/app/hooks/use-bonus-product-modal.js
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/components/product-view/index.test.js
Outdated
Show resolved
Hide resolved
| @@ -55,6 +55,8 @@ jest.mock('@salesforce/retail-react-app/app/constants', () => { | |||
| } | |||
| }) | |||
|
|
|||
There was a problem hiding this comment.
Same here, do we need a bonus product test in this file?
|
May I have the design link for this? If I understand correctly, we are aiming to have 2 modal open back to back if there is a bonus item? one is bonus and the other is the existing addToCart modal? It does not look like a good UX experience for shopper. |
packages/template-retail-react-app/app/pages/product-detail/index.test.js
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/hooks/use-bonus-product-modal.js
Outdated
Show resolved
Hide resolved
| isGuest = false, | ||
| bypassAuth = true | ||
| bypassAuth = true, | ||
| basket = {bonusDiscountLineItems: []} |
There was a problem hiding this comment.
IMO, TestProviders should be agnostic of the storefront data, and be as generic as possible
packages/template-retail-react-app/app/components/product-view/index.jsx
Show resolved
Hide resolved
packages/template-retail-react-app/app/components/product-view/index.jsx
Show resolved
Hide resolved
packages/template-retail-react-app/app/hooks/use-bonus-product-modal.test.js
Outdated
Show resolved
Hide resolved
| <MockComponent product={child} addToCart={() => {}} addToWishlist={() => {}} />, | ||
| { | ||
| wrapperProps: {basket: mockBasket} | ||
| } |
There was a problem hiding this comment.
Seeing this getting reused in every test case, if possible, can we move this to beforeEach?
packages/template-retail-react-app/app/hooks/use-bonus-product-modal.test.js
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/hooks/use-derived-product.test.js
Show resolved
Hide resolved
|
| children: PropTypes.node.isRequired | ||
| } | ||
|
|
||
| export const BonusProductModal = () => { |
There was a problem hiding this comment.
I would move components to the component directory
There was a problem hiding this comment.
Let's break this down into two parts - the hook for business logic (keep it under hooks) and component for presentation (under components)
There was a problem hiding this comment.
@kzheng-sfdc Sure. This PR was already merged by the time this comment was made. @sf-vrushal-kulkarni will be making this change in his PR
|
@alexvuong can you please review this as we are dependent on this PR for other WIs? |
Screen.Recording.2025-06-10.at.12.31.06.PM.mov
Description
Modal is triggered to open when there are new bonus products in AddToCart response
AddToCart modal will be triggered when the user closes bonus modal.
Persisting existing bonus products information using context and local storage in browser
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