feat(leaderboard): paginate entries via infinite scroll - #1796
Open
bilhokista wants to merge 4 commits into
Open
feat(leaderboard): paginate entries via infinite scroll#1796bilhokista wants to merge 4 commits into
bilhokista wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1547.
A note on which page this is
The issue lists
frontend/src/app/leaderboard/page.tsx. That file renders a hardcodedINITIAL_ENTRIESarray and never calls the API, so there is nothing there to paginate.The page that actually fetches the leaderboard is
frontend/src/app/(authenticated)/leaderboards/page.tsx, viauseLeaderboard. That is also the only one with the current-user "your rank" card the issue asks to preserve, so I took it as the intended target. Happy to move the work if the marketing page was meant instead.What was missing
The API already supports pagination —
getLeaderboardacceptspageand returns{ data, total, page, limit }. The hook simply never used it:One request, first 100 rows, no way to reach row 101.
The change
frontend/src/lib/leaderboard.ts(new) holds the merge rules as pure functions, so the append behaviour is testable without mounting a hook or mocking the network:mergeLeaderboardPages(existing, incoming)— deduplicates byuser_id. This is the crux of the issue's test requirement, and it is not a theoretical concern: the ranking is live, so a user who climbs between the page-1 and page-2 requests is legitimately returned in both. Appending blindly would show them twice and hand React two children with the same key.hasMorePages/remainingCount— guard against atotalof 0 and alimitof 0, either of which would otherwise describe an endless list. An infinite scroll that never stops asking is worse than one that stops early.useLeaderboardgainsloadMore,hasMore,isLoadingMore, andremaining.Three details worth pointing out:
loadMorereadspageInfoandseasonIdfrom refs so the callback stays referentially stable —useInfiniteScrollrebuilds itsIntersectionObserverwheneveronLoadMorechanges identity, and an unstable callback would tear down and re-create the observer on every render.A
loadMorefailure reports the error but deliberately leaves the loaded entries on screen — the first page is still valid, and clearing it would punish the user for a failed scroll.Page. The observer sentinel renders only while
hasMore, so a finished list stops asking. Infinite scroll is disabled during the initial load, because the sentinel sits in an empty list and is therefore on screen — without that guard it would request page 2 before page 1 arrived. Tail skeletons are sized toMath.min(remaining, 5)so the list does not promise more rows than the server has, and ansr-onlystatus region announces progress.The existing "your rank" card is derived from
entriesand so survives page loads unchanged.Tests
New
leaderboard.test.ts, 17 cases: appending, the overlap case, a whole page repeating, newer-copy-wins, position stability, both empty inputs, non-mutation, three-page accumulation, and thehasMorePages/remainingCountboundaries including the zero-limit and empty-leaderboard traps.The existing
useLeaderboard.test.tsasserts onexpect.objectContaining({ season_id: undefined }), so the addedpageparameter does not break it.Honest note on verification: I could not run the frontend Vitest suite locally (no full workspace install). All four files were parsed with the TypeScript compiler API, and
leaderboard.tswas transpiled and executed standalone against 23 assertions — all passed, including a simulated three-page scroll session against a reshuffling ranking that returns overlapping rows, which ends with no duplicates and unique keys. The hook and page changes need CI.One thing I did not change:
LEADERBOARD_PAGE_SIZEis 50, down from the previous hardcoded 100. That makes the first paint lighter now that more pages are reachable, but it is a behaviour change on the first request and easy to put back if you would rather keep 100.🤖 Generated with Claude Code
https://claude.ai/code/session_01CrfEY1tvXrbeMDAUzxfuk7