-
-
Notifications
You must be signed in to change notification settings - Fork 279
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
feat(suite): added search by native token to tx list #17251
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe changes update the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (7)
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 (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
suite-common/wallet-utils/src/transactionUtils.ts (1)
940-956
: Good implementation of native token search feature.The code correctly implements searching by native token symbol by:
- Getting the display symbol using
getNetworkDisplaySymbol
- Performing a case-insensitive check against the search term
- Ensuring the transaction has targets or internal transfers and a non-zero amount
- Adding matching transaction IDs to the results
The implementation follows the existing pattern in the codebase and is well integrated with the current search functionality.
One small enhancement to consider:
- // native token + // Find by native token symbol
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
suite-common/wallet-utils/src/transactionUtils.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: EAS Update
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: build-web
- GitHub Check: prepare_android_test_app
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: build-web
🔇 Additional comments (2)
suite-common/wallet-utils/src/transactionUtils.ts (2)
7-7
: LGTM on new import.The import for
getNetworkDisplaySymbol
is correctly added to support the new search by native token functionality.
959-960
: Good adjustment to clarify token search comment.The comment change from implicit understanding to explicitly mentioning "other tokens" helps distinguish between the new native token search and the existing token search functionality.
🚀 Expo preview is ready!
|
return []; | ||
}); | ||
txsToSearch.push(...foundTxsForNativeToken); | ||
|
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.
When I look for eth
there are also txs with token USDT
because the name Tether
contains eth
😀
One option is to remove token.name
from isTokenTransferMatchesSearch
(with comment)
Another option is to when search === displaySymbolName
do not do this token searching?
Please open this discussion also with Jirka. Thanks
Description
Support filtering of account transactions by native token symbol.
Related Issue
Resolve #16116
Screenshots:
Screen.Recording.2025-02-26.at.13.39.36.mov