-
Notifications
You must be signed in to change notification settings - Fork 4
✨ Feat: 토스트 컴포넌트 구현 #15
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
토스트 컴포넌트 구현 codeit-FE18-part3#13
|
@BaeZzi813 is attempting to deploy a commit to the projects Team on Vercel. A member of the Team first needs to authorize it. |
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.
고생 많으셨습니다 ! 코멘트 확인부탁드립니다
| return ( | ||
| <> | ||
| <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 text-black tablet:text-[28px]'>{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-black 의 경우에는 global.css 에서 선언하고 있어서 이부분은 제외해도 좋을것 같습니다 :)
| const Frame = ({ title, content }: FrameProps) => { | ||
| return ( | ||
| <> | ||
| <section className='flex flex-col gap-4 px-3 py-10 tablet:px-8 tablet:py-[60px] desktop:px-[237px] desktop:py-[60px]'> |
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.
desktop:px-[237px] 이부분은 container로 만들면 될것같습니다 !
container 아예 유동적으로 사용할 수 있도록 조금 변경해둘게요!
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 text-black tablet:text-[28px]'>{title}</h1> | ||
| <div className='flex flex-col items-center justify-center gap-4 rounded-xl border border-solid border-gray-200 px-6 py-[60px]'> | ||
| <h2 className='text-sm font-normal text-black tablet:text-base'>{content}</h2> |
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.
여기 텍스트도 동일하게 기본 컬러는 선언안해주셔도 됩니다 :)
| {toastRoot && | ||
| createPortal( | ||
| message ? ( | ||
| <div className='fixed bottom-28 right-[38%] rounded-[5px] bg-red-300 px-4 py-[10px] text-white tablet:right-2/4'> |
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.
토스트는 하단 중앙이라고 알고있는데 혹시 38% 로 설정하신 이유가 있으실까요!?
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.
또한 접근성을 고려한다면 role="alert" 를 넣을수도 있을것 같아요 (추천)
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.
left-1/2 -translate-x-1/2 으로 설정해서 모바일에서도 바로 적용되게 수정했습니다! role 도 넣어놨어요!
|
|
||
| const showToast = (msg: string) => { | ||
| setMessage(msg); | ||
| setTimeout(() => setMessage(null), 3000); |
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.
setTimeout 사용후 unmount 될때 메모리 누수 발생할 수 있을것 같아요!
만약 토스트가 연속적으로 (현재는 그럴일은 없겠지만) 호출된다면 타임아웃이 꼬일 수도 있구요!
useEffect 에 clearTimeout 넣어두면 조금더 안전하게 작동될것 같습니다!
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.
useEffect 안으로 마운트시에만 작동하게 하고, 정리함수로 clearTimeout 적용했습니다~!
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.
고생하셨습니다 :)
| <h1 className='text-xl font-bold tablet:text-[28px]'>{title}</h1> | ||
| <div className='flex flex-col items-center justify-center gap-4 rounded-xl border border-solid border-gray-200 px-6 py-[60px]'> | ||
| <h2 className='text-sm font-normal tablet:text-base'>{content}</h2> |
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.
이부분 제가 미쳐 못봤었는데 font-size 관련 tailwind config 에 전부 등록되어있습니다 !
스토리북에서도 확인가능합니다 :)
text-heading-l = 28px 이런식으로 작성하시면 됩니다!
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
토스트 컴포넌트 구현 #13
📝 작업 개요 (필수)
토스트 기능을 전역적으로 Context 설정하여 전체 페이지에서 쓸 수 있도록 설계
✨ 작업 내용 (필수)
📸 스크린샷
🧐 해결해야 하는 문제
🤔 리뷰어 확인 필요 사항
직접 테스트 페이지를 만들어서 테스트 해보았는데
사용시에는
위와 같이 사용하시면 될 것 같습니다.
🔗 관련 이슈
🛠️ 후속 작업
✅ 체크리스트 (필수)