show scored soldier positions#14
Conversation
|
@Fierillo is attempting to deploy a commit to the La Crypta Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR refactors soldiers scoring display to replace medal counts with a positions-based breakdown grouped by hackathon. Position entries are computed, labeled, sorted, and rendered as chips with detailed tooltips. Related labels and styling are updated for consistency across the soldiers table and detail page. ChangesPositions Scoring Display
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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)
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
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 (1)
app/soldados/SoldiersTable.tsx (1)
255-255: ⚡ Quick winUse
entry.pointsinstead of redundantPOSITION_POINTS[position]lookup.The
entryobject already containspoints: p.positionPoints(computed on line 81), so accessingPOSITION_POINTS[position]is redundant and less robust.♻️ Proposed fix
- left: `${positionLabel(position)} · ${hackathonLabel} · ${POSITION_POINTS[position]} pts`, + left: `${positionLabel(position)} · ${hackathonLabel} · ${entry.points} pts`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/soldados/SoldiersTable.tsx` at line 255, Replace the redundant constant lookup with the computed value on the entry object: in SoldiersTable (the template building the left label that currently uses positionLabel(position) · hackathonLabel · POSITION_POINTS[position] pts), use entry.points instead of POSITION_POINTS[position]; update the interpolation that composes the left string so it references entry.points (which is set from p.positionPoints) to make the display robust and avoid duplicated logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/soldados/`[slug]/page.tsx:
- Around line 212-215: Update the label text in the detail page component so it
matches the table terminology: change the label prop currently set to
"Posiciones" to "Puestos" where the component renders label="Posiciones" (in
app/soldados/[slug]/page.tsx) and ensure it still displays the same value using
soldier.scoreBreakdown.positions; this keeps the UI consistent with
SoldiersTable.tsx which uses "Puestos".
In `@app/soldados/SoldiersTable.tsx`:
- Around line 264-272: The position chip keys can collide because
`${projectId}-${position}` lacks hackathon context; update the entries.map key
to include hackathonId (e.g., `${projectId}-${hackathonId}-${position}`) so each
chip is unique across hackathons—locate the entries.map rendering in
SoldiersTable.tsx and modify the key accordingly while leaving
positionLabel(position) and other props unchanged.
---
Nitpick comments:
In `@app/soldados/SoldiersTable.tsx`:
- Line 255: Replace the redundant constant lookup with the computed value on the
entry object: in SoldiersTable (the template building the left label that
currently uses positionLabel(position) · hackathonLabel ·
POSITION_POINTS[position] pts), use entry.points instead of
POSITION_POINTS[position]; update the interpolation that composes the left
string so it references entry.points (which is set from p.positionPoints) to
make the display robust and avoid duplicated logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4855a70e-1379-45b3-8728-8d0928b5fd3f
📒 Files selected for processing (2)
app/soldados/SoldiersTable.tsxapp/soldados/[slug]/page.tsx
| label="Posiciones" | ||
| value={`+${soldier.scoreBreakdown.positions}`} | ||
| sub="podio" | ||
| sub="top 6" | ||
| /> |
There was a problem hiding this comment.
Inconsistent terminology: "Posiciones" vs "Puestos".
The soldiers table now uses "Puestos" (SoldiersTable.tsx lines 125, 276, 502), but this detail page still uses "Posiciones" as the label. Consider using "Puestos" here as well for consistency across the UI.
📝 Proposed fix for consistency
<ScoreCell
- label="Posiciones"
+ label="Puestos"
value={`+${soldier.scoreBreakdown.positions}`}
sub="top 6"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| label="Posiciones" | |
| value={`+${soldier.scoreBreakdown.positions}`} | |
| sub="podio" | |
| sub="top 6" | |
| /> | |
| label="Puestos" | |
| value={`+${soldier.scoreBreakdown.positions}`} | |
| sub="top 6" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/soldados/`[slug]/page.tsx around lines 212 - 215, Update the label text
in the detail page component so it matches the table terminology: change the
label prop currently set to "Posiciones" to "Puestos" where the component
renders label="Posiciones" (in app/soldados/[slug]/page.tsx) and ensure it still
displays the same value using soldier.scoreBreakdown.positions; this keeps the
UI consistent with SoldiersTable.tsx which uses "Puestos".
| {entries.map(({ position, hackathonLabel, projectId }) => ( | ||
| <span | ||
| key={`${projectId}-${position}`} | ||
| title={hackathonLabel} | ||
| className="inline-flex items-center gap-1 rounded-full border border-border bg-white/[0.03] px-2 py-0.5 text-foreground-muted" | ||
| > | ||
| <span>{positionLabel(position)}</span> | ||
| </span> | ||
| )} | ||
| ))} |
There was a problem hiding this comment.
Fix potential key collision in position chips.
The key ${projectId}-${position} could collide if the same project appears in multiple hackathons with the same position. Include hackathonId in the key for uniqueness.
🔑 Proposed fix
{entries.map(({ position, hackathonLabel, projectId }) => (
+ {entries.map(({ position, hackathonLabel, projectId, hackathonId }) => (
<span
- key={`${projectId}-${position}`}
+ key={`${projectId}-${hackathonId ?? 'none'}-${position}`}
title={hackathonLabel}
className="inline-flex items-center gap-1 rounded-full border border-border bg-white/[0.03] px-2 py-0.5 text-foreground-muted"
>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/soldados/SoldiersTable.tsx` around lines 264 - 272, The position chip
keys can collide because `${projectId}-${position}` lacks hackathon context;
update the entries.map key to include hackathonId (e.g.,
`${projectId}-${hackathonId}-${position}`) so each chip is unique across
hackathons—locate the entries.map rendering in SoldiersTable.tsx and modify the
key accordingly while leaving positionLabel(position) and other props unchanged.
1 participant
improve visualization of scores in soldier table
Summary by CodeRabbit
Release Notes