-
Notifications
You must be signed in to change notification settings - Fork 176
fix: correct todos navigation link path #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Change /todos to /dashboard/todos to match actual route - Fixes 404 error when clicking Todos in navigation - Actual route is in app/dashboard/todos/page.tsx
WalkthroughNavigation component updated to use route constants for dashboard and todos, swapped container class from Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Nav as Navigation Component
participant Routes as Route Constants
participant Router as App Router
Note over Nav,Routes: Navigation now reads route constants
User->>Nav: Click "Home" / "Todos" / Logo
Nav->>Routes: Resolve route (dashboardRoutes.dashboard / todosRoutes.list)
Routes-->>Nav: Return route path
Nav->>Router: Navigate to resolved path
Router-->>User: Render target page
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
Enhanced with Route Constants & Dark Mode SupportAdded improvements to this PR: Changes
Benefits
Commit: c07ed1f |
- Import dashboardRoutes and todosRoutes for type-safe navigation - Replace hardcoded paths with route constants (dashboardRoutes.dashboard, todosRoutes.list) - Fix dark mode colors: bg-white → bg-card, remove text-gray-900 - Improves maintainability and dark mode support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/navigation.tsx(1 hunks)
🔇 Additional comments (4)
src/components/navigation.tsx (4)
10-10: LGTM! Dark mode theming improvement.Switching from
bg-whitetobg-cardensures the navigation adapts properly to both light and dark themes.
14-19: LGTM! Proper theming for the logo link.Using the route constant and removing the hardcoded
text-gray-900class allows the text color to adapt to the current theme.
21-21: LGTM! Consistent use of route constants.The Home button now uses the same route constant pattern, ensuring consistency across navigation items.
5-6: Route constants properly integrated with correct paths and dark mode support.All changes verified:
- Route modules exist with correct exports (dashboardRoutes:
/dashboard; todosRoutes:/dashboard/todos,/dashboard/todos/new,/dashboard/todos/{id}/edit)- All hardcoded routes replaced with constants (lines 16, 21, 27)
- Todos link correctly points to
/dashboard/todosviatodosRoutes.list- Dark mode styling applied (
bg-cardon line 10)- Type-safe implementation using
as constin route definitions
| </Button> | ||
| </Link> | ||
| <Link href="/todos"> | ||
| <Link href={todosRoutes.list}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
LGTM! Core fix for the 404 navigation issue.
Using todosRoutes.list resolves the original problem where the link pointed to /todos instead of the correct route. This type-safe approach prevents similar issues in the future.
Run the following script to verify the route value and check for any remaining hardcoded todo routes:
🏁 Script executed:
#!/bin/bash
# Description: Verify todosRoutes.list value and find any hardcoded todo paths
echo "=== Checking todosRoutes.list value ==="
rg -A 5 "export.*todosRoutes|const todosRoutes" --type ts --type tsx
echo -e "\n=== Searching for hardcoded /todos or /dashboard/todos paths ==="
rg -n 'href=["'"'"'](/todos|/dashboard/todos)["'"'"']' --type tsx --type ts -g '!*.route.ts'Length of output: 161
🏁 Script executed:
#!/bin/bash
# Description: Verify todosRoutes.list value and find any hardcoded todo paths
echo "=== Checking todosRoutes.list definition ==="
rg -n "export.*todosRoutes|const todosRoutes" -g '*.ts' -g '*.tsx' -A 5
echo -e "\n=== Searching for hardcoded /todos paths in components ==="
rg -n "href.*['\"].*todos" -g '*.ts' -g '*.tsx' -g '!*.test.*' -g '!*.spec.*'
echo -e "\n=== Checking navigation.tsx around line 27 ==="
rg -n -A 3 -B 3 "todosRoutes\.list" -g '*.tsx'Length of output: 2483
Correct fix, but inconsistent route usage elsewhere in codebase.
The change at line 27 correctly uses todosRoutes.list (resolves to /dashboard/todos), which fixes the original 404 navigation issue and maintains type safety.
However, src/modules/dashboard/dashboard.page.tsx still contains hardcoded routes that should be refactored for consistency:
- Line 37:
href="/dashboard/todos"→ should usetodosRoutes.list - Line 57:
href="/dashboard/todos/new"→ should usetodosRoutes.new
🤖 Prompt for AI Agents
In src/components/navigation.tsx around line 27 and in
src/modules/dashboard/dashboard.page.tsx at lines 37 and 57, replace the
remaining hardcoded dashboard todo routes with the route constants: change
href="/dashboard/todos" to use todosRoutes.list and href="/dashboard/todos/new"
to use todosRoutes.new; ensure the file imports todosRoutes from its module (add
the import if missing) and keep the Link usage consistent and type-safe across
the codebase.
Problem
The navigation bar has a "Todos" link that points to
/todos, but the actual route is/dashboard/todos. This causes a 404 error when users click the link.Solution
Update the link in
src/components/navigation.tsxfrom/todosto/dashboard/todosto match the actual route structure.Testing
Changes
src/components/navigation.tsx:25href="/todos"tohref="/dashboard/todos"Summary by CodeRabbit