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

Tweak login styles #187

Merged
merged 2 commits into from
Feb 13, 2023
Merged

Tweak login styles #187

merged 2 commits into from
Feb 13, 2023

Conversation

humphd
Copy link
Contributor

@humphd humphd commented Feb 12, 2023

This makes a few adjustments to the login and root routes.

  1. I've set a min-height on the html and body so they always fill 100% of the available hight
  2. I've centred the login components vertically in the screen, so it works at all sizes
  3. I've darkened the gray we use, and repeated it for the secondary text

Here's what it looks like, but you really need to try in a window you can resize:

Screenshot 2023-02-12 at 5 26 01 PM

@humphd humphd added enhancement Improving an existing solution category: front end Front end part of our web service labels Feb 12, 2023
@humphd humphd added this to the Milestone 0.3 milestone Feb 12, 2023
@humphd humphd self-assigned this Feb 12, 2023
Copy link
Contributor

@Ririio Ririio left a comment

Choose a reason for hiding this comment

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

Other than my two questions it looks great

@@ -20,18 +20,18 @@ export async function loader({ request }: LoaderArgs) {

function Document({ children }: { children: React.ReactNode }) {
return (
<html lang="en">
Copy link
Contributor

Choose a reason for hiding this comment

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

Should lang be 'en-CA'?

@@ -16,20 +16,22 @@ export const action = async ({ request }: ActionArgs) => {

export default function Login() {
return (
<Center height="container.md" marginTop="16">
<Center height="full">
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also consider resizing the container and its contents to allow a much more responsive login page? Or is that not what we're going for with this pr? Currently at lower res the contents aren't properly sized
image

@humphd
Copy link
Contributor Author

humphd commented Feb 13, 2023

@Ririio Great feedback. I've added en-CA and made it responsive:

Mobile

Screenshot 2023-02-13 at 8 23 46 AM

Desktop

Screenshot 2023-02-13 at 8 24 33 AM

Copy link
Contributor

@Ririio Ririio left a comment

Choose a reason for hiding this comment

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

Looks great to me

Copy link
Contributor

@Myrfion Myrfion left a comment

Choose a reason for hiding this comment

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

LGTM!

@humphd
Copy link
Contributor Author

humphd commented Feb 13, 2023

@Ririio I can't merge this until you accept the invite to the new team and get write access.

@humphd humphd merged commit 970be82 into DevelopingSpace:main Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: front end Front end part of our web service enhancement Improving an existing solution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants