Feat(design-system): Button 컴포넌트 구현#32
Conversation
Walkthrough스토리북 Button 스토리 네임스페이스를 Components로 변경하고 다양한 스토리(variant/size/disabled/onClick)를 추가했으며, Button 컴포넌트에 cva 기반의 variant/size/isDisabled props, className 병합 및 disabled 전달 로직을 도입했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Storybook
participant Button
participant Browser
User->>Storybook: 스토리 선택 (예: "variant: secondary")
Storybook->>Button: props 전달 (variant, size, isDisabled, children, onClick)
Note right of Button #E8F6EF: cva로 클래스 계산\ncn으로 className 병합
Button->>Browser: 렌더 <button disabled={isDisabled} class="...">
alt onClick 제공
User-->>Browser: click
Browser-->>Button: 이벤트 수신
Button-->>Storybook: onClick 실행 (예: alert)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
✅ Storybook chromatic 배포 확인: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
packages/design-system/src/components/button/Button.tsx (3)
4-8: API 정합성:isDisabled대신 HTML 표준disabled사용을 우선 고려Button은 네이티브
<button>속성과의 일관성이 중요합니다. 커스텀isDisabled는 소비자 입장에서 혼란을 만들 수 있고, 스토리/컨트롤/테스트에서도 이중 표기 부담을 줍니다. 가능한 한disabled를 1차 API로 사용하고,isDisabled는 (있어야 한다면) alias로만 동작하도록 하거나 제거를 권장합니다.아래와 같이 타입을 조정하면 호환성과 일관성을 확보할 수 있습니다(점진적 이전을 위해 JSDoc으로 deprecate 표기):
interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> { - variant?: 'primary' | 'secondary'; - size?: 'small' | 'medium' | 'large'; - isDisabled?: boolean; + variant?: 'primary' | 'secondary'; + size?: 'small' | 'medium' | 'large'; + /** + * @deprecated disabled를 사용하세요. isDisabled는 추후 제거될 예정입니다. + */ + isDisabled?: boolean; }추가로 컴포넌트 내부에서
disabled와isDisabled를 병합 처리하는 로직을 아래 코멘트(라인 33-47)에서 제안합니다.
10-31: 키보드 접근성: focus-visible 스타일 추가 권장현재 hover/active/disabled 상태는 있으나 키보드 포커스가 명확히 드러나지 않습니다.
focus-visible:아웃라인(또는 ring) 스타일을 기본 클래스로 추가해 주세요. 이는 WCAG 포커스 가시성 요구사항 충족에 도움됩니다.- const buttonVariants = cva( - 'cursor-pointer rounded-[0.4rem] disabled:cursor-not-allowed w-full', + const buttonVariants = cva( + 'cursor-pointer rounded-[0.4rem] disabled:cursor-not-allowed w-full focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-gray500 transition-colors',참고: 토큰 시스템을 사용 중이므로
outline-gray500은 디자인 토큰 체계에 맞는 값으로 조정하세요(예:outline-brand등).
53-53: default export는 유지 OK구현이 안정화되면 named export(
export { Button })도 병행 제공하면 트리쉐이킹/자동 임포트 측면에서 이점이 있습니다.packages/design-system/src/components/button/Button.stories.tsx (4)
8-16: 스토리 문서에 폭(w-full) 기본 정책을 명시해 주세요PR 노트에 적어주신 “기본적으로 버튼에 w-full을 적용” 방침을 문서에도 함께 노출하면 소비자가 더 빨리 이해할 수 있습니다.
description: { component: - '**애플리케이션 전반에서 사용될 기본 버튼입니다.** `variant`로 주/보조 스타일을, `size`로 크기를, `isDisabled`로 비활성화 상태를 제어합니다.', + '**애플리케이션 전반에서 사용될 기본 버튼입니다.** `variant`로 주/보조 스타일을, `size`로 크기를, `isDisabled`로 비활성화 상태를 제어합니다. 기본적으로 `w-full`이 적용되며, 컨테이너의 너비에 맞춰 크기가 결정됩니다. 필요 시 `className`으로 너비를 오버라이드하세요.', },추가로 예시 스토리에
className="w-[240px]"변형 하나를 두면 발견성이 더 좋아집니다.
83-98: Disabled 스토리의 커스텀 렌더 패턴 적절주/보조 변형을 한 화면에서 비교할 수 있어 유용합니다. 추후 접근성 점검을 위해 키보드 탭 포커스 동작도 확인 가능한
play함수 추가를 고려해 주세요.예시:
export const Disabled: Story = { // ... play: async ({ canvasElement }) => { const canvas = within(canvasElement); await userEvent.tab(); // 포커스 표시 확인 }, };(필요 시
@storybook/test에서within,userEvent임포트)
100-108: Storybook에서alert사용은 지양 — actions 또는fn사용 권장
alert는 테스트/CI와 상호작용이 좋지 않습니다. Storybook Actions 패널에 로깅되도록fn을 사용하거나 기존action: 'clicked'를 그대로 활용해 주세요.+import { fn } from '@storybook/test'; export const WithAlertOnClick: Story = { name: 'onClick with Alert', args: { - children: 'onClick test', - onClick: () => { - alert('버튼이 클릭되었습니다.'); - }, + children: 'onClick test', + onClick: fn('clicked'), }, };
4-6: 타입 안전성 강화:satisfies Meta<typeof Button>패턴 권장명시적 타입 주석 대신
satisfies를 사용하면 잘못된args/argTypes선언 시 컴파일 타임에 잡히고,meta객체의 과도한 확장도 방지됩니다.-const meta: Meta<typeof Button> = { +const meta = { title: 'Components/Button', component: Button, tags: ['autodocs'], // ... -}; +} satisfies Meta<typeof Button>; export default meta;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/design-system/src/components/button/Button.stories.tsx(1 hunks)packages/design-system/src/components/button/Button.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#2
File: pnpm-workspace.yaml:3-3
Timestamp: 2025-08-18T13:48:59.065Z
Learning: constantly-dev는 docs 디렉터리를 컨벤션 문서 추가용으로 사용할 예정이라고 명시했습니다.
🧬 Code graph analysis (1)
packages/design-system/src/components/button/Button.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
🔇 Additional comments (2)
packages/design-system/src/components/button/Button.stories.tsx (2)
36-37: 컨트롤 테이블에서className숨김 처리 Good소비자가 스타일 침투보다 공식 variant/size를 우선 사용하도록 유도하는 데 적절합니다.
51-57: Primary 스토리 자체는 간결하고 적절기본 variant를 명시적으로 보여주는 예시로 충분합니다.
| @@ -1,10 +1,50 @@ | |||
| import { cn } from '@/lib'; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
cn import 경로(바렐) 확인 필요
@/lib가 cn을 재노출(re-export)하지 않으면 빌드가 실패합니다. 현재 구현대로라면 @/lib/utils에서 직접 가져오거나, @/lib/index.ts에서 cn을 내보내도록 보장해야 합니다.
아래 스크립트로 재노출 여부를 점검해 주세요. 기대값: lib 폴더 어딘가에서 cn이 export 되거나, index.*에서 utils를 재노출.
만약 재노출이 없다면 다음 중 하나로 수정해 주세요.
- import { cn } from '@/lib';
+ import { cn } from '@/lib/utils';또는 packages/design-system/src/lib/index.ts에서:
+ export * from './utils';🏁 Script executed:
#!/bin/bash
set -euo pipefail
# cn 선언/재노출 위치 확인
rg -n -C1 -S --type ts --type tsx '\bexport\b.*\bcn\b' packages/design-system/src/lib || true
# index에서 utils 재노출 확인
rg -n -S --type ts --type tsx 'export\s+\*\s+from\s+["'\'']\.\/utils["'\'']' packages/design-system/src/lib || trueLength of output: 391
아래 스크립트로 다시 한 번 cn의 선언/재노출 위치를 점검해주세요.
– 이번에는 파일 타입 제한을 제거해 범용 검색을 수행합니다.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) packages/design-system/src/lib 내부 전체에서 'export ... cn' 검색
rg -n -C1 '\bexport\b.*\bcn\b' packages/design-system/src/lib || echo "cn export 없음"
# 2) packages/design-system/src/lib/index.* 에서 utils 재노출 확인
rg -n 'export \* from [\'"]\.\/utils[\'"]' packages/design-system/src/lib || echo "utils 재노출 없음"Length of output: 414
아래 스크립트로 index.ts(x)에서 utils 재노출 여부를 다시 확인합니다.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) lib 디렉터리에서 인덱스 파일 찾기
INDEX_FILES=$(find packages/design-system/src/lib -maxdepth 1 -type f \( -name 'index.ts' -o -name 'index.tsx' \))
if [ -z "$INDEX_FILES" ]; then
echo "index 파일이 없습니다."
exit 0
fi
echo "찾은 인덱스 파일:"
echo "$INDEX_FILES"
# 2) 각 인덱스 파일에서 utils 재노출 확인
for f in $INDEX_FILES; do
echo "파일: $f"
grep -n -E "export \* from ['\"]/utils['\"]" "$f" || echo "utils 재노출 없음"
doneLength of output: 592
cn 재노출(바렐) 누락 확인 – 조치 필요
검증 결과, packages/design-system/src/lib/utils.ts에서 export const cn 구현은 확인되었으나, packages/design-system/src/lib/index.ts에 utils 재노출이 빠져 있어
import { cn } from '@/lib' 구문이 빌드 오류를 일으킵니다. 아래 두 가지 중 한 가지 방법으로 반드시 수정해 주세요.
• 직접 utils 경로에서 가져오기
• 파일: packages/design-system/src/components/button/Button.tsx (라인 1)
• 변경 예시:
diff - import { cn } from '@/lib'; + import { cn } from '@/lib/utils';
• lib 인덱스 파일에 utils 재노출 추가
• 파일: packages/design-system/src/lib/index.ts
• 변경 예시:
diff + export * from './utils';
📝 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.
| import { cn } from '@/lib'; | |
| ++ b/packages/design-system/src/components/button/Button.tsx | |
| @@ -1,1 +1,1 | |
| import { cn } from '@/lib/utils'; |
🤖 Prompt for AI Agents
In packages/design-system/src/components/button/Button.tsx around line 1, the
import "import { cn } from '@/lib'" fails because
packages/design-system/src/lib/index.ts does not re-export utils; fix by either
changing this file to import cn directly from the utils module (import from
'@/lib/utils' or the correct relative path to utils.ts) or add a re-export in
packages/design-system/src/lib/index.ts that exports cn (export { cn } from
'./utils') so that '@/lib' exposes cn.
| const Button = ({ | ||
| variant = 'primary', | ||
| size, | ||
| children, | ||
| className, | ||
| isDisabled = false, | ||
| ...props | ||
| }: ButtonProps) => { | ||
| return ( | ||
| <button type="button" {...props}> | ||
| <button | ||
| type="button" | ||
| className={cn(buttonVariants({ variant, size }), className)} | ||
| disabled={isDisabled} | ||
| {...props} | ||
| > |
There was a problem hiding this comment.
중요: disabled 우선순위가 {...props}에 의해 덮여 비활성화 상태가 틀어질 수 있습니다
현재 {...props}가 마지막에 배치되어 있어, 소비자가 disabled를 전달하면 isDisabled로 설정한 값이 덮여 버립니다. 예: <Button isDisabled onClick=... disabled={false} />인 경우 실제 DOM에는 disabled={false}가 적용될 수 있습니다. 또한 type="button"도 {...props}에 의해 임의로 덮일 수 있어 의도하지 않은 submit이 발생할 여지가 있습니다.
아래처럼 type과 disabled를 구조분해 후 병합하고, 나머지를 rest로 분리해 충돌을 제거하세요. 이때 isDisabled가 최우선으로 적용되도록 하는 것을 권장합니다.
-const Button = ({
- variant = 'primary',
- size,
- children,
- className,
- isDisabled = false,
- ...props
-}: ButtonProps) => {
+const Button = ({
+ variant = 'primary',
+ size,
+ children,
+ className,
+ isDisabled,
+ type,
+ disabled,
+ ...rest
+}: ButtonProps) => {
return (
<button
- type="button"
- className={cn(buttonVariants({ variant, size }), className)}
- disabled={isDisabled}
- {...props}
+ type={type ?? 'button'}
+ className={cn(buttonVariants({ variant, size }), className)}
+ disabled={isDisabled ?? disabled}
+ {...rest}
>
{children}
</button>
);
};추가 제안(선택): ref 전달이 필요한 시나리오를 위해 forwardRef로 감싸는 것도 고려해 주세요.
+import React from 'react';
-const Button = ({ ... }: ButtonProps) => {
+const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
+ ({ ...props }, ref) => {
+ const { /* 위 병합 로직 동일 */ } = props;
+ return (
+ <button ref={ref} /* ... */ />
+ );
+ }
+);📝 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.
| const Button = ({ | |
| variant = 'primary', | |
| size, | |
| children, | |
| className, | |
| isDisabled = false, | |
| ...props | |
| }: ButtonProps) => { | |
| return ( | |
| <button type="button" {...props}> | |
| <button | |
| type="button" | |
| className={cn(buttonVariants({ variant, size }), className)} | |
| disabled={isDisabled} | |
| {...props} | |
| > | |
| const Button = ({ | |
| variant = 'primary', | |
| size, | |
| children, | |
| className, | |
| isDisabled, | |
| type, | |
| disabled, | |
| ...rest | |
| }: ButtonProps) => { | |
| return ( | |
| <button | |
| type={type ?? 'button'} | |
| className={cn(buttonVariants({ variant, size }), className)} | |
| disabled={isDisabled ?? disabled} | |
| {...rest} | |
| > | |
| {children} | |
| </button> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In packages/design-system/src/components/button/Button.tsx around lines 33 to
47, the spread {...props} placed last allows consumer-supplied type/disabled to
override component intent (so isDisabled or default type="button" can be
clobbered); fix by destructuring type and disabled (e.g. const { type, disabled,
...rest } = props) and then explicitly set type={type ?? "button"} and
disabled={isDisabled ?? disabled} (or disabled={Boolean(isDisabled) ||
Boolean(disabled)} with isDisabled taking precedence) and spread the remaining
rest props afterward; optionally wrap the component with forwardRef to support
ref forwarding.
jllee000
left a comment
jllee000
left a comment
There was a problem hiding this comment.
고생하셨습니다! 간단한 질문들만 확인해주세요!
| size, | ||
| children, | ||
| className, | ||
| isDisabled = false, | ||
| ...props |
There was a problem hiding this comment.
질문있습니당
요거 props 형태로 받는 className는 외부에서 어떤 방식으로 주면 되는걸까요?
There was a problem hiding this comment.
일반 tailwind css 스타일 주듯이 똑같이 넘겨주시면 됩니다!! 기본 스타일은 거의 지정이 되어있어서 PR 내용처럼 width 정도 주면 될 것 같아요 👍
| const buttonVariants = cva( | ||
| 'cursor-pointer rounded-[0.4rem] disabled:cursor-not-allowed w-full', | ||
| { |
There was a problem hiding this comment.
w-full로 화면에 따라 그냥 자동 반응형으로 맞춰지니 레이아웃 맞추기 편리하겠네요!
혹시 그럼 따로 버튼 width 설정이 필요한 경우는 무조건 부모 컨테이너로 계속 감싸야하는거죠? 버튼 크기가 상황에 따라 계속 달라진다면,, width 값을 props로 줘서 제어하는 거랑 필요시에 계속 width 있는 컨테이너로 wrapping는 것 중 어느 것이 더 유연성 있는 방향일지 고민되는 포인트 같습니다!
There was a problem hiding this comment.
맞습니다! 그렇기 때문에 width값도 className으로 주입 받을 수 있도록 설정했어요.
width 값을 props로 줘서 제어하는 거 vs 필요시에 계속 width 있는 컨테이너로 wrapping
말씀해주신 것 중에 저는 후자를 선택했는데 PR에 작성한 것처럼 className으로 width 값을 주면 전자 props 제어랑 똑같은 방법으로 커스텀이 돼서 선택을 했어요.
재림님은 어떻게 생각하시나요??
There was a problem hiding this comment.
아하 그럼, 사실 상 width 커스텀 시 후자(wrapping)랑 전자(className) 방안이 다 되는 식으로 열려있군용
버튼 컴포넌트는 앞으로 많이 여기저기 쓸 것 같아서! width 지정하는 방법을 통일하면 좋을 것 같아요! 저는 개인적으로 전자 방식이 좀더 width를 커스텀해준다는 유연한 느낌도 주고, 추가 컨테이너로 인한 구조가 필요없으니 코드가 복잡해지지 않을 것 같아요!
There was a problem hiding this comment.
만약 전자로 하게 되면 어떤 식으로 넘겨야 할까요?
'10rem' 이런 형태 혹은 숫자만 넘겨서 단위를 안에서 붙여서 쓰는 방법..!? props로 할 때 내부 처리가 조금 고민이네요.
There was a problem hiding this comment.
개인적으로 어차피 props에 넣어서 커스텀 유연성을 높이는 의미를 유지하려면, 아예 단위 자체로 '10rem'으로 넘겨서 그자체로 스타일 먹여주는게 좋을 것 같아요!
(그럴리는 없지만 혹시 뭐 부모요소나 디바이스에 맞춘 스타일링 적용하면서 70%, 10dvh 등의 단위도 쓸 수 있으니..)
jjangminii
left a comment
jjangminii
left a comment
There was a problem hiding this comment.
ui보면 글자에 따로 px값이 없어 보이는데 이것도 외부에서 넘겨줘야하는걸까요?
width값을 주입하기 때문에 그렇습니다. PR내용과 아래 comment 확인! |
3 participants
📌 Related Issues
✅ 체크 리스트
📄 Tasks
⭐ PR Point (To Reviewer)
기본적으로 버튼에
w-full속성을 줘서 버튼 사용 시 외부 container에width값으로 버튼 크기 조절하시면 됩니다.혹시나
container에 주기 애매한 경우는 button className에width값을 주입해서 커스텀 하시면 됩니다.UI 확인은 Chromatic을 확인해주세요 👍
Summary by CodeRabbit