Skip to content

@W-18669904- PDP changes for pickup in store feature#2537

Merged
snilakandan13 merged 23 commits intofeature/shop-in-storefrom
t/commerce/W-18669904/pdpChangesForPickupInStore
Jun 11, 2025
Merged

@W-18669904- PDP changes for pickup in store feature#2537
snilakandan13 merged 23 commits intofeature/shop-in-storefrom
t/commerce/W-18669904/pdpChangesForPickupInStore

Conversation

@snilakandan13
Copy link
Contributor

@snilakandan13 snilakandan13 commented Jun 9, 2025

@W-18669904

Screenshot 2025-06-09 at 11 31 17 AM Screenshot 2025-06-09 at 11 31 44 AM

Description

Screenshot 2025-06-09 at 3 34 43 PM

  • Added support in PDP page for doing a Pickup In store
  • If the localStorage has inventoryId set, the PDP page will do a fetch based on inventoryId
  • UI will show whether the Product is In stock or Out of stock at the selected store
  • If In stock, then the Pickup in Store radio button will be enabled
    -If the Pickup in Store radio button is selected, the AddtoCart API call will have the inventoryId set for the particular store and the ProductItem

Types of Changes

  • Bug fix (non-breaking change that fixes an issue)
  • [x ] New feature (non-breaking change that adds functionality)
  • Documentation update
  • [ X] 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

  • [ X] Changes are covered by test cases
  • [X ] 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...

Localization

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

@snilakandan13 snilakandan13 requested a review from a team as a code owner June 9, 2025 17:05
@cc-prodsec
Copy link
Collaborator

cc-prodsec commented Jun 9, 2025

🎉 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)

snilakandan13 and others added 3 commits June 10, 2025 10:01
…angesForPickupInStore

Signed-off-by: snilakandan13 <119348013+snilakandan13@users.noreply.github.com>
Copy link
Contributor

@patricksullivansf patricksullivansf left a comment

Choose a reason for hiding this comment

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

Great discussion. Approving for merge to feature branch with understanding discussion items will be followed up with additional WI based on TLR feedback.

@snilakandan13
Copy link
Contributor Author

The follow up PR will be W-18751492 for PDP page changes

@alexvuong
Copy link
Contributor

How do I test drive this?

- Improved the layout of product tiles in product scroll and product list [#2446](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2446)
- Updated 6 new languagues [#2495](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2495)
- Show Bonus Product Label on OrderSummary component [#2524](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2524)
- Added support for Shop in Store Functionality
Copy link
Contributor

Choose a reason for hiding this comment

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

no PR link?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is just the group heading, the PR link is in line #7, the shopInStore is going to have multiple Prs, the first one is the one to the PDP page

useTheme,
Stack,
Radio,
RadioGroup
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep the hook at the bottom, and arrange the components in alphabetical order for readability

const [pickupError, setPickupError] = useState('')
const {site} = useMultiSite()

// Get store info from localStorage for stock status display (move to main render scope)
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a lot of logic is added into this component. Can we create a hook or components for the entire "delivery" section ?

Copy link
Contributor Author

@snilakandan13 snilakandan13 Jun 11, 2025

Choose a reason for hiding this comment

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

This whole section is going to be modified based on the changes to the hook for "useDerivedProduct" which Yuming is going on currently, this is more of a placeHolder until then

With that the "useDerivedProduct" will return the storeStockStatus

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again all this goes to the feature branch, with the caveat that future WIs/stories incrementally are present to encapsulate code better

} catch (e) {
return null
}
})
Copy link
Contributor

@alexvuong alexvuong Jun 10, 2025

Choose a reason for hiding this comment

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

This is not a good React practise to listen to a value change.

If you want to create listener for any value change, you should useEffect and put your logic there.
Using useState is not a way to listen for a change. useState is a way to trigger a re-rendering by changing the state.


useEffect(() => {
// update logic here 
const val = JSON.parse(window.localStorage.getItem(storeInfoKey))?.inventoryId
setSelectedInventoryId(val)
}, [site.?id])

Copy link
Contributor Author

@snilakandan13 snilakandan13 Jun 11, 2025

Choose a reason for hiding this comment

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

I have used useState to set the initial value, for listening to the storechanges I have used "useEffect" in line 85 below

@snilakandan13 snilakandan13 requested a review from alexvuong June 11, 2025 02:12
@snilakandan13
Copy link
Contributor Author

How do I test drive this?

Testing the PDP page after using the store locator , and then verifying using the radio buttons, I have added the screenshots

@snilakandan13 snilakandan13 merged commit 9ce5366 into feature/shop-in-store Jun 11, 2025
35 checks passed
Comment on lines -369 to -371
// The add item endpoint in the shopper baskets API does not respect variant selections
// for bundle children, so we have to make a follow up call to update the basket
// with the chosen variant selections
Copy link
Contributor

Choose a reason for hiding this comment

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

QQ: i missed this discussion. why are all these unrelated comments removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add back this comment since this comment explains why we are doing the following code

Copy link
Contributor

Choose a reason for hiding this comment

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

Will do, I'm addressing these with my next PR


// since the returned data includes all products in basket
// here we compare list of productIds in bundleProductItems of each productItem to filter out the
// current bundle that was last added into cart
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with this comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants