feat(account-details): refresh the account details screen - #2930
Merged
Conversation
yarn build:extension:translations (i18next-scanner-webpack) currently
throws "TypeError: I18nextWebpackPlugin is not a constructor" on this
Node 22 toolchain -- pre-existing, reproducible on master, unrelated to
this PR's component change (fails at webpack config-load time, before
any source scanning). Added the three new keys ("Stellar", "This
address supports Stellar network.", "Copy wallet address") by hand to
both locale files, in the same sorted position and with the same
untranslated-placeholder convention (English text as the pt value)
that the tool itself would have produced.
…t address The plan's index.tsx used variant="primary" (lilac-09 background, white text -- confirmed via node_modules/@stellar/design-system/build/styles.min.css), but design-specs/node-9569-38987.md's color table specifies this button as background var(--sds-clr-gray-12) with text/icon var(--sds-clr-base-00) -- exactly what SDS's variant="secondary" produces. The token map cross-references this same off-white/black-text treatment against node 9593-26538's "Go to Discover" button, so it's a recurring, deliberate pattern in this design refresh, not a one-off. Confirmed the mismatch by actually rendering the screen (Playwright screenshot against the dev build) before and after.
Contributor
|
PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-c80dbc3ebaf5a9ecea3b (SDF collaborators only — install instructions in the release description) |
CopyText's doneLabel defaults to "Copied", so keeping it would have shown a balloon and a toast together. Copy the address directly via navigator.clipboard.writeText (same pattern as SwapTokenMenu) and surface the result with toast.custom + an SDS Notification, matching the existing toast.custom convention used elsewhere (Discover, useSwapQuoteExpiry) rather than toast.success, which the app's Toaster never styles. The accountPublicKeyCopied metric now fires only after a successful write instead of unconditionally on click, and a copy failure shows an error toast instead of a false "copied" message.
…Figma - Shrink the Stellar network chip icon from 16px to 14px - Bump the chip text weight to semi-bold (600) with correct 14/20 line-height and 10px horizontal padding, per Figma node 9569-39341 - Drop the "Copy wallet address" button weight one step to medium (500) by targeting the SDS-rendered .Button class from the footer wrapper, avoiding the className-merge trap - Give the identicon the same background token as the chip - Remove the dead CopyText selector left over after CopyText was removed from this screen
… medium - Grow the ViewPublicKey identicon container from 24px to the Figma-specced 32px circle, and constrain the descendant IdenticonImg <img> to 16x16 so a visible ring of background shows around it, instead of the identicon filling the circle edge to edge via overflow: hidden. - Investigated a 550 font-weight as a middle ground between medium (500, "too thin") and semi-bold (600, "too bold") for the Copy wallet address button label. Measured in the real built extension: 550's rendered label width is identical to 600's (both differ from 500's), because Inter (SDS's configured base font) is never loaded in this repo, so text falls back to the system sans-serif, which only exposes normal/bold faces and snaps 550 to bold. 550 renders as theater, not a real midpoint, so kept var(--sds-fw-medium) with a comment documenting the finding.
Drops the font-weight override entirely rather than restating 600, since
that is already SDS Button's default. A 500 and a raw 550 were both tried
first; 550 measured identical to 600 because SDS's --sds-ff-base ("Inter")
is never loaded in this repo, so text falls back to the system sans-serif
which has no face between medium and bold.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both the success and error toasts now share one stable sonner id, so repeated taps of "Copy wallet address" update the visible toast rather than stacking, and a failure after a success shows only the latest outcome. Mirrors the swap quote-expiry toast. Verified in a built extension: 5 rapid taps leave exactly 1 toast in the DOM, with the correct address text, still auto-dismissing after ~4.3s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Refreshes the account-details screen to match the Home redesign.
Changes:
- Replaces renaming/explorer actions with updated account and QR presentation.
- Adds clipboard success/error toasts and success-only analytics.
- Adds corresponding styling and locale keys.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
ViewPublicKey/index.tsx |
Implements refreshed screen and copy feedback. |
ViewPublicKey/styles.scss |
Adds redesigned account, QR, chip, and footer styling. |
locales/en/translation.json |
Adds English strings. |
locales/pt/translation.json |
Adds Portuguese locale entries. |
Suppressed comments (3)
extension/src/popup/locales/pt/translation.json:164
- This new Portuguese entry is still English, so the primary action remains untranslated for Portuguese users. The locale rule requires an actual Portuguese value for every string (
.claude/skills/freighter-best-practices/references/i18n.md:73-76).
"Copy wallet address": "Copy wallet address",
extension/src/popup/locales/pt/translation.json:656
- This new Portuguese entry is still English, so the account-details caption remains untranslated. The locale rule requires an actual Portuguese value for every string (
.claude/skills/freighter-best-practices/references/i18n.md:73-76).
"This address supports Stellar network.": "This address supports Stellar network.",
extension/src/popup/locales/pt/translation.json:167
- This new Portuguese entry is still English, so clipboard failures display an untranslated error. The locale rule requires an actual Portuguese value for every string (
.claude/skills/freighter-best-practices/references/i18n.md:73-76).
"Couldn’t copy your wallet address": "Couldn’t copy your wallet address",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…le one Reusing one stable sonner id made a stale-update path reachable: create() takes an "update existing" branch when the id is already in this.toasts, while dismiss() only adds the id to dismissedToasts and never removes the entry. After manually swiping a toast away, tapping Copy again could publish an update to a toast the Toaster had already unmounted, showing nothing at all. Keep the last toast's id in a ref, dismiss it explicitly, then create a new toast with a fresh auto-generated id. Replace-not-stack behaviour is preserved without ever reusing an id. Verified in a built extension: 5 rapid taps leave exactly 1 toast; swipe-dismiss then tap again always shows a toast; auto-dismiss and swipe-to-dismiss both still work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A visible toast made the header close button unclickable: elementFromPoint at the X's centre returned the sonner container, not the button. sonner's container is position:fixed with no pointer-events rule of its own, and at the 360px popup width its @media (max-width:600px) branch stretches it to width:100% anchored under the header. Two changes: - offset/mobileOffset top of 64px, clearing the Home icon row (~56px) and the account-details header control (~52px). Both props are set because the mobile media query reads --mobile-offset-*, so `offset` alone has no effect at popup width; `offset` still matters for fullscreen mode. - pointer-events: none on the container, auto on the toast, so clicks in the container's empty regions pass through while the toast stays swipeable. Affects every toast in the app (Soroban outage, Discover errors, swap quote expiry), all of which blocked header controls the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CassioMG
force-pushed
the
feature/home-refresh-account-details
branch
from
August 6, 2026 17:48
364ff7c to
67533ac
Compare
Keeps only what isn't inferable from the code: why the offset is set on both sonner props, that sonner ships no pointer-events rule on its container, and the warning against switching the copy toast back to a stable id. Drops the font-weight comment entirely - it explained an absent override, and the rejected alternatives belong in the PR rather than the stylesheet. Comments only: 45 changed lines, all of them comments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Three new Portuguese strings shipped as untranslated EnglishNit — not a merge blocker. Cosmetic for pt users, trivial to fix, but it's a documented step rather than a style preference. TL;DR: Four of the new user-facing strings on this screen need Portuguese translations and three of them are still sitting as English text in the Portuguese file. A pt user opening the account details screen will see "Copy wallet address" and "This address supports Stellar network." in English. You already hand-translated the "Address copied!" toast in this same PR, so this reads as three that got missed rather than a deliberate call. Steps to reproduce:
Detailed explanation (for agents)Root cause: freighter/extension/src/popup/locales/pt/translation.json Lines 163 to 168 in eb0fcbb freighter/extension/src/popup/locales/pt/translation.json Lines 655 to 657 in eb0fcbb This PR adds five keys.
Why this isn't just the usual placeholder debt: freighter/docs/skills/freighter-best-practices/references/i18n.md Lines 73 to 76 in a56400c That's both the hard rule ("Every string must have both English ( Prior art cuts slightly the other way: 43 of 847 pt keys (5.1%) on Suggested fix: replace the three values in "Copy wallet address": "Copiar endereço da carteira",
"Couldn’t copy your wallet address": "Não foi possível copiar o endereço da sua carteira",
"This address supports Stellar network.": "Este endereço é compatível com a rede Stellar.",Note the existing file already uses |
piyalbasu
approved these changes
Aug 7, 2026
They shipped as English placeholders, which the generator writes when a key is missing. Wording follows the file's existing conventions: "Copiar endereço" for the copy verb, "Não foi possível ..." matching the sibling "Couldn't clear recent dApps" / "Couldn't open this dApp" errors, and "compatível com" as already used elsewhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Fixed in a44234a — all three now translated, using your suggestions verbatim after checking them against the file's existing conventions: "Copy wallet address": "Copiar endereço da carteira",
"Couldn’t copy your wallet address": "Não foi possível copiar o endereço da sua carteira",
"This address supports Stellar network.": "Este endereço é compatível com a rede Stellar.", |
CassioMG
added a commit
that referenced
this pull request
Aug 11, 2026
Addresses Copilot review on #2933. The explorer action was gated on `!isCustomNetwork`, which only excludes STANDALONE. Experimental mode switches the active network to Futurenet (saveExperimentalFeatures), and stellar.expert has no Futurenet explorer, so the action rendered and built an unsupported /explorer/futurenet/ URL. This is long-standing logic from #616 rather than a regression here, but #2930 removed the QR screen's copy, leaving this the only caller — so it gets fixed here. Note that switching to the shared `getStellarExpertUrl` helper alone does not fix it: the helper falls through to /public for anything that isn't testnet, so an ungated Futurenet link would render a *mainnet* lookup for a Futurenet account — wrong data, silently. The gate is what matters, so this adds `isStellarExpertSupported` beside the helper to keep the two together, with unit tests pinning the Futurenet and custom-network exclusions and documenting that fallback. WalletRow gains `role="button"` and `aria-current`, matching BalanceRow, the sibling list row. The active account was previously conveyed only by the badge on its avatar, which says nothing to a screen reader. Left alone: full keyboard focusability. No row component in this codebase (BalanceRow, ProtocolRow) sets tabIndex or key handlers, so making this one a semantic button would be inconsistent without addressing that codebase-wide. Also adds e2e coverage for the copy action, asserting the clipboard contents and the toast. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CassioMG
added a commit
that referenced
this pull request
Aug 13, 2026
* feat(wallets): refresh the wallets list with an account header Restyle the Wallets screen per node 9573-23172: an active-account header block (identicon, name, address, and four action buttons for QR, copy, stellar.expert, and rename) replaces the per-row ellipsis dropdown. Each WalletRow becomes a three-column row (identicon / name+address / balance) with "Imported" shown inline beside the address. - Copy and stellar.expert buttons add call sites for the existing account.public_key_copied / account.stellar_expert_opened metrics, with no arguments (a `source` property is deferred to a follow-up PR). - The explorer button is hidden via isCustomNetwork(networkDetails), mirroring ViewPublicKey. - Tapping a row still makes that account active and navigates to Home (spec D4) - rename/copy/QR/explorer now act on the active account only. - Removes dead `.detail-indentifier`, `.WalletRow__options-actions`, and `.Wallets__fail` CSS/attribute orphans. - Hand-added new i18n keys to en/pt since build:extension:translations is broken on master (pre-existing, out of scope here). * test(wallets): rename via the account header instead of the row menu The per-row ellipsis dropdown (and its wallet-row-options testid) is gone; rename now acts on the active account through the header pencil button. Repoints "Renames wallets" at wallets-header-edit-name, and scopes its final assertions to the header/row locators specifically since the new name now legitimately renders in two places (and this seed phrase has several accounts sharing the row list). Also updates "Loads wallets data and token prices on Mainnet in batches": address and balance are no longer a single concatenated string, so each per-account assertion now checks a row's balance cell scoped by its address rather than matching literal "address - $balance" text. * fix(wallets): stop the add-wallet button's className from erasing its own styles SDS Button assigns its own className internally and then spreads the rest of its props over the element, so any className passed in by a caller replaces (rather than merges with) Button/Button--tertiary/etc. instead of augmenting them. That silently dropped every rule meant for the restyled add-wallet button, leaving it transparent, gray, and sized off its unconstrained icon/text content. Wrap the Button in a plain div instead and scope the token overrides through that wrapper, targeting `.Button` and `.Button__icon` by descendant selector so they win on specificity without touching the className prop at all. Confirmed via computed-style checks that the button now resolves to the intended lilac-02/09/11 tokens at a 34px pill height, matching the design spec. * fix(wallets): align the wallets screen with the Figma spec Corrects eight deviations from Figma frame 9573-23172: - Header name and address are now one tight block; the 16px gap applies only between the identicon, that block, and the action row. - QR icon qr-code-01 -> qr-code-02, pencil icon edit-05 -> edit-01. - The divider sits inside the 24px content gutter instead of bleeding to the window edges, and is held 16px off the list below it. - Row avatars lose their border and white fill for a flat gray circle, with the identicon inset to 16px inside the 40px avatar. - The active row's selected state drops its colored border in favor of a 16px navy badge flush to the avatar's bottom-right corner. - "Imported" is separated from the address by a bullet. - The add-wallet control keeps its tint on the icon chip alone rather than behind the whole button, at medium weight. The button's copy becomes "Add wallet", which already exists as a key (it is the AddWallet sheet's own title), so this adds no new strings and retires the now-orphaned "Add a wallet". That collision is also why the e2e click moves to the add-wallet testid: the label alone would be ambiguous once the sheet it opens is on screen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(wallets): match the QR screen's copy toast, drop the row hover - The copy button now shows the same success/error toast the QR screen shows, replacing CopyText's inline "Copied!" balloon. The two surfaces do the same thing, so they should confirm it the same way. The metric moves into the handler and fires only after the clipboard write resolves, so failed copies still aren't counted. - Restores SDS's `xl` padding on the add-wallet button. Its 16px/24px text already matches the spec, so only the colors, gap and weight stay overridden; the tint remains on the icon chip alone. - Removes the wallet row's hover background. "Copied!" stays in the catalog — AccountHeader still uses it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(wallets): repair the medium font weight, tighten add-wallet spacing `--font-weight-medium` is defined nowhere — not in this repo and not in SDS, which names it `--sds-fw-medium`. An undefined custom property makes the whole declaration invalid, and because font-weight inherits, every element referencing it silently rendered at the body's 400. Measured in a browser before the fix, the add-wallet label, the header name, the row names and the balances were all 400; the spec calls for 500 on each. Swapping the six occurrences in these two files for `--sds-fw-medium` and `--sds-fw-regular` puts them at 500/400 as the spec's Text/MD/500, Text/SM/500 and Text/SM/400 styles require. This also means the earlier "less bold" pass on the add-wallet label was not setting medium at all, only cancelling SDS's semi-bold down to regular by accident. The same undefined token is referenced from BackButton, TransactionHeading, Notification and AmountCard among others, all with the same silent fallback. Those are left alone here. Also drops the add-wallet button's left padding so its icon chip lines up with the list's avatars in the 24px gutter, and gives the content footer an even 8px above and below, scoped with :has() so no other view's footer changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(wallets): gate stellar.expert to supported networks, label the row Addresses Copilot review on #2933. The explorer action was gated on `!isCustomNetwork`, which only excludes STANDALONE. Experimental mode switches the active network to Futurenet (saveExperimentalFeatures), and stellar.expert has no Futurenet explorer, so the action rendered and built an unsupported /explorer/futurenet/ URL. This is long-standing logic from #616 rather than a regression here, but #2930 removed the QR screen's copy, leaving this the only caller — so it gets fixed here. Note that switching to the shared `getStellarExpertUrl` helper alone does not fix it: the helper falls through to /public for anything that isn't testnet, so an ungated Futurenet link would render a *mainnet* lookup for a Futurenet account — wrong data, silently. The gate is what matters, so this adds `isStellarExpertSupported` beside the helper to keep the two together, with unit tests pinning the Futurenet and custom-network exclusions and documenting that fallback. WalletRow gains `role="button"` and `aria-current`, matching BalanceRow, the sibling list row. The active account was previously conveyed only by the badge on its avatar, which says nothing to a screen reader. Left alone: full keyboard focusability. No row component in this codebase (BalanceRow, ProtocolRow) sets tabIndex or key handlers, so making this one a semantic button would be inconsistent without addressing that codebase-wide. Also adds e2e coverage for the copy action, asserting the clipboard contents and the toast. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(identicon): drop the now-unused getColorPubKey WalletRow's redesign removed the last caller — it used this to tint the selected row's border, which the new design replaces with a fixed navy badge. The export then had no references anywhere in the repo. `no-unused-vars` cannot see it because the symbol is exported, so it would have stayed green in CI indefinitely rather than surfacing later. Only the getColorPubKey block goes. `publicKeyToBytes` and `HSVtoRGB` stay: drawIdenticon and setFillStyle still call them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the Home refresh project.
What changed
The account details / QR screen, per Figma frames
9569-38987and9569-39341:isEditingNamestate are removed. Renaming now lives on the Wallets screen.account-details-720p.mov
Toast top-offset change
This PR also includes a global Toast top-offset change so that it doesn't cover the navigation bar. This matches the Figma designs and also prevent the Toast frame from blocking interaction (clicks) on the nav bar buttons.
Currently only the 4 places below are using the Toast component, posting screenshots here for verification.
Analytics — one deliberate removal
This is the only intentional metric removal in the whole Home refresh:
account.renamedwithsource: "view_public_key"no longer fires, because renaming is no longer possible here. The event survives viasource: "wallets"from the rename modal, so only that one source value disappears. Any dashboard segmenting renames by source will see it flatline.account.public_key_copiedis kept, still with no arguments — deliberately nosourceproperty yet, so all such properties land together in one follow-up analytics PR. It now fires after a successful copy rather than on click. Same event, same (absent) properties; it just stops counting failed copies as successes.account.stellar_expert_openedloses its only call site here. TheMETRIC_NAMESentry stays; feat(wallets): refresh the wallets list with an account header #2933 adds a new call site. Between those merges the event is briefly unreferenced — expected, not a bug.🤖 Generated with Claude Code