Refactor Navigation to use react-router-dom v6 #250
Conversation
|
|
||
| export const useAdminSettings = (): NavDataItem[] => { | ||
| // get auth access for example set admin as true | ||
| const isAdmin = true //this should be a call to getting auth / role access |
There was a problem hiding this comment.
You can also test that changing this to false will remove the fake Settings nav items
There was a problem hiding this comment.
mmm not quite what I expected, this is great to hide the settings for the user, but they can still navigate to those routes adding the path to the browser.
A great example would be havving Setting 1 as a new mock page called "Admin", and inside either not mounting that route with conditional rendering or creating a private route.
|
|
||
| export const useAdminSettings = (): NavDataItem[] => { | ||
| // get auth access for example set admin as true | ||
| const isAdmin = true //this should be a call to getting auth / role access |
There was a problem hiding this comment.
mmm not quite what I expected, this is great to hide the settings for the user, but they can still navigate to those routes adding the path to the browser.
A great example would be havving Setting 1 as a new mock page called "Admin", and inside either not mounting that route with conditional rendering or creating a private route.
Signed-off-by: Griffin-Sullivan <gsulliva@redhat.com>
|
@lucferbux: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ederign, lucferbux The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
app/indextoapp/Appsince it exports<App />index.tsxandapp/App.tsxto reflect all the changesHow Has This Been Tested?
Running the frontend and making sure routes work as expected and nav groups work correctly. Make sure the sidebar can open and close correctly.
We could consider adding a UI test for the nav, however I'm not sure how beneficial this is yet since we plan on adding "real" pages soon. At the moment everything is placeholder.
Merge criteria: