fix: token selector balance display to usd value #705
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request refines the UI and utility logic across several components. It removes the space after the dollar sign in wallet balance displays, adds an Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as ConfirmView Component
participant W as useWallet Hook
U->>C: Open Confirm View
C->>W: Retrieve wallet details (address, isPeanutWallet, etc.)
alt Peanut Wallet
C->>C: Display transaction cost as "$0"
else Other Wallet
C->>C: Display transaction cost based on transactionCostUSD
end
sequenceDiagram
participant U as User
participant T as TokenSelector
participant A as AdvancedButton
U->>T: Interact with token selector
T->>T: Get selectedBalance and compute tokenUsdValue
T->>A: Pass tokenUsdValue prop (or fallback balance) to AdvancedButton
A->>A: Render token amount with appropriate formatting
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/constants/chains.consts.ts (1)
96-104: 🛠️ Refactor suggestionRemove @ts-ignore and fix type safety.
The @ts-ignore comment suggests potential type issues. Consider fixing the type safety by:
- Explicitly typing the spread result
- Using type assertions properly
-//@ts-ignore -export const chains = [ - ...Object.values(wagmiChains), - milkomeda, - milkomedaTestnet, - baseTestnet, - taikoGrimsvotn, - ZKSyncSepolia, -] as [Chain, ...Chain[]] +export const chains = [ + ...(Object.values(wagmiChains) as Chain[]), + milkomeda, + milkomedaTestnet, + baseTestnet, + taikoGrimsvotn, + ZKSyncSepolia, +] as [Chain, ...Chain[]]
🧹 Nitpick comments (2)
src/utils/balance.utils.ts (1)
107-113: Rename function to better reflect its purpose.The function name 'printableUsdc' is misleading as it's used for general token values, not just USDC. Consider renaming it to something more generic like 'formatTokenValueUSD' or 'printableTokenValue'.
-export const printableUsdc = (balance: bigint): string => { +export const formatTokenValueUSD = (balance: bigint): string => {Additionally, consider adding input validation to handle edge cases:
export const printableUsdc = (balance: bigint): string => { + if (balance < 0n) { + throw new Error('Balance cannot be negative'); + } const formatted = formatUnits(balance, PEANUT_WALLET_TOKEN_DECIMALS) // floor the formatted value const value = Number(formatted) const flooredValue = Math.floor(value * 100) / 100 return flooredValue.toFixed(2) }src/components/Global/TokenSelector/Components/AdvancedButton.tsx (1)
59-72: Improve accessibility for the token selector button.Consider enhancing the accessibility of the button:
- Add aria-expanded to indicate dropdown state
- Add aria-controls to associate with the dropdown menu
<section role="button" tabIndex={0} aria-label="Open token selector" + aria-expanded={isVisible} + aria-controls="token-selector-dropdown" className={`flex w-full ${!isStatic && ' cursor-pointer '} h-18 flex-row items-center justify-between border border-n-1 px-4 py-2 hover:bg-grey-1/10 dark:border-white ${classNameButton}`} onClick={() => { !isStatic && onClick() }} onKeyDown={(e) => { if (e.key !== 'Enter' && e.key !== ' ') return !isStatic && onClick() }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/app/(mobile-ui)/wallet/page.tsx(2 hunks)src/components/Create/Link/Confirm.view.tsx(2 hunks)src/components/Global/TokenSelector/Components/AdvancedButton.tsx(3 hunks)src/components/Global/TokenSelector/TokenSelector.tsx(1 hunks)src/components/Profile/Components/ProfileSection.tsx(1 hunks)src/constants/chains.consts.ts(1 hunks)src/utils/balance.utils.ts(2 hunks)src/utils/general.utils.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/(mobile-ui)/wallet/page.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (9)
src/components/Profile/Components/ProfileSection.tsx (1)
32-36: LGTM! KYC badge visibility logic updated correctly.The implementation correctly addresses TASK-9108 by hiding the KYC badge when KYC approval has not been granted.
src/constants/chains.consts.ts (1)
96-98:❓ Verification inconclusive
Verify the implications of including all chains from wagmiChains.
While using spread operator simplifies chain management, it could potentially include unwanted chains or miss required chains if the wagmiChains module changes. Consider:
- Explicitly listing required chains to maintain control over supported networks.
- Adding validation to ensure critical chains are available.
🏁 Script executed:
#!/bin/bash # Description: Check which chains are being included and if any critical chains might be missing # List all exported chains from wagmi echo "Checking available chains in wagmi:" node -e " const wagmi = require('wagmi/chains'); console.log('Available chains:', Object.keys(wagmi).join(', ')); " # Search for chain usage in the codebase to identify critical chains echo -e "\nChecking chain usage in the codebase:" rg -l "chainId.*[\"'](?:1|10|100|137|8453|5000|42161|56)[\"']" --type tsLength of output: 1358
Action Required: Reconsider Chain Inclusion Strategy
The current implementation uses the spread operator to include all values from
wagmiChains, which raises concerns over accidentally incorporating unintended chains. Our verification attempt to list available chains viawagmi/chainsfailed (due to a module resolution issue), and a scan of the codebase revealed explicit references to critical chain IDs across several files (e.g., insrc/components/Offramp/Confirm.view.tsx,src/constants/cashout.consts.ts, etc.).Please consider one or both of the following actions:
- Explicit Chain Listing: Enumerate the supported chains directly rather than spreading all available chains. This approach prevents inadvertent inclusion of unsupported or experimental chains if the underlying
wagmiChainsmodule changes.- Validation Mechanism: Implement a validation step to ensure that all critical chain IDs (as referenced in various parts of the codebase) are present and correctly configured.
Review and adjust the implementation at
src/constants/chains.consts.ts(around lines 96–98) to ensure that your chain inclusion strategy remains predictable and secure across future dependency updates.src/components/Global/TokenSelector/Components/AdvancedButton.tsx (1)
100-109: LGTM! Token value display logic implemented correctly.The implementation correctly addresses the requirement to show USD values for tokens with a proper fallback to token balance when USD value is not available.
src/components/Create/Link/Confirm.view.tsx (3)
76-76: LGTM!The addition of
isPeanutWalletto the destructured properties fromuseWallethook is appropriate for implementing the fee display logic.
318-324: LGTM!The conditional logic for displaying network cost is well-structured and handles all cases:
- Shows $0 for peanut wallet
- Shows $0 for zero transaction cost
- Shows $<0.01 for very small costs
- Shows formatted amount for other cases
327-332: LGTM!The MoreInfo text is appropriately updated to reflect whether the transaction is sponsored by peanut based on the wallet type.
src/components/Global/TokenSelector/TokenSelector.tsx (1)
327-327: LGTM!The addition of
tokenValue={selectedBalance?.value}to the AdvancedTokenSelectorButton component enables USD value display in the token selector, aligning with the PR objectives.src/utils/general.utils.ts (2)
2-4: LGTM!The imports are correctly updated to include required dependencies.
299-304:❓ Verification inconclusive
Verify the impact of flooring token amounts.
The addition of flooring logic in
formatTokenAmountensures consistent display of token values by removing sub-decimal precision. However, this change could affect display of very small token amounts.Run the following script to check usage of this function with small token amounts:
🏁 Script executed:
#!/bin/bash # Description: Find calls to formatTokenAmount with small values rg -A 2 "formatTokenAmount\(" | grep -E "0\.[0-9]{8,}|1e-[0-9]+"Length of output: 66
Action: Reassess the impact of flooring token amounts before finalizing.
The automated search did not reveal any instances where
formatTokenAmountis called with very small token values (e.g., values matching "0.[0-9]{8,}" or scientific notation like "1e-[0-9]+"). However, because the search returned no output, it's uncertain whether such cases exist or if they were missed by the query. Please perform a manual verification or extend the analysis to ensure that the flooring logic does not inadvertently drop significant digits for extremely small amounts.
- Next steps:
- Manually verify code paths using
formatTokenAmountto confirm they do not pass token amounts that rely on sub-decimal precision.- Consider augmenting tests or adding usage examples with very small values to validate the intended behavior.
src/components/Global/TokenSelector/Components/AdvancedButton.tsx
Outdated
Show resolved
Hide resolved
| contracts: {}, | ||
| } as const satisfies Chain | ||
|
|
||
| //@ts-ignore |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Global/TokenSelector/Components/AdvancedButton.tsx (1)
104-105: Consider using a more robust USD value formatting approach.The current string concatenation with
$could be improved for better internationalization support.Consider using a dedicated currency formatting utility:
-`$${utils.formatTokenAmount(parseFloat(tokenUsdValue), 2)}` +utils.formatUsdValue(tokenUsdValue)This would encapsulate the currency symbol and decimal places in a reusable utility function.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Global/TokenSelector/Components/AdvancedButton.tsx(3 hunks)src/components/Global/TokenSelector/TokenSelector.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Global/TokenSelector/TokenSelector.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (2)
src/components/Global/TokenSelector/Components/AdvancedButton.tsx (2)
15-15: Consider a more explicit prop name.The prop name
tokenUsdValuecould be more explicit to clearly indicate it represents the USD value.Consider renaming to
tokenBalanceInUsdortokenValueInUsdfor better clarity.
100-109: LGTM! Clear implementation with good fallback logic.The balance display logic aligns well with the PR objectives, showing USD value when available and falling back to token balance. The comments clearly explain the logic.
Summary by CodeRabbit
Style
New Features