Add division breakdown dialog to registration form#327
Open
zacjones93 wants to merge 1 commit intomainfrom
Open
Add division breakdown dialog to registration form#327zacjones93 wants to merge 1 commit intomainfrom
zacjones93 wants to merge 1 commit intomainfrom
Conversation
Adds a "Not sure which division to choose?" help link below the division selector on the registration page. Clicking it opens a dialog showing all division descriptions so athletes can compare skill levels, movement standards, and typical athlete profiles without leaving the registration flow. The dialog only renders when divisions have descriptions set by the organizer. https://claude.ai/code/session_01RuakjTmn8yJgMyUW8F4wFy
Contributor
WalkthroughIntroduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/wodsmith-start/src/components/registration/division-breakdown-dialog.tsx (1)
16-22: Use a props interface for this component.The inline object type makes this new TSX component inconsistent with the repo’s TypeScript convention and harder to reuse.
As per coding guidelines, `**/*.{ts,tsx}`: Use TypeScript everywhere, prefer interfaces over types.♻️ Proposed refactor
+interface DivisionBreakdownDialogProps { + scalingLevels: ScalingLevel[] + publicDivisions: PublicCompetitionDivision[] +} + export function DivisionBreakdownDialog({ scalingLevels, publicDivisions, -}: { - scalingLevels: ScalingLevel[] - publicDivisions: PublicCompetitionDivision[] -}) { +}: DivisionBreakdownDialogProps) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/wodsmith-start/src/components/registration/division-breakdown-dialog.tsx` around lines 16 - 22, Replace the inline object type on the DivisionBreakdownDialog component with a named props interface: create an interface (e.g., DivisionBreakdownDialogProps) that declares scalingLevels: ScalingLevel[] and publicDivisions: PublicCompetitionDivision[], then update the component signature to accept props: DivisionBreakdownDialogProps (e.g., function DivisionBreakdownDialog(props: DivisionBreakdownDialogProps) or function DivisionBreakdownDialog({ scalingLevels, publicDivisions }: DivisionBreakdownDialogProps)). Ensure the interface is exported if this component is imported elsewhere and keep the existing exported component name DivisionBreakdownDialog 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
`@apps/wodsmith-start/src/components/registration/division-breakdown-dialog.tsx`:
- Around line 25-35: The filter currently keeps entries whose description may be
only whitespace; update the mapping for divisionsWithDescriptions so the
assigned description is trimmed and set to null when empty (use scalingLevels,
publicDivisions and the description property), then filter based on the trimmed
value; in short, replace description: publicDiv?.description ?? null with a
trimmed version (e.g. trim and fallback to null) and keep the .filter((d) =>
d.description) check.
In `@apps/wodsmith-start/src/components/registration/registration-form.tsx`:
- Around line 997-1049: The summary is skipped when divisionFees.size === 0 even
though users may have selected divisions with $0 fees; update the render guard
and subtotal calc so zero-cost selections still show totals: change the outer
condition from "divisionFees.size > 0" to "(divisionFees.size > 0 ||
selectedDivisionIds.length > 0)" and compute subtotal from selectedDivisionIds
(e.g., subtotal = selectedDivisionIds.reduce((s,id)=>s + (divisionFees.get(id)
?? 0), 0)) so activeCoupon, discount, and total logic (using activeCoupon,
discount, total) still render for $0 totals.
---
Nitpick comments:
In
`@apps/wodsmith-start/src/components/registration/division-breakdown-dialog.tsx`:
- Around line 16-22: Replace the inline object type on the
DivisionBreakdownDialog component with a named props interface: create an
interface (e.g., DivisionBreakdownDialogProps) that declares scalingLevels:
ScalingLevel[] and publicDivisions: PublicCompetitionDivision[], then update the
component signature to accept props: DivisionBreakdownDialogProps (e.g.,
function DivisionBreakdownDialog(props: DivisionBreakdownDialogProps) or
function DivisionBreakdownDialog({ scalingLevels, publicDivisions }:
DivisionBreakdownDialogProps)). Ensure the interface is exported if this
component is imported elsewhere and keep the existing exported component name
DivisionBreakdownDialog unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 19157d94-3fbb-489e-9b02-2d863a4b17f7
📒 Files selected for processing (2)
apps/wodsmith-start/src/components/registration/division-breakdown-dialog.tsxapps/wodsmith-start/src/components/registration/registration-form.tsx
Comment on lines
+25
to
+35
| // Only show divisions that have descriptions | ||
| const divisionsWithDescriptions = scalingLevels | ||
| .map((level) => { | ||
| const publicDiv = publicDivisions.find((d) => d.id === level.id) | ||
| return { | ||
| ...level, | ||
| description: publicDiv?.description ?? null, | ||
| } | ||
| }) | ||
| .filter((d) => d.description) | ||
|
|
Contributor
There was a problem hiding this comment.
Trim before deciding a description is present.
Whitespace-only descriptions still pass this filter, so the dialog can render empty cards.
🩹 Proposed fix
const divisionsWithDescriptions = scalingLevels
.map((level) => {
const publicDiv = publicDivisions.find((d) => d.id === level.id)
return {
...level,
description: publicDiv?.description ?? null,
}
})
- .filter((d) => d.description)
+ .filter((d) => (d.description?.trim().length ?? 0) > 0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/wodsmith-start/src/components/registration/division-breakdown-dialog.tsx`
around lines 25 - 35, The filter currently keeps entries whose description may
be only whitespace; update the mapping for divisionsWithDescriptions so the
assigned description is trimmed and set to null when empty (use scalingLevels,
publicDivisions and the description property), then filter based on the trimmed
value; in short, replace description: publicDiv?.description ?? null with a
trimmed version (e.g. trim and fallback to null) and keep the .filter((d) =>
d.description) check.
Comment on lines
+997
to
+1049
| {divisionFees.size > 0 && | ||
| (() => { | ||
| const subtotal = Array.from(divisionFees.values()).reduce( | ||
| (sum, c) => sum + c, | ||
| 0, | ||
| ) | ||
| } | ||
| // With coupon — always show subtotal, discount, and adjusted total | ||
| const discount = Math.min(activeCoupon.amountOffCents, subtotal) | ||
| const total = subtotal - discount | ||
| return ( | ||
| <> | ||
| {selectedDivisionIds.length > 1 && ( | ||
| <div className="flex justify-between text-sm pt-2 border-t"> | ||
| <span>Subtotal</span> | ||
| <span>${(subtotal / 100).toFixed(2)}</span> | ||
| if (!activeCoupon) { | ||
| // No coupon — show simple total for multi-division | ||
| if (selectedDivisionIds.length <= 1) return null | ||
| return ( | ||
| <div className="flex justify-between font-medium pt-2 border-t"> | ||
| <span>Total</span> | ||
| <span className="text-lg"> | ||
| ${(subtotal / 100).toFixed(2)} | ||
| </span> | ||
| </div> | ||
| )} | ||
| <div className={cn( | ||
| "flex justify-between text-sm text-emerald-700 dark:text-emerald-400", | ||
| selectedDivisionIds.length <= 1 && "pt-2 border-t", | ||
| )}> | ||
| <span className="flex items-center gap-1.5"> | ||
| <Tag className="h-3.5 w-3.5" /> | ||
| Coupon ({activeCoupon.code}) | ||
| </span> | ||
| <span>-${(discount / 100).toFixed(2)}</span> | ||
| </div> | ||
| <div className="flex justify-between font-medium pt-2 border-t"> | ||
| <span>Total</span> | ||
| <span className="text-lg"> | ||
| ${(total / 100).toFixed(2)} | ||
| </span> | ||
| </div> | ||
| </> | ||
| ) | ||
| })()} | ||
| ) | ||
| } | ||
| // With coupon — always show subtotal, discount, and adjusted total | ||
| const discount = Math.min( | ||
| activeCoupon.amountOffCents, | ||
| subtotal, | ||
| ) | ||
| const total = subtotal - discount | ||
| return ( | ||
| <> | ||
| {selectedDivisionIds.length > 1 && ( | ||
| <div className="flex justify-between text-sm pt-2 border-t"> | ||
| <span>Subtotal</span> | ||
| <span>${(subtotal / 100).toFixed(2)}</span> | ||
| </div> | ||
| )} | ||
| <div | ||
| className={cn( | ||
| "flex justify-between text-sm text-emerald-700 dark:text-emerald-400", | ||
| selectedDivisionIds.length <= 1 && "pt-2 border-t", | ||
| )} | ||
| > | ||
| <span className="flex items-center gap-1.5"> | ||
| <Tag className="h-3.5 w-3.5" /> | ||
| Coupon ({activeCoupon.code}) | ||
| </span> | ||
| <span>-${(discount / 100).toFixed(2)}</span> | ||
| </div> | ||
| <div className="flex justify-between font-medium pt-2 border-t"> | ||
| <span>Total</span> | ||
| <span className="text-lg"> | ||
| ${(total / 100).toFixed(2)} | ||
| </span> | ||
| </div> | ||
| </> | ||
| ) | ||
| })()} |
Contributor
There was a problem hiding this comment.
Don’t hide the summary for valid $0 totals.
This block only renders when divisionFees.size > 0, but zero-cost selections are excluded from divisionFees. In multi-division/coupon flows, that can leave the fee card with no overall total at all.
💸 Proposed fix
- {divisionFees.size > 0 &&
+ {divisionFees.size === selectedDivisionIds.length &&
(() => { const handleFeesLoaded = (
divisionId: string,
fees: { isFree: boolean; totalChargeCents?: number } | null,
) => {
setDivisionFees((prev) => {
const next = new Map(prev)
- if (fees && !fees.isFree && fees.totalChargeCents) {
- next.set(divisionId, fees.totalChargeCents)
+ if (fees) {
+ next.set(divisionId, fees.totalChargeCents ?? 0)
} else {
next.delete(divisionId)
}
return next
})
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/wodsmith-start/src/components/registration/registration-form.tsx` around
lines 997 - 1049, The summary is skipped when divisionFees.size === 0 even
though users may have selected divisions with $0 fees; update the render guard
and subtotal calc so zero-cost selections still show totals: change the outer
condition from "divisionFees.size > 0" to "(divisionFees.size > 0 ||
selectedDivisionIds.length > 0)" and compute subtotal from selectedDivisionIds
(e.g., subtotal = selectedDivisionIds.reduce((s,id)=>s + (divisionFees.get(id)
?? 0), 0)) so activeCoupon, discount, and total logic (using activeCoupon,
discount, total) still render for $0 totals.
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.
Summary
Added a new
DivisionBreakdownDialogcomponent to the registration form that helps users understand the differences between available divisions before making their selection.Key Changes
New Component: Created
DivisionBreakdownDialogthat displays division descriptions in a modal dialogRegistration Form Integration:
scalingLevelsandpublicDivisionsas props to populate the dialog contentCode Organization:
registration-form.tsxfor better readability (grouped by type)Implementation Details
Dialog,ScrollArea,Badge) for consistencyhttps://claude.ai/code/session_01RuakjTmn8yJgMyUW8F4wFy
Summary by cubic
Adds a help dialog to the division selector so athletes can compare divisions without leaving registration. Improves clarity by showing division descriptions and team size when available.
DivisionBreakdownDialogtriggered by a “Not sure which division to choose?” link.scalingLevelsandpublicDivisionsprops; renders only when data exists.Written for commit 7919554. Summary will update on new commits.
Summary by CodeRabbit
New Features
Refactor