fix: currency filter when over thousands or below a certain value#700
fix: currency filter when over thousands or below a certain value#700kvhnuke merged 6 commits intodevop/release-v2-8-0from
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant FiltersUtil
Caller->>FiltersUtil: parseCurrency(value)
FiltersUtil->>FiltersUtil: Clean input (remove commas, '<')
FiltersUtil->>FiltersUtil: Retrieve currency, locale, exchange rate
FiltersUtil->>FiltersUtil: Convert to BigNumber, check NaN/zero
FiltersUtil->>FiltersUtil: Multiply by exchange rate
FiltersUtil->>FiltersUtil: Format using Intl.NumberFormat
FiltersUtil-->>Caller: Return formatted currency string
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
|
💼 Build Files |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/extension/src/ui/action/utils/filters.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/extension/src/ui/action/utils/filters.ts (2)
packages/extension/src/ui/action/views/settings/store.ts (1)
useCurrencyStore(12-41)packages/extension/src/ui/action/utils/currencyConfig.ts (1)
LANG_INFO(173-173)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: buildAll
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (3)
packages/extension/src/ui/action/utils/filters.ts (3)
24-24: Good input sanitization for currency parsing.The regex
/[,<]/geffectively handles both thousands separators (commas) and small value indicators (less-than signs), which aligns with the PR objective of fixing currency filters for values over thousands or below certain values.
30-31: Robust BigNumber handling with proper exchange rate conversion.The implementation correctly:
- Handles NaN and zero values by returning 0
- Applies exchange rate conversion using BigNumber for precision
- Converts to number for formatting
33-36: Excellent use of internationalized formatting.Using
Intl.NumberFormatwith locale-specific currency formatting is the right approach for handling different currency display formats across locales.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
packages/extension/src/providers/bitcoin/types/bitcoin-network.ts(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-alert.vue(1 hunks)packages/extension/src/providers/common/ui/send-transaction/send-alert.vue(1 hunks)packages/extension/src/providers/common/ui/verify-transaction/verify-transaction-amount.vue(1 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts(4 hunks)packages/extension/src/providers/ethereum/networks/skale/skale-base.ts(1 hunks)packages/extension/src/providers/ethereum/types/evm-network.ts(2 hunks)packages/extension/src/providers/ethereum/ui/eth-verify-transaction.vue(1 hunks)packages/extension/src/providers/kadena/types/kadena-network.ts(2 hunks)packages/extension/src/providers/kadena/ui/send-transaction/components/send-token-item.vue(1 hunks)packages/extension/src/providers/polkadot/types/substrate-network.ts(1 hunks)packages/extension/src/providers/polkadot/ui/send-transaction/components/send-token-item.vue(1 hunks)packages/extension/src/providers/solana/types/sol-network.ts(1 hunks)packages/extension/src/providers/solana/ui/sol-verify-transaction.vue(1 hunks)packages/extension/src/ui/action/composables/account-info.ts(2 hunks)packages/extension/src/ui/action/utils/filters.ts(2 hunks)packages/extension/src/ui/action/views/network-assets/components/network-assets-item.vue(1 hunks)packages/extension/src/ui/action/views/send-process/components/send-process-amount.vue(1 hunks)packages/extension/src/ui/action/views/settings/components/settings-select.vue(1 hunks)packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/components/best-offer-error.vue(1 hunks)packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/index.vue(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/components/best-offer-error.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension/src/ui/action/utils/filters.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (2)
packages/swap/src/configs.ts (1)
NATIVE_TOKEN_ADDRESS(123-123)packages/extension/src/libs/market-data/types.ts (1)
CoinGeckoTokenMarket(23-37)
🪛 Biome (1.9.4)
packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts
[error] 287-287: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: buildAll
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (19)
packages/extension/src/ui/action/views/send-process/components/send-process-amount.vue (1)
11-11: Verify that parseCurrency handles all edge cases correctly.The change from
formatFiatValue(token.valueUSD).valueto$filters.parseCurrency(token.valueUSD)is consistent with the broader refactor to standardize currency formatting. However, ensure that theparseCurrencyfilter handles all the same edge cases (null values, very large numbers, very small numbers) thatformatFiatValuepreviously handled.packages/extension/src/ui/action/views/network-assets/components/network-assets-item.vue (1)
48-48: Currency formatting refactor looks good.The change from
token.balanceUSDfto$filters.parseCurrency(token.balanceUSD)is consistent with the broader refactor to use theparseCurrencyfilter instead of pre-formatted fields. This should provide more consistent currency formatting across the application.packages/extension/src/providers/common/ui/verify-transaction/verify-transaction-amount.vue (1)
11-11: LGTM! Consistent currency formatting refactor.The change to use
$filters.parseCurrency(token.valueUSD)is consistent with the broader refactor across the extension and should provide standardized currency formatting.packages/extension/src/ui/action/views/settings/components/settings-select.vue (1)
76-76:Details
✅ Verification successful
Verify the updated filtering logic is correct.
The change from filtering on
itemdirectly toitem.fiat_currencysuggests the data structure has changed. Ensure that the list items now have afiat_currencyproperty and that this filtering logic aligns with the expected behavior.
🏁 Script executed:
#!/bin/bash # Description: Verify the structure of list items passed to settings-select component # Expected: Items should have fiat_currency property # Search for usages of settings-select component to understand the data structure rg -A 10 -B 5 "settings-select" --type vueLength of output: 70
🏁 Script executed:
#!/bin/bash set -e # Locate the settings-select.vue file FILE_PATH=$(find . -type f -iname 'settings-select.vue' | head -n 1) echo "Found file: $FILE_PATH" # Print component props and filtering logic around line 76 echo "---- Lines 1-40 (props) ----" sed -n '1,40p' "$FILE_PATH" echo "---- Lines 50-100 (including filter at line 76) ----" sed -n '50,100p' "$FILE_PATH"Length of output: 2865
🏁 Script executed:
#!/bin/bash set -e echo "Searching for <settings-select> usages in templates:" rg -n "<settings-select" -A3 -B3 . echo echo "Searching for definitions or assignments of 'fiat_currency':" rg -n "fiat_currency" -C2 .Length of output: 6174
Currency filtering verified
The
listprop items conform to theCurrencyinterface (which defines afiat_currency: stringinstore.ts). ThesearchedListfilter insettings-select.vueand theSettingSelectOptionbindings consistently useitem.fiat_currency, matching the intended behavior. No further changes required.• packages/extension/src/ui/action/views/settings/store.ts –
type Currencyincludesfiat_currency: string
• packages/extension/src/ui/action/views/settings/components/settings-select.vue –searchedListusesitem.fiat_currency.includes(...)
• packages/extension/src/ui/action/views/settings/components/setting-select-option.vue –:title="item.fiat_currency"and flag/icon bindingspackages/extension/src/providers/kadena/ui/send-transaction/components/send-token-item.vue (1)
20-21: LGTM! Currency formatting update aligns with the broader refactor.The replacement of
formatFiatValuewithparseCurrencyis consistent with the PR objective to fix currency filtering for large values and follows the pattern established across other provider components.packages/extension/src/providers/common/ui/send-transaction/send-alert.vue (1)
8-8: Currency formatting improvement for insufficient funds alert.Good change replacing
formatFiatValuewithparseCurrency. The removal of the manual dollar sign prefix suggests thatparseCurrencyhandles currency symbols internally, which is cleaner and more consistent.packages/extension/src/providers/ethereum/ui/eth-verify-transaction.vue (1)
64-64: Currency formatting update maintains existing conditional logic.The replacement of
formatFiatValuewithparseCurrencyis correctly implemented while preserving the conditional display logic for handling the '~' placeholder value.packages/extension/src/providers/solana/ui/sol-verify-transaction.vue (1)
72-72:Details
✅ Verification successful
Verify the necessity of parseFloat conversion.
The change correctly updates to
parseCurrency, but theparseFloat(item.USDval)conversion differs from other files that pass values directly. Please verify that this conversion is necessary for theitem.USDvaldata type.
🏁 Script executed:
#!/bin/bash # Description: Check if other components also use parseFloat with parseCurrency filter # Expected: Find patterns of parseFloat usage with parseCurrency to confirm consistency rg -A 2 -B 2 "parseCurrency.*parseFloat|parseFloat.*parseCurrency"Length of output: 1251
Confirm parseFloat usage for USDval
It looks like all instances ofparseCurrencyin the repo first normalize their inputs withparseFloat(for example, intransaction-fee-item.vue). Insol-verify-transaction.vue,item.USDvalcomes through as a string, so wrapping it inparseFloat(...)is consistent with existing patterns.No change is required here. Keeping the
parseFloat(item.USDval)conversion ensures the filter always receives a numeric value.packages/extension/src/providers/solana/types/sol-network.ts (1)
132-133: LGTM! Consistent with balanceUSDf change.The
valueffield change maintains consistency with thebalanceUSDfmodification by using raw string representation instead of pre-formatted values.packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-alert.vue (1)
10-10: LGTM! Consistent with currency formatting refactor.The change from
formatFiatValuetoparseCurrencysimplifies the currency formatting and should automatically handle the currency symbol placement. ThepriceDifferencecomputed value (BigNumber string) is compatible with the new filter.packages/extension/src/providers/kadena/types/kadena-network.ts (2)
118-118: LGTM! Consistent with network provider refactor.The change to use
usdBalance.toString()forbalanceUSDfis consistent with the same modification in other network providers (like Solana). This moves formatting responsibility to the UI layer whereparseCurrencyfilter will handle it.
123-123: LGTM! Maintains consistency with balanceUSDf change.The
valueffield change follows the same pattern asbalanceUSDf, using raw string representation for currency values that will be formatted by the UI layer.packages/extension/src/providers/polkadot/ui/send-transaction/components/send-token-item.vue (1)
20-21: LGTM! Proper migration to parseCurrency filter.The changes correctly migrate from
formatFiatValuetoparseCurrencyfilter for both token balance and token price displays. The data types (tokenBalanceas BigNumber string andtokenPriceas number) are compatible with the new filter.packages/extension/src/ui/action/views/swap/views/swap-best-offer/components/swap-best-offer-block/index.vue (1)
19-19: LGTM! Proper migration to parseCurrency filter.The change from
formatFiatValue(toTokenPrice).valuetoparseCurrency(toTokenPrice)correctly implements the new currency formatting approach and should address the currency filter issues mentioned in the PR title.packages/extension/src/ui/action/composables/account-info.ts (1)
34-38:Details
✅ Verification successful
Simplified fiat value assignment - verify UI formatting.
The removal of
formatFiatValueand direct string conversion simplifies the logic. However, ensure that UI components consumingfiatAmount.valueproperly format the currency using theparseCurrencyfilter, as raw values frommarketData.getTokenValuemay not be user-friendly for display.
🏁 Script executed:
#!/bin/bash # Description: Verify that components using fiatAmount from account-info composable properly format the value # Search for usage of fiatAmount in Vue templates rg -A 3 -B 3 "fiatAmount" --type vue # Look for account-info composable usage rg -A 5 -B 2 "account-info" --include="*.vue" --include="*.ts"Length of output: 569
I’ll re-run searches with corrected ripgrep flags to locate any uses of
fiatAmountand ensure currency formatting is applied.
🏁 Script executed:
#!/bin/bash # Search for usage of fiatAmount in .vue and .ts files rg "fiatAmount" -n --glob '*.vue' --glob '*.ts' # Search for account-info composable import/usage rg "useAccountInfo" -n --glob '*.vue' --glob '*.ts' rg "account-info" -n --glob '*.vue' --glob '*.ts' # Check for template interpolation of fiatAmount in Vue templates rg "\{\{\s*fiatAmount" -n --glob '*.vue' # Verify parseCurrency filter usage where fiatAmount might be passed through formatting rg "parseCurrency" -n --glob '*.vue' --glob '*.ts'Length of output: 11970
I’ll inspect the network-assets view to see how
fiatAmountis rendered and whether it’s formatted downstream.
🏁 Script executed:
#!/bin/bash # Print imports and template usage around fiat-amount in network-assets index.vue sed -n '1,80p' packages/extension/src/ui/action/views/network-assets/index.vueLength of output: 3088
Formatting is applied in downstream components – no changes needed.
The raw
fiatAmount.valuestring is passed into components likeNetworkActivityTotalandNetworkActivityTransaction, both of which wrap it in{{ $filters.parseCurrency(fiatAmount) }}. This ensures proper currency formatting in the UI.No further action required.
packages/extension/src/providers/bitcoin/types/bitcoin-network.ts (1)
133-138:Details
❓ Verification inconclusive
Ensure proper currency formatting for Bitcoin assets.
Similar to other network providers, the removal of
formatFiatValueand direct assignment of raw string values could lead to inconsistent currency formatting. RawBigNumber.toString()andcurrent_price?.toString()may not provide user-friendly formatting for large numbers or small decimals.Verify that UI components consuming Bitcoin asset data properly format
balanceUSDfandvaluefusing theparseCurrencyfilter.
🏁 Script executed:
#!/bin/bash # Description: Verify consistent currency formatting across all network providers # Search for similar patterns in other network provider files rg -A 2 -B 2 "balanceUSDf.*toString|valuef.*toString" --type ts # Check if all network providers have been updated consistently fd "network.ts$" --exec grep -l "balanceUSDf\|valuef" {} \;Length of output: 6498
Confirm fiat formatting consistency for Bitcoin assets
Across all network providers (Bitcoin, Solana, Ethereum EVM, Polkadot, Kadena), the
balanceUSDfandvalueffields now receive raw strings via.toString()rather than usingformatFiatValue. RawBigNumber.toString()andcurrent_price?.toString()may omit separators or inconsistent decimal places.Please verify that your UI layer applies the
parseCurrencyfilter (or equivalent) when rendering these fiat values to ensure a consistent, user-friendly display.Files to check:
- packages/extension/src/providers/bitcoin/types/bitcoin-network.ts (lines 133–138)
- packages/extension/src/providers/solana/types/sol-network.ts
- packages/extension/src/providers/ethereum/types/evm-network.ts
- packages/extension/src/providers/polkadot/types/substrate-network.ts
- packages/extension/src/providers/kadena/types/kadena-network.ts
packages/extension/src/providers/polkadot/types/substrate-network.ts (1)
162-162: LGTM: Clean architectural separation of concerns.The removal of
formatFiatValueand use of raw string values is a good architectural improvement that separates data processing from presentation formatting. TheBigNumber.toString()call is safe and the UI layer will handle formatting viaparseCurrencyfilter.packages/extension/src/providers/ethereum/types/evm-network.ts (1)
153-156: LGTM: Consistent implementation with proper null safety.The changes consistently remove
formatFiatValueusage and maintain proper null safety with optional chaining and nullish coalescing operators. TheBigNumber.toString()calls are safe and the approach aligns with the architectural refactor.Also applies to: 270-273
packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (1)
340-340: LGTM: Consistent formatting refactor implementation.The removal of
formatFiatValuefollows the same pattern as other network providers and maintains type safety with proper string conversion.Also applies to: 345-345, 380-380, 388-388
packages/extension/src/ui/action/views/settings/components/settings-select.vue
Outdated
Show resolved
Hide resolved
packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/extension/src/providers/bitcoin/ui/btc-verify-transaction.vue(1 hunks)packages/extension/src/ui/action/views/settings/components/settings-select.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/extension/src/ui/action/views/settings/components/settings-select.vue
packages/extension/src/providers/bitcoin/ui/btc-verify-transaction.vue
Outdated
Show resolved
Hide resolved
3 participants
Summary by CodeRabbit