-
Notifications
You must be signed in to change notification settings - Fork 277
Append chain name for L2-native assets in RampCreateScene
#5842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
swansontec
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but I think it could be made easier to read.
| // Append chain name for L2-native assets like Optimism ETH | ||
| const selectedCryptoCurrencyCodeDisplay = | ||
| selectedCrypto != null && | ||
| selectedWallet != null && | ||
| selectedCryptoCurrencyCode != null | ||
| ? (() => { | ||
| const isL2Native = | ||
| selectedCrypto.tokenId == null && | ||
| !isAssetNativeToChain(selectedWallet.currencyInfo, undefined) | ||
| return isL2Native | ||
| ? `${selectedCryptoCurrencyCode} (${selectedWallet.currencyInfo.displayName})` | ||
| : selectedCryptoCurrencyCode | ||
| })() | ||
| : undefined | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about:
function getSelectedCryptoDisplay(): string | undefined {
if (selectedCrypto == null) return
if (selectedWallet == null) return
if (selectedCryptoCurrencyCode == null) return
const isL2Native =
selectedCrypto.tokenId == null &&
!isAssetNativeToChain(selectedWallet.currencyInfo, undefined);
return isL2Native
? `${selectedCryptoCurrencyCode} (${selectedWallet.currencyInfo.displayName})`
: selectedCryptoCurrencyCode;
}And then later you can replace selectedCryptoCurrencyCodeDisplay with getSelectedCryptoDisplay() in those two spots.
bfa00ed to
aa75226
Compare
aa75226 to
39c85b7
Compare
| placeholder={sprintf( | ||
| lstrings.trade_create_amount_s, | ||
| selectedCryptoCurrencyCode | ||
| getSelectedCryptoDisplay() ?? selectedCryptoCurrencyCode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Loading State Breaks Crypto Display
When isLoadingPersistedCryptoSelection is true, both getSelectedCryptoDisplay() and selectedCryptoCurrencyCode can be undefined, causing undefined to be passed to sprintf for the placeholder text. This happens because the input is rendered when either selectedCryptoCurrencyCode != null OR isLoadingPersistedCryptoSelection == true, but the fallback pattern doesn't handle the loading state where all crypto selection values are undefined.
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have:
Note
Appends the network name to L2-native asset labels in RampCreateScene placeholders and alerts for clearer asset identification.
src/components/scenes/RampCreateScene.tsx):getSelectedCryptoDisplay()to append chain display name for L2-native parent assets (e.g., "ETH (Optimism)").isAssetNativeToChainto detect L2-native context.Written by Cursor Bugbot for commit 39c85b7. This will update automatically on new commits. Configure here.