Added: Navigation and Routing #2281
Conversation
|
|
Important Auto Review SkippedAuto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThis update introduces a new "Home" page linked to the "Statistics" section, enriches the navigation with additional routes for "Statistics" and "Workflows", and modifies redirection paths for authenticated/unauthenticated users to the newly established "Statistics" page. These changes aim to streamline navigation and enhance user experience within the application. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (5)
- apps/backoffice-v2/src/Router/Router.tsx (3 hunks)
- apps/backoffice-v2/src/common/components/organisms/Header/hooks/useNavbarLogic/useNavbarLogic.tsx (2 hunks)
- apps/backoffice-v2/src/common/components/templates/Providers/Providers.tsx (1 hunks)
- apps/backoffice-v2/src/pages/Statistics/Statistics.page.tsx (1 hunks)
- apps/backoffice-v2/src/pages/Workflows/Workflows.page.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
- apps/backoffice-v2/src/pages/Statistics/Statistics.page.tsx
- apps/backoffice-v2/src/pages/Workflows/Workflows.page.tsx
Additional comments not posted (2)
apps/backoffice-v2/src/common/components/templates/Providers/Providers.tsx (1)
14-20: Ensure the new redirection paths (/en/statisticsand/en/auth/sign-in) are valid and correctly integrated into the application's routing configuration.apps/backoffice-v2/src/Router/Router.tsx (1)
24-25: Verify the correctness of the paths (/en/statisticsand/en/workflows) and ensure that the error elements are properly configured for the new routes for "Statistics" and "Workflows" pages.Also applies to: 112-121
Verification successful
The paths for the "Statistics" and "Workflows" pages are correctly implemented as
/:locale/statisticsand/:locale/workflows, respectively, with appropriate error handling configured viaerrorElement: <RouteError />. This confirms that the changes align with the objectives of integrating these pages into the application's routing framework.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the correctness of the paths and the configuration of the error elements. rg '/en/statistics' apps/backoffice-v2/src/Router/Router.tsx rg '/en/workflows' apps/backoffice-v2/src/Router/Router.tsx rg '<RouteError />' apps/backoffice-v2/src/Router/Router.tsxLength of output: 857
Script:
#!/bin/bash # Inspect the entire routing configuration within Router.tsx to verify the implementation of the new routes and their error handling. cat apps/backoffice-v2/src/Router/Router.tsxLength of output: 5308
| { | ||
| text: 'Home', | ||
| icon: <Home size={20} />, | ||
| href: '/en/statistics', | ||
| key: 'nav-item-Home', | ||
| }, |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process has confirmed that the '/en/statistics' path does not exist in the router configuration, indicating an issue with the validity of the link for the "Home" navigation item. However, the 'nav-item-Home' key is confirmed to be unique within the navigation items. Based on these findings, the review comment is partially verified, highlighting the need to address the non-existent path issue.
Analysis chain
Ensure the link for the "Home" navigation item (/en/statistics) is valid and correctly integrated into the application's routing configuration. Also, verify the consistency and uniqueness of the navigation item's key (nav-item-Home).
The verification process is inconclusive due to the lack of output for the '/en/statistics' path in the router configuration. This could indicate that the path is not defined, or there might have been an issue with the search command. Additionally, while the 'nav-item-Home' key is present, the output does not confirm its uniqueness. Further investigation is required to conclusively verify both aspects.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the existence of the '/en/statistics' path in the router configuration.
rg '/en/statistics' apps/backoffice-v2/src/Router/Router.tsx
# Ensure the key 'nav-item-Home' is unique across the navigation items.
rg 'nav-item-Home' apps/backoffice-v2/src/common/components/organisms/Header/hooks/useNavbarLogic/useNavbarLogic.tsx
Length of output: 205
Script:
#!/bin/bash
# Re-verify the existence of the '/en/statistics' path in the router configuration with a corrected command.
rg '/en/statistics' apps/backoffice-v2/src/Router/Router.tsx || echo "Path not found"
# Check for multiple occurrences of the 'nav-item-Home' key to ensure its uniqueness.
rg 'nav-item-Home' apps/backoffice-v2/src/common/components/organisms/Header/hooks/useNavbarLogic/useNavbarLogic.tsx -c
Length of output: 221
|
Great work! Before merging try and see if you can understand why the nav item is not styled as the active item. Notice its a shade of gray instead of the expected black. |
Thanks for addressing the issue, I checked it and got to know that the 'Home' nav-item is not styled as active item when it's active, and added changes to make sure it works perfectly fine. |
| { | ||
| path: '/:locale/auth/sign-in', | ||
| element: <SignIn />, | ||
| errorElement: <RouteError />, | ||
| }, | ||
| ] |
There was a problem hiding this comment.
Please ensure your IDE is configured to run Prettier and ESLint on save. Install the respective plugins if you haven't and make sure you're not running a custom format on save. 🙏🏼
apps/backoffice-v2/src/common/components/organisms/Header/Header.Navbar.tsx
Outdated
Show resolved
Hide resolved
...backoffice-v2/src/common/components/organisms/Header/hooks/useNavbarLogic/useNavbarLogic.tsx
Outdated
Show resolved
Hide resolved
apps/backoffice-v2/src/common/components/organisms/Header/Header.Navbar.tsx
Outdated
Show resolved
Hide resolved
…-0010/feat/navigation-and-routing
| if (!navItem.children) { | ||
| return navItem.href === pathname; | ||
| } |
There was a problem hiding this comment.
Revert please. This should never be true since a filter group always has children. The suggestion in the previous comment was general advice for the future.
shashankvish0010
deleted the
shashankvish-0010/feat/navigation-and-routing
branch
Description
Added a "Home" page to the navigation bar, represented by the 'Home' icon.
Integrated the "Statistics" page and "Workflows" page routes into the application's routing structure within the authenticated layout for now.
I modified the authentication redirect; now it redirects to statistics.
fixes: #2263
How these changes were tested
Tested with local development setup.
Examples and references