feat: implement onboarding flow & create league#198
Conversation
… forms to support sport selection
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the initial user experience by introducing a dynamic onboarding flow for new users, guiding them through the creation of their first team or league. It also brings consistency to sport selection across various forms and improves client-side validation, making the application more robust and user-friendly. Backend data handling for league migration and various data access patterns have been optimized for efficiency and data integrity. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great step forward in improving the user onboarding experience and strengthening data integrity. The introduction of the OnboardingFlow component provides a much cleaner and more guided process for new users. The transition of the sport field to a database-level enum, along with the corresponding updates to validation schemas and forms, is a significant improvement for consistency and correctness. The performance optimizations in the backend actions using select instead of include are also excellent and show great attention to detail.
I've left a few comments with suggestions for improving code clarity, reusability, and accessibility. These are mostly minor points in what is overall a very solid set of changes.
| {orderedSports.map((sport, index) => [ | ||
| index === FEATURED_SPORTS.length && ( | ||
| <MenuItem key="divider" disabled divider sx={{ opacity: 0 }} /> | ||
| ), | ||
| <MenuItem key={sport} value={sport}> | ||
| {SPORT_LABELS[sport]} | ||
| </MenuItem>, | ||
| ])} |
There was a problem hiding this comment.
The logic for rendering the sports dropdown with a divider is a bit complex inside the JSX. Returning an array [element, element] from .map() can be hard to read. A more declarative approach would be to render the featured sports and other sports in separate blocks with a proper <Divider /> component in between. This is more readable and semantic than using a disabled MenuItem with opacity: 0 as a separator. You would need to import Divider from @mui/material.
Additionally, this dropdown logic is now duplicated in CreateTeamForm and CreateLeagueTeamForm. Consider extracting it into a reusable SportSelectOptions component to improve maintainability.
{FEATURED_SPORTS.map((sport) => (
<MenuItem key={sport} value={sport}>
{SPORT_LABELS[sport]}
</MenuItem>
))}
<Divider />
{SPORTS.filter((s) => !FEATURED_SPORTS.includes(s)).map((sport) => (
<MenuItem key={sport} value={sport}>
{SPORT_LABELS[sport]}
</MenuItem>
))}
| disabled={ | ||
| isSubmitting || | ||
| !formData.name || | ||
| !formData.sport || | ||
| !formData.contactEmail | ||
| } |
There was a problem hiding this comment.
The submit button's disabled logic doesn't account for existing validation errors. After a failed submission, fieldErrors will be populated, but the button could become enabled again if the user makes a trivial change to a field. To prevent submission of a form with known errors, you should also disable the button if fieldErrors contains any errors.
disabled={
isSubmitting ||
!formData.name ||
!formData.sport ||
!formData.contactEmail ||
Object.values(fieldErrors).some((error) => !!error)
}
| <Typography | ||
| component="button" | ||
| onClick={onBack} | ||
| sx={{ | ||
| background: "none", | ||
| border: "none", | ||
| cursor: "pointer", | ||
| color: "text.secondary", | ||
| fontSize: "0.875rem", | ||
| mb: 3, | ||
| display: "flex", | ||
| alignItems: "center", | ||
| gap: 0.5, | ||
| p: 0, | ||
| "&:hover": { color: "text.primary" }, | ||
| }} | ||
| > | ||
| ← Back | ||
| </Typography> |
There was a problem hiding this comment.
For better accessibility and semantic correctness, it's recommended to use the MUI <Button> component instead of <Typography component="button">. The <Button> component provides proper keyboard navigation, focus handling, and screen reader support out of the box. You can use variant="text" to achieve a similar visual appearance. You will need to import Button from @mui/material.
<Button
onClick={onBack}
sx={{
background: "none",
border: "none",
color: "text.secondary",
fontSize: "0.875rem",
mb: 3,
display: "flex",
alignItems: "center",
gap: 0.5,
p: 0,
textTransform: "none",
"&:hover": { color: "text.primary", backgroundColor: "transparent" },
}}
>
← Back
</Button>
|
|
||
| // Validate individual field on blur | ||
| if (name === 'name' || name === 'sport' || name === 'season') { | ||
| if (name === "name" || name === "sport" || name === "season") { |
There was a problem hiding this comment.
Since the onBlur handler is no longer attached to the 'sport' field (which is now a select dropdown), the check for name === 'sport' inside handleBlur is redundant and can be removed for clarity.
| if (name === "name" || name === "sport" || name === "season") { | |
| if (name === "name" || name === "season") { |
| name: sanitizedStringWithMin(1, 100).refine(val => val.length > 0, "Team name is required"), | ||
| sport: sanitizedStringWithMin(1, 50).refine(val => val.length > 0, "Sport is required"), | ||
| sport: sportSchema, | ||
| season: sanitizedStringWithMin(1, 50).refine(val => val.length > 0, "Season is required"), |
There was a problem hiding this comment.
The .refine() calls here are redundant for validation because sanitizedStringWithMin(1, ...) already enforces a minimum length. To provide a custom error message for the minimum length check, it's more idiomatic to chain .min() with a message after getting the base sanitized string schema using the sanitizedString helper. This avoids the redundant check and makes the intent clearer.
| name: sanitizedStringWithMin(1, 100).refine(val => val.length > 0, "Team name is required"), | |
| sport: sanitizedStringWithMin(1, 50).refine(val => val.length > 0, "Sport is required"), | |
| sport: sportSchema, | |
| season: sanitizedStringWithMin(1, 50).refine(val => val.length > 0, "Season is required"), | |
| name: sanitizedString(100).min(1, "Team name is required"), | |
| sport: sportSchema, | |
| season: sanitizedString(50).min(1, "Season is required"), |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds an interactive onboarding experience for new users (no teams/leagues) and standardizes sport selection/validation across team & league creation flows, while tightening Prisma query selection in several actions for improved performance/safety.
Changes:
- Replaced the dashboard’s empty-state welcome screen with a new
OnboardingFlowthat routes users to team or league creation. - Standardized sport input across forms via a shared Zod enum + label mapping and switched Prisma
sportfields to aSportenum with defaults. - Reduced over-fetching in multiple Prisma queries by switching
includeto targetedselect.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| prisma/schema.prisma | Introduces Sport enum; updates Team.sport and League.sport to use it with defaults. |
| lib/utils/validation.ts | Adds SPORTS/SPORT_LABELS/FEATURED_SPORTS and sportSchema; applies to team/league schemas. |
| lib/utils/league-mode.ts | Narrows membership checks to select: { id: true }. |
| lib/actions/venues.ts | Narrows membership queries to select: { role: true }. |
| lib/actions/team-context.ts | Replaces include with explicit select for team context queries. |
| lib/actions/league.ts | Narrows migrateTeamToLeague/admin verification queries with explicit select. |
| components/ui/Logo.tsx | Uses inherit color outside footer for better theming control. |
| components/features/team/CreateTeamForm.tsx | Converts sport field to a standardized select with featured-first ordering and improved validation UX. |
| components/features/team/CreateLeagueTeamForm.tsx | Aligns sport selection UX with shared sport constants/labels. |
| components/features/onboarding/OnboardingFlow.tsx | New onboarding intent UI (team vs league) with back navigation wrapper. |
| components/features/onboarding/CreateLeagueOnboardingForm.tsx | New league-creation form used during onboarding with client-side validation. |
| components/features/dashboard/DashboardShell.tsx | Tweaks logo styling in desktop sidebar. |
| app/(dashboard)/dashboard/page.tsx | Uses OnboardingFlow when the user has no teams/leagues. |
| {orderedSports.map((sport, index) => [ | ||
| // Divider after featured sports | ||
| index === FEATURED_SPORTS.length && ( | ||
| <MenuItem key="divider" disabled divider sx={{ opacity: 0 }} /> | ||
| ), | ||
| <MenuItem key={sport} value={sport}> | ||
| {SPORT_LABELS[sport]} | ||
| </MenuItem>, | ||
| ])} |
There was a problem hiding this comment.
The “divider” menu item is styled with opacity: 0, which also hides the divider line and creates an invisible option in the dropdown. Prefer using Divider/ListSubheader (supported by MUI Select) or render a visible separator without inserting an invisible MenuItem.
| {orderedSports.map((sport, index) => [ | ||
| index === FEATURED_SPORTS.length && ( | ||
| <MenuItem key="divider" disabled divider sx={{ opacity: 0 }} /> | ||
| ), | ||
| <MenuItem key={sport} value={sport}> | ||
| {SPORT_LABELS[sport]} | ||
| </MenuItem>, | ||
| ])} |
There was a problem hiding this comment.
The “divider” menu item is styled with opacity: 0, which hides the divider and inserts an invisible option into the select menu. Consider using Divider/ListSubheader instead (or a visible separator) to avoid confusing keyboard/screen-reader navigation.
| {([...FEATURED_SPORTS, ...SPORTS.filter(s => !FEATURED_SPORTS.includes(s))] as SportValue[]).map((sport, index) => [ | ||
| index === FEATURED_SPORTS.length && ( | ||
| <MenuItem key="divider" disabled divider sx={{ opacity: 0 }} /> | ||
| ), | ||
| <MenuItem key={sport} value={sport}> | ||
| {SPORT_LABELS[sport]} | ||
| </MenuItem>, | ||
| ])} |
There was a problem hiding this comment.
This dropdown inserts an invisible MenuItem (opacity 0) as a divider, which hides the separator and can create a confusing blank option for assistive tech. Prefer Divider/ListSubheader (or another visible grouping approach) instead.
| // Team model | ||
| model Team { | ||
| id String @id @default(cuid()) | ||
| name String | ||
| sport String | ||
| sport Sport @default(HOCKEY) | ||
| season String |
There was a problem hiding this comment.
Changing Team.sport and League.sport from String to the new Sport enum will require a Prisma migration (including handling/mapping any existing rows whose sport values aren’t one of the enum variants). This PR updates the schema but doesn’t add a corresponding migration, which will break deployments that run prisma migrate deploy against an existing database.
| VOLLEYBALL: "Volleyball", | ||
| OTHER: "Other", | ||
| }; | ||
|
|
There was a problem hiding this comment.
Now that sports are stored as enum values (e.g., HOCKEY), rendering team.sport / league.sport directly will show uppercase values across the UI. Consider exporting a small formatter (e.g., based on SPORT_LABELS) and updating sport displays to use it so user-facing chips/labels remain readable.
| /** | |
| * Format a sport enum value into a user-friendly label. | |
| * | |
| * Use this for all user-facing sport chips/labels instead of rendering | |
| * the raw enum (e.g. "HOCKEY") directly. | |
| */ | |
| export function formatSport(sport: SportValue | null | undefined): string { | |
| if (!sport) { | |
| return "Unknown"; | |
| } | |
| return SPORT_LABELS[sport] ?? "Unknown"; | |
| } |
| boxShadow: "0 0 0 2px", | ||
| boxShadowColor: "primary.light", |
There was a problem hiding this comment.
boxShadowColor isn’t a valid CSS property, so this hover style won’t apply as intended. Set the shadow color by including it directly in the boxShadow value (or use a theme callback to build the shadow string).
| boxShadow: "0 0 0 2px", | |
| boxShadowColor: "primary.light", | |
| boxShadow: (theme) => `0 0 0 2px ${theme.palette.primary.light}`, |
| export const addTeamToLeagueSchema = z.object({ | ||
| leagueId: z.string().cuid("Invalid league ID format"), | ||
| name: sanitizedStringWithMin(1, 100), | ||
| sport: sanitizedStringWithMin(1, 50), | ||
| sport: sportSchema, | ||
| season: sanitizedStringWithMin(1, 50), | ||
| divisionId: z.string().cuid("Invalid division ID format").optional(), | ||
| }); |
There was a problem hiding this comment.
Bug: The getLeagueTeamsSchema uses z.string() for the sport field instead of the enum-based sportSchema, allowing invalid sport values to cause runtime errors in the database query.
Severity: MEDIUM
Suggested Fix
In lib/utils/validation.ts, update the getLeagueTeamsSchema to use the correct enum-based validation for the sport field. Change sport: z.string().optional() to sport: sportSchema.optional() to ensure type safety.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: lib/utils/validation.ts#L314-L320
Potential issue: The `getLeagueTeamsSchema` validation schema incorrectly defines the
`sport` field as `z.string().optional()` instead of the more specific
`sportSchema.optional()`, which is based on the `SPORTS` enum. This allows an API
request with an invalid sport string to pass validation. When this invalid value is used
in the `prisma.team.findMany` query within the `getLeagueTeamsPaginated` function, it
will cause Prisma or the database to throw a runtime error because the value does not
conform to the expected `Sport` enum type. This leads to an unexpected API failure
instead of a proper validation error.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
This pull request introduces a new onboarding flow for users without teams or leagues, streamlining the process of creating a team or league and improving the overall user experience. It also standardizes the way sports are selected across forms and includes several UI and validation enhancements.
Onboarding Experience Improvements:
OnboardingFlowcomponent that guides users through creating either a team or a league, replacing the previous static welcome screen in the dashboard. This provides a more interactive and user-friendly onboarding process. [1] [2] [3]CreateLeagueOnboardingFormfor league creation, with client-side validation and improved error handling.Form and Validation Enhancements:
CreateTeamForm,CreateLeagueTeamForm, andCreateLeagueOnboardingForm:CreateTeamForm, including more precise error clearing and helper text updates. [1] [2] [3]UI and Styling Updates:
Logocomponent to useinheritinstead of alwaysprimary.main, except in the footer.Backend/Data Handling:
migrateTeamToLeagueaction to select only necessary fields when returning the new league admin, improving performance and data safety.