feat(frontend): add multi-currency display toggle beyond UsdAmount - #1151
feat(frontend): add multi-currency display toggle beyond UsdAmount#1151Spagero763 wants to merge 1 commit into
Conversation
Extends UsdAmount into a generic CurrencyAmount that renders in the currency the reader picked, with a selector in the header and the choice persisted across reloads. The selected currency lives in a CurrencyProvider rather than in each component, so one change updates every amount on the page at once. It is persisted through the existing useLocalStorage hook, which also keeps other open tabs in step. Rates come from a new currency.ts: one USD-based lookup, cached in memory and in localStorage for twelve hours, with concurrent callers sharing a single request. A page showing twenty amounts makes one call, and a reload makes none. Amounts resolve to USD once and are converted for display, so switching currency never re-hits the token price feed. Every step degrades to USD rather than failing. A failed lookup, a rate the response omitted, or a currency the runtime cannot format all fall back to the USD display the component had before, and the selector hides itself when no rates are available rather than offering choices that would do nothing. utils.ts gains xlmToUsdValue, returning the USD value as a number. xlmToUsd formats for display and so cannot be converted onward; it now delegates to the numeric helper and is otherwise unchanged. UsdAmount stays as a deprecated wrapper so existing imports keep working. BountyCard had its own copy of the conversion logic inline, which is now the shared component. That copy correctly rendered nothing for tokens with no price feed, unlike UsdAmount, which valued any non-USDC token at the XLM rate; CurrencyAmount takes the correct behaviour so unifying the two does not regress the card.
|
@Spagero763 is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@Spagero763 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
There was a problem hiding this comment.
🟡 Changes recommended
There are user-visible fallback inconsistencies (e.g., selector offering non-convertible currencies and XLM price lookup failures rendering nothing instead of the prior “USD unavailable” marker).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces app-wide display-currency support by adding a CurrencyProvider + header selector and replacing UsdAmount usage with a new CurrencyAmount component that can render amounts in the reader’s selected currency (persisted via localStorage) while caching USD exchange rates client-side.
Changes:
- Add
currency.ts,CurrencyContext,CurrencySelector, andCurrencyAmountto support a persisted, global display-currency choice with cached USD-based rates. - Deprecate
UsdAmountinto a thin wrapper aroundCurrencyAmount, and update existing pages/components to useCurrencyAmountwhere appropriate. - Add comprehensive tests for switching, persistence, caching, and failure fallbacks; add
xlmToUsdValuefor numeric USD computation.
File summaries
| File | Description |
|---|---|
| frontend/src/utils.ts | Adds numeric xlmToUsdValue helper and updates xlmToUsd to delegate to it. |
| frontend/src/UsdAmount.tsx | Deprecates UsdAmount into a wrapper around CurrencyAmount. |
| frontend/src/UsdAmount.test.tsx | Updates mocking and async handling to reflect numeric conversion path. |
| frontend/src/RecommendedBounties.tsx | Switches bounty fiat display from UsdAmount to CurrencyAmount. |
| frontend/src/main.tsx | Wraps the app in CurrencyProvider to make currency selection global. |
| frontend/src/index.css | Adds .currency-selector styling for the header select control. |
| frontend/src/CurrencySelector.tsx | New header selector component driven by CurrencyContext. |
| frontend/src/CurrencyContext.tsx | New provider/hook managing selected currency, persistence, and rate loading. |
| frontend/src/CurrencyAmount.tsx | New component that resolves token→USD once, then converts USD→display currency. |
| frontend/src/CurrencyAmount.test.tsx | Adds tests for selector switching, persistence, caching, and failure fallbacks. |
| frontend/src/currency.ts | New rate fetching/caching + locale currency detection + formatting/conversion helpers. |
| frontend/src/BountyDetailPage.tsx | Replaces UsdAmount usage with CurrencyAmount for displayed bounties. |
| frontend/src/BountyCard.tsx | Removes inline conversion logic and uses shared CurrencyAmount instead. |
| frontend/src/App.tsx | Adds CurrencySelector to the header actions area. |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| .catch(() => { | ||
| if (active) { | ||
| setUsdValue(null); | ||
| setIsLoading(false); | ||
| } | ||
| }); | ||
|
|
||
| return () => { | ||
| active = false; | ||
| }; | ||
| }, [amount, tokenSymbol]); | ||
|
|
||
| // Wait for the rates before painting a converted figure, so a non-USD reader | ||
| // never sees the amount jump from dollars to their currency after a beat. | ||
| if (isLoading || (currency !== USD && !ratesResolved)) { | ||
| return <span className="usd-amount">{bare ? 'Loading...' : '(Loading...)'}</span>; | ||
| } | ||
|
|
||
| if (usdValue === null) return null; | ||
|
|
||
| // A missing or unusable rate falls back to USD rather than hiding the amount. | ||
| const converted = convertFromUsd(usdValue, currency, rates); | ||
| const displayCurrency = converted === null ? USD : currency; | ||
| const displayValue = converted === null ? usdValue : converted; |
| const options = useMemo(() => currencyOptions(detectLocalCurrency()), []); | ||
|
|
||
| // A stored currency that is no longer offered (a different device, or a | ||
| // locale change) would otherwise leave the selector showing nothing. | ||
| const currency = options.includes(stored) ? stored : USD; |
|
CI is showing 13 failures on this PR. I went through each one, and none of them are reached by this change. Posting the detail so it is not a guess. This PR touches 14 files, all under
The three that look like committed-file corruption rather than ordinary breakage:
pub struct ContractUnpaused {
pub admin: Address,
}
#[contracttype]
}
How I verified this change itselfSince the CI steps cannot run on a clean checkout, I ran them locally, pinning
Identical failure sets, no newly failing file, passed tests up by exactly the 12 added.
Happy to open a separate PR repairing the root causes, since they currently block every contribution: regenerate the frontend lockfile, delete the stray |
Description
Extends
UsdAmountinto a genericCurrencyAmountthat renders in the currency the reader picked, with a selector in the header and the choice persisted across reloads.Fixes #834
Type of Change
How the acceptance criteria are met
CurrencyProvider, not in each component, so one change re-renders every amount at once. Covered by a test that mounts two amounts and asserts both follow a single switch.useLocalStoragehook, which also syncs other open tabs. Tested both directions: that switching writes the key, and that a fresh mount with the key already set renders in that currency.Design notes
One lookup per page, not per amount.
currency.tsdoes a single USD-based rate fetch, cached in memory and inlocalStoragefor twelve hours, with concurrent callers sharing one in-flight request. A page showing twenty amounts makes one call; a reload makes none. Amounts resolve to USD once and are then converted for display, so switching currency never re-hits the token price feed. There is a test asserting onefetchfor three simultaneous amounts.Failing to USD, never failing to an error. A rejected fetch, a rate the response omitted, an unusable rate, or a currency
Intlcannot format all fall back to the exact USD display the component had before. The selector also hides itself when no rates are available, rather than offering choices that would silently do nothing."Local" currency is derived from the browser locale via
Intl.Locale.prototype.maximize().currency, with a small region-to-currency table behind it because that property is still missing in some engines. When neither resolves, the selector simply offers USD and EUR.utils.tsgainsxlmToUsdValue. The existingxlmToUsdreturns a formatted string, which cannot be converted onward into another currency. The numeric helper is the minimum needed;xlmToUsdnow delegates to it and is otherwise unchanged, so its existing callers are unaffected.Two things worth a reviewer's attention
1.
UsdAmountis now a deprecated wrapper. The issue asks to extend it intoCurrencyAmount, so the logic moved andUsdAmountstays as a thin delegating wrapper. Existing imports keep working. Its test needed one change: it stubbedxlmToUsd, and the component now takes the numeric path, so the stub moved toxlmToUsdValue. Same three assertions, same expected output.2. A small behaviour fix, needed to avoid a regression.
BountyCardhad its own copy of the conversion logic inline, which this PR replaces with the shared component so the card follows the selector too. That copy was more correct thanUsdAmount: it rendered nothing for a token with no price feed, whereasUsdAmountvalued any non-USDC token at the XLM rate.CurrencyAmounttakes the card's behaviour, so unifying them does not regress the card. This does mean a non-XLM, non-USDC token now shows no fiat equivalent anywhere, instead of showing a wrong one.Validation
CurrencyAmount.test.tsx, one per acceptance criterion plus the caching and fallback edges. All pass.upstream/mainand on this branch, and compared, because several suites already fail on main:upstream/mainIdentical failure sets, no newly failing file, passed tests up by exactly the 12 added.
tsc --noEmit: 10 pre-existing errors before and after, none in any file this PR touches.eslint --max-warnings 0passes on every file this PR adds or changes.vite buildsucceeds and bundles this code (see the note below on what had to be worked around to run it at all).Frontend CI is currently red on
main, for reasons unrelated to this PRI could not run the CI steps as written, because all four fail on a clean checkout of
main. Flagging them since they will make this PR look red:npm cifails —frontend/package-lock.jsonis committed as a 0-byte file, so npm reports "can only install with an existing package-lock.json". I installed withnpm installlocally and restored the empty file so it stays out of this diff.npx tsc --noEmitfails —tsconfig.jsonsets"moduleResolution": "Node", andfrontendpins no TypeScript, sonpxfetches TS 6, which removednode10. Reproduces onmainwith no changes.npx eslint . --max-warnings 0fails — 21 pre-existing problems (12 errors) in files this PR does not touch, including unused variables and a stale disable directive.npx vite buildfails —frontend/vite.config.jsis a committed 72-byte fragment (/// <reference types="vitest" />followed by four dangling closing braces) which shadows the realvite.config.ts. With it moved aside the build gets further and then fails onsrc/hooks/useWallet.tsimporting@stellar/freighter-api, which is not infrontend/package.json.To verify my own work I pinned
typescript@5.9.3and installed@stellar/freighter-api, both with--no-save, and temporarily moved the brokenvite.config.jsaside. With only those pre-existing blockers removed, the build completes cleanly:None of those four are touched by this PR. Happy to open a separate PR fixing them (regenerate the lockfile, delete the stray
vite.config.js, pin TypeScript, add the missing dependency) since they block every frontend contribution right now — just say the word, as it is a big and entirely unrelated diff.Checklist