Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update introduces dynamic player slot management and player count display for tournament competitors, enhances sorting and filtering logic for competitors and pairings, and refactors several UI components and styles. It also adds new helpers for user name visibility in tournaments, consolidates card styling, and improves internal state handling for tournament pairing grids. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TournamentCompetitorForm
participant Server
User->>TournamentCompetitorForm: Click "Add Player Slot"
TournamentCompetitorForm->>TournamentCompetitorForm: Append empty player slot if allowed
User->>TournamentCompetitorForm: Fill player info, submit form
TournamentCompetitorForm->>TournamentCompetitorForm: Filter out players without userId
TournamentCompetitorForm->>Server: Submit filtered players
Server-->>TournamentCompetitorForm: Respond with update result
sequenceDiagram
participant TournamentRoster
participant PlayerCount
TournamentRoster->>PlayerCount: Render with competitor and competitorSize
PlayerCount->>PlayerCount: Count active players
PlayerCount-->>TournamentRoster: Display "active/total" player count
Possibly related PRs
Poem
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
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (46)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
src/components/ToastProvider/ToastProvider.module.scss (1)
29-40: Shadow now defined twice
variants.card($elevated: true)supplies its own shadow.
Lines 38-40 set an explicitbox-shadow, which will override (or duplicate) the mixin’s value. This can lead to inconsistent depth compared to other components that rely solely on the mixin.- background-color: var(--card-bg); - box-shadow: - hsl(206deg 22% 7% / 35%) 0 10px 38px -10px, - hsl(206deg 22% 7% / 20%) 0 10px 20px -15px; + /* variants.card already handles bg + shadow */Drop the manual styles unless there’s a specific visual requirement.
convex/_model/tournaments/_helpers/deepenTournament.ts (1)
33-41:activePlayerCountmay double-count duplicates – normalise with aSetIf a user appears in multiple competitors (edge-case but possible with data issues), the current reduce will count them multiple times.
De-duplicate first soactivePlayerCounttruly reflects unique active players.-const activePlayerUserIds = tournamentCompetitors.reduce((acc, c) => [ - ...acc, - ...c.players.filter((p) => p.active).map((p) => p.userId), -], [] as Id<'users'>[]); +const activePlayerUserIds = [ + ...new Set( + tournamentCompetitors.flatMap((c) => + c.players.filter((p) => p.active).map((p) => p.userId), + ), + ), +] as Id<'users'>[];This change keeps
activePlayerCountcorrect without other modifications.
🧹 Nitpick comments (15)
src/components/TournamentPairingsGrid/PairingsGridRow/PairingsGridRow.tsx (3)
4-4: Consider colocating all pairing-related types
TournamentCompetitorRankedis still imported from~/apiwhileDraftTournamentPairingwas moved to a local types file. Keeping related domain types in one place (e.g. the newTournamentPairingsGrid.types.ts) reduces coupling to the API layer and makes refactors cheaper.
8-9: Nit: use barrel export to avoid deep relative paths
'../TournamentPairingsGrid.types'reaches outside the sub-folder. A barrel file atTournamentPairingsGrid/index.ts(already present for the component) could re-export the type:-import { DraftTournamentPairing } from '../TournamentPairingsGrid.types'; +import type { DraftTournamentPairing } from '..';This keeps import paths stable if the file hierarchy changes again.
30-31: Guard expression could be clearer & properly typedThe tuple access on a
Partial<DraftTournamentPairing>loses type safety. A small rewrite avoids falsey checks and narrows the type:-const isValid = pairing && pairing[0] && pairing[1] ? checkDraftPairingIsValid(pairing) : undefined; +const isComplete = pairing?.[0] && pairing?.[1]; +const isValid = isComplete ? checkDraftPairingIsValid(pairing as DraftTournamentPairing) : undefined;Makes intent explicit and prevents accidental truthy coercion on
0/ empty strings.src/pages/TournamentAdvanceRoundPage/components/ConfirmPairingsDialog/ConfirmPairingsDialog.tsx (2)
1-5: Minor: keep import grouping consistentNow that both
UnassignedTournamentPairingandDraftTournamentPairingcome from separate layers (~/apivs component), consider grouping external/domain imports first and local component imports after for readability.
24-29:unassignedPairingsis recomputed on every renderIf
manualPairingsis large, this conversion could be memoised:-const unassignedPairings = convertDraftPairingsToUnassignedPairings(manualPairings); +const unassignedPairings = useMemo( + () => convertDraftPairingsToUnassignedPairings(manualPairings), + [manualPairings], +);Not critical but avoids needless work during typing-heavy pairing steps.
src/pages/TournamentsPage/TournamentsPage.module.scss (1)
14-14: Great consolidation, but drop the now-unusedshadowsimport
variants.card($elevated: true)bundles the elevation styles, so the direct@use "/src/style/shadows";at the top of the file is no longer referenced.
Trimming unused imports keeps the build faster and the intent clear.-@use "/src/style/shadows";src/components/generic/InputDateTime/InputDateTime.module.scss (1)
22-27: Unusedshadowsimport & redundant overrides
variants.card($elevated: true)already adds the elevation; remove the@use "/src/style/shadows";import (line 4) for cleanliness.- The explicit
background-color&border-radiusblock duplicates what the mixin supplies—unless you’re intentionally overriding values, it’s safe to drop.-@use "/src/style/shadows"; ... - & { - margin: 0.25rem 0; - background-color: var(--card-bg); - border-radius: variables.$corner-radius; - } + margin: 0.25rem 0;src/components/generic/InputLocation/InputLocation.module.scss (1)
8-12: Same clean-up opportunity as other card popovers
variants.card($elevated: true)supersedes the explicit elevation and makes theshadowsimport (line 4) obsolete.
Consider dropping the unused import and redundantbackground-color/border-radiusdeclarations for parity with the rest of the refactor.src/components/TournamentCompetitorEditDialog/TournamentCompetitorEditDialog.tsx (1)
30-35: Remove duplicate player filtering logic.The
TournamentCompetitorFormcomponent already filters out players withoutuserIdin its ownhandleSubmitmethod (as shown in the relevant code snippet). This additional filtering in the dialog creates redundant logic.Consider removing the duplicate filtering here since the form component already handles it:
- const { players, ...restData } = formData; - updateTournamentCompetitor({ - id: data?.tournamentCompetitor._id, - ...restData, - players: players.filter((player) => player.userId), - }); + updateTournamentCompetitor({ + id: data?.tournamentCompetitor._id, + ...formData, + });convex/_model/users/_helpers/checkUserTournamentForcedName.ts (1)
14-14: Consider performance optimization.Querying all tournaments could be inefficient for large datasets. Consider filtering tournaments that contain either user ID before checking relationships.
src/components/TournamentCompetitorForm/TournamentCompetitorForm.tsx (1)
102-108: Consider preventing default only for button elements.The
handleAddPlayerfunction prevents default on all mouse events. Since this handler is attached to a button, you might want to be more specific about when to prevent default behavior.-const handleAddPlayer = (e: MouseEvent): void => { - e.preventDefault(); +const handleAddPlayer = (e: MouseEvent<HTMLButtonElement>): void => { + e.preventDefault();src/components/TournamentPairingsGrid/TournamentPairingsGrid.utils.ts (1)
24-45: Good explicit handling of all pairing scenarios, but consider performance optimization.The expanded logic correctly handles all four possible pairing states. However, the spread syntax in the reducer creates O(n²) time complexity.
Consider this more performant approach:
return draftPairings.reduce((acc, pairing, i) => { if (pairing[0] && !pairing[1]) { - return { - ...acc, - [pairing[0].id]: 'unpaired', - }; + acc[pairing[0].id] = 'unpaired'; + return acc; } if (!pairing[0] && pairing[1]) { - return { - ...acc, - [pairing[1].id]: 'unpaired', - }; + acc[pairing[1].id] = 'unpaired'; + return acc; } if (pairing[0] && pairing[1]) { - return { - ...acc, - [pairing[0].id]: `${i}_0`, - [pairing[1].id]: `${i}_1`, - }; + acc[pairing[0].id] = `${i}_0`; + acc[pairing[1].id] = `${i}_1`; + return acc; } return acc; }, {} as Record<TournamentCompetitorId, string>);src/components/TournamentPairingsGrid/TournamentPairingsGrid.tsx (3)
115-132: Consider optimizing the state update logic for better performance.The current implementation has O(n²) time complexity due to spread syntax in the reducer. While tournament sizes are typically small, this can be optimized.
-setGridState(Object.entries(gridState).map(([competitorId, slotId]) => { - // If this ID is the active one, we're dragging it. Set it's slotID to 'over': - if (competitorId === active.id) { - return [competitorId, over.id]; - } - // If this slot is the target, move its competitor to 'unpaired': - if (slotId === over.id) { - return [competitorId, 'unpaired']; - } - // Otherwise do nothing: - return [competitorId, slotId]; -}).reduce((acc, [pairingCompetitorId, slotId]) => ({ - ...acc, - [pairingCompetitorId as TournamentCompetitorId]: slotId, -}), {})); +const newState: PairingsGridState = {}; +Object.entries(gridState).forEach(([competitorId, slotId]) => { + if (competitorId === active.id) { + newState[competitorId as TournamentCompetitorId] = over.id as string; + } else if (slotId === over.id) { + newState[competitorId as TournamentCompetitorId] = 'unpaired'; + } else { + newState[competitorId as TournamentCompetitorId] = slotId; + } +}); +setGridState(newState);
137-140: Optimize the pivot operation for better performance.The pivot operation also suffers from O(n²) complexity due to spread syntax.
-const gridStatePivoted = Object.entries(gridState ?? {}).reduce((acc, [competitorId, slotId]) => ({ - ...acc, - [slotId]: competitors.find((c) => c.id === competitorId), -}), {} as Record<string, TournamentCompetitorRanked | undefined>); +const gridStatePivoted: Record<string, TournamentCompetitorRanked | undefined> = {}; +Object.entries(gridState ?? {}).forEach(([competitorId, slotId]) => { + gridStatePivoted[slotId] = competitors.find((c) => c.id === competitorId); +});
51-197: Excellent architectural refactor for state encapsulation.The refactor from a controlled to an uncontrolled component with imperative methods is well-executed. This pattern appropriately encapsulates the complex drag-and-drop state management within the grid component while maintaining clean communication with the parent through the
onChangecallback and exposed handle methods.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (42)
convex/_model/tournamentCompetitors/queries/getTournamentCompetitors.ts(1 hunks)convex/_model/tournamentCompetitors/queries/getTournamentCompetitorsByTournament.ts(1 hunks)convex/_model/tournaments/_helpers/deepenTournament.ts(2 hunks)convex/_model/users/_helpers/checkUserTournamentForcedName.ts(1 hunks)convex/_model/users/_helpers/redactUser.ts(2 hunks)src/components/AccountMenu/AccountMenu.module.scss(1 hunks)src/components/AvatarEditable/AvatarEditable.module.scss(1 hunks)src/components/FowV4MatchResultForm/FowV4MatchResultForm.tsx(3 hunks)src/components/MatchResultCard/MatchResultCard.module.scss(0 hunks)src/components/MatchResultPlayers/MatchResultPlayers.module.scss(1 hunks)src/components/ToastProvider/ToastProvider.module.scss(1 hunks)src/components/TournamentCard/TournamentCard.module.scss(0 hunks)src/components/TournamentCompetitorEditDialog/TournamentCompetitorEditDialog.tsx(2 hunks)src/components/TournamentCompetitorForm/TournamentCompetitorForm.module.scss(1 hunks)src/components/TournamentCompetitorForm/TournamentCompetitorForm.schema.ts(2 hunks)src/components/TournamentCompetitorForm/TournamentCompetitorForm.tsx(6 hunks)src/components/TournamentInfoBlock/TournamentInfoBlock.tsx(1 hunks)src/components/TournamentPairingsGrid/PairingsGridRow/PairingsGridRow.tsx(1 hunks)src/components/TournamentPairingsGrid/PairingsGridRow/PairingsGridRow.utils.ts(1 hunks)src/components/TournamentPairingsGrid/TournamentPairingsGrid.tsx(6 hunks)src/components/TournamentPairingsGrid/TournamentPairingsGrid.types.ts(1 hunks)src/components/TournamentPairingsGrid/TournamentPairingsGrid.utils.ts(2 hunks)src/components/TournamentPairingsGrid/index.ts(1 hunks)src/components/TournamentRoster/TournamentRoster.module.scss(1 hunks)src/components/TournamentRoster/TournamentRoster.tsx(2 hunks)src/components/TournamentRoster/components/CompetitorActions/CompetitorActions.module.scss(1 hunks)src/components/TournamentRoster/components/CompetitorActions/CompetitorActions.tsx(3 hunks)src/components/TournamentRoster/components/PlayerCount/PlayerCount.module.scss(1 hunks)src/components/TournamentRoster/components/PlayerCount/PlayerCount.tsx(1 hunks)src/components/TournamentRoster/components/PlayerCount/index.ts(1 hunks)src/components/generic/Accordion/AccordionItem.module.scss(1 hunks)src/components/generic/Accordion/AccordionItem.tsx(1 hunks)src/components/generic/Card/Card.module.scss(0 hunks)src/components/generic/DataTable/DataTable.module.scss(1 hunks)src/components/generic/InputDateTime/InputDateTime.module.scss(1 hunks)src/components/generic/InputLocation/InputLocation.module.scss(1 hunks)src/components/generic/PopoverMenu/PopoverMenu.module.scss(1 hunks)src/pages/TournamentAdvanceRoundPage/components/ConfirmPairingsDialog/ConfirmPairingsDialog.tsx(1 hunks)src/pages/TournamentAdvanceRoundPage/components/PairingsStep/PairingsStep.tsx(6 hunks)src/pages/TournamentAdvanceRoundPage/components/PairingsStep/PairingsStep.utils.ts(1 hunks)src/pages/TournamentsPage/TournamentsPage.module.scss(1 hunks)src/style/_variants.scss(2 hunks)
💤 Files with no reviewable changes (3)
- src/components/TournamentCard/TournamentCard.module.scss
- src/components/MatchResultCard/MatchResultCard.module.scss
- src/components/generic/Card/Card.module.scss
🧰 Additional context used
🧬 Code Graph Analysis (7)
src/components/TournamentCompetitorEditDialog/TournamentCompetitorEditDialog.tsx (1)
src/components/TournamentCompetitorForm/TournamentCompetitorForm.tsx (1)
TournamentCompetitorForm(49-192)
convex/_model/users/_helpers/redactUser.ts (1)
convex/_model/users/_helpers/checkUserTournamentForcedName.ts (1)
checkUserTournamentForcedName(5-30)
src/components/TournamentPairingsGrid/TournamentPairingsGrid.types.ts (1)
src/components/TournamentPairingsGrid/index.ts (1)
DraftTournamentPairing(6-6)
src/components/TournamentRoster/TournamentRoster.tsx (1)
src/components/TournamentRoster/components/PlayerCount/PlayerCount.tsx (1)
PlayerCount(14-26)
convex/_model/users/_helpers/checkUserTournamentForcedName.ts (2)
convex/_generated/server.d.ts (1)
QueryCtx(106-106)convex/_generated/dataModel.d.ts (1)
Id(48-49)
src/pages/TournamentAdvanceRoundPage/components/PairingsStep/PairingsStep.utils.ts (1)
src/components/TournamentPairingsGrid/TournamentPairingsGrid.types.ts (1)
DraftTournamentPairing(5-5)
src/components/TournamentPairingsGrid/TournamentPairingsGrid.utils.ts (2)
src/components/TournamentPairingsGrid/TournamentPairingsGrid.types.ts (2)
PairingsGridState(3-3)DraftTournamentPairing(5-5)src/components/TournamentPairingsGrid/index.ts (1)
DraftTournamentPairing(6-6)
🪛 Biome (1.9.4)
src/components/TournamentPairingsGrid/TournamentPairingsGrid.utils.ts
[error] 27-27: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 33-33: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 39-39: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
src/components/TournamentPairingsGrid/TournamentPairingsGrid.tsx
[error] 130-130: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
[error] 138-138: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🔇 Additional comments (44)
src/components/TournamentCompetitorForm/TournamentCompetitorForm.module.scss (1)
15-17: LGTM! Clean grid styling implementation.The
grid-column: 1/-1;property correctly spans the button across all columns in the grid layout, which is appropriate for an "Add Player Slot" button that should take full width.src/components/generic/Accordion/AccordionItem.module.scss (2)
18-23: Excellent conditional styling implementation.Using the
data-enabled="true"attribute selector to conditionally apply interactive styles (variants.ghostandcursor: pointer) is a clean approach that improves accessibility and user experience by providing clear visual feedback only when the accordion item is interactive.
30-30: Good separation of layout and interactive styles.Moving the flex row layout to always apply regardless of enabled state is the right choice, as layout should remain consistent while only interactive behaviors change based on the disabled state.
src/components/generic/Accordion/AccordionItem.tsx (2)
44-44: Good coordination with conditional styling.The
data-enabled={!disabled}attribute properly coordinates with the SCSS conditional styling, enabling clean separation between interactive and non-interactive states.
47-55: Logical conditional rendering of interactive elements.Conditionally rendering the chevron icon only when the accordion item is not disabled makes perfect sense from a UX perspective - non-interactive items shouldn't display interactive visual cues.
src/components/TournamentRoster/components/PlayerCount/PlayerCount.module.scss (1)
1-12: Clean and consistent styling implementation.The PlayerCount styling follows established project patterns with appropriate use of flex layout, muted UI text for supplementary information, and consistent SVG sizing. The implementation is clean and maintainable.
src/components/TournamentPairingsGrid/PairingsGridRow/PairingsGridRow.utils.ts (1)
1-1: Good type organization refactoring.Moving the
DraftTournamentPairingtype import from global API types to a local component-specific types file improves encapsulation and type organization. Using relative imports for local types is the appropriate approach.src/components/MatchResultPlayers/MatchResultPlayers.module.scss (1)
111-118: Possible duplicate shadow / elevation styles
variants.card($elevated: true)already applies an elevated shadow in the new mixin implementation; verify the parent containers don’t also add additional shadows, otherwise cards may look doubly-elevated compared to neighbouring components.src/components/TournamentRoster/components/CompetitorActions/CompetitorActions.module.scss (1)
12-13: Flex item may still growYou replaced
margin-left: autowithflex-shrink: 0.
If the intent was “stick to the right but never shrink”, also ensureflex-growstays0(default) and parent layout still pushes the element rightward now that automatic margin is gone, especially after moving the header to CSS-grid.src/components/TournamentRoster/components/PlayerCount/index.ts (1)
1-2: Barrel export LGTMSimple re-export is clear and matches the surrounding pattern.
src/components/TournamentInfoBlock/TournamentInfoBlock.tsx (1)
56-56: Approve use of activePlayerCount in TournamentInfoBlockVerified that
activePlayerCountis populated (in convex/_model/tournaments/_helpers/deepenTournament.ts) and that there are no remainingplayerCountreferences in tournament-display contexts. This change is safe and accurately reflects live participation.• Location: src/components/TournamentInfoBlock/TournamentInfoBlock.tsx:56
• No other components break or need updates—other “PlayerCount” uses are scoped to roster items and derive their counts locally.src/components/generic/PopoverMenu/PopoverMenu.module.scss (1)
13-13:variants.cardmixin supports$elevatedparameter – Approved!The
@mixin card($elevated: false)definition insrc/style/_variants.scsscorrectly includesshadows.elevatedwhen$elevated: true, so the refactoring insrc/components/generic/PopoverMenu/PopoverMenu.module.scssis valid and maintains the intended elevated appearance.• src/style/_variants.scss:
@mixin card($elevated: false) { @include borders.normal; @include corners.normal; @include shadows.surface; background-color: var(--card-bg); @if $elevated { @include shadows.elevated; } }• src/components/generic/PopoverMenu/PopoverMenu.module.scss, line 13:
@include variants.card($elevated: true);src/components/AccountMenu/AccountMenu.module.scss (1)
19-19: LGTM! Consistent with styling pattern consolidation.This change follows the same beneficial pattern seen across other components, consolidating multiple mixin calls into a single
variants.card($elevated: true)call. This maintains consistency with the broader styling refactoring effort.src/components/generic/DataTable/DataTable.module.scss (1)
31-31: LGTM! Maintains consistency with styling refactoring pattern.The consolidation to
variants.card($elevated: true)is consistent with the systematic styling improvements across the codebase, enhancing maintainability while preserving visual appearance.src/components/AvatarEditable/AvatarEditable.module.scss (1)
28-28: LGTM! Completes the systematic styling consolidation.This final change completes the consistent application of the
variants.card($elevated: true)pattern across all reviewed components. The systematic approach to this styling refactoring demonstrates good architectural discipline and will improve long-term maintainability.src/components/TournamentPairingsGrid/TournamentPairingsGrid.types.ts (1)
1-5: LGTM! Well-defined type for draft tournament pairings.The
DraftTournamentPairingtype appropriately models incomplete pairings with nullable competitors, which is essential for draft tournament management functionality.convex/_model/users/_helpers/redactUser.ts (2)
6-6: Good addition of tournament-specific name visibility logic.The import of the helper function enables checking for tournament-specific real name requirements.
55-65: LGTM! Proper implementation of tournament-forced name visibility.The logic correctly:
- Awaits the async tournament name requirement check
- Extends the existing name visibility conditions without breaking them
- Respects tournament-specific rules for showing real names
src/style/_variants.scss (2)
3-4: Good import additions for styling consolidation.Adding corners and shadows modules enables the enhanced card mixin functionality.
195-205: Excellent consolidation of card styling logic.The enhanced card mixin:
- Maintains backward compatibility with default
$elevated: false- Consolidates borders, corners, shadows, and background styling
- Properly applies conditional elevated shadows
- Simplifies usage across components
src/components/TournamentRoster/TournamentRoster.tsx (2)
7-7: Good addition of PlayerCount component for team tournaments.The import enables displaying active player counts in team-based tournaments.
18-31: Excellent UI enhancements for tournament roster.The changes properly:
- Extract
competitorSizefrom the tournament context- Conditionally render
PlayerCountonly for team tournaments- Apply appropriate CSS classes for grid layout styling
- Maintain existing behavior for non-team tournaments
The conditional rendering and prop passing align well with the
PlayerCountcomponent interface.src/components/TournamentCompetitorEditDialog/TournamentCompetitorEditDialog.tsx (1)
41-49: Good addition of ScrollArea for enhanced UX.Wrapping the form in a
ScrollAreais a good improvement that will handle potentially longer forms due to dynamic player slot additions, ensuring the dialog remains usable with larger team sizes.src/components/TournamentRoster/TournamentRoster.module.scss (1)
10-30: Excellent migration to CSS Grid layout.The transition from flexbox to CSS Grid with named areas provides better layout control and maintainability. The semantic naming ("Identity", "PlayerCount", "Actions") makes the layout structure clear and self-documenting.
src/components/FowV4MatchResultForm/FowV4MatchResultForm.tsx (3)
8-8: Good addition of UserId import.The
UserIdtype import is appropriately used for type casting in the form value setting logic.
126-126: Excellent filtering logic for completed pairings.Filtering out pairings that have already reached their required number of match results prevents users from submitting duplicate results and improves the UX by showing only relevant options.
135-138: Smart auto-population of user data.The logic to automatically set the current user as
player0UserIdand resetplayer1UserIdwhen selecting "single" match provides a good user experience by reducing manual input.src/components/TournamentPairingsGrid/index.ts (1)
1-6: Excellent expansion of the module's public API.Exporting the component types (
TournamentPairingsGridHandle,TournamentPairingsGridProps) and theDraftTournamentPairingtype improves TypeScript integration for consumers of this module and follows best practices for library design.src/components/TournamentCompetitorForm/TournamentCompetitorForm.schema.ts (2)
20-33: Clean validation logic simplification.The refactored validation logic maintains the same functionality while being more readable and concise. The merged conditions for active player count validation are well-structured.
50-56: Good refactoring of getDefaultValues function.The simplified implementation is more concise and removes unnecessary complexity. The removal of the default array fallback aligns with the dynamic player slot management approach mentioned in the summary.
src/components/TournamentRoster/components/PlayerCount/PlayerCount.tsx (1)
1-26: Well-implemented PlayerCount component.This is a clean, focused component with clear responsibilities:
- Properly filters active players for accurate count display
- Uses semantic data attributes for styling hooks
- Well-typed with clear props interface
- Follows React best practices
The implementation correctly handles the core functionality of displaying active player counts relative to competitor size.
src/components/TournamentRoster/components/CompetitorActions/CompetitorActions.tsx (2)
24-24: Good addition of className prop for styling flexibility.The optional className prop enhances component reusability by allowing external styling control. The implementation using clsx is appropriate.
Also applies to: 29-29, 100-100
82-82: Clean simplification of visibility logic.The refactored conditions are more concise and maintain the same functionality:
- The Leave condition is more explicit about excluded statuses
- Removed redundant user checks since user existence is already validated in variable definitions
Also applies to: 87-87, 92-92
src/pages/TournamentAdvanceRoundPage/components/PairingsStep/PairingsStep.utils.ts (2)
2-2: Import source change aligns with type reorganization.The import change from
~/apito~/components/TournamentPairingsGridaligns with the type definition being moved to the grid component as mentioned in the summary.
6-33: Excellent refactoring with robust edge case handling.The rewritten function is much more explicit and handles edge cases effectively:
- Correctly filters out pairings with no competitors
- Explicitly handles partial pairings (one competitor null)
- Uses Set to merge
playedTablesarrays, avoiding duplicates- Safe use of non-null assertion after proper null checks
The logic is clear and the implementation is robust.
src/components/TournamentCompetitorForm/TournamentCompetitorForm.tsx (3)
113-113: Good practice filtering out empty player slots.The logic correctly filters out players without user IDs before submission, preventing empty slots from being sent to the backend.
146-152: Excellent defensive programming with null-safe access.The null-safe access patterns for
players[i]?.activeandplayers[i]?.userIdprevent potential runtime errors when dealing with dynamic player arrays.
160-168: Well-implemented dynamic player slot management.The "Add Player Slot" button implementation includes appropriate disabled states and clear visual indicators. The logic to disable the button when empty slots exist prevents user confusion.
src/pages/TournamentAdvanceRoundPage/components/PairingsStep/PairingsStep.tsx (3)
64-65: Good use of imperative handle pattern for state delegation.The ref-based approach cleanly delegates state management to the child component. The null-safe access on
isDirtyappropriately handles the case where the ref might not be initialized yet.
85-85: Clean delegation of reset logic to the grid component.The reset handler correctly delegates to the grid's imperative reset method, maintaining proper separation of concerns.
114-114: Appropriate use of uncontrolled component pattern.Using
defaultValueinstead of a controlledvalueprop is the right choice here, as it allows the grid to manage its own complex internal state while still notifying the parent of changes.src/components/TournamentPairingsGrid/TournamentPairingsGrid.utils.ts (1)
48-51: Improved type safety and defensive null checks.The updated function signature with
PairingsGridState | nullprovides better type safety, and the comprehensive null/empty checks prevent potential runtime errors.src/components/TournamentPairingsGrid/TournamentPairingsGrid.tsx (2)
91-95: Well-designed imperative API exposure.The
useImperativeHandleimplementation provides a clean, minimal interface for parent components to interact with the grid's internal state. The exposedresetmethod andisDirtyflag are exactly what's needed for the use case.
76-80: Correct uncontrolled component state initialization.The state initialization logic properly implements the uncontrolled component pattern by only setting the internal state once from
defaultValue.
convex/_model/tournamentCompetitors/queries/getTournamentCompetitorsByTournament.ts
Show resolved
Hide resolved
…titorsByTournament.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/pages/AuthPage/components/SignInForm/SignInForm.schema.ts (1)
4-4: Improve error message specificity for email validation.The change from
min(1)toemail()is excellent as it provides proper email format validation. However, consider updating the error message to be more specific about email format requirements.- email: z.string().email('Please enter your email.').transform((val) => val.trim().toLowerCase()), + email: z.string().email('Please enter a valid email address.').transform((val) => val.trim().toLowerCase()),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
convex/_model/fowV4/aggregateFowV4TournamentData.ts(1 hunks)convex/_model/fowV4/flattenFowV4StatMap.ts(2 hunks)convex/_model/tournamentCompetitors/queries/getTournamentCompetitorsByTournament.ts(1 hunks)convex/_model/users/_helpers/checkUserTournamentForcedName.ts(1 hunks)convex/_model/users/_helpers/redactUser.ts(2 hunks)src/pages/AuthPage/components/SignInForm/SignInForm.schema.ts(1 hunks)src/services/auth/useSignIn.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- convex/_model/users/_helpers/checkUserTournamentForcedName.ts
- convex/_model/users/_helpers/redactUser.ts
- convex/_model/tournamentCompetitors/queries/getTournamentCompetitorsByTournament.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
convex/_model/fowV4/aggregateFowV4TournamentData.ts (1)
convex/_model/fowV4/flattenFowV4StatMap.ts (1)
flattenFowV4StatMap(14-33)
src/services/auth/useSignIn.ts (1)
src/components/ToastProvider/ToastProvider.store.ts (1)
toast(29-50)
🔇 Additional comments (5)
convex/_model/fowV4/flattenFowV4StatMap.ts (2)
16-16: Function signature change is a breaking change but looks correct.The addition of
gamesPlayed: numberto the return type is a breaking change for any consumers of this function, but it's necessary for the filtering functionality implemented in the aggregation function.
30-30: Good defensive programming with nullish coalescing.The use of
stats.gamesPlayed ?? 0properly handles cases where thegamesPlayedproperty might be undefined, defaulting to a sensible value of 0.convex/_model/fowV4/aggregateFowV4TournamentData.ts (2)
134-137: Filtering logic for players is well-implemented.The filter correctly excludes players with zero games played using the
gamesPlayedproperty that was added to theflattenFowV4StatMapoutput. The boolean coercion approach is appropriate since 0 is falsy.
138-142: Consistent filtering logic applied to competitors.The same filtering approach is consistently applied to competitors, which maintains logical consistency in the data aggregation. This ensures that both players and competitors are filtered using the same criteria.
src/services/auth/useSignIn.ts (1)
63-65: LGTM! Clean error handling implementation.The usage of the error mapping helper is implemented correctly, providing a good user experience with clear error messages.
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| function mapConvexAuthError(error: any): string { | ||
| if (!error?.message) { | ||
| return 'An unexpected error occurred. Please try again.'; | ||
| } | ||
|
|
||
| if (error.message.includes('InvalidAccountId')) { | ||
| return 'Your email or password is incorrect.'; | ||
| } | ||
|
|
||
| return 'Sign-in failed. Please check your details and try again.'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and error checking robustness.
The error mapping approach is good for providing user-friendly messages, but consider these improvements:
- Type Safety: Replace
anywith a more specific type - Robust Error Checking: Use more reliable error identification methods
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-function mapConvexAuthError(error: any): string {
- if (!error?.message) {
- return 'An unexpected error occurred. Please try again.';
- }
-
- if (error.message.includes('InvalidAccountId')) {
- return 'Your email or password is incorrect.';
- }
-
- return 'Sign-in failed. Please check your details and try again.';
-}
+interface ConvexAuthError {
+ message?: string;
+ code?: string;
+}
+
+function mapConvexAuthError(error: unknown): string {
+ const authError = error as ConvexAuthError;
+
+ if (!authError?.message) {
+ return 'An unexpected error occurred. Please try again.';
+ }
+
+ // Use more specific error code checking if available
+ if (authError.code === 'InvalidAccountId' || authError.message.includes('InvalidAccountId')) {
+ return 'Your email or password is incorrect.';
+ }
+
+ return 'Sign-in failed. Please check your details and try again.';
+}🤖 Prompt for AI Agents
In src/services/auth/useSignIn.ts between lines 17 and 28, replace the use of
'any' for the error parameter with a more specific type such as 'unknown' or a
custom error interface to improve type safety. Then, enhance error checking by
verifying the error object structure more robustly, for example by using type
guards or checking if the error is an instance of Error before accessing the
message property. This will prevent runtime errors and make the error handling
more reliable.
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
1 participant
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Style
Refactor
Documentation