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

A1 sign in UI #28

Merged
merged 5 commits into from
Feb 13, 2024
Merged

A1 sign in UI #28

merged 5 commits into from
Feb 13, 2024

Conversation

SachiGoto
Copy link
Contributor

Description

Added styling to the sign-in page.

In the Figma design, there is a 'Forgot your password?' link, but since we haven't got the functionality done, I didn't include that link on the page.

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Check List

URL

http://localhost:3000/signin

Screenshot

Screenshot 2024-02-01 at 17 58 11 Desktop Screenshot 2024-02-01 at 17 58 22 Mobile

Copy link
Contributor

@samuraikun samuraikun left a comment

Choose a reason for hiding this comment

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

LGTM 🍎

}}
color="black"
>
<AbsoluteCenter>
Copy link
Contributor

Choose a reason for hiding this comment

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

For PC, it is center, but for SP it is not the center. For example, the iPhone 12 Pro is too below (because it is center). Could you adjust with padding for SP? Padding-top should be 40px.
Screenshot 2024-02-04 at 11 16 30 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added top 40px on mobile size!

>
<AbsoluteCenter>
<Card
bg="#F8F8F8"
Copy link
Contributor

Choose a reason for hiding this comment

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

Use variable theme colour?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed to theme color variable 'white' !

</Flex>
</Box>
</>
<Box
Copy link
Contributor

Choose a reason for hiding this comment

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

I personally think it's better to make a component for Fom because page.tsx's main job is routing. I usually don't put lots of jsx in page files for easier maintenance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I actually made three components and imported them into the page.tsx like this below. Let me know what you think.

<Card
          position="absolute"
          top={{ base: '0%', md: '50%' }}
          left="50%"
          transform={{
            base: 'translate(-50%, 0%)',
            md: 'translate(-50%, -50%)'
          }}
          bg="white"
          boxShadow={{ base: '0', md: '0 4px 16px rgba(0, 0, 0, 0.1)' }}
          maxW={{ base: '100%', md: '542px' }}
          py="40px"
          px={{ base: '16px', md: '80px' }}
        >
          <SignInHeader />
          <SignInForm />
          <SignInFooter />
 </Card>

</FormControl>
<FormControl isInvalid={!!errors.password}>
<FormLabel>Password</FormLabel>
<InputForm type="password" {...register('password')} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the icon to show or hide the password like the Figma?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added it!

} from '@chakra-ui/react'
import Link from 'next/link'

Copy link
Contributor

Choose a reason for hiding this comment

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

Delete space?


import { InputForm } from '@/components/input'
import { Link } from '@/components/link'
import logo from 'public/logo/logo.png'
Copy link
Contributor

Choose a reason for hiding this comment

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

Need space under this?

Comment on lines 123 to 132
<FormControl isInvalid={!!errors.email}>
<FormLabel>Email</FormLabel>
<InputForm
{...register('email')}
minW={{ base: '300px', md: '380px' }}
/>
{errors.email && (
<FormErrorMessage>{errors.email.message}</FormErrorMessage>
)}
</FormControl>
Copy link
Contributor

Choose a reason for hiding this comment

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

If you use darkmode in this app, you see like this. Could you fix this? This auth page does not care dark mode or light mode and same style!
Screenshot 2024-02-04 at 11 15 00 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for catching that!
I fixed it and now it looks the same whether you selected dark mode or light mode :)
All I had to do was to wrap the component in Tag! I had to override background and text color for autofill but it looks good now.

 const signInInputStyle = {
    'input:-webkit-autofill, input:-webkit-autofill:focus': {
      boxShadow: '0 0 0 1000px #E8F0FE inset',
      textFillColor: 'black'
    }
  }

@SachiGoto SachiGoto merged commit 3c48398 into main Feb 13, 2024
2 checks passed
@SachiGoto SachiGoto deleted the A1_sign_in_UI branch February 13, 2024 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants