Fetch weekly data fixes and other UI minor fixes#341
Conversation
WalkthroughThis update introduces UI improvements and data handling adjustments to leaderboard points cards. It adds a tooltip to the GasNewDropCard, updates conditional rendering for points display, makes the points prop optional, and removes default zero fallbacks. The useLeaderboardData hook is refactored to simplify its API and enhance data freshness. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LeaderboardPage
participant useLeaderboardData
participant API
User->>LeaderboardPage: Visit leaderboard
LeaderboardPage->>useLeaderboardData: Call hook (no timeTab)
useLeaderboardData->>API: Fetch leaderboard data (with refetchOnFocus)
API-->>useLeaderboardData: Return data
useLeaderboardData-->>LeaderboardPage: Provide leaderboard data
LeaderboardPage->>User: Render PointsCards (with updated points logic and tooltips)
Note over LeaderboardPage: Tooltip appears on hover in GasNewDropCard
Possibly related PRs
Suggested reviewers
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 (
|
![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
🔭 Outside diff range comments (1)
src/apps/leaderboard/hooks/useLeaderboardData.tsx (1)
92-130: Optimize weekly data processing to avoid unnecessary computations.The removal of the
timeTab === 'weekly'condition means weekly trading data is always processed, even when the weekly tab is not active. This could impact performance unnecessarily.Consider adding the timeTab check back or implementing a more efficient data processing strategy:
useEffect(() => { + // Only process if weekly data is needed + if (timeTab !== 'weekly') return; + if ( weeklyQuery.data && lastWeeklyQuery.data && !weeklyQuery.isLoading && !lastWeeklyQuery.isLoading ) { // ... existing processing logic } }, [ weeklyQuery.data, lastWeeklyQuery.data, weeklyQuery.isLoading, lastWeeklyQuery.isLoading, compareIndexes, + timeTab, ]);
🧹 Nitpick comments (4)
src/apps/leaderboard/hooks/useLeaderboardData.tsx (1)
37-37: Consider the performance impact of refetchOnFocus.Adding
refetchOnFocus: trueto all queries will improve data freshness but may cause unnecessary API calls when users frequently switch between browser tabs/windows. Monitor API usage patterns to ensure this doesn't negatively impact performance.Also applies to: 46-46, 55-55, 60-60
src/apps/leaderboard/components/PointsCards/GasNewDropCard.tsx (1)
21-24: Enhance tooltip accessibility for better user experience.The tooltip implementation is functional but could be improved for accessibility. Consider adding keyboard support and ARIA attributes for screen readers.
-<div className="group flex flex-col w-full gap-3 rounded-[10px] p-2 relative"> - <div className="absolute bottom-full mb-2 left-0 rounded-lg border border-white/[.05] bg-[#25232D] px-2.5 py-2 text-[10px] text-white italic font-normal opacity-0 group-hover:opacity-100 transition-all duration-200 z-10 max-w-max w-fit origin-bottom scale-y-0 group-hover:scale-y-100 transform"> +<div className="group flex flex-col w-full gap-3 rounded-[10px] p-2 relative" tabIndex={0}> + <div + className="absolute bottom-full mb-2 left-0 rounded-lg border border-white/[.05] bg-[#25232D] px-2.5 py-2 text-[10px] text-white italic font-normal opacity-0 group-hover:opacity-100 group-focus:opacity-100 transition-all duration-200 z-10 max-w-max w-fit origin-bottom scale-y-0 group-hover:scale-y-100 group-focus:scale-y-100 transform" + role="tooltip" + aria-label="Gas usage explanation" + > This is the amount of gas used to migrate your assets over to PillarX. </div>src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx (2)
38-42: Consider removing the zero fallback for consistency.The conditional rendering logic is good, but the
|| 0fallback when accessingtotalPointsis inconsistent with the new approach of showing dashes for missing data. If a user exists in the leaderboard (index !== -1) buttotalPointsis undefined, it will display "0 PX" instead of "-".Consider this approach for better consistency:
- {myAllTimeMerged.index === -1 - ? '-' - : formatAmountDisplay( - Math.floor(myAllTimeMerged.entry?.totalPoints || 0) - )} + {myAllTimeMerged.index === -1 || !myAllTimeMerged.entry?.totalPoints + ? '-' + : formatAmountDisplay(Math.floor(myAllTimeMerged.entry.totalPoints))}
87-91: Apply the same consistency improvement for weekly points.Similar to the all-time points, consider removing the zero fallback here as well for consistent handling of missing data.
- {myWeeklyMerged.index === -1 - ? '-' - : formatAmountDisplay( - Math.floor(myWeeklyMerged.entry?.totalPoints || 0) - )} + {myWeeklyMerged.index === -1 || !myWeeklyMerged.entry?.totalPoints + ? '-' + : formatAmountDisplay(Math.floor(myWeeklyMerged.entry.totalPoints))}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
src/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/pillarx-app/components/MediaGridCollection/tests/__snapshots__/DisplayCollectionImage.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
src/apps/leaderboard/components/PointsCards/GasNewDropCard.tsx(1 hunks)src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx(2 hunks)src/apps/leaderboard/components/PointsCards/PointsCard.tsx(2 hunks)src/apps/leaderboard/components/PointsCards/tests/OverviewPointsCard.test.tsx(1 hunks)src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx(2 hunks)src/apps/leaderboard/hooks/useLeaderboardData.tsx(4 hunks)src/apps/leaderboard/index.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/apps/leaderboard/components/PointsCards/OverviewPointsCard.tsx (1)
src/utils/number.tsx (1)
formatAmountDisplay(3-33)
src/apps/leaderboard/components/PointsCards/PointsCard.tsx (1)
src/utils/number.tsx (1)
formatAmountDisplay(3-33)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (7)
src/apps/leaderboard/hooks/useLeaderboardData.tsx (1)
27-27: To accurately locate all hook usages across.tsand.tsxfiles, let’s broaden the search without relying on unsupported file-type filters:#!/bin/bash # Search for useLeaderboardData calls in all TS/TSX files rg -n 'useLeaderboardData\(' --glob '*.ts' --glob '*.tsx'src/apps/leaderboard/components/PointsCards/tests/OverviewPointsCard.test.tsx (1)
75-75: Test update aligns with component behavior changes.The increase from 2 to 4 expected fallback dashes correctly reflects the updated component logic that now displays more dashes for missing data.
src/apps/leaderboard/index.tsx (2)
64-64: Hook call correctly updated to match new signature.The removal of the
timeTabargument aligns perfectly with the refactoreduseLeaderboardDatahook signature.
196-196: Improved gas amount display with currency symbol.Adding the dollar sign prefix makes the gas cost display more intuitive and consistent with other monetary values in the UI.
src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx (1)
112-112: Verify PointsCard handles undefined points prop correctly.Removing the
|| 0fallbacks is good for explicit handling of missing data, but ensure thePointsCardcomponent properly handlesundefinedvalues for thepointsprop.#!/bin/bash # Description: Verify PointsCard component handles undefined points prop # Expected: Component should handle undefined/null points gracefully # Search for PointsCard component implementation ast-grep --pattern 'const PointsCard = ({ $$$, points, $$$ }: $$$) => { $$$ }' # Also check for points prop usage in the component rg -A 10 -B 5 'points.*\?' src/apps/leaderboard/components/PointsCards/PointsCard.tsxAlso applies to: 125-125
src/apps/leaderboard/components/PointsCards/PointsCard.tsx (2)
11-11: Good approach to making points optional.Making the
pointsprop optional properly supports the new data handling approach where missing points should be displayed as dashes rather than defaulting to zero.
42-42: Excellent null handling implementation.The
!= nullcheck properly handles bothnullandundefinedvalues, andMath.floor()is only called whenpointsis guaranteed to be a number. This is a clean and consistent approach to displaying missing data.
Deploying x with
|
| Latest commit: |
b5f2ad9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://947d2560.x-e62.pages.dev |
| Branch Preview URL: | https://feat-px-points-migration-boa.x-e62.pages.dev |
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Improvements
Bug Fixes