-
Notifications
You must be signed in to change notification settings - Fork 4
✨ Feat: Footer 컴포넌트 구현 #24
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
|
@BaeZzi813 is attempting to deploy a commit to the projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
sohyun0
left a comment
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.
고생 많으셨습니다 ! 코멘트 확인부탁드립니다 ~ :)
| import envelopeIcon from '@/assets/icon/ic-envelope.svg'; | ||
| import facebookIcon from '@/assets/icon/ic-facebook.svg'; | ||
| import instagramIcon from '@/assets/icon/ic-instagram.svg'; |
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.
Icon 으로 이미 import 되어 있어서 Icon에서 대체해서 사용하면 될것 같습니다 ! (아래 설명 넣어놓겠습니다)
| <a href='https://mail.google.com/' target='_blank'> | ||
| <Image src={envelopeIcon} alt='메일' width={25} height={25} /> | ||
| </a> | ||
| </li> | ||
| <li className='hover:-translate-y-1'> | ||
| <a href='https://www.facebook.com/' target='_blank'> | ||
| <Image src={facebookIcon} alt='메일' width={25} height={25} /> | ||
| </a> | ||
| </li> | ||
| <li className='hover:-translate-y-1'> | ||
| <a href='https://www.instagram.com/' target='_blank'> | ||
| <Image src={instagramIcon} alt='인스타그램' width={25} height={25} /> | ||
| </a> |
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.
외부연결도 Link 컴포넌트 사용해도 괜찮습니다 !
<Link href="https://www.naver.com" target="_blank">
<Icon iconName='facebook' ariaLabel='페이스북' className='bg-gray-400' />
<Icon iconName='instagram' ariaLabel='인스타그램' />
<Icon iconName='envelope' ariaLabel='메일' />
</Link>
3개의 아이콘도 이미 지정되어 있어서 이런식으로 사용하시면 됩니다 !
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.
Icon 만들어주신 걸 미처 못봤습니다.ㅠㅠ 변경했습니다!
| <Container as={'footer'} className='max-w-full bg-gray-100'> | ||
| <div className='mx-auto flex flex-wrap justify-between gap-10 px-5 py-8 desktop:w-[964px]'> |
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.
Container as={'footer'} 로 사용하시는것 보단 서로 위치를 변경하는것을 제안드립니다!
<footer>
<Container></Container>
</footer>
현재 컨테이너 속성이 전부 중복되어서 하단에 나타나고 있기 때문에 컨테이너의 컴포넌트 의미가 사라지고 하단에 새로 정의되고 있어서 구조는 변경하는게 좋을것 같습니다!
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.
그러네요 as 로 선언하면 footer 랑 Container 같이 들어갔겠네요..
구조 변경 해놓았습니다!
| <Link href={'/'} className='text-body-s hover:text-gray-300 tablet:text-body-m'> | ||
| Privacy Policy | ||
| </Link> | ||
| <Link href={'/'} className='text-body-s hover:text-gray-300 tablet:text-body-m'> |
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.
hover 했을때 컬러가 과하게 밝아져서 이부분은 컬러셋을 추가해서 진한컬러로 변경하는게 좋을것 같습니다!
해상도 저하 모니터, 유저의 시력저하 같은 접근성 에는 배경컬러와 텍스트 컬러의 대비가 어느정도 높아야하는 점이 있어서 이부분은 제가 진한 컬러셋 한번 찾아보겠습니다 :)
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.
네 기본 black 으로 하려다가 너무 어두워서 고민했는데 다른 괜찮다고 생각하시는 컬러 있으시면 바로 수정하겠습니다!
확인 후 머지 부탁드려도 될까요~?
이미지 Icon 컴포넌트 사용 / Link 적용 / 구조 변경
sohyun0
left a comment
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.
고생하셨습니다 :) 참고하실만한 코멘트 하나 남겨놓았습니다!
| <> | ||
| <section className='flex flex-col gap-4 px-3 py-10 tablet:px-8 tablet:py-[60px] desktop:px-[237px] desktop:py-[60px]'> | ||
| <h1 className='text-xl font-bold tablet:text-[28px]'>{title}</h1> | ||
| <h1 className='text-heading-s font-bold tablet:text-heading-l'>{title}</h1> |
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.
하나더 알려드리자면 text-heading-l 만 선언해도 반응형으로 만들어졌기 때문에 640 이하에서는 20px , 640 이상에서는 28로 보여지게 됩니다 :) global.css 참고해보시면 좋을것 같습니다~!
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.
아 그러네요! 꼼꼼히 읽어보겠습니다ㅠ 이 부분도 다시 수정해 놓을게요~!
📝 작업 개요 (필수)
공통 컴포넌트 Footer 설계
관리자 도구로 모바일 사이즈 시 구조변경도 해놨습니다.
SNS 경우 배열에 담고 map 써서 리스트 돌리려고 했는데 전역으로 3가지만 고정되게 사용될 거 같아서 베이직하게 만들었습니다.
SNS 연결은 내부 페이지 아니라 a 태그로 연결했고,
호버 시 구분 쉽게 컬러변경 및 애니메이션 살짝 넣어봤습니다.
추가로, 폰트사이즈 선언 스타일로 수정했습니다.
✨ 작업 내용 (필수)
📸 스크린샷
🧐 해결해야 하는 문제
🤔 리뷰어 확인 필요 사항
🔗 관련 이슈
🛠️ 후속 작업
✅ 체크리스트 (필수)