Skip to content

dropdown asset selection#35

Closed
toppo101 wants to merge 2 commits intodevfrom
dropdown_asset_selection
Closed

dropdown asset selection#35
toppo101 wants to merge 2 commits intodevfrom
dropdown_asset_selection

Conversation

@toppo101
Copy link

@toppo101 toppo101 commented Aug 14, 2025

fixed the issue where, even if we select different asset types for USDC, the first one gets selected by default.
Fixed the URL for tokens with same symbol.

Solution also for : URL re-direction for unique assets #34

Validated the solution by applying transaction. Shared the screenshot before and after the swap for reference.
New Microsoft Word Document.docx

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR implements a token identification system to fix a critical UX issue where users selecting different variants of tokens with the same symbol (like multiple USDC types) would always have the first variant selected by default. The solution replaces the previous symbol-based token selection with a unique identifier system that appends 4-digit hash suffixes to duplicate symbols (e.g., USDC-0002, USDC-0003).

The implementation spans multiple files: a new utility module (tokenUtils.ts) provides the core identification logic, the main token hook (useSwapTokens.ts) is updated to use identifiers instead of symbols for token selection and URL management, URL query parameter handling is modified to remove hard-coded defaults, and comprehensive documentation explains the new system.

This change integrates with the existing swap interface by maintaining the same UI components while upgrading the underlying token selection mechanism. The system preserves backward compatibility by falling back to symbol-based matching when identifiers aren't found, ensuring existing URLs continue to work. The new approach also makes URLs more user-friendly by using readable token symbols instead of numeric asset IDs, while solving the ambiguity problem for tokens with duplicate symbols.

Important Files Changed

Files Changed
Filename Score Overview
apps/web/src/lib/tokenUtils.ts 2/5 New utility module for token identification with critical bug in hash generation function
apps/web/src/components/swap/hooks/useSwapTokens.ts 4/5 Updated token selection logic to use identifiers instead of symbols with proper error handling
apps/web/src/components/swap/hooks/utils/queryParams.ts 4/5 Removed hard-coded default tokens from URL parameters to allow proper token selection
apps/web/docs/token-url-system.md 4/5 Added comprehensive documentation explaining the new token URL system

Confidence score: 2/5

  • This PR has a critical implementation bug that prevents the core functionality from working as intended
  • Score lowered due to the generateTokenIdentifier function not implementing the documented hash generation, which means duplicate tokens will still have identical identifiers
  • Pay close attention to apps/web/src/lib/tokenUtils.ts which needs the hash generation logic implemented to actually solve the duplicate symbol problem

Sequence Diagram

sequenceDiagram
    participant User
    participant SwapContainer
    participant useSwapTokens
    participant queryParams
    participant tokenUtils
    participant API
    participant AssetList
    
    User->>SwapContainer: "Load swap page"
    SwapContainer->>useSwapTokens: "Initialize tokens"
    useSwapTokens->>API: "api.assets.getAll()"
    API-->>useSwapTokens: "Return assets array"
    
    useSwapTokens->>tokenUtils: "createTokenIdentifierMap(assets)"
    tokenUtils-->>useSwapTokens: "Return identifier->assetId mapping"
    
    useSwapTokens->>tokenUtils: "createAssetIdToIdentifierMap(assets)"
    tokenUtils-->>useSwapTokens: "Return assetId->identifier mapping"
    
    useSwapTokens->>queryParams: "useFromTokenState()"
    queryParams-->>useSwapTokens: "Return fromTokenIdentifier from URL"
    
    useSwapTokens->>tokenUtils: "findAssetByIdentifier(assets, identifier)"
    tokenUtils-->>useSwapTokens: "Return matching asset"
    
    useSwapTokens-->>SwapContainer: "Return inputToken, outputToken, tokens"
    SwapContainer-->>User: "Display token selection interface"
    
    User->>AssetList: "Click token dropdown"
    AssetList-->>User: "Show available tokens with unique identifiers"
    
    User->>AssetList: "Select USDC-0002 token"
    AssetList->>useSwapTokens: "setInputToken(token)"
    
    useSwapTokens->>tokenUtils: "assetIdToIdentifierMap.get(token.id)"
    tokenUtils-->>useSwapTokens: "Return 'USDC-0002'"
    
    useSwapTokens->>queryParams: "setFromTokenIdentifier('USDC-0002')"
    queryParams->>queryParams: "Update URL: ?from=USDC-0002"
    
    queryParams-->>SwapContainer: "Trigger re-render with new token"
    SwapContainer-->>User: "Display selected USDC-0002 token"
Loading

4 files reviewed, 3 comments

Edit Code Review Bot Settings | Greptile

// Convert symbols to token objects
// Set default tokens if none are selected and assets are loaded
useEffect(() => {
if (assets.length > 0 && isInitialized && (!fromTokenIdentifier || !toTokenIdentifier)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: the condition !fromTokenIdentifier || !toTokenIdentifier will trigger default selection even when empty strings are set in URL params, which may override user's explicit empty selections

Comment on lines +21 to +23
export function generateTokenIdentifier(asset: AssetWithId): string {
return asset.metadata.symbol;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Critical bug: this function doesn't implement the hash logic described in its documentation. It should call the same logic used in createTokenIdentifierMap to ensure consistency.

Comment on lines +10 to +11
hash = ((hash << 5) - hash) + char;
hash = hash & hash; // Convert to 32-bit integer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: The bitwise operation hash & hash is redundant and should be removed as it doesn't convert to 32-bit integer as stated in the comment.

@muddlebee
Copy link
Member

closing this, there will be a entire revamp of the API backend and UI.

@muddlebee muddlebee closed this Aug 29, 2025
@muddlebee muddlebee deleted the dropdown_asset_selection branch September 27, 2025 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants