-
Notifications
You must be signed in to change notification settings - Fork 12
Feature/pulse token selector fix #482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRefactors Pulse Buy and Sell token selector UIs to compact absolute-positioned tiles, updates Search buy-path to use snake_case price_change_24h when present, changes an unauthorized catch-all route to redirect home, relaxes PnLDetailModal view type to allow null, and removes explicit font declarations in PortfolioTokenList headers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-09-09T12:40:15.629ZApplied to files:
📚 Learning: 2025-11-21T13:10:33.422ZApplied to files:
📚 Learning: 2025-08-12T07:42:24.656ZApplied to files:
📚 Learning: 2025-12-11T12:40:09.964ZApplied to files:
📚 Learning: 2025-04-23T15:04:20.826ZApplied to files:
📚 Learning: 2025-03-28T09:22:22.712ZApplied to files:
📚 Learning: 2025-04-23T15:04:20.826ZApplied to files:
🧬 Code graph analysis (2)src/apps/pulse/components/Sell/Sell.tsx (2)
src/apps/pulse/components/Buy/Buy.tsx (1)
⏰ 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)
🔇 Additional comments (2)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/apps/insights/utils/signalUtils.ts (1)
51-98: Wrap variable declarations in switch cases with blocks.Variables declared in switch cases without block scope can be accessed by other cases due to hoisting. Wrap the case bodies in braces to restrict variable access to their respective cases.
Apply this diff to fix the scoping issue:
case 'tp_hit': + { - const tpPercent = - (((event.tp_price || 0) - signal.entry_price) / signal.entry_price) * - 100; - const lockedIn = tpPercent * 0.3333; + const tpPercent = + (((event.tp_price || 0) - signal.entry_price) / signal.entry_price) * + 100; + const lockedIn = tpPercent * 0.3333; - timelineItems.push({ - icon: 'Target', - iconColor: 'text-emerald-400', - label: `${event.tp_level?.toUpperCase()} Hit`, - price: event.tp_price || 0, - timestamp: event.timestamp, - pnl: lockedIn, - }); + timelineItems.push({ + icon: 'Target', + iconColor: 'text-emerald-400', + label: `${event.tp_level?.toUpperCase()} Hit`, + price: event.tp_price || 0, + timestamp: event.timestamp, + pnl: lockedIn, + }); - // Add stop loss move event if it moved - if (event.moved && event.old_stop && event.new_stop) { - timelineItems.push({ - icon: 'RefreshCw', - iconColor: 'text-green-400', - label: 'Stop Loss Moved', - price: event.new_stop, - timestamp: event.timestamp, - pnl: null, - detail: `$${formatPrice(event.old_stop, signal.ticker)} → $${formatPrice(event.new_stop, signal.ticker)}`, - }); - } - break; + // Add stop loss move event if it moved + if (event.moved && event.old_stop && event.new_stop) { + timelineItems.push({ + icon: 'RefreshCw', + iconColor: 'text-green-400', + label: 'Stop Loss Moved', + price: event.new_stop, + timestamp: event.timestamp, + pnl: null, + detail: `$${formatPrice(event.old_stop, signal.ticker)} → $${formatPrice(event.new_stop, signal.ticker)}`, + }); + } + break; + } case 'stop_loss_hit': + { - // Calculate stop loss P&L based on position type - const isShortSL = signal.order_side?.toLowerCase() === 'sell'; - const slPercent = isShortSL - ? ((signal.entry_price - (event.stop_price || latestStop)) / - signal.entry_price) * - 100 // SHORT - : (((event.stop_price || latestStop) - signal.entry_price) / - signal.entry_price) * - 100; // LONG - timelineItems.push({ - icon: 'XCircle', - iconColor: 'text-red-400', - label: 'Stop Loss Hit', - price: event.stop_price || latestStop, - timestamp: event.timestamp, - pnl: slPercent, - }); - break; + // Calculate stop loss P&L based on position type + const isShortSL = signal.order_side?.toLowerCase() === 'sell'; + const slPercent = isShortSL + ? ((signal.entry_price - (event.stop_price || latestStop)) / + signal.entry_price) * + 100 // SHORT + : (((event.stop_price || latestStop) - signal.entry_price) / + signal.entry_price) * + 100; // LONG + timelineItems.push({ + icon: 'XCircle', + iconColor: 'text-red-400', + label: 'Stop Loss Hit', + price: event.stop_price || latestStop, + timestamp: event.timestamp, + pnl: slPercent, + }); + break; + }src/apps/insights/components/ui/use-toast.ts (1)
168-186: Incorrect dependency array in useEffect.The
useEffectat line 179 has[state]as a dependency, but this causes the effect to re-run on every state change, unnecessarily unsubscribing and resubscribing the listener. Since the intent is to subscribe once on mount and unsubscribe on unmount, the dependency array should be empty.React.useEffect(() => { listeners.push(setState); return () => { const index = listeners.indexOf(setState); if (index > -1) { listeners.splice(index, 1); } }; - }, [state]); + }, []);src/apps/insights/components/ui/command.tsx (1)
24-36: CommandDialog export lacks integration and testing.The CommandDialog component shows the command menu in a dialog and is correctly implemented. However, verification found zero usages of CommandDialog in the codebase and no associated tests. Exporting a public component without integration or test coverage should be reconsidered—either integrate this component into the application and add tests, or remove it from the public API until it has actual usage.
♻️ Duplicate comments (1)
src/apps/pulse/components/Sell/Sell.tsx (1)
302-405: Token selector duplication noted.This token selector implementation is nearly identical to the one in
src/apps/pulse/components/Buy/Buy.tsx(lines 643-743). See the refactoring suggestion in the Buy.tsx review for details on extracting a shared component.
🧹 Nitpick comments (8)
src/apps/insights/components/ui/carousel.tsx (1)
1-260: Consider using automated formatting tools for consistency.While the manual formatting standardization is well-executed, the project could benefit from tools like Prettier or ESLint with auto-fix enabled to automatically enforce consistent quote styles, indentation, and multi-line formatting across the entire codebase. This would save time and ensure consistency in future changes.
src/apps/developer-apps/api/developerAppsApi.ts (1)
116-145: Pre-existing patterns worth noting for future improvement.The formatting changes here are fine. However, for future consideration:
- Empty signature string (line 118): The
signatureis always empty, which may indicate incomplete implementation.alert()for error feedback (lines 134-136): Using browser alerts in an API layer is not ideal for UX. Consider using a toast notification system or returning the error for the UI layer to handle.as anycasts (lines 140, 142): These reduce type safety and could mask issues.These appear to be pre-existing patterns rather than changes introduced by this PR, so flagging as optional future improvements.
src/apps/developer-apps/components/AppForm.tsx (1)
50-56: Review the dependency array for infinite loop risk.The auto-generation logic is a nice UX enhancement, but including
formData.appIdin the dependency array may cause issues:
- When the effect runs and updates
appIdviasetFormData, it triggers a re-render- On re-render,
formData.appIdis now truthy, so the condition!formData.appIdprevents further updates ✓- However, if a user manually clears the
appIdfield whilenamestill has a value, the effect will run again and re-populate itThis behavior may or may not be desired. If you want to allow users to manually clear or customize the
appIdwithout it being regenerated, consider using a ref to track whether the user has manually edited the field.Example approach using a ref:
+const hasManuallyEditedAppId = useRef(false); + // Auto-generate appId from name in create mode useEffect(() => { - if (mode === 'create' && formData.name && !formData.appId) { + if (mode === 'create' && formData.name && !formData.appId && !hasManuallyEditedAppId.current) { const sanitized = sanitizeAppId(formData.name); setFormData((prev) => ({ ...prev, appId: sanitized })); } -}, [formData.name, formData.appId, mode, setFormData]); +}, [formData.name, formData.appId, mode, setFormData]);Then update the
appIdinput'sonChangeto set the ref:onChange={(e) => { + hasManuallyEditedAppId.current = true; updateField('appId', sanitizeAppId(e.target.value)) }}src/apps/insights/components/ui/sidebar.tsx (2)
75-88: Consider optimizing the setOpen callback dependency array.Including
openin the dependency array causes the callback to recreate on every state change, which may lead to unnecessary re-renders of child components.Consider removing
openfrom the dependency array and using the functional update form exclusively:const setOpen = React.useCallback( (value: boolean | ((value: boolean) => boolean)) => { - const openState = typeof value === 'function' ? value(open) : value; if (setOpenProp) { - setOpenProp(openState); + setOpenProp(typeof value === 'function' ? (prev) => { const next = value(prev); document.cookie = `${SIDEBAR_COOKIE_NAME}=${next}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; SameSite=Lax; Secure`; return next; } : value); } else { - _setOpen(openState); + _setOpen((prev) => { const next = typeof value === 'function' ? value(prev) : value; document.cookie = `${SIDEBAR_COOKIE_NAME}=${next}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}; SameSite=Lax; Secure`; return next; }); } - - // This sets the cookie to keep the sidebar state. - document.cookie = `${SIDEBAR_COOKIE_NAME}=${openState}; path=/; max-age=${SIDEBAR_COOKIE_MAX_AGE}`; }, - [setOpenProp, open] + [setOpenProp] );This eliminates the dependency on
openwhile maintaining the same functionality.
433-437: Type assertions withas anyweaken type safety.Multiple components use
ref as anywhen working with theSlotcomponent. While this is a common pattern with Radix UI's Slot, it bypasses TypeScript's type checking.Consider using a more specific type assertion or exploring Radix UI's recommended typing patterns for Slot components. For example:
ref={ref as React.Ref<HTMLDivElement>}Or investigate if Radix UI provides helper types for this pattern. However, if this is the established pattern in the codebase and works reliably, it may be acceptable to defer this refactor.
Also applies to: 454-458, 554-559, 600-604, 714-718
src/apps/insights/hooks/useTradingSignals.ts (1)
32-59: Consider conditionalizing verbose console logs for production.The extensive logging (raw response, full signals array, first signal structure) is excellent for debugging but may be too verbose for production. Consider wrapping these in a debug flag or removing before production deployment.
Example pattern:
+const DEBUG = process.env.NODE_ENV === 'development'; + const fetchSignals = useCallback( async (isRefresh = false) => { try { const result = await getTradingSignals(); - console.log('🔍 [useTradingSignals] API response:', result); + if (DEBUG) { + console.log('🔍 [useTradingSignals] API response:', result); + } // ... rest of logging similarly guardedsrc/apps/insights/components/PnLDetailModal.tsx (1)
129-129: Redundant null check.This
if (!view) return null;is redundant since the same check already exists at line 57. After fixing the hooks order issue, this can be removed.- if (!view) return null; - const totalPnL =src/apps/pulse/components/Buy/Buy.tsx (1)
643-743: Consider extracting the token selector UI into a shared component.The token selector implementation here is nearly identical to the one in
src/apps/pulse/components/Sell/Sell.tsx(lines 302-405), with ~70 lines of duplicated JSX including:
- Absolute positioning layout
- Logo rendering with chain badge
- Symbol/name row
- Price/change row with triangle indicator
- Chevron icon
A shared
TokenSelectorcomponent would eliminate duplication, improve maintainability, and ensure consistent behavior across Buy and Sell flows. For example:// src/apps/pulse/components/TokenSelector/TokenSelector.tsx interface TokenSelectorProps { token: SelectedToken | null; onClick: () => void; isSearching?: boolean; testIdPrefix: 'pulse-buy' | 'pulse-sell'; } export const TokenSelector = ({ token, onClick, isSearching, testIdPrefix }: TokenSelectorProps) => { // Extract the common rendering logic here return ( <button onClick={onClick} type="button" data-testid={`${testIdPrefix}-token-selector`}> {/* Common token display logic */} </button> ); };Then use it in both Buy and Sell components:
-<button onClick={() => setSearching(true)} type="button" ...> - {/* 70+ lines of token display JSX */} -</button> +<TokenSelector + token={token} + onClick={() => setSearching(true)} + isSearching={isSearchingToken} + testIdPrefix="pulse-buy" +/>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/apps/pulse/components/App/tests/__snapshots__/AppWrapper.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/App/tests/__snapshots__/HomeScreen.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/Buy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/Sell.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/CardsSwap/test/__snapshots__/CardSwap.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (100)
src/apps/developer-apps/api/developerAppsApi.ts(5 hunks)src/apps/developer-apps/components/AppCard.tsx(4 hunks)src/apps/developer-apps/components/AppForm.tsx(18 hunks)src/apps/developer-apps/components/AppsList.tsx(8 hunks)src/apps/developer-apps/components/ImageUpload.tsx(3 hunks)src/apps/developer-apps/components/__tests__/AppCard.test.tsx(0 hunks)src/apps/developer-apps/components/__tests__/ImageUpload.test.tsx(13 hunks)src/apps/developer-apps/hooks/useAppForm.ts(2 hunks)src/apps/developer-apps/index.tsx(1 hunks)src/apps/developer-apps/manifest.json(0 hunks)src/apps/developer-apps/styles/developers.css(4 hunks)src/apps/developer-apps/utils/imageUtils.ts(2 hunks)src/apps/developer-apps/utils/validation.ts(0 hunks)src/apps/insights/api/insightsApi.ts(3 hunks)src/apps/insights/components/FeedEventCard/FeedEventCard.tsx(5 hunks)src/apps/insights/components/Filters/Filters.tsx(3 hunks)src/apps/insights/components/Header/Header.tsx(2 hunks)src/apps/insights/components/KPICard/KPICard.tsx(4 hunks)src/apps/insights/components/PnLDetailModal.tsx(15 hunks)src/apps/insights/components/SignalCard/SignalCard.tsx(11 hunks)src/apps/insights/components/SparklineChart.tsx(6 hunks)src/apps/insights/components/consent/ConsentModal.tsx(3 hunks)src/apps/insights/components/consent/ConsentModalLayout.tsx(2 hunks)src/apps/insights/components/consent/ConsentTabContent.tsx(2 hunks)src/apps/insights/components/consent/RiskTabContent.tsx(3 hunks)src/apps/insights/components/consent/TermsTabContent.tsx(5 hunks)src/apps/insights/components/consent/constants.ts(1 hunks)src/apps/insights/components/consent/index.ts(1 hunks)src/apps/insights/components/consent/types.ts(1 hunks)src/apps/insights/components/consent/useConsentValidation.ts(3 hunks)src/apps/insights/components/ui/accordion.tsx(3 hunks)src/apps/insights/components/ui/alert-dialog.tsx(4 hunks)src/apps/insights/components/ui/alert.tsx(1 hunks)src/apps/insights/components/ui/aspect-ratio.tsx(1 hunks)src/apps/insights/components/ui/avatar.tsx(3 hunks)src/apps/insights/components/ui/badge.tsx(1 hunks)src/apps/insights/components/ui/breadcrumb.tsx(1 hunks)src/apps/insights/components/ui/button.tsx(2 hunks)src/apps/insights/components/ui/calendar.tsx(2 hunks)src/apps/insights/components/ui/card.tsx(1 hunks)src/apps/insights/components/ui/carousel.tsx(8 hunks)src/apps/insights/components/ui/chart.tsx(10 hunks)src/apps/insights/components/ui/checkbox.tsx(2 hunks)src/apps/insights/components/ui/collapsible.tsx(1 hunks)src/apps/insights/components/ui/command.tsx(8 hunks)src/apps/insights/components/ui/context-menu.tsx(10 hunks)src/apps/insights/components/ui/dialog.tsx(5 hunks)src/apps/insights/components/ui/drawer.tsx(5 hunks)src/apps/insights/components/ui/dropdown-menu.tsx(10 hunks)src/apps/insights/components/ui/form.tsx(4 hunks)src/apps/insights/components/ui/hover-card.tsx(2 hunks)src/apps/insights/components/ui/input-otp.tsx(3 hunks)src/apps/insights/components/ui/input.tsx(1 hunks)src/apps/insights/components/ui/label.tsx(1 hunks)src/apps/insights/components/ui/menubar.tsx(11 hunks)src/apps/insights/components/ui/navigation-menu.tsx(7 hunks)src/apps/insights/components/ui/pagination.tsx(1 hunks)src/apps/insights/components/ui/popover.tsx(2 hunks)src/apps/insights/components/ui/progress.tsx(1 hunks)src/apps/insights/components/ui/radio-group.tsx(2 hunks)src/apps/insights/components/ui/resizable.tsx(2 hunks)src/apps/insights/components/ui/scroll-area.tsx(2 hunks)src/apps/insights/components/ui/select.tsx(7 hunks)src/apps/insights/components/ui/separator.tsx(1 hunks)src/apps/insights/components/ui/sheet.tsx(3 hunks)src/apps/insights/components/ui/sidebar.tsx(4 hunks)src/apps/insights/components/ui/skeleton.tsx(1 hunks)src/apps/insights/components/ui/slider.tsx(1 hunks)src/apps/insights/components/ui/sonner.tsx(2 hunks)src/apps/insights/components/ui/switch.tsx(1 hunks)src/apps/insights/components/ui/table.tsx(1 hunks)src/apps/insights/components/ui/tabs.tsx(4 hunks)src/apps/insights/components/ui/textarea.tsx(1 hunks)src/apps/insights/components/ui/toast.tsx(5 hunks)src/apps/insights/components/ui/toaster.tsx(2 hunks)src/apps/insights/components/ui/toggle-group.tsx(2 hunks)src/apps/insights/components/ui/toggle.tsx(1 hunks)src/apps/insights/components/ui/tooltip.tsx(2 hunks)src/apps/insights/components/ui/use-toast.ts(8 hunks)src/apps/insights/hooks/use-mobile.tsx(1 hunks)src/apps/insights/hooks/useLogoMap.ts(4 hunks)src/apps/insights/hooks/useSparklineData.ts(3 hunks)src/apps/insights/hooks/useSubscriptionStatus.ts(0 hunks)src/apps/insights/hooks/useTradingSignals.ts(1 hunks)src/apps/insights/index.tsx(15 hunks)src/apps/insights/lib/stopLossUtils.ts(8 hunks)src/apps/insights/lib/utils.ts(1 hunks)src/apps/insights/manifest.json(0 hunks)src/apps/insights/styles/insights.css(4 hunks)src/apps/insights/tailwind.insights.config.js(1 hunks)src/apps/insights/types/index.ts(1 hunks)src/apps/insights/utils/formatUtils.ts(1 hunks)src/apps/insights/utils/logoUtils.ts(3 hunks)src/apps/insights/utils/signalUtils.ts(14 hunks)src/apps/insights/utils/supabaseAdapter.ts(4 hunks)src/apps/pulse/components/Buy/Buy.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(1 hunks)src/apps/pulse/components/Sell/Sell.tsx(1 hunks)src/containers/Main.tsx(1 hunks)src/test-utils/setupTests.ts(2 hunks)
💤 Files with no reviewable changes (5)
- src/apps/developer-apps/components/tests/AppCard.test.tsx
- src/apps/developer-apps/utils/validation.ts
- src/apps/insights/hooks/useSubscriptionStatus.ts
- src/apps/developer-apps/manifest.json
- src/apps/insights/manifest.json
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:02.519Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
📚 Learning: 2025-10-15T10:31:06.760Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 432
File: src/containers/Main.tsx:135-141
Timestamp: 2025-10-15T10:31:06.760Z
Learning: In src/containers/Main.tsx, the React Native webview messaging setup intentionally only activates when the `devicePlatform` URL parameter is present ('ios' or 'android'). On reloads or direct navigation without URL params, the site should behave normally and not attempt to re-establish RN messaging. This is by design to ensure the site functions properly in both RN webview and standard browser contexts.
Applied to files:
src/containers/Main.tsx
📚 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/developer-apps/components/AppsList.tsxsrc/apps/pulse/components/Buy/Buy.tsxsrc/apps/pulse/components/Sell/Sell.tsxsrc/apps/insights/components/SignalCard/SignalCard.tsxsrc/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/apps/developer-apps/components/AppsList.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/developer-apps/components/AppsList.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/Buy/Buy.tsxsrc/apps/pulse/components/Sell/Sell.tsxsrc/apps/pulse/components/Search/Search.tsx
📚 Learning: 2025-12-11T12:40:02.519Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:02.519Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
Applied to files:
src/apps/pulse/components/Buy/Buy.tsxsrc/apps/pulse/components/Sell/Sell.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 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/Buy/Buy.tsxsrc/apps/insights/components/ui/toast.tsxsrc/apps/pulse/components/Sell/Sell.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/Buy/Buy.tsxsrc/apps/pulse/components/Sell/Sell.tsxsrc/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/Buy/Buy.tsxsrc/apps/pulse/components/Sell/Sell.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/Buy/Buy.tsxsrc/apps/insights/components/ui/toast.tsx
📚 Learning: 2025-12-03T10:01:15.801Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 471
File: src/apps/pillarx-app/components/AlgoInsightsTile/AlgoInsightsTile.tsx:36-48
Timestamp: 2025-12-03T10:01:15.801Z
Learning: In the AlgoInsightsTile component (src/apps/pillarx-app/components/AlgoInsightsTile/AlgoInsightsTile.tsx), the data is always hardcoded and guaranteed to be present with complete history arrays for all timeframes, so edge case handling for empty or sparse history data is not required.
Applied to files:
src/apps/insights/hooks/useSparklineData.tssrc/apps/insights/components/SparklineChart.tsxsrc/apps/insights/index.tsxsrc/apps/insights/components/SignalCard/SignalCard.tsxsrc/apps/insights/components/PnLDetailModal.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/insights/components/PnLDetailModal.tsx
🧬 Code graph analysis (59)
src/apps/insights/components/ui/toaster.tsx (1)
src/apps/insights/components/ui/toast.tsx (1)
ToastDescription(124-124)
src/apps/insights/components/ui/avatar.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/label.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/checkbox.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/skeleton.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/context-menu.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/toggle.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/accordion.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/alert.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/calendar.tsx (2)
src/apps/insights/lib/utils.ts (1)
cn(4-6)src/apps/insights/components/ui/button.tsx (1)
buttonVariants(56-56)
src/apps/insights/components/ui/slider.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/hooks/useLogoMap.ts (2)
src/apps/insights/utils/formatUtils.ts (1)
normalizeSymbol(8-13)src/apps/insights/utils/logoUtils.ts (2)
loadLogoCache(16-38)fetchLogoFromMobula(65-128)
src/apps/insights/components/ui/textarea.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/hooks/useTradingSignals.ts (2)
src/apps/insights/api/insightsApi.ts (1)
getTradingSignals(53-72)src/apps/insights/types/index.ts (1)
TradingSignal(5-35)
src/apps/pulse/components/Buy/Buy.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
src/apps/developer-apps/components/AppForm.tsx (2)
src/apps/developer-apps/hooks/useAppForm.ts (1)
useAppForm(58-187)src/apps/developer-apps/utils/validation.ts (1)
sanitizeAppId(40-46)
src/apps/insights/components/ui/progress.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/sheet.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/resizable.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/hooks/useSparklineData.ts (2)
src/apps/insights/types/index.ts (2)
SparklineDataPoint(57-60)TradingSignal(5-35)src/apps/insights/api/insightsApi.ts (1)
fetchSparklineData(20-48)
src/apps/insights/components/ui/command.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/dropdown-menu.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/hover-card.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/radio-group.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/drawer.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/KPICard/KPICard.tsx (1)
src/apps/insights/components/ui/badge.tsx (1)
Badge(36-36)
src/apps/insights/components/ui/toast.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/input-otp.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/input.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/utils/supabaseAdapter.ts (1)
src/apps/insights/api/insightsApi.ts (2)
fetchSparklineData(20-48)updateSignalPrices(77-99)
src/apps/insights/components/ui/switch.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/popover.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/table.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/toggle-group.tsx (2)
src/apps/insights/components/ui/toggle.tsx (1)
toggleVariants(43-43)src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/separator.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/pulse/components/Sell/Sell.tsx (2)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/utils/number.tsx (1)
formatExponentialSmallNumber(119-149)
src/apps/insights/components/ui/pagination.tsx (2)
src/apps/insights/lib/utils.ts (1)
cn(4-6)src/apps/insights/components/ui/button.tsx (2)
ButtonProps(36-40)buttonVariants(56-56)
src/apps/insights/index.tsx (2)
src/apps/insights/hooks/useSparklineData.ts (1)
useSparklineData(9-87)src/apps/insights/utils/signalUtils.ts (3)
generateOverallPnLSparkline(234-251)generateOpenPnLSparkline(256-273)generateClosedPnLSparkline(278-310)
src/apps/insights/components/ui/badge.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/select.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/dialog.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/SignalCard/SignalCard.tsx (3)
src/apps/insights/utils/formatUtils.ts (1)
formatPrice(18-21)src/apps/insights/lib/stopLossUtils.ts (2)
hasTrailingStop(42-46)getEffectiveStopLoss(25-37)src/apps/insights/components/SparklineChart.tsx (1)
SparklineChart(12-196)
src/apps/insights/components/ui/button.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/scroll-area.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/consent/useConsentValidation.ts (1)
src/apps/insights/components/consent/types.ts (1)
ValidationState(30-36)
src/apps/insights/components/ui/form.tsx (3)
src/apps/insights/lib/utils.ts (1)
cn(4-6)src/apps/insights/components/ui/label.tsx (1)
Label(24-24)functions/api/coinbase/platform/v2/onramp/sessions.js (1)
body(8-8)
src/apps/insights/components/consent/ConsentModalLayout.tsx (4)
src/apps/insights/components/ui/button.tsx (1)
Button(56-56)src/apps/insights/components/ui/tabs.tsx (3)
Tabs(53-53)TabsList(53-53)TabsTrigger(53-53)src/apps/insights/components/ui/drawer.tsx (2)
Drawer(106-106)DrawerContent(111-111)src/apps/insights/components/ui/dialog.tsx (2)
Dialog(110-110)DialogContent(115-115)
src/apps/insights/components/ui/navigation-menu.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/alert-dialog.tsx (2)
src/apps/insights/lib/utils.ts (1)
cn(4-6)src/apps/insights/components/ui/button.tsx (1)
buttonVariants(56-56)
src/apps/insights/components/ui/menubar.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/card.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/ui/breadcrumb.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/components/consent/ConsentTabContent.tsx (3)
src/apps/insights/components/ui/checkbox.tsx (1)
Checkbox(28-28)src/apps/insights/components/ui/label.tsx (1)
Label(24-24)src/apps/insights/components/ui/radio-group.tsx (2)
RadioGroup(42-42)RadioGroupItem(42-42)
src/apps/insights/utils/signalUtils.ts (3)
src/apps/insights/utils/formatUtils.ts (1)
formatPrice(18-21)src/apps/insights/types/index.ts (1)
TradingSignal(5-35)src/apps/insights/lib/stopLossUtils.ts (1)
getEffectiveStopLoss(25-37)
src/apps/insights/components/ui/chart.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
src/apps/insights/lib/stopLossUtils.ts (1)
src/apps/insights/types/index.ts (1)
TradingSignal(5-35)
src/apps/insights/components/PnLDetailModal.tsx (2)
src/apps/insights/lib/stopLossUtils.ts (2)
normalizeTrailingHistory(90-248)getEffectiveStopLoss(25-37)src/apps/insights/utils/formatUtils.ts (1)
formatPrice(18-21)
src/apps/insights/components/ui/sidebar.tsx (8)
src/apps/insights/hooks/use-mobile.tsx (1)
useIsMobile(5-21)src/apps/insights/components/ui/tooltip.tsx (3)
TooltipProvider(28-28)TooltipContent(28-28)Tooltip(28-28)src/apps/insights/lib/utils.ts (1)
cn(4-6)src/apps/insights/components/ui/sheet.tsx (2)
Sheet(128-128)SheetContent(130-130)src/apps/insights/components/ui/button.tsx (1)
Button(56-56)src/apps/insights/components/ui/input.tsx (1)
Input(22-22)src/apps/insights/components/ui/separator.tsx (1)
Separator(29-29)src/apps/insights/components/ui/skeleton.tsx (1)
Skeleton(15-15)
src/apps/insights/components/ui/carousel.tsx (1)
src/apps/insights/lib/utils.ts (1)
cn(4-6)
🪛 Biome (2.1.2)
src/apps/insights/utils/signalUtils.ts
[error] 52-54: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 55-55: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 82-82: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 83-89: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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: Cloudflare Pages: pillarx-debug
- Refine Token Selector UI in Buy.tsx and Sell.tsx to match Figma specs. - Fix missing price change for 'My Holdings' tokens in Search.tsx. - Add green/red triangle indicator for price changes. - Increase token selector dropdown arrow size. - Redirect unauthenticated users from /pulse to /. - Update snapshots.
013c96e to
8e83651
Compare
Deploying pillarx-debug with
|
| Latest commit: |
3dd6f77
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2c403416.pillarx-debug.pages.dev |
| Branch Preview URL: | https://feature-pulse-token-selector.pillarx-debug.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/apps/pulse/components/Sell/Sell.tsx (1)
316-321: Missingrelativeclass on avatar container may cause misaligned initials.The
spanusesabsolute inset-0but its immediate parentdiv(line 316) lacksposition: relative. The initials will be positioned relative to the ancestor at line 308 rather than the avatar container, potentially causing visual misalignment.) : ( - <div className="w-full h-full overflow-hidden rounded-full"> + <div className="relative w-full h-full overflow-hidden rounded-full"> <RandomAvatar name={token.name || ''} /> <span className="absolute inset-0 flex items-center justify-center text-white text-[10px]"> {token.name?.slice(0, 2)} </span> </div> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/apps/pulse/components/Buy/tests/__snapshots__/Buy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/Sell.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
src/apps/pulse/components/Buy/Buy.tsx(1 hunks)src/apps/pulse/components/Search/Search.tsx(1 hunks)src/apps/pulse/components/Sell/Sell.tsx(1 hunks)src/containers/Main.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/containers/Main.tsx
- src/apps/pulse/components/Buy/Buy.tsx
- src/apps/pulse/components/Search/Search.tsx
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:02.519Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
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: 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: 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-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/Sell/Sell.tsx
📚 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/Sell/Sell.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.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/Sell/Sell.tsx
📚 Learning: 2025-12-11T12:40:02.519Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:02.519Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Sell/Sell.tsx (2)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/utils/number.tsx (1)
formatExponentialSmallNumber(119-149)
⏰ 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/Sell/Sell.tsx (2)
304-344: LGTM on the token selector layout restructure.The absolute-positioned layout with dedicated areas for logo, token info, and price change indicators is well-organized. The truncation classes properly handle overflow for long token names/symbols.
357-375: Remove this review comment—dailyPriceChangeis a required field onSelectedTokenand cannot be undefined or null.The code is safe. The
SelectedTokentype definesdailyPriceChange: numberas a required field (not optional), and the component conditionally renders this UI block only whentokenis truthy (line 303:{token ? ().Math.abs(token.dailyPriceChange).toFixed(1)will always execute safely.
Deploying x with
|
| Latest commit: |
3dd6f77
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://62b0e802.x-e62.pages.dev |
| Branch Preview URL: | https://feature-pulse-token-selector.x-e62.pages.dev |
…ins 12px) and update snapshots
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/apps/pulse/components/Buy/Buy.tsx (2)
672-679: Verify symbol + name fits within parent max-width.The top row has
max-w-[90px]while the symbol alone can occupy up tomax-w-[50px]plus a 4px gap. If both symbol and name are long, the combined width might exceed the parent constraint, thoughtruncateclasses should handle overflow. Consider testing with edge cases like long symbol + long name combinations.
684-684: Consider formatting very small USD values.The price displays
token.usdValuedirectly without formatting, while Sell.tsx (line 352) usesformatExponentialSmallNumberfor the same field. For tokens with very small USD values (e.g., 0.000001), the raw display may show scientific notation or truncate improperly.🔎 Apply this diff to format small values consistently:
- ${token.usdValue} + ${formatExponentialSmallNumber(token.usdValue)}Add the import at the top if not already present:
import { getLogoForChainId } from '../../../../utils/blockchain'; +import { formatExponentialSmallNumber } from '../../../../utils/number';src/apps/pulse/components/Sell/Sell.tsx (1)
303-404: Extract shared token selector UI to reduce duplication.The token selector UI (lines 303-404 in Sell.tsx) is nearly identical to Buy.tsx (lines 644-743), with ~80-100 lines of duplicated layout code. Both implement the same absolute-positioned structure with logo, symbol/name row, price/change row, triangle indicator, and chevron.
Consider extracting to a shared
TokenSelectorButtoncomponent that accepts props for the token data and onClick handler. This would centralize the layout logic, ensure consistent formatting (likeformatExponentialSmallNumber), and simplify future UI changes.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/apps/pulse/components/App/tests/__snapshots__/AppWrapper.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/App/tests/__snapshots__/HomeScreen.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/Buy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/Sell.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
hypercopy-reference(1 hunks)src/apps/pulse/components/Buy/Buy.tsx(1 hunks)src/apps/pulse/components/Sell/Sell.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- hypercopy-reference
🧰 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.
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:09.964Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
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: 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-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/Sell/Sell.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 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/Sell/Sell.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/Buy/Buy.tsx
📚 Learning: 2025-12-11T12:40:09.964Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:09.964Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
Applied to files:
src/apps/pulse/components/Sell/Sell.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/Sell/Sell.tsxsrc/apps/pulse/components/Buy/Buy.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/Sell/Sell.tsxsrc/apps/pulse/components/Buy/Buy.tsx
🧬 Code graph analysis (2)
src/apps/pulse/components/Sell/Sell.tsx (2)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/utils/number.tsx (1)
formatExponentialSmallNumber(119-149)
src/apps/pulse/components/Buy/Buy.tsx (1)
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: unit-tests
- GitHub Check: lint
- GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (2)
src/apps/pulse/components/Buy/Buy.tsx (1)
644-719: Token selector UI refactor looks solid.The absolute-positioned layout with fixed logo area, symbol/name row, price/change row, and chevron is well-structured. The triangle indicator logic correctly shows directional change, and the chain logo ring creates a clean visual separation.
src/apps/pulse/components/Sell/Sell.tsx (1)
303-404: Token selector UI is well-implemented with proper formatting.The absolute-positioned layout mirrors the Buy component structure. The use of
formatExponentialSmallNumberfor the price (line 352) correctly handles very small token values. The placeholder state includes therelativeclass (line 389), properly addressing the earlier review feedback.
|
|
||
| {/* Top Row: Symbol and Name */} | ||
| <div className="absolute left-[36px] top-[6px] flex items-center gap-[4px] max-w-[90px]"> | ||
| <p className="font-['Poppins'] font-normal text-[12px] leading-[12px] tracking-[-0.02em] text-white truncate shrink-0 max-w-[50px]"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once again no need font-['Poppins'] there as this is the default font anyway
|
|
||
| {/* Bottom Row: Price and Change */} | ||
| <div className="absolute left-[36px] top-[20px] flex items-center gap-[6px]"> | ||
| <p className="font-['Poppins'] font-normal text-[10px] leading-[10px] tracking-[-0.02em] text-white opacity-50"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment here about font-poppins
| ${token.usdValue} | ||
|
|
||
| <p | ||
| className={`font-['Poppins'] font-normal text-[10px] leading-[10px] tracking-[-0.02em] opacity-50 ${ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
| : 'text-[#FF366C]' | ||
| }`} | ||
| > | ||
| {Math.abs(token.dailyPriceChange).toFixed(1)}% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was it meant to be toFixed(1) ? or toFixed(2)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toFixed(2) changed
| <div className="flex items-center absolute left-[12px] top-[10px]"> | ||
| <TailSpin width={16} height={16} /> | ||
| <div className="flex font-normal desktop:text-sm tablet:text-sm mobile:text-xs xs:text-xs"> | ||
| <div className="ml-2 font-['Poppins'] font-normal text-[12px] leading-[12px] tracking-[-0.02em] text-white opacity-50"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same for font-poppins here
| /> | ||
| </div> | ||
| </> | ||
| <div className="absolute left-[12px] top-[12px] font-['Poppins'] font-normal text-[12px] leading-[12px] tracking-[-0.02em] text-white"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok that's the last font-Poppins I am suggesting here, but please check if it is applied anywhere else
| }`} | ||
| > | ||
| ${formatExponentialSmallNumber(token.usdValue)} | ||
| {Math.abs(token.dailyPriceChange).toFixed(1)}% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just so you know, this change will not be able to cover exponential number so very small values that are like '1e16'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exponential number is fine changed to toFixed(2)
| store.dispatch(setUsdPriceSwapToken(0.1)); | ||
| store.dispatch(setUsdPriceReceiveToken(100)); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah deleting a test is never a solution.... this should be added back on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/apps/insights/components/PnLDetailModal.tsx (1)
483-483: Remove stray::artifact from JSX.The
::at the end of line 483 will render as visible text in the dialog. This appears to be the same issue flagged in a previous review comment (at the old line 752), but it remains unresolved.🔎 Apply this diff to remove the artifact:
- <DialogContent className="w-full sm:w-full max-w-[calc(100vw-2rem)] sm:max-w-4xl mx-4 sm:mx-0 h-[92vh] sm:h-auto max-h-[90vh] overflow-y-auto rounded-3xl sm:rounded-3xl p-5 sm:p-7 glass-card border-primary/20">:: + <DialogContent className="w-full sm:w-full max-w-[calc(100vw-2rem)] sm:max-w-4xl mx-4 sm:mx-0 h-[92vh] sm:h-auto max-h-[90vh] overflow-y-auto rounded-3xl sm:rounded-3xl p-5 sm:p-7 glass-card border-primary/20">
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/apps/pulse/components/App/tests/__snapshots__/AppWrapper.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/App/tests/__snapshots__/HomeScreen.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/Buy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Search/tests/__snapshots__/PortfolioTokenList.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/Sell.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
src/apps/insights/components/PnLDetailModal.tsx(13 hunks)src/apps/pulse/components/Buy/Buy.tsx(1 hunks)src/apps/pulse/components/Search/PortfolioTokenList.tsx(3 hunks)src/apps/pulse/components/Sell/Sell.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:09.964Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
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: 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: 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.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/PortfolioTokenList.tsxsrc/apps/pulse/components/Buy/Buy.tsxsrc/apps/pulse/components/Sell/Sell.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/PortfolioTokenList.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/Search/PortfolioTokenList.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 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/PortfolioTokenList.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/PortfolioTokenList.tsxsrc/apps/pulse/components/Buy/Buy.tsxsrc/apps/pulse/components/Sell/Sell.tsx
📚 Learning: 2025-12-11T12:40:09.964Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:09.964Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
Applied to files:
src/apps/pulse/components/Search/PortfolioTokenList.tsxsrc/apps/pulse/components/Buy/Buy.tsxsrc/apps/pulse/components/Sell/Sell.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/Buy/Buy.tsxsrc/apps/pulse/components/Sell/Sell.tsx
📚 Learning: 2025-12-03T10:01:15.801Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 471
File: src/apps/pillarx-app/components/AlgoInsightsTile/AlgoInsightsTile.tsx:36-48
Timestamp: 2025-12-03T10:01:15.801Z
Learning: In the AlgoInsightsTile component (src/apps/pillarx-app/components/AlgoInsightsTile/AlgoInsightsTile.tsx), the data is always hardcoded and guaranteed to be present with complete history arrays for all timeframes, so edge case handling for empty or sparse history data is not required.
Applied to files:
src/apps/insights/components/PnLDetailModal.tsx
🧬 Code graph analysis (3)
src/apps/pulse/components/Buy/Buy.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
src/apps/insights/components/PnLDetailModal.tsx (4)
src/apps/insights/components/ui/table.tsx (2)
TableRow(72-72)TableCell(72-72)src/apps/insights/utils/formatUtils.ts (1)
formatPrice(18-21)src/apps/insights/lib/stopLossUtils.ts (1)
getEffectiveStopLoss(25-33)src/apps/insights/components/ui/badge.tsx (1)
Badge(29-29)
src/apps/pulse/components/Sell/Sell.tsx (2)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)src/utils/number.tsx (1)
formatExponentialSmallNumber(119-149)
⏰ 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 (3)
src/apps/pulse/components/Search/PortfolioTokenList.tsx (1)
318-348: LGTM! Redundant font declarations removed.The removal of
font-['Poppins']from the three header labels is correct since Poppins is already the default font for the application. This cleanup aligns with the broader PR styling consistency improvements.src/apps/pulse/components/Sell/Sell.tsx (1)
304-404: Token selector layout refactor looks good.The conversion to absolute positioning with fixed dimensions (113x36px) creates a consistent, compact token selector. The structure includes:
- Absolute logo container with chain badge and ring styling
- Top row: symbol and name with proper truncation
- Bottom row: USD value and daily price change with color-coded triangle indicator
- Right-aligned chevron in both selected and placeholder states
The use of
formatExponentialSmallNumberat line 352 for the USD value display ensures very small values render correctly.src/apps/pulse/components/Buy/Buy.tsx (1)
645-742: Token selector layout is consistent with Sell component.The token selector refactor mirrors the Sell component structure with:
- Fixed 113x36px dimensions
- Absolute positioning for logo, text rows, and chevron
- Top row: symbol and name
- Bottom row: USD value and daily price change with triangle indicator
- Loading state in placeholder ("Searching...")
The layout and visual treatment are well-structured and consistent across Buy/Sell flows.
| {/* Bottom Row: Price and Change */} | ||
| <div className="absolute left-[36px] top-[20px] flex items-center gap-[6px]"> | ||
| <p className="font-normal text-[10px] leading-[10px] tracking-[-0.02em] text-white opacity-50"> | ||
| ${token.usdValue} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent USD value formatting compared to Sell component.
Line 684 displays token.usdValue directly without formatting, but the Sell component (line 352) uses formatExponentialSmallNumber(token.usdValue). This inconsistency could cause display issues for very small values (e.g., tokens priced at 1e-8) where:
- Buy would show: "1e-8"
- Sell would show: "0.00000001"
🔎 Apply this diff to match Sell component formatting:
- <p className="font-normal text-[10px] leading-[10px] tracking-[-0.02em] text-white opacity-50">
- ${token.usdValue}
+ <p className="font-normal text-[10px] leading-[10px] tracking-[-0.02em] text-white opacity-50">
+ ${formatExponentialSmallNumber(token.usdValue)}
</p>Note: formatExponentialSmallNumber is already imported at line 18.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ${token.usdValue} | |
| <p className="font-normal text-[10px] leading-[10px] tracking-[-0.02em] text-white opacity-50"> | |
| ${formatExponentialSmallNumber(token.usdValue)} | |
| </p> |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Buy/Buy.tsx around line 684, the USD value is
rendered directly as token.usdValue which is inconsistent with the Sell
component; replace the direct usage with the shared formatter by calling
formatExponentialSmallNumber(token.usdValue) so very small values render like
the Sell component; formatExponentialSmallNumber is already imported at the top
so just swap the displayed expression to use that function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vignesha22 Just this to review otherwise approved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has spoken to Aldin and he says exponential value is fine so merging it now
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.