Added new drop time and further QA minor fixes#339
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GasNewDropCard
participant FormattedNewDrop
User->>GasNewDropCard: Render with newDropTime
GasNewDropCard->>FormattedNewDrop: Pass timestamp (newDropTime + 24h)
FormattedNewDrop->>FormattedNewDrop: Calculate time difference
FormattedNewDrop-->>GasNewDropCard: Display formatted time (Xd Xh Xm)
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! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
✨ 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: 4
🧹 Nitpick comments (7)
src/types/api.ts (1)
865-865: Specify time unit and intent fornewDropTime.Readers can’t infer whether the value is seconds or milliseconds, nor whether it refers to “last update” or “next drop”. A terse JSDoc comment eliminates that ambiguity and prevents subtle bugs elsewhere.
newDropTime?: number; + /** + * Unix-timestamp in **milliseconds** marking when the user’s points were last + * updated (UI adds +24 h to display the next drop). + */src/apps/leaderboard/utils/index.tsx (2)
162-164: Sorting ok, but keep original arrays pristine.
Array.sortmutates the array in-place.
Here you sort a freshly-spread array, so it’s safe, yet an explicit clone (e.g..toSorted(...)in modern TS orslice().sort(...)) removes any doubt.- return [...processedTradingData, ...unprocessedMigrationData].sort( - (a, b) => b.totalPoints - a.totalPoints - ); + return [...processedTradingData, ...unprocessedMigrationData] + .slice() + .sort((a, b) => b.totalPoints - a.totalPoints);
230-230: Apply the same immutability pattern here for consistency.src/apps/leaderboard/components/PointsCards/GasNewDropCard.tsx (1)
46-49: Time arithmetic belongs outside the render tree.Adding
24 hinline mixes presentation with business logic and forces every render to redo the math.
Compute the “next drop timestamp” once (e.g. in the hook) and pass it directly:-<FormattedNewDrop timestamp={newDropTime + 24 * 60 * 60 * 1000} /> +<FormattedNewDrop timestamp={nextDropTimestamp} />This also avoids double-adding 24 h if upstream code starts doing it later.
src/apps/leaderboard/index.tsx (1)
196-197: Currency symbol inconsistency
totalVolumeis prefixed with$, buttotalGasis shown bare.
IftotalGasis also a monetary figure (USD), add the symbol; if it is a unit of gas, clarify via a suffix (e.g.gwei) to avoid user confusion.src/apps/leaderboard/components/PointsCards/FormattedNewDrop.tsx (2)
28-40: Redundant state initialisation & tight interval
setTimeLeft(getNextDrop())is called:
- In the
useStateinitializer.- Immediately again inside
useEffect.The second call is redundant.
Additionally, running the interval every 60 000 ms means the displayed minute ticks drift relative to the real minute boundary.
Consider computing the remaining milliseconds to the next full minute and scheduling the first timeout accordingly, then switching to a 60 000 ms interval.
44-60: Accessibility: missing aria-labelThe countdown is purely numeric. Add an
aria-labelthat conveys the meaning for screen-reader users, e.g.:<p aria-label={`${days} days ${hours} hours ${minutes} minutes until next drop`} className="text-sm text-white font-semibold" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/apps/leaderboard/components/PointsCards/tests/__snapshots__/GasNewDropCard.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
src/apps/leaderboard/components/PointsCards/FormattedNewDrop.tsx(1 hunks)src/apps/leaderboard/components/PointsCards/GasNewDropCard.tsx(2 hunks)src/apps/leaderboard/components/PointsCards/tests/GasNewDropCard.test.tsx(1 hunks)src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx(1 hunks)src/apps/leaderboard/hooks/useLeaderboardData.tsx(1 hunks)src/apps/leaderboard/index.tsx(2 hunks)src/apps/leaderboard/utils/index.tsx(2 hunks)src/apps/pillarx-app/components/TopTokens/TopTokens.tsx(1 hunks)src/types/api.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/apps/leaderboard/index.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 (3)
src/apps/leaderboard/hooks/useLeaderboardData.tsx (1)
73-74: Let’s retry fetching the field without process-substitution:#!/bin/bash # Inspect a sample API payload for `pointsUpdatedAt` (avoid /dev/fd) curl -s https://api.example.com/leaderboard | jq '.results[0].pointsUpdatedAt'src/apps/leaderboard/components/PxPointsSummary/PxPointsSummary.tsx (1)
134-135: Good call –newDropTimenow flows from real data.The prop change removes the magic
0and aligns with the new API field.src/apps/leaderboard/components/PointsCards/GasNewDropCard.tsx (1)
7-8: 👍 Nice extraction of formatting logic intoFormattedNewDrop.
src/apps/leaderboard/components/PointsCards/tests/GasNewDropCard.test.tsx
Show resolved
Hide resolved
Deploying x with
|
| Latest commit: |
cd9f733
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://14a89d6d.x-e62.pages.dev |
| Branch Preview URL: | https://feat-px-points-migration-boa.x-e62.pages.dev |
github-actions
bot
temporarily deployed
to
Preview (feat/px-points-migration-board)
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit