Route set up#45
Route set up#45akinsanmi60 wants to merge 2 commits intoRealDevSquad:developfrom akinsanmi60:Route-set-up
Conversation
|
Thanks for the PR. Can you please mention the description of this PR & if it was tested in the browser? |
Neha
left a comment
Neha
left a comment
There was a problem hiding this comment.
Please add the description and test-cases too for the PR
| @@ -1,8 +1,17 @@ | |||
| import Header from "./components/Header/Header"; | |||
There was a problem hiding this comment.
The header is not getting used here. This should be removed from here.
| return ( | ||
| <div>Hey</div> | ||
| <Routes> | ||
| <Route path="/" /> |
There was a problem hiding this comment.
What would come at /? As of now, I can only see blank page
| <div>Hey</div> | ||
| <Routes> | ||
| <Route path="/" /> | ||
| <Route path="/login" element={<Login />} /> |
There was a problem hiding this comment.
These components are not imported hence it is not working. only blank pages are coming.
| <Routes> | ||
| <Route path="/" /> | ||
| <Route path="/login" element={<Login />} /> | ||
| <Route path="/flag?mode=edit" element={<FlagEdit />} /> |
There was a problem hiding this comment.
Same here, the components have not imported.
| @@ -0,0 +1,7 @@ | |||
| import React from "react"; | |||
There was a problem hiding this comment.
@pallabez are we going to have separate components for edit and create?
There was a problem hiding this comment.
Nope. We were planning on reusing same component.
There was a problem hiding this comment.
Please remove the component and route.
| function App() { | ||
| return ( | ||
| <div>Hey</div> | ||
| <Routes> |
There was a problem hiding this comment.
This should be moved to a separate folder name Routes.
@pallabez What would be the homepage? As we are not focusing on the Login screen as of now.
There was a problem hiding this comment.
Homepage would be dashboard. No?
There was a problem hiding this comment.
Well, as we decided the login is not the priority right now then yes dashboard. We need to also highlight in UI till login is not their dashboard view is of the super user or non-super user.
| <Route path="/" /> | ||
| <Route path="/login" element={<Login />} /> | ||
| <Route path="/flag?mode=edit" element={<FlagEdit />} /> | ||
| <Route path="/flag?mode=create" element={<FlagCreate />} /> |
There was a problem hiding this comment.
What would be the advantage here of the mode=create
There was a problem hiding this comment.
The query would help us use same feature flag forum component to both edit & create as discussed before.
| @@ -0,0 +1,5 @@ | |||
| import React from "react"; | |||
|
|
|||
| export const Login = () => { | |||
There was a problem hiding this comment.
We are not using this pattern for component creation. Please check the button or header to follow the same pattern.
| @@ -0,0 +1,7 @@ | |||
| import React from "react"; | |||
|
|
|||
| const FlagEdit = () => { | |||
There was a problem hiding this comment.
We are not using this pattern for component creation. Please check the button or header to follow the same pattern.
| @@ -0,0 +1,7 @@ | |||
| import React from "react"; | |||
|
|
|||
| const FlagCreate = () => { | |||
There was a problem hiding this comment.
We are not using this pattern for component creation. Please check the button or header to follow the same pattern.
|
There are new requirements for the Route. We would be having: Left Nav:
and then within the pages, we will have:
|
|
Closing this PR as a new PR #64 is raised |
3 participants
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: