-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OIDC header variant link fixes for new features #103
OIDC header variant link fixes for new features #103
Conversation
Signed-off-by: Nicholas Bucher <behappy54321@gmail.com>
Issues linked to changelog: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I combined most of the logic that was spread out between the 2 header variant files into this one. It was easy to forget to maintain both variants, so this should make it easier to support both.
/** | ||
* Since we support different authorization types, this is the way to tell if someone is logged in. | ||
*/ | ||
export function useIsLoggedIn() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran into some issues keeping this in the context, related to the access token state and SWR. Splitting it out into a separate hook and using useIsOidcAuthLoggedIn
fixed this.
</div> | ||
<div className="siteNavigating"> | ||
<NavLink to={"/"} className={"navLink"} end> | ||
Home |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we might want to consider moving all the nav links into an array or some such above with all if/else logic outside of the render, and then just have a map here (similar to what we did for sidebar in mesh repo)
This PR adds links for the new teams + apps pages to the OIDC auth header variant, as well as some refactoring of that area.