-
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] Chip 컴포넌트 추가 #70
Conversation
Warning Rate limit exceeded@kongnayeon has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 56 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
""" 워크스루이 풀 리퀘스트는 UI 구성 요소와 스타일링에 대한 다양한 변경 사항을 포함하고 있습니다. 주요 변경 사항은 새로운 변경 사항
관련 가능성 있는 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 (
|
* chore(apps/web): react-hook-form 설치 * feat(packages/ui): isNill 함수 추가 * chore(packages/ui): isNill export * feat(packages/ui): TextField 컴포넌트 구현 * test(apps/web): 예시 추가 * fix(packages/ui): 디자인 요구사항 수정
* feat(packages/theme): 아이콘 변경 사항 반영 * fix(packages/ui): 누락된 아이콘 추가
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: 4
🧹 Nitpick comments (2)
packages/ui/src/components/Text/Text.css.ts (1)
11-11
: lineHeight를 변수화하는 것을 고려해주세요.현재 하드코딩된 lineHeight 값을 다른 텍스트 속성들처럼 변수화하여 재사용성과 일관성을 높이는 것이 좋을 것 같습니다.
다음과 같이 변경하는 것을 제안드립니다:
- lineHeight: '150%', // TODO 추후 분리 + lineHeight: lineHeightVar, // 상단에 export const lineHeightVar = createVar(); 추가 필요packages/ui/src/components/Chip/Chip.css.ts (1)
5-36
: 커서 스타일 재검토 필요모든 Chip이 클릭 가능한 것은 아닐 수 있습니다.
closable
속성이나 onClick 핸들러가 있는 경우에만cursor: pointer
를 적용하는 것이 좋습니다.다음과 같이 수정하는 것을 제안합니다:
base: { display: 'inline-flex', alignItems: 'center', justifyContent: 'center', whiteSpace: 'nowrap', gap: '0.4rem', padding: '0.4rem 1.2rem', height: 'fit-content', width: 'fit-content', border: 'none', borderRadius: '2.4rem', - cursor: 'pointer', },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/ui/src/assets/icons/IconCircle.svg
is excluded by!**/*.svg
packages/ui/src/assets/icons/IconX.svg
is excluded by!**/*.svg
📒 Files selected for processing (9)
apps/web/src/app/page.module.css
(1 hunks)apps/web/src/app/page.tsx
(2 hunks)packages/ui/src/components/Chip/Chip.css.ts
(1 hunks)packages/ui/src/components/Chip/Chip.tsx
(1 hunks)packages/ui/src/components/Icon/Icon.css.ts
(1 hunks)packages/ui/src/components/Icon/assets.ts
(2 hunks)packages/ui/src/components/Text/Text.css.ts
(1 hunks)packages/ui/src/components/Text/Text.tsx
(2 hunks)packages/ui/src/components/index.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/app/page.module.css
🔇 Additional comments (5)
packages/ui/src/components/Icon/Icon.css.ts (1)
8-11
: 선택자 변경의 영향 범위를 검토해주세요.선택자를
path
에서*
로 변경하면 모든 하위 요소에 스타일이 적용되어 의도하지 않은 부작용이 발생할 수 있습니다. 특히 SVG 내부의 다른 요소들(rect, circle 등)에도 영향을 미칠 수 있습니다.다음 스크립트를 실행하여 현재 사용 중인 아이콘들의 구조를 확인해보세요:
✅ Verification successful
선택자 변경이 적절합니다.
현재
IconCircle
컴포넌트가circle
요소를 사용하고 있어 범용 선택자(*
)가 필요하며, 향후 다양한 SVG 요소를 사용하는 아이콘을 추가할 수 있도록 하는 것이 바람직합니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # SVG 구조 분석을 위한 스크립트 rg -g "*.svg" "<(?!path|svg|\?xml)[^>]+>" .Length of output: 275
Script:
#!/bin/bash # Find SVG elements other than path echo "=== Checking for rect elements ===" rg -g "*.svg" "<rect" . echo "=== Checking for circle elements ===" rg -g "*.svg" "<circle" . echo "=== Checking for other common SVG elements ===" rg -g "*.svg" "<(polygon|line|ellipse)" . echo "=== Checking Icon component usage ===" rg "Icon" -A 2 -B 2 .Length of output: 20654
packages/ui/src/components/Text/Text.tsx (2)
38-38
: color 처리 로직 변경에 대한 테스트 추가가 필요합니다.color prop이 없을 때 'inherit'로 처리되는 새로운 로직에 대한 테스트 케이스 추가가 필요해 보입니다.
테스트 코드 작성을 도와드릴까요?
26-29
: color prop의 기본값 제거에 대한 영향도를 확인해주세요.기존 'grey950' 기본값이 제거되었습니다. 이는 기존 Text 컴포넌트를 사용하는 곳에서 예상치 못한 스타일 변경을 일으킬 수 있습니다.
다음 스크립트로 Text 컴포넌트의 현재 사용 현황을 확인해보세요:
✅ Verification successful
color prop 기본값 제거는 안전합니다
코드베이스 분석 결과, Text 컴포넌트의 현재 사용 패턴에서 모든 중요 사례들이 color prop을 명시적으로 지정하고 있어 기본값 제거로 인한 부작용이 없을 것으로 확인됩니다.
•
Chip
: 색상 상속 사용
•Label
: "primary600", "grey300" 명시적 지정
•BreadcrumbItem
: "grey900"/"grey400" 조건부 지정
• Web app: "purple600" 명시적 지정🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Text 컴포넌트 사용 분석 rg "<Text\b[^>]*>" --type tsxLength of output: 59
Script:
#!/bin/bash # Text 컴포넌트 사용 패턴 분석 echo "=== Text 컴포넌트 임포트 검색 ===" rg "import.*Text.*from.*components/Text" -t js -t jsx -t ts -t tsx echo -e "\n=== Text 컴포넌트 사용 검색 ===" rg "<Text\b[^>]*>" -t js -t jsx -t ts -t tsx echo -e "\n=== AST 기반 Text 컴포넌트 검색 ===" ast-grep --pattern '<Text $$$>$$$</Text>'Length of output: 2236
packages/ui/src/components/index.ts (1)
31-32
: Chip 컴포넌트 export 추가가 적절히 이루어졌습니다.새로운 Chip 컴포넌트와 관련 타입이 올바르게 export 되었습니다.
packages/ui/src/components/Icon/assets.ts (1)
12-12
: 아이콘 추가 승인IconCircle 아이콘이 올바르게 추가되었습니다.
Also applies to: 42-42
export const chipCloseButtonStyle = style({ | ||
border: 'none', | ||
background: 'inherit', | ||
display: 'inline-block', | ||
width: 'auto', | ||
height: 'auto', | ||
lineHeight: 0, | ||
}); |
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.
🛠️ Refactor suggestion
접근성 및 상호작용 스타일 개선 필요
닫기 버튼에 hover, focus 상태에 대한 스타일이 없어 사용자 경험이 제한적입니다. 또한 접근성을 위한 스타일도 추가되어야 합니다.
다음과 같이 수정하는 것을 제안합니다:
export const chipCloseButtonStyle = style({
border: 'none',
background: 'inherit',
display: 'inline-block',
width: 'auto',
height: 'auto',
lineHeight: 0,
+ padding: '0.4rem',
+ borderRadius: '50%',
+ cursor: 'pointer',
+ ':hover': {
+ backgroundColor: 'rgba(0, 0, 0, 0.05)',
+ },
+ ':focus-visible': {
+ outline: `2px solid ${tokens.colors.purple400}`,
+ outlineOffset: '2px',
+ },
});
📝 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.
export const chipCloseButtonStyle = style({ | |
border: 'none', | |
background: 'inherit', | |
display: 'inline-block', | |
width: 'auto', | |
height: 'auto', | |
lineHeight: 0, | |
}); | |
export const chipCloseButtonStyle = style({ | |
border: 'none', | |
background: 'inherit', | |
display: 'inline-block', | |
width: 'auto', | |
height: 'auto', | |
lineHeight: 0, | |
padding: '0.4rem', | |
borderRadius: '50%', | |
cursor: 'pointer', | |
':hover': { | |
backgroundColor: 'rgba(0, 0, 0, 0.05)', | |
}, | |
':focus-visible': { | |
outline: `2px solid ${tokens.colors.purple400}`, | |
outlineOffset: '2px', | |
}, | |
}); |
export type ChipProps = ComponentPropsWithoutRef<'span'> & { | ||
variant: ButtonVariant; | ||
leftAddon?: ReactElement; | ||
rightAddon?: ReactElement; | ||
closable?: boolean; | ||
}; |
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.
필수 Props 누락
닫기 가능한 Chip의 경우 다음 속성들이 누락되었습니다:
onClose
핸들러- 접근성을 위한
aria-label
다음과 같이 수정하는 것을 제안합니다:
export type ChipProps = ComponentPropsWithoutRef<'span'> & {
variant: ButtonVariant;
leftAddon?: ReactElement;
rightAddon?: ReactElement;
closable?: boolean;
+ onClose?: () => void;
+ 'aria-label'?: string;
};
📝 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.
export type ChipProps = ComponentPropsWithoutRef<'span'> & { | |
variant: ButtonVariant; | |
leftAddon?: ReactElement; | |
rightAddon?: ReactElement; | |
closable?: boolean; | |
}; | |
export type ChipProps = ComponentPropsWithoutRef<'span'> & { | |
variant: ButtonVariant; | |
leftAddon?: ReactElement; | |
rightAddon?: ReactElement; | |
closable?: boolean; | |
onClose?: () => void; | |
'aria-label'?: string; | |
}; |
variant, | ||
leftAddon, | ||
rightAddon, | ||
closable = false, |
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.
closable이 있다면 onClose 콜백도 필요할 것 같아요!
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.
저 요거 고민했는데... 그냥 onClick 속성을 활용해도 될 거 같아서 냅뒀거든요! chip 자체를 선택하면 close 될 수 있도록... onClose가 있는 게 나을까요?
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.
명시적으로 하는게 좋지 않을까 생각했었어요! 그리고 혹시 closable할 때
cursor: 'pointer',
요게 필요할 것 같아요. 그리고,
':hover': {
backgroundColor: 'rgba(0, 0, 0, 0.05)',
},
:hover
같은 경우는 디자이너님과 소통 부탁드립니다!
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.
넵! 확인했어요!
|
||
export type ChipRecipeVariants = RecipeVariants<typeof chipRecipe>; | ||
|
||
export const addonRootRecipe = recipe({ |
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.
span으로 한번 더 감싸서 요 스타일을 적용하는 이유가 궁금해요!
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.
아이콘의 색상을 따로 내려 주지 않아도 최상단의 span에 적용된 color 속성을 상속받을 수 있도록 하려고 감쌌어요!
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.
근데 생각해 보니 굳이 span을 여기서 감싸 주지 않아도 스타일이 적용되겠군요?.... 😇
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.
아 제가 착각해서 답변을 잘못 드렸는데 최상단의 span에서 적용하는 color는 600 스케일이고, 아이콘에 적용되는 컬러는 400 스케일이어서 다시 한 번 span으로 감싸 주어 컬러를 적용하고 있어요!
</Text> | ||
{rightAddon && ( | ||
<span className={addonRootRecipe({ color: variant })}> | ||
{rightAddon} |
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.
leftAddon, rightAddon을 ReactElement로써 자유도를 열어두는 것은 좋지만, 자유도를 열어두고, 디자인 가이드에 맞는(Chip 색상에 맞는) Chip.Icon도 함께 제공하는 것이 어떨까요?
조금 더 풀어서 말씀드리자면, span 태그를 여기서 제거하고, 따로 Chip.Icon 컴포넌트를 만들어서 거기에서 variant prop을 받고 그에 맞게 조건부로 Icon 컴포넌트에 색상을 보내는 방식인거죠!
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.
음음 그러면 rightAddon도 받고, icon prop도 따로 받자는 말씀이실까요??
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.
rightAddon에 ReactElement 또는 ReactNode를 받되, 디자인 가이드에 맞는 Chip.Icon을 제공하자는 뜻입니다!
RadioCards 컴포넌트도 이렇게 만들었는데, 아래와 같은 구조를 생각했어요!
<RadioCards.Item
value="5"
leftAddon={<RadioCards.Icon name="picture" size={24} />}
>
<RadioCards.Label>짧은 게시물</RadioCards.Label>
</RadioCards.Item>
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.
Chip.Icon에 대한 정의가 따로 없고 Icon 컴포넌트를 그대로 사용할 뿐인데 Chip.Icon으로 제공했을 때의 이점이 따로 있을까요?
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.
Chip.Icon은 Icon 컴포넌트를 재사용한게 맞아요! 그런데 제가 전달드리고 싶었던 말은 Chip.Icon에서 name과 type의 default value를 name="circle" type="fill" 로 주자는 뜻이었어요! 그리고 span태그로 감싸지 않고, default value로 각 variant 마다 컬러를 지정해주면 되지 않을까요?
지금 현재의 구현대로라면, 이 컴포넌트를 사용할 때마다 Icon의 name과 type을 기억해두고 있어야해서 다소 불편할 수도 있겠다 생각했어요!
@minseong0324 description은 원래 closable=true일 때만 있었어요! |
</Text> | ||
{rightAddon && ( | ||
<span className={addonRootRecipe({ color: variant })}> | ||
{rightAddon} |
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.
Chip.Icon은 Icon 컴포넌트를 재사용한게 맞아요! 그런데 제가 전달드리고 싶었던 말은 Chip.Icon에서 name과 type의 default value를 name="circle" type="fill" 로 주자는 뜻이었어요! 그리고 span태그로 감싸지 않고, default value로 각 variant 마다 컬러를 지정해주면 되지 않을까요?
지금 현재의 구현대로라면, 이 컴포넌트를 사용할 때마다 Icon의 name과 type을 기억해두고 있어야해서 다소 불편할 수도 있겠다 생각했어요!
variant, | ||
leftAddon, | ||
rightAddon, | ||
closable = false, |
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 ( | ||
<span className={`${chipRecipe({ variant })} ${className}`} {...rest}> | ||
{leftAddon && ( |
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.
그리고 여기서 isNill
로 nullish 체크를 해주는게 어떨까요? 현재는 암묵적 타입변환이 발생하는 코드입니다! isNil
은 packages/ui/src/utils 에서 import해서 사용하시면 돼요! 이건 제가 만든 컴포넌트에도 다시 체크해봐야겠군요..!
@minseong0324 left, rightAddon도 열어 두고 Chip.Icon도 제공하는 게 맞을까요? 반영해 두었습니다! |
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
🧹 Nitpick comments (4)
packages/ui/src/components/Chip/ChipIcon.tsx (2)
5-9
: 타입 정의에 대한 추가 설명 권장
ChipIconProps에 포함된 props에 대한 추가적인 JSDoc이나 주석을 제공하면 사용자가 더 쉽게 이해할 수 있어 보입니다.
17-24
: variant 미지정 상황 처리 고려
현재 variant가 선언된 값(grey, purple, green) 중 하나가 아닐 경우의 처리 로직이 없습니다.packages/ui/src/components/Chip/ChipItem.tsx (1)
9-15
: 접근성 속성 적용 고려
ChipProps에 aria-label 등 접근성 관련 속성이 포함되면 사용자 경험 향상에 도움이 됩니다.apps/web/src/app/page.tsx (1)
224-235
: closable 처리를 위한 onClose 핸들러 필요
closable이 true임에도 onClose 콜백이 정의되어 있지 않아, 사용자가 닫기 동작을 인식하지 못할 수 있습니다. Chip에 onClose 함수를 전달하거나 UX 설계를 명확히 해 보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/src/app/page.tsx
(2 hunks)packages/ui/src/components/Chip/Chip.css.ts
(1 hunks)packages/ui/src/components/Chip/Chip.tsx
(1 hunks)packages/ui/src/components/Chip/ChipIcon.tsx
(1 hunks)packages/ui/src/components/Chip/ChipItem.tsx
(1 hunks)packages/ui/src/components/index.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ui/src/components/index.ts
- packages/ui/src/components/Chip/Chip.tsx
🔇 Additional comments (5)
packages/ui/src/components/Chip/ChipItem.tsx (1)
47-49
: 토큰 유효성 확인 권장
색상으로${variant}400
가 올바른지 확인이 필요합니다. 테마에 정의된 스케일과 불일치할 가능성이 있으니, 실제로grey400
,purple400
,green400
가 정확히 존재하는지 검토하시기 바랍니다.packages/ui/src/components/Chip/Chip.css.ts (3)
4-34
: Chip 스타일 정의
각 variant별로 배경색과 글자색이 직관적으로 매핑되어 있으며, chipRecipe 구조가 간결해 보입니다.
36-36
: 타입 선언 문제 없음
ChipRecipeVariants를 통해 variant 타입을 유연하게 처리할 수 있습니다.
57-74
: hover/focus 스타일 보강 필요
닫기 버튼의 접근성과 시각적 피드백을 위해 hover, focus 상태 스타일을 추가하면 좋겠습니다.apps/web/src/app/page.tsx (1)
215-217
: Chip.Icon 활용
Chip 컴포넌트 내부에서 variant에 맞게 Chip.Icon을 사용하시는 방식이 일관적이고 이해하기 쉽습니다.
{children} | ||
</Text> | ||
{rightAddon} | ||
{isNil(closable) && ( |
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.
closable 로직 문제
isNil(closable)
조건은 null
또는 undefined
인 경우만 닫기 버튼을 표시하므로, closable
이 false
일 때도 숨겨지는 문제가 있습니다. 의도대로라면 closable === true
인 경우 버튼을 표시하는 로직으로 수정해야 합니다.
-{isNil(closable) && (
+{closable && (
<button
type="button"
aria-label="Close"
...
>
...
</button>
)}
📝 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.
{isNil(closable) && ( | |
{closable && ( |
}) => { | ||
return ( | ||
<span className={`${chipRecipe({ variant })} ${className}`} {...rest}> | ||
{leftAddon} |
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.
여기다가 !isNil 다는 것이 좋지 않을까 리뷰 드렸었어요! isNotNil은 제가 다른 브랜치에 만들어놔서, 그 브랜치 반영되면 추후에 수정해둘게요!
{children} | ||
</Text> | ||
{rightAddon} | ||
{isNil(closable) && ( |
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.
closable하지 않으면 close 버튼을 띄워주는 것이 아니라 반대가 아닌가요?.?
그리고 이미 closable은 boolean 값이라 nullish 체크를 별도로 할 필요가 없어요!
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 (3)
packages/ui/src/components/Icon/assets/IconCircle.tsx (2)
3-4
: 인터페이스가 사용되지 않습니다.
IconCircleProps
가 선언되었지만 실제로 컴포넌트에서는 사용되지 않고 있습니다. 필요하다면props
대신IconCircleProps
를 활용하도록 수정하거나, 인터페이스가 불필요하다면 제거하시길 제안드립니다.
5-16
:fill
색상을 더욱 유연하게 처리해 주세요.
현재<circle>
의fill="#788391"
값이 하드코딩되어 있어 아이콘 스타일링을 유연하게 제어하기 어렵습니다. 향후 다양한 색 요건을 지원해야 할 가능성이 있다면,fill
을props
로부터 받아오도록 개선하면 재사용성이 높아집니다.packages/ui/src/components/Icon/assets/IconX.tsx (1)
15-23
: React SVG 속성은 카멜 케이스를 권장합니다.
stroke-width
를 JSX 규칙에 따라strokeWidth
로 변경하시면 가독성이 향상되고, 일관된 React 사용 방식을 준수하게 됩니다.예시 수정안:
- stroke-width="3.04762" + strokeWidth="3.04762"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/src/app/page.tsx
(1 hunks)packages/ui/src/components/Icon/assets.ts
(2 hunks)packages/ui/src/components/Icon/assets/IconCircle.tsx
(1 hunks)packages/ui/src/components/Icon/assets/IconX.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ui/src/components/Icon/assets.ts
- apps/web/src/app/page.tsx
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
♻️ Duplicate comments (1)
packages/ui/src/components/Chip/ChipItem.tsx (1)
35-35
:⚠️ Potential issueclosable 로직 수정이 필요합니다
!isNil(closable)
조건은closable
이false
일 때도 버튼이 표시됩니다.closable
prop이 이미 boolean 타입이므로 단순히closable
조건만 사용하는 것이 더 명확합니다.-{!isNil(closable) && ( +{closable && (
🧹 Nitpick comments (3)
packages/ui/src/components/Chip/ChipItem.tsx (3)
47-47
: Icon 컴포넌트의 color prop 타입 안정성 개선이 필요합니다템플릿 리터럴을 사용한 color 값은 타입 안정성을 보장하지 않습니다. 색상 상수나 타입이 정의된 색상 값을 사용하는 것이 좋습니다.
-color={`${variant}400`} +color={`var(--color-${variant}-400)`}
39-39
: 불필요한 clickable prop 제거가 필요합니다
chipCloseButtonRecipe
에 전달되는clickable
prop이closable
prop과 중복됩니다. 이미closable
이true
일 때만 버튼이 렌더링되므로clickable
prop은 불필요합니다.-className={chipCloseButtonRecipe({ clickable: closable })} +className={chipCloseButtonRecipe()}
36-41
: 접근성 개선이 필요합니다닫기 버튼의 접근성을 개선하기 위해 다음 사항들을 고려해주세요:
- 키보드 이벤트 처리 (
onKeyDown
)- 스크린 리더를 위한
aria-describedby
속성 추가<button type="button" aria-label="Close" + aria-describedby={`chip-description-${id}`} className={chipCloseButtonRecipe({ clickable: closable })} onClick={onClose} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + onClose?.(); + } + }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/ui/src/components/Chip/ChipItem.tsx
(1 hunks)
🔇 Additional comments (1)
packages/ui/src/components/Chip/ChipItem.tsx (1)
1-15
: 타입 정의가 잘 구성되어 있습니다! 👍타입 정의가 명확하고 확장성이 좋습니다.
variant
를 필수 prop으로 지정한 것이 타입 안정성 측면에서 좋습니다.
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.
고생하셨습니다!
* [Feat] 프로젝트 스캐폴딩 (#2) * docs: 이슈, PR 템플릿 추가 * docs: codeoners 추가 * feat: web 앱 Next.js, React 버전 다운그레이드 * feat: eslint, tsconfig 종속성, prettier 추가 * feat: husky, lint-staged 설치 * feat: husky, lint-staged 테스트 * chore: husky, lint-staged 테스트 * chore: husky, lint-staged 테스트 2 * fix: eslint config 수정 * chore: husky, lint-staged 테스트 3 * chore: husky, lint-staged 테스트 4 * chore: husky, lint-staged 테스트 5 * feat: .nvmrc 추가 * feat: vanilla extract 설치, config 수정 * chore: web 앱 type: module 설정 제거 * chore: vanilla extract 설치, next.config.js 수정 * [Chore]: 리뷰어 선정 시 슬랙 알림 workflow 적용 (#33) * [Chore]: ui 패키지 초기 셋팅 (#34) * feat(packages/theme): theme 패키지를 생성해요. (#36) * [Chore]: build ci 추가 (#38) * feat: pnpm workspace 추가 (#42) * [Fix]: theme, ui 패키지의 vanilla-extract 빌드 설정 수정 (#45) * fix(apps/web, packages/theme, packages/ui): theme, ui 패키지의 vanilla-extract 빌드 설정, css 관련 수정 * chore(packages/theme, packages/ui): build 스크립트 수정 * chore(packages/theme, packages/ui): exports 수정 * chore(packages/theme, packages/ui): exports 수정 * wip * chore(packages/theme, packages/ui): 불필요 dependency 제거 * chore(packages/theme, packages/ui): 불필요 필드 제거 * fix: 공통 패키지 루트로 끌어올리기, 패키지 버전 의존성 적용 * fix: 테스트 컴포넌트 제거 * fix: 테스트 컴포넌트 제거 * chore(apps/web): 주석처리 * chore: 빌드 테스트 * chore: 테스트용 컴포넌트 제거 --------- Co-authored-by: kangnayeon <[email protected]> * [Feat]: theme 적용을 위한 Provider 추가 (#47) * [Feat] Spacing 컴포넌트 추가 (#49) * [Feat] Icon 컴포넌트 추가 (#48) * feat(packages/ui): Icon 컴포넌트 추가 * fix(packages/ui): 아이콘 속성 camelCase로 수정 * feat(packages/ui): svg 파일 import 스크립트, svgr 플러그인 설정 추가 * feat(packages/ui): Icon 컴포넌트 * fix(packages/theme): color 뎁스 낮추기 * [Feat] Spacing 컴포넌트 추가 (#49) * fix: resolve conflicts * fix(packages/theme): 컬러 토큰 변경에 따른 dark, theme 토큰 변경 * fix: resolve conflicts * chore: 주석 제거 --------- Co-authored-by: minseong <[email protected]> * [Fix] Icon 컴포넌트 수정 (#51) * fix(packages/ui): Icon 컴포넌트 수정 * chore(packages/ui): 색상 스타일 로직 개선 * fix(packages/ui): IconProps의 color 타입을 string으로 수정해요 * [Fix] 컬러 시스템 내의 색상만 사용할 수 있도록 Icon 컴포넌트 수정 (#53) * fix(packages/ui): Icon 컴포넌트 수정 * chore(packages/ui): 색상 스타일 로직 개선 * fix(packages/ui): IconProps의 color 타입을 string으로 수정해요 * fix(packages/ui, apps/web): 컬러 시스템 내의 색상만 사용할 수 있도록 Icon 컴포넌트 수정 * [Fix] Spacing 컴포넌트 수정 (#54) * fix(packages/ui): Spacing 컴포넌트 수정 * chore: name import 하도록 수정 * chore: 배럴 파일 수정 * chore: empty file 삭제 * chore(packages/theme): 색상 추가 (#56) * [Feat] Text 컴포넌트 추가 (#57) * feat(packages/ui): Text 컴포넌트 * feat(packages/ui): 컴파운드 객체 추가 * chore(packages/ui): TypographyType theme에서 가져오도록 수정 * fix(packages/ui): TextCompoundType 선언 시 중복되는 구문 수정 * fix(packages/ui): Compound Text 컴포넌트 선언 시 중복되는 로직 수정 * feat(packages/ui): Text 컴포넌트 사용 시 대문자 사용하도록 수정 * [Feat] Toast 컴포넌트 추가 (#52) * chore(packages/ui): ovarlay-kit 의존성 추가 * feat(packages/ui): Toast 컴포넌트 구현 * test(apps/web): Toast 컴포넌트 사용 예시 추가 * fix(packages/ui): 접근성 개선 * chore(packages/ui): lock 파일 업데이트 * chore(packages/themes): violet 색상 추가 * chore(packages/ui): success일 경우의 색상 변경 * fix(apps/web): Providers 컴포넌트 분리, OverlayProvider 이동 * [Feat] Badge 컴포넌트 구현 (#58) * feat(packages/ui, apps/web): Badge 컴포넌트 구현 * fix(packages/ui, apps/web): Badge 컴포넌트 수정 * chore(packages/ui): named import 변경 * [Feat] Checkbox 컴포넌트 추가 (#60) * feat(packages/ui): 누름 상태 관리를 위한 usePress 추가 * chore(packages/ui): 체크박스 아이콘 추가 * feat(packages/ui): 체크박스 컴포넌트 추가 * chore(apps/web): 사용 예시 추가 * fix(packages/ui): 키보드 접근성 수정 * [Feat] Label 컴포넌트 구현 (#61) * feat(packages/ui): Label 컴포넌트 구현 * chore(packages/ui): Label 컴포넌트 예시 * [Feat] Button 컴포넌트 추가 (#59) * feat(packages/ui): Button 컴포넌트 * fix(packages/ui): Icon color 속성 없을 시 currentColor 적용되도록 수정 * chore(packages/ui): Button 컴포넌트 export * fix(packages/ui): Button 컴포넌트 스타일 * fix(packages/ui): Icon default size 100%로 변경 * fix(packages/ui): Button 컴포넌트의 leftIcon, rightIcon prop을 leftAddon, rightAddon으로 수정 * feat(packages/ui): twincle icon 추가 * fix(packages/ui): 기존 태그의 prop을 상속 받는 경우 ComponentPropsWithoutRef 사용하도록 수정 * [Feat] Breadcrumb 컴포넌트 추가 (#62) * chore(packages/ui): @radix-ui/react-slot 설치 * feat(packages/ui): Breadcrumb 컴포넌트 구현 * test(apps/web): 사용 예시 추가 * [Feat] px -> rem 변환 (#64) * fix: px 단위 rem으로 변환 * fix: body font-size 1.6rem으로 지정 * [Chore] Checkbox 컴포넌트의 label fontSize 변환 (#66) * chore(packages/ui): Checkbox 컴포넌트의 label fontSize 변환 * chore(packages/ui): className 빈 값 string 기본값 지정 * [Feat] LottieAnimation 컴포넌트 추가 (#63) * feat(packages/ui): 로티 파일, 생성 스크립트 추가 * feat(packages/ui): LottieAnimation 컴포넌트 추가 * fix(packages/ui): LottieAnimation 따로 export 하도록 수정, 에셋 상대경로로 수정 * fix(packages/ui): 스크립트 오타 수정 * feat(packages/ui): 코드리뷰 반영 * fix: 빌드 에러 수정 * [Feat] TextField 컴포넌트 구현 (#65) * chore(apps/web): react-hook-form 설치 * feat(packages/ui): isNill 함수 추가 * chore(packages/ui): isNill export * feat(packages/ui): TextField 컴포넌트 구현 * test(apps/web): 예시 추가 * fix(packages/ui): 디자인 요구사항 수정 * [Feat] 아이콘, 색상 변경 사항 반영 (#72) * feat(packages/theme): 아이콘 변경 사항 반영 * fix(packages/ui): 누락된 아이콘 추가 * [Feat] IconButton 컴포넌트 (#68) * [Feat] Spinner 컴포넌트 추가 (#69) * feat(packages/ui): Spinner 컴포넌트 * feat(packages/ui): Spinner span 태그로 감싸기 * fix(packages/ui): SpinnerColorType 추가 * [�Fix]: ThemeProvider 패키지 이동, 불필요 배럴 파일 정리 (#74) * wip * chore(apps/web): 파일 구조 변경 * chore(packages/theme, apps/web): 배럴파일 정리 및 provider 별도 export * chore(packages/theme): provider 별도 export * fix(packages/theme, packages/ui): ThemeProvider 이동 * remove(apps/web): 퍼블리싱 삭제 (별도 PR 예정) * [Feat] RadioCards 컴포넌트 추가 (#73) * feat(packages/ui): RadioCards 관리를 위한 context 추가 * feat(packages/ui): RadioCards 컴포넌트 구현 및 JSDoc 추가 * feat(packages/ui): RadioCards 사용 예시 추가 * fix(packages/ui): RadioCards 구조 변경 * test(packages/ui): RadioCards 예시 변경 * docs(packages/ui): JSDoc 업데이트 * fix(packages/ui): 키보드 접근성 개선 * [Feat] Modal 컴포넌트 추가 (#83) * feat(packages/ui): PortalConsumer 컴포넌트 * feat(packages/ui): PortalConsumer 컴포넌트 * feat(packages/ui): Modal로의 포커스를 위한 FocusTrap 구현 * feat(packages/ui): Modal 컴포넌트 구현 및 JSDoc 작성 * test(apps/web): Modal 컴포넌트 사용 예시 추가 * fix(packages/ui): overlay-kit에서 이미 제공하고 있으므로 PortalConsumer 제거 * [Feat] 스켈레톤 컴포넌트 (#84) * feat(packages/ui): 스켈레톤 컴포넌트 * feat: displayName 속성 추가 * feat: 리뷰 반영 * [Fix] ui 패키지 컴포넌트 사용 시 모든 컴포넌트가 암묵적으로 import 되는 현상 (#79) * fix(packages/ui): 아이콘 생성 스크립트 확장자 변경 * feat(packages/ui) 컴포넌트 별 배럴 파일 생성 * fix: 공통 로직 임포트 경로 수정 * fix(packages/ui): esm 방식만 지원하도록 수정 * chore: 사용되지 않는 컴포넌트 제거 * chore: 사용되지 않는 아이콘 제거 * fix(packages/ui): 사용되지 않는 플러그인 제거 * fix: 빌드 에러 * fix: 빌드 에러 * fix: 빌드 에러 수정 * feat: svgr 라이브러리 제거, 아이콘 컴포넌트 생성 스크립트 추가 * fix: build error * fix: icon 매핑 스크립트 수정 * feat(packages/ui): 컴포넌트 전체 export 추가 * [Feat] Chip 컴포넌트 추가 (#70) * feat(packages/ui): chip 컴포넌트 * [Feat] TextField 컴포넌트 구현 (#65) * chore(apps/web): react-hook-form 설치 * feat(packages/ui): isNill 함수 추가 * chore(packages/ui): isNill export * feat(packages/ui): TextField 컴포넌트 구현 * test(apps/web): 예시 추가 * fix(packages/ui): 디자인 요구사항 수정 * [Feat] 아이콘, 색상 변경 사항 반영 (#72) * feat(packages/theme): 아이콘 변경 사항 반영 * fix(packages/ui): 누락된 아이콘 추가 * feat(packages/ui): chip 컴포넌트 * feat(packages/ui): Chip에 사용되는 아이콘 추가 * fix(packages/ui): Text 컴포넌트 color prop 없을 때 inherit으로 지정, 기본 line height 값 지정 * fix(packages/ui): Icon 컴포넌트 color 상속 시 path 태그 외의 다른 태그들도 상속 받을 수 있도록 수정 * fix(packages/ui): Chip 컴포넌트 스타일 수정 * feat: x 아이콘 color 수정 * feat: onClose 추가 * fix: x 아이콘 호버링 시 cursor pointer로 지정 * feat: 리뷰 반영 * fix(packages/ui): closable 버튼 보이지 않는 문제 * fix: 리뷰 반영 --------- Co-authored-by: MINSEONG KIM <[email protected]> * [Refactor] Spinner 컴포넌트 css 방식으로 변경 (#92) * fix:(packages/ui): Spinner 컴포넌트 css 방식으로 구현 * fix(packages/ui): Button 컴포넌트 스타일 수정, isLoading 상태 스피너 추가 * [Feat] useToast 추가, DynamicLottie 컴포넌트 추가 (#89) * [Feat]: useModal 추가 (#91) * chore(packages/ui): overlay-kit 설치 * feat(packages/ui): useModal 추가 * test(apps/web): useModal 사용 예시 추가 * fix(packages/ui): 옵셔널로 치환 * fix(packages/ui): 자잘한 수정 * fix(packages/ui): 자잘한 수정 * docs(packages/ui): JSDoc 추가 * test(apps/web): 사용 예시 추가 * chore(packages/ui): 코드 간소화 * fix(packages/ui): 프로미스를 반환할 수 있도록 수정 * fix(packages/ui): isNil수정, isNotNill 추가, export 수정 * fix(package/ui): nullish 체크 * fix(packages/ui): 모달 doubleCTA의 각각 버튼 공간이 절반 차지하도록 수정 * test(packages/ui): 예시 수정 * chore: lock 파일 업데이트 * [Feat] ImageManager TypeA 컴포넌트 추가 (#94) * chore(apps/web): Provider -> provider로 변경 (네이밍 컨벤션 통일) * feat(apps/web): ImageManager TypeA 컴포넌트 구현 * chore(apps/web): 대소문자 git 이슈 해결 * chore(apps/web): baseUrl 설정 * fix(apps/web): 자잘한 수정, 접근성 개선 * chore(apps/web): 키 상수화 * fix(apps/web): 유효성 검사 수정 * chore(apps/web): 경로 수정 * [Feat] 주제 설정 페이지 퍼블리싱 및 폼 연동 (#95) * chore(apps/web): motion 추가 * chore(packages/ui): styles export 변경 * feat(packages/theme): 색상 추가 * feat(packages/theme): spacing 추가 * feat(apps/web): KeywordChip 컴포넌트 추가 * feat(apps/web): ImageManager 컴포넌트 추가 * feat(apps/web): 주제 설정 페이지 퍼블리싱 * refactor(apps/web): 컴포넌트 분리 및 자잘한 수정 * fix(apps/web): react-hook-form watch를 통한 조건부 렌더링 * feat(apps/web): react-hook-form을 통한 폼 관리 * fix(apps/web): ImageManager 컴포넌트 packages/ui로 이동 예정 * fix(apps/web): ImageManager TypeA css 수정 * fix(apps/web): GradientAnimatedTitle로 수정 * fix(apps/web): GradientAnimatedTitle로 수정 * feat(apps/web): isEmptyStringOrNil 유틸 추가 * fix(apps/web): ImageManager TypeA 제어형으로 사용 가능하도록 변경 * refactor(apps/web): 컴포넌트 분리 * chore(apps/web): px->rem 변경 * fix(app/web): defaultValues 수정 * fix(apps/web): css 수정 * fix(apps/web): placeholder 수정 * [Feat] ky, tanstack-query 설정, presigned-url 모듈, 뉴스 카테고리 api 연동 (#97) * fix(packages/ui): onClick 누락 수정 * fix(packages/ui): onClick 누락 수정 * feat(apps/web): 필수 항목 입력 후 홈 BreadCrumb 클릭 시 모달 * feat(apps/web): tanstack-query 셋팅 * fix(apps/web): tanstack-query 셋팅 수정 * feat(apps/web): ky 셋팅 * fix(apps/web): KeywordChipGroup 컴포넌트 수정 * feat(apps/web): 뉴스 카테고리 api 연듕 * feat(apps/web): put, patch, delete 추가 * feat(apps/web): put, patch, delete 추가 * fix(apps/web): nullish 검증 * chore(.github/workflows): env 추가 * fix(apps/web): staleTime, gcTime 수정 * fix(apps/web): 자잘한 수정 * fix(apps/web): 토큰 string으로 관리 * chore(apps/web): shared로 폴더명 변경 * feat(apps/web): presigned-url 모듈 구현 및 연동 * [Feat] 주제 설정 페이지 api 연동, 디자인 추가 반영 (#100) * fix(apps/web): MainBreadcrumbItem 이미지 수정 * fix(apps/web): suspense 래핑 * remove(apps/web): 불필요 코드 제거 * feat(apps/web): 스크롤 감지 훅 추가 * feat(apps/web): 반투명 NavBar 추가 * fix(apps/web): 스타일링 수정 * fix(apps/web): 스타일링 수정 * fix(apps/web): ImageManager TypeA가 File이 아닌 이미지의 url로 받도록 수정 * feat(apps/web): 공통 타입 분리 * feat(apps/web): 게시물 그룹 및 게시물 생성 API 연동 * feat(apps/web): ImageManager TypeA 최적화 및 UI만 그리도록 관심사 분리 * fix: Images 폴더명을 images로 수정 * fix(apps/web): 자잘한 수정 * fix(apps/web): 자잘한 수정 * fix(apps/web): 타입 개선 * fix(apps/web): ImageManager 단일로 관리 * [Feat] Accordion 컴포넌트 (#99) * fix(packages/ui): Chip 컴포넌트 개별 export * feat(packages/ui): Accordion 컴포넌트 * chore: Accordion 컴포넌트 예시 추가 * chore(packages/ui): displayName 추가 * fix: 코드리뷰 반영 * fix: resolve conflicts * [Fix] 컴포넌트 스타일 수정 (#102) * fix: 폴더명 Images를 images로 변경 * fix(apps/web): 간단한 레이아웃 수정 * fix(apps/web): NavBar 레이아웃 수정 * fix(packages/ui, apps/web): TextField 레이아웃 깨짐 수정 * fix(packages/ui): Checkbox width 수정 * docs(packages/ui): TextField JSDoc 수정 * [Feat] 결과 수정 - 상세 페이지 퍼블리싱 (#86) * feat(apps/web): ContentItem 컴포넌트 * feat(apps/web): getMinutesAgo 유틸 함수 추가 * feat(apps/web): ContentItem 컴포넌트 * fix(packages/ui): Chip 컴포넌트 개별 export * feat(packages/ui): Accordion 컴포넌트 * chore: Accordion 컴포넌트 예시 추가 * feat(apps/web): EditSidebar 추가 * feat(packages/ui): 아이콘 에셋 추가 * fix(apps/web): getMinutesAgo 함수 getTimesAgo 함수로 수정 * fix(packages/ui): IconButton 컴포넌트 props에서 type도 받도록 수정 * fix(apps/web): ContentItem 컴포넌트 디자인 수정 반영 * feat(apps/web): PostEditor 추가 * feat(apps/web): EditPromptField 추가 * feat(apps/web) EditPost 영역 추가 * feat(apps/web): ContentItem 컴포넌트 * chore: Accordion 컴포넌트 예시 추가 * fix: resolve conflicts * fix: resolve conflicts * fix: 빌드 에러 * fix: 빌드 에러 * fix(apps/web): TextField 수정 반영 * fix(packages/ui): Accordion 컴포넌트 스타일 수정 * [Feat] vercel 자동 배포 빌드 스크립트 (#105) --------- Co-authored-by: MINSEONG KIM <[email protected]> Co-authored-by: minseong <[email protected]>
관련 이슈
#39
변경 사항
Chip 컴포넌트 만들었습니다!
레퍼런스
Summary by CodeRabbit
릴리즈 노트
새로운 기능
ChipItem
및ChipIcon
컴포넌트 추가개선 사항
스타일 변경