Skip to content
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

WIP: Add internal employee login page based on hi-fi designs #15

Merged
merged 6 commits into from
Jun 23, 2021

Conversation

jeffreyzhang2001
Copy link
Contributor

Notion ticket link

Build hi-fi login pages + logout button

Implementation description

DONE

  • Added the login page based on the hi-fi design
    image
    image

WIP

  • Add logout button after I find the design

Notes

  • Please test locally!

Checklist

  • My PR name is descriptive, is in imperative tense and starts with one of the following: [Feature],[Improvement] or [Fix],
  • I have run the appropriate linter(s)
  • I have requested a review from the RCD team on GitHub, or specific people who are associated with this ticket

@jeffreyzhang2001 jeffreyzhang2001 added the wip Work in progress label Jun 18, 2021
@jeffreyzhang2001 jeffreyzhang2001 requested a review from a team June 18, 2021 10:38
@railway-app
Copy link

railway-app bot commented Jun 18, 2021

This PR is being deployed to Railway 🚅

Live → richmond-centre-for-disability-pr-15.up.railway.app

[View Deployment]

@jeffreyzhang2001 jeffreyzhang2001 added don't merge Do not merge needs dev review Needs dev review and removed don't merge Do not merge wip Work in progress labels Jun 18, 2021
pages/login.tsx Outdated
</Flex>

{/* If NextAuth's callback doesn't contain a URL, auth (sending email) wasn't successful. */}
{!authState?.url ? (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was reading up on the nextAuth docs and was just curious - why did you use authState.url and not authState.ok here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

authState.ok will always contain 200 (the HTTP code) if the user's login request is succesfully received by the NextAuth client, regardless of whether or not the user has a valid email. We need to check for url instead since it only gets returned when an actual login email gets sent out.

@EmilioMena EmilioMena requested a review from a team June 21, 2021 02:20
@@ -18,9 +18,9 @@ export default function Layout({ children, header = true, footer = true }: Props
<Meta />
<Flex flexDirection="column" alignItems="center" minHeight="100vh">
{header && <Header />}
<Box flexGrow={1} paddingTop={20}>
<Flex flexGrow={1} width="100%" justifyContent="center">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference: Grid was not spanning 100% width. This was a necessary fix.

Copy link
Member

@OustanDing OustanDing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jeffreyzhang2001 jeffreyzhang2001 merged commit cca8515 into staging Jun 23, 2021
@jeffreyzhang2001 jeffreyzhang2001 deleted the jz/feature-add-login-page branch June 23, 2021 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs dev review Needs dev review
Projects
None yet
3 participants