-
Notifications
You must be signed in to change notification settings - Fork 2
[#143] 마이페이지 스켈레톤 UI 적용 #216
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
junye0l
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.
스켈레톤 UI 작업 수고하셨습니다!
하단 코멘트 확인 후 답변 부탁드립니다 🙇🏻
| import Loader from "@/components/loader/loader"; | ||
| import CardSkeleton from "@/components/card/card-skeleton"; | ||
|
|
||
| const WINE_ITEM_CONTAINER = cn( |
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.
별거 아니지만 모바일 -> 태블릿 -> 데스크탑 순서로 바꾸시면 연수님이 작성하신 다른 파일들과 동일해서 좋을 것 같아요!
| </div> | ||
|
|
||
| {/* WineTaste 컴포넌트 */} | ||
| <div className="flex w-full flex-col gap-3 tablet:grid tablet:grid-cols-2 tablet:gap-4 pc:grid pc:grid-cols-2 pc:gap-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.
이 부분도 cn으로 반응형에 따라 정리가 가능할 것 같습니다!
wlrnjs
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.
수고하셨습니다 코멘트 확인해주세요!
| if (skeleton) { | ||
| return <ReviewItemSkeleton />; | ||
| } | ||
|
|
||
| if (!review) return null; |
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.
위에서는 if 문에 블록({})을 사용했는데, 아래에서는 생략되어 있네요.
일관된 코드 스타일을 유지하려면 둘 다 동일하게 처리하는 게 좋을 것 같습니다.
| )} | ||
| > | ||
| <div className="flex w-full flex-col items-start justify-center gap-8 px-[14px]"> | ||
| <div className="flex w-full flex-col items-center justify-center gap-[26px]"> |
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.
flex flex-col items-center justify-center 부분은 globals.css에 정의된 flex-col-center로 대체해도 좋을 것 같습니다.
사소한 부분이라 수정하지 않으셔도 괜찮습니다!
| <div className="relative h-[80px] w-[60px]"> | ||
| <Image | ||
| src="/images/placeholder/img-wine.svg" | ||
| alt="wine placeholder" |
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.
스켈레톤 상태에서는 alt=""로 비워두는 것이 접근성 측면에서 더 적절합니다.
- 실제 콘텐츠가 아니라 시각적인 위치나 형태만 보여주는 용도이기 때문입니다.
- alt=""를 사용하면 스크린 리더가 해당 이미지를 건너뛰어 불필요하게 파일명을 읽는 문제를 방지할 수 있습니다.
- wine placeholder라는 텍스트를 읽는 것은 사용자 입장에서 오히려 혼란을 줄 수 있습니다.
스켈레톤 이미지나 placeholder 이미지는 alt를 비워두는 방식을 많이 사용합니다.
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.
웹 콘텐츠 접근성 지침 에서 alt 검색하면 나오는 내용입니다!
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.
앗 네! 말씀해주신 부분은 인지하고 있는 내용인데 놓치고 있었습니다 체크해주셔서 감사합니다!!
📄 PR 내용 요약
마이페이지에도 스켈레톤 UI을 적용했습니다.
✅ 작업 내용 상세
게시물이 없을 시에도 로더 대신 각 게시물에 맞는 스켈레톤 UI를 보여지도록 적용
기타
📸 스크린샷 (선택사항)
💬 참고 사항