Winners tab refactor#438
Merged
Benjtalkshow merged 21 commits intoMar 4, 2026
Merged
Conversation
…nto participating-page Merge into main
…nto participating-page Merge into main
…nto submission-page Merge into main
…nto submission-page Merge into main
|
@Michaelkingsdev is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughRefactors WinnersTab into a podium-aware layout: sorts winners by rank, extracts/reorders top three into a podium ([2,1,3]) for dedicated rendering, renders ranks 4+ in a separate responsive grid, and updates WinnerCard for podium styling, avatars, ribbons, tooltips, prize/trophy header, and deep links. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WinnersTab
participant WinnerCard
participant Router
User->>WinnersTab: page loads with winners[]
WinnersTab->>WinnersTab: sort winners, split podium & others
WinnersTab->>WinnerCard: render podium cards (isPodium=true)
WinnersTab->>WinnerCard: render other winner cards (isPodium=false)
User->>WinnerCard: click project area
WinnerCard->>Router: navigate to /projects/{projectId}?type=submission
User->>WinnerCard: click avatar/name
WinnerCard->>Router: navigate to /profile/{username}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
components/hackathons/winners/WinnersTab.tsx (4)
23-28: Consider removing unusedhackathonSlugfrom destructuring or interface.The
hackathonSlugprop is defined inWinnersTabProps(line 25) but is intentionally not used in the component. While the call site still passes it, this creates a disconnect. Consider either:
- Removing
hackathonSlugfromWinnersTabPropsif it's no longer needed, or- Adding a comment explaining why it's retained (e.g., for future use or API consistency).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/hackathons/winners/WinnersTab.tsx` around lines 23 - 28, The WinnersTabProps currently declares hackathonSlug but WinnersTab destructures only winners, leaving hackathonSlug unused; either remove hackathonSlug from the WinnersTabProps interface (and any call-sites that only pass it for this component) or keep it and add an inline comment in the WinnersTabProps definition explaining it’s intentionally retained for API consistency/future use; update the interface named WinnersTabProps and/or the component signature for WinnersTab to reflect the chosen approach so the prop list and usage are consistent.
183-186: Potential undefinedaltattribute value.Line 183 uses
alt={p.username}directly, but line 185 usesp.username?.charAt(0)suggestingusernamemight be undefined. Per theHackathonWinnertype,usernameis a requiredstring, so this should be safe. However, the defensive coding on line 185 creates inconsistency.Consider aligning both usages for consistency:
💡 Suggested consistency fix
- <AvatarImage src={p.avatar} alt={p.username} /> + <AvatarImage src={p.avatar} alt={p.username || 'Participant'} /> <AvatarFallback className='bg-gray-800 text-lg text-white uppercase'> - {p.username?.charAt(0) || '?'} + {p.username.charAt(0)} </AvatarFallback>Or keep the defensive approach on both if there's concern about runtime data:
- <AvatarImage src={p.avatar} alt={p.username} /> + <AvatarImage src={p.avatar} alt={p.username ?? 'Participant'} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/hackathons/winners/WinnersTab.tsx` around lines 183 - 186, The Avatar usage is inconsistent: AvatarImage sets alt={p.username} while AvatarFallback defensively uses p.username?.charAt(0); align them by choosing one approach—either rely on the HackathonWinner required string and use p.username in both places, or defensively handle possible undefined values in both places (e.g., alt={p.username ?? 'Unknown'} and AvatarFallback using p.username?.charAt(0) || '?'). Update the AvatarImage and AvatarFallback usages (references: AvatarImage, AvatarFallback, p.username, HackathonWinner) accordingly so they follow the same defensive or non-defensive pattern.
208-210: Minor: Avoid redundant function calls.
getRibbonColors(winner.rank)is called twice to extractprimaryColorandsecondaryColor. Consider destructuring once for cleaner code.♻️ Suggested optimization
+ const { primaryColor, secondaryColor } = getRibbonColors(winner.rank); <Ribbon - primaryColor={getRibbonColors(winner.rank).primaryColor} - secondaryColor={getRibbonColors(winner.rank).secondaryColor} + primaryColor={primaryColor} + secondaryColor={secondaryColor} className='w-48' />Note: This requires moving the destructuring to the component body before the return statement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/hackathons/winners/WinnersTab.tsx` around lines 208 - 210, Call getRibbonColors(winner.rank) once inside the WinnersTab component body, destructure its result into primaryColor and secondaryColor (e.g., const { primaryColor, secondaryColor } = getRibbonColors(winner.rank)), then pass those variables to the <Ribbon primaryColor={primaryColor} secondaryColor={secondaryColor} /> props to avoid duplicate calls and improve readability.
117-119: Consider usingsubmissionIdas fallback for project navigation.Per the
HackathonWinnertype,submissionIdis required whileprojectIdis optional. Currently, whenprojectIdis missing, the project content becomes non-clickable. If the submission page can be accessed viasubmissionId, consider using it as a fallback to improve user experience.That said, if the design intentionally requires non-clickable fallbacks when
projectIdis absent (as stated in PR objectives), this is fine as-is.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/hackathons/winners/WinnersTab.tsx` around lines 117 - 119, The project link currently uses only winner.projectId so rows become non-clickable when projectId is missing; update the projectUrl logic (the projectUrl variable) to fallback to winner.submissionId when projectId is falsy (e.g. projectUrl = winner.projectId ? `/projects/${winner.projectId}?type=submission` : winner.submissionId ? `/submissions/${winner.submissionId}` : null) so the UI remains navigable; ensure you reference HackathonWinner fields winner.projectId and winner.submissionId and preserve null when neither exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/hackathons/winners/WinnersTab.tsx`:
- Around line 190-202: The wrapper elements in the participant avatar mapping
use inconsistent keys which can break React reconciliation: change the div
branch to use the same key pattern as the Link branch (e.g.,
`${p.username}-${i}`) so both branches use an identical key; locate the
conditional that returns profileUrl ? <Link ...> : <div ...> around
AvatarElement and update the div's key to match the Link's key (still
referencing p.username and i).
- Around line 230-233: The external Link rendering in WinnersTab (the JSX branch
using projectUrl and rendering {ProjectContent} inside the Next.js Link) lacks
rel attributes for target="_blank"; update the Link element that wraps
ProjectContent to include rel="noopener noreferrer" whenever target="_blank" is
present (i.e., when projectUrl is truthy) so the rendered anchor includes
rel="noopener noreferrer" to prevent tabnabbing.
---
Nitpick comments:
In `@components/hackathons/winners/WinnersTab.tsx`:
- Around line 23-28: The WinnersTabProps currently declares hackathonSlug but
WinnersTab destructures only winners, leaving hackathonSlug unused; either
remove hackathonSlug from the WinnersTabProps interface (and any call-sites that
only pass it for this component) or keep it and add an inline comment in the
WinnersTabProps definition explaining it’s intentionally retained for API
consistency/future use; update the interface named WinnersTabProps and/or the
component signature for WinnersTab to reflect the chosen approach so the prop
list and usage are consistent.
- Around line 183-186: The Avatar usage is inconsistent: AvatarImage sets
alt={p.username} while AvatarFallback defensively uses p.username?.charAt(0);
align them by choosing one approach—either rely on the HackathonWinner required
string and use p.username in both places, or defensively handle possible
undefined values in both places (e.g., alt={p.username ?? 'Unknown'} and
AvatarFallback using p.username?.charAt(0) || '?'). Update the AvatarImage and
AvatarFallback usages (references: AvatarImage, AvatarFallback, p.username,
HackathonWinner) accordingly so they follow the same defensive or non-defensive
pattern.
- Around line 208-210: Call getRibbonColors(winner.rank) once inside the
WinnersTab component body, destructure its result into primaryColor and
secondaryColor (e.g., const { primaryColor, secondaryColor } =
getRibbonColors(winner.rank)), then pass those variables to the <Ribbon
primaryColor={primaryColor} secondaryColor={secondaryColor} /> props to avoid
duplicate calls and improve readability.
- Around line 117-119: The project link currently uses only winner.projectId so
rows become non-clickable when projectId is missing; update the projectUrl logic
(the projectUrl variable) to fallback to winner.submissionId when projectId is
falsy (e.g. projectUrl = winner.projectId ?
`/projects/${winner.projectId}?type=submission` : winner.submissionId ?
`/submissions/${winner.submissionId}` : null) so the UI remains navigable;
ensure you reference HackathonWinner fields winner.projectId and
winner.submissionId and preserve null when neither exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b11f17a-d82d-4d24-b527-2ca0a34a9e70
📒 Files selected for processing (1)
components/hackathons/winners/WinnersTab.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/hackathons/winners/WinnersTab.tsx (1)
70-77: Avoid nested ternary for class selection in JSXThis class composition is hard to scan and maintain. Consider moving podium layout selection to a small map/helper and keep JSX className flat.
As per coding guidelines, "For conditional classes, prefer clsx or similar helper functions over ternary operators in JSX".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/hackathons/winners/WinnersTab.tsx` around lines 70 - 77, The nested ternary inside the WinnersTab JSX for computing className is hard to read; replace it by extracting the logic into a small helper (e.g., getPodiumClasses or a PODIUM_CLASS_MAP) that takes podiumToDisplay.length and returns the appropriate class string, then call cn(getPodiumClasses(podiumToDisplay.length), 'grid gap-6 md:gap-8') (or use clsx) so the JSX className is flat and readable; update references to podiumToDisplay in WinnersTab to use that helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/hackathons/winners/WinnersTab.tsx`:
- Around line 117-121: The current projectUrl ternary creates a clickable
/submissions/... fallback when winner.projectId is missing; change it so
projectUrl is only set when winner.projectId exists (i.e.,
`/projects/${winner.projectId}?type=submission`) and otherwise set projectUrl to
null to render a non-clickable fallback; update the declaration of projectUrl in
WinnersTab (the variable named projectUrl that references winner.projectId and
winner.submissionId) accordingly and ensure any rendering logic uses projectUrl
=== null to show the non-clickable UI.
- Around line 223-229: The display name in WinnersTab.tsx is rendered as plain
text; change the single-participant branch so that when winner.teamName is falsy
and winner.participants.length === 1 and winner.participants[0].username exists,
the username is wrapped with a profile link to `/profile/${username}` (using
your project's Link component/anchor pattern) while keeping the existing
styling; locate the block rendering winner.teamName / participant fallback (the
JSX around the h3 in WinnersTab.tsx) and replace the plain text username with a
linked element that preserves classes and accessibility.
---
Nitpick comments:
In `@components/hackathons/winners/WinnersTab.tsx`:
- Around line 70-77: The nested ternary inside the WinnersTab JSX for computing
className is hard to read; replace it by extracting the logic into a small
helper (e.g., getPodiumClasses or a PODIUM_CLASS_MAP) that takes
podiumToDisplay.length and returns the appropriate class string, then call
cn(getPodiumClasses(podiumToDisplay.length), 'grid gap-6 md:gap-8') (or use
clsx) so the JSX className is flat and readable; update references to
podiumToDisplay in WinnersTab to use that helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 732a25af-a39d-4633-80d4-c306fcb301e4
📒 Files selected for processing (1)
components/hackathons/winners/WinnersTab.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/hackathons/winners/WinnersTab.tsx (1)
182-185: Prefercnconditional object instead of ternary in JSX class composition.Line 184 uses a ternary for class selection; this should use
cn/clsx-style conditional composition per repo convention.♻️ Suggested refactor
<Avatar className={cn( 'border-background h-16 w-16 border-2 shadow-xl', - profileUrl ? 'transition-transform hover:scale-105' : '' + { + 'transition-transform hover:scale-105': Boolean(profileUrl), + } )} >As per coding guidelines, “For conditional classes, prefer clsx or similar helper functions over ternary operators in JSX”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/hackathons/winners/WinnersTab.tsx` around lines 182 - 185, Replace the ternary used inside the cn() call for the avatar classes with a conditional object form: locate the className prop in WinnersTab (where cn is invoked with 'border-background h-16 w-16 border-2 shadow-xl' and profileUrl), and change the ternary expression using profileUrl ? 'transition-transform hover:scale-105' : '' to an object-style conditional entry (e.g., { 'transition-transform hover:scale-105': profileUrl }) so the clsx/cn convention is followed; keep the existing static classes and the cn import/usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/hackathons/winners/WinnersTab.tsx`:
- Around line 52-60: getPodiumOrder currently uses find to pick a single winner
per rank (first/second/third) which drops co-winners; change it to collect all
winners per rank using winners.filter(w => w.rank === 1), .filter(... === 2),
.filter(... === 3), then return an array that concatenates seconds, firsts,
thirds (e.g., [...seconds, ...firsts, ...thirds]) so ties are preserved; also
update the code that builds the remaining/grid list (the logic near line ~63
that excludes podium entries) to exclude all entries with rank 1–3 (use w.rank
=== 1 || w.rank === 2 || w.rank === 3) instead of removing only the single found
winners so tied top ranks are not silently dropped.
---
Nitpick comments:
In `@components/hackathons/winners/WinnersTab.tsx`:
- Around line 182-185: Replace the ternary used inside the cn() call for the
avatar classes with a conditional object form: locate the className prop in
WinnersTab (where cn is invoked with 'border-background h-16 w-16 border-2
shadow-xl' and profileUrl), and change the ternary expression using profileUrl ?
'transition-transform hover:scale-105' : '' to an object-style conditional entry
(e.g., { 'transition-transform hover:scale-105': profileUrl }) so the clsx/cn
convention is followed; keep the existing static classes and the cn import/usage
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02eb31c4-67f9-4b79-84cc-80c395bbe440
📒 Files selected for processing (1)
components/hackathons/winners/WinnersTab.tsx
Comment on lines
+52
to
+60
| const getPodiumOrder = (winners: HackathonWinner[]) => { | ||
| if (winners.length < 2) return winners; | ||
| const first = winners.find(w => w.rank === 1); | ||
| const second = winners.find(w => w.rank === 2); | ||
| const third = winners.find(w => w.rank === 3); | ||
|
|
||
| return [second, first, third].filter( | ||
| (w): w is HackathonWinner => w !== undefined | ||
| ); |
There was a problem hiding this comment.
Podium extraction can silently drop tied winners in top ranks.
On Line 54–56, find keeps only one winner per rank. If there are co-winners (e.g., two rank-1 entries), extra entries are omitted from both podium and ranks 4+ grid.
💡 Suggested fix
- const getPodiumOrder = (winners: HackathonWinner[]) => {
- if (winners.length < 2) return winners;
- const first = winners.find(w => w.rank === 1);
- const second = winners.find(w => w.rank === 2);
- const third = winners.find(w => w.rank === 3);
-
- return [second, first, third].filter(
- (w): w is HackathonWinner => w !== undefined
- );
- };
-
- const podiumToDisplay = getPodiumOrder(podiumWinners);
+ const podiumRankOrder = [2, 1, 3] as const;
+ const podiumToDisplay = podiumRankOrder.flatMap(rank =>
+ podiumWinners.filter(w => w.rank === rank)
+ );Also applies to: 63-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/hackathons/winners/WinnersTab.tsx` around lines 52 - 60,
getPodiumOrder currently uses find to pick a single winner per rank
(first/second/third) which drops co-winners; change it to collect all winners
per rank using winners.filter(w => w.rank === 1), .filter(... === 2),
.filter(... === 3), then return an array that concatenates seconds, firsts,
thirds (e.g., [...seconds, ...firsts, ...thirds]) so ties are preserved; also
update the code that builds the remaining/grid list (the logic near line ~63
that excludes podium entries) to exclude all entries with rank 1–3 (use w.rank
=== 1 || w.rank === 2 || w.rank === 3) instead of removing only the single found
winners so tied top ranks are not silently dropped.
Benjtalkshow
approved these changes
Mar 4, 2026
Merged
7 tasks
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
2 participants
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 #399
Winners Tab Refactor — Premium Organizer Design Integration
Overview
This PR overhauls the participant-facing Winners Tab to match the premium Podium design already established in the organizer's
PublishWinnersWizard.The previous basic card grid has been replaced with a high-fidelity visual hierarchy that mirrors the administrative view.
Key Changes
UI & Design Replication
Podium Layout
Premium Components
Ribboncomponent for rank displayProject Preview
USDC Branding
Deep Linking & Navigation
Project Links
/projects/${projectId}?type=submissionParticipant Links
/profile/${username}Graceful Fallbacks
Technical Details
Component Updated
components/hackathons/winners/WinnersTab.tsxData Flow
useHackathonData()hookType Safety
HackathonWinnerinterfaceanytypesAnimation
motion/reactfor premium UX polishVideo Proof
Summary by CodeRabbit
New Features
Style