Conversation
WalkthroughThis update introduces a comprehensive migration and trading leaderboard system. It adds new API slices, Redux state management, React components, hooks, and utility functions to manage and display leaderboard data, including migration-specific logic. The UI is refactored for improved responsiveness, and new types are defined for detailed tracking of points, migration progress, and user transactions. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant ReduxStore
participant API
participant Utils
UI->>ReduxStore: Dispatch setActiveTab/setTimeTab
UI->>ReduxStore: useAppSelector to read state
UI->>API: useLeaderboardData hook triggers API queries
API->>API: Fetch trading/migration data (weekly, all-time)
API->>Utils: Process and merge leaderboard data
Utils-->>API: Return merged/processed data
API-->>UI: Provide data, loading, error states
UI->>UI: Render InfoBanner, PxPointsSummary, Leaderboards, etc.
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: |
97ee70b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6b5a3d0c.x-e62.pages.dev |
| Branch Preview URL: | https://feat-px-points-migration-boa.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: 14
🔭 Outside diff range comments (2)
src/apps/leaderboard/api/leaderboard.ts (1)
41-43: Unsafe string concatenation when building queryManual templating is error-prone and skips URL encoding.
Refactor tonew URLSearchParams()(orqslibrary) to:• auto-encode values (
address)
• drop trailing&noise
• simplify maintenanceThis will also allow re-use between volume & migration endpoints.
src/apps/leaderboard/components/UserInfo/UserInfo.tsx (1)
74-88: UndefinedrankChangeshows a red down-triangle by defaultBecause the fallback path covers both “increased” and
undefined, wallets with no delta get a misleading red indicator. Guard explicitly:const rankChangeTriangle = rankChange === undefined ? null : rankChange === LeaderboardRankChange.DECREASED ? (/* green up */) : (/* red down */);Ensures accurate visual cues.
🧹 Nitpick comments (23)
src/apps/leaderboard/hooks/useRankComparison.tsx (1)
6-12: Confirm semantic correctness of rank direction“Higher index ⇒
INCREASED” assumes that e.g. moving from rank #1 → #3 is an “increase”. If the product treats lower numbers as better ranks (common for leaderboards), this should instead be flagged as a decrease. Please double-check the intended wording before this goes live.src/types/api.ts (1)
758-865: Inconsistent casing & singular/plural namingNew migration-related types mix camelCase (
totalAmountUsd) with snake_case (amount_usd) and singular history item (transactionHistory) vs plural (transactionHistory[]elsewhere). Consistency eases API consumption—align names or document the deviation.src/apps/pillarx-app/components/WalletAdddressOverview/WalletAddressOverview.tsx (1)
8-50: Minor UI polish
style={{ width: 'full', height: 'full' }}is invalid CSS; use'100%'or rely on Tailwind classes.- Alt text typo:
"copy-adress-icon"→"copy-address-icon".
These tweaks improve accessibility & rendering fidelity.src/apps/pillarx-app/components/ReceiveModal/ReceiveModal.tsx (1)
115-120: Use valid width/height values
className="w-3 h-3.5"already sizes the icon; the inline style isn’t needed, but if kept, avoid non-numeric"12px"literals inside an object intended for numbers.src/apps/leaderboard/tailwind.leaderboard.config.js (1)
18-27: Hard-coded palette strings – extract to theme tokensMultiple hex strings are sprinkled here and inside components (e.g.,
InfoBanner,GasNewDropCard).
Move these to a singlecolorsmap (already started here) and reference via class utilities (bg-dark_blueetc.).
Keeps palette changes single-source and prevents drift across the codebase.src/apps/leaderboard/hooks/useDateRanges.tsx (1)
8-12: Time-zone implicitness
DateTime.now()uses the client locale. If your backend expects UTC timestamps, convert explicitly:-const now = DateTime.now(); +const now = DateTime.now().toUTC();This avoids users in different zones sending mismatched epoch values.
src/apps/leaderboard/components/InfoBanner/InfoBanner.tsx (1)
11-14: Dismissal state is ephemeral
visibleresets totrueon every mount, so the banner re-appears after page reloads or route changes.
If this banner should stay dismissed for the session/user, persist the flag (e.g.,localStorage, Redux, or URL param).src/apps/leaderboard/components/PointsCards/GasNewDropCard.tsx (1)
37-47:newDropTimeneeds human-readable formattingRaw epoch/seconds won’t be meaningful to users.
Format withluxonordate-fns, e.g.:<DateTime fromSeconds={newDropTime} .toRelative() // “in 3 h” />Improves UX and keeps consistency with other date displays.
src/apps/leaderboard/components/PointsCards/PointsCard.tsx (2)
25-32: Guard-render the tooltip to avoid empty (and still focusable) overlay
textTooltipis optional, but the tooltip DOM is always rendered.
WhentextTooltipisundefined, an empty absolutely-positioned element is injected, which can create unexpected layout / accessibility side-effects.- <div className="absolute ..."> - {textTooltip} - </div> + {textTooltip && ( + <div className="absolute ..."> + {textTooltip} + </div> + )}
25-32: Add basic a11y attributesWhile you are touching the tooltip, add
role="tooltip"andaria-label={textTooltip}for screen-reader support.
A11y is cheap here and pays off immediately.src/apps/leaderboard/components/LeaderboardTabsButton/LeaderboardFiltersButton.tsx (1)
12-18: Expose active state viaaria-pressedfor better accessibilityTab / filter buttons are essentially toggle buttons. Conveying the state only through colour leaves screen-reader users in the dark.
- <button + <button type="button" + aria-pressed={isActive}src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx (1)
59-60: Alt texts don’t match the images
alt="pillarx-icon"forWeeklyRankIcon(and likewise forEarnedLastWeekIcon) is misleading and harms accessibility.
Provide descriptive, image-specific alt text.Also applies to: 73-74
src/apps/leaderboard/components/Leaderboards/Leaderboards.tsx (1)
30-36: Minor: avoid index as key in static skeleton listIndexes are okay for static, non-reordering lists, but a constant string is even simpler and avoids ESLint noise:
- {[...Array(3)].map((_, index) => ( - <SkeletonLoader key={index} ... /> + {Array.from({ length: 3 }).map((_, i) => ( + <SkeletonLoader key={`skeleton-${i}`} ... /> ))}src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx (1)
35-48: Consider memoisingfindMatchingEntryresults
findMatchingEntryscans the full data array on every re-render.
Wrap each call inuseMemo(keyed by[walletAddress, data]) to avoid unnecessary linear scans on unrelated state updates. Not critical but improves perf on large leaderboards.src/apps/leaderboard/hooks/useLeaderboardData.tsx (1)
92-133: Effect silently retains stale weekly data when tab switches
weeklyTradingDatais only recomputed whiletimeTab === 'weekly'.
Switching back to “All Time” keeps the last weekly snapshot in state, which can leak into consumers that don’t additionally gate ontimeTab.Consider clearing this slice when leaving the weekly tab or compute it unconditionally in
useMemoto guarantee consistency.src/apps/leaderboard/api/leaderboard.ts (1)
47-56: Duplicate base-query boilerplate – consider a helper
baseVolumeQueryandbaseMigrationQueryare identical apart frombaseUrl.
Extract a factory (createRetriedBaseQuery(baseUrl)) to DRY the slice setup and keep retry policy in one place.src/apps/leaderboard/components/LeaderboardTabsButton/LeaderboardTabsButton.tsx (1)
19-37: Add accessibility attributes to tab buttonsThe buttons visually behave as tab toggles but lack ARIA hints.
Addrole="tab"andaria-selected={timeTab==='weekly'}(likewise for “All Time”) to improve keyboard navigation and screen-reader context.src/apps/leaderboard/components/UserInfo/UserInfo.tsx (2)
35-46: Repeated string slicing in resize handler
walletAddress.slice(...)runs on every resize event; minor, but can be avoided by computing once and memoising with breakpoint checks, or by using CSStext-overflow: ellipsisfor a simpler solution.
125-140: Missingaria-labelon copy buttonAdd
aria-label="Copy wallet address"to the clickable div to meet accessibility guidelines.src/apps/leaderboard/index.tsx (2)
63-74: Collapse the three nearly-identical tab handlers
handleAllTabClick,handleMigrationTabClick, andhandleTradingTabClickdiffer only by the dispatched payload. A single factory/helper keeps the component slimmer and avoids threeuseCallbacks capturing the same deps.-const handleAllTabClick = useCallback(() => dispatch(setActiveTab('all')), [dispatch]); -const handleMigrationTabClick = useCallback(() => dispatch(setActiveTab('migration')), [dispatch]); -const handleTradingTabClick = useCallback(() => dispatch(setActiveTab('trading')), [dispatch]); +const onTabClick = useCallback( + (tab: LeaderboardTabsType) => dispatch(setActiveTab(tab)), + [dispatch], +);…then pass
onTabClick('all' | 'migration' | 'trading')to the buttons.
91-124: MovecalculateTotalsoutsideuseMemoDefining
calculateTotalsinsideuseMemorecreates the same function every render, defeating memo-isation’s point. Pull it out (or wrap withuseCallback) so the memo truly caches on the dependency array.src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx (1)
30-51: Observer recreated each scroll batch
useEffectdepends onvisibleCount; every 10 rows it tears down & rebuilds theIntersectionObserver. Observing once on mount is enough:-}, [visibleCount]); +// eslint-disable-next-line react-hooks/exhaustive-deps +}, []);Minimises GC churn while scrolling.
src/apps/leaderboard/utils/index.tsx (1)
188-209: Union, not intersection, of addresses when collapsing overlapsWhen multiple migration entries share addresses you return only
commonAddresses, losing unique aliases and making reverse look-ups miss. Consider:-addresses: commonAddresses, +addresses: Array.from( + new Set(allEntries.flatMap((e) => e.addresses)), +),so every PX-linked address survives the merge.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
src/apps/leaderboard/images/current-rank-icon.svgis excluded by!**/*.svgsrc/apps/leaderboard/images/earned-last-week-icon.svgis excluded by!**/*.svgsrc/apps/leaderboard/images/gas-icon.svgis excluded by!**/*.svgsrc/apps/leaderboard/images/info-icon.svgis excluded by!**/*.svgsrc/apps/leaderboard/images/lottery-qualified-icon.pngis excluded by!**/*.pngsrc/apps/leaderboard/images/migration-icon.svgis excluded by!**/*.svgsrc/apps/leaderboard/images/new-drop-icon.svgis excluded by!**/*.svgsrc/apps/leaderboard/images/pillar-wallet-icon.svgis excluded by!**/*.svgsrc/apps/leaderboard/images/pillarx-icon.svgis excluded by!**/*.svgsrc/apps/leaderboard/images/trading-icon.svgis excluded by!**/*.svgsrc/apps/leaderboard/images/weekly-rank-icon.svgis excluded by!**/*.svg
📒 Files selected for processing (23)
src/apps/leaderboard/api/leaderboard.ts(3 hunks)src/apps/leaderboard/components/InfoBanner/InfoBanner.tsx(1 hunks)src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx(3 hunks)src/apps/leaderboard/components/LeaderboardTabsButton/LeaderboardFiltersButton.tsx(1 hunks)src/apps/leaderboard/components/LeaderboardTabsButton/LeaderboardTabsButton.tsx(1 hunks)src/apps/leaderboard/components/Leaderboards/Leaderboards.tsx(1 hunks)src/apps/leaderboard/components/PointsCards/GasNewDropCard.tsx(1 hunks)src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx(1 hunks)src/apps/leaderboard/components/PointsCards/PointsCard.tsx(1 hunks)src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx(1 hunks)src/apps/leaderboard/components/UserInfo/UserInfo.tsx(3 hunks)src/apps/leaderboard/hooks/useDateRanges.tsx(1 hunks)src/apps/leaderboard/hooks/useLeaderboardData.tsx(1 hunks)src/apps/leaderboard/hooks/useRankComparison.tsx(1 hunks)src/apps/leaderboard/hooks/useReducerHooks.tsx(1 hunks)src/apps/leaderboard/index.tsx(1 hunks)src/apps/leaderboard/reducer/LeaderboardSlice.ts(1 hunks)src/apps/leaderboard/tailwind.leaderboard.config.js(1 hunks)src/apps/leaderboard/utils/index.tsx(1 hunks)src/apps/pillarx-app/components/ReceiveModal/ReceiveModal.tsx(1 hunks)src/apps/pillarx-app/components/WalletAdddressOverview/WalletAddressOverview.tsx(1 hunks)src/store.ts(2 hunks)src/types/api.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/store.ts (1)
src/apps/leaderboard/reducer/LeaderboardSlice.ts (1)
leaderboardSlice(35-36)
src/apps/leaderboard/hooks/useReducerHooks.tsx (1)
src/store.ts (2)
AppDispatch(100-100)RootState(98-98)
src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx (1)
src/types/api.ts (1)
LeaderboardTableData(857-865)
src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx (1)
src/types/api.ts (1)
LeaderboardTableData(857-865)
src/apps/leaderboard/api/leaderboard.ts (3)
src/utils/blockchain.ts (1)
isTestnet(29-35)src/types/api.ts (1)
MigrationApiResponse(853-855)src/store.ts (1)
addMiddleware(50-60)
src/apps/leaderboard/components/LeaderboardTabsButton/LeaderboardTabsButton.tsx (2)
src/apps/leaderboard/hooks/useReducerHooks.tsx (2)
useAppDispatch(5-5)useAppSelector(6-6)src/apps/leaderboard/reducer/LeaderboardSlice.ts (2)
LeaderboardTimeTabsType(5-5)setTimeTab(26-28)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (2)
src/store.ts (1)
14-15: Verify slice shape matchesaddReducercontract
addReducerexpects an object withreducerPath&reducer. A plain RTK slice produced bycreateSlicedoesn’t havereducerPath, soleaderboardSlicemust follow the same custom convention as the other slices.
If it’s a standard slice, pass:addReducer({ reducerPath: 'leaderboard', reducer: leaderboardSlice.reducer });to avoid runtime key
undefinedinmiddlewareReducers.Also applies to: 91-92
src/apps/leaderboard/tailwind.leaderboard.config.js (1)
11-16: Breakpoint overlap – confirm intended cascade
desktopstarts at1024pxwhiletablethasmax: 1024px.
Because Tailwind’smaxincludes the boundary value, both breakpoints fire at exactly1024px.
If the overlap is unintended, bump one value (1023px/1025px) to avoid ambiguous styling.
src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx
Outdated
Show resolved
Hide resolved
github-actions
bot
requested a deployment
to
Preview (feat/px-points-migration-board)
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/apps/leaderboard/components/LeaderboardTab/test/LeaderboardTab.test.tsx (1)
74-85: ** Assertion too strict on full wallet address**
UserInfotruncates addresses for desktop/tablet, so the raw0xAddress1string might never appear in the DOM.
This test will become flaky across breakpoints.Suggest matching the truncated form (same helper as component) or asserting on the “My rank” label alone.
-expect( - screen.getAllByText(new RegExp(mockWalletAddress, 'i')).length -).toBeGreaterThanOrEqual(2); +expect(screen.getByText(/my rank/i)).toBeInTheDocument();
🧹 Nitpick comments (15)
src/apps/leaderboard/components/PointsCards/tests/OverviewPointsCard.test.tsx (3)
27-45: Reduce reliance on broad snapshot testsBoth test cases use full-DOM snapshots (
expect(container).toMatchSnapshot()). These quickly become noisy and brittle when purely presentational details or third-party library upgrades change the markup.
Consider replacing them with narrowly scoped expectations that assert only the bits of DOM that actually matter for behaviour (e.g. specific text content, aria roles, class names, etc.). This will cut down on snapshot churn and make failures more meaningful.
47-65: String-literal assertions are fragile against number formatting
getByText('1234'),getByText('1'),getByText('3'), andgetByText('345')assume the component renders raw numbers. If the UI starts usingtoLocaleString, rounding, or abbreviation (e.g. “1,234” or “1.2k”), this test will break even though the UI is still correct.Prefer a more resilient approach:
- expect(screen.getByText('1234')).toBeInTheDocument(); + expect(screen.getByText(/1[,.]?234/)).toBeInTheDocument(); // or use regex / formatterEven better, query by test-id or role where possible to decouple from formatting details.
67-76: Dash-count assertion is brittle
expect(screen.getAllByText('-').length).toBe(2);presumes the component will always render exactly two “-” symbols when data is missing. A future design tweak (e.g. additional placeholders or changing the placeholder character) will cause an unnecessary failure.- expect(screen.getAllByText('-').length).toBe(2); + expect(screen.getAllByText('-', { exact: true })).toHaveLength(2); // at least keeps the exact match + // or, if the number of placeholders is not important: + expect(screen.getByText('-', { exact: true })).toBeInTheDocument();Alternatively, assert specific sections (points placeholder, rank placeholder) individually by test-id to make intent clearer.
src/apps/leaderboard/components/PointsCards/tests/GasNewDropCard.test.tsx (3)
7-17: Snapshot-only assertions are brittle – favour explicit expectations.The first two tests rely exclusively on Jest snapshots.
UI-focused snapshots tend to generate noisy diffs for minor cosmetic changes and often mask the real intent of the tests (business logic). You are already asserting the important conditional rendering a few lines below; consider dropping the snapshot cases or, at minimum, asserting key DOM nodes/values instead.
19-25: Hard-coded currency / number strings may break on formatting changes.
expect(screen.getByText('$1234'))will fail if the component later formats the value as$1,234,1 234 $, etc.
Prefer a regexp that ignores thousand separators (or query by test-id) to make the test resilient:- expect(screen.getByText('$1234')).toBeInTheDocument(); + expect(screen.getByText(/\$?\s?1[, ]?234/)).toBeInTheDocument();
27-39: Minor duplication – group similar assertions throughdescribe.each.The last two cases only vary by
isMigratedflag and expected presence of “Gas Used”. Parameterising them reduces duplication and makes intent clear:describe.each` isMigrated | shouldShowGas ${false} | ${false} ${undefined} | ${false} `('renders correctly when isMigrated=$isMigrated', ({ isMigrated, shouldShowGas }) => { // ... });src/apps/leaderboard/components/PointsCards/tests/PointsCard.test.tsx (3)
16-28: Snapshot may hide regression on dynamic props.Similar to the previous file: snapshotting an entire card that receives many props (points, rank, tooltip, colours) can quickly become noisy. Focus on explicit expectations (you already cover most of them) and drop the snapshot unless you have a very stable static markup.
42-47: Alt-text assertion coupled to implementation detail.
getByAltText('Trading-icon')will fail if someone changes the alt text to a more accessible variant ('Trading icon'). Prefer:- const icon = screen.getByAltText('Trading-icon'); + const icon = screen.getByRole('img', { name: /trading/i });This keeps the test aligned with accessibility guidelines rather than string exactness.
88-100: Zero-points case good – minor readability nit.Using a regex would avoid potential false negatives when the component localises the number:
- expect(screen.getByText('0')).toBeInTheDocument(); + expect(screen.getByText(/^0$/)).toBeInTheDocument();src/apps/leaderboard/components/PxPointsSummary/tests/PxPointsSummary.test.tsx (2)
150-164: Good conditional-render coverage – include negative assertion for lottery badge.In the “does not show migration card” case you already check positive renders; also assert that “Qualified” badge is absent to capture unintended regressions:
expect(screen.queryByText('Qualified')).not.toBeInTheDocument();
185-201: Great graceful-degradation test – consider adding zero-state snapshot.To document the zero-data UI, consider adding a small snapshot or specific assertions for points = 0 / ranks = '–' in this path.
src/apps/leaderboard/components/Leaderboards/tests/Leaderboards.test.tsx (2)
20-25: Skeleton count assertion is brittleHard-coding
3skeletons will break as soon as the component changes the placeholder count (e.g. responsive layouts).
Prefer asserting at least one skeleton or querying by role/test-id instead of count.-expect(SkeletonLoader).toHaveBeenCalledTimes(3); -expect(screen.getAllByText('SkeletonLoader Mock')).toHaveLength(3); +expect(SkeletonLoader).toHaveBeenCalled(); +expect(screen.getAllByText('SkeletonLoader Mock').length).toBeGreaterThan(0);
66-68:toHaveBeenCalledWithsecond argument is React-version dependentReact only passes the legacy
contextparam to function components in very old versions.
expect(LeaderboardTab).toHaveBeenCalledWith({ data: mockData })is future-proof and avoids false negatives once the team bumps React.src/apps/leaderboard/components/LeaderboardTab/test/LeaderboardTab.test.tsx (1)
51-61:Loading morecopy tightly coupledAny wording tweak (“Load more”, button replacement, i18n) will break these assertions.
Prefer selecting byrole,data-testid, or a regex that tolerates variations.src/apps/leaderboard/components/LeaderboardTabsButton/test/LeaderboardTabsButton.test.tsx (1)
28-36: Snapshot test relies on mocked selector but still wires real storeSince
useAppSelectoris mocked, wrapping with<Provider store={store}>is redundant noise and can be removed to speed up the test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
src/apps/leaderboard/components/LeaderboardTab/test/__snapshots__/LeaderboardTab.test.tsx.snapis excluded by!**/*.snapsrc/apps/leaderboard/components/LeaderboardTabsButton/test/__snapshots__/LeaderboardTabsButton.test.tsx.snapis excluded by!**/*.snapsrc/apps/leaderboard/components/PointsCards/tests/__snapshots__/GasNewDropCard.test.tsx.snapis excluded by!**/*.snapsrc/apps/leaderboard/components/PointsCards/tests/__snapshots__/OverviewPointsCard.test.tsx.snapis excluded by!**/*.snapsrc/apps/leaderboard/components/PointsCards/tests/__snapshots__/PointsCard.test.tsx.snapis excluded by!**/*.snapsrc/apps/leaderboard/components/PxPointsSummary/tests/__snapshots__/PxPointsSummary.test.tsx.snapis excluded by!**/*.snapsrc/apps/leaderboard/components/UserInfo/test/__snapshots__/UserInfo.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/PortfolioOverview/test/__snapshots__/PortfolioOverview.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!**/*.snapsrc/apps/pillarx-app/components/WalletAdddressOverview/test/__snapshots__/WalletAddressOverview.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
src/apps/leaderboard/components/LeaderboardTab/test/LeaderboardTab.test.tsx(4 hunks)src/apps/leaderboard/components/LeaderboardTabsButton/test/LeaderboardTabsButton.test.tsx(1 hunks)src/apps/leaderboard/components/Leaderboards/tests/Leaderboards.test.tsx(1 hunks)src/apps/leaderboard/components/PointsCards/tests/GasNewDropCard.test.tsx(1 hunks)src/apps/leaderboard/components/PointsCards/tests/OverviewPointsCard.test.tsx(1 hunks)src/apps/leaderboard/components/PointsCards/tests/PointsCard.test.tsx(1 hunks)src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx(1 hunks)src/apps/leaderboard/components/PxPointsSummary/tests/PxPointsSummary.test.tsx(1 hunks)src/apps/leaderboard/components/UserInfo/test/UserInfo.test.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/apps/leaderboard/components/PointsCards/tests/OverviewPointsCard.test.tsx (1)
src/types/api.ts (1)
LeaderboardTableData(857-865)
src/apps/leaderboard/components/PxPointsSummary/tests/PxPointsSummary.test.tsx (1)
src/types/api.ts (1)
LeaderboardTableData(857-865)
src/apps/leaderboard/components/LeaderboardTab/test/LeaderboardTab.test.tsx (1)
src/types/api.ts (1)
LeaderboardTableData(857-865)
src/apps/leaderboard/components/LeaderboardTabsButton/test/LeaderboardTabsButton.test.tsx (1)
src/apps/leaderboard/reducer/LeaderboardSlice.ts (1)
setTimeTab(26-28)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (4)
src/apps/leaderboard/components/PointsCards/tests/PointsCard.test.tsx (1)
68-85: Tooltip interaction not fully asserted.You trigger
mouseOverbut never verify the tooltip visibility change. Add an expectation such as:fireEvent.mouseOver(screen.getByText('Trading')); expect(screen.getByText('This is a tooltip')).toHaveClass('opacity-100');(or however the component reveals the tooltip). Without it, the interaction part of the test provides no coverage.
src/apps/leaderboard/components/PxPointsSummary/tests/PxPointsSummary.test.tsx (2)
1-3: Missing explicit React import inside TSX mocks can break type-checking.Because the file declares inline JSX inside
jest.mock, older TS configs (or strict builds) still requireimport React from 'react';. Add once at the top to satisfy both classic andreact-jsxruntimes:+import React from 'react';
166-183: Edge-case modification correctly covered – nice.Solid test ensuring badge suppression when
completedSwapis falsy.
No action required.src/apps/leaderboard/components/UserInfo/test/UserInfo.test.tsx (1)
79-88: Icon expectations reversed?The test labelled “rank decrease” expects
leaderboard-rank-up, while “rank increase” expectsleaderboard-rank-down.
Double-check the component: if ⬆️ actually represents a better (numerically lower) rank, rename the tests to avoid cognitive dissonance.
If not, swap the expected test-ids.
src/apps/leaderboard/components/PxPointsSummary/tests/PxPointsSummary.test.tsx
Show resolved
Hide resolved
src/apps/leaderboard/components/LeaderboardTabsButton/test/LeaderboardTabsButton.test.tsx
Show resolved
Hide resolved
github-actions
bot
requested a deployment
to
Preview (feat/px-points-migration-board)
IAmKio
left a comment
IAmKio
left a comment
There was a problem hiding this comment.
There's just one small question but other than that it LGTM and it's not a showstopper
github-actions
bot
temporarily deployed
to
Preview (feat/px-points-migration-board)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/apps/leaderboard/hooks/useLeaderboardData.tsx (1)
216-233: Variable naming & comment mismatch could confuse future readersconst lastAddress = entry.addresses[0]; // comment: “last address”
- The identifier says “lastAddress” but grabs first element.
- The preceding comment “based on last address” contradicts the implementation.
Either rename the variable to
firstAddressor useentry.addresses[entry.addresses.length - 1].-const lastAddress = entry.addresses[0]; +const addressKey = entry.addresses[0]; // single-address array in merged dataSmall but valuable for maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/apps/leaderboard/components/LeaderboardTab/test/__snapshots__/LeaderboardTab.test.tsx.snapis excluded by!**/*.snapsrc/apps/leaderboard/components/PointsCards/tests/__snapshots__/GasNewDropCard.test.tsx.snapis excluded by!**/*.snapsrc/apps/leaderboard/components/PointsCards/tests/__snapshots__/OverviewPointsCard.test.tsx.snapis excluded by!**/*.snapsrc/apps/leaderboard/components/UserInfo/test/__snapshots__/UserInfo.test.tsx.snapis excluded by!**/*.snapsrc/apps/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
src/apps/leaderboard/api/leaderboard.ts(4 hunks)src/apps/leaderboard/components/PointsCards/GasNewDropCard.tsx(1 hunks)src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx(1 hunks)src/apps/leaderboard/components/PointsCards/tests/GasNewDropCard.test.tsx(1 hunks)src/apps/leaderboard/components/UserInfo/UserInfo.tsx(3 hunks)src/apps/leaderboard/hooks/useLeaderboardData.tsx(1 hunks)src/apps/leaderboard/utils/index.tsx(1 hunks)src/types/api.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/apps/leaderboard/components/PointsCards/tests/GasNewDropCard.test.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/apps/leaderboard/components/PointsCards/GasNewDropCard.tsx
- src/apps/leaderboard/api/leaderboard.ts
- src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx
- src/apps/leaderboard/utils/index.tsx
- src/apps/leaderboard/components/UserInfo/UserInfo.tsx
- src/types/api.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/apps/leaderboard/hooks/useLeaderboardData.tsx (5)
src/apps/leaderboard/reducer/LeaderboardSlice.ts (1)
LeaderboardTimeTabsType(5-5)src/apps/leaderboard/hooks/useDateRanges.tsx (1)
useDateRanges(4-14)src/apps/leaderboard/hooks/useRankComparison.tsx (1)
useRankComparison(6-13)src/types/api.ts (1)
LeaderboardTableData(857-865)src/apps/leaderboard/utils/index.tsx (4)
getMergeLeaderboardMigrationDataByAddresses(158-217)getCurrentWeekMigrationData(27-64)getLastWeekMigrationData(70-107)getMergeLeaderboardData(112-151)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (2)
src/apps/leaderboard/hooks/useLeaderboardData.tsx (2)
115-118:totalPointsis populated with USD value – intentional?Both
totalPointsandtotalAmountUsdare set tocurrentUserData.totalSwapAmountUsd.
If points and USD are distinct concepts in the UI, this will double-count the same figure and break any aggregate statistics.Confirm the API payload – if you really want points here, replace with the correct field (e.g.
currentUserData.totalPoints).
65-90: All-time data processing looks solidClear transformation, memoised correctly, and leverages the merge helper. No concerns here.
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation
Chores