-
Notifications
You must be signed in to change notification settings - Fork 132
Closed
Description
Is there an existing issue for this?
- I have searched the existing issues
What happened?
📌 Issue Overview
The frontend application's routing logic in frontend/src/App.tsx is implemented with a custom switch statement controlled by component state (activePage). This is an overly complex and unconventional approach, especially since the project already includes the react-router-dom library, which is designed for this purpose. This custom implementation makes the code difficult to understand, maintain, and extend.
🔍 Steps to Reproduce
Inspect the Original Routing Logic:
In the original version of the code, if you open the file frontend/src/App.tsx, you would notice a state variable, activePage, and a function called renderPage().
The renderPage() function contained a large switch statement that manually selected which page component to display based on the value of activePage.
Analyze Navigation Behavior:
In the original `frontend/src/components/layout/Sidebar.tsx` file, navigation links were <button> elements.
Clicking a button would call a function passed via props (setActivePage) to update the state in the main `App.tsx` component, triggering a re-render.
Observe the Anti-Pattern:
This manual, state-based routing is considered an anti-pattern because the project already includes the react-router-dom library, which is specifically designed to handle this declaratively.
A key symptom of this issue is that the browser's URL would not change when you navigated between different pages (e.g., Dashboard, Settings), making it impossible to bookmark or share links to specific pages.
🎯 Expected Behavior
The application should leverage the declarative routing features of react-router-dom.
The renderPage() function and activePage state should be removed.
Navigation should be handled declaratively using react-router-dom's <Route> components, with nested routes for protected pages.
The Sidebar component should use NavLink components to link to different routes.
The URL in the browser should always be synchronized with the currently displayed page, making the application state more predictable.
This change will lead to cleaner, more maintainable, and more idiomatic React code.
🚨 Actual Behavior
Navigation is managed by an activePage state variable and a renderPage() function with a large switch statement in App.tsx.
The Sidebar component uses a callback (setActivePage) to update the state in the App component, which then triggers a re-render of the correct page.
This results in prop-drilling and a centralized, imperative routing logic that is not standard for React applications.
The browser's URL does not consistently reflect the component being displayed, as routing is handled internally by state.
💡 Suggested Improvements
If you assign this issue I'll work on this issue
Record
- I agree to follow this project's Code of Conduct
- I want to work on this issue
Metadata
Metadata
Assignees
Labels
No labels