Feature/supabase auth - #34
Conversation
- Add supabaseClient.ts for Supabase initialization - Add authListener.ts to handle auth state changes - Add authSlice.ts for Redux auth state management - Integrate authSlice into Redux store
- Add paths.ts with PATHS, LABELS, and MENU_ITEMS constants - Add requiresAuth flag for protected routes - Improve maintainability by centralizing route definitions
- Create ProtectedRoute wrapper for protected pages - Add loading state during auth initialization - Redirect to login with return URL for unauthenticated users - Use Ant Design components (Flex, Space, Spin, Typography)
- Create Login component with toggle between sign up and sign in modes - Integrate Supabase authentication (signUp, signInWithPassword) - Add form validation using Zod schema - Implement return URL functionality after login - Add responsive design with Ant Design components - Use centralized constants for UI text and configuration
- Create Header component with navigation menu - Filter menu items based on authentication status - Add login/logout functionality - Display user email when authenticated - Add responsive design with SCSS - Replace old NavBar with new Header
- Add auth state initialization on app mount - Setup auth listener for session changes - Add ProtectedRoute for Todo and Tarot pages - Add Login route - Replace NavBar with new Header component - Update background color to use SCSS variable
- Check authentication before navigating to protected routes - Redirect to login with return URL if not authenticated - Prevent flash of protected pages before redirect - Remove unused Home.scss file
- Add todo-container wrapper with flexbox layout - Add todo-list-body for better content grouping - Use gap for consistent spacing between elements - Update card styling with white background and border-radius - Add subtle box-shadow to cards - Clean up unused CSS (legacy grid, sortable, action buttons) - Use SCSS variables consistently (gray-50 for background) - Organize responsive styles within component blocks
- Add try-catch for WebGLRenderer initialization - Add onError callback to TarotSceneCallbacks interface - Display user-friendly error message when WebGL is not supported - Add error state handling in TarotAR component - Show Result component with reload option on WebGL failure - Fix unused variable lint error
- Add proper typing for form submission handler - Export FormRef type for external use - Improve component reusability
- Update BackLink default path from /todo-list to /todo - Update TodoDetail to use PATHS constant - Ensure consistency with centralized path configuration
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Supabase authentication, Redux auth state, protected routes, centralized paths, a Header, and a Login page. The change also updates application initialization, form behavior, todo navigation, Tarot error handling, and related layouts and styles. ChangesAuthentication and application structure
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/views/Page/TodoDetail/TodoDetail.tsx (1)
55-56: Use PATHS constant for route checking.Line 55 hardcodes the path string "/todo-create" for checking create mode. Consider using the imported PATHS constant for consistency.
♻️ Proposed fix
-const isCreateMode = window.location.pathname === "/todo-create"; +const isCreateMode = window.location.pathname === PATHS.TODO_CREATE;src/views/Page/TodoList/components/TodoForm/TodoForm.tsx (1)
229-257: Blob URLs are created on every render, causing memory leaks.
URL.createObjectURL(item)is called during the render cycle every time the component re-renders. While you track URLs for cleanup on unmount, new URLs are created each render for the same files, accumulating in memory.Proposed fix using useMemo or stable URL mapping
+ const blobUrlMap = React.useRef<Map<File, string>>(new Map()); const fileList: UploadFile[] = value.map( (item: any, index: number) => { if (item instanceof File) { - // Create blob URL for preview - const previewUrl = URL.createObjectURL(item); - // Track for cleanup - if (!createdBlobUrls.current.includes(previewUrl)) { - createdBlobUrls.current.push(previewUrl); - } + // Reuse existing blob URL or create new one + let previewUrl = blobUrlMap.current.get(item); + if (!previewUrl) { + previewUrl = URL.createObjectURL(item); + blobUrlMap.current.set(item, previewUrl); + createdBlobUrls.current.push(previewUrl); + } return { uid: (item as any).uid || `new-${index}`, name: item.name, status: "done", url: previewUrl, thumbUrl: previewUrl, originFileObj: item, } as UploadFile; }
🤖 Fix all issues with AI agents
In @src/components/ProtectedRoute/ProtectedRoute.tsx:
- Line 41: The Ant Design Space usage is passing an invalid prop name
"orientation" on the Space component; replace the "orientation" prop with the
correct "direction" prop on the Space element in ProtectedRoute (i.e., change
Space orientation="vertical" to Space direction="vertical") so the layout
direction is applied correctly.
- Around line 17-27: The useEffect in ProtectedRoute currently uses a hardcoded
100ms timeout (isLoading/setIsLoading) which can cause a race where Supabase
session restoration is slower and authenticated users are redirected; replace
this mechanism by selecting the auth initialization/loading state from Redux
(e.g., useAppSelector(state => state.auth.isLoading or state.auth.initialized)
instead of the timeout), derive isLoading from that selector and only perform
redirect logic after that auth loading flag is false, and remove the artificial
setTimeout and its cleanup so the component waits for real auth readiness (refer
to useAppSelector, state.auth.user, isLoading, setIsLoading and the useEffect
inside ProtectedRoute).
In @src/views/Page/Login/Login.scss:
- Around line 2-3: Add an inline comment in Login.scss beside the existing rules
to document the margin-offset pattern: note that min-height: calc(100vh - 64px)
corresponds to the Header's explicit height (64px) and that margin: -24px
intentionally counteracts the parent .app-content 24px padding to produce an
edge-to-edge, full-viewport layout; keep the comment concise and colocated with
the min-height/margin declarations so future maintainers understand the layout
intent.
🧹 Nitpick comments (10)
src/utils/supabaseClient.ts (1)
6-10: Consider failing fast when credentials are missing.The client is created with empty strings when env vars are missing, which will cause confusing runtime errors during auth operations (e.g., sign-in, sign-out). Consider throwing an error to fail fast during initialization rather than allowing a non-functional client.
♻️ Suggested improvement
if (!supabaseUrl || !supabaseAnonKey) { - console.error("Missing Supabase URL or Anon Key. Check .env.local"); + throw new Error("Missing Supabase URL or Anon Key. Check .env.local"); } -export const supabase = createClient(supabaseUrl || "", supabaseAnonKey || ""); +export const supabase = createClient(supabaseUrl, supabaseAnonKey);Alternatively, if you want the app to run in degraded mode without auth, wrap auth-dependent operations with guards.
src/views/common/Header/Header.tsx (2)
27-30: Add error handling for sign-out.If
signOut()fails (e.g., network error), the user is still redirected to HOME while potentially remaining authenticated. This could cause confusion.♻️ Suggested fix
const handleLogout = async () => { - await supabase.auth.signOut(); - history.push(PATHS.HOME); + const { error } = await supabase.auth.signOut(); + if (error) { + console.error("Sign out failed:", error.message); + // Optionally show user notification + return; + } + history.push(PATHS.HOME); };
45-51: Menu highlighting won't work for sub-routes.When on
/todo/123,currentPathwon't matchPATHS.TODO("/todo"), so the Todo menu item won't be highlighted. Consider matching by prefix for nested routes.♻️ Possible approach
// Derive selected key by finding the menu item whose path is a prefix of currentPath const selectedKey = visibleMenuItems.find( (item) => currentPath === item.path || currentPath.startsWith(item.path + "/") )?.key; // Then use: selectedKeys={selectedKey ? [selectedKey] : []}src/views/common/Header/Header.scss (1)
62-68: Targeting Ant Design internal class may break on version updates.Styling
.ant-menu-itemdirectly is fragile as Ant Design may change class names between versions. Consider using Ant Design's theming/token system or applying styles via the component'sclassNameprop with a more specific selector.src/views/Page/Tarot/TarotScene.ts (1)
214-226: Code ordering is confusing:matBackreferenced before declaration.The error callback references
matBack(lines 223-224) before it's declared (line 232). While this works because the callback executes asynchronously aftermatBackis initialized, the code order is misleading and could cause issues if refactored.♻️ Consider reordering for clarity
Declare
matBackbefore thetextureLoader.loadcall, or move the error handler logic after the material declarations.src/constants/paths.ts (1)
23-29: Consider i18n for menu labels.The labels are hardcoded in English. Based on learnings, the project enforces Vietnamese (vi-VN) locale. Consider using an i18n library (e.g., react-i18next) for these labels to support localization, or update to Vietnamese if that's the target language.
src/views/App.tsx (1)
24-29: Consider adding error handling for session fetch.The
getSession()call doesn't handle potential errors. If the Supabase client fails to fetch the session (e.g., network issues), the error is silently ignored.Proposed fix
useEffect(() => { - supabase.auth.getSession().then(({ data: { session } }) => { - if (session) { - dispatch(setAuthUser({ user: session.user, session })); - } - }); + supabase.auth.getSession().then(({ data: { session }, error }) => { + if (error) { + console.error("Failed to fetch session:", error); + return; + } + if (session) { + dispatch(setAuthUser({ user: session.user, session })); + } + });src/views/Page/Tarot/TarotAR.tsx (1)
88-99: Control flow is confusing and may cause unintended behavior.The guard logs an error if
sceneManager.currentis null, but only returns early whenerroris truthy. If the scene manager is null for reasons other than an error (e.g., component not yet initialized),startGame()is still called via optional chaining, which silently does nothing.Proposed fix
const handleTopicSelect = (topic: TarotTopic) => { + if (!sceneManager.current) { + console.error("❌ Scene Manager not found during topic select!"); + return; + } + dispatch(setSelectedTopic(topic)); dispatch(setStep(TarotStep.PICKING)); - if (!sceneManager.current) { - console.error("❌ Scene Manager not found during topic select!"); - if (error) return; - } - sceneManager.current?.startGame(); + sceneManager.current.startGame(); // Start Audio soundManager.playSelectSound(); };src/views/Page/Login/Login.tsx (1)
130-131: Avoidas anytype cast for onSubmit handler.The
as anycast bypasses type safety. Consider properly typing theFormcomponent'sonSubmitprop or adjusting the handler signature.src/views/common/Form/Form.tsx (1)
54-59: Automatic reset on unmount may not be necessary.React Hook Form already cleans up its internal state when the component unmounts. This explicit reset could cause unnecessary re-renders before unmount or interfere with transitions where form state should persist.
Consider removing this effect unless there's a specific use case requiring form reset before unmount.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (31)
.env.example.gitignorepackage.jsonsrc/components/ProtectedRoute/ProtectedRoute.tsxsrc/constants/paths.tssrc/store/index.tssrc/store/slices/authSlice.tssrc/utils/authListener.tssrc/utils/supabaseClient.tssrc/views/App.scsssrc/views/App.tsxsrc/views/Page/Home/Home.scsssrc/views/Page/Home/Home.tsxsrc/views/Page/Login/Login.scsssrc/views/Page/Login/Login.tsxsrc/views/Page/Tarot/TarotAR.tsxsrc/views/Page/Tarot/TarotScene.tssrc/views/Page/TodoDetail/TodoDetail.scsssrc/views/Page/TodoDetail/TodoDetail.tsxsrc/views/Page/TodoList/TodoList.scsssrc/views/Page/TodoList/components/TodoForm/TodoForm.tsxsrc/views/Page/TodoList/components/TodoListContent/TodoListContent.tsxsrc/views/Page/TodoList/components/TodoListHeader/TodoListHeader.scsssrc/views/Page/TodoList/components/TodoListHeader/TodoListHeader.tsxsrc/views/common/BackLink/BackLink.tsxsrc/views/common/Form/Form.tsxsrc/views/common/Header/Header.scsssrc/views/common/Header/Header.tsxsrc/views/common/NavBar/NavBar.scsssrc/views/common/NavBar/NavBar.tsxsrc/views/common/NavBar/index.ts
💤 Files with no reviewable changes (8)
- src/views/common/NavBar/index.ts
- src/views/Page/TodoDetail/TodoDetail.scss
- src/views/common/NavBar/NavBar.scss
- .gitignore
- src/views/common/NavBar/NavBar.tsx
- .env.example
- src/views/Page/Home/Home.scss
- src/views/Page/TodoList/components/TodoListHeader/TodoListHeader.scss
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-10T19:29:31.324Z
Learnt from: DucCuong159
Repo: DucCuong159/Reactjs_test PR: 24
File: src/views/Page/TodoList/TodoList.tsx:307-311
Timestamp: 2025-12-10T19:29:31.324Z
Learning: Enforce Vietnamese (vi-VN) locale usage across the project for date formatting, numbers, and localization. In code, apply toLocaleString / Intl APIs with 'vi-VN' or configure i18n libraries to default to 'vi-VN'. Ensure UI components format dates, currencies, and relative times using the locale, and centralize locale configuration to avoid hard-coded formats per file.
Applied to files:
src/utils/authListener.tssrc/components/ProtectedRoute/ProtectedRoute.tsxsrc/utils/supabaseClient.tssrc/constants/paths.tssrc/views/Page/TodoList/components/TodoListContent/TodoListContent.tsxsrc/views/Page/Login/Login.tsxsrc/views/Page/Tarot/TarotAR.tsxsrc/views/common/Header/Header.tsxsrc/store/slices/authSlice.tssrc/views/App.tsxsrc/views/common/Form/Form.tsxsrc/views/Page/TodoList/components/TodoListHeader/TodoListHeader.tsxsrc/views/Page/TodoDetail/TodoDetail.tsxsrc/views/Page/Home/Home.tsxsrc/views/Page/Tarot/TarotScene.tssrc/views/common/BackLink/BackLink.tsxsrc/store/index.tssrc/views/Page/TodoList/components/TodoForm/TodoForm.tsx
🧬 Code graph analysis (12)
src/utils/authListener.ts (2)
src/utils/supabaseClient.ts (1)
supabase(10-10)src/store/index.ts (1)
store(7-14)
src/components/ProtectedRoute/ProtectedRoute.tsx (2)
src/store/hooks.ts (1)
useAppSelector(5-5)src/constants/paths.ts (1)
PATHS(6-17)
src/views/Page/TodoList/components/TodoListContent/TodoListContent.tsx (4)
src/views/common/StatusTag/index.ts (1)
StatusTag(1-1)src/views/common/StatusTag/StatusTag.tsx (1)
StatusTag(11-30)src/utils/statusUtils.ts (1)
getStatusLabel(7-9)src/utils/dateUtils.ts (1)
formatDate(8-10)
src/views/Page/Login/Login.tsx (4)
src/store/hooks.ts (1)
useAppDispatch(4-4)src/views/common/Form/Form.tsx (2)
FormRef(24-27)Form(75-77)src/utils/supabaseClient.ts (1)
supabase(10-10)src/constants/paths.ts (1)
PATHS(6-17)
src/views/Page/Tarot/TarotAR.tsx (1)
src/views/types/tarot.ts (1)
TarotTopic(27-30)
src/views/common/Header/Header.tsx (3)
src/store/hooks.ts (1)
useAppSelector(5-5)src/constants/paths.ts (3)
PATHS(6-17)MENU_ITEMS(35-54)LABELS(23-29)src/utils/supabaseClient.ts (1)
supabase(10-10)
src/views/App.tsx (5)
src/store/hooks.ts (1)
useAppDispatch(4-4)src/utils/supabaseClient.ts (1)
supabase(10-10)src/utils/authListener.ts (1)
setupAuthListener(5-23)src/views/common/ScrollToTop/ScrollToTop.tsx (1)
ScrollToTop(8-16)src/constants/paths.ts (1)
PATHS(6-17)
src/views/Page/TodoList/components/TodoListHeader/TodoListHeader.tsx (1)
src/constants/paths.ts (1)
PATHS(6-17)
src/views/Page/TodoDetail/TodoDetail.tsx (1)
src/constants/paths.ts (1)
PATHS(6-17)
src/views/Page/Home/Home.tsx (2)
src/store/hooks.ts (1)
useAppSelector(5-5)src/constants/paths.ts (1)
PATHS(6-17)
src/store/index.ts (3)
src/store/slices/todoSlice.ts (1)
todoSlice(132-143)src/store/slices/todoDetailSlice.ts (1)
todoDetailSlice(30-31)src/store/slices/authSlice.ts (1)
authSlice(47-48)
src/views/Page/TodoList/components/TodoForm/TodoForm.tsx (2)
src/views/common/Form/Form.tsx (1)
Form(75-77)src/views/common/Form/index.ts (1)
Form(1-1)
🔇 Additional comments (40)
src/views/App.scss (2)
12-21: LGTM! Layout changes align with the new header-based design.The flexbox layout with column direction and the updated content styling (auto overflow, flex expansion, and padding) properly support the new authentication-aware header layout introduced in this PR.
18-18: The$gray-50SCSS variable is properly defined and no issues exist.The variable is correctly declared in
src/styles/_variables.scsswith the value#f9fafb;and is accessible toApp.scss. No compilation errors will occur.src/views/Page/TodoList/components/TodoListHeader/TodoListHeader.tsx (2)
4-4: LGTM! Good use of centralized routing constants.Replacing the hard-coded route with
PATHS.TODO_CREATEimproves maintainability and consistency across the application.Also applies to: 33-33
42-43: LGTM! Added input attributes improve accessibility and testability.Adding
idandnameattributes to the Search component enhances form semantics and makes the input easier to target in automated tests.src/views/Page/Login/Login.scss (1)
14-35: LGTM! Responsive design breakpoints are well-structured.The progressive width adjustments (30% → 40% → 50% → 60% → 70%) provide good responsiveness across device sizes, ensuring the login card remains usable on smaller screens while maintaining a compact appearance on larger displays.
src/views/Page/TodoList/components/TodoListContent/TodoListContent.tsx (1)
43-102: LGTM! Structural reorganization aligns with layout improvements.Wrapping the results count and todo grid in a
.todo-list-bodycontainer is a clean refactoring that supports the CSS layout changes introduced in this PR. All existing rendering logic and functionality are preserved.package.json (1)
13-13: @supabase/supabase-js version specification is appropriate and free from known vulnerabilities.Version ^2.89.0 is actively maintained by Supabase with no reported security advisories. The caret (^) allows automatic updates to patch and minor versions (e.g., 2.90.0), ensuring the project benefits from bug fixes and improvements while maintaining backward compatibility.
src/store/slices/authSlice.ts (3)
1-9: LGTM!The imports and AuthState interface are properly structured with appropriate nullable types for user, session, and error states.
18-45: LGTM!The reducers are well-structured and follow Redux Toolkit best practices. State transitions are handled correctly with proper cleanup of loading and error states.
47-49: LGTM!Actions and reducer are properly exported following Redux Toolkit conventions.
src/views/Page/TodoDetail/TodoDetail.tsx (4)
15-15: LGTM!The centralized PATHS constants are correctly imported and consistently used throughout the component, replacing hardcoded route strings. This improves maintainability and aligns with the PR's routing refactor.
Also applies to: 89-89, 119-119, 152-152, 159-159
58-80: LGTM!The cleanup mechanism properly prevents memory leaks by:
- Capturing attachments in a ref to avoid stale closures
- Revoking blob URLs on unmount using the ref
- Resetting component state on unmount
This is a solid pattern for resource cleanup in React components.
84-94: LGTM!The 404 Result component is correctly simplified by removing the unnecessary wrapper div, and properly uses the centralized PATHS constant for navigation.
45-45: LGTM!The refactor to a const arrow function with default export and the removal of the root div wrapper in favor of a fragment are both appropriate. The fragment removes an unnecessary DOM element while preserving the component structure.
Also applies to: 166-166, 333-337
src/views/Page/TodoList/TodoList.scss (3)
4-19: LGTM!The layout refactor from grid to flexbox column is clean and modern. The use of gap for spacing and the container padding/background settings are appropriate.
21-76: LGTM!The card styling with proper padding, shadows, and typography is well-structured. The centered pagination container follows good UI patterns.
78-129: LGTM!The responsive styles appropriately adjust spacing and typography for tablet and mobile viewports. The Ant Design component overrides maintain consistent styling across the form elements.
src/views/common/BackLink/BackLink.tsx (1)
10-10: LGTM!The default route update from "/todo-list" to "/todo" correctly aligns with the centralized PATHS constants introduced in this PR (PATHS.TODO = "/todo").
src/utils/authListener.ts (1)
5-23: Initial session handling is already in place in App.tsx, making INITIAL_SESSION handling in setupAuthListener unnecessary.The app correctly handles initial session in App.tsx using
supabase.auth.getSession()before callingsetupAuthListener(). This two-step approach means setupAuthListener only needs to handle subsequent auth state changes (SIGNED_IN, TOKEN_REFRESHED, USER_UPDATED, SIGNED_OUT), which it does correctly. No changes needed.src/store/index.ts (1)
1-17: LGTM!The auth slice is correctly integrated into the Redux store following the same pattern as existing slices. The RootState type will automatically include the new
authfield.src/views/Page/Tarot/TarotScene.ts (2)
62-72: Good addition of error boundary for scene initialization.The try/catch with onError callback provides proper error propagation to consumers, enabling graceful error UI in TarotAR.tsx.
92-100: Appropriate WebGL detection with clear error message.Good practice to catch WebGL initialization failures and provide a descriptive error message for debugging.
src/constants/paths.ts (1)
1-58: Well-structured centralized constants.Good use of
as constfor literal types and clean separation of paths, labels, and menu configuration. The exported types (PathKey,LabelKey) provide good type safety for consumers.src/views/Page/Home/Home.tsx (3)
4-5: LGTM!Clean imports for centralized path constants and typed Redux selector.
7-15: LGTM!Good practice extracting UI text into a typed constant with
as constfor literal types.
21-27: LGTM!The
navigateTohelper correctly implements auth-aware navigation by redirecting unauthenticated users to the login page with the return path in state.src/views/App.tsx (2)
31-35: LGTM!Proper cleanup of the auth subscription on unmount prevents memory leaks and stale callbacks.
44-58: LGTM!Routes are well-organized using centralized PATHS constants, with appropriate protection applied to authenticated routes.
src/views/Page/Tarot/TarotAR.tsx (3)
35-35: LGTM!Good addition of error state for WebGL/AR initialization failures.
71-78: LGTM!Proper cleanup of the scene manager when an error occurs, preventing resource leaks and allowing clean re-initialization.
114-150: LGTM!Clean error UI with helpful guidance for users experiencing WebGL issues. The reload button provides a straightforward recovery path.
src/views/Page/Login/Login.tsx (4)
17-20: LGTM!Clean Zod schema with appropriate validation rules for email format and password minimum length.
47-55: LGTM!Good UX practice mapping authentication errors to user-friendly messages.
75-108: LGTM!Well-structured authentication flow with proper error handling, success feedback, and redirect logic.
114-118: No changes needed —orientationis the correct prop for newer versions of Ant Design.The
orientationprop is the modern API for Ant Design's Space component. In antd v4/v5, the prop was nameddirection, but newer releases replaced it withorientation(supporting the same values: "horizontal" | "vertical"). The code is using the current standard.Likely an incorrect or invalid review comment.
src/views/Page/TodoList/components/TodoForm/TodoForm.tsx (2)
114-130: LGTM!Clean migration to the new Form.Item wrapper pattern.
144-145: LGTM!Good accessibility practice adding explicit
idandnameattributes to form controls.src/views/common/Form/Form.tsx (3)
12-13: LGTM!Clean extension of the FormProps interface with optional style and onSubmit props.
49-51: LGTM!Exposing the reset method via the imperative handle allows parent components to programmatically reset the form.
74-77: LGTM!Clean compound component pattern attaching
AntdForm.ItemasForm.Itemfor convenient usage.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
- Remove artificial 100ms timeout that could cause race condition - Use auth.loading from Redux to track real auth initialization - Set loading state in App.tsx before checking session - Prevents authenticated users from being redirected during slow session restoration - Fixes race condition where Supabase session loads slower than timeout
- Change initial loading from false to true in authSlice - Prevents ProtectedRoute from redirecting before session check completes - Fixes issue where reload on protected pages redirects to login - Remove redundant setAuthLoading(true) dispatch in App.tsx
…9/Reactjs_test into feature/supabase-auth
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/views/App.tsx:
- Around line 24-38: The getSession() call in the useEffect lacks error handling
so failures leave the app loading; wrap supabase.auth.getSession() with a catch
(or use try/catch if converted to async) to handle errors, log the error, and
dispatch setAuthLoading(false) (and/or dispatch an auth error action if
available) instead of leaving the app stuck; keep the existing success path that
dispatches setAuthUser({ user: session.user, session }) and ensure the
subscription from setupAuthListener is still unsubscribed in the cleanup.
🧹 Nitpick comments (2)
src/components/ProtectedRoute/ProtectedRoute.tsx (2)
21-21: Consider making theexactprop configurable.The
exactprop is hardcoded totrue, which works for the current routes but limits flexibility if you later need non-exact protected routes (e.g., nested paths).♻️ Proposed refactor to allow configurable exact matching
<Route - exact {...rest} render={(props) => {This way, callers can pass
exactvia...restwhen needed.
29-29: Consider extracting the header height as a constant.The
64pxvalue appears to be the header height but is hardcoded. If the header height changes, this calculation will be incorrect.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/ProtectedRoute/ProtectedRoute.tsxsrc/store/slices/authSlice.tssrc/views/App.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-10T19:29:31.324Z
Learnt from: DucCuong159
Repo: DucCuong159/Reactjs_test PR: 24
File: src/views/Page/TodoList/TodoList.tsx:307-311
Timestamp: 2025-12-10T19:29:31.324Z
Learning: Enforce Vietnamese (vi-VN) locale usage across the project for date formatting, numbers, and localization. In code, apply toLocaleString / Intl APIs with 'vi-VN' or configure i18n libraries to default to 'vi-VN'. Ensure UI components format dates, currencies, and relative times using the locale, and centralize locale configuration to avoid hard-coded formats per file.
Applied to files:
src/components/ProtectedRoute/ProtectedRoute.tsxsrc/store/slices/authSlice.tssrc/views/App.tsx
🧬 Code graph analysis (2)
src/components/ProtectedRoute/ProtectedRoute.tsx (2)
src/store/hooks.ts (1)
useAppSelector(5-5)src/constants/paths.ts (1)
PATHS(6-17)
src/views/App.tsx (5)
src/store/hooks.ts (1)
useAppDispatch(4-4)src/utils/supabaseClient.ts (1)
supabase(10-10)src/utils/authListener.ts (1)
setupAuthListener(5-23)src/views/common/ScrollToTop/ScrollToTop.tsx (1)
ScrollToTop(8-16)src/constants/paths.ts (1)
PATHS(6-17)
🔇 Additional comments (4)
src/components/ProtectedRoute/ProtectedRoute.tsx (1)
16-17: LGTM! Race condition resolved correctly.The component now uses Redux auth loading state instead of the arbitrary timeout, eliminating the race condition where slow session restoration could incorrectly redirect authenticated users.
src/views/App.tsx (1)
46-60: LGTM! Route protection properly implemented.The routes are correctly configured with
PATHSconstants for maintainability, andProtectedRouteproperly guards authenticated pages while keeping Home and Login public.src/store/slices/authSlice.ts (2)
11-16: LGTM! Initial loading state prevents race conditions.The
loading: trueinitialization (with explanatory comment) ensures ProtectedRoute won't redirect before the session check completes, directly addressing the race condition mentioned in the PR objectives.
18-45: LGTM! Reducers correctly manage auth state lifecycle.All reducers properly handle the loading flag and state transitions. The implementation is clean, type-safe, and follows Redux Toolkit best practices.
- Add box-shadow override in global.scss for all .ant-card components - Remove local .ant-card override from TodoList.scss - Fix Space component prop: orientation -> direction in ProtectedRoute - Apply consistent shadow (0 2px 8px rgba(0,0,0,0.1)) across all cards
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/styles/global.scss`:
- Around line 3-5: Update the global font-family declaration to remove quotes
only from the single-word names "Roboto", "Oxygen", "Ubuntu", and "Cantarell";
retain quotes around multi-word names such as "Segoe UI", "Helvetica Neue", and
any other names requiring them.
In `@src/views/App.tsx`:
- Around line 25-37: Update the supabase.auth.getSession flow in App so it
inspects the resolved error alongside session, dispatches setAuthError for both
resolved refresh failures and rejected promises, and preserves loading-state
handling. Add an authentication error/retry rendering path before the
unauthenticated redirect so ProtectedRoute is not reached when auth.error is
present; do not treat an errored null session as a normal signed-out state.
🪄 Autofix
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3250586f-ab3f-4196-9bfc-22e33d3b3e8b
📒 Files selected for processing (3)
src/styles/global.scsssrc/views/App.tsxsrc/views/Page/TodoList/TodoList.scss
💤 Files with no reviewable changes (1)
- src/views/Page/TodoList/TodoList.scss
Fixes Applied SuccessfullyFixed 3 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
1 participant
Summary by CodeRabbit
Release Notes
New Features
Added authentication system with login and sign-up functionality.
Protected pages now require authentication with automatic redirects to login.
New header with user profile display and logout option.
Enhanced error handling and recovery for Tarot AR feature.
Style
Improved layout and responsive design throughout the app.
Restructured navigation interface.
Chores
Added Supabase authentication backend support.