Conversation
- Implemented user registration and login features in UserModel. - Created Login and Register pages with form validation and error handling. - Integrated React Router for navigation between login, register, and main pages. - Updated App component to include routing logic. - Added axios-based apiService for handling API requests to the backend. - Enhanced styling with Satoshi font and improved CSS structure. - Updated package.json and package-lock.json to include necessary dependencies.
❌ Deploy Preview for passkeep failed.
|
There was a problem hiding this comment.
Pull Request Overview
This PR integrates the backend API, adds routing with React Router, and introduces React Query for server state management.
- Adds
apiServicefor register, login, and logout via Axios - Implements
RegisterandLoginpages with form validation - Wraps the app in
BrowserRouterandQueryClientProvider, and defines routes inApp.jsx
Reviewed Changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/apiService.js | New Axios client setup and API methods with error handling |
| src/pages/Register.jsx | Registration form component with validation and navigation |
| src/pages/Login.jsx | Login form component with validation and navigation |
| src/models/UserModel.jsx | User model functions wrapping apiService calls |
| src/main.jsx | Added BrowserRouter and React Query provider |
| src/index.css | Imported global font and updated styling rules |
| src/App.jsx | Defined routes for login, register, and protected content |
| src/.env.example | Example environment variable for API URL |
| package.json | Added axios, react-router-dom, and React Query deps |
| import Manager from "./components/Manager"; | ||
| import Footer from "./components/Footer"; | ||
| import { Routes, Route, Navigate } from "react-router-dom"; | ||
| import Login from "./pages/login"; |
There was a problem hiding this comment.
The import path for the Login component uses lowercase 'login', but the file is named 'Login.jsx'. This will break in case-sensitive environments; update to './pages/Login'.
| import Login from "./pages/login"; | |
| import Login from "./pages/Login"; |
| throw error.response.data.message; | ||
| } | ||
| throw { status: false, message: "Network error occurred" }; |
There was a problem hiding this comment.
This throws a string here but throws an object in the network error case, leading to inconsistent error shapes. Consider always throwing a single Error object with a message property.
| throw error.response.data.message; | |
| } | |
| throw { status: false, message: "Network error occurred" }; | |
| throw new Error(error.response.data.message); | |
| } | |
| throw new Error("Network error occurred"); |
| const apiService = { //register user | ||
| register: async (userData) => { | ||
| try { | ||
| const response = await apiClient.post('auth/register', userData); | ||
| return response.data; | ||
| } catch (error) { | ||
| console.error('Registration failed:', error); | ||
| // Throw server response message instead of axios error | ||
| if (error.response && error.response.data) { | ||
| throw error.response.data.message; | ||
| } | ||
| throw { status: false, message: "Network error occurred" }; |
There was a problem hiding this comment.
[nitpick] Error handling in register, login, and logout is duplicated. Extract a shared helper to parse and rethrow API errors to DRY up this logic.
| const apiService = { //register user | |
| register: async (userData) => { | |
| try { | |
| const response = await apiClient.post('auth/register', userData); | |
| return response.data; | |
| } catch (error) { | |
| console.error('Registration failed:', error); | |
| // Throw server response message instead of axios error | |
| if (error.response && error.response.data) { | |
| throw error.response.data.message; | |
| } | |
| throw { status: false, message: "Network error occurred" }; | |
| const handleApiError = (error, context) => { | |
| console.error(`${context} failed:`, error); | |
| if (error.response && error.response.data) { | |
| throw error.response.data.message; | |
| } | |
| throw { status: false, message: "Network error occurred" }; | |
| }; | |
| const apiService = { //register user | |
| register: async (userData) => { | |
| try { | |
| const response = await apiClient.post('auth/register', userData); | |
| return response.data; | |
| } catch (error) { | |
| handleApiError(error, 'Registration'); |
| navigate('/login'); | ||
| } catch (error) { | ||
| console.error('Registration failed:', error); | ||
| setErrors({ ...errors, email: error.message }); |
There was a problem hiding this comment.
The catch block assumes error has a message property, but apiService may throw a raw string. This could set email to undefined—normalize the error into an object before accessing message.
| console.error("Login failed:", error); | ||
| setErrors({ ...errors, email: error.message }); |
There was a problem hiding this comment.
The catch block accesses error.message, but thrown errors may be plain strings, leading to undefined. Normalize or wrap the error so you always have a message property.
| console.error("Login failed:", error); | |
| setErrors({ ...errors, email: error.message }); | |
| const normalizedError = error instanceof Error ? error : new Error(String(error)); | |
| console.error("Login failed:", normalizedError); | |
| setErrors({ ...errors, email: normalizedError.message }); |
| @import "tailwindcss"; | ||
| @plugin "daisyui"; | ||
|
|
||
| * { |
There was a problem hiding this comment.
[nitpick] The global * selector applies the font to every element, which can be heavy and affect third-party widgets. Consider scoping this to body or a root container.
| * { | |
| body { |
Add Routing
Implement Backend
Add necessary Dependencies