Conversation
|
Pending:
|
ui/src/Constant/constant.js
Outdated
| export const NAV = { | ||
| "Home" : "/", | ||
| "Create Users" : "/users", | ||
| "Flags" : "/flags", | ||
| "SDK" : "/sdk", | ||
| "Docs" : "/docs", | ||
| } | ||
|
|
||
| export const PATH = { | ||
| "HOME" : "/", | ||
| "CREATE_USERS" : "/users", | ||
| "FLAGS" : "/flags", | ||
| "SDK" : "/sdk", | ||
| "DOCS" : "/docs", | ||
| } No newline at end of file |
There was a problem hiding this comment.
I think we are violating DRY principle here.
How about?
| export const NAV = { | |
| "Home" : "/", | |
| "Create Users" : "/users", | |
| "Flags" : "/flags", | |
| "SDK" : "/sdk", | |
| "Docs" : "/docs", | |
| } | |
| export const PATH = { | |
| "HOME" : "/", | |
| "CREATE_USERS" : "/users", | |
| "FLAGS" : "/flags", | |
| "SDK" : "/sdk", | |
| "DOCS" : "/docs", | |
| } | |
| export const NAV = { | |
| "Home" : PATH.HOME, | |
| "Create Users" : PATH.CREATE_USERS, | |
| "Flags" : PATH.FLAGS, | |
| "SDK" : PATH.SDK, | |
| "Docs" : PATH.DOCS, | |
| } | |
| export const PATH = { | |
| "HOME" : "/", | |
| "CREATE_USERS" : "/users", | |
| "FLAGS" : "/flags", | |
| "SDK" : "/sdk", | |
| "DOCS" : "/docs", | |
| } |
There was a problem hiding this comment.
I am thinking to create an array object. An object will have a name and URL. Something like this:
and loop over to create the navigation and URL.
One down point I see in this is scaling to different languages could be not easy. WYT?
export const NAV = [
{
name: 'Home',
path: '/'
},
{
name: "Create Users",
path: "/users"
},
{
name: "Flags",
path: "/flags"
},
{
name: "SDK",
path: "/SDK"
},
{
name: "Docs",
path: "/docs"
}
]
Basically like this: https://reactrouter.com/en/main/hooks/use-routes
ui/src/test/Docs.test.js
Outdated
|
|
||
| describe('<Docs/> component', () => { | ||
| it('component should render',() => { | ||
| render(<Router><Docs /></Router>); |
There was a problem hiding this comment.
I'm not sure what purpose wrapping serves. Mind explaining?
There was a problem hiding this comment.
Good point. Removed from one test-case. Will add a new issue to update rest of the test-cases
ui/src/test/LeftNav.test.js
Outdated
| import { MemoryRouter as Router } from 'react-router-dom'; | ||
| import LeftNav from '../Components/LeftNav/LeftNav'; | ||
|
|
||
| describe('<LeftNav/> component', () => { |
There was a problem hiding this comment.
Is it a good idea to call it LeftNav? We might move it to right perhaps in future. Something like NavPanel would be better I guess. What do you think?
2 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 #45
Type of change
Please delete options that are not relevant.
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: