feat(pulse): Mobile-responsive token search UI with blockchain filter… #461
Conversation
…ing and data refinements ## Overview Comprehensive refinement of the Pulse token search UI with focus on mobile responsiveness, blockchain filtering, data accuracy, and pixel-perfect design alignment. ## Key Features ### Mobile Responsiveness - Implemented full-screen search modal on mobile devices (width < 768px) - Added back button navigation on mobile (replaces close button) - Fixed filter buttons to wrap to multiple lines instead of horizontal scroll - Adjusted search bar dimensions to match Figma specs (236px width, 40px height, 10px radius) - Made ChainOverlay mobile-responsive with proper positioning - Ensured TokenList and SearchSkeleton components are mobile-optimized ### Blockchain Filtering - Removed XDAI from blockchain options - Moved 'All chains' option to top of chain selector - Implemented chain-based filtering for Trending/Fresh/Search results - Fixed click-outside behavior to prevent modal from closing when selecting chains - Converted ChainOverlay to use React.forwardRef for proper ref handling ### Data Quality & Accuracy - Filter out assets with zero volume or market cap from all views - Deduplicate assets across chains, showing each asset once - Aggregate volume data across blockchains for Trending view - Sort Trending by total aggregated volume (descending) - Sort Fresh by newest timestamp (descending) - Ensure consistent data presentation across Assets and Markets sections ### UI/UX Improvements - Updated MCap/Vol values to white color (matching Markets section) - Maintained consistent font sizing across Assets and Markets - Added double-layer background styling to action buttons - Improved token name truncation and spacing - Fixed price formatting to 2 decimals for better readability - Enhanced header spacing and button alignment ### Bug Fixes - Fixed JSX syntax errors in Buy.tsx and Sell.tsx - Resolved click-outside handler conflicts with ChainOverlay - Fixed responsive breakpoint issues by using useIsMobile hook - Corrected store.ts reducer initialization ## Technical Changes ### New Components - MarketList.tsx: Display trading pairs with liquidity/volume data - SearchSkeleton.tsx: Loading state for search results ### Modified Components - Search.tsx: Mobile responsiveness, chain filtering, sorting logic - TokenList.tsx: Styling updates, responsive text handling - ChainOverlay.tsx: forwardRef implementation, XDAI removal, ordering - Buy.tsx & Sell.tsx: JSX syntax fixes - TokenPrice.tsx: Price formatting improvements ### Utilities - parseSearchData.ts: Deduplication, filtering, aggregation logic - store.ts: Improved reducer initialization ## Testing - Verified on mobile viewports (< 768px width) - Tested blockchain filtering across all views - Confirmed data accuracy and deduplication - Validated responsive behavior and styling
WalkthroughAdds multi-chain asset and market parsing, new MarketList and SearchSkeleton components, refactors Search and TokenList for sorting/filtering and mobile behavior, converts ChainOverlay to forwardRef, adjusts TokenPrice precision, tightens Buy/Sell runtime guards, and pre-populates store reducers. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Search as Search Component
participant Parse as parseSearchData
participant MarketList as MarketList Component
participant TokenList as TokenList Component
participant ChainOverlay as ChainOverlay/Selection
User->>Search: type query / open search
Search->>Parse: parseSearchData(input, chains, searchTerm)
Parse->>Parse: aggregate multi-chain assets, parse markets, filter by liquidity
Parse-->>Search: return { assets, markets }
alt User selects a market
User->>MarketList: click market row
MarketList->>Search: handleMarketSelect(market)
Search->>ChainOverlay: set buy token, infer chainId, close overlay
else User selects an asset
User->>TokenList: click asset
TokenList->>Search: getChainWithMostUSDC(asset)
Search->>ChainOverlay: set buy token (selected chain), close overlay
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Deploying x with
|
| Latest commit: |
a0bcfb4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6ff7e6d7.x-e62.pages.dev |
| Branch Preview URL: | https://feature-pulse-search-ui-mobi.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
src/store.ts (1)
75-79: Update comment to reflect non-empty initial reducersThe comment above
configureStorestill says “Empty reducer for now…”, but you now seed the store withinitialReducers. It’d be good to update this doc so future readers don’t assume everything is dynamically added.src/apps/pulse/components/Buy/Buy.tsx (2)
166-194: Unify token-balance lookup with Sell and consider more robust selectionThis new
getTokenBalancemirrors the logic inSell.tsx, but:
- It only logs to
console.erroron failure, whereas Sell also reports vialogPulseErrorwith context.- It first narrows by
asset.asset.symbol === token.symboland then byaddress/chainIdinsidecontracts_balances. If multiple assets share the same symbol across chains,.findonassetscould stop on an entry that doesn’t contain the exact(address, chainId)pair and incorrectly return0even when another asset entry has the correct contract.Two suggestions:
- Extract a shared helper (e.g., in a
portfolioutil) used by both Buy and Sell that:
- Searches all assets’
contracts_balancesfor the(address, chainId)match directly, rather than first gating by symbol.- Reports portfolio access issues via
logPulseErrorin addition toconsole.error, similar to Sell.This would keep Buy/Sell behavior aligned and make any future portfolio schema tweaks easier to centralize. Based on learnings.
805-825: Avoidanycasts for Relay buy offer; widen types insteadHere and in
handleBuySubmityou’re castingbuyOffertoany/ExpressIntentResponseto fit the existingexpressIntentResponseprop/state shape:expressIntentResponse={ USE_RELAY_BUY ? (buyOffer as any) : expressIntentResponse }That hides type mismatches and makes it easy to accidentally rely on
ExpressIntentResponsefields that don’t exist onBuyOffer(or vice versa).Instead, consider:
- Updating
BuyButton’s prop to accept a union, e.g.ExpressIntentResponse | BuyOffer | null, with an explicit discriminant orUSE_RELAY_BUYflag passed alongside.- Widening the local
expressIntentResponsestate and the parent setter type similarly, rather than casting.This keeps Relay and Intent flows type-safe and makes refactors less brittle.
src/apps/pulse/utils/parseSearchData.ts (3)
53-106: Multi-chain asset shaping and dedup look sound; consider small cleanupsThe new multi-chain handling is a solid improvement:
parseAssetData/parseTokenData:
- Skip non-supported chains and wrapped native tokens.
- Create a single primary
Assetentry per Mobula asset, withallChains/allContracts/allDecimalscapturing the rest.deduplicateAssetsBySymbol:
- Uses Mobula
idas the primary key where available.- Falls back to symbol-level keys when no ID exists, merging
allChains/allContracts/allDecimalsand dropping duplicated token-only entries.A couple of optional polish ideas:
- The valid-chain index collection logic is duplicated between
parseAssetDataandparseTokenData; extracting a small helper (e.g.getValidChainIndices(asset, chains)) would reduce drift if chain rules change.deduplicateAssetsBySymbolmutatesexisting.allChains/existing.allContracts/existing.allDecimalsin place; that’s fine internally, but if you ever reuse the original arrays elsewhere, a shallow clone before merging would make side effects more obvious.Overall, the data shaping is coherent and matches the multi-chain aggregation goals.
Also applies to: 109-154, 397-451
308-320: Consider gating verbose console logging behind a debug flagThe AAVE-specific debug logs and duplicate-token logs are useful for diagnosing search issues, but they will:
- Log the full
searchDataitem list whenever the user searches for “AAVE”.- Log every skipped duplicate token symbol in
deduplicateAssetsBySymbol.In production this can clutter the console and potentially leak implementation details.
You might want to:
- Wrap these
console.logcalls in a debug guard (e.g.if (process.env.NODE_ENV === 'development')), or- Route them through whatever internal logging/debug utility you use elsewhere, so they can be toggled or sampled.
Also applies to: 341-377, 429-430
453-543:parseFreshAndTrendingTokensaggregates correctly; ensure callers handle sortingThis helper now:
- Aggregates
volumeandmCapper symbol (or name) across chains.- Tracks the newest
timestampper asset for Fresh ordering.- Populates
allChains/allContracts/allDecimalsfor multi-chain awareness.- Filters out assets with zero
volumeormCap.It does not apply any explicit sorting for “Trending” (by aggregated volume) or “Fresh” (by newest timestamp); it just returns the filtered array from the
Map.If the UI requirements rely on specific ordering, make sure:
- Callers explicitly sort the returned array according to context (e.g., trending:
volumedesc; fresh:timestampdesc), or- You move that sorting logic into this helper (e.g., with a
mode: 'trending' | 'fresh'parameter) to guarantee consistent behavior across call sites.src/apps/pulse/components/Sell/Sell.tsx (1)
512-515: Unified percentage button styles across Buy/SellThe conditional className for percentage buttons now matches the Buy component (different background/text colors and cursor state when disabled), which improves consistency and makes the disabled state clearer.
src/apps/pulse/components/Search/MarketList.tsx (1)
10-15: Unused propsshowLiquidityFilter/minLiquidity
MarketListPropsexposesshowLiquidityFilterandminLiquidity, but the component only consumesmarketsandhandleMarketSelect. Since filtering is currently done inSearch.tsxviafilterMarketsByLiquidity, these props are dead.Either wire them up here (and let MarketList own the filtering) or remove them from the public API to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/apps/pulse/components/Buy/Buy.tsx(4 hunks)src/apps/pulse/components/Price/TokenPrice.tsx(1 hunks)src/apps/pulse/components/Search/ChainOverlay.tsx(2 hunks)src/apps/pulse/components/Search/MarketList.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(11 hunks)src/apps/pulse/components/Search/SearchSkeleton.tsx(1 hunks)src/apps/pulse/components/Search/TokenList.tsx(3 hunks)src/apps/pulse/components/Sell/Sell.tsx(4 hunks)src/apps/pulse/utils/parseSearchData.ts(5 hunks)src/store.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/MarketList.tsxsrc/apps/pulse/components/Search/Search.tsxsrc/apps/pulse/utils/parseSearchData.tssrc/apps/pulse/components/Search/TokenList.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsxsrc/apps/pulse/components/Sell/Sell.tsxsrc/apps/pulse/components/Price/TokenPrice.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsxsrc/apps/pulse/components/Price/TokenPrice.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/pulse/components/Search/ChainOverlay.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required text field. The TileTitleProps interface aligns with the TokensMarketData.title type in api.ts which also has optional fields.
Applied to files:
src/apps/pulse/components/Price/TokenPrice.tsxsrc/apps/pulse/components/Search/TokenList.tsx
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required title field. The TileTitleProps structure aligns with how it's used in the TokensMarketData type in api.ts.
Applied to files:
src/apps/pulse/components/Search/TokenList.tsx
🧬 Code graph analysis (8)
src/apps/pulse/components/Search/MarketList.tsx (5)
src/apps/pulse/utils/parseSearchData.ts (1)
Market(40-51)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/apps/pulse/utils/number.ts (1)
formatBigNumber(1-15)src/apps/pulse/components/Price/TokenPriceChange.tsx (1)
TokenPriceChange(5-57)
src/store.ts (5)
src/apps/the-exchange/reducer/theExchangeSlice.ts (1)
swapSlice(121-140)src/apps/token-atlas/reducer/tokenAtlasSlice.ts (1)
tokenAtlasSlice(131-149)src/apps/deposit/reducer/depositSlice.ts (1)
depositSlice(33-33)src/apps/pillarx-app/reducer/WalletPortfolioSlice.ts (1)
walletPortfolioSlice(149-171)src/apps/leaderboard/reducer/LeaderboardSlice.ts (1)
leaderboardSlice(35-36)
src/apps/pulse/components/Search/Search.tsx (9)
src/apps/pulse/utils/parseSearchData.ts (5)
Asset(20-38)Market(40-51)parseSearchData(300-391)filterMarketsByLiquidity(293-298)parseFreshAndTrendingTokens(453-543)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/media.ts (1)
useIsMobile(34-34)src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-35)src/apps/pulse/components/Search/ChainSelect.tsx (1)
ChainSelectButton(3-13)src/apps/pulse/components/Search/SearchSkeleton.tsx (1)
SearchSkeleton(7-45)src/apps/pulse/components/Search/Sort.tsx (1)
Sort(7-29)src/apps/pulse/components/Search/TokenList.tsx (1)
TokenList(20-228)src/apps/pulse/components/Search/MarketList.tsx (1)
MarketList(17-160)
src/apps/pulse/utils/parseSearchData.ts (5)
src/types/api.ts (5)
MobulaToken(506-522)Exchange(524-527)TokenAssetResponse(549-568)Asset(18-25)PairResponse(570-656)src/utils/blockchain.ts (2)
isWrappedNativeToken(514-521)getChainName(269-288)src/apps/pulse/utils/constants.ts (2)
MOBULA_CHAIN_NAMES(13-15)getChainName(51-70)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/apps/pulse/utils/number.ts (1)
parseNumberString(17-39)
src/apps/pulse/components/Sell/Sell.tsx (1)
src/apps/pulse/utils/blockchain.ts (1)
isNativeToken(17-18)
src/apps/pulse/components/Search/ChainOverlay.tsx (3)
src/apps/key-wallet/utils/blockchain.ts (1)
chains(32-34)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
src/apps/pulse/components/Search/TokenList.tsx (6)
src/apps/pulse/components/Search/Sort.tsx (1)
Sort(7-29)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/apps/pulse/utils/time.ts (1)
formatElapsedTime(1-25)src/apps/pulse/utils/number.ts (1)
formatBigNumber(1-15)src/apps/pulse/components/Price/TokenPrice.tsx (1)
TokenPrice(5-50)
src/apps/pulse/components/Buy/Buy.tsx (1)
src/apps/pulse/utils/blockchain.ts (1)
isNativeToken(17-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (7)
src/apps/pulse/components/Buy/Buy.tsx (1)
208-212: Native gas token check for min gas fee looks goodAdding
isNativeToken(t.contract)to thenativeTokenfilter tightens the min-gas check so you only consider actual native gas tokens on the settlement chain, which better matches the requirement “Min. $1 native required for gas”. This aligns with the pattern already used in Sell. Based on learnings.src/apps/pulse/components/Price/TokenPrice.tsx (1)
24-31: Price formatting to 2 decimals is consistent with UI goalsChanging the standard branch to
value.toFixed(2)keeps the existing small-number handling while aligning typical prices with a 2-decimal display, which matches the PR’s UX goals. Logic aroundfirstNonZeroIndexand the sub-cent rendering path remains intact.src/apps/pulse/components/Sell/Sell.tsx (2)
196-200: Native-token guard for gas requirement matches Buy behaviorAdding
isNativeToken(t.contract)to thenativeTokenselection ensures the min-gas check only considers true native gas tokens on the settlement chain, rather than any asset on that chain. This is consistent with the Buy-side change and with how gas requirements are communicated in the error message. Based on learnings.
412-421: Sell input UX tweaks are consistent with Buy and non-breakingThe updated input:
- Uses a conditional width based on whether a token is selected, which keeps layouts balanced with/without the token symbol suffix.
- Adds
type="text"and clears the placeholder on focus, mirroring the Buy component’s input behavior.These are purely presentational/UX changes and don’t affect the numeric parsing or liquidity checks, which still run off
handleTokenAmountChangeandtokenBalance. Based on learnings.src/apps/pulse/components/Search/SearchSkeleton.tsx (1)
3-44: Skeleton component structure looks solidThe SearchSkeleton implementation is simple, self-contained, and matches the intended “sections + rows” loading layout. No issues from a correctness or UX standpoint.
src/apps/pulse/components/Search/TokenList.tsx (1)
17-18: TokenList header toggling and single‑chain badge logic look good
hideHeaderscleanly allows the same list to be reused for both standalone Trending/Fresh views and embedded search results.- The header layout (MCap/Vol vs Price/24h %) is consistent across Search and TokenList.
- Showing the chain badge only when
allChainsis absent or length 1 correctly distinguishes multi‑chain aggregated assets from single‑chain ones.No blocking issues spotted in these changes.
Also applies to: 60-125, 173-210
src/apps/pulse/components/Search/Search.tsx (1)
323-355: Multi‑chain selection logic is sensible; consider verifying portfolio balance unitsThe new
getChainWithMostUSDChelper and the multi‑chain branch inhandleTokenSelectcorrectly:
- Scan the portfolio for USDC positions (by symbol/name).
- Pick the chain with the highest USDC balance.
- Map that chain ID back to a specific entry in
allChains/allContracts/allDecimals, falling back to the primary chain if no match is found.The flow is solid. One thing to double‑check outside this PR is whether
contracts_balances.balanceis already normalized to human‑readable units (post‑decimals). If it’s in raw integer units, you’d want to factor decimals into the comparison to avoid skewing toward chains where USDC is represented differently.Also applies to: 365-409
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/apps/pulse/utils/parseSearchData.ts (1)
172-180: Chain filter inparseMarketPairsstill doesn't apply the selected chain.The condition only checks if
pair.blockchainis inMOBULA_CHAIN_NAMES, not whether it matches the selectedchainsvalue:if ( chains !== MobulaChainNames.All && !MOBULA_CHAIN_NAMES.includes(pair.blockchain) ) { continue; }When a user selects a specific chain (e.g.,
MobulaChainNames.Ethereum), this still includes pairs from all supported chains. Compare toparseAssetData(lines 62-64), which correctly useschains === blockchains[i].Apply this fix:
// Filter by chain if specified if ( chains !== MobulaChainNames.All && - !MOBULA_CHAIN_NAMES.includes(pair.blockchain) + pair.blockchain !== chains ) { continue; }src/apps/pulse/components/Search/Search.tsx (1)
612-628: Still nesting button elements – invalid HTML.The outer
<button>at line 613 wraps theRefreshcomponent, which itself renders a<button>(see src/apps/pulse/components/Misc/Refresh.tsx lines 9-34). Nested buttons are invalid HTML and can cause inconsistent behavior.Replace the outer
<button>with a<div>:- <button + <div onClick={handleRefresh} - disabled={walletPortfolioFetching || isLoading} className="flex items-center justify-center w-10 h-10 bg-[#121116] rounded-[10px] flex-shrink-0 group p-0.5" - type="button" aria-label="Refresh data" data-testid="pulse-search-refresh-button" > <div className="flex items-center justify-center w-full h-full bg-[#1E1D24] rounded-[8px] group-hover:bg-[#2A2A2A] transition-colors"> <Refresh isLoading={walletPortfolioFetching || isLoading} onClick={handleRefresh} disabled={walletPortfolioFetching || isLoading} /> </div> - </button> + </div>Remove the outer
onClickanddisabledsince they're redundant with the innerRefreshbutton.
🧹 Nitpick comments (2)
src/apps/pulse/components/Buy/Buy.tsx (1)
779-803: AvoidanyforbuyOffer→ alignBuyButtonprop typing insteadThe new
expressIntentResponseprop value:expressIntentResponse={ USE_RELAY_BUY ? // eslint-disable-next-line @typescript-eslint/no-explicit-any (buyOffer as any) : expressIntentResponse }drops type-safety and adds a lint suppression. It would be cleaner to widen
BuyButton’s prop type to accept bothExpressIntentResponseandBuyOffer, then pass the union directly withoutanyor eslint-disable:- expressIntentResponse={ - USE_RELAY_BUY - ? // eslint-disable-next-line @typescript-eslint/no-explicit-any - (buyOffer as any) - : expressIntentResponse - } + expressIntentResponse={ + USE_RELAY_BUY ? buyOffer : expressIntentResponse + }This keeps Relay vs. Intent flows explicit and preserves TypeScript checking instead of opting out with
any. You’d just need to updateBuyButton’s prop definition accordingly (e.g.ExpressIntentResponse | BuyOffer | null) and ensure its internal logic narrows based on which shape it receives.src/apps/pulse/utils/parseSearchData.ts (1)
442-444: Consider removing or conditionalizing the console.log statement.The debug logging in the production code path may be too verbose:
console.log( `🚫 Skipping duplicate token: ${asset.name} (${asset.symbol}) - already have asset with ID ${symbolToAssetId.get(symbol)}` );Consider either removing this or making it conditional on a debug flag, as it will fire for every skipped duplicate during normal operation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/apps/pulse/components/Buy/Buy.tsx(1 hunks)src/apps/pulse/components/Search/ChainOverlay.tsx(2 hunks)src/apps/pulse/components/Search/MarketList.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(11 hunks)src/apps/pulse/components/Search/SearchSkeleton.tsx(1 hunks)src/apps/pulse/components/Search/TokenList.tsx(3 hunks)src/apps/pulse/utils/parseSearchData.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/apps/pulse/components/Search/MarketList.tsx
- src/apps/pulse/components/Search/SearchSkeleton.tsx
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/TokenList.tsxsrc/apps/pulse/components/Search/Search.tsxsrc/apps/pulse/utils/parseSearchData.ts
📚 Learning: 2025-11-21T13:10:33.401Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/TokenList.tsxsrc/apps/pulse/components/Search/Search.tsxsrc/apps/pulse/utils/parseSearchData.ts
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required text field. The TileTitleProps interface aligns with the TokensMarketData.title type in api.ts which also has optional fields.
Applied to files:
src/apps/pulse/components/Search/TokenList.tsxsrc/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required title field. The TileTitleProps structure aligns with how it's used in the TokensMarketData type in api.ts.
Applied to files:
src/apps/pulse/components/Search/TokenList.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-20T14:01:31.715Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 387
File: src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx:118-125
Timestamp: 2025-08-20T14:01:31.715Z
Learning: In src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx, the "My rank" calculation uses 'trading' context while the list uses 'migration' context when calling calculateFinalPxPoints. This is intentional design to control where the 200-point bonus is applied - the 'trading' context with 'all' timeTab bypasses the bonus while 'migration' context allows it, preventing points from being added in the wrong place.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/pulse/components/Search/ChainOverlay.tsx
🧬 Code graph analysis (4)
src/apps/pulse/components/Search/TokenList.tsx (6)
src/apps/pulse/components/Search/Sort.tsx (1)
Sort(7-29)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/apps/pulse/utils/time.ts (1)
formatElapsedTime(1-25)src/apps/pulse/utils/number.ts (1)
formatBigNumber(1-15)src/apps/pulse/components/Price/TokenPrice.tsx (1)
TokenPrice(5-50)
src/apps/pulse/components/Search/Search.tsx (9)
src/apps/pulse/utils/parseSearchData.ts (5)
Asset(19-37)Market(39-50)parseSearchData(299-392)filterMarketsByLiquidity(292-297)parseFreshAndTrendingTokens(480-576)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/media.ts (1)
useIsMobile(34-34)src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-35)src/apps/pulse/components/Search/ChainSelect.tsx (1)
ChainSelectButton(3-13)src/apps/pulse/components/Search/SearchSkeleton.tsx (1)
SearchSkeleton(7-44)src/apps/pulse/components/Search/Sort.tsx (1)
Sort(7-29)src/apps/pulse/components/Search/TokenList.tsx (1)
TokenList(20-240)src/apps/pulse/components/Search/MarketList.tsx (1)
MarketList(15-158)
src/apps/pulse/utils/parseSearchData.ts (4)
src/types/api.ts (7)
MobulaToken(506-522)Exchange(524-527)TokenAssetResponse(549-568)Asset(18-25)PairResponse(570-656)Projection(179-191)TokensMarketData(170-177)src/utils/blockchain.ts (2)
isWrappedNativeToken(514-521)getChainName(269-288)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/apps/pulse/utils/number.ts (1)
parseNumberString(17-39)
src/apps/pulse/components/Search/ChainOverlay.tsx (3)
src/apps/key-wallet/utils/blockchain.ts (1)
chains(32-34)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (17)
src/apps/pulse/components/Search/TokenList.tsx (3)
17-17: LGTM! Clean header visibility control.The
hideHeadersprop provides a clean way to reuseTokenListwith or without column headers, and the conditional rendering logic correctly restricts headers to Trending/Fresh views.Also applies to: 21-21, 60-126
174-190: LGTM! Chain badge logic correctly handles multi-chain assets.The conditional rendering shows the chain badge only for single-chain assets (
!item.allChains || item.allChains.length === 1), which correctly handles both legacy assets withoutallChainsand multi-chain assets where displaying a single badge would be misleading.
192-231: LGTM! Well-structured layout with proper text truncation.The row layout uses correct flex patterns (
flex-1 min-w-0) for text truncation, inline MCap/Vol display with semantic color coding, and a fixed-width price container for consistent alignment.src/apps/pulse/utils/parseSearchData.ts (3)
19-50: LGTM! Well-designed type extensions for multi-chain support.The Asset type extension with
id,allChains,allContracts, andallDecimalsproperly supports multi-chain asset aggregation. The new Market type is well-structured with all necessary fields for representing trading pairs.
52-153: Excellent refactoring for multi-chain asset handling.The refactored
parseAssetDataandparseTokenDatafunctions now correctly:
- Filter wrapped native tokens early
- Create a single Asset entry per symbol using the first valid chain as primary
- Store all valid chains in
allChains/allContracts/allDecimalsfor multi-chain selectionThis eliminates duplicate assets while preserving multi-chain support.
480-576: Excellent multi-chain aggregation inparseFreshAndTrendingTokens.The function correctly:
- Aggregates volume and market cap across chains for the same symbol
- Tracks the newest timestamp for Fresh sorting
- Builds
allChains/allContracts/allDecimalsarrays for multi-chain selection- Filters assets with zero volume or market cap
This provides accurate trending/fresh data across multiple chains.
src/apps/pulse/components/Search/ChainOverlay.tsx (3)
17-44: LGTM! The forwarded ref now correctly wraps both backdrop and card.The refactored structure addresses the previous review concern:
- The ref is applied to the outer container (line 27)
- Both backdrop and card are children of this container
- Search.tsx's click-outside handler will now treat backdrop clicks as "inside" the overlay
- The backdrop's onClick still closes the overlay itself while preventing Search modal closure
This correctly fixes the interaction issue.
47-54: LGTM! Chain filtering and sorting align with PR objectives.The chain list correctly:
- Filters out XDAI as specified in the PR requirements
- Places "All chains" at the top
- Sorts remaining chains alphabetically using
localeCompare
128-130: LGTM! Proper displayName for DevTools.The
displayNameassignment improves debugging experience in React DevTools.src/apps/pulse/components/Search/Search.tsx (8)
431-434: LGTM! Nullish coalescing now correctly preserves 0% price changes.The updated fallback logic uses
item.priceChange24h != nullinstead of a truthy check, which correctly handles zero values and only falls back to-0.02fornull/undefined. This fixes the data correctness issue from the previous review.Also applies to: 466-469
656-715: LGTM! Active tab detection now correctly uses enum comparison.The refactored logic maps
actualIndexto the correspondingSearchTypeenum value via thesearchTypesarray (lines 674-680) and compares withsearchType === typeForIndex(line 692). This fixes the brittle string-matching issue from the previous review.
22-22: LGTM! Clean mobile-responsive modal layout.The
useIsMobilehook drives appropriate layout changes:
- Mobile: full-screen fixed positioning
- Desktop: constrained dimensions with styling
This aligns with the PR's mobile-first objectives.
Also applies to: 515-529
329-357: LGTM! Well-implemented chain selection based on USDC balance.The
getChainWithMostUSDCfunction correctly:
- Filters USDC assets from the portfolio
- Finds the chain with the highest USDC balance
- Parses the chainId from the EIP-155 format
This provides intelligent default chain selection for multi-chain assets.
367-493: LGTM! Robust multi-chain asset selection logic.The enhanced
handleTokenSelectcorrectly:
- Detects multi-chain assets via
allChains.length > 1- Selects the chain with most USDC using
getChainWithMostUSDC- Falls back to the primary chain when USDC-based selection isn't available
- Handles both Asset and Token types with appropriate type guards
495-513: LGTM! Market selection correctly auto-selects the liquidity pool chain.The
handleMarketSelectfunction appropriately:
- Sets the buy token to
token0(the searched token)- Auto-selects the chain where the liquidity pool exists via
market.blockchainThis aligns with the PR's market integration objectives.
734-831: LGTM! Well-organized search results with Assets and Markets sections.The search results UI correctly:
- Renders sortable column headers at the top (lines 744-800)
- Groups results into Assets and Markets sections with counts
- Passes
hideHeaderstoTokenListto avoid duplicate headers- Uses
MarketListfor market pairsThis provides a clear, organized presentation of search results.
452-452: The hardcoded value is correct—Token types do not have priceChange24h.Verification confirms that the Token type (src/services/tokensData.ts:19-29) lacks a priceChange24h field entirely. Only the Asset type (src/apps/pulse/utils/parseSearchData.ts:19-37) has priceChange24h. The code correctly distinguishes the two cases: Asset branch uses nullish coalescing because the field exists; Token branch uses the hardcoded fallback because it does not. This is intentional and consistent behavior, not an inconsistency.
Likely an incorrect or invalid review comment.
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/apps/pulse/components/Search/tests/Search.test.tsx (1)
349-360: Skipped test should be fixed or removed.This test is skipped with
it.skip, likely because the close button behavior changed with the mobile UI refactor (replaced by back button on mobile). Either update the test to reflect the new UI or remove it if the functionality is covered elsewhere.Would you like me to help update this test to work with the new mobile back button, or verify if this functionality is already covered by other tests?
🧹 Nitpick comments (1)
src/apps/pulse/components/Search/Search.tsx (1)
290-323: Extract the duplicated trending/fresh fetch logic into a shared function.The fetch logic in
handleRefresh(lines 290-323) duplicates the fetch logic in theuseEffect(lines 262-288), including URL construction, parsing, and sorting. This violates DRY and creates a maintenance burden.Extract the fetch logic into a shared function:
+ const fetchTrendingOrFreshData = useCallback(() => { + if (!searchType || searchType === SearchType.MyHoldings) return; + + setIsLoading(true); + setParsedAssets(undefined); + fetch(`${getUrl(searchType)}?chainIds=${getChainId(chains)}`) + .then((response) => response.json()) + .then((result) => { + const assets = parseFreshAndTrendingTokens(result.projection); + + // Sort based on search type + if (searchType === SearchType.Trending) { + assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); + } else if (searchType === SearchType.Fresh) { + assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); + } + + setParsedAssets(assets); + setIsLoading(false); + }) + .catch((err) => { + setIsLoading(false); + console.error(err); + }); + }, [searchType, chains]); + useEffect(() => { - if (searchType) { - setIsLoading(true); - setParsedAssets(undefined); - fetch(`${getUrl(searchType)}?chainIds=${getChainId(chains)}`) - .then((response) => response.json()) - .then((result) => { - const assets = parseFreshAndTrendingTokens(result.projection); - - // Sort based on search type - if (searchType === SearchType.Trending) { - // Sort by volume (descending - highest first) - assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); - } else if (searchType === SearchType.Fresh) { - // Sort by timestamp (descending - newest first) - assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); - } - - setParsedAssets(assets); - setIsLoading(false); - }) - .catch((err) => { - setIsLoading(false); - console.error(err); - }); - } + fetchTrendingOrFreshData(); }, [searchType, chains]); // Comprehensive refresh handler const handleRefresh = () => { // Refetch wallet portfolio if available if (refetchWalletPortfolio) { refetchWalletPortfolio(); } - // Refetch Trending/Fresh/Top Gainers data if applicable - if (searchType && searchType !== SearchType.MyHoldings) { - setIsLoading(true); - setParsedAssets(undefined); - fetch(`${getUrl(searchType)}?chainIds=${getChainId(chains)}`) - .then((response) => response.json()) - .then((result) => { - const assets = parseFreshAndTrendingTokens(result.projection); - - // Sort based on search type - if (searchType === SearchType.Trending) { - // Sort by volume (descending - highest first) - assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); - } else if (searchType === SearchType.Fresh) { - // Sort by timestamp (descending - newest first) - assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); - } - - setParsedAssets(assets); - setIsLoading(false); - }) - .catch((err) => { - setIsLoading(false); - console.error(err); - }); - } + fetchTrendingOrFreshData(); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/apps/pulse/components/Search/tests/__snapshots__/Search.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/apps/pulse/components/Search/Search.tsx(11 hunks)src/apps/pulse/components/Search/tests/Search.test.tsx(6 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
📚 Learning: 2025-11-21T13:10:33.401Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/tests/Search.test.tsxsrc/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required text field. The TileTitleProps interface aligns with the TokensMarketData.title type in api.ts which also has optional fields.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Search/Search.tsx (9)
src/apps/pulse/utils/parseSearchData.ts (5)
Asset(19-37)Market(39-50)parseSearchData(299-392)filterMarketsByLiquidity(292-297)parseFreshAndTrendingTokens(480-576)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/media.ts (1)
useIsMobile(34-34)src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-35)src/apps/pulse/components/Search/ChainSelect.tsx (1)
ChainSelectButton(3-13)src/apps/pulse/components/Search/SearchSkeleton.tsx (1)
SearchSkeleton(7-44)src/apps/pulse/components/Search/Sort.tsx (1)
Sort(7-29)src/apps/pulse/components/Search/TokenList.tsx (1)
TokenList(20-240)src/apps/pulse/components/Search/MarketList.tsx (1)
MarketList(15-158)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (8)
src/apps/pulse/components/Search/Search.tsx (6)
325-357: LGTM: Multi-chain USDC detection logic is solid.The function correctly identifies USDC assets across chains, finds the chain with the highest balance, and properly parses the chain ID from the
eip155:Xformat.
367-493: Complex but correct multi-chain token selection logic.The function properly handles:
- Single vs. multi-chain assets
- Smart chain selection based on USDC balance
- Fallback to primary chain when needed
- Both Asset and Token types
The nullish checks for
priceChange24h(lines 431-434, 466-469) now correctly preserve0values, addressing the previous review concern.
495-513: LGTM: Market selection correctly sets buy token and chain.The function appropriately selects
token0as the buy token (the searched token) and automatically sets the chain where the liquidity pool exists.Based on learnings
519-714: Excellent mobile-responsive header implementation.The restructured UI properly handles:
- Mobile full-screen vs. desktop modal sizing
- Mobile-only back button
- Accessible search input with clear functionality
- Proper refresh button structure (no longer nested)
- Correct active tab detection using enum comparison
All previous review concerns about button nesting and active state detection have been addressed.
719-858: Well-structured scrollable results with proper loading states and section grouping.The implementation correctly:
- Shows loading skeleton during fetch
- Groups Assets and Markets in search results
- Applies sorting headers for Assets
- Uses
hideHeadersto avoid duplicate headers- Filters markets by liquidity threshold
- Handles Trending/Fresh/MyHoldings modes appropriately
861-870: LGTM: ChainOverlay properly integrated with ref forwarding.The overlay correctly uses the
refpattern and receives all necessary props for positioning and visibility control.src/apps/pulse/components/Search/tests/Search.test.tsx (2)
37-39: LGTM: Appropriate mock for mobile responsive testing.Mocking
useMediaQueryto returntrueensures the mobile UI path is tested.
144-144: Test selector updates correctly reflect UI changes.The updated text expectations and button selectors properly match the changes in Search.tsx:
- Added space in emoji label (lines 144, 250)
- Updated aria-label from "save" to "select blockchain" (line 293)
- Changed to more reliable
testIdselector for refresh button (line 307)Also applies to: 250-250, 293-293, 307-307
Also applies to: 250-250, 293-293, 307-307
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/apps/pulse/components/Search/Search.tsx (1)
105-119: Memoizelistto prevent unnecessary effect runs and sort resets.The inline computation of
listcreates a new object reference on every render, causinglist?.assetsin the dependency array to trigger the effect unnecessarily. This resets the sort state whenever the component re-renders for any reason, not just when search data changes.Wrap
listinuseMemo:+import React, { + Dispatch, + SetStateAction, + useEffect, + useMemo, + useRef, + useState, +} from 'react'; ... - let list: { assets: Asset[]; markets: Market[] } | undefined; - if (searchData?.result.data) { - list = parseSearchData(searchData?.result.data!, chains, searchText); - } + const list = useMemo(() => { + if (searchData?.result.data) { + return parseSearchData(searchData.result.data, chains, searchText); + } + return undefined; + }, [searchData?.result.data, chains, searchText]); // Update sorted assets when search results change useEffect(() => { if (list?.assets) { setSortedSearchAssets([...list.assets]); } else { setSortedSearchAssets(undefined); } // Reset sort when search changes setSearchSort({}); - }, [searchText, searchData, list?.assets]); + }, [list]);
🧹 Nitpick comments (1)
src/apps/pulse/components/Search/Search.tsx (1)
262-323: Extract duplicated sorting logic into a helper function.The sorting logic at lines 272-278 is duplicated in lines 307-313. Extract this into a reusable helper to follow DRY principles.
Add a helper function before the
useEffect:const sortAssetsBySearchType = (assets: Asset[], type: SearchType) => { if (type === SearchType.Trending) { assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); } else if (type === SearchType.Fresh) { assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); } };Then use it in both places:
.then((result) => { const assets = parseFreshAndTrendingTokens(result.projection); - - // Sort based on search type - if (searchType === SearchType.Trending) { - // Sort by volume (descending - highest first) - assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); - } else if (searchType === SearchType.Fresh) { - // Sort by timestamp (descending - newest first) - assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); - } - + sortAssetsBySearchType(assets, searchType); setParsedAssets(assets);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/apps/pulse/components/Search/Search.tsx(11 hunks)src/apps/pulse/components/Search/tests/Search.test.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/pulse/components/Search/tests/Search.test.tsx
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
📚 Learning: 2025-11-21T13:10:33.401Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required text field. The TileTitleProps interface aligns with the TokensMarketData.title type in api.ts which also has optional fields.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Search/Search.tsx (5)
src/apps/pulse/utils/parseSearchData.ts (5)
Asset(19-37)Market(39-50)parseSearchData(299-392)filterMarketsByLiquidity(292-297)parseFreshAndTrendingTokens(480-576)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/media.ts (1)
useIsMobile(34-34)src/apps/pulse/components/Search/TokenList.tsx (1)
TokenList(20-240)src/apps/pulse/components/Search/MarketList.tsx (1)
MarketList(15-158)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/apps/pulse/utils/parseSearchData.ts (1)
155-223: Chain filter inparseMarketPairsnow correctly applies the selected chainThe updated condition:
if (chains !== MobulaChainNames.All && pair.blockchain !== chains) { continue; }properly constrains markets to the selected chain, fixing the earlier behavior where all supported chains were included regardless of selection. The search‑term matching and re‑ordering logic (ensuring the searched token is token0 in
pairName) also look correct.If you care about preserving legitimate
0volumes from the API, you might later switchvolume24h: (pair as any).volume_24h || pair.volume24h || 0,to use nullish coalescing so that a real
0doesn’t get treated as “missing”:volume24h: (pair as any).volume_24h ?? pair.volume24h ?? 0;
🧹 Nitpick comments (2)
src/apps/pulse/utils/parseSearchData.ts (2)
19-37: Multi‑chain Asset/Market modeling and parsing look solid; consider DRYing common chain filteringThe extended
Asset/Marketshapes and the logic inparseAssetData/parseTokenDatato:
- filter to
MOBULA_CHAIN_NAMES,- respect the selected
chainsvalue,- and drop wrapped native tokens
are consistent with the rest of the codebase and the PR goals. The “single primary entry + multi‑chain metadata” approach fits the UI needs.
There is noticeable duplication between
parseAssetDataandparseTokenData(computingvalidChainIndices, choosingprimaryIndex, and building the multi‑chain fields). If both functions are going to stay, you might want to extract a small helper (e.g.buildMultiChainAsset(asset, validChainIndices, primaryIndex, overrides)) and share it between them to reduce maintenance overhead, but that’s purely a readability/DRY improvement.Also applies to: 39-50, 52-105, 107-153
225-241: Preserve 0 values forvolume24handpriceChange24hinparsePairResponseUsing
||on numeric fields will treat0as falsy and replace it:volume24h: pair.volume_24h || pair.volume24h, priceChange24h: pair.price_change_24h || null,If the API legitimately returns
0(no volume or no price change), this will currently get turned intopair.volume24hornull, losing information. Prefer??to only fall back onnull/undefined:- volume24h: pair.volume_24h || pair.volume24h, + volume24h: pair.volume_24h ?? pair.volume24h ?? 0, @@ - priceChange24h: pair.price_change_24h || null, + priceChange24h: pair.price_change_24h ?? null,This keeps “no change” (
0) distinct from “no data”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/pulse/utils/parseSearchData.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
📚 Learning: 2025-11-21T13:10:33.401Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/utils/parseSearchData.ts
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/utils/parseSearchData.ts
🧬 Code graph analysis (1)
src/apps/pulse/utils/parseSearchData.ts (5)
src/types/api.ts (7)
MobulaToken(506-522)Exchange(524-527)TokenAssetResponse(549-568)Asset(18-25)PairResponse(570-656)Projection(179-191)TokensMarketData(170-177)src/utils/blockchain.ts (2)
isWrappedNativeToken(514-521)getChainName(269-288)src/apps/pulse/utils/constants.ts (2)
MOBULA_CHAIN_NAMES(13-15)getChainName(51-70)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/apps/pulse/utils/number.ts (1)
parseNumberString(17-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (2)
src/apps/pulse/utils/parseSearchData.ts (2)
243-294: Sorting and liquidity filtering behavior aligns with search requirements
sortAssetsandsortMarketscorrectly prioritize:
- exact symbol matches (for assets) and exact token0 symbol matches (for markets), then
- fall back to descending
mCap(assets) and descendingliquidity(markets),which is a sensible relevance model for this UI.
filterMarketsByLiquidityis straightforward and safe, and using it separately keeps the thresholds configurable at the call site.Just be aware that
Array.prototype.sortmutates in place; in this file you only pass in freshly‑filtered arrays (filteredAssets,uniqueMarkets), so there’s no observable side‑effect right now.
296-389:parseSearchData’s asset/market aggregation and filtering match the stated specThe main
parseSearchDataflow looks coherent:
- Correctly distinguishes
assetvstokenvs rawPairResponse.- Only “asset” types feed the
assetscollection; tokens contribute markets viaparseMarketPairsas intended.- Applies chain filtering consistently through
parseAssetData/parseMarketPairs.- Deduplicates assets, filters out entries with zero/absent volume or mCap, and deduplicates markets by
address + blockchain.- Sorts assets and markets only when a non‑empty
searchTermis present, preserving original ordering otherwise.- AAVE debug logging is narrowly scoped to
searchTermcontainingaave, which keeps noise down.No functional issues stand out here; it’s a solid consolidation layer for the search UI.
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
aldin4u
force-pushed
the
feature/pulse-search-ui-mobile-refinements
branch
from
d070b8f to
c1148b1
Compare
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/apps/pulse/components/Search/Search.tsx (4)
418-418:dailyPriceChangefallback converts 0% to -2%.Lines 418 and 450 use
item.priceChange24h || -0.02, which treats a valid 0% price change as falsy and replaces it with -0.02 (-2%). This causes assets with 0% change to display as "-2% down".Use nullish coalescing instead:
- dailyPriceChange: 'priceChange24h' in item ? (item.priceChange24h || -0.02) : -0.02, + dailyPriceChange: + 'priceChange24h' in item && item.priceChange24h != null + ? item.priceChange24h + : -0.02,Apply this fix to both lines 418 and 450.
Note: This issue was previously flagged and marked as addressed in commit ddf887c, but it still appears in the current code.
Also applies to: 450-450
562-577: Nested buttons: outer button wrapsRefreshcomponent's button.The outer
<button>(lines 563-569) wraps aRefreshcomponent (line 571) which itself renders a<button>. Nested buttons are invalid HTML and cause inconsistent behavior.Replace the outer
<button>with a<div>:- {/* Refresh button */} - <button - onClick={handleRefresh} - disabled={walletPortfolioFetching || isLoading} - className="flex items-center justify-center w-10 h-10 bg-[#121116] rounded-[10px] flex-shrink-0 group p-0.5" - type="button" - data-testid="pulse-search-refresh-button" - > + {/* Refresh button */} + <div + className="flex items-center justify-center w-10 h-10 bg-[#121116] rounded-[10px] flex-shrink-0 group p-0.5" + data-testid="pulse-search-refresh-button" + > <div className="flex items-center justify-center w-full h-full bg-[#1E1D24] rounded-[8px] group-hover:bg-[#2A2A2A] transition-colors"> <Refresh isLoading={walletPortfolioFetching || isLoading} onClick={handleRefresh} disabled={walletPortfolioFetching || isLoading} /> </div> - </button> + </div>Note: This issue was previously flagged and marked as addressed in commit ddf887c, but it still appears in the current code.
602-644: Active tab detection fails for emoji-prefixed labels.Line 622 uses
item.includes(searchType)to determine if a tab is active. However, labels like'🔥 Trending'don't contain the enum valueSearchType.Trending, so this check always returns false for those tabs.Map the
actualIndexto the correspondingSearchTypeand compare:).map((item, index) => { // For sell screen, always map to MyHoldings index (3) const actualIndex = isBuy ? index : 3; + + const typeForIndex = + actualIndex === 0 + ? SearchType.Trending + : actualIndex === 1 + ? SearchType.Fresh + : actualIndex === 2 + ? SearchType.TopGainers + : SearchType.MyHoldings; if (!isBuy) { return ( <div key={item} className="flex items-center"> <p className="text-[13px] font-normal text-white tracking-[-0.26px] px-3"> {item} </p> </div> ); } - const isActive = searchType && item.includes(searchType); + const isActive = searchType === typeForIndex;Note: This issue was previously flagged and marked as addressed in commit ddf887c, but it still appears in the current code.
106-120: Memoizelistto prevent unnecessary recalculations and effect runs.The
listvariable is computed inline during render (lines 106-109), creating a new object reference on every render. The useEffect (lines 112-120) captureslistin its closure but doesn't include it in the dependency array, which can lead to stale closures. Additionally,listshould be memoized for performance.Wrap
listinuseMemo:+ const list = useMemo(() => { + if (searchData?.result.data) { + return parseSearchData(searchData.result.data, chains, searchText); + } + return undefined; + }, [searchData?.result.data, chains, searchText]); + - let list: { assets: Asset[]; markets: Market[] } | undefined; - if (searchData?.result.data) { - list = parseSearchData(searchData?.result.data!, chains, searchText); - } // Update sorted assets when search results change useEffect(() => { if (list?.assets) { setSortedSearchAssets([...list.assets]); } else { setSortedSearchAssets(undefined); } // Reset sort when search changes setSearchSort({}); - }, [searchText, searchData]); + }, [list]);Don't forget to import
useMemoat the top of the file.Based on learnings
🧹 Nitpick comments (3)
src/apps/pulse/components/Search/Search.tsx (3)
152-155: MemoizefilteredMarketsto avoid recalculating on every render.The
filteredMarketsvariable is computed on every render. For better performance, wrap it inuseMemo:- // Apply liquidity filter automatically - const filteredMarkets = list?.markets - ? filterMarketsByLiquidity(list.markets, MIN_LIQUIDITY_THRESHOLD) - : []; + // Apply liquidity filter automatically + const filteredMarkets = useMemo( + () => list?.markets + ? filterMarketsByLiquidity(list.markets, MIN_LIQUIDITY_THRESHOLD) + : [], + [list?.markets] + );
269-276: Extract duplicate sorting logic to a helper function.The sorting logic for Trending (by volume) and Fresh (by timestamp) is duplicated in two places: the initial fetch effect (lines 269-276) and the
handleRefreshfunction (lines 304-311).Extract to a helper function:
+ // Helper to sort assets based on search type + const sortAssetsByType = (assets: Asset[], type: SearchType) => { + if (type === SearchType.Trending) { + // Sort by volume (descending - highest first) + assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); + } else if (type === SearchType.Fresh) { + // Sort by timestamp (descending - newest first) + assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); + } + }; useEffect(() => { if (searchType) { setIsLoading(true); setParsedAssets(undefined); fetch(`${getUrl(searchType)}?chainIds=${getChainId(chains)}`) .then((response) => response.json()) .then((result) => { const assets = parseFreshAndTrendingTokens(result.projection); - - // Sort based on search type - if (searchType === SearchType.Trending) { - // Sort by volume (descending - highest first) - assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); - } else if (searchType === SearchType.Fresh) { - // Sort by timestamp (descending - newest first) - assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); - } + sortAssetsByType(assets, searchType); setParsedAssets(assets); setIsLoading(false); })Apply the same change in
handleRefresh(lines 304-311).Also applies to: 304-311
323-355: Consider usingreducefor cleaner chain selection logic.The nested
forEachloops with imperative variables work correctly but could be more functional and readable.const getChainWithMostUSDC = (): number | null => { if (!walletPortfolioData?.assets) return null; // Find all USDC assets in the portfolio const usdcAssets = walletPortfolioData.assets.filter( (asset) => asset.asset.symbol.toUpperCase() === 'USDC' || asset.asset.name.toLowerCase().includes('usd coin') ); if (usdcAssets.length === 0) return null; - // Find the chain with the highest USDC balance - let maxBalance = 0; - let bestChainId: number | null = null; - - usdcAssets.forEach((usdcAsset) => { - usdcAsset.contracts_balances.forEach((balance) => { - if (balance.balance > maxBalance) { - maxBalance = balance.balance; - // Extract chain ID from chainId string (format: "eip155:1") - const chainIdStr = balance.chainId.split(':')[1]; - bestChainId = parseInt(chainIdStr, 10); - } - }); - }); - - return bestChainId; + // Find the chain with the highest USDC balance + const bestBalance = usdcAssets + .flatMap((asset) => asset.contracts_balances) + .reduce<{ balance: number; chainId: number | null }>( + (best, balance) => { + if (balance.balance > best.balance) { + const chainIdStr = balance.chainId.split(':')[1]; + return { balance: balance.balance, chainId: parseInt(chainIdStr, 10) }; + } + return best; + }, + { balance: 0, chainId: null } + ); + + return bestBalance.chainId; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/apps/pulse/components/Search/tests/__snapshots__/Search.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/apps/pulse/components/Misc/Refresh.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/pulse/components/Misc/Refresh.tsx
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
📚 Learning: 2025-11-21T13:10:33.401Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Search/Search.tsx (10)
src/apps/pulse/utils/parseSearchData.ts (5)
Asset(20-38)Market(40-51)parseSearchData(300-391)filterMarketsByLiquidity(293-298)parseFreshAndTrendingTokens(453-543)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/number.tsx (2)
formatExponentialSmallNumber(119-149)limitDigitsNumber(42-69)src/utils/media.ts (1)
useIsMobile(34-34)src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-35)src/apps/pulse/components/Search/ChainSelect.tsx (1)
ChainSelectButton(3-13)src/apps/pulse/components/Search/SearchSkeleton.tsx (1)
SearchSkeleton(7-45)src/apps/pulse/components/Search/Sort.tsx (1)
Sort(7-29)src/apps/pulse/components/Search/TokenList.tsx (1)
TokenList(20-228)src/apps/pulse/components/Search/MarketList.tsx (1)
MarketList(17-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (5)
src/apps/pulse/components/Search/Search.tsx (5)
505-508: Mobile-responsive modal implementation looks great!The conditional styling properly implements full-screen mobile view (
fixed inset-0) and constrained desktop view (w-[446px] h-[512px]), matching the PR objectives for mobile responsiveness.
514-600: Fixed header implementation is well-structured.The header section properly adapts to mobile with:
- Conditional back button for mobile (lines 516-529)
- Inline search input with clear button (lines 531-560)
- Conditional chain selector for buy mode only (lines 579-600)
The flex layout and responsive design align well with the PR objectives.
664-755: Search results rendering with grouped sections is excellent.The implementation properly:
- Groups results into Assets and Markets sections with counts
- Provides inline sorting controls for search results (lines 670-724)
- Uses the new
hideHeadersprop to avoid duplicate headers- Conditionally renders sections based on data availability
This matches the PR objectives for improved data presentation.
783-794: ChainOverlay withforwardRefpattern is correctly implemented.The conditional rendering and ref forwarding (line 786) allow proper click-outside detection, addressing the click-handling conflicts mentioned in the PR objectives.
476-494: Market selection handler is well-implemented.The
handleMarketSelectfunction correctly:
- Sets the buy token using
market.token0data- Auto-selects the chain where the liquidity pool exists
- Properly formats the price and handles fallbacks
- Closes the search modal and cleans up state
This aligns with the PR's multi-chain market selection feature.
aldin4u
force-pushed
the
feature/pulse-search-ui-mobile-refinements
branch
from
75aa141 to
c1148b1
Compare
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
- Added min-w-0 to search input on mobile to prevent overflow
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/apps/pulse/components/Search/Search.tsx (4)
106-120:listcomputed inline causes the effect to rely on stale closure or miss updates.The
listvariable is computed during render as a new object each time, but theuseEffectat lines 112-120 depends onsearchTextandsearchDatawhile usinglist?.assets. This creates a mismatch: the effect won't see updates tolistderived fromchainschanges, and the inline computation is inefficient.Wrap
listinuseMemoso it's stable across renders:+ const list = useMemo(() => { + if (searchData?.result.data) { + return parseSearchData(searchData.result.data, chains, searchText); + } + return undefined; + }, [searchData?.result.data, chains, searchText]); + - let list: { assets: Asset[]; markets: Market[] } | undefined; - if (searchData?.result.data) { - list = parseSearchData(searchData?.result.data!, chains, searchText); - } // Update sorted assets when search results change useEffect(() => { if (list?.assets) { setSortedSearchAssets([...list.assets]); } else { setSortedSearchAssets(undefined); } // Reset sort when search changes setSearchSort({}); - }, [searchText, searchData]); + }, [list]);Don't forget to add
useMemoto the imports at the top of the file.
418-422:dailyPriceChangefallback converts valid 0% change into fake ‑2% change.Both the buy and sell branches use:
dailyPriceChange: 'priceChange24h' in item ? (item.priceChange24h || -0.02) : -0.02,The
|| -0.02operator treatspriceChange24h === 0as falsy and replaces it with-0.02, so assets with 0% change will incorrectly render as "‑2% down".Use a nullish check to preserve 0:
- dailyPriceChange: 'priceChange24h' in item ? (item.priceChange24h || -0.02) : -0.02, + dailyPriceChange: + 'priceChange24h' in item && item.priceChange24h != null + ? item.priceChange24h + : -0.02,- dailyPriceChange: 'priceChange24h' in item ? (item.priceChange24h || -0.02) : -0.02, + dailyPriceChange: + 'priceChange24h' in item && item.priceChange24h != null + ? item.priceChange24h + : -0.02,Also applies to: 450-451
562-576: Nested<button>elements create invalid HTML.The outer wrapper is a
<button>at line 562, and the innerRefreshcomponent (per the code snippets) also renders a<button>. Nested<button>elements are invalid HTML and can cause inconsistent focus, keyboard, and click behavior across browsers.Change the outer wrapper to a non-interactive container:
- <button - onClick={handleRefresh} - disabled={walletPortfolioFetching || isLoading} - className="flex items-center justify-center w-10 h-10 bg-[#121116] rounded-[10px] flex-shrink-0 group p-0.5" - type="button" - data-testid="pulse-search-refresh-button" - > + <div + className="flex items-center justify-center w-10 h-10 bg-[#121116] rounded-[10px] flex-shrink-0 group p-0.5" + > <div className="flex items-center justify-center w-full h-full bg-[#1E1D24] rounded-[8px] group-hover:bg-[#2A2A2A] transition-colors"> <Refresh isLoading={walletPortfolioFetching || isLoading} onClick={handleRefresh} disabled={walletPortfolioFetching || isLoading} /> </div> - </button> + </div>Move the
data-testidto the innerRefreshcomponent if needed for testing.
621-621: Active tab detection viaitem.includes(searchType)will always be false for emoji-prefixed labels.The active state check at line 621:
const isActive = searchType && item.includes(searchType);This attempts to find the enum value (e.g.,
SearchType.Trending) within the label string (e.g.,'🔥 Trending'). The enum values won't literally appear in these emoji-prefixed labels, soisActivewill always befalseand tabs will never visually appear active.Derive the active state from the index/enum mapping:
).map((item, index) => { // For sell screen, always map to MyHoldings index (3) const actualIndex = isBuy ? index : 3; + + const typeForIndex = + actualIndex === 0 + ? SearchType.Trending + : actualIndex === 1 + ? SearchType.Fresh + : actualIndex === 2 + ? SearchType.TopGainers + : SearchType.MyHoldings; if (!isBuy) { return ( ... ); } - const isActive = searchType && item.includes(searchType); + const isActive = searchType === typeForIndex;
🧹 Nitpick comments (3)
src/apps/pulse/components/Search/Search.tsx (3)
152-156: Consider memoizingfilteredMarketsto avoid repeated filtering on every render.
filteredMarketsis computed inline and will runfilterMarketsByLiquidityon every render, even whenlist.marketshasn't changed. For better performance, wrap it inuseMemo:- const filteredMarkets = list?.markets - ? filterMarketsByLiquidity(list.markets, MIN_LIQUIDITY_THRESHOLD) - : []; + const filteredMarkets = useMemo( + () => + list?.markets + ? filterMarketsByLiquidity(list.markets, MIN_LIQUIDITY_THRESHOLD) + : [], + [list?.markets] + );
288-321: Extract duplicated fetch logic into a shared function.The
handleRefreshfunction duplicates the entire fetch and parse logic from theuseEffectat lines 260-286. This creates a maintenance burden—any changes to the fetching, parsing, or sorting must be replicated in both places.Extract the common logic into a reusable function:
+ const fetchAndParseTrendingFresh = useCallback( + (type: SearchType) => { + setIsLoading(true); + setParsedAssets(undefined); + fetch(`${getUrl(type)}?chainIds=${getChainId(chains)}`) + .then((response) => response.json()) + .then((result) => { + const assets = parseFreshAndTrendingTokens(result.projection); + + // Sort based on search type + if (type === SearchType.Trending) { + assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); + } else if (type === SearchType.Fresh) { + assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); + } + + setParsedAssets(assets); + setIsLoading(false); + }) + .catch((err) => { + setIsLoading(false); + console.error(err); + }); + }, + [chains] + );Then replace both usages with calls to this function.
486-486: Use nullish coalescing fordailyPriceChangeto handle 0 explicitly.Line 486 uses
market.priceChange24h || 0, which treats a validpriceChange24h === 0as falsy. While the fallback is also0(so the result is correct), this pattern is inconsistent and may confuse readers.Use nullish coalescing for clarity:
- dailyPriceChange: market.priceChange24h || 0, + dailyPriceChange: market.priceChange24h ?? 0,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/pulse/components/Search/Search.tsx(11 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
📚 Learning: 2025-11-21T13:10:33.401Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Search/Search.tsx (9)
src/apps/pulse/utils/parseSearchData.ts (5)
Asset(20-38)Market(40-51)parseSearchData(300-391)filterMarketsByLiquidity(293-298)parseFreshAndTrendingTokens(453-543)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/media.ts (1)
useIsMobile(34-34)src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-35)src/apps/pulse/components/Search/ChainSelect.tsx (1)
ChainSelectButton(3-13)src/apps/pulse/components/Search/SearchSkeleton.tsx (1)
SearchSkeleton(7-45)src/apps/pulse/components/Search/Sort.tsx (1)
Sort(7-29)src/apps/pulse/components/Search/TokenList.tsx (1)
TokenList(20-228)src/apps/pulse/components/Search/MarketList.tsx (1)
MarketList(17-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/apps/pulse/components/Search/tests/Search.test.tsx (1)
343-354: Skipped test needs updating or removal, not just disabling.This test is skipped without explanation or a tracking issue. Based on the PR changes introducing mobile-responsive behavior with a back button replacing the close button, this test likely needs to be rewritten rather than disabled.
Either update the test to cover the new close/back button behavior for both mobile and desktop viewports, or remove it entirely if the functionality is covered elsewhere. If keeping it skipped temporarily, add a TODO comment explaining why and link to a tracking issue.
Would you like me to generate an updated test that covers both the mobile back button and desktop close behavior?
♻️ Duplicate comments (4)
src/apps/pulse/components/Search/Search.tsx (4)
106-120:listcomputed inline causes unnecessary effect runs and sort resets.The
listobject is computed during render (lines 106-109), creating a new reference on every render even whensearchDatahasn't changed. This causes theuseEffect(lines 112-120) to run unnecessarily, resetting the sort state whenever the component re-renders for any reason.Wrap the computation in
useMemoto stabilize the reference:+ const list = useMemo(() => { + if (searchData?.result.data) { + return parseSearchData(searchData.result.data, chains, searchText); + } + return undefined; + }, [searchData?.result.data, chains, searchText]); + - let list: { assets: Asset[]; markets: Market[] } | undefined; - if (searchData?.result.data) { - list = parseSearchData(searchData?.result.data!, chains, searchText); - } // Update sorted assets when search results change useEffect(() => { if (list?.assets) { setSortedSearchAssets([...list.assets]); } else { setSortedSearchAssets(undefined); } // Reset sort when search changes setSearchSort({}); - }, [searchText, searchData]); + }, [list]);Don't forget to import
useMemofrom React.
418-422:dailyPriceChangefallback converts genuine 0% change to fake ‑2%.Lines 418 and 450 use
|| -0.02, which treatspriceChange24h === 0as falsy and replaces it with-0.02. This means assets with 0% change will display as "‑2% down"—a data correctness issue.Use a nullish check to preserve 0:
- dailyPriceChange: 'priceChange24h' in item ? (item.priceChange24h || -0.02) : -0.02, + dailyPriceChange: + 'priceChange24h' in item && item.priceChange24h != null + ? item.priceChange24h + : -0.02,Apply the same fix to line 450.
562-577: Nested<button>elements are invalid HTML.The outer wrapper (line 563) is a
<button>, and theRefreshcomponent (lines 571-575) also renders a<button>. Nested buttons violate HTML spec and cause inconsistent focus, keyboard, and click behavior across browsers.Change the outer wrapper to a non-interactive container:
- {/* Refresh button */} - <button - onClick={handleRefresh} - disabled={walletPortfolioFetching || isLoading} - className="flex items-center justify-center w-10 h-10 bg-[#121116] rounded-[10px] flex-shrink-0 group p-0.5" - type="button" - data-testid="pulse-search-refresh-button" - > + {/* Refresh button */} + <div + className="flex items-center justify-center w-10 h-10 bg-[#121116] rounded-[10px] flex-shrink-0 group p-0.5" + data-testid="pulse-search-refresh-button" + > <div className="flex items-center justify-center w-full h-full bg-[#1E1D24] rounded-[8px] group-hover:bg-[#2A2A2A] transition-colors"> <Refresh isLoading={walletPortfolioFetching || isLoading} onClick={handleRefresh} disabled={walletPortfolioFetching || isLoading} /> </div> - </button> + </div>This keeps the visual treatment while centralizing interactivity in the inner
Refreshbutton.
602-644: Active tab detection viaitem.includes(searchType)is broken.Line 622 computes
isActiveassearchType && item.includes(searchType). Sinceitemcontains emoji-prefixed labels like'🚀 Top Gainers'andsearchTypeis an enum value likeSearchType.TopGainers, the substring check will always fail—so tabs never appear active even when selected.Derive active state from the index mapping you already use:
).map((item, index) => { // For sell screen, always map to MyHoldings index (3) const actualIndex = isBuy ? index : 3; + + const typeForIndex = + actualIndex === 0 + ? SearchType.Trending + : actualIndex === 1 + ? SearchType.Fresh + : actualIndex === 2 + ? SearchType.TopGainers + : SearchType.MyHoldings; if (!isBuy) { return ( <div key={item} className="flex items-center"> <p className="text-[13px] font-normal text-white tracking-[-0.26px] px-3"> {item} </p> </div> ); } - const isActive = searchType && item.includes(searchType); + const isActive = searchType === typeForIndex;This ensures visual state stays in sync with
handleSearchTypeChange.
🧹 Nitpick comments (3)
src/apps/pulse/components/Search/Search.tsx (3)
260-321: Extract duplicated sorting logic into a helper function.The asset sorting logic (lines 270-276 and 304-311) is duplicated in both the
useEffectandhandleRefresh. Extract this into a reusable helper to follow DRY principles:+ const sortAssetsByType = (assets: Asset[], type: SearchType) => { + if (type === SearchType.Trending) { + assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); + } else if (type === SearchType.Fresh) { + assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); + } + }; + useEffect(() => { if (searchType) { setIsLoading(true); setParsedAssets(undefined); fetch(`${getUrl(searchType)}?chainIds=${getChainId(chains)}`) .then((response) => response.json()) .then((result) => { const assets = parseFreshAndTrendingTokens(result.projection); - - // Sort based on search type - if (searchType === SearchType.Trending) { - // Sort by volume (descending - highest first) - assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); - } else if (searchType === SearchType.Fresh) { - // Sort by timestamp (descending - newest first) - assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); - } - + sortAssetsByType(assets, searchType); setParsedAssets(assets); setIsLoading(false); })Apply the same change to
handleRefreshat lines 304-311.
327-355: Add validation for chainId parsing to prevent NaN.Line 349 uses
parseIntwithout validating the result. If thechainIdformat is unexpected or parsing fails,bestChainIdcould be set toNaN, causing incorrect chain selection logic.Add validation:
const chainIdStr = balance.chainId.split(':')[1]; - bestChainId = parseInt(chainIdStr, 10); + const parsed = parseInt(chainIdStr, 10); + if (!Number.isNaN(parsed)) { + bestChainId = parsed; + }
365-474: Consider extracting multi-chain selection logic for readability.The
handleTokenSelectfunction has grown to ~110 lines with complex nested conditionals for multi-chain asset selection (lines 367-409). This reduces readability and makes testing harder.Consider extracting the chain/contract/decimals selection logic into a helper function:
const selectChainForAsset = ( item: Asset, chainWithMostUSDC: number | null ): { chainId: number; contract: string; decimals: number } => { // Multi-chain selection logic here // ... return { chainId: selectedChainId, contract: selectedContract, decimals: selectedDecimals }; };Then call it from
handleTokenSelect:const { chainId, contract, decimals } = selectChainForAsset(item, getChainWithMostUSDC());This would make the function easier to understand, test, and maintain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/apps/pulse/components/Search/Search.tsx(11 hunks)src/apps/pulse/components/Search/tests/Search.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
📚 Learning: 2025-11-21T13:10:33.401Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.401Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Search/Search.tsx (9)
src/apps/pulse/utils/parseSearchData.ts (5)
Asset(20-38)Market(40-51)parseSearchData(300-391)filterMarketsByLiquidity(293-298)parseFreshAndTrendingTokens(453-543)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/media.ts (1)
useIsMobile(34-34)src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-35)src/apps/pulse/components/Search/ChainSelect.tsx (1)
ChainSelectButton(3-13)src/apps/pulse/components/Search/SearchSkeleton.tsx (1)
SearchSkeleton(7-45)src/apps/pulse/components/Search/Sort.tsx (1)
Sort(7-29)src/apps/pulse/components/Search/TokenList.tsx (1)
TokenList(20-228)src/apps/pulse/components/Search/MarketList.tsx (1)
MarketList(17-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (1)
src/apps/pulse/components/Search/Search.tsx (1)
498-796: Mobile-responsive UI implementation looks solid.The mobile-aware layout with conditional rendering based on
isMobile, the new back button for mobile viewports, redesigned search input, and grouped Assets/Markets sections all align well with the PR objectives. The full-screen modal for mobile (<768px) and the responsive ChainOverlay integration are well-implemented.The structure effectively separates the fixed header (search bar, filters) from the scrollable results section, providing a good mobile UX.
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
github-actions
bot
temporarily deployed
to
Preview (PRO-3733-OnboardingFlow)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/apps/pulse/components/Search/Search.tsx (3)
423-435:dailyPriceChangefallback still turns real 0% moves into fake ‑2% movesBoth in the buy‑asset and sell flows you still have:
dailyPriceChange: 'priceChange24h' in item ? item.priceChange24h || -0.02 : -0.02,This treats a legitimate
priceChange24h === 0as falsy and replaces it with-0.02, so flat assets render as “‑2% down”, which is a data correctness issue.Use a nullish check so 0 is preserved and only
null/undefinedfall back:- dailyPriceChange: - 'priceChange24h' in item ? item.priceChange24h || -0.02 : -0.02, + dailyPriceChange: + 'priceChange24h' in item && item.priceChange24h != null + ? item.priceChange24h + : -0.02, @@ - dailyPriceChange: - 'priceChange24h' in item ? item.priceChange24h || -0.02 : -0.02, + dailyPriceChange: + 'priceChange24h' in item && item.priceChange24h != null + ? item.priceChange24h + : -0.02,Also applies to: 456-467
650-694: Tab active state based onitem.includes(searchType)is brittle
isActiveis computed as:const isActive = searchType && item.includes(searchType);For labels like
'🚀 Top Gainers'/'💰 My Holdings'and enum values likeSearchType.TopGainers/SearchType.MyHoldings, the enum string is unlikely to literally occur in the label (spacing/emojis), so those tabs may never be marked active even when selected.More robust to derive from the index mapping you already use:
- ).map((item, index) => { - // For sell screen, always map to MyHoldings index (3) - const actualIndex = isBuy ? index : 3; + ).map((item, index) => { + // For sell screen, always map to MyHoldings index (3) + const actualIndex = isBuy ? index : 3; + + const typeForIndex = + actualIndex === 0 + ? SearchType.Trending + : actualIndex === 1 + ? SearchType.Fresh + : actualIndex === 2 + ? SearchType.TopGainers + : SearchType.MyHoldings; @@ - const isActive = searchType && item.includes(searchType); + const isActive = searchType === typeForIndex;This keeps the visual active state aligned with
handleSearchTypeChangeregardless of label text.
605-629: Based on my thorough verification, I can now provide the final rewritten review comment:
Chain selector
divlacks keyboard handling despiterole="button"The chain selector has proper accessibility attributes (
role="button",tabIndex={0},aria-label), but keyboard users cannot activate it—only mouse users can viaonClick. This violates WCAG 2.1 Level A keyboard accessibility (2.1.1). Native<button>elements automatically handle Enter and Space keys, but this div requires explicit handlers.Add an
onKeyDownhandler that mirrors the click logic:<div ref={chainButtonRef} onClick={() => { const rect = chainButtonRef?.current?.getBoundingClientRect(); if (rect) { setShowChainOverlay(true); setOverlayStyle({ ...overlayStyling, top: `${rect.bottom + 5}px`, left: `${rect.right - 200}px`, }); } }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + const rect = chainButtonRef?.current?.getBoundingClientRect(); + if (rect) { + setShowChainOverlay(true); + setOverlayStyle({ + ...overlayStyling, + top: `${rect.bottom + 5}px`, + left: `${rect.right - 200}px`, + }); + } + } + }}
🧹 Nitpick comments (3)
src/apps/pulse/components/Search/Search.tsx (3)
2-4: Scope down global ESLint disables to specific lines/blocksDisabling
jsx-a11y/control-has-associated-label,no-nested-ternary, andreact-hooks/exhaustive-depsat file scope hides genuine issues elsewhere. Prefer targeted// eslint-disable-next-line ...on the few lines that truly need them to keep lint signal useful.
88-121: Memoizelist/markets to avoid re‑parsing on every render and tighten effect deps
parseSearchDatais called on every render to computelist, andfilteredMarketsis derived directly from that locallist. This means simple state changes (like sorting) re-run the parsing and liquidity filtering even whensearchData/chains/searchTexthaven’t changed.Consider memoizing once per data change and keying the effect on that memo:
+ const list = React.useMemo(() => { + if (searchData?.result.data) { + return parseSearchData(searchData.result.data, chains, searchText); + } + return undefined; + }, [searchData?.result.data, chains, searchText]); - - let list: { assets: Asset[]; markets: Market[] } | undefined; - if (searchData?.result.data) { - list = parseSearchData(searchData?.result.data!, chains, searchText); - } @@ - }, [searchText, searchData]); + }, [list]); @@ - const filteredMarkets = list?.markets - ? filterMarketsByLiquidity(list.markets, MIN_LIQUIDITY_THRESHOLD) - : []; + const filteredMarkets = React.useMemo( + () => + list?.markets + ? filterMarketsByLiquidity(list.markets, MIN_LIQUIDITY_THRESHOLD) + : [], + [list, MIN_LIQUIDITY_THRESHOLD] + );This keeps parsing and filtering aligned with the actual data inputs and avoids extra work on unrelated re-renders.
Also applies to: 152-156
261-287: DRY up duplicated Trending/Fresh fetch logicThe effect for
searchTypechanges andhandleRefreshboth implement near-identical fetch / parse / sort / setParsedAssets logic. This is correct but harder to maintain.Consider extracting a small helper (e.g.,
fetchFreshOrTrending(searchType, chains)) and call it from both the effect andhandleRefreshto keep behavior in sync and reduce future drift.Also applies to: 289-322
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/apps/pulse/components/Search/tests/__snapshots__/Search.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
src/apps/pulse/components/Search/Search.tsx(11 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
📚 Learning: 2025-11-21T13:10:33.422Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages: pillarx-debug
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/apps/pulse/components/Search/Search.tsx (3)
102-117: Stalelistin effect and repeated parsing suggest usinguseMemo
listis recomputed on every render viaparseSearchData, but the effect depends only onsearchTextandsearchDataand intentionally ignoreslist(ESLint disabled). This has two downsides:
parseSearchDataruns on every render even when inputs haven’t changed.- The effect’s dependency set is indirect/fragile (it doesn’t explicitly depend on
chainsor on the parsed result), which can make future changes error-prone.Consider memoizing
listand driving the effect off the memoized value so you can drop the eslint-disable and avoid redundant parsing:-import React, { - Dispatch, - SetStateAction, - useEffect, - useRef, - useState, -} from 'react'; +import React, { + Dispatch, + SetStateAction, + useEffect, + useMemo, + useRef, + useState, +} from 'react'; @@ - let list: { assets: Asset[]; markets: Market[] } | undefined; - if (searchData?.result.data) { - list = parseSearchData(searchData.result.data, chains, searchText); - } + const list = useMemo( + () => + searchData?.result.data + ? parseSearchData(searchData.result.data, chains, searchText) + : undefined, + [searchData?.result.data, chains, searchText] + ); @@ - useEffect(() => { - if (list?.assets) { - setSortedSearchAssets([...list.assets]); - } else { - setSortedSearchAssets(undefined); - } - // Reset sort when search changes - setSearchSort({}); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [searchText, searchData]); + useEffect(() => { + if (list?.assets) { + setSortedSearchAssets([...list.assets]); + } else { + setSortedSearchAssets(undefined); + } + // Reset sort when search changes + setSearchSort({}); + }, [list]);This keeps the sort-reset logic tied directly to the parsed search result and removes the need for suppressing lint.
421-433:dailyPriceChangefallback still turns real 0% into fake -2% move (buy & sell)In both buy and sell token selection you still have:
dailyPriceChange: 'priceChange24h' in item ? item.priceChange24h || -0.02 : -0.02,If
item.priceChange24h === 0, the|| -0.02treats it as falsy and replaces it with-0.02, so a true 0% move is displayed as “-2%”. That’s a data correctness issue for both Buy and Sell flows.Prefer a nullish check so only
null/undefinedfall back:- dailyPriceChange: - 'priceChange24h' in item ? item.priceChange24h || -0.02 : -0.02, + dailyPriceChange: + 'priceChange24h' in item && item.priceChange24h != null + ? item.priceChange24h + : -0.02, @@ - dailyPriceChange: - 'priceChange24h' in item ? item.priceChange24h || -0.02 : -0.02, + dailyPriceChange: + 'priceChange24h' in item && item.priceChange24h != null + ? item.priceChange24h + : -0.02,This preserves legitimate 0% moves while still providing a fallback when the field is missing.
Also applies to: 461-462
645-695: Filter tab active state logic is still brittle and likely always falseThe active state is computed as:
const isActive = searchType && item.includes(searchType);For labels like
'🔥 Trending'and'💰 My Holdings'and enum values likeSearchType.Trending/SearchType.MyHoldings, the enum strings won’t literally appear in the label, soincludeswill almost always be false. Users won’t see any tab visually selected even whensearchTypeis set.You already map indices to
SearchTypeinhandleSearchTypeChange; re-use that mapping here:).map((item, index) => { // For sell screen, always map to MyHoldings index (3) const actualIndex = isBuy ? index : 3; @@ - const isActive = searchType && item.includes(searchType); + const typeForIndex = + actualIndex === 0 + ? SearchType.Trending + : actualIndex === 1 + ? SearchType.Fresh + : actualIndex === 2 + ? SearchType.TopGainers + : SearchType.MyHoldings; + + const isActive = searchType === typeForIndex;This guarantees the visual active state stays in sync with
handleSearchTypeChangeregardless of label text or emojis.
🧹 Nitpick comments (3)
src/apps/pulse/components/Search/Search.tsx (3)
259-320: Trending/Fresh fetch logic duplicated inuseEffectandhandleRefreshThe fetch + parse + sort logic for Trending/Fresh is nearly identical in the
useEffectand inhandleRefresh, differing mainly in the early guard onsearchType:setIsLoading(true); setParsedAssets(undefined); fetch(`${getUrl(searchType)}?chainIds=${getChainId(chains)}`) .then(...) .catch(...);Duplicating this increases the risk of drift if sort order or URL params change later.
Consider extracting a small helper, e.g.
loadFreshOrTrending(searchType, chains), and reusing it in both places:+ const loadFreshOrTrending = (type: SearchType) => { + setIsLoading(true); + setParsedAssets(undefined); + fetch(`${getUrl(type)}?chainIds=${getChainId(chains)}`) + .then((response) => response.json()) + .then((result) => { + const assets = parseFreshAndTrendingTokens(result.projection); + if (type === SearchType.Trending) { + assets.sort((a, b) => (b.volume || 0) - (a.volume || 0)); + } else if (type === SearchType.Fresh) { + assets.sort((a, b) => (b.timestamp || 0) - (a.timestamp || 0)); + } + setParsedAssets(assets); + setIsLoading(false); + }) + .catch((err) => { + setIsLoading(false); + console.error(err); + }); + }; @@ - useEffect(() => { - if (searchType) { - setIsLoading(true); - setParsedAssets(undefined); - fetch(`${getUrl(searchType)}?chainIds=${getChainId(chains)}`) - .then((response) => response.json()) - .then((result) => { - const assets = parseFreshAndTrendingTokens(result.projection); - // Sort based on search type - ... - setParsedAssets(assets); - setIsLoading(false); - }) - .catch((err) => { - setIsLoading(false); - console.error(err); - }); - } - }, [searchType, chains]); + useEffect(() => { + if (searchType && searchType !== SearchType.MyHoldings) { + loadFreshOrTrending(searchType); + } + }, [searchType, chains]); @@ - const handleRefresh = () => { + const handleRefresh = () => { // Refetch wallet portfolio if available ... - if (searchType && searchType !== SearchType.MyHoldings) { - setIsLoading(true); - setParsedAssets(undefined); - fetch(`${getUrl(searchType)}?chainIds=${getChainId(chains)}`) - .then((response) => response.json()) - .then((result) => { - const assets = parseFreshAndTrendingTokens(result.projection); - // Sort based on search type - ... - setParsedAssets(assets); - setIsLoading(false); - }) - .catch((err) => { - setIsLoading(false); - console.error(err); - }); - } + if (searchType && searchType !== SearchType.MyHoldings) { + loadFreshOrTrending(searchType); + } };This keeps the behaviour consistent and easier to maintain.
326-354: Defensive handling for unexpectedchainIdformat ingetChainWithMostUSDC
getChainWithMostUSDCassumesbalance.chainIdis always of the form"eip155:1"and blindly doessplit(':')[1]. If the format ever changes (e.g., missing colon, different namespace),chainIdStrbecomesundefinedandparseIntwill yieldNaN, which then flows intobestChainIdand later comparisons.Given this is portfolio-driven logic that affects chain selection:
- Guard against malformed
chainIdbefore parsing.- Optionally skip balances with invalid chain IDs.
For example:
- // Extract chain ID from chainId string (format: "eip155:1") - const chainIdStr = balance.chainId.split(':')[1]; - bestChainId = parseInt(chainIdStr, 10); + // Extract chain ID from chainId string (expected format: "eip155:1") + const parts = balance.chainId?.split(':'); + if (parts?.length === 2) { + const parsed = Number.parseInt(parts[1], 10); + if (!Number.isNaN(parsed)) { + bestChainId = parsed; + } + }This keeps the heuristic robust against any unexpected data shape from the portfolio API.
606-629: Chain selector wrapper lacks keyboard handling forrole="button"The chain selector wrapper is now a
<div>withrole="button"andtabIndex={0}, which is good, but keyboard activation (Enter/Space) is not handled. Currently only mouse clicks will open the chain overlay.To make it keyboard accessible and closer to native
<button>behaviour:{isBuy ? ( <div ref={chainButtonRef} - onClick={() => { + onClick={() => { const rect = chainButtonRef?.current?.getBoundingClientRect(); if (rect) { setShowChainOverlay(true); setOverlayStyle({ ...overlayStyling, top: `${rect.bottom + 5}px`, left: `${rect.right - 200}px`, }); } }} @@ role="button" tabIndex={0} aria-label="Select blockchain" data-testid="pulse-search-chain-selector" + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + const rect = + chainButtonRef?.current?.getBoundingClientRect(); + if (rect) { + setShowChainOverlay(true); + setOverlayStyle({ + ...overlayStyling, + top: `${rect.bottom + 5}px`, + left: `${rect.right - 200}px`, + }); + } + } + }} >This keeps the positioning logic but makes the control usable via keyboard as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/apps/pulse/components/Search/MarketList.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/pulse/components/Search/MarketList.tsx
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
📚 Learning: 2025-11-21T13:10:33.422Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Search/Search.tsx (9)
src/apps/pulse/utils/parseSearchData.ts (2)
Market(39-50)parseSearchData(365-419)src/apps/pulse/utils/constants.ts (1)
getChainId(28-49)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/media.ts (1)
useIsMobile(34-34)src/apps/pulse/components/Search/ChainSelect.tsx (1)
ChainSelectButton(3-13)src/apps/pulse/components/Search/SearchSkeleton.tsx (1)
SearchSkeleton(7-45)src/apps/pulse/components/Search/Sort.tsx (1)
Sort(7-29)src/apps/pulse/components/Search/TokenList.tsx (1)
TokenList(20-223)src/apps/pulse/components/Search/MarketList.tsx (1)
MarketList(14-99)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
- GitHub Check: Cloudflare Pages: pillarx-debug
github-actions
bot
temporarily deployed
to
Preview (feature/pulse-search-ui-mobile-refinements)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/apps/pulse/components/Search/TokenList.tsx (1)
41-46: Avoid mutating theassetsprop directly.
assets.sort()mutates the array in-place. Sinceassetsis a prop, this causes side effects and violates React's unidirectional data flow. This can lead to unexpected behavior and makes the component harder to reason about.Either create a copy before sorting, or lift the sorting logic to the parent component:
- assets.sort((a, b) => { + const sorted = [...assets].sort((a, b) => { if (sortType === SortType.Up) { return (b[key] || 0) - (a[key] || 0); } return (a[key] || 0) - (b[key] || 0); }); + // Note: You'll need to add a callback prop or state to actually use the sorted resultHowever, looking at how this component is used in Search.tsx, the parent already maintains
sortedSearchAssetsstate. Consider removing the sorting logic from TokenList entirely and relying on the parent's sort handlers.
♻️ Duplicate comments (2)
src/apps/pulse/components/Search/TokenList.tsx (1)
122-134: Convert inline styles to Tailwind classes for consistency.The token row uses a mix of inline
styleprops and Tailwind classes. For consistency with the rest of the codebase and easier maintenance, prefer Tailwind:<button key={item.contract || item.symbol} - className="flex w-full" - style={{ - height: 36, - marginTop: 10, - marginBottom: 10, - }} + className="flex w-full h-9 my-2.5" onClick={() => { handleTokenSelect(item); }}Based on past review comments, this was already noted.
src/apps/pulse/utils/parseSearchData.ts (1)
230-230: Reverse the fallback order forvolume_24handvolume24hfields on line 230.The current code tries the optional
volume_24hfield before the requiredvolume24hfield. Sincevolume24his the primary required field onPairResponse, it should be checked first:- volume24h: pair.volume_24h || pair.volume24h, + volume24h: pair.volume24h || pair.volume_24h,Or simply use the required field directly since
volume24his guaranteed to exist:- volume24h: pair.volume_24h || pair.volume24h, + volume24h: pair.volume24h,This aligns with the pattern used elsewhere in the file (line 207:
volume24h: pair.volume24h || 0).
🧹 Nitpick comments (2)
src/apps/pulse/components/Search/Search.tsx (2)
120-135:listis recomputed on every render, triggering unnecessary effect runs.The
listvariable is computed inline during render, creating a new object reference each time. While the effect depends onsearchTextandsearchData, this pattern can still cause subtle issues if the component re-renders for other reasons.Consider memoizing the parsed list:
+import { useMemo } from 'react'; ... - let list: { assets: Asset[]; markets: Market[] } | undefined; - if (searchData?.result.data) { - list = parseSearchData(searchData.result.data, chains, searchText); - } + const list = useMemo(() => { + if (searchData?.result.data) { + return parseSearchData(searchData.result.data, chains, searchText); + } + return undefined; + }, [searchData?.result.data, chains, searchText]); useEffect(() => { if (list?.assets) { setSortedSearchAssets([...list.assets]); } else { setSortedSearchAssets(undefined); } setSearchSort({}); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [searchText, searchData]); + }, [list]);This removes the need for the eslint-disable comment and ensures the effect only runs when the parsed data actually changes.
667-667: Active tab detection viaitem.includes(searchType)is fragile.The condition
searchType && item.includes(searchType)relies on the enum value appearing literally within the emoji-prefixed label string. This works forTrending,Fresh,Top Gainers,My Holdingssince those exact words appear in the labels, but it's a fragile coupling.A more robust approach maps the index to the enum explicitly:
+ const typeForIndex = + actualIndex === 0 + ? SearchType.Trending + : actualIndex === 1 + ? SearchType.Fresh + : actualIndex === 2 + ? SearchType.TopGainers + : SearchType.MyHoldings; + - const isActive = searchType && item.includes(searchType); + const isActive = searchType === typeForIndex;This was flagged in a past review as addressed, but the current code still uses the
includespattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/apps/pulse/assets/back-arrow-icon.svgis excluded by!**/*.svgsrc/apps/pulse/assets/clear-search-icon.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
src/apps/pulse/components/Buy/BuyButton.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(10 hunks)src/apps/pulse/components/Search/SearchSkeleton.tsx(1 hunks)src/apps/pulse/components/Search/TokenList.tsx(3 hunks)src/apps/pulse/components/Sell/Sell.tsx(1 hunks)src/apps/pulse/utils/parseSearchData.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/apps/pulse/components/Buy/BuyButton.tsx
- src/apps/pulse/components/Sell/Sell.tsx
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
📚 Learning: 2025-11-21T13:10:33.422Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/Search.tsxsrc/apps/pulse/components/Search/TokenList.tsxsrc/apps/pulse/utils/parseSearchData.ts
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/Search.tsxsrc/apps/pulse/components/Search/TokenList.tsxsrc/apps/pulse/utils/parseSearchData.ts
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required text field. The TileTitleProps interface aligns with the TokensMarketData.title type in api.ts which also has optional fields.
Applied to files:
src/apps/pulse/components/Search/TokenList.tsx
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required title field. The TileTitleProps structure aligns with how it's used in the TokensMarketData type in api.ts.
Applied to files:
src/apps/pulse/components/Search/TokenList.tsx
🧬 Code graph analysis (2)
src/apps/pulse/components/Search/TokenList.tsx (4)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/apps/pulse/utils/time.ts (1)
formatElapsedTime(1-25)src/apps/pulse/utils/number.ts (1)
formatBigNumber(1-15)
src/apps/pulse/utils/parseSearchData.ts (5)
src/types/api.ts (3)
MobulaToken(506-522)Exchange(524-527)PairResponse(570-656)src/apps/pulse/utils/constants.ts (2)
MOBULA_CHAIN_NAMES(13-15)getChainName(51-70)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/blockchain.ts (2)
isWrappedNativeToken(514-521)getChainName(269-288)src/apps/pulse/utils/number.ts (1)
parseNumberString(17-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (4)
src/apps/pulse/components/Search/SearchSkeleton.tsx (1)
1-44: LGTM - Clean skeleton component implementation.The component is well-structured with good use of Tailwind utility classes and the optional
showSectionsprop for flexibility. The skeleton layout properly mimics the actual token list structure.src/apps/pulse/utils/parseSearchData.ts (3)
396-400: Filtering requires both volume AND mCap to be non-zero.The filter uses
&&requiring both conditions:return hasValidVolume && hasValidMCap;This will exclude legitimate tokens that have trading volume but haven't established a market cap yet (or vice versa). Verify this is the intended behavior.
If the intent is to show tokens that have either valid volume or market cap, use
||:- return hasValidVolume && hasValidMCap; + return hasValidVolume || hasValidMCap;
292-314: Good implementation ofmergeMultiChainDatahelper.This correctly addresses the previous review feedback about maintaining alignment between
allChains,allContracts, andallDecimalsarrays. The chain-based merging preserves the zipped relationship.
380-390: Token types are intentionally excluded from the assets list and only contribute markets.The distinction between 'asset' and 'token' response types is by design. While the API can return the same asset in both formats, the code deliberately excludes 'token' type entries from the assets section (TokenList) and only uses them for market pair data. The
deduplicateAssetsBySymbol()function handles cases where both types are returned for the same asset. This is working as intended.
| const maxStableBalance = Math.max( | ||
| ...Object.values(stableBalance).map((s) => s.balance) | ||
| ); |
There was a problem hiding this comment.
Math.max with empty array returns -Infinity, causing unexpected behavior.
If stableBalance has no entries, spreading an empty array into Math.max returns -Infinity:
const maxStableBalance = Math.max(
...Object.values(stableBalance).map((s) => s.balance)
);This could lead to incorrect chain selection when the user has no stable currency balance.
Add a guard for empty balances:
+ const balances = Object.values(stableBalance).map((s) => s.balance);
+ if (balances.length === 0) return null;
+
- const maxStableBalance = Math.max(
- ...Object.values(stableBalance).map((s) => s.balance)
- );
+ const maxStableBalance = Math.max(...balances);🤖 Prompt for AI Agents
In src/apps/pulse/components/Search/Search.tsx around lines 352-354, Math.max is
called with a spread of balances which yields -Infinity when there are no
entries; add a guard to handle empty stableBalance by computing the array of
balances first, then if it's empty set maxStableBalance to a safe default (e.g.,
0) or use Math.max(0, ...balances) so Math.max never receives an empty list;
ensure types remain correct and downstream logic that selects chains treats the
default as "no stable balance."
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/apps/pulse/components/Search/Search.tsx (1)
277-304: FixisErrorlifecycle and avoid unnecessary Trending fetch/skeleton on MyHoldingsThere are a few related issues around the Trending/Fresh fetch, refresh, and error/empty states:
isErrorstaystrueafter a successful refresh
handleRefreshsetssetIsError(true)in thecatch, but never clears it on success. After a failed refresh, subsequent successful refreshes will still show the “⚠️ Loading Failed” block even thoughparsedAssetsis populated.Initial Trending/Fresh load doesn’t surface errors
- The
useEffectfetch’scatchonly callssetIsLoading(false)and logs, but doesn’t setisError. So failures on the first load will silently fall back to the placeholder instead of the explicit error state you added.Trending/Fresh fetch runs for
MyHoldingsand drives skeleton on the sell screen
- The effect currently runs whenever
searchTypeis truthy, includingSearchType.MyHoldings. That issues an unnecessary network call and will flipisLoading, causing the genericSearchSkeletonto show on the My Holdings screen even though those results aren’t used.Empty-state placeholder can overlap with loading skeleton
- The “Search by token or paste address...” block doesn’t gate on
!isLoading/!isFetching, so it can render at the same time as<SearchSkeleton />.Consider tightening this logic roughly like:
useEffect(() => { - if (searchType) { - setIsLoading(true); - setParsedAssets(undefined); - fetch(`${getUrl(searchType)}?chainIds=${getChainId(chains)}`) + if (searchType && searchType !== SearchType.MyHoldings) { + setIsLoading(true); + setParsedAssets(undefined); + setIsError(false); + fetch(`${getUrl(searchType)}?chainIds=${getChainId(chains)}`) .then((response) => response.json()) .then((result) => { const assets = parseFreshAndTrendingTokens(result.projection); @@ - setParsedAssets(assets); - setIsLoading(false); + setParsedAssets(assets); + setIsLoading(false); + setIsError(false); }) .catch((err) => { setIsLoading(false); - console.error(err); + setIsError(true); + console.error(err); }); } }, [searchType, chains]); @@ const handleRefresh = () => { @@ - if (searchType && searchType !== SearchType.MyHoldings) { - setIsLoading(true); - setParsedAssets(undefined); + if (searchType && searchType !== SearchType.MyHoldings) { + setIsLoading(true); + setParsedAssets(undefined); + setIsError(false); @@ - setParsedAssets(assets); - setIsLoading(false); + setParsedAssets(assets); + setIsLoading(false); + setIsError(false); }) .catch((err) => { setIsLoading(false); setIsError(true); console.error(err); }); } }; @@ - {!searchText && - parsedAssets === undefined && - searchType !== SearchType.MyHoldings && - !isError && ( + {!searchText && + parsedAssets === undefined && + searchType !== SearchType.MyHoldings && + !isError && + !isFetching && + !isLoading && (This keeps the error banner in sync with the last fetch outcome, avoids extra Trending requests when on My Holdings, and prevents the skeleton/placeholder from rendering simultaneously.
Also applies to: 305-340, 700-709, 712-721
♻️ Duplicate comments (2)
src/apps/pulse/components/Search/Search.tsx (2)
345-365: Optional: GuardgetChainWithMostUSDCagainst empty balances for clarityFunctionally this works even when
stableBalanceis empty (sinceMath.max(...[])ends up as-Infinityand the subsequent lookup returnsnull→0→null), but it’s a bit non-obvious and relies onMath.max’s-Infinitybehavior.You could make the intent clearer and avoid spreading an empty array:
const stableBalance = getStableCurrencyBalanceOnEachChain({ result: { data: walletPortfolioData }, }); - const maxStableBalance = Math.max( - ...Object.values(stableBalance).map((s) => s.balance) - ); + const balances = Object.values(stableBalance).map((s) => s.balance); + if (balances.length === 0) return null; + + const maxStableBalance = Math.max(...balances);This keeps the external behavior the same (returns
nullwhen no balances) but is more explicit about the “no stablecoins” case.
640-689: Fix brittle tab active state detection for “Top Gainers” and “My Holdings”
isActiveis currently derived via:const isActive = searchType && item.includes(searchType);For labels like
'🚀 Top Gainers'and'💰 My Holdings', the enum values (SearchType.TopGainers,SearchType.MyHoldings) don’t literally occur in the label string (no space), so those tabs will never be marked active even when selected.You already derive the semantic index via
actualIndex; reusing that for active state is more robust:).map((item, index) => { // For sell screen, always map to MyHoldings index (3) const actualIndex = isBuy ? index : 3; @@ - const isActive = searchType && item.includes(searchType); + const typeForIndex = + actualIndex === 0 + ? SearchType.Trending + : actualIndex === 1 + ? SearchType.Fresh + : actualIndex === 2 + ? SearchType.TopGainers + : SearchType.MyHoldings; + + const isActive = searchType === typeForIndex;This keeps the visual state aligned with
handleSearchTypeChangeregardless of label text or emojis.
🧹 Nitpick comments (1)
src/apps/pulse/components/Search/Search.tsx (1)
600-625: Add keyboard handling to the chain selectordivfor full button semanticsThe chain selector wrapper is now a
<div>withrole="button"andtabIndex={0}, which is good, but it only handles mouse clicks. Keyboard users who focus it won’t be able to activate it via Enter/Space.Consider adding a simple key handler:
{isBuy ? ( <div ref={chainButtonRef} - onClick={() => { + onClick={() => { const rect = chainButtonRef?.current?.getBoundingClientRect(); if (rect) { setShowChainOverlay(true); setOverlayStyle({ ...overlayStyling, top: `${rect.bottom + 5}px`, left: `${rect.right - 200}px`, }); } }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + const rect = chainButtonRef?.current?.getBoundingClientRect(); + if (rect) { + setShowChainOverlay(true); + setOverlayStyle({ + ...overlayStyling, + top: `${rect.bottom + 5}px`, + left: `${rect.right - 200}px`, + }); + } + } + }} className="flex items-center justify-center w-10 h-10 bg-[#121116] rounded-[10px] flex-shrink-0 group p-0.5 cursor-pointer" role="button" tabIndex={0}This keeps the visual structure and positioning logic intact while making the control keyboard-activatable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/pulse/components/Search/Search.tsx(9 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
📚 Learning: 2025-11-21T13:10:33.422Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/pulse/components/Search/Search.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (2)
src/apps/pulse/components/Search/Search.tsx (2)
99-121: Search sorting state and parsed list wiring look solidThe
searchTypedefaulting (buy → Trending, sell → MyHoldings),searchSort/sortedSearchAssetsstate, and theuseEffectthat reinitializes sorted assets on new search data are coherent and match the intended UX (sort reset + error reset on new search). UsingparseSearchDataas the single source of truth forassets/marketsalso keeps the data flow clean.No functional issues spotted here.
Also applies to: 123-171
375-444: Multi-chain asset selection and token/market handlers look correctThe updated
handleTokenSelectcorrectly:
- Picks the chain with the highest stablecoin balance when
allChains/allContracts/allDecimalsare available, with a sane fallback to the primary chain.- Normalizes
dailyPriceChangeto0.0when unavailable, avoiding the earlier hardcoded-0.02bias.- Ensures
chainId,decimals, andaddressare always set before callingsetBuyToken/setSellToken.
handleMarketSelectalso cleanly sets the buy token frommarket.token0and usesmarket.blockchainforchainId, matching the new Market data model.No functional issues here.
Also applies to: 451-488, 499-517
…ing and data refinements
Overview
Comprehensive refinement of the Pulse token search UI with focus on mobile responsiveness, blockchain filtering, data accuracy, and pixel-perfect design alignment.
Key Features
Mobile Responsiveness
Blockchain Filtering
Data Quality & Accuracy
UI/UX Improvements
Bug Fixes
Technical Changes
New Components
Modified Components
Utilities
Testing
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
UI/UX Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.