Skip to content

feat(wallets): refresh the rename wallet modal - #2931

Merged
CassioMG merged 15 commits into
masterfrom
feature/home-refresh-rename-modal
Aug 14, 2026
Merged

CassioMG merged 15 commits into
masterfrom
feature/home-refresh-rename-modal

Conversation

@CassioMG

@CassioMG CassioMG commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

Part of the Home refresh project.

What changed

The rename-wallet modal, per Figma frame 9585-24423:

  • The card is now centred instead of pinned at top: 30%.
  • Circular X close button top-right inside the card.
  • Centred identicon above the input.
  • "Save" → "Set name", and Cancel / Set name sit side by side at equal width.
  • Removed: the "Rename Wallet" heading and the Address: G… line.

The form itself is untouched — same Formik setup, same 24-character Yup limit, same dispatch.

Screenshot 2026-08-13 at 11 13 40

Verification

  • yarn test:ci — 196/202 suites, 1520/1571 tests, identical to baseline
  • yarn build:extension — clean
  • yarn test:e2e loadAccount.test.ts — 9 passed / 2 skipped, matching baseline
  • Clicked through the modal in a real browser build (that's how bug 1 surfaced)

🤖 Generated with Claude Code

Restyle the rename-wallet modal to match frame 9585-24423: a centered
card with a circular close button, a centered identicon, the name
input, and Cancel / Set name side by side. "Save" is renamed to "Set
name", and the "Rename Wallet" heading and Address line are removed.

Update the e2e test's button-text assertion to match the new label.
@github-actions

github-actions Bot commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-1443d9b577130cc37f35 (SDF collaborators only — install instructions in the release description)

CassioMG and others added 6 commits August 7, 2026 11:07
The rename modal was wrapped in View.Content, whose
View__inset--scroll-shadows class paints an opaque full-width
background. That band covered the card's horizontal margins and its
rounded corners, so the modal read as a flat edge-to-edge panel.

- Drop the View.Content wrapper so .RenameWallet is the card itself
  (312px wide, centered, 32px radius).
- Move the close button out of absolute positioning into a flow header
  row, so it respects the card's 24px padding and stacks above the
  avatar as designed.
- Size the action buttons to spec via SDS custom properties: 36px tall,
  8/12 padding, 14/20 type. SDS size="md" alone gives 32px and 6/10.
- Set the button label weight with --sds-fw-semi-bold.

Verified against Figma by measuring the rendered DOM in the extension:
card 312x248 with 24px padding, header 82 tall, 24px gaps between
avatar/input/actions, buttons 36 tall with an 8px gap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@CassioMG CassioMG self-assigned this Aug 13, 2026
@CassioMG
CassioMG marked this pull request as ready for review August 13, 2026 00:11
Copilot AI balanced review requested due to automatic review settings August 13, 2026 00:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refreshes the rename-wallet modal to match the Home refresh design.

Changes:

  • Centers and restyles the modal with a close button and identicon.
  • Replaces “Save” with “Set name” and updates button layout.
  • Updates translations and the rename-wallet E2E flow.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
extension/src/popup/views/Wallets/styles.scss Centers the modal wrapper.
extension/src/popup/components/account/RenameWallet/index.tsx Restructures modal content and controls.
extension/src/popup/components/account/RenameWallet/styles.scss Implements the refreshed card styling.
extension/src/popup/locales/en/translation.json Adds the English action label.
extension/src/popup/locales/pt/translation.json Adds the Portuguese locale entry.
extension/e2e-tests/loadAccount.test.ts Updates the submit-button locator.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread extension/src/popup/locales/pt/translation.json Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@CassioMG

Copy link
Copy Markdown
Contributor Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@CassioMG
CassioMG requested a review from piyalbasu August 13, 2026 01:30
CassioMG and others added 5 commits August 12, 2026 19:08
Replace the local Button custom-property overrides with the props SDS
already provides. size="lg" supplies the 8/12 padding and 14px type the
spec calls for, and isFullWidth splits the row evenly, which is what the
`flex: 1` rule was there to do.

Also drop the redundant font-weight declaration: SDS .Button already
applies --sds-fw-semi-bold, so the local rule had no effect.

The one remaining difference from Figma is button height, 40px from SDS
lg versus 36px in the design. Taking SDS's scale is preferable to
overriding it for 4px.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…h-rename-modal

# Conflicts:
#	extension/e2e-tests/loadAccount.test.ts
@piyalbasu

Copy link
Copy Markdown
Contributor

You didn't introduce this (this is an existing bug), but I noticed that you can enter an empty string as a new account name. This would be nice to fix while we're here, but not a blocker

@piyalbasu piyalbasu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Other than my 1 note, this looks good to me!

Three defects in the rename modal:

- A blank or whitespace-only name saved fine, leaving the wallet with no
  visible label. The submit button is now disabled unless the trimmed
  name is non-empty.
- Surrounding whitespace was stored verbatim, so "  Wallet  " persisted
  padded. The name is trimmed before saving.
- Submitting an unchanged name did nothing at all: the equality guard
  wrapped onClose() along with the dispatch, so the modal stayed open and
  the button read as broken. The guard now covers only the dispatch,
  metric, and refetch, and onClose() always runs. An unchanged submit
  dismisses the modal without touching storage or recording a rename.

Mirrors the mobile app's RenameAccountModal, which already trims, bounds
the name by ACCOUNT_NAME_MIN_LENGTH/MAX_LENGTH, and disables the button
rather than surfacing an error message.

Covered by two e2e tests, one per reported defect.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@CassioMG

Copy link
Copy Markdown
Contributor Author

You didn't introduce this (this is an existing bug), but I noticed that you can enter an empty string as a new account name. This would be nice to fix while we're here, but not a blocker

@piyalbasu nice catch on that! I've just fixed it on 0120815 - now it won't allow blank spaces and you can also use the "Set name" button even if you haven't changed the name as the UX felt broken for this particular case too. This is mirroring mobile behavior now.

rename-fixes.mov

@CassioMG
CassioMG merged commit c857645 into master Aug 14, 2026
11 checks passed
@CassioMG
CassioMG deleted the feature/home-refresh-rename-modal branch August 14, 2026 18:05
@minkyeongshin

Copy link
Copy Markdown

A few design comments:

Profile bottom sheet

  • We need to adjust the Add wallet icon size to 40px to match the wallet account icons.
  • Let's reduce the spacing between list items from 24px to 16px.
  • When there are many profiles are added, we need to limit the modal height and make the list scrollable within the bottom sheet. Attached is a screenshot for reference. (Design Link)
Screenshot 2026-08-17 at 10 30 31 AM

Empty state

  • Let's make the CTA behavior consistent across the Tokens and Collectibles empty states.
  • When both Tokens and Collectibles are empty, we can show the Add tokens/Add collectibles Secondary(White) CTA within each empty state instead of using the floating action button. (Collectibles Empty)
Screenshot 2026-08-17 at 10 32 09 AM
  • Once the user has at least one token or collectible, switch to the floating action button for both tabs like what we have now.
Screenshot 2026-08-17 at 10 33 36 AM
  • On extension, the bottom content currently appears slightly cut off. We can address this as part of the extension layout update to match with the mobile experience.

cc @CassioMG @piyalbasu
Thanks!

@CassioMG

Copy link
Copy Markdown
Contributor Author

@minkyeongshin thanks for the review, I'll address those UI updates.

On this specific note:

- When there are many profiles are added, we need to limit the modal height and make the list scrollable within the bottom sheet. Attached is a screenshot for reference. (Design Link)

The below is how the current implementation is behaving. It snaps the bottom sheet at ~80% of screen height and the whole content scrolls. I can make it snap higher like 90% of screen so we have more room for the list. But in regards to the scrolling, are you suggesting we scroll only the wallets list and make the whole header sticky?

ScreenRecording_08-17-2026.12-01-56_1.MP4

@CassioMG

Copy link
Copy Markdown
Contributor Author

@minkyeongshin for consistency, I'm thinking when we need to display the floating button over the empty state on the Tokens tab it should actually read "Add XLM" instead of "Add token" and route to the same "Add XLM" action we use today. How does that sound?

@minkyeongshin

Copy link
Copy Markdown

@minkyeongshin for consistency, I'm thinking when we need to display the floating button over the empty state on the Tokens tab it should actually read "Add XLM" instead of "Add token" and route to the same "Add XLM" action we use today. How does that sound?

yes that make sense to me! @CassioMG

@minkyeongshin

Copy link
Copy Markdown

@minkyeongshin thanks for the review, I'll address those UI updates.

On this specific note:

- When there are many profiles are added, we need to limit the modal height and make the list scrollable within the bottom sheet. Attached is a screenshot for reference. (Design Link)

The below is how the current implementation is behaving. It snaps the bottom sheet at ~80% of screen height and the whole content scrolls. I can make it snap higher like 90% of screen so we have more room for the list. But in regards to the scrolling, are you suggesting we scroll only the wallets list and make the whole header sticky?

ScreenRecording_08-17-2026.12-01-56_1.MP4

Oh I see it's already implemented. Looks good! Could we just adjust the size of the icon on the left of the 'Add wallet' button at the bottom? looks a little small. Thanks!

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.

4 participants