Repository navigation
feat(leaderboard): redesign /users as the storybook's full-width Option B - #7317
Conversation
…on B Implements the leaderboard redesign from the storybook (#7126) on production data only: the 7 LEADERBOARD_QUERY boards plus Highest level, asked with a limit of 12, each board's top 50 fetched when its tile opens, your rank from leaderboardPosition where it answers, and the Plus referral ladder. The page opens on you (your best rank with a board picker and Snapshot, and the Plus invite card with Copy invite link), then Highest reputation in columns, Standouts (the #1 of every board) and all 8 boards as tiles. A tile opens its top 50 in place as a popover, or as a bottom sheet on phones. Phones get the two cards as a sideways strip and lose the floating "Search people" field, as the design states. Leaderboard rows carry the UserShortInfo fragment, so UserLeaderboard's user is now a UserShortProfile; the types moved next to their queries and the old exports stay. leaderboardPosition and the per-board queries became options creators so the rank card, the tiles and CurrentUserPositionRow share one cache. Clicks log through three new target types (leaderboard board, rank picker, user) and the invite copy through a new target id, so the popover that replaces the /users/[board] page view still counts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
rebelchris
left a comment
There was a problem hiding this comment.
Overall the structure works well. Sharing the cache through leaderboardPositionQueryOptions and leaderboardBoardQueryOptions is a good cleanup, the board list only loads when it's opened, and the static props keep the HighestLevel schema fallback. I found one blocker and a few smaller things, all inline.
Things that apply to the whole PR:
- Fewer crawlable links on
/users. I compared the server-rendered HTML of the preview with production. Profile links drop from 53 unique to 16 (only the Highest reputation columns and Standouts are left). Links to/users/<board>drop from 8 to 1. This page used to be the internal-link hub for every board and its top users. Details are in the inline comments. - Verification. The description says this wasn't rendered in a browser, and it replaces the whole page with no flag. Before merging, please attach preview screenshots at 1440, 1024, 768 and 375. Cover: logged out; logged in with a rank, past the cap, and with no rank; Plus-ladder eligible and not eligible; a tile popover on the last grid row; and the phone sheet. If there's no flag, rolling back means a revert, so please list which states you checked on the final head.
- Size. This is about 2.1k added lines in a single commit. It's cohesive (almost all new files under
features/leaderboard), so I'm not asking for a split. It's just above the size where line-by-line review stays reliable.
Reviewed by AI.
| ); | ||
|
|
||
| /** A row on a board tile and in its full list: rank, who, and the score. */ | ||
| export const LeaderboardMiniRow = ({ |
There was a problem hiding this comment.
Blocking: you can't get to a user's profile from 6 of the 8 boards. LeaderboardMiniRow renders a plain <li>. It's used for the tile rows, the top-50 popover and the phone sheet. On main, every row on every board linked to the user's profile through UserHighlight → ProfileLink. With this change, only the Highest reputation columns and Standouts link anywhere. On the other boards, clicking a person opens the list again, and clicking a person in the list does nothing. Crawlers lose those links too: the server-rendered /users goes from 53 unique profile links to 16.
Suggested fix: take an optional href (the user's permalink) and render the row as an anchor, the way PersonRow in LeaderboardTopReputation already does. That needs the tile to stop being one big role="button" (see the comment on pressable). Opening the list from the header cue keeps rows clickable and avoids nesting interactive elements. If you want clicks logged, leaderboardUserClickEvent could take a placement: 'board'.
Reviewed by AI.
There was a problem hiding this comment.
Fixed in 111e2c0: LeaderboardMiniRow takes an href and renders the row as the profile link (same shape as PersonRow), on the tile, in the popover and in the sheet, including your pinned row. Clicks log leaderboard user with placement: board. Company rows stay plain since there is no page for them.
| ); | ||
|
|
||
| // An empty board is a plain card: nothing to open. | ||
| const pressable = empty |
There was a problem hiding this comment.
Should fix: making the whole tile a button removes the /users/<board> links and nests interactive elements.
- The tile titles used to link to
/users/<board>. Now only Highest reputation does, through "Full ranking". The board pages are still served, but the hub no longer links to the other seven, which hurts both crawlability and anyone who wants the full list. You could keep thelistLabelcue as a realLinkto/users/${type}and callpreventDefaultin itsonClickto open the popover or sheet. Crawlers and middle-click would still get the page. role="button"wrapsProfileTooltiptriggers, which are interactive, so assistive tech gets a button nested inside another interactive element. The tooltip is also portaled, so amousedowninside it while the popover is open counts as an outside press and closes the popover. If the list opened from the header cue only, both problems go away, and the rows could become profile links (see the comment onLeaderboardMiniRow).
Reviewed by AI.
There was a problem hiding this comment.
Fixed in 111e2c0: the tile is a plain card again. The header cue is a real Link to /users/<board>; a plain press calls preventDefault and opens the popover or sheet, while a modifier or middle press keeps the page. The popover closes from a scrim and Escape instead of a document mousedown, so a press inside a portaled hover card keeps it open, and nothing interactive is nested in a button anymore.
| const sheetTitle = `${title} · ${listLabel}`; | ||
| const rows = (entries: T[]) => | ||
| entries.map((item, index) => renderRow(item, index + 1)); | ||
| const allRows = isPending |
There was a problem hiding this comment.
Non-blocking: a failed fetch looks like an empty board. When the top-50 query fails after its retries, isPending turns false and all is undefined. The popover or sheet then shows its "Top 50" header over an empty list, with no error and no way to retry. Could it fall back to items (the page already has up to 12 rows) and show a short error with a retry when isError && !all?
Reviewed by AI.
There was a problem hiding this comment.
Fixed in 111e2c0: when the top-50 query fails the list falls back to the page's rows, with a "Couldn't load the full list." line and a Retry button above them. Covered by a new case in LeaderboardBoardTile.spec.tsx.
| (item) => item.type === board, | ||
| ); | ||
| const position = positions[boardIndex].data; | ||
| const state = getLeaderboardRankState(position); |
There was a problem hiding this comment.
Non-blocking: an error shows up as "No rank yet". If one of the leaderboardPosition queries fails, its data is undefined. getLeaderboardRankState then returns 'empty', so a user who does have a rank sees "No rank yet on …" and the how-to copy, and the Snapshot button disappears. getBestBoard also skips that board without saying so. Could you check isError here (and in useLeaderboardPosition) so it shows a neutral state instead of telling the user they have no rank?
A small nit nearby: board === 'highestReputation' / 'longestStreak' on lines 126 and 133 compare against string literals. Please use LeaderboardType.HighestReputation / LeaderboardType.LongestStreak like the rest of the file.
Reviewed by AI.
There was a problem hiding this comment.
Fixed in 111e2c0: a failed leaderboardPosition answer now reads "Your rank on isn't available right now" with "Try again in a moment.", hides Snapshot, and useLeaderboardPosition returns isError. getBestBoard still skips a failed board, so a board that did answer is chosen first. The string comparisons use the enum now. Covered in LeaderboardRankCard.spec.tsx.
Review follow-ups on the /users redesign. A board tile was one big button, so the rows no longer linked anywhere and the hub lost its links to the board pages: the server-rendered page went from 53 unique profile links to 16 and from 8 board links to 1. Now the header cue is a real link to /users/<board> that opens the top 50 in place on a plain press (a modifier or middle press keeps the page), and every row is the person's profile link, logged with placement "board". The popover closes from a scrim rather than a document mousedown, so a press in a portaled hover card keeps it up. A failed top-50 fetch falls back to the page's rows with a short notice and a retry, and a failed leaderboardPosition answer reads as unavailable rather than as "No rank yet". Board comparisons use the enum. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…type requires Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Review follow-upFixes are in 111e2c0 (links, scrim, error states) and 7cf1198 (a fixture type). Replies on each thread say what changed. Links on the server-rendered page: every row on every board is the person's profile link again, and each tile's cue is a real link to Preview screenshotsTaken with Playwright against the preview of 111e2c0 (the UI head; 7cf1198 only touches a spec), signed out, dark theme. The signed-out signup banner was hidden for the shots since it is fixed over the last grid row. Each "list" shot opens the last tile of the grid, Most verified employees.
States checked on the final headChecked on the preview:
Covered by specs rather than the preview (they need a session or a failing API):
Not checked by me, since I have no session on the preview:
Those four need a signed-in pass on the preview before merge. |
The storybook closes the Standouts row with Highest level, as its #1 reads as a level rather than a score, while the page's board order keeps it first. The rank card's picker on phones says the board's short name on the button and keeps the full titles in the sheet, as the storybook's "Picker open, short names" frame shows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Design pass against Leaderboard / FinalCompared the branch with the storybook (built from
Everything else matched: the rank card's ranked, outside-the-top, no-rank and picker states, the invite card and ladder-complete copy, the top-50 popover and phone sheet with your row pinned, the hover card, your company lit and pinned on Most verified employees, the loading placeholders and the empty page. The only thing that is still not possible is the verified badge next to a company (backend field, see above). |
…them
Rendering the page signed in beside the storybook's frames showed four
details the design writes differently. Scores from 10K up keep one
decimal ("29.6K", "2.1M") where largeNumberFormat truncated them to
"29K". Your pinned row says "842", the rank card alone says "#842". The
cap reads "1K" or "100K", not "1,000". And a board with no score of
yours is never your best board, so an empty position card opens on the
first board instead of a leftover rank.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-in pass against the designTo compare the signed-in states for real, I mounted the production
Everything else lines up with the frames: rank card in every state, picker open, invite card and ladder complete, your pinned company, every popover and the phone sheet, the loading placeholders and the empty page. Production page, signed in, rendered in the storybook shell (light theme):
Two things stay deliberately different from the storybook: the picker menu and the phone sheet are production's |
The invite card built its "You unlocked" headline before checking isLoading, so an eligible user crashed /users with "Cannot read properties of undefined (reading 'months')" until the ladder arrived. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rebelchris
left a comment
There was a problem hiding this comment.
Final pass on 83ac347, covering the full diff rather than only the commits since the last round.
Earlier threads. I checked all four fixes in the code. Every row on every board is now a profile link. Each tile's cue is a real /users/<board> link, and nothing is nested inside a button. A failed top-50 fetch falls back to the page's rows and offers Retry. A failed leaderboardPosition answer reads as unavailable, not as "No rank yet". The enum comparisons are fixed. CI is green on this head. I found no added binaries, no force-on leftovers and no unknown Tailwind utilities.
One gate before merge: a signed-in pass on the real preview at this head. The 9 Nov checklist left four items unchecked: the rank card in its ranked, capped and no-rank states, the Snapshot share, the Plus ladder for eligible and ineligible users, and the hover-card Follow. The signed-in pass on 11 Nov used the storybook shell with mock data. It reported that the invite card matched, yet 83ac347 then fixed a crash that hit every eligible user while the ladder loaded. Only a render with real data found it. This replaces /users with no flag, so rollback means a revert. Please run those four states on the preview at 83ac347 with a real session, at 375 and at one desktop width, and list them here.
Three non-blocking notes are inline. The new TargetTypes and TargetId are taxonomy additions, so it may be worth getting an analytics owner to look at them.
Reviewed by AI.
| const { isEligible, isLoading } = useReferralLadder(); | ||
| const hasInvite = isEligible || isLoading; | ||
|
|
||
| if (!user) { |
There was a problem hiding this comment.
Non-blocking: signed-in visitors see the page shift. /users is ISR, so the server HTML and the first client render have no user. When auth resolves, this row mounts above Highest reputation (the rank card plus the invite card, a skeleton first) and pushes every section below it down. This happens to every logged-in reader on every visit. Could the row render its placeholder while !isAuthReady, then collapse for signed-out users? The cost is a brief skeleton for signed-out visitors, so it's a call to make between the two. Either way, a CLS number signed in vs signed out would settle it.
Reviewed by AI.
| return `${(score / 1000000).toFixed(1).replace(/\.0$/, '')}M`; | ||
| } | ||
| if (score >= 10000) { | ||
| return `${(score / 1000).toFixed(1).replace(/\.0$/, '')}K`; |
There was a problem hiding this comment.
Nit: 999,950–999,999 renders as "1000K". (999950 / 1000).toFixed(1) is "1000.0", which becomes 1000K because the >= 1000000 branch sits above it and doesn't catch it. Post views and upvotes can land in that range. Rounding first, or checking score >= 999950 for the M branch, avoids it. A spec case at the boundary would also help.
Reviewed by AI.
| /> | ||
| } | ||
| value={formatLeaderboardScore(item.score)} | ||
| highlight={rank - 1 === mine} |
There was a problem hiding this comment.
Non-blocking: your company is only lit when it is in the page's top 12. mine is an index into items, the page's 12 rows. renderRow is reused for the top-50 list, so a company ranked 13–50 shows unlit in the popover or sheet, and it isn't pinned either. Matching on the row itself (user.companies vs item.company.name) inside row would light it wherever it appears. Pinning past 12 would need the top-50 data and could stay out of scope.
Reviewed by AI.






Implements the leaderboard redesign from #7126 (Storybook → Leaderboard / Final) on production data only, no backend changes.
What changed
leaderboardPositionfor Highest reputation, Longest streak and Most reading days; opens on your best rank, with a board picker (bottom sheet on phones) and Snapshot throughProfileSnapshotButton(Origin.Leaderboard). States: ranked, past the cap (cappedAt, "outside the top 100K"), no rank yet (how-to copy, no Snapshot), loading, hidden signed out.useReferralLadder+useReferralCampaign, the 3-step ladder and Copy invite link with the production toast. Only for eligible users; the rank card widens when it is absent.Drawersheet on phones, and only asks for the list on open. Short boards say "All N". Most verified employees lights and pins your verified company.LEADERBOARD_QUERYasks for 12;leaderboardPositionQueryOptionsandleaderboardBoardQueryOptionsadded as options creators;UserLeaderboard/CompanyLeaderboardmoved tographql/leaderboard.ts(old exports kept), the user typed asUserShortProfile;CurrentUserPositionRowreuses the options.clickwith newTargetType.LeaderboardBoard(extrasurface: popover | sheet | page),TargetType.LeaderboardRankPicker,TargetType.LeaderboardUser(extraplacement,board,rank), andTargetId.LeaderboardInviteCardforcopy referral link.Not done
user_company), so it needs a backend field first.Verification
tscin shared and webapp, oxlint and oxfmt.LeaderboardRankCard.spec.tsxandLeaderboardBoardTile.spec.tsx, updatedUsersLeaderboardStaticProps.spec.ts; related suites in both packages pass (BoostPostModal.spec.tsxfails identically on main).🤖 Generated with Claude Code
Preview domain
https://feat-leaderboard-redesign.preview.app.daily.dev