[FE] Chip 컴포넌트 구현 #100
[FE] Chip 컴포넌트 구현 #100
Conversation
Summary of ChangesHello @pakxe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 프론트엔드에 재사용 가능한 Chip 컴포넌트를 도입합니다. 이 컴포넌트는 유연한 태그 선택( Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 재사용 가능한 Chip 컴포넌트를 구현한 점이 좋습니다. cva를 사용하여 변형(variant)을 관리하고 as prop을 통해 다형성(polymorphism)을 지원하는 접근 방식은 훌륭합니다.
다만, 현재 다형성 컴포넌트의 타입 정의에 심각한 버그가 있어 런타임 오류를 유발할 수 있습니다. 또한, 코드 가독성과 모범 사례 측면에서 개선할 수 있는 몇 가지 사항을 발견하여 리뷰에 포함했습니다. 제안된 변경 사항들을 확인해 보시고 반영해 주시면 감사하겠습니다.
| type AllowedElementType = "button" | "span"; | ||
|
|
||
| type Props<T extends AllowedElementType> = ComponentPropsWithoutRef<T> & | ||
| VariantProps<typeof variants> & { | ||
| as?: T; | ||
| leftSlot?: ReactNode; | ||
| }; | ||
|
|
||
| const Chip = <T extends AllowedElementType = "button">({ | ||
| variant = "primary", | ||
| size = "md", | ||
| leftSlot, | ||
| children, | ||
| as, | ||
| ...rest | ||
| }: Props<T>) => { | ||
| const Component = as ? as : "button"; |
There was a problem hiding this comment.
현재 Chip 컴포넌트의 타입 정의는 안전하지 않습니다. 제네릭 타입 T와 as prop이 동기화되지 않아 타입 불일치 및 런타임 오류가 발생할 수 있습니다.
예를 들어, <Chip<"span"> />과 같이 컴포넌트를 호출하면, T는 "span"이 되지만 as prop은 undefined이므로 실제 렌더링되는 요소는 <button>이 됩니다. 이 경우 <span>에만 유효한 props가 <button>에 전달되어 React에서 오류를 발생시킬 수 있습니다.
이 문제를 해결하기 위해, 식별 가능한 유니온 타입(discriminated union type)을 사용하여 as prop의 값에 따라 props 타입이 결정되도록 수정하는 것을 강력히 권장합니다. 이렇게 하면 타입 안정성이 보장됩니다.
type ChipBaseProps = VariantProps<typeof variants> & {
leftSlot?: ReactNode;
};
type ChipProps =
| (ChipBaseProps &
ComponentPropsWithoutRef<"button"> & {
as?: "button";
})
| (ChipBaseProps &
ComponentPropsWithoutRef<"span"> & {
as: "span";
});
const Chip = ({
variant = "primary",
size = "md",
leftSlot,
children,
as,
...rest
}: ChipProps) => {
const Component = as === "span" ? "span" : "button";
|
|
||
| return ( | ||
| <Component className={variants({ variant, size })} {...rest}> | ||
| {leftSlot ? leftSlot : null} |
There was a problem hiding this comment.
leftSlot을 렌더링할 때 삼항 연산자를 사용하고 계십니다. React (JSX)에서는 null, undefined, false 값을 렌더링하지 않으므로, {leftSlot}과 같이 더 간결하게 작성할 수 있습니다. 이렇게 하면 코드가 더 깔끔해지고 읽기 쉬워집니다.
{leftSlot}
| primary: [COLOR_SET.primary], | ||
| secondary: [COLOR_SET.secondary], | ||
| tertiary_outlined: [COLOR_SET.tertiary_outlined], | ||
| quaternary: [COLOR_SET.quaternary], | ||
| quaternary_accent_outlined: [COLOR_SET.quaternary_accent_outlined], | ||
| basic: [COLOR_SET.basic], | ||
| notification: [COLOR_SET.notification], |
There was a problem hiding this comment.
cva의 variants 객체에서 각 값은 COLOR_SET에서 가져온 문자열입니다. 이 값들을 불필요하게 배열로 감싸고 있습니다. cva는 문자열 값을 직접 받을 수 있으므로, 가독성을 높이고 코드를 간결하게 만들기 위해 배열을 제거하는 것이 좋습니다.
primary: COLOR_SET.primary,
secondary: COLOR_SET.secondary,
tertiary_outlined: COLOR_SET.tertiary_outlined,
quaternary: COLOR_SET.quaternary,
quaternary_accent_outlined: COLOR_SET.quaternary_accent_outlined,
basic: COLOR_SET.basic,
notification: COLOR_SET.notification,
close #70
작업 내용
➊ button으로 사용될 때도 있고 그냥 꾸미는 요소로만 사용될 때도 있다
as prop으로 "span", "button"만 노출하여 적절한 태그를 골라 쓸 수 있게 했다.
기본 값은 "button"이다.
➋ 아이콘이 있는 경우가 있다.
leftSlot으로 넣을 수 있다. 이때 Icon의 컬러를 지정해주지 않는다면, text 컬러를 자동으로 사용한다.
결과