feat: Login/Signup UI Page and Corresponding Handling Functions#58
feat: Login/Signup UI Page and Corresponding Handling Functions#58
Conversation
cbolles
left a comment
cbolles
left a comment
There was a problem hiding this comment.
Nice start! I added in my comments, some high level things to consider
- Leverage MUI for the components, you'll notice in the code base there is little to no plain HTML tags and that's because MUI handles thinks like being dynamic, follow global styling, and solve common HTML problems out of the box.
- You shouldn't need much custom CSS thanks to MUI
- Instead of using react routing, the authentication should be implemented as a single page.
Overall great work and let me know if you need any assistance!
packages/client/src/context/UIComponents/ResetPassword.component.tsx
Outdated
Show resolved
Hide resolved
cbolles
left a comment
cbolles
left a comment
There was a problem hiding this comment.
Another good step in the right direction, I think there may also be some benefit in some group programming. MUI can actually handle a lot of the tricky parts.
packages/client/src/components/auth/NavigationSideBar.component.tsx
Outdated
Show resolved
Hide resolved
| maxWidth="sm" | ||
| sx={{ display: 'flex', flexDirection: 'column', alignItems: 'center', justifyContent: 'center', height: '80vh' }} | ||
| > | ||
| <NavigationSidebar /> |
| )} | ||
| {authenticated && children} | ||
| </AuthContext.Provider> | ||
| <Container |
There was a problem hiding this comment.
With the switch to have a lot of view logic in this component, its becoming difficult to reason through.
For example, you'll notice after you login that the tabs of the login logic are still visible over the rest of the webpage. I'd recommend either making an Auth.component to store the tab view or move the tab view into the Firebase wrapper.
| </AuthContext.Provider> | ||
| <Container | ||
| maxWidth="sm" | ||
| sx={{ display: 'flex', flexDirection: 'column', alignItems: 'center', justifyContent: 'center', height: '80vh' }} |
There was a problem hiding this comment.
I see this used in a few places, you probably want to checkout the MUI Stack component for column oriented views
| } | ||
|
|
||
| // Login Page Component | ||
| const LoginComponent: FC<LoginComponentProps> = ({ auth, onLoginSuccess }) => { |
There was a problem hiding this comment.
For consistency with the project, having things exported via export const LoginComponent instead of having a default export
* Some cleaning up of the MUI usage
cbolles
left a comment
cbolles
left a comment
There was a problem hiding this comment.
Looks great! Excellent job. Good job sticking with it and adding in those improvements!
2 participants
Uh oh!
There was an error while loading. Please reload this page.