@W-19347548 feat: Guest users - add modal warning for potential address loss prior to switch to one address toggle action by sf-henry-semaganda · Pull Request #3280 · SalesforceCommerceCloud/pwa-kit

sf-henry-semaganda · 2025-09-10T16:37:59Z

Description

implemented a warning modal for guest users switching from multi-ship to single-address mode when they have unsaved local addresses

modal_warn.mov

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

Created SingleAddressToggleModal - Warning modal component for address loss
Implemented onUnsavedGuestAddressesToggleWarning callback - communication between shipping and multi address components

How to Test-Drive This PR

Guest with No Addresses (Should NOT show warning)

Go to checkout as guest with 2+ items in cart
Go to shipping address step
Toggle to "Ship to Multiple Addresses"
Toggle back to "Ship to Single Address"
Expected: No warning modal, toggle works normally

Guest with Saved Addresses (Should NOT show warning)

Go to checkout as guest with 2+ items in cart
Go to shipping address step
Toggle to "Ship to Multiple Addresses"
Add addresses and click "Continue to Shipping Method" (saves addresses)
Go back to shipping address step
Toggle back to "Ship to Single Address"
Expected: No warning modal, toggle works normally

Guest with Unsaved Addresses (Should show warning)

Go to checkout as guest with 2+ items in cart
Go to shipping address step
Toggle to "Ship to Multiple Addresses"
Add addresses but DON'T click "Continue to Shipping Method"
Toggle back to "Ship to Single Address"
Expected: Warning modal appears

Registered User (Should NOT show warning)

Login as registered user
Follow any of the above scenarios
Expected: No warning modal ever (only for guests)

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)

cc-prodsec · 2025-09-10T16:38:10Z

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

patricksullivansf · 2025-09-10T22:28:01Z

+
+    // Handle toggle between single and multi-shipping
+    const handleToggleShippingMode = () => {
+        if (isMultiShipping && customer?.isGuest && hasUnpersistedGuestAddresses) {


the logic around hasUnpersistedGuestAddresses here and in the child component seems overly complicated. why not just run the test here? that way it only rns id the shopper actually clicks the button, and we simplify the logic.

I agree, that was my initial implementation, there was a race condition between the hook here (useProductAddressassignemnt hook) and one in multi address, around the unsaved addresses, but I look again, and see if it is feasible

i see, the child component basket with the new addresses is in a local state var from useProductAddressAssignment so the parent basket ref isn't updated? hmm... yeah, i don't see a good solution. ok.

yes exactly, the local addresses present in one of the components i.e multi address was not available or did not reflect in the other - shipping address

I wonder if it makes more sense to just have a generic onChange handler and a generic: warning your address changes will be lost.

@patricksullivansf not sure I understand what you meant sadly but I have trimmed down the function to do just a basic comparison. thoughts?
I am against moving logic to parent (shipping address) as moving the logic to the parent would require the parent to somehow access the child's (shipping multi address) local state, which isn't possible without lifting state up like moving useProductAddressAssignment to parent

i was thinking with this small wording change "If you switch to one address, ANY shipping addresses you added for the items will be removed." then the entire effect handler driving onUnsavedGuestAddressesToggleWarning could be removed and replaced with an explicit call like `onCreateAddress(adr) => handleCreateAddress(adr), onUnsavedGuestAddressesToggleWarning?.()

patricksullivansf

I still think this solution is overly complex, but I'm not going to block it.

aditek-sf · 2025-09-11T18:20:16Z

+            closeOnOverlayClick={true}
+        >
+            <AlertDialogOverlay />
+            <AlertDialogContent maxW="448px" w="448px" h="196px" borderRadius="6px">


How would this look in a mobile layout? With specific values provided here.

thanks for calling this out, mid-size devices like ipads are good, but need to account for even the smaller form factor of smartphones

adjusted.
mobile:

ipad:

add modal warning prior to switch to one address

c4212f6

sf-henry-semaganda added 4 commits September 10, 2025 12:30

update

7ba2612

update 2

9f95127

update variable naming

0a5c68e

add test coverage

ae9a719

sf-henry-semaganda marked this pull request as ready for review September 10, 2025 21:47

sf-henry-semaganda requested a review from a team as a code owner September 10, 2025 21:47

sf-henry-semaganda added 4 commits September 10, 2025 14:55

Merge branch 'develop' into W-19347548_multi_ship_toggle_warning_modal

d5f31ea

update tests

a2b59c0

update tests

25c1fb1

update change log

f6dde85

patricksullivansf reviewed Sep 10, 2025

View reviewed changes

Comment thread packages/template-retail-react-app/CHANGELOG.md Outdated

update file import

38bed45

patricksullivansf reviewed Sep 10, 2025

View reviewed changes

Comment thread packages/template-retail-react-app/app/components/single-address-toggle-modal/index.test.jsx Outdated

patricksullivansf reviewed Sep 10, 2025

View reviewed changes

Comment thread packages/template-retail-react-app/app/pages/checkout/partials/shipping-address.jsx Outdated

sf-henry-semaganda changed the title ~~@W-19347548 feat: add modal warning for potential address loss prior to switch to one address toggle action~~ @W-19347548 feat: Guest users: add modal warning for potential address loss prior to switch to one address toggle action Sep 10, 2025

sf-henry-semaganda changed the title ~~@W-19347548 feat: Guest users: add modal warning for potential address loss prior to switch to one address toggle action~~ @W-19347548 feat: Guest users - add modal warning for potential address loss prior to switch to one address toggle action Sep 10, 2025

patricksullivansf reviewed Sep 10, 2025

View reviewed changes

sf-henry-semaganda added 4 commits September 11, 2025 09:40

update change log

73fbbc4

prefer useDisclosure as per feedback

d5d7adc

update to clean up complex logic

fd4ff03

update variable names more descriptive

8a489be

sf-henry-semaganda requested a review from patricksullivansf September 11, 2025 17:42

sf-henry-semaganda added 2 commits September 11, 2025 10:45

update with build files

d1e8318

update build files and translation files with correct approved text

5be16fd

patricksullivansf previously approved these changes Sep 11, 2025

View reviewed changes

aditek-sf reviewed Sep 11, 2025

View reviewed changes

Comment thread packages/template-retail-react-app/app/components/single-address-toggle-modal/index.jsx

aditek-sf reviewed Sep 11, 2025

View reviewed changes

update styling to accomodate mobile fits

e223c0c

sf-henry-semaganda dismissed patricksullivansf’s stale review via e223c0c September 11, 2025 18:37

sf-henry-semaganda requested review from aditek-sf, knhage and patricksullivansf September 11, 2025 18:37

aditek-sf approved these changes Sep 11, 2025

View reviewed changes

patricksullivansf approved these changes Sep 11, 2025

View reviewed changes

sf-henry-semaganda merged commit 57bb51e into develop Sep 11, 2025
42 checks passed

sf-henry-semaganda deleted the W-19347548_multi_ship_toggle_warning_modal branch September 11, 2025 18:58

sf-shikhar-prasoon mentioned this pull request Sep 12, 2025

update the feature branch with latest changes in develop branch #3300

Merged

12 tasks

Uh oh!

Conversation

sf-henry-semaganda commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of Changes

Changes

How to Test-Drive This PR

Checklists

General

Accessibility Compliance

Localization

Uh oh!

cc-prodsec commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎉 Snyk checks have passed. No issues have been found so far.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patricksullivansf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sf-henry-semaganda commented Sep 10, 2025 •

edited

Loading

cc-prodsec commented Sep 10, 2025 •

edited

Loading