Conversation
📝 WalkthroughWalkthroughRefactors leaderboard header: consolidates sorting UI to a single CommonDropdown, adds breakpoint-aware rendering in curation group select (mobile CommonDropdown vs desktop tabs) with per-group parallel queries, adjusts CommonDropdown styling, and updates tests/mocks to match these behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WaveCuration as WaveLeaderboardCurationGroupSelect
participant QueryLayer as useQueries
participant Dropdown as CommonDropdown
participant Parent as ParentHandler
User->>WaveCuration: render (viewport check)
WaveCuration->>QueryLayer: fetch per-group details (parallel)
QueryLayer-->>WaveCuration: group details (avatars)
alt mobile
WaveCuration->>Dropdown: render mobile dropdown with items
User->>Dropdown: click item
Dropdown-->>WaveCuration: onChange(value)
WaveCuration->>Parent: propagate selection
else desktop
WaveCuration->>WaveCuration: render inline tab buttons
User->>WaveCuration: click tab
WaveCuration->>Parent: propagate selection
end
sequenceDiagram
participant User
participant WaveSort as WaveleaderboardSort
participant Dropdown as CommonDropdown
participant Parent as ParentHandler
User->>WaveSort: view component
WaveSort->>Dropdown: render sort items
User->>Dropdown: open & select sort
Dropdown-->>WaveSort: onChange(sort)
WaveSort->>Parent: onSortChange(sort)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
components/waves/leaderboard/header/WaveLeaderboardCurationGroupSelect.tsx (2)
86-123: Desktop tabs lack semanticroleandariaattributes.The view mode tabs in
WaveleaderboardHeader.tsx(Line 139-148) userole="tablist",role="tab", andaria-selected. These curation group tabs serve a similar purpose but lack equivalent accessibility attributes, creating an inconsistency.♿ Suggested accessibility improvement
return ( <div data-testid="curation-group-select" - className="tw-flex tw-items-center tw-gap-x-1" + className="tw-flex tw-items-center tw-gap-x-1" + role="tablist" + aria-label="Curation groups" > {items.map((item) => { const pfp = item.value ? pfpMap.get(item.value) : null; return ( <button key={item.key} type="button" + role="tab" + aria-selected={selectedGroupId === item.value} className={getTabClassName(item.value)} onClick={() => onChange(item.value)} >
43-53: Spreading query results intouseMemodeps is fragile.The
// eslint-disable-next-line react-hooks/exhaustive-depssuppression on the spread...groupDetailsQueries.map((q) => q.data)works in practice because react-query preserves referential identity for unchanged data, but the deps array length changes ifgroupschanges, which can confuse the hooks contract. Consider deriving a stable dependency instead.♻️ Alternative: use a JSON-serialized key
const pfpMap = useMemo(() => { const map = new Map<string, string>(); groups.forEach((group, i) => { const pfp = groupDetailsQueries[i]?.data?.created_by?.pfp; if (pfp) { map.set(group.id, pfp); } }); return map; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [groups, ...groupDetailsQueries.map((q) => q.data)]); + }, [groups, groupDetailsQueries]);Since
useQueriesreturns a new array reference only when query results change, using the array directly is equivalent and avoids the lint suppression. Alternatively, if you want more precise control, you could extract a serialized key from the query data.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@components/waves/leaderboard/header/WaveLeaderboardCurationGroupSelect.tsx`:
- Line 21: The hydration mismatch comes from createBreakpoint initializing
differently on server vs client; fix by client-only rendering: change the export
of WaveLeaderboardCurationGroupSelect to use Next's dynamic import with ssr:
false (i.e., export default dynamic(() =>
import('./WaveLeaderboardCurationGroupSelect'), { ssr: false })), remove or keep
createBreakpoint usage only for client, and ensure any breakpoint-dependent
rendering (CommonDropdown vs tab buttons) happens only in the client-only
component so server and client markup match.
- Around line 43-53: The current WaveLeaderboardCurationGroupSelect creates
pfpMap with a variable-length dependency array ([groups,
...groupDetailsQueries.map(q=>q.data)]) which triggers React warnings and is
suppressed by eslint-disable; instead refactor to produce a single stable
dependency (e.g., use useQueries' combine to return { data:
results.map(r=>r.data?.created_by?.pfp) } or compute a stable
key/hash/JSON.stringify of the data) and then compute pfpMap in useMemo
depending only on [groups, groupData] (or [groups, stableKey]); remove the
eslint-disable and the spread dependency, and reference the pfpMap computation
and the groupDetailsQueries/useQueries call in
WaveLeaderboardCurationGroupSelect when applying the fix.
🧹 Nitpick comments (1)
components/waves/leaderboard/header/WaveLeaderboardCurationGroupSelect.tsx (1)
86-125: Desktop tab rendering is well-structured.Good details:
type="button"prevents accidental form submissions,alt=""is correct for decorative avatars, and the conditional pfp rendering gracefully handles the loading state (no avatar until data arrives).One optional nit:
getTabClassNameis re-created on every render. Since it capturesselectedGroupIdfrom closure, you could extract it to auseCallbackor passselectedGroupIdas a parameter to a module-level function. This is purely a minor optimization and not necessary given the component's complexity.
|



Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Style
Tests