-
Notifications
You must be signed in to change notification settings - Fork 0
[DESIGN] 로그인페이지 ui 수정사항 반영 #27
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
Conversation
✅ Deploy Preview for umc-nect ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
94d8e53 to
204bb19
Compare
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.
Pull request overview
Copilot reviewed 12 out of 23 changed files in this pull request and generated 15 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {(errors.email || loginError) && ( | ||
| <div className='flex items-center mb-3'> | ||
| <CheckIcon className='w-3 h-3 text-danger-700 mr-1' /> | ||
| <p className='body-2 text-danger-700'>{errors.email.message}</p> | ||
| <p className='body-2 text-danger-700'>{errors.email?.message || loginError}</p> | ||
| </div> | ||
| )} |
Copilot
AI
Jan 17, 2026
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 error display logic only shows email validation errors and ignores password validation errors. If a password error exists (e.g., empty password since min(1) is now required), it won't be displayed to the user. The condition should check for both errors.email and errors.password to provide complete validation feedback.
| auth: 'title-2 bg-primary-500-normal text-neutral-000 font-medium disabled:font-normal disabled:bg-primary-200-light', | ||
| socialLogin: | ||
| 'w-full h-14 title-2 text-neutral-900 py-3.5 border border-neutral-700 flex justify-center items-center gap-2.5 hover:bg-neutral-200 duration-200 ease-in-out', | ||
| 'w-full h-14 title-2 text-neutral-900 py-3.5 border border-neutral-300 rounded-12 flex justify-center items-center gap-3', |
Copilot
AI
Jan 17, 2026
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 hover state for social login buttons has been removed. The previous implementation had hover:bg-neutral-200 duration-200 ease-in-out which provided visual feedback. Removing hover effects makes the buttons feel less interactive. Consider adding a hover state that's appropriate for the new design (e.g., opacity change, slight color shift, or shadow effect).
| 'w-full h-14 title-2 text-neutral-900 py-3.5 border border-neutral-300 rounded-12 flex justify-center items-center gap-3', | |
| 'w-full h-14 title-2 text-neutral-900 py-3.5 border border-neutral-300 rounded-12 flex justify-center items-center gap-3 hover:bg-neutral-50', |
| </Button> | ||
|
|
||
| <Button color='socialLogin' fullWidth> | ||
| <Button color='socialLogin' fullWidth className='bg-social-kakao border-none'> |
Copilot
AI
Jan 17, 2026
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 Kakao login button applies 'bg-social-kakao border-none' as className props, but the socialLogin button style already includes 'border border-neutral-300'. When applying 'border-none' through className, it should override the border, but there might be specificity issues. Consider refactoring the Button component to accept a variant prop for different social login providers rather than overriding styles through className, which is more maintainable and follows component design best practices.
| <Button color='socialLogin' fullWidth className='bg-social-kakao border-none'> | |
| <Button color='socialLogin' fullWidth className='bg-social-kakao'> |
|
|
||
| return ( | ||
| <div className=' w-[50%] mb-6.5'> | ||
| <div className=' w-105 mb-6.5'> |
Copilot
AI
Jan 17, 2026
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 class 'w-105' uses a non-standard Tailwind width value. Tailwind's default spacing scale goes up to 96 (384px), and '105' is not a standard value. This will likely not work unless a custom width has been defined in the Tailwind config. Consider using standard Tailwind width values like 'w-96' (24rem / 384px) or a custom arbitrary value like 'w-[420px]' if a specific pixel width is required, or define this custom value in your Tailwind configuration.
| <div className=' w-105 mb-6.5'> | |
| <div className=' w-[420px] mb-6.5'> |
| if (email === 'test@naver.com' && password === 'qwerty1!') { | ||
| console.log('로그인 성공!', email, password) | ||
| navigate('/') |
Copilot
AI
Jan 17, 2026
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.
Hardcoded test credentials should not be committed to the repository, even with a comment indicating they are temporary. This is a security risk as it exposes valid test credentials that could be used maliciously. Consider using environment variables or a separate configuration file that is not committed to version control for test credentials.
| getValues, | ||
| } | ||
| } | ||
|
|
Copilot
AI
Jan 17, 2026
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 getValues function is exported from the hook but is never used anywhere in the consuming component (AuthForm). This adds unnecessary API surface. Remove the getValues from the return object if it's not needed, or remove this comment if it will be used in future implementations.
| getValues, | |
| } | |
| } | |
| } | |
| } |
|
|
||
| const colors = { | ||
| auth: 'title-2 bg-primary-500-normal text-neutral-000 disabled:bg-primary-150-light disabled:text-primary-300-light hover:bg-primary-600-normal duration-300 ease-in-out', | ||
| auth: 'title-2 bg-primary-500-normal text-neutral-000 font-medium disabled:font-normal disabled:bg-primary-200-light', |
Copilot
AI
Jan 17, 2026
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 hover state has been removed from the auth button styling. Previously it had hover:bg-primary-600-normal which provided visual feedback on hover. Removing hover effects can negatively impact user experience by making it unclear whether the button is interactive. Consider re-adding hover state styling for better UX.
| auth: 'title-2 bg-primary-500-normal text-neutral-000 font-medium disabled:font-normal disabled:bg-primary-200-light', | |
| auth: 'title-2 bg-primary-500-normal text-neutral-000 font-medium hover:bg-primary-600-normal disabled:font-normal disabled:bg-primary-200-light', |
| const LoginPage = () => { | ||
| return ( | ||
| <div className=' min-h-screen flex flex-col justify-center items-center'> | ||
| <div className=' min-h-screen flex flex-col pt-38.75 items-center'> |
Copilot
AI
Jan 17, 2026
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 class 'pt-38.75' uses a non-standard Tailwind spacing value. Tailwind CSS uses a spacing scale based on 0.25rem (4px) increments, and '38.75' doesn't correspond to a standard spacing value in Tailwind's default configuration. This will likely not work unless a custom spacing value has been defined in the Tailwind config. Consider using standard Tailwind spacing values like 'pt-40' (10rem / 160px) or 'pt-36' (9rem / 144px), or define this custom value in your Tailwind configuration if 155px (38.75 * 4px) is required.
| <div className=' min-h-screen flex flex-col pt-38.75 items-center'> | |
| <div className=' min-h-screen flex flex-col pt-40 items-center'> |
|
|
||
| {/* 소셜 로그인 */} | ||
| <div className='w-[50%] flex flex-col gap-3'> | ||
| <div className='w-105 flex flex-col gap-3'> |
Copilot
AI
Jan 17, 2026
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 class 'w-105' uses a non-standard Tailwind width value. Tailwind's default spacing scale goes up to 96 (384px), and '105' is not a standard value. This will likely not work unless a custom width has been defined in the Tailwind config. Consider using standard Tailwind width values like 'w-96' (24rem / 384px) or a custom arbitrary value like 'w-[420px]' if a specific pixel width is required, or define this custom value in your Tailwind configuration.
| <div className='w-105 flex flex-col gap-3'> | |
| <div className='w-[420px] flex flex-col gap-3'> |
|
|
||
| const baseStyles = { | ||
| auth: 'w-full h-14 px-4 py-3.5 rounded-10 border-[1.5px] border-neutral-200 bg-neutral-100 focus:border-primary-500-normal focus:outline-none title-2 placeholder:text-neutral-300', | ||
| auth: 'w-full h-14 px-4 py-3.5 rounded-[12px] border-[1.5px] border-neutral-200 text-neutral-900 bg-neutral-100 focus:border-primary-500-normal focus:outline-none title-2 placeholder:text-neutral-300', |
Copilot
AI
Jan 17, 2026
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 border-radius has been changed from 'rounded-10' to 'rounded-[12px]'. The 'rounded-10' was likely using a custom radius value from the CSS theme (--radius-10: 10px), while 'rounded-[12px]' is an arbitrary value. For consistency with the design system defined in index.css which includes radius values (--radius-10, --radius-12, etc.), consider using 'rounded-12' instead of the arbitrary value 'rounded-[12px]' if a 12px radius is desired and defined in the theme.
| auth: 'w-full h-14 px-4 py-3.5 rounded-[12px] border-[1.5px] border-neutral-200 text-neutral-900 bg-neutral-100 focus:border-primary-500-normal focus:outline-none title-2 placeholder:text-neutral-300', | |
| auth: 'w-full h-14 px-4 py-3.5 rounded-12 border-[1.5px] border-neutral-200 text-neutral-900 bg-neutral-100 focus:border-primary-500-normal focus:outline-none title-2 placeholder:text-neutral-300', |
[DESIGN] 로그인페이지 ui 수정사항 반영
🎯 Issue
#24
📝 변경 내용
로그인 페이지에서의 디자인 변경사항을 반영하였습니다.
✅ 변경 사항
📷 스크린샷
issue.24.mov
📋 체크리스트