Conversation
isstuev
force-pushed
the
fe-2293
branch
5 times, most recently
from
92957f0 to
0bf78fe
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| const hasCurrent = Boolean(exchangeRate); | ||
| const hasToggle = hasHistoric && hasCurrent && hasExchangeRateToggle && amount !== '0'; | ||
|
|
||
| const [ showHistoric, setShowHistoric ] = React.useState(hasHistoric); |
There was a problem hiding this comment.
I believe the correct adjective will be "historical" not "historic"
In general, historic means ‘notable in history, significant in history,’ as in a Supreme Court decision, a battlefield, or a great discovery. Historical means ‘relating to history or past events’: (‘historical society’; ‘historical documents’). To write historic instead of historical may imply a greater significance than is warranted: a historical lecture may simply tell about something that happened, whereas a historic lecture would in some way change the course of human events.
There was a problem hiding this comment.
it's the name from the API
There was a problem hiding this comment.
Nevertheless, I don't think we should copy their error into our code.
|
Important Review skippedAuto reviews are disabled on this repository. To trigger a review, include You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Stale state causes brief wrong exchange rate display
- Replaced synced derived state with a user-only override so historic-rate visibility now defaults directly from props without a post-paint state sync gap.
Or push these changes by commenting:
@cursor push 169b339a97
Preview (169b339a97)
diff --git a/ui/shared/value/AssetValue.tsx b/ui/shared/value/AssetValue.tsx
--- a/ui/shared/value/AssetValue.tsx
+++ b/ui/shared/value/AssetValue.tsx
@@ -45,17 +45,14 @@
const hasCurrent = Boolean(exchangeRate);
const hasToggle = hasHistoric && hasCurrent && hasExchangeRateToggle && amount !== '0';
- const [ showHistoric, setShowHistoric ] = React.useState(hasHistoric);
+ const [ showHistoricOverride, setShowHistoricOverride ] = React.useState<boolean | undefined>(undefined);
+ const showHistoric = hasHistoric && (showHistoricOverride ?? true);
- React.useEffect(() => {
- setShowHistoric(hasHistoric);
- }, [ hasHistoric ]);
-
const activeExchangeRate = showHistoric ? historicExchangeRate : exchangeRate;
const handleToggle = React.useCallback(() => {
if (hasToggle) {
- setShowHistoric(prev => !prev);
+ setShowHistoricOverride(prev => (prev === undefined ? false : !prev));
}
}, [ hasToggle ]);|
|
||
| React.useEffect(() => { | ||
| setShowHistoric(hasHistoric); | ||
| }, [ hasHistoric ]); |
There was a problem hiding this comment.
Stale state causes brief wrong exchange rate display
Low Severity
The showHistoric state is initialized from hasHistoric on mount and synced via useEffect, but since effects fire after paint, there's a render frame where showHistoric is stale. When data loads (stub has historic_exchange_rate: null, real data has a string), hasHistoric becomes true but showHistoric remains false for one render, causing activeExchangeRate to use exchangeRate (current) instead of historicExchangeRate. This briefly displays the wrong USD value before the effect corrects it. Deriving the default from props directly — and using state only for the user's explicit toggle override — would avoid this flash.
Additional Locations (1)
2 participants
Description and Related Issue(s)
resolves #2293
Additional Information
historical exchange rate would be set to null for the latest txs in the 10.0 backend release
Checklist for PR author
Note
Medium Risk
Touches shared value/fee rendering used across transaction pages and lists, adding new toggle state and a new
historic_exchange_ratefield; mistakes could cause incorrect USD display or inconsistent tooltips across many views.Overview
Adds support for a
historic_exchange_rateonTransactiondata (types, RPC formatting, and stubs/mocks) so the UI can distinguish no historical rate provided vs explicitly null.Updates
AssetValue/SimpleValue(and wiring throughNativeCoinValue/TxFee) to display USD using the historical rate by default when available, with an optional clickable tag to toggle between historical and current USD values and clearer tooltip labeling. Propagates this through multiple tx views (tx details, fees/burnt fees, rollup fee rows, and tx list/table items), and adds Playwright screenshot coverage for the newAssetValuebehaviors.Written by Cursor Bugbot for commit a09eeb0. This will update automatically on new commits. Configure here.