Skip to content

feat: implement address management components and functionality#396

Open
Abhishek-Punhani wants to merge 6 commits intoshopstr-eng:mainfrom
Abhishek-Punhani:Issue374
Open

feat: implement address management components and functionality#396
Abhishek-Punhani wants to merge 6 commits intoshopstr-eng:mainfrom
Abhishek-Punhani:Issue374

Conversation

@Abhishek-Punhani
Copy link
Copy Markdown
Contributor

Implemented address management components and functionality

Description

  • Add AddressPicker component for selecting and managing saved addresses.
  • Enhance PreferencesPage to include address management features.
  • Added Use Saved Address in cart view to use saved address during checkout.
  • Add unit tests for saved address helper functions.
  • Update local storage helper functions to manage saved addresses.
  • Define SavedAddress type in typescript types.

Resolved or fixed issue

Fixes #374

Screenshots (if applicable)

2026-04-10.23-17-11.mp4

Affirmation

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 10, 2026

@Abhishek-Punhani is attempting to deploy a commit to the shopstr-eng Team on Vercel.

A member of the Team first needs to authorize it.

@Abhishek-Punhani Abhishek-Punhani force-pushed the Issue374 branch 2 times, most recently from 0ef250f to adc4620 Compare April 11, 2026 11:44
Copy link
Copy Markdown
Contributor

@arnavkirti arnavkirti left a comment

Choose a reason for hiding this comment

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

Thanks for the solid work on this PR - Would like to point out some things:

  • Please confirm that we validate all required fields (name, address, city, state, zip, country) both on the client and at the boundary where these are sent to the backend.
  • Consider what happens if zip or country are missing or malformed (especially for non‑US addresses) and add tests to cover these edge cases.
  • isDefault is a boolean on SavedAddress, but there is no obvious guard to ensure only one address can be default at a time.
  • Can you add logic (and tests) to ensure we never end up with multiple defaults, and clarify the behavior when the current default is deleted?
  • Please double‑check that all interactive elements in the Address picker / management UI are keyboard accessible and have appropriate ARIA labels where needed.
  • If we are showing destructive actions (like delete), a confirmation step or undo pattern would be helpful to prevent accidental loss of an address.
  • it would be good to have a few focused tests on:
    • Rendering of saved addresses list.
    • Creating a new address and setting it as default.
    • Editing/deleting an existing address.
    • Ensuring only one default address after updates.

@Abhishek-Punhani
Copy link
Copy Markdown
Contributor Author

Abhishek-Punhani commented Apr 12, 2026

Thanks for the solid work on this PR - Would like to point out some things:

  • Please confirm that we validate all required fields (name, address, city, state, zip, country) both on the client and at the boundary where these are sent to the backend.

  • Consider what happens if zip or country are missing or malformed (especially for non‑US addresses) and add tests to cover these edge cases.

  • isDefault is a boolean on SavedAddress, but there is no obvious guard to ensure only one address can be default at a time.

  • Can you add logic (and tests) to ensure we never end up with multiple defaults, and clarify the behavior when the current default is deleted?

  • Please double‑check that all interactive elements in the Address picker / management UI are keyboard accessible and have appropriate ARIA labels where needed.

  • If we are showing destructive actions (like delete), a confirmation step or undo pattern would be helpful to prevent accidental loss of an address.

  • it would be good to have a few focused tests on:

    • Rendering of saved addresses list.
    • Creating a new address and setting it as default.
    • Editing/deleting an existing address.
    • Ensuring only one default address after updates.

Ensuring one default address and destructive action modal is there already, could you once pull the changes and test it before suggesting

@Abhishek-Punhani
Copy link
Copy Markdown
Contributor Author

Handling the pin codes and duplicate checks , ARIA, these I will handle in another PR

Copy link
Copy Markdown
Contributor

@GautamBytes GautamBytes left a comment

Choose a reason for hiding this comment

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

  1. Can you fix the saved-address default handling. Right now updating/saving an existing default address can leave the address book with no default at all. We should always preserve exactly one default address.

  2. Also, Please update the helper tests to use the real saved-address helpers with mocked localStorage instead of re-implementing the logic inside the test file. As written, the tests can still pass even when the actual helper behavior is wrong.

Once those are fixed, ping me again and I’ll re-review.

- Add AddressPicker component for selecting and managing saved addresses.
- Enhance PreferencesPage to include address management features.
- Add unit tests for saved address helper functions.
- Update local storage helper functions to manage saved addresses.
- Define SavedAddress type in typescript types.

Signed-off-by: Abhishek-Punhani <[email protected]>
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.

[FEATURE] Encrypted Saved Delivery Addresses Using NIP-17

3 participants