-
Notifications
You must be signed in to change notification settings - Fork 0
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 login page #36
Add login page #36
Conversation
The text boxes are a little different, but I just used whatever was available in the repo from previous PRs. |
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 great!
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.
Just need that one minor change to the Home page navbar and should be good to merge.
Not sure what the deployment issue is about, the page works fine on my end. |
frontend/src/pages/profile.tsx
Outdated
/> | ||
<Textfield | ||
register={register} | ||
setValue={setValue} |
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 deployment fails because the frontend build fails (can check that by running npm run build
).
The issue seems to be setValue
does not exist on Textfield
, so if you change this line to setCalendarValue={setValue}
it'll fix the build and make the calendar popover work.
Visit the preview URL for this PR (updated for commit fa97bb5): https://pia-dev-60cea--pr36-feature-vs2961-login-ct81y5v8.web.app (expires Fri, 01 Mar 2024 00:53:19 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: b780ee12a240535f7ca0729d49968573a1f3e284 |
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.
Added some basic comments
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.
LGTM! 🚀
Tracking Info
Resolves #27
Changes
Added the Login Page for mobile and web views.
Testing
Checked the pages against Figma.
Confirmation of Change