-
Notifications
You must be signed in to change notification settings - Fork 0
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
[FEAT] Skeleton 컴포넌트 개발, 스토리북 제작 #26
base: main
Are you sure you want to change the base?
Conversation
Walkthrough이 풀 리퀘스트는 새로운 Skeleton 컴포넌트를 도입하고 있습니다. Skeleton 컴포넌트는 데이터 로딩 중 레이아웃의 플레이스홀더로 사용되며, Storybook을 통해 문서화되었습니다. 컴포넌트는 기본 스타일과 함께 추가 CSS 클래스를 지원하며, 애니메이션 효과를 포함하고 있습니다. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Update: 2025년 01월 31일 08시 33분 02초 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/shared/ui/Skeleton/Skeleton.tsx (1)
10-12
: 접근성 개선을 위한 aria 속성 추가 필요스켈레톤 UI는 로딩 상태를 나타내는 중요한 UI 요소이므로, 스크린 리더 사용자를 위한 접근성 속성을 추가하는 것이 좋습니다.
export function Skeleton({ className, ...props }: Props) { - return <div className={cn('animate-pulse rounded-md bg-gray-200', className)} {...props} />; + return ( + <div + className={cn('animate-pulse rounded-md bg-gray-200', className)} + role="status" + aria-label="로딩 중..." + {...props} + /> + ); }src/shared/ui/Skeleton/Skeleton.stories.tsx (1)
22-33
: 스토리 구현에 대한 개선 제안기본 사용 예시는 잘 구현되어 있으나, 다양한 크기와 사용 사례를 보여주는 추가 스토리가 있으면 좋을 것 같습니다.
다음과 같은 추가 스토리 구현을 제안드립니다:
export const Variants: Story = { render: () => { return ( <div className="space-y-4"> <Skeleton className="h-4 w-3/4" /> <Skeleton className="h-8 w-1/2" /> <Skeleton className="h-32 w-full" /> </div> ); }, }; export const WithCustomAnimation: Story = { args: { className: 'h-8 w-full animate-[pulse_2s_ease-in-out_infinite]' }, };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/shared/ui/Skeleton/Skeleton.stories.tsx
(1 hunks)src/shared/ui/Skeleton/Skeleton.tsx
(1 hunks)src/shared/ui/Skeleton/index.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/shared/ui/Skeleton/index.ts
🔇 Additional comments (2)
src/shared/ui/Skeleton/Skeleton.tsx (1)
3-8
: Props 타입 정의가 명확하고 잘 문서화되어 있습니다.컴포넌트의 Props 타입이 잘 정의되어 있으며, className prop에 대한 JSDoc 문서화도 적절합니다.
src/shared/ui/Skeleton/Skeleton.stories.tsx (1)
5-17
: 스토리북 메타데이터가 잘 구성되어 있습니다.컴포넌트 설명이 한글로 명확하게 작성되어 있고, autodocs 태그를 통한 자동 문서화 설정이 적절합니다.
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.
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.
저도 동일하게 className으로 사용하는 것이 좋네요
수고하셨습니다 ~~
형태가 고정되어져있고,개수만 차이를 갖는다면 말씀하신 것처럼 일괄로 처리를 하는게 더 좋아보여요. |
🔥 연관 이슈
🚀 작업 내용
🤔 고민했던 내용
속성 제거
반응형을 고려하면서
1. tailwind는 동적 className생성을 권장하지않는다
2. 우리 서비스는 일반적으로 반응형 처리를 요구하므로, 명시적 작성으로 누릴 효과가 적다.
3. 각 속성을 유형화하기 어렵다.
폴더 구조
픽스존.skeleton.tsx
)을 어디에 위치시키면 좋을 지, 다음 회의 때 구두로 논의하면 좋을 것 같아요기본 값
width
,height
)에 대한 기본값을 설정하지 않았어요.💬 리뷰 중점사항
Summary by CodeRabbit
새로운 기능
문서화