feat/PRO-3165/placeholders-tokens-no-logo#276
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 PR introduces several improvements across multiple components. In the grid component, a new variable filters out invalid grid items before rendering. The token display components (HorizontalToken, TokenInfoHorizontal, TokensHorizontalTile, and TokensVerticalList) now accept an optional Changes
Sequence Diagram(s)sequenceDiagram
participant T as Token Component
participant Q as Blockchain Query (useGetBlockchainsListQuery)
participant E as Effect Hook
participant S as State (blockchainLogo)
T->>Q: Fetch blockchain list data
Q-->>T: Return blockchain list
T->>E: Trigger useEffect (if tokenChains length == 1)
E->>S: Check for matching blockchain logo
S-->>T: Update blockchainLogo state
T->>T: Render token logo or overlay fallback text
sequenceDiagram
participant Test as Test Runner
participant P as Redux Provider
participant R as MemoryRouter
participant C as Component Under Test
Test->>P: Wrap component with Redux Provider
P->>R: Nest within MemoryRouter
R->>C: Render component with Redux state context
C-->>Test: Component renders with proper context
Possibly related PRs
Suggested reviewers
Poem
🪧 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: |
39a9999
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2e6f4f6c.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3165-placeholders-t.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/apps/token-atlas/components/TokenCard/TokenCard.tsx (1)
46-46: Potential issue with RandomAvatar name propThe fallback logic for the RandomAvatar component could be improved to handle undefined tokenName values more gracefully.
- <RandomAvatar name={`${tokenName}-chain` || ''} /> + <RandomAvatar name={tokenName ? `${tokenName}-chain` : ''} />src/apps/token-atlas/components/TokenGraphColumn/TokenGraphColumn.tsx (1)
135-155: Enhanced token logo display with text fallbackThe restructured component follows the same pattern as other token logo components, providing better visual consistency and a text fallback when no logo is available.
There appears to be conflicting text size classes on line 151:
text-lg text-xs. Consider choosing one text size class to avoid potential styling conflicts.- <span className="absolute inset-0 flex items-center justify-center text-lg text-xs font-bold"> + <span className="absolute inset-0 flex items-center justify-center text-xs font-bold">src/apps/pillarx-app/components/TokenInfoHorizontal/test/TokenInfoHorizontal.test.tsx (1)
17-32: Tests properly updated for Redux integration.The tests have been correctly modified to wrap components in Redux Provider, which is necessary for components that use Redux hooks.
However, there's some inconsistency in the testing approach - some tests use
@testing-library/reactmethods while others still use direct renderer access. Consider standardizing on one approach for better maintainability.Consider migrating all tests to use
@testing-library/reactmethods for consistency:- const tree = renderer.create(...).toJSON() as ReactTestRendererJSON; - const tokenNameProp = tree.children?.find(...) as ReactTestRendererJSON; - expect(tokenNameProp.children).toContain(tokenName); + render(<Provider store={store}><TokenInfoHorizontal ... /></Provider>); + expect(screen.getByText(tokenName)).toBeInTheDocument();Also applies to: 34-50, 52-67, 69-89, 91-108, 110-131, 133-151, 153-171
src/apps/pillarx-app/components/HorizontalToken/test/HorizontalToken.test.tsx (1)
25-43: Tests properly updated for Redux integration.All tests have been correctly modified to wrap the HorizontalToken component in a Redux Provider, which is necessary for components that use Redux hooks like useGetBlockchainsListQuery.
However, similar to the TokenInfoHorizontal tests, there's inconsistency in the testing approach. Some tests use modern
@testing-library/reactmethods while others still use direct renderer access.Consider standardizing on
@testing-library/reactfor all tests:- const tree = renderer.create(...).toJSON() as ReactTestRendererJSON; - const divElement = tree.children?.filter(...) as ReactTestRendererJSON[]; - const tokenValueProp = divElement[1].children?.find(...) as ReactTestRendererJSON; - expect(tokenValueProp.children).toContain(`${tokenValue.toFixed(4)}`); + render(<Provider store={store}><HorizontalToken ... /></Provider>); + expect(screen.getByText(`$${tokenValue.toFixed(4)}`)).toBeInTheDocument();Also applies to: 45-62, 64-82, 84-100, 102-129, 131-158, 160-180
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
src/apps/pillarx-app/components/HorizontalToken/test/__snapshots__/HorizontalToken.test.tsx.snapis excluded by!**/*.snapsrc/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/TokenInfoHorizontal/test/__snapshots__/TokenInfoHorizontal.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokensHorizontalTile/test/__snapshots__/TokensHorizontalTile.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokensVerticalList/test/__snapshots__/TokensVerticalList.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/TokensVerticalTile/test/__snapshots__/TokensVerticalTile.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/CardsSwap/test/__snapshots__/CardSwap.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/DropdownTokensList/test/__snapshots__/DropdownTokensList.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/ExchangeAction/test/__snapshots__/ExchangeAction.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/SelectToken/test/__snapshots__/SelectToken.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/TokenListItem/test/__snapshots__/TokenListItem.test.tsx.snapis excluded by!**/*.snapsrc/apps/the-exchange/components/TokenLogo/test/__snapshots__/TokenLogo.test.tsx.snapis excluded by!**/*.snapsrc/apps/token-atlas/components/TokenCard/test/__snapshots__/TokenCard.test.tsx.snapis excluded by!**/*.snapsrc/apps/token-atlas/components/TokenGraphColumn/test/__snapshots__/TokenGraphColumn.test.tsx.snapis excluded by!**/*.snapsrc/apps/token-atlas/components/TokenResultCard/test/__snapshots__/TokenResultCard.test.tsx.snapis excluded by!**/*.snapsrc/apps/token-atlas/components/TokensSearchResult/test/__snapshots__/TokensSearchResult.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
src/apps/pillarx-app/components/HighlightedMediaGridTile/HighlightedMediaGridTile.tsx(1 hunks)src/apps/pillarx-app/components/HorizontalToken/HorizontalToken.tsx(4 hunks)src/apps/pillarx-app/components/HorizontalToken/test/HorizontalToken.test.tsx(7 hunks)src/apps/pillarx-app/components/TokenInfoHorizontal/TokenInfoHorizontal.tsx(3 hunks)src/apps/pillarx-app/components/TokenInfoHorizontal/test/TokenInfoHorizontal.test.tsx(6 hunks)src/apps/pillarx-app/components/TokensHorizontalTile/TokensHorizontalTile.tsx(1 hunks)src/apps/pillarx-app/components/TokensHorizontalTile/test/TokensHorizontalTile.test.tsx(4 hunks)src/apps/pillarx-app/components/TokensVerticalList/TokensVerticalList.tsx(1 hunks)src/apps/pillarx-app/components/TokensVerticalTile/test/TokensVerticalTile.test.tsx(3 hunks)src/apps/the-exchange/components/TokenLogo/TokenLogo.tsx(1 hunks)src/apps/the-exchange/components/TokenLogo/test/TokenLogo.test.tsx(1 hunks)src/apps/token-atlas/components/TokenCard/TokenCard.tsx(1 hunks)src/apps/token-atlas/components/TokenCard/test/TokenCard.test.tsx(1 hunks)src/apps/token-atlas/components/TokenGraphColumn/TokenGraphColumn.tsx(1 hunks)src/apps/token-atlas/components/TokenResultCard/TokenResultCard.tsx(1 hunks)src/apps/token-atlas/components/TokensSlider/TokensSlider.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
src/apps/pillarx-app/components/HorizontalToken/HorizontalToken.tsx (1)
src/types/api.ts (1)
TokenContract(150-155)
src/apps/pillarx-app/components/TokenInfoHorizontal/TokenInfoHorizontal.tsx (1)
src/types/api.ts (1)
TokenContract(150-155)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: Cloudflare Pages
🔇 Additional comments (27)
src/apps/token-atlas/components/TokenCard/test/TokenCard.test.tsx (1)
58-58: Test expectations updated correctly.The test now expects 1 random avatar instead of 2, matching the updated component behavior for rendering when no logos are provided.
src/apps/pillarx-app/components/TokensVerticalList/TokensVerticalList.tsx (1)
35-35: New tokenChains prop enhances the token display.The addition of the
tokenChainsprop to the HorizontalToken component enables blockchain information to be passed through, allowing for the display of chain logos as mentioned in the PR objectives.src/apps/pillarx-app/components/TokensHorizontalTile/TokensHorizontalTile.tsx (1)
78-78: TokenInfoHorizontal now includes chain information.The addition of the
tokenChainsprop is consistent with the PR objectives to update token logo placeholders and chain logos. This change enables the component to display blockchain information.src/apps/pillarx-app/components/HighlightedMediaGridTile/HighlightedMediaGridTile.tsx (2)
55-58: Improved filtering logic for grid items.The new
validGridsItemsvariable filters out grids with no valid items, enhancing the validation logic before rendering.
64-68: Enhanced rendering conditions improve component robustness.The updated conditional check now includes verification for empty
validGridsItems, preventing attempts to render with invalid data structures.src/apps/pillarx-app/components/TokensVerticalTile/test/TokensVerticalTile.test.tsx (3)
4-6: Proper Redux Provider setup addedThe addition of Redux Provider imports is appropriate and follows best practices for testing components that rely on Redux state.
52-57: Correctly wrapped component with Redux ProviderThe component is now properly wrapped with the Redux Provider, ensuring it has access to the Redux store during testing. This change aligns with the PR's focus on enhancing token display components that need store access.
77-81: Consistent Redux Provider wrapping across testsThe empty data test case now consistently uses the same Provider wrapping pattern, ensuring reliable test behavior.
src/apps/pillarx-app/components/TokensHorizontalTile/test/TokensHorizontalTile.test.tsx (4)
4-6: Proper Redux Provider setup addedThe addition of Redux Provider imports is appropriate and follows best practices for testing components that rely on Redux state.
53-57: Correctly wrapped component with Redux ProviderThe component is now properly wrapped with the Redux Provider, ensuring it has access to the Redux store during testing, which is necessary for blockchain logo overlay functionality mentioned in the PR.
67-71: Redux Provider added to loading state testThe loading state test now correctly includes the Redux Provider wrapper, maintaining consistency across test cases.
92-99: Consistent test structure maintainedThe empty data test case properly maintains the Provider wrapping pattern while preserving the component's props structure.
src/apps/the-exchange/components/TokenLogo/test/TokenLogo.test.tsx (2)
50-50: Updated class expectations to match responsive stylingThe test expectations have been updated to match the component's shift from fixed dimensions to responsive styling, which is more maintainable and aligned with modern CSS practices.
58-58: Consistent class expectations across test casesThis test case has been similarly updated to maintain consistency with the new responsive styling approach used in the component.
src/apps/token-atlas/components/TokenCard/TokenCard.tsx (3)
27-34: Simplified conditional rendering for blockchain logoThe conditional rendering for the blockchain logo has been appropriately simplified using a logical AND operator, which is cleaner and more readable than the previous approach.
36-48: Improved token logo rendering structureThe token logo rendering has been enhanced with a proper container and conditional rendering between the actual logo and fallback. This structure allows for better styling control and visual consistency.
50-55: Great addition of text overlay for missing logosThe addition of text overlay showing the first two characters of the token name when no logo is available is an excellent fallback mechanism that improves the user experience.
src/apps/token-atlas/components/TokensSlider/TokensSlider.tsx (1)
143-148: Improved logic for blockchain logo determinationThe change properly handles cases where tokens have multiple contracts by only showing a blockchain logo when exactly one contract exists. This ensures we don't arbitrarily display a logo for multi-chain tokens.
src/apps/the-exchange/components/TokenLogo/TokenLogo.tsx (1)
23-46: Enhanced token logo rendering with fallback textThe restructured component provides better visual consistency by maintaining a fixed-size container and adding a text overlay that displays the first two characters of the token name when no logo is available. The use of responsive sizing classes (
w-full h-full) is also a good practice.src/apps/pillarx-app/components/TokenInfoHorizontal/TokenInfoHorizontal.tsx (2)
1-45: Added blockchain logo support with proper data fetchingThe addition of the
tokenChainsprop and blockchain data fetching is well-implemented. The effect correctly sets the blockchain logo when there's exactly one token chain and the data is successfully loaded.
51-82: Enhanced token display with blockchain overlayThe restructured token logo rendering now includes both a text fallback for missing logos and a blockchain logo overlay when the token exists on exactly one blockchain. The implementation is clean and visually consistent with other token components.
src/apps/token-atlas/components/TokenResultCard/TokenResultCard.tsx (1)
33-52: Improved token logo display with fallback text!The updated implementation enhances the token logo display by:
- Using a responsive container with proper positioning
- Providing an organized structure for both the logo and random avatar
- Adding an overlay text showing the first two characters of the token name when no logo is available
This change improves user experience by ensuring there's always a visual identifier for tokens.
src/apps/pillarx-app/components/HorizontalToken/HorizontalToken.tsx (3)
1-8: LGTM: New imports and props for blockchain integration.The additions of React hooks, TokenContract type, and the blockchain query hook provide the necessary foundation for displaying blockchain information alongside tokens.
Also applies to: 21-21, 33-33
38-50: Well-implemented blockchain logo fetching logic.The state management implementation correctly:
- Initializes the blockchain logo state
- Fetches blockchain data
- Updates the logo only when there's exactly one blockchain for the token
- Includes all necessary dependencies in the useEffect
This ensures efficient rendering and proper blockchain logo display.
61-92: Excellent responsive token and blockchain logo implementation.The token logo display has been enhanced with:
- A responsive container using relative positioning
- Proper fallback to RandomAvatar when no logo is available
- Text overlay showing the first two characters of the token name as a visual identifier
- Blockchain logo overlay with appropriate positioning when a single blockchain is associated
This implementation improves the visual representation of tokens and provides users with more context.
src/apps/pillarx-app/components/TokenInfoHorizontal/test/TokenInfoHorizontal.test.tsx (1)
1-1: LGTM: Updated imports for Redux integration in tests.Proper imports for testing library and Redux provider components.
Also applies to: 4-7
src/apps/pillarx-app/components/HorizontalToken/test/HorizontalToken.test.tsx (1)
1-1: LGTM: Updated imports for Redux integration in tests.Proper imports for testing library and Redux provider components.
Also applies to: 4-7
github-actions
bot
temporarily deployed
to
Preview (feat/PRO-3165/placeholders-tokens-no-logo)
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit