[Feat] Spinner 컴포넌트 추가 #69
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/web/src/app/page.tsxOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@repo/eslint-config' imported from /eslint.config.mjs 워크스루이 풀 리퀘스트는 새로운 변경 사항
관련 가능성 있는 PR
제안된 레이블
제안된 리뷰어
토끼의 시
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/components/Spinner/Spinner.tsx (1)
26-37: 성능 최적화를 위한 제안현재 구현은 잘 작동하지만, PR 설명에서 언급된 대로 동적 임포트 이슈가 있습니다. CSS로 구현하는 것이 더 효율적일 것 같습니다.
CSS 기반 구현으로 전환 시, 다음과 같은 이점이 있습니다:
- 번들 크기 감소
- 초기 로딩 성능 향상
- 동적 임포트 제거
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/page.tsx(2 hunks)packages/ui/package.json(1 hunks)packages/ui/src/components/Spinner/Spinner.tsx(1 hunks)
🔇 Additional comments (4)
packages/ui/src/components/Spinner/Spinner.tsx (2)
17-19: Props 타입 정의가 잘 구현되었습니다.타입 안전성을 보장하면서도 필요한 속성만 노출하도록 잘 설계되었습니다.
21-25: JSDoc 문서화가 잘 되어있습니다.한글로 작성된 명확한 문서화가 되어있어 컴포넌트 사용이 용이합니다.
apps/web/src/app/page.tsx (1)
23-28: 동적 임포트 구현이 적절합니다.SSR을 비활성화하고 동적으로 임포트하는 방식이 올바르게 구현되었습니다.
packages/ui/package.json (1)
13-13: 패키지 내보내기가 올바르게 구성되었습니다.Spinner 컴포넌트의 export 경로가 적절하게 설정되었습니다.
apps/web/src/app/page.tsx
Outdated
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| gap: '8px', | ||
| backgroundColor: 'grey', | ||
| }} | ||
| > | ||
| <Spinner color="black" /> | ||
| <Spinner color="white" /> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
스피너 표시를 위한 배경색 개선 필요
현재 배경색이 'grey'로 하드코딩되어 있습니다. 테마 시스템의 색상을 사용하는 것이 좋겠습니다.
다음과 같이 수정을 제안합니다:
- backgroundColor: 'grey',
+ backgroundColor: 'var(--color-grey-900)',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| style={{ | |
| display: 'flex', | |
| gap: '8px', | |
| backgroundColor: 'grey', | |
| }} | |
| > | |
| <Spinner color="black" /> | |
| <Spinner color="white" /> | |
| </div> | |
| <div | |
| style={{ | |
| display: 'flex', | |
| gap: '8px', | |
| backgroundColor: 'var(--color-grey-900)', | |
| }} | |
| > | |
| <Spinner color="black" /> | |
| <Spinner color="white" /> | |
| </div> |
|
|
||
| type LottieType = keyof typeof lotties; | ||
|
|
||
| const SpinnerColor: Record<'black' | 'white', LottieType> = { |
There was a problem hiding this comment.
'black' | 'white' 를 따로 관리하는게 어떨까요? 고민이 되네요!
There was a problem hiding this comment.
색상이 추가될 수도 있으니 따로 빼는 게 좋겠네요!! 수정했어요!
| }; | ||
|
|
||
| export type SpinnerProps = { | ||
| color?: keyof typeof SpinnerColor; |
There was a problem hiding this comment.
그러면 keyof typeof 까지 안해도 되지 않을까.. 근데 뭐가 더 좋은 방법일지 모르겠네요
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/ui/src/components/Spinner/Spinner.tsx (2)
12-17: 상수값을 별도의 파일로 분리하는 것을 제안드립니다.현재 구현은 이전 리뷰 의견을 반영하여 색상 관리를 잘 분리했습니다. 더 나아가 다음과 같은 개선을 제안드립니다:
- 상수값들을
constants.ts파일로 분리하여 재사용성과 유지보수성을 높일 수 있습니다.- 추후 색상이 추가될 때 한 곳에서 관리가 가능해집니다.
+ // constants.ts + export const SPINNER_COLORS = { + black: 'loadingBlack', + white: 'loadingWhite', + } as const; + + export type SpinnerColorType = keyof typeof SPINNER_COLORS;
19-21: 타입 정의를 단순화할 수 있습니다.이전 리뷰 의견처럼
keyof typeof를 사용하지 않고도 타입을 정의할 수 있습니다:- export type SpinnerProps = { - color?: keyof typeof SpinnerColor; - } & Omit<LottieAnimationProps, 'animationData'>; + export type SpinnerProps = { + color?: SpinnerColorType; + } & Omit<LottieAnimationProps, 'animationData'>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/app/page.tsx(2 hunks)packages/ui/src/components/Spinner/Spinner.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app/page.tsx
🔇 Additional comments (1)
packages/ui/src/components/Spinner/Spinner.tsx (1)
1-8: 코드가 깔끔하고 필요한 import만 포함되어 있습니다!클라이언트 사이드 렌더링을 위한 'use client' 지시문과 필요한 import문이 잘 구성되어 있습니다.
관련 이슈
#39
변경 사항
Spinner 컴포넌트 추가했습니다.
lottie를 활용하다 보니 사용 시 매번 dynamic import를 해야 하는 문제가 있어 추후 공통 컴포넌트들 구현이 끝나면 css로 직접 구현해 수정할 예정입니다 ^...^
다른 라이브러리들은 어떻게 동적 임포트 없이 사용하는 건가 싶었는데 다들 직접 구현을 하셨더군요 🗿
접근성을 위해 span 태그의 prop을 받을 수 있도록 했고, 수정 후에는 아래와 같은 형식으로 제공할 예정이에요!
레퍼런스
Summary by CodeRabbit
새로운 기능
개선 사항