-
Notifications
You must be signed in to change notification settings - Fork 5.2k
feat: fetch asset metadata on search #31258
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
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.
Pull Request Overview
This PR introduces a new hook (useAssetMetadata) to fetch asset details that aren’t available in the local token lists and improves token filtering in the asset picker. Key changes include refactoring multichain balance processing to use a robust CAIP parser, updating the tokens filtering logic with enhanced image URL fallbacks, and integrating a debounced search with cancellation in the asset picker modal.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
ui/hooks/useMultichainBalances.ts | Refactored CAIP parsing and adjusted asset property types to improve balance handling. |
ui/hooks/bridge/useTokensWithFiltering.ts | Updated token filtering logic and image URL fallback for native assets. |
ui/components/multichain/asset-picker-amount/asset-picker-modal/hooks/useAssetMetadata.ts | Added new hook to fetch metadata for tokens not in the local list with an external service check. |
ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx | Integrated debounced search input, aborting previous fetches and using asset metadata when available. |
shared/lib/asset-utils.ts | Updated helper functions to generate asset IDs and corresponding image URLs. |
Test files | Extended coverage to validate asset metadata fetching and asset picker search behavior. |
Comments suppressed due to low confidence (4)
ui/hooks/useMultichainBalances.ts:30
- The 'address' property type has been changed from a CAIP template literal to a generic string. Please verify that downstream consumers correctly handle this updated format.
address: string;
ui/hooks/bridge/useTokensWithFiltering.ts:203
- Ensure that the 'isNative' property is consistently set across token objects, as the updated condition now relies on it for correctly identifying native tokens.
if (isNativeAddress(token.address) || token.isNative) {
ui/components/multichain/asset-picker-amount/asset-picker-modal/hooks/useAssetMetadata.ts:50
- [nitpick] The metadata fetch condition requires the trimmed search query to be longer than 30 characters; please confirm that this threshold is intended to avoid skipping valid queries.
if (allowExternalServices && shouldFetchMetadata && trimmedSearchQuery.length > 30) {
ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx:157
- [nitpick] Ensure that the debouncedSetSearchQuery function is properly memoized or stable across renders to prevent unnecessary effect calls.
useEffect(() => { debouncedSetSearchQuery(searchQuery); }, [searchQuery, debouncedSetSearchQuery]);
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.
Worked as expected! Awesome work @micaelae !
Suggestion:
If a token's info is being searched for, show a spinner or some type of feedback message so the user knows that work is happening.
The base branch was changed.
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.
Pull Request Overview
This pull request adds support for fetching asset metadata during token search and improves token filtering and rendering across multiple chains. Key changes include the introduction of a new hook (useAssetMetadata) to fetch missing asset details, updates to token filtering logic and image URL resolution in the bridge and asset picker components, and corresponding unit and end-to-end test updates.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
ui/hooks/useMultichainBalances.ts | Adjusts asset data structure by replacing the CAIP-formatted address with a generic string and adding an assetId field via parseCaipAssetType. |
ui/hooks/bridge/useTokensWithFiltering.ts | Enhances image URL fallback logic and token native address checks with added conditions and external image map usage. |
ui/components/multichain/asset-picker-amount/asset-picker-modal/hooks/useAssetMetadata.ts | Introduces a new hook for asynchronously fetching asset metadata based on search input. |
ui/components/multichain/asset-picker-amount/asset-picker-modal/asset-picker-modal.tsx | Incorporates debounced search input and aborting of previous metadata fetch requests to improve responsiveness. |
shared/lib/asset-utils.ts & Tests | Implements asset ID conversion, image URL generation, and metadata fetching logic with extended test coverage. |
Builds ready [dd3baa7]
UI Startup Metrics (1212 ± 65 ms)
Bundle size diffs
|
Description
This changes adds a
useAssetMetadata
hook that fetches asset details if it is not found in existing token lists. I've also added more unit test coverage for the asset-picker token filteringSee Copilot summary for more details
Related issues
Fixes: https://consensyssoftware.atlassian.net/browse/MMS-2094
Manual testing steps
Screenshots/Recordings
Before
No asset is shown
After
Screen.Recording.2025-03-25.at.5.29.08.PM.mov
Pre-merge author checklist
Pre-merge reviewer checklist