-
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] Icon 컴포넌트, 스토리북 제작 #14
Conversation
워크스루이 풀 리퀘스트는 Next.js 애플리케이션에 아이콘 컴포넌트를 도입하는 변경 사항을 포함하고 있습니다. SVG 파일을 React 컴포넌트로 가져올 수 있도록 Webpack 설정을 수정하고, 새로운 변경 사항
연결된 이슈 평가
관련 가능성 있는 PR
제안된 리뷰어
✨ Finishing Touches
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
|
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: 2
🧹 Nitpick comments (5)
src/shared/ui/Icon/Icon.tsx (1)
3-19
: Props 타입 정의가 잘 되어있습니다만, 몇 가지 개선사항이 있습니다.
- size prop에 대한 단위 변환 설명이 한글로 되어있는데, 영문으로 통일하는 것이 좋습니다.
- color와 size의 타입에 대한 제약조건을 추가하면 좋을 것 같습니다.
다음과 같이 개선해보세요:
type Props = { /** * icon name to be displayed. */ name: IconName; /** * color of the icon. * @default 'gray' */ color?: IconColor; /** * size of the icon. - * @description px 단위로 변환합니다. + * @description converts to px unit * @default 25 + * @minimum 0 */ size?: number; } & React.SVGProps<SVGSVGElement>;src/shared/ui/Icon/Icon.stories.tsx (1)
16-37
: 스토리북 구성이 좋습니다만, 몇 가지 제안사항이 있습니다.
- 기본 스토리 외에도 다양한 사용 사례를 보여주는 추가 스토리가 있으면 좋을 것 같습니다.
- 아이콘 그리드 레이아웃에 아이콘 이름을 표시하면 사용자가 더 쉽게 식별할 수 있을 것 같습니다.
아이콘 이름을 표시하도록 수정해보세요:
<div className="flex flex-row flex-wrap gap-2"> {iconNames.map((icon) => ( - <div className="flex-shrink-0" key={icon}> + <div className="flex-shrink-0 flex flex-col items-center" key={icon}> <Icon {...args} name={icon} /> + <span className="text-xs mt-1">{icon}</span> </div> ))} </div>추가로 다음과 같은 스토리를 고려해보세요:
- 다양한 크기의 아이콘을 나란히 보여주는 스토리
- 다양한 색상의 아이콘을 나란히 보여주는 스토리
- 실제 사용 사례를 보여주는 스토리 (예: 버튼 내부의 아이콘)
src/shared/ui/assets/index.tsx (3)
18-34
: Icons 객체의 구조화를 개선해주세요.현재 Icons 객체가 평면적으로 구성되어 있습니다. 아이콘의 성격에 따라 그룹화하면 관리와 사용이 더 용이할 것 같습니다.
예시:
export const Icons = { actions: { add: Add, close: Close, write: Write, trash: Trash, }, navigation: { arrowDown: ArrowDown, arrowUp: ArrowUp, list: List, }, status: { new: New, check: Check, pin: Pin, }, social: { instagram: Insta, youtube: Youtube, }, };🧰 Tools
🪛 GitHub Actions: Preview Storybook
[error] Module not found: Error: Can't resolve '../Colors/colors'
36-42
: 색상 관리 방식을 개선해주세요.현재 COLORS 객체가 특정 색상 값만 추출하고 있습니다. 테마 시스템의 확장성을 고려하여 더 유연한 구조로 개선하면 좋을 것 같습니다.
예시:
export const COLORS = { primary: { default: colors['primary'][300], hover: colors['primary'][400], active: colors['primary'][500], }, gray: { default: colors['gray'][300], hover: colors['gray'][400], active: colors['gray'][500], }, // ... 다른 색상들 } as const;🧰 Tools
🪛 GitHub Actions: Preview Storybook
[error] Module not found: Error: Can't resolve '../Colors/colors'
44-46
: 타입 정의를 개선해주세요.Icons 객체 구조 변경 시 타입도 함께 업데이트가 필요합니다. 또한, 색상 타입도 더 구체적으로 정의하면 좋을 것 같습니다.
type IconCategory = keyof typeof Icons; type IconName<T extends IconCategory> = keyof typeof Icons[T]; type IconColor = keyof typeof COLORS; type ColorVariant = keyof typeof COLORS[keyof typeof COLORS];🧰 Tools
🪛 GitHub Actions: Preview Storybook
[error] Module not found: Error: Can't resolve '../Colors/colors'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (15)
package-lock.json
is excluded by!**/package-lock.json
src/shared/ui/assets/add.svg
is excluded by!**/*.svg
src/shared/ui/assets/arrow-down.svg
is excluded by!**/*.svg
src/shared/ui/assets/arrow-up.svg
is excluded by!**/*.svg
src/shared/ui/assets/check.svg
is excluded by!**/*.svg
src/shared/ui/assets/close.svg
is excluded by!**/*.svg
src/shared/ui/assets/download.svg
is excluded by!**/*.svg
src/shared/ui/assets/etc.svg
is excluded by!**/*.svg
src/shared/ui/assets/instagram.svg
is excluded by!**/*.svg
src/shared/ui/assets/list.svg
is excluded by!**/*.svg
src/shared/ui/assets/new.svg
is excluded by!**/*.svg
src/shared/ui/assets/pin.svg
is excluded by!**/*.svg
src/shared/ui/assets/trash.svg
is excluded by!**/*.svg
src/shared/ui/assets/write.svg
is excluded by!**/*.svg
src/shared/ui/assets/youtube.svg
is excluded by!**/*.svg
📒 Files selected for processing (10)
next.config.ts
(1 hunks)package.json
(2 hunks)src/shared/ui/Icon/Icon.stories.tsx
(1 hunks)src/shared/ui/Icon/Icon.tsx
(1 hunks)src/shared/ui/Icon/index.ts
(1 hunks)src/shared/ui/assets/index.tsx
(1 hunks)src/shared/ui/colors/colors.ts
(0 hunks)svg.d.ts
(1 hunks)tailwind.config.ts
(1 hunks)tsconfig.json
(1 hunks)
💤 Files with no reviewable changes (1)
- src/shared/ui/colors/colors.ts
✅ Files skipped from review due to trivial changes (2)
- src/shared/ui/Icon/index.ts
- tailwind.config.ts
🧰 Additional context used
🪛 GitHub Actions: Preview Storybook
src/shared/ui/assets/index.tsx
[error] Module not found: Error: Can't resolve '../Colors/colors'
🔇 Additional comments (5)
svg.d.ts (1)
1-4
: SVG 타입 선언이 적절합니다.SVG 파일을 React 컴포넌트로 임포트할 수 있도록 타입이 올바르게 정의되어 있습니다.
next.config.ts (2)
6-6
: React Strict Mode를 비활성화한 이유를 검토해주세요.Strict Mode는 개발 과정에서 잠재적인 문제를 발견하는데 도움이 됩니다. 비활성화가 꼭 필요한 상황이 아니라면, 활성화 상태를 유지하는 것이 좋습니다.
7-14
: 웹팩 설정이 적절합니다.SVG 처리를 위한 @svgr/webpack 설정과 경로 별칭 설정이 올바르게 되어있습니다.
src/shared/ui/Icon/Icon.tsx (1)
21-25
: Icon 컴포넌트 구현이 깔끔합니다.크기와 색상 처리가 적절하게 구현되어 있습니다. SVG 속성도 올바르게 전달되고 있습니다.
tsconfig.json (1)
21-26
: 설정이 적절해 보입니다!SVG 파일을 React 컴포넌트로 사용하기 위한 TypeScript 설정이 잘 되어있습니다.
Update: 2025년 01월 22일 20시 31분 33초 |
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
next.config.ts
(1 hunks)src/shared/lib/colors.ts
(0 hunks)src/shared/ui/assets/index.tsx
(1 hunks)src/shared/ui/colors/Color.mdx
(1 hunks)tailwind.config.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- src/shared/lib/colors.ts
✅ Files skipped from review due to trivial changes (1)
- src/shared/ui/colors/Color.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- tailwind.config.ts
- src/shared/ui/assets/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: preview-storybook
🔇 Additional comments (1)
next.config.ts (1)
5-11
: 🛠️ Refactor suggestionSVG 설정이 PR 요구사항과 일치하는지 확인이 필요합니다.
현재 설정은 SVG를 React 컴포넌트로 변환하는 기본적인 처리만 수행합니다. PR 요구사항에 명시된 width/height="current"와 fill/stroke="currentColor" 설정이 기본값으로 적용되는지 확인이 필요합니다.
다음과 같이 SVGR 설정을 추가하는 것을 고려해보세요:
webpack(config) { config.module.rules.push({ test: /\.svg$/, - use: ['@svgr/webpack'], + use: [{ + loader: '@svgr/webpack', + options: { + svgoConfig: { + plugins: [{ + name: 'preset-default', + params: { + overrides: { + removeViewBox: false + } + } + }] + }, + svgProps: { + width: 'current', + height: 'current', + fill: 'currentColor' + } + } + }], }); return config; },
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.
수고하셨습니다 👍
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 (4)
src/shared/ui/Icon/Icon.tsx (2)
5-21
: Props 타입 정의에 대한 개선사항이 있습니다.Props 타입 정의가 잘 되어있지만, 몇 가지 개선할 수 있는 부분이 있습니다:
- size prop에 대한 유효성 검사가 없습니다 (음수 값 처리)
- aria-label prop이 필수가 아닙니다
다음과 같이 수정하는 것을 제안합니다:
type Props = { /** * icon name to be displayed. */ name: IconName; /** * color of the icon. * @default 'gray' */ color?: IconColor; /** * size of the icon. * @description px 단위로 변환합니다. * @default 25 + * @min 0 */ size?: number; + /** + * aria-label for accessibility + */ + 'aria-label': string; } & React.SVGProps<SVGSVGElement>;
23-27
: size prop 유효성 검사와 접근성 개선이 필요합니다.컴포넌트 구현에서 다음 사항들을 개선하면 좋을 것 같습니다:
- size prop의 음수 값 처리
- 접근성을 위한 role="img" 추가
다음과 같이 수정하는 것을 제안합니다:
-export function Icon({ name, color = 'gray', size = 25, ...props }: Props) { +export function Icon({ name, color = 'gray', size = 25, 'aria-label': ariaLabel, ...props }: Props) { const SVGIcon = Icons[name]; + const validSize = Math.max(0, size); - return <SVGIcon width={`${size}px`} height={`${size}px`} color={COLORS[color]} {...props} />; + return ( + <SVGIcon + width={`${validSize}px`} + height={`${validSize}px`} + color={COLORS[color]} + role="img" + aria-label={ariaLabel} + {...props} + /> + ); }src/shared/ui/assets/index.tsx (1)
16-32
: 아이콘 네이밍과 그룹화에 대한 개선사항이 있습니다.Icons 객체의 구조와 네이밍에 대해 몇 가지 개선할 점이 있습니다:
- 일부 아이콘 이름이 일관성이 없습니다 (예: DownLoad vs download)
- SNS 관련 아이콘들이 별도로 그룹화되어 있지 않습니다
다음과 같이 수정하는 것을 제안합니다:
export const Icons = { add: Add, arrowDown: ArrowDown, arrowUp: ArrowUp, check: Check, close: Close, - download: DownLoad, + download: Download, etc: Etc, list: List, new: New, pin: Pin, trash: Trash, write: Write, - - instagram: Insta, - youtube: Youtube, + // Social Media Icons + social: { + instagram: Insta, + youtube: Youtube, + }, };이에 따라
IconName
타입도 수정이 필요할 수 있습니다.src/shared/lib/colors.ts (1)
41-46
: COLORS 객체의 문서화와 일관성 개선이 필요합니다.COLORS 객체 정의에 대해 몇 가지 개선할 점이 있습니다:
- 각 색상의 용도나 사용 맥락에 대한 문서화가 없습니다
- 일부 색상은 직접 참조되고 있습니다 (예: black)
다음과 같이 수정하는 것을 제안합니다:
+/** + * Icon 컴포넌트에서 사용되는 색상 매핑 + * @description 각 색상값은 디자인 시스템의 기본 색상을 참조합니다 + */ export const COLORS = { primary: colors['primary'][300], gray: colors['gray'][300], red: colors['red'][300], green: colors['green'][300], - black: colors['black'], + black: colors['black'] as string, };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/shared/lib/colors.ts
(1 hunks)src/shared/ui/Icon/Icon.stories.tsx
(1 hunks)src/shared/ui/Icon/Icon.tsx
(1 hunks)src/shared/ui/assets/index.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/shared/ui/Icon/Icon.stories.tsx
🔥 연관 이슈
🚀 작업 내용
npm i
필요합니다.Summary by CodeRabbit
새로운 기능
구성 변경
타입스크립트 개선
기타 변경
current
속성 제거