-
Notifications
You must be signed in to change notification settings - Fork 1
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
New login page #12
New login page #12
Conversation
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.
Excellent work with refactoring everything, and implementing the animations! Also good job with following the PR protocol and including visuals.
I've included in the comments some suggestions for improvements, as well as a couple of things to remove before merging. Please let me know if you have any questions about them
transcription/requirements.txt
Outdated
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.
Please revert changes here before pushing to main, as they are environment-specific changes for the transcription api and do not directly affect the login/sign up code pages.
frontend/src/assets/images/logo.png
Outdated
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.
Since this is the logo for now this is okay, but in the future we should avoid committing Flowleaflets images on our repo (there should already be a .gitignore file in assets if you pull from the latest main)
/> | ||
</div> | ||
|
||
<div className="password-group"> |
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.
We should add a confirm password field as well, directly below this field. Ideally should also let designers know to update the figma
frontend/src/main.jsx
Outdated
|
||
createRoot(document.getElementById("root")).render( | ||
<AuthProvider> | ||
<BrowserRouter> | ||
<Routes> | ||
<Route path="/login" element={<Login />} /> | ||
<Route path="/homepage" element={<Home />} /> | ||
<Route path="/signup" element={<SignUp />} /> | ||
<Route path="/logcode" element={<LogCode />} /> |
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.
logCode should be in protected routes (the full flow for going from signin to accessing the logcode page will be implemented later, but the idea is that the logCode page will only be accessible for accounts that have been registered with email and password)
Could you also rename "homepage" to "home"? Currently if I try to access a protected route without logging in, it will direct the user back to home, and it will say page not found
|
||
createRoot(document.getElementById("root")).render( | ||
<AuthProvider> | ||
<BrowserRouter> | ||
<Routes> | ||
<Route path="/login" element={<Login />} /> | ||
<Route path="/homepage" element={<Home />} /> | ||
<Route path="/signup" element={<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.
We need the signup page to redirect to the login page. Currently, all signup buttons do not work with the route changes. Alternatively could change signup button to redirect to login page.
frontend/src/pages/LogCode.jsx
Outdated
} | ||
try { | ||
setLoading(true); | ||
await new Promise((resolve) => setTimeout(resolve, 1000)); |
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.
do we need the loading here?
frontend/src/pages/LogCode.jsx
Outdated
href="#" | ||
onClick={(e) => { | ||
e.preventDefault(); | ||
alert("Located inside your logbook."); |
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.
Screen.Recording.2024-11-23.at.9.31.17.PM.mov
Screen.Recording.2024-11-23.at.9.49.25.PM.mov
Updated Rerouting and Log Code Page:
Screen.Recording.2024-11-26.at.10.14.50.PM.mov
Authentication for the log code must be implemented, ensuring that each user has a unique code stored in our database.
Refactored Homepage:
Screen.Recording.2024-11-26.at.9.58.03.PM.mov