-
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
Add navbar to all pages #51
base: main
Are you sure you want to change the base?
Conversation
added the NavBar to all pages except for /login and /signup
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.
Good work so far, we do need to fix a few things I added some comments.
Also make sure to run the linter, github yells at us if we don't lol
const Layout: React.FC<LayoutProps> = ({ children }) => { | ||
const location = useLocation(); | ||
|
||
const getPageName = () => { |
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.
Use the constants in constants/Routes.ts for the paths.
} | ||
}; | ||
|
||
return ( |
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.
The navbar is cutting off the top part of pages (user management, dev utility).
We may need some global CSS here.
@vips11 Do you have any recommendations on how to get a global navbar to work without cutting off the actual page content?
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.
Looks good to me!
great work on finding a CSS fix!
Notion ticket link
Add Navbar To All Pages
Implementation description
/login
and/signup
).Steps to test
What should reviewers focus on?
Checklist