feature/migrate to Tailwind (shadcn.ui) and Zod #1629
Conversation
afternoon2
added
improvement
javascript
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the UI from React-Bootstrap and Yup to Tailwind CSS with shadcn/ui components and Zod for form validation. The migration modernizes the styling approach and introduces a more type-safe form validation system.
- Complete replacement of React-Bootstrap components with Tailwind CSS-based shadcn/ui components
- Migration from Yup to Zod schema validation for all forms
- Consolidation of path aliases to use a single
@/*pattern for better maintainability
Reviewed Changes
Copilot reviewed 79 out of 80 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Adds Tailwind CSS plugin and simplifies path aliases to use @/* pattern |
| tsconfig.json/tsconfig.app.json | Updates path aliases configuration for the new alias pattern |
| src/index.css | Replaces Bootstrap CSS with Tailwind and shadcn/ui theme variables |
| src/pages/**/index.tsx | Migrates all page components from React-Bootstrap/Yup to shadcn/ui/Zod |
| src/components/ui/* | Adds new shadcn/ui component library |
| src/components/AppNavbar/* | Creates new responsive navigation using shadcn/ui components |
| src/App.tsx | Restructures routing and adds new layout components |
| package.json | Updates dependencies to include Tailwind, shadcn/ui, and Zod |
| when(payload) { | ||
| return schema | ||
| .pick({ password: true, repeatedPassword: true }) | ||
| .safeParse(payload.value).success; | ||
| }, |
There was a problem hiding this comment.
The when method in Zod should be used within a schema definition, not directly on the schema object. This syntax appears incorrect and may cause runtime errors.
| when(payload) { | |
| return schema | |
| .pick({ password: true, repeatedPassword: true }) | |
| .safeParse(payload.value).success; | |
| }, |
Copilot uses AI. Check for mistakes.
| .object({ | ||
| login: z.string('Login is required').min(1), | ||
| email: z.email('Email is required'), | ||
| password: z.string('Password is required').min(8), |
There was a problem hiding this comment.
The password minimum length validation (8) is inconsistent with the description that states 'Password must be at least 8 characters long' but the old Yup validation required only 5 characters. Consider updating the description or validation to be consistent.
Copilot uses AI. Check for mistakes.
| import { ErrorMessage } from '@/components/ErrorMessage'; | ||
|
|
||
| const schema = z.object({ | ||
| loginOrEmail: z.email('Login or email is required').or(z.string().min(1)), |
There was a problem hiding this comment.
The schema validation logic is incorrect. The .or() method should come before the error message. The correct syntax should be z.string().email().or(z.string().min(1)) or use z.union().
| loginOrEmail: z.email('Login or email is required').or(z.string().min(1)), | |
| loginOrEmail: z.string().email().or(z.string().min(1, 'Login or email is required')), |
Copilot uses AI. Check for mistakes.
| import { NavLink } from 'react-router'; | ||
|
|
||
| const schema = z.object({ | ||
| loginOrEmail: z.email().or(z.string().min(1)), |
There was a problem hiding this comment.
The validation logic is incorrect. z.email() will always fail first before trying the .or() alternative. Use z.union([z.string().email(), z.string().min(1)]) or z.string().min(1) with custom validation.
| loginOrEmail: z.email().or(z.string().min(1)), | |
| loginOrEmail: z.union([z.string().email(), z.string().min(1)]), |
Copilot uses AI. Check for mistakes.
| </p> | ||
| </div> | ||
| <div className="w-full h-[50%] flex flex-col items-center justify-center text-center"> | ||
| <p className="fs-3">Brought to you by</p> |
There was a problem hiding this comment.
The fs-3 class appears to be a Bootstrap class that should be replaced with Tailwind CSS classes like text-3xl to maintain consistency with the migration.
| <p className="fs-3">Brought to you by</p> | |
| <p className="text-2xl">Brought to you by</p> |
Copilot uses AI. Check for mistakes.
| console.log( | ||
| 'Mock setup correctly:', | ||
| vi.mocked(apiComponents.usePostUserRegister).mock.results[0]?.value | ||
| ); | ||
|
|
There was a problem hiding this comment.
Console.log statements should be removed from test files as they can clutter test output and serve no functional purpose in production code.
| console.log( | |
| 'Mock setup correctly:', | |
| vi.mocked(apiComponents.usePostUserRegister).mock.results[0]?.value | |
| ); |
Copilot uses AI. Check for mistakes.
|
@grubach maybe you could take a look, not necessarily at the code in detail, but overall on the library choices - is that something that would be your first pick when writing a simple "CRUD" app? |
The following PR closes #1550, #1548
Here, we're migrating from React-Bootstrap & Yup to Tailwind-based UI that uses shadcn.ui component set and Zod library with React Hook Form for the form schema validations.
Sneak peek:
Feel free to run this PR locally to check if things work correctly, comment, and review.