Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/shared/components/ui/chip/chip.tsx

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ChipButton에서 disabledactive가 동시에 들어올 수 있는데 이 경우 시각적으로는 disabled 스타일이 우선 적용되지만 aria-pressed는 여전히 active 값을 따릅니다. 따라서 비활성 상태에서도 선택된 상태로 인식될 수 있을 것 같아요

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Figma의 변경된 Chip 스펙을 확인하다가 타이포그래피 부분은 디자인 측에 한 번 확인이 필요해 보여요!

현재 스펙상 Line/Medium은 Default와 Pressed 모두 body-m-15를 사용하고 있는데, 새로 추가된 Fill/Medium은 Default에서 body-r-14, Pressed에서 body-sb-14를 사용하고 있어요. 같은 Medium 크기이지만 타입과 상태에 따라 글자 크기와 굵기가 달라지는 구조로 되어 있어요.

기존에는 버튼 사이즈에 따라 타이포그래피가 동일했기 때문에 아래처럼 정의해두고 사용할 수 있었지만, 만약 이번에 변경된 스펙처럼 같은 사이즈라 하더라도 type에 따라 타이포그래피가 달라진다면 코드를 수정할 필요가 있을 것 같아요.

const chipSizeStyles = {
  sm: 'px-3 text-body-r-14',
  md: 'px-4 text-body-m-15',
};

현재 구현은 size="md"일 때 공통으로 body-m-15를 적용하고 Fill은 활성 색상만 변경하고 있어서, 지금 Figma 스펙과는 일치하지 않아요.

일단 무조건적으로 수정하기 전에 Fill에서만 타이포그래피를 다르게 사용하려는 의도가 맞는지 디자인 측에 먼저 확인해 보면 좋을 것 같아요. 의도된 스펙이라면 Fill의 Default/Pressed 스타일을 별도로 분리해서 적용하고, 아니라면 Figma 스펙을 정리한 뒤 그 기준에 맞추면 될 것 같습니다!!

Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,19 @@ type ChipSize = 'sm' | 'md';
const chipBase =
'inline-flex shrink-0 items-center justify-center whitespace-nowrap rounded-[30px] border border-gray-200 bg-white py-2 text-gray-800';

const chipActive = 'border-mint-300 bg-mint-100 text-mint-300';
const chipTypeStyles = {
line: 'border-mint-300 bg-mint-100 text-mint-300',
fill: 'border-gray-800 bg-gray-800 text-white',
};

type ChipType = 'line' | 'fill';

const chipDisabled = 'border-gray-200 bg-gray-50 text-gray-200';

interface ChipProps extends React.HTMLAttributes<HTMLSpanElement> {
size?: ChipSize;
active?: boolean;
variant?: ChipType;
}

interface ChipButtonProps extends Omit<
Expand All @@ -25,20 +31,22 @@ interface ChipButtonProps extends Omit<
> {
size?: ChipSize;
active?: boolean;
variant?: ChipType;
}

export const Chip = ({
size = 'sm',
className,
active = false,
variant = 'line',
...props
}: ChipProps) => {
return (
<span
className={cn(
chipBase,
chipSizeStyles[size],
active && chipActive,
active && chipTypeStyles[variant],
className,
)}
{...props}
Expand All @@ -51,6 +59,7 @@ export const ChipButton = ({
className,
active = false,
disabled,
variant = 'line',
...props
}: ChipButtonProps) => {
return (
Expand All @@ -61,7 +70,7 @@ export const ChipButton = ({
className={cn(
chipBase,
chipSizeStyles[size],
active && chipActive,
active && chipTypeStyles[variant],
disabled && chipDisabled,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 disabled를 비활성 스타일을 적용하는 데만 사용하고, 실제 <button>에는 전달하지 않고 있는 것 같아요.

<button disabled={disabled}>가 되도록 native 속성도 함께 전달해 주면 좋을 것 같습니다 ~~

className,
)}
Expand Down
Loading