Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis update introduces a new "Tokens With Market Data" tile feature to the codebase. It adds several new React components for rendering token market data rows, including specialized components for left and right columns and token logos with fallback and overlay logic. The tile is integrated into the component mapping and type system, with new types and enum values defined to support structured token market data. Supporting utility functions and Tailwind color configuration are also added. Comprehensive tests accompany each new component to verify rendering and edge cases. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TokensWithMarketDataTile
participant TokenMarketDataRow
participant LeftColumnTokenMarketDataRow
participant RightColumnTokenMarketDataRow
participant TokenLogoMarketDataRow
User->>TokensWithMarketDataTile: Renders with data
TokensWithMarketDataTile->>TokenMarketDataRow: For each row, render
TokenMarketDataRow->>TokenLogoMarketDataRow: Render token and chain logos
TokenMarketDataRow->>LeftColumnTokenMarketDataRow: Render left column data
TokenMarketDataRow->>RightColumnTokenMarketDataRow: Render right column data
Possibly related PRs
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying x with
|
| Latest commit: |
b0ffb03
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7d24dfb9.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3225-new-token-tile.x-e62.pages.dev |
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3225/new-token-tiles)
![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/apps/pillarx-app/components/TileTitle/TitleTitle.tsx (1)
1-43: TileTitle component implementation looks good but filename appears redundantThe component correctly handles optional title and decorators with proper image error handling to prevent broken images from displaying.
The filename "TitleTitle.tsx" seems redundant. Consider renaming to "TileTitle.tsx" to match the component name.
src/apps/pillarx-app/components/TokenMarketDataRow/TokenMarketDataRow.tsx (2)
19-25: Consider adding error handling for navigation and extracting conditional styling logic.The click handler doesn't include error handling for navigation. Additionally, the conditional border styling logic is complex and embedded directly in the className.
Consider extracting the border styling logic to a separate function:
- <div - className={`flex w-full h-full items-center justify-between gap-2 py-3 border-b-[1px] border-[#25232D] - ${listNumber === 8 && 'desktop:border-b-0 tablet:border-b-0 mobile:border-b-0'} - ${listNumber === 4 && 'desktop:border-b-0'} - ${data.link && 'cursor-pointer'}`} - onClick={() => (data.link ? navigate(`${data.link}`) : undefined)} - > + <div + className={`flex w-full h-full items-center justify-between gap-2 py-3 border-b-[1px] border-[#25232D] + ${getBorderClasses(listNumber)} + ${data.link && 'cursor-pointer'}`} + onClick={() => handleRowClick(data.link)} + >Then add these functions:
const getBorderClasses = (listNumber) => { if (listNumber === 8) return 'desktop:border-b-0 tablet:border-b-0 mobile:border-b-0'; if (listNumber === 4) return 'desktop:border-b-0'; return ''; }; const handleRowClick = (link) => { if (!link) return; try { navigate(`${link}`); } catch (error) { console.error('Navigation error:', error); } };
20-20: Consider using theme variables instead of hardcoded colors.The border color
#25232Dis hardcoded, which might create inconsistency if the theme colors change.If you have a theming system, consider using theme variables:
- className={`flex w-full h-full items-center justify-between gap-2 py-3 border-b-[1px] border-[#25232D] + className={`flex w-full h-full items-center justify-between gap-2 py-3 border-b-[1px] border-border-colorWhere
border-colorwould be defined in your Tailwind theme.src/apps/pillarx-app/components/TokenMarketDataRow/RightColumnTokenMarketDataRow.tsx (2)
24-27: Simplify complex conditional class names.The conditional class logic is complex and might be difficult to maintain.
Extract the logic to helper functions:
- <Body - className={`font-normal ${rightColumn?.line1?.direction === 'UP' && 'desktop:text-market_row_green tablet:text-market_row_green'} ${rightColumn?.line1?.direction === 'DOWN' && 'desktop:text-percentage_red tablet:text-percentage_red'} mobile:text-white desktop:text-base tablet:text-base mobile:text-sm`} - > + <Body + className={`font-normal ${getDirectionTextColor(rightColumn?.line1?.direction)} mobile:text-white desktop:text-base tablet:text-base mobile:text-sm`} + >Add this helper function:
const getDirectionTextColor = (direction) => { if (direction === 'UP') return 'desktop:text-market_row_green tablet:text-market_row_green'; if (direction === 'DOWN') return 'desktop:text-percentage_red tablet:text-percentage_red'; return ''; };
32-40: Simplify direction check condition.The condition checks for both 'UP' and 'DOWN' when the icon color is determined by another condition anyway.
- {(rightColumn?.line1?.direction === 'UP' || - rightColumn?.line1?.direction === 'DOWN') && ( + {rightColumn?.line1?.direction && ( <TbTriangleFilled size={6} color={ rightColumn?.line1?.direction === 'UP' ? '#5CFF93' : '#FF366C' } /> )}src/apps/pillarx-app/components/TokenMarketDataRow/LeftColumnTokenMarketDataRow.tsx (1)
25-42: Consider extracting timestamp formatting to a utility functionThe timestamp formatting logic is complex and could be reused elsewhere. Consider extracting it to a separate utility function.
+ // utils/formatters.ts + export const formatRelativeTime = (timestamp: number): string => { + let formattedTime = formatDistanceToNowStrict( + DateTime.fromSeconds(timestamp || 0).toISO() || '', + { addSuffix: true } + ); + + // Replace long units with shorter units and delete white space before the units + return formattedTime + .replace('seconds', 's') + .replace('second', 's') + .replace('minutes', 'min') + .replace('minute', 'min') + .replace('hours', 'h') + .replace('hour', 'h') + .replace('days', 'd') + .replace('day', 'd') + .replace('months', 'mo') + .replace('month', 'mo') + .replace(/(\d+)\s+(?=[a-zA-Z])/g, '$1'); + };Then in your component:
- let timestamp = formatDistanceToNowStrict( - DateTime.fromSeconds(leftColumn?.line2?.timestamp || 0).toISO() || '', - { addSuffix: true } - ); - - // Replace long units with shorter units and delete white space before the units - timestamp = timestamp - .replace('seconds', 's') - .replace('second', 's') - .replace('minutes', 'min') - .replace('minute', 'min') - .replace('hours', 'h') - .replace('hour', 'h') - .replace('days', 'd') - .replace('day', 'd') - .replace('months', 'mo') - .replace('month', 'mo') - .replace(/(\d+)\s+(?=[a-zA-Z])/g, '$1'); + const timestamp = formatRelativeTime(leftColumn?.line2?.timestamp || 0);src/apps/pillarx-app/components/TokenMarketDataRow/TokenLogoMarketDataRow.tsx (2)
31-41: Add validation for tokenName when using it for fallback displayThe component should validate that tokenName is not empty before using it for initials.
{(!tokenLogo || isBrokenImage) && ( <span className="absolute inset-0 flex items-center justify-center text-white text-base font-normal"> - {tokenName?.slice(0, 2)} + {tokenName?.trim() ? tokenName.slice(0, 2) : '??'} </span> )}
44-53: Add a data-testid attribute to chain logo for testingFor consistency with the token logo, add a data-testid attribute to the chain logo image.
<img src={chainLogo} alt="logo" className="w-full h-full object-contain" + data-testid="chain-logo" onError={() => setIsBrokenImageChain(true)} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
src/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/PointsTile/test/__snapshots__/PointsTile.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokenMarketDataRow/tests/__snapshots__/LeftColumnTokenMarketDataRow.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokenMarketDataRow/tests/__snapshots__/RightColumnTokenMarketDataRow.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokensWithMarketDataTile/test/__snapshots__/TokensWithMarketDataTile.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/images/token-market-data-copy.pngis excluded by!**/*.png
📒 Files selected for processing (12)
src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx(1 hunks)src/apps/pillarx-app/components/TokenMarketDataRow/LeftColumnTokenMarketDataRow.tsx(1 hunks)src/apps/pillarx-app/components/TokenMarketDataRow/RightColumnTokenMarketDataRow.tsx(1 hunks)src/apps/pillarx-app/components/TokenMarketDataRow/TokenLogoMarketDataRow.tsx(1 hunks)src/apps/pillarx-app/components/TokenMarketDataRow/TokenMarketDataRow.tsx(1 hunks)src/apps/pillarx-app/components/TokenMarketDataRow/tests/LeftColumnTokenMarketDataRow.test.tsx(1 hunks)src/apps/pillarx-app/components/TokenMarketDataRow/tests/RightColumnTokenMarketDataRow.test.tsx(1 hunks)src/apps/pillarx-app/components/TokensWithMarketDataTile/TokensWithMarketDataTile.tsx(1 hunks)src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx(1 hunks)src/apps/pillarx-app/tailwind.pillarx.config.js(1 hunks)src/apps/pillarx-app/utils/configComponent.ts(2 hunks)src/types/api.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx (2)
src/types/api.ts (1)
TileTitle(354-356)src/apps/simpleswap/components/NewCoinsDropdown/styles.ts (1)
Body(29-35)
src/apps/pillarx-app/components/TokensWithMarketDataTile/TokensWithMarketDataTile.tsx (1)
src/types/api.ts (3)
Projection(173-185)TokensMarketData(164-171)TileTitle(354-356)
🪛 Biome (1.9.4)
src/apps/pillarx-app/components/TokensWithMarketDataTile/TokensWithMarketDataTile.tsx
[error] 47-62: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: Cloudflare Pages
🔇 Additional comments (25)
src/apps/pillarx-app/tailwind.pillarx.config.js (1)
25-25: Positive market indicator color addition looks good!The new color
market_row_green: '#5CFF93'is appropriately added to the color palette and follows the existing naming conventions. This will be used for styling positive market direction indicators in the token market data components.src/types/api.ts (4)
9-9: New ApiLayout enum value added correctlyAdding
TOKENS_WITH_MARKET_DATAto the ApiLayout enum properly extends the available layout options for the application, allowing for the new token tiles feature.
134-162: Well-structured TokensMarketDataRow type definitionThe type definition is comprehensive with all fields marked as optional, which provides flexibility when implementing this feature. The nested structure clearly separates token display concerns between left and right columns.
164-171: TokensMarketData type is properly organizedThe type includes an optional title with configurable decorators and a collection of rows. This structure aligns well with the UI components that will render this data.
177-182: Projection type correctly updated to include TokensMarketDataThe union type has been properly extended to include the new TokensMarketData type, ensuring that the API projection system can handle the new token market data tile format.
src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx (2)
16-17: Good use of useState for image error handlingUsing state to track broken images is a good practice. It prevents the component from attempting to render broken images, which improves user experience.
20-38: Well-structured component rendering with proper conditional logicThe component's render logic is clean with appropriate conditional rendering based on prop availability and image loading state. The Tailwind classes are appropriate for the layout.
src/apps/pillarx-app/utils/configComponent.ts (2)
12-12: Import for the new TokensWithMarketDataTile component added correctlyThe import statement follows the existing pattern and imports the new tile component.
26-26: New component correctly registered in the component mapThe TokensWithMarketDataTile component is properly mapped to the ApiLayout.TOKENS_WITH_MARKET_DATA enum value, ensuring it can be rendered when this layout type is specified.
src/apps/pillarx-app/components/TokenMarketDataRow/tests/LeftColumnTokenMarketDataRow.test.tsx (1)
1-71: Good test coverage for the LeftColumnTokenMarketDataRow component.The test suite is well-structured with a comprehensive mock data object and tests that cover:
- Component rendering and snapshot matching
- Presence of key text elements
- Rendering of volume and liquidity information
- Graceful handling of missing data
This follows good testing practices for React components with appropriate assertions.
src/apps/pillarx-app/components/TokenMarketDataRow/tests/RightColumnTokenMarketDataRow.test.tsx (1)
1-80: Well-structured test suite that covers both happy paths and edge cases.The tests thoroughly verify the component's behavior with:
- Standard rendering and snapshot matching
- Verification of price, percentage, and transaction count display
- Handling of missing percentage data
- Graceful degradation when line1 data is entirely missing
This comprehensive approach ensures the component will behave predictably with varying data inputs.
src/apps/pillarx-app/components/TokenMarketDataRow/TokenMarketDataRow.tsx (1)
30-34: 🛠️ Refactor suggestionAdd null checks for data.leftColumn properties.
The component doesn't check if
data.leftColumnexists before accessing its nested properties, which could cause runtime errors if the data structure is incomplete.Add null checks to avoid potential errors:
<TokenLogoMarketDataRow - tokenLogo={data.leftColumn?.token?.primaryImage} - chainLogo={data.leftColumn?.token?.secondaryImage} - tokenName={data.leftColumn?.line1?.text2} + tokenLogo={data.leftColumn?.token?.primaryImage || undefined} + chainLogo={data.leftColumn?.token?.secondaryImage || undefined} + tokenName={data.leftColumn?.line1?.text2 || undefined} />Likely an incorrect or invalid review comment.
src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx (7)
14-86: LGTM: Good test mock data setupThe mock data is comprehensive and well-structured, providing a clear representation of the expected data format for testing.
88-99: Good snapshot test implementationThe snapshot test is properly implemented within a MemoryRouter to handle the links correctly.
101-114: Well-defined title and decorators testThis test provides good coverage for the TileTitle component integration.
116-140: Thorough data rendering testThe test effectively verifies all token data fields are rendered correctly for both token rows.
142-151: Loading state handled properlyThe test correctly verifies the component doesn't render during loading state.
153-169: Empty data case handled correctlyGood test case to verify the component doesn't render when rows are empty.
171-190: Row limit test is valuableExcellent test case to verify the component respects the 8-row limit, which matches the implementation logic.
src/apps/pillarx-app/components/TokensWithMarketDataTile/TokensWithMarketDataTile.tsx (3)
9-12: LGTM: Clear prop type definitionThe prop types are clearly defined for the component, making it easy to understand the expected inputs.
22-31: Clean data preparation and handlingThe row limiting and column splitting logic is clear, and the null rendering for edge cases is appropriate.
39-43: Good component compositionThe TileTitle component is properly used with all the necessary props.
src/apps/pillarx-app/components/TokenMarketDataRow/LeftColumnTokenMarketDataRow.tsx (1)
69-86: Good responsive design for secondary informationThe layout for timestamp, volume and liquidity is well-structured with appropriate visibility rules for mobile.
src/apps/pillarx-app/components/TokenMarketDataRow/TokenLogoMarketDataRow.tsx (2)
17-19: Good use of state for handling broken imagesUsing separate state variables for token and chain logos properly handles broken image scenarios.
20-55: Well-structured token logo component with proper fallbacksThe component handles image loading, broken images, and fallbacks appropriately. The responsive design is also well-implemented.
src/apps/pillarx-app/components/TokenMarketDataRow/RightColumnTokenMarketDataRow.tsx
Show resolved
Hide resolved
src/apps/pillarx-app/components/TokenMarketDataRow/RightColumnTokenMarketDataRow.tsx
Show resolved
Hide resolved
src/apps/pillarx-app/components/TokensWithMarketDataTile/TokensWithMarketDataTile.tsx
Outdated
Show resolved
Hide resolved
src/apps/pillarx-app/components/TokensWithMarketDataTile/TokensWithMarketDataTile.tsx
Show resolved
Hide resolved
src/apps/pillarx-app/components/TokenMarketDataRow/LeftColumnTokenMarketDataRow.tsx
Show resolved
Hide resolved
src/apps/pillarx-app/components/TokenMarketDataRow/LeftColumnTokenMarketDataRow.tsx
Show resolved
Hide resolved
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3225/new-token-tiles)
IAmKio
left a comment
IAmKio
left a comment
There was a problem hiding this comment.
Just a few questions and concerns but otherwise great work!
There was a problem hiding this comment.
Is the file name supposed to be TileTitle?
There was a problem hiding this comment.
Yes :) I thought this could be in the future a re-usable component for any Tile Title
| timestamp = timestamp | ||
| .replace('seconds', 's') | ||
| .replace('second', 's') | ||
| .replace('minutes', 'min') | ||
| .replace('minute', 'min') | ||
| .replace('hours', 'h') | ||
| .replace('hour', 'h') | ||
| .replace('days', 'd') | ||
| .replace('day', 'd') | ||
| .replace('months', 'mo') | ||
| .replace('month', 'mo') | ||
| .replace(/(\d+)\s+(?=[a-zA-Z])/g, '$1'); |
There was a problem hiding this comment.
This is quite useful as a utility - could be moved into a utils file?
src/apps/pillarx-app/components/TokenMarketDataRow/LeftColumnTokenMarketDataRow.tsx
Show resolved
Hide resolved
| > | ||
| <div className="flex items-center flex-1 min-w-0"> | ||
| <Body className="desktop:text-base tablet:text-base mobile:text-sm font-normal text-white/[0.5] mr-4 mobile:mr-2.5"> | ||
| 0{listNumber} |
There was a problem hiding this comment.
Just a note on the 0 here - this list, in future, could be more than 9 entries - could we remove the 0?
There was a problem hiding this comment.
Ok I have now made changes to make this work
| // Limit the rows to the first 8 items | ||
| const limitedRows = dataTokens?.rows?.slice(0, 8) || []; | ||
|
|
||
| // Divide the rows into two separate columns | ||
| const leftColumn = limitedRows.slice(0, 4); | ||
| const rightColumn = limitedRows.slice(4, 8); |
There was a problem hiding this comment.
Okay i can see what's going on here (see my previous comment) - we definitely need to cater fo the fact that there may be a situation where we want to show more than 8 entries... we can check the requirement but i think it's a better idea to accommodate for bigger lists just incase
There was a problem hiding this comment.
Ok I have now made changes to make this work
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/utils/common.ts (1)
43-57: Improve the time unit abbreviation function.The new
getShorterTimeUnitsfunction works correctly for basic cases, but there are some improvements that could be made:
Consider a more descriptive name like
abbreviateTimeUnitsorshortenTimeFormatsince the function transforms a string rather than "getting" units.The multiple
.replace()calls could be replaced with a more efficient approach using a mapping object:-export const getShorterTimeUnits = (formattedDistanceToNowDate: string) => { - // Replace long units with shorter units and delete white space before the units - return formattedDistanceToNowDate - .replace('seconds', 's') - .replace('second', 's') - .replace('minutes', 'min') - .replace('minute', 'min') - .replace('hours', 'h') - .replace('hour', 'h') - .replace('days', 'd') - .replace('day', 'd') - .replace('months', 'mo') - .replace('month', 'mo') - .replace(/(\d+)\s+(?=[a-zA-Z])/g, '$1'); -}; +export const getShorterTimeUnits = (formattedDistanceToNowDate: string) => { + // Map of time units to their shorter forms + const unitMap: Record<string, string> = { + seconds: 's', + second: 's', + minutes: 'min', + minute: 'min', + hours: 'h', + hour: 'h', + days: 'd', + day: 'd', + months: 'mo', + month: 'mo', + }; + + // Replace long units with shorter units + let shortened = formattedDistanceToNowDate; + Object.entries(unitMap).forEach(([longUnit, shortUnit]) => { + shortened = shortened.replace(new RegExp(longUnit, 'g'), shortUnit); + }); + + // Remove spaces between numbers and units + return shortened.replace(/(\d+)\s+(?=[a-zA-Z])/g, '$1'); +};
Consider adding some basic validation for the input string.
Add unit tests to verify the function works with various inputs, including edge cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/PointsTile/test/__snapshots__/PointsTile.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokenMarketDataRow/tests/__snapshots__/LeftColumnTokenMarketDataRow.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokensWithMarketDataTile/test/__snapshots__/TokensWithMarketDataTile.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
src/apps/pillarx-app/components/TokenMarketDataRow/LeftColumnTokenMarketDataRow.tsx(1 hunks)src/apps/pillarx-app/components/TokenMarketDataRow/TokenMarketDataRow.tsx(1 hunks)src/apps/pillarx-app/components/TokensWithMarketDataTile/TokensWithMarketDataTile.tsx(1 hunks)src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx(1 hunks)src/utils/common.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/apps/pillarx-app/components/TokensWithMarketDataTile/TokensWithMarketDataTile.tsx
- src/apps/pillarx-app/components/TokensWithMarketDataTile/test/TokensWithMarketDataTile.test.tsx
- src/apps/pillarx-app/components/TokenMarketDataRow/TokenMarketDataRow.tsx
- src/apps/pillarx-app/components/TokenMarketDataRow/LeftColumnTokenMarketDataRow.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
- GitHub Check: Cloudflare Pages
github-actions
bot
requested a deployment
to
Preview (feat/PRO-3225/new-token-tiles)
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3225/new-token-tiles)
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3225/new-token-tiles)
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Style
Bug Fixes
Tests