Feat(design-system): progress bar 구현#25
Conversation
Walkthrough디자인 시스템에 Radix 기반 Progress 컴포넌트를 추가하고 Storybook 스토리를 도입했으며, 패키지 의존성에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Consumer (App)
participant P as DesignSystem.Progress
participant R as Radix.ProgressPrimitive
participant D as Browser DOM
C->>P: render <Progress value, variant, className?>
note right of P: value를 0–100으로 클램프\nvariant에 따라 클래스 선택
P->>R: <Root max=100 value=clamped ...props>
P->>R: <Indicator style=width:clamped% class=variant>
R-->>D: role="progressbar" aria-valuenow=clamped
D-->>C: 시각적 트랙/인디케이터 렌더링
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
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/package.json (1)
57-57: @radix-ui/react-progress 의존성 관리 권장v1.1.7의
peerDependencies를 확인한 결과, React 및 React DOM에 대해^16.8 || ^17.0 || ^18.0 || ^19.0 || 19.0.0-rc범위로 지정되어 있어 React 19.1.0과도 호환됩니다. 디자인 시스템 패키지 특성상, 소비 애플리케이션에서 중복 번들을 방지하려면 아래와 같이dependencies가 아닌peerDependencies(+devDependencies)로 옮기는 방식을 고려해 주세요.
- 위치:
packages/design-system/package.json(현재 57행)제안하는 변경 예시:
{ "dependencies": { "@pivanov/vite-plugin-svg-sprite": "^3.1.3", - "@radix-ui/react-progress": "^1.1.7", "@radix-ui/react-slot": "^1.2.3", "@radix-ui/react-switch": "^1.2.6", ... }, + "peerDependencies": { + "@radix-ui/react-progress": "^1.1.7" + }, + "devDependencies": { + "@radix-ui/react-progress": "^1.1.7" + } }이렇게 구성하면:
- 소비 애플리케이션이 자신의 React 버전에 맞춰
@radix-ui/react-progress를 설치하도록 유도- 디자인 시스템 패키지 번들 크기 최소화
위 변경을 검토해 보시길 권장드립니다.
packages/design-system/src/components/progress/progress.tsx (3)
16-27: 작은 스타일 중복 제거: tree variant에서 indicator 높이 중복
indicator기본 클래스에h-full이 있고, track(tree)에서 이미h-[1.2rem]을 지정합니다. indicator(tree)의h-[1.2rem]는 중복/덮어쓰기라 제거 가능합니다. 가독성↑, 유지보수 비용↓.제안 수정:
const indicator = cva( 'h-full rounded-full transition-[width] duration-300 ease-out', { variants: { variant: { profile: 'bg-main400', - tree: 'h-[1.2rem] bg-gradient-to-r from-gradient-start to-gradient-end', + tree: 'bg-gradient-to-r from-gradient-start to-gradient-end', }, },
35-53: ref 전달 지원으로 조합성 향상라이브러리 컴포넌트는 ref 전달을 지원하는 편이 좋습니다(포커스 관리, 측정 등). Radix Root에 ref를 연결하도록
forwardRef전환을 권장합니다. 외부 API 표면은 그대로 유지됩니다.예시 수정:
-type RootProps = React.ComponentProps<typeof ProgressPrimitive.Root>; +type RootProps = React.ComponentProps<typeof ProgressPrimitive.Root>; +type RootRef = React.ElementRef<typeof ProgressPrimitive.Root>; -export function Progress({ className, variant, value, ...props }: Props) { +export const Progress = React.forwardRef<RootRef, Props>(function Progress( + { className, variant, value, ...props }, + ref +) { const raw = typeof value === 'string' ? Number.parseFloat(value) : value; const safe = Number.isFinite(raw) ? raw : 0; const v = Math.max(0, Math.min(100, safe)); return ( <ProgressPrimitive.Root + ref={ref} data-slot="progress" className={cn(track({ variant }), className)} value={v} max={100} {...props} > <ProgressPrimitive.Indicator data-slot="progress-indicator" className={indicator({ variant })} style={{ width: `${v}%` }} /> </ProgressPrimitive.Root> ); -} +});
6-14: 사소한 포맷 통일 제안형식 일관성을 위해
packages/design-system/src/components/progress/progress.tsx의tree변형에서- 'h-[1.2rem] bg-gray100' + 'h-[1.2rem] bg-gray100'처럼 이중 공백을 단일 공백으로 수정해 주세요.
나머지 색상 토큰(
bg-gray100·bg-main400·from-gradient-start·to-gradient-end)은
packages/tailwind-config/shared-styles.css와 디자인 시스템 유틸(utils.ts,icon.tsx등)에서 정의되어 있어 문제없음을 확인했습니다.—
packages/design-system/src/components/progress/progress.stories.tsx (3)
71-80: Chromatic 스냅샷/플레이 환경에서 애니메이션 플리키 완화
setInterval기반 자동 진행은 스냅샷 타이밍에 따라 변동이 생길 수 있습니다. 해당 스토리는 상호작용 테스트(play)를 수행하지 않으므로 괜찮을 수 있으나, 안정성을 높이려면 캡처 지연/정지 옵션을 고려해 주세요.옵션 예시:
parameters: { chromatic: { pauseAnimationAtEnd: true } }또는- mount 시 최초 1초 정지 후 진행, 혹은 스토리 이름에 “demo-only” 명시하여 비시각 스냅샷 제외
127-140: 플레이 단언 강화: DOM 업데이트 동기화와 접근성 쿼리 활용현재
getAttribute비교는 충분히 동작하지만, 테스트 의도를 더 명확히 하려면 접근성 쿼리와 jest-dom 매처를 사용하는 편이 읽기 쉽습니다.예시:
- const roots = canvasElement.querySelectorAll('[data-slot="progress"]'); - roots.forEach((el) => expect(el.getAttribute('aria-valuenow')).toBe('40')); + const roots = await canvas.findAllByRole('progressbar'); + roots.forEach((el) => expect(el).toHaveAttribute('aria-valuenow', '40'));또는 비동기 반영 보장을 위해
await를 한 번 더 양념하는 것도 방법입니다.
51-69: 경계값 시나리오 추가 제안Zero/Full 스토리는 좋습니다. “입력 초과/미만 → 클램프”를 보여주는 스토리도 있으면 API 보증을 문서로 남기기에 좋습니다.
예시:
export const Clamped: Story = { args: { value: 120 }, // 또는 -10 render: (args) => <Frame {...args} />, };
📜 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 ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/design-system/package.json(1 hunks)packages/design-system/src/components/progress/progress.stories.tsx(1 hunks)packages/design-system/src/components/progress/progress.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/design-system/src/components/progress/progress.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
packages/design-system/src/components/progress/progress.stories.tsx (1)
packages/design-system/src/components/progress/progress.tsx (1)
Progress(35-53)
| }; | ||
|
|
||
| export function Progress({ className, variant, value, ...props }: Props) { | ||
| const v = Math.max(0, Math.min(100, Number(value))); |
There was a problem hiding this comment.
NaN 전파 가능성: 문자열 입력 시 width/aria-valuenow가 깨질 수 있습니다
Number(value)가 NaN이면 Math.max/min 결과도 NaN이 되어 style={{ width: 'NaN%' }}와 aria-valuenow="NaN"이 발생할 수 있습니다. 스토리에서는 숫자를 쓰고 있으나, 공개 API가 string을 허용하므로 실제 사용처에서 쉽게 재현됩니다. 안전 가드가 필요합니다.
권장 수정:
-export function Progress({ className, variant, value, ...props }: Props) {
- const v = Math.max(0, Math.min(100, Number(value)));
+export function Progress({ className, variant, value, ...props }: Props) {
+ const raw =
+ typeof value === 'string' ? Number.parseFloat(value) : value;
+ const safe = Number.isFinite(raw) ? raw : 0;
+ const v = Math.max(0, Math.min(100, safe));📝 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 v = Math.max(0, Math.min(100, Number(value))); | |
| export function Progress({ className, variant, value, ...props }: Props) { | |
| const raw = | |
| typeof value === 'string' ? Number.parseFloat(value) : value; | |
| const safe = Number.isFinite(raw) ? raw : 0; | |
| const v = Math.max(0, Math.min(100, safe)); |
🤖 Prompt for AI Agents
In packages/design-system/src/components/progress/progress.tsx around line 36,
Number(value) can produce NaN for string/invalid inputs which then yields style
width: 'NaN%' and aria-valuenow="NaN"; replace the direct Number(value) usage
with a safe parse that defaults to 0 when the result is NaN (e.g., const raw =
Number(value); const num = Number.isFinite(raw) ? raw : 0), then clamp num to
the 0–100 range with Math.max/Math.min and use that safe numeric value for both
style width and aria-valuenow so invalid inputs no longer propagate NaN.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/design-system/src/components/progress/progress.stories.tsx (4)
129-137: play 함수의 비동기 업데이트 안정화 및 쿼리 방식 개선.클릭 직후의 React 상태 반영 타이밍에 따라 드물게 레이스가 생길 수 있습니다. 또한 DOM 셀렉터 대신 접근성 역할(Role) 기반 쿼리를 쓰면 구조 변경에도 강인해집니다.
다음과 같이
waitFor를 도입하고,getAllByRole('progressbar')와toHaveAttribute매처를 사용해 주세요.-import { within, userEvent, expect } from '@storybook/test'; +import { within, userEvent, expect, waitFor } from '@storybook/test'; @@ - play: async ({ canvasElement }) => { - const canvas = within(canvasElement); - const btn = await canvas.findByRole('button', { name: /\+20% \(both\)/i }); - await userEvent.click(btn); - await userEvent.click(btn); - - const roots = canvasElement.querySelectorAll('[data-slot="progress"]'); - roots.forEach((el) => expect(el.getAttribute('aria-valuenow')).toBe('40')); - }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const btn = await canvas.findByRole('button', { name: /\+20% \(both\)/i }); + await userEvent.click(btn); + await userEvent.click(btn); + + await waitFor(() => { + const roots = canvas.getAllByRole('progressbar'); + roots.forEach((el) => expect(el).toHaveAttribute('aria-valuenow', '40')); + }); + },Also applies to: 3-3
15-17: Docs에 값 클램핑(0–100) 명시 추가 제안.구현(Progress.tsx)에서
value가 0–100으로 클램핑되는 점을 문서에 명시하면 사용자가 범위를 오해하지 않습니다.아래처럼 한 줄을 보강해 주세요.
'단일 로직을 공유하고 **variant**로 스타일만 분기하는 progress입니다.\n' + '- `variant="profile"`: 얇은 트랙 + 단색 인디케이터 (기본)\n' + - '- `variant="tree"`: 두꺼운 트랙 + 그라데이션 인디케이터\n', + '- `variant="tree"`: 두꺼운 트랙 + 그라데이션 인디케이터\n' + + '- 모든 value는 컴포넌트 내부에서 0–100으로 클램핑됩니다.\n',
81-101: Auto 데모의 타이머 관리 적절함(LGTM) + 미세 개선 아이디어.
setInterval과 클린업 처리가 적절합니다. 추가로, 크로스탭 집중 손실 시 CPU 점유를 더 줄이고 싶다면setTimeout루프로 전환하거나 간격을 조금 늘리는 것도 고려 가능합니다(선택 사항).
44-48: 접근성 이름 부여 고려(선택).현재
role="progressbar"요소에 접근성 이름이 없어 스크린리더에서 식별이 모호할 수 있습니다. 스토리 레벨에서만이라도aria-label을 부여하면 데모와 테스트 모두 더 명확해집니다.예시:
-const Frame: React.FC<React.ComponentProps<typeof Progress>> = (props) => ( +const Frame: React.FC<React.ComponentProps<typeof Progress>> = (props) => ( <div style={{ width: 320 }}> - <Progress {...props} /> + <Progress aria-label={props.variant ?? 'profile'} {...props} /> </div> ); @@ - render: (args) => <Frame {...args} />, + render: (args) => <Frame {...args} aria-label="profile" />, @@ - render: (args) => <Frame {...args} />, + render: (args) => <Frame {...args} aria-label="tree" />, @@ - <Progress value={v} variant="profile" /> - <Progress value={v} variant="tree" /> + <Progress value={v} variant="profile" aria-label="profile" /> + <Progress value={v} variant="tree" aria-label="tree" />Also applies to: 50-68, 102-108
📜 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 (1)
packages/design-system/src/components/progress/progress.stories.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/design-system/src/components/progress/progress.stories.tsx (1)
packages/design-system/src/components/progress/progress.tsx (1)
Progress(35-53)
🔇 Additional comments (1)
packages/design-system/src/components/progress/progress.stories.tsx (1)
102-124: 상호작용 스토리 구성 훌륭함(LGTM).두 Progress를 동일 상태로 동기화하고 조작 단추로 값을 변화시키는 데모는 사용성/명세 검증에 매우 유용합니다. 불필요한 복잡성 없이 목적에 부합합니다.
| @@ -0,0 +1,138 @@ | |||
| import type { Meta, StoryObj } from '@storybook/react-vite'; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Storybook 타입 import 경로 및 Meta 타이핑 방식 수정 권장(빌드 실패 가능성).
Meta, StoryObj 타입은 통상 @storybook/react에서 가져옵니다. @storybook/react-vite로부터 타입을 import하면 환경에 따라 트랜스파일/타입체크가 실패할 수 있습니다. 또한 Storybook 8 권장 패턴인 satisfies Meta<typeof ...>를 사용하면 메타 객체 검증이 더 견고해집니다.
아래처럼 변경해 주세요:
-import type { Meta, StoryObj } from '@storybook/react-vite';
+import type { Meta, StoryObj } from '@storybook/react';
@@
-const meta: Meta<typeof Progress> = {
+const meta = {
title: 'Components/Progress',
component: Progress,
tags: ['autodocs'],
parameters: {
@@
-};
+} satisfies Meta<typeof Progress>;다른 스토리 파일에서도 동일한 경로를 사용 중인지 확인해 주세요:
Also applies to: 6-6, 39-39
🏁 Script executed:
#!/bin/bash
# storybook 타입 import 경로 점검
rg -nP "from\s+'@storybook/react-vite'" --type ts --type tsx -C2Length of output: 100
다른 .ts/.tsx 스토리 파일에서 @storybook/react-vite를 import 하고 있는지 재확인하기 위해 아래 스크립트를 실행해주세요:
🏁 Script executed:
#!/bin/bash
# storybook 타입 import 경로 점검 (ts / tsx 파일 대상)
rg -nP "from\s+['\"]@storybook/react-vite['\"]" -g '*.ts' -g '*.tsx' -C2Length of output: 1122
스토리북 타입 import 경로 및 Meta 타이핑 방식 수정 필요
아래 스토리 파일들이 현재 @storybook/react-vite로부터 Meta, StoryObj를 import 하고 있어, 빌드 실패나 타입체크 오류가 발생할 수 있습니다. Storybook 8 권장 패턴인 satisfies Meta<typeof …>를 사용하고, 타입은 @storybook/react에서 가져오도록 변경해주세요.
• packages/design-system/src/components/progress/progress.stories.tsx
• packages/design-system/src/components/input/Input.stories.tsx
• packages/design-system/src/components/button/Button.stories.tsx
예시(diff):
-import type { Meta, StoryObj } from '@storybook/react-vite';
+import type { Meta, StoryObj } from '@storybook/react';
@@
-const meta: Meta<typeof Progress> = {
+const meta = {
title: 'Components/Progress',
component: Progress,
tags: ['autodocs'],
parameters: {
@@
-};
+} satisfies Meta<typeof Progress>;위와 동일한 방식으로 세 파일 모두 적용해 주세요.
🤖 Prompt for AI Agents
In packages/design-system/src/components/progress/progress.stories.tsx (and also
apply the same change to
packages/design-system/src/components/input/Input.stories.tsx and
packages/design-system/src/components/button/Button.stories.tsx), replace the
Storybook type imports from '@storybook/react-vite' with imports from
'@storybook/react' and change the default export typing to use the Storybook 8
pattern: export default ... satisfies Meta<typeof Component>; keep StoryObj
usage for individual stories but import it from '@storybook/react' as well;
update any existing Meta/StoryObj references accordingly and remove the
'@storybook/react-vite' import to fix type errors.
좋은 접근 같아요 👍 |
constantly-dev
left a comment
constantly-dev
left a comment
There was a problem hiding this comment.
수고하셨습니다 👍 깔끔하게 구현 잘 해주셨네요.
코멘트 몇 개 달았으니 확인 부탁드려요~~
그리고 추가로 지금은 단순 넘겨주는 value에 따라 정적으로 progress 진행도/바가 보일 수도 있지만 추후 애니메이션 형태로 값이 늘어나거나 줄어드는 것이 추가로 고려해야 한다면 최적화가 필요할 수 있을 것 같아요!
브라우저 렌더링 과정에서 리플로우, 리페인트 과정이 성능에 영향이 많이 가는데 transition등으로 width를 조절해서 애니메이션을 구현하면 2개 과정을 사용해서 뚝뚝 끊기는 경우가 생길 수 있을 것 같아요.
bandicam.2025-08-24.22-31-34-160.mp4
따라서 transform과 같이 GPU를 사용하는 등의 애니메이션 최적화 관련해서 공부해보고 적용해보는 것도 좋을 것 같아요 👍 굿굿~~~~
| type RootProps = React.ComponentProps<typeof ProgressPrimitive.Root>; | ||
| type Props = Omit<RootProps, 'value' | 'max'> & | ||
| VariantProps<typeof track> & { | ||
| value: number | string; | ||
| }; |
There was a problem hiding this comment.
textarea PR 리뷰 참고하셔서 컨벤션 맞게 변경해주세요!
| type RootProps = React.ComponentProps<typeof ProgressPrimitive.Root>; | ||
| type Props = Omit<RootProps, 'value' | 'max'> & | ||
| VariantProps<typeof track> & { | ||
| value: number | string; |
There was a problem hiding this comment.
value를 어떻게 받을지 정하셔서 타입을 더 구체화하는 것이 좋을 것 같아요! number 혹은 string 중 하나로!
특별한 이유가 있으시다면 궁금합니다!
There was a problem hiding this comment.
string을 받게 했던 이유는 간혹 문자열이 들어오는 경우를 예방하려했는데 사용처에서 숫자로 보정하는 게 더 깔끔할것같네요-!! 수정하겠습니다
| }; | ||
|
|
||
| export function Progress({ className, variant, value, ...props }: Props) { | ||
| const v = Math.max(0, Math.min(100, Number(value))); |
There was a problem hiding this comment.
조금 더 의미 있는 네이밍으로 바꾸면 어떨까요?
There was a problem hiding this comment.
이 부분은 확실히 수정하는편이 좋을거 같아요-!! shadecn을 사용해서 만들다보니 이런 부분은 넘긴 부분이 있는거같아요 앞으로 더 주의하겠습니다
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/design-system/src/components/progress/progress.tsx (2)
29-36:value를number로 한정한 결정 굿 — API 명확성/오용 방지에 효과적입니다이전 리뷰에서 권장되던 타입 축소가 반영되어, 사용처에서의 파싱 책임이 명확해졌고 컴포넌트의 역할(시각화 전담)이 분리되었습니다. 향후 불확정(indeterminate) 상태 지원이 필요하면
value?: number로 확장하는 방향을 고려해도 됩니다.
44-45: NaN/Infinity 입력 시NaN%스타일·aria-valuenow="NaN"가능성 — 안전 가드 추가 필요현재 클램핑은 유효 숫자라는 전제하에 동작합니다. 그러나
value가NaN/Infinity이면Math.min/max결과도NaN이 되어 인라인 스타일이 깨지며 접근성 속성도 비정상이 됩니다. 방어적으로 유한수만 허용하고, 그렇지 않으면 0으로 폴백하세요.권장 수정:
- const progressPercent = Math.max(0, Math.min(100, value)); + const raw = Number.isFinite(value) ? value : 0; + const progressPercent = Math.min(100, Math.max(0, raw));
🧹 Nitpick comments (3)
packages/design-system/src/components/progress/progress.tsx (3)
38-61: ref 전달 누락 — Radix 컴포넌트 패턴에 맞춰forwardRef로 래핑 권장테스트 및 포커스/측정(예: IntersectionObserver, scrollIntoView) 용이성과 Radix 컴포넌트 일관성을 위해
ref를 루트에 전달하는 것이 일반적입니다. 또한 DevTools 가독성을 위해displayName도 지정하세요.-export function Progress({ - className, - variant, - value, - ...props -}: ProgressProps) { - const progressPercent = Math.max(0, Math.min(100, value)); - - return ( - <ProgressPrimitive.Root +export const Progress = React.forwardRef< + React.ElementRef<typeof ProgressPrimitive.Root>, + ProgressProps +>(({ className, variant, value, ...props }, ref) => { + const progressPercent = Math.max(0, Math.min(100, value)); + + return ( + <ProgressPrimitive.Root + ref={ref} data-slot="progress" className={cn(track({ variant }), className)} value={progressPercent} max={100} {...props} > <ProgressPrimitive.Indicator data-slot="progress-indicator" className={indicator({ variant })} style={{ width: `${progressPercent}%` }} /> </ProgressPrimitive.Root> - ); -} + ); +}); + +Progress.displayName = 'Progress';
16-27: 움직임 최소화 접근성 지원:prefers-reduced-motion대응모션 민감 사용자를 위해 폭 애니메이션을 비활성화하는 유틸리티를 추가하는 것을 권장합니다.
-const indicator = cva( - 'h-full rounded-full transition-[width] duration-300 ease-out', +const indicator = cva( + 'h-full rounded-full transition-[width] motion-reduce:transition-none duration-300 ease-out',
6-14: variant 값(profile/tree)이 사용처 맥락 종속 — 의미 기반 네이밍/별칭 검토 제안팀 러닝(semantic union 타입 선호)에 비춰보면, 현재 값은 “어디서 쓰이는가”를 드러내지만 “어떻게 보이는가”는 드러내지 않습니다. 디자인 변경/재사용 시 인지 부하가 커질 수 있어 다음 중 하나를 고려해보세요:
- 의미 기반으로 재명명: 예)
variant: 'thin' | 'gradient'- 기존 값 유지 + 별칭 도입:
'profile' | 'tree' | 'thin' | 'gradient'를 같은 스타일에 매핑- 최소한 JSDoc로 각 variant의 시각적 의도를 명시
이 변경은 API 안정성 요구도에 따라 추후 리팩터 우선순위를 결정하시면 됩니다.
Also applies to: 16-27
📜 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 (1)
packages/design-system/src/components/progress/progress.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-08T11:47:27.279Z
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#30
File: apps/extension/src/components/InfoBox/InfoBox.tsx:10-11
Timestamp: 2025-07-08T11:47:27.279Z
Learning: In React TypeScript components, prefer semantic prop names with union types over abstract version numbers. For example, use `size: 'small' | 'large'` instead of `version: 1 | 2` for better code readability, type safety, and extensibility.
Applied to files:
packages/design-system/src/components/progress/progress.tsx
🧬 Code graph analysis (1)
packages/design-system/src/components/progress/progress.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/design-system/src/components/index.ts (1)
4-4: DX 개선: 컴포넌트 타입도 배럴에서 재노출소비 측에서
ProgressProps(및 필요한 경우 variant 타입)를 함께 가져올 수 있도록 타입 재노출을 권장합니다. 트리쉐이킹에 영향 없고 사용성이 좋아집니다.export { default as Input } from './input/Input'; -export { Progress } from './progress/Progress'; +export { Progress } from './progress/progress'; +export type { ProgressProps } from './progress/progress';
📜 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/index.ts(1 hunks)packages/design-system/src/components/progress/progress.stories.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/design-system/src/components/progress/progress.stories.tsx
🔇 Additional comments (1)
packages/design-system/src/components/index.ts (1)
4-4: 내보내기 방식(named vs default) 확인 완료 — 변경 불필요Progress 컴포넌트는 packages/design-system/src/components/progress/progress.tsx 파일에서 named export(
export const Progress등)로 내보내지고 있어, 배럴(index.ts)의export { Progress } from './progress/Progress';구문을 그대로 유지하시면 됩니다.
| export { default as Button } from './button/Button'; | ||
| export { Switch } from './switch/switch'; | ||
| export { default as Input } from './input/Input'; | ||
| export { Progress } from './progress/Progress'; |
There was a problem hiding this comment.
Linux/CI에서 모듈 해석 실패 가능성: import 경로 대소문자 불일치
AI 요약에 따르면 구현 파일은 packages/design-system/src/components/progress/progress.tsx(소문자)인데, 현재 배럴에서는 ./progress/Progress(대문자)로 참조합니다. 케이스-센서티브 파일시스템(대부분의 CI/Linux)에서는 Module not found로 빌드가 깨질 수 있습니다. 우선 경로를 소문자로 맞추는 게 안전합니다.
적용 제안:
-export { Progress } from './progress/Progress';
+export { Progress } from './progress/progress';📝 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 { Progress } from './progress/Progress'; | |
| export { Progress } from './progress/progress'; |
🤖 Prompt for AI Agents
In packages/design-system/src/components/index.ts around line 4, the export path
uses './progress/Progress' while the actual implementation file is lowercase
(progress/progress.tsx); on case-sensitive filesystems this causes Module not
found errors. Update the export to match the real file path casing (use
'./progress/progress') so the import exactly matches the filesystem, and run a
local build or CI to verify the module resolves.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/design-system/src/components/switch/Switch.tsx (1)
14-14: 매직 넘버(치수/이동 거리) 제거: CSS 변수로 일반화 제안현재 고정값 h-[2rem] w-[4rem] 및 Thumb의 translate-x 수치가 사용처의 임의 사이즈 오버라이드와 충돌할 수 있습니다. Root에 CSS 변수로 기본값을 선언하고, Thumb 이동 거리를 calc(var(--..))로 계산하면 재사용성과 안정성이 올라갑니다. 사용처는 className에서 변수만 덮어쓰면 됩니다. (예: [--sw-h:2.5rem] [--sw-w:5rem] [--sw-thumb:2rem] [--sw-gap:0.25rem])
변경 제안(해당 라인 내 수정):
- 'data-[state=checked]:bg-main400 data-[state=unchecked]:bg-gray200 focus-visible:border-ring focus-visible:ring-ring/50 dark:data-[state=unchecked]:bg-input/80 shadow-xs peer inline-flex h-[2rem] w-[4rem] shrink-0 items-center rounded-full border border-transparent outline-none transition-all focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50', + 'data-[state=checked]:bg-main400 data-[state=unchecked]:bg-gray200 focus-visible:border-ring focus-visible:ring-ring/50 dark:data-[state=unchecked]:bg-input/80 shadow-xs peer inline-flex h-[var(--sw-h)] w-[var(--sw-w)] [--sw-h:2rem] [--sw-w:4rem] [--sw-thumb:1.6rem] [--sw-gap:0.2rem] shrink-0 items-center rounded-full border border-transparent outline-none transition-all focus-visible:ring-[3px] disabled:cursor-not-allowed disabled:opacity-50',Thumb 쪽 보완(참고 코드 — 라인 22 수정 필요):
// line 22 className 제안 'bg-background dark:data-[state=unchecked]:bg-foreground dark:data-[state=checked]:bg-primary-foreground pointer-events-none block size-[var(--sw-thumb)] rounded-full ring-0 transition-transform data-[state=checked]:translate-x-[calc(var(--sw-w)-var(--sw-thumb)-var(--sw-gap))] data-[state=unchecked]:translate-x-[var(--sw-gap)]'packages/design-system/src/components/progress/Progress.tsx (2)
49-49: variant 강제력을 높이려면 className 병합 순서를 바꾸세요현재 순서에서는 소비자가 전달한 className이 Tailwind Merge 규칙상 마지막 승리(last-wins)로 variant의 배경색/높이를 쉽게 덮어쓸 수 있습니다. 디자인 일관성을 지키려면 variant 클래스를 마지막에 두세요.
적용 diff:
- className={cn(track({ variant }), className)} + className={cn(className, track({ variant }))}
16-27: 애니메이션 비용 최적화: width → transform 전환 제안width 트랜지션은 레이아웃 계산을 유발합니다. transform 기반으로 바꾸면 GPU 가속 경로를 타서 부드럽고 효율적입니다. 또한 사용자 환경설정(모션 축소)을 존중해 transition을 끌 수 있게 하세요.
권장 diff(Indicator 클래스/스타일 조정):
-const indicator = cva( - 'h-full rounded-full transition-[width] duration-300 ease-out', +const indicator = cva( + 'h-full w-full rounded-full ease-out transition-transform motion-reduce:transition-none will-change-transform', { variants: { variant: { profile: 'bg-main400', tree: 'bg-gradient-to-r from-gradient-start to-gradient-end', }, }, defaultVariants: { variant: 'profile' }, } );- style={{ width: `${progressPercent}%` }} + style={{ transform: `translateX(-${100 - progressPercent}%)` }}트랙에 overflow-hidden이 이미 적용되어 있어 시각적 동작은 동일합니다.
Also applies to: 55-58
packages/design-system/src/components/progress/Progress.stories.tsx (2)
70-79: 데모 자동 진행 인터벌 완화 제안(선택)250ms 간격은 스토리북/테스트 러너 환경에서 그려지지 않은 프레임이 생길 수 있습니다. 400–500ms로 완화하면 CPU 부하와 깜빡임이 줄어듭니다.
예:
- const id = setInterval(() => setV((p) => (p >= 100 ? 0 : p + 10)), 250); + const id = setInterval(() => setV((p) => (p >= 100 ? 0 : p + 10)), 500);
126-137: play 함수의 쿼리 안정성 개선(역할 기반 쿼리 사용)내부 구현 의존적인 data-attribute 대신 ARIA 역할 기반 쿼리가 더 견고합니다.
적용 예:
- const roots = canvasElement.querySelectorAll('[data-slot="progress"]'); - roots.forEach((el) => expect(el.getAttribute('aria-valuenow')).toBe('40')); + const roots = await canvas.findAllByRole('progressbar'); + roots.forEach((el) => expect(el.getAttribute('aria-valuenow')).toBe('40'));현재 구현도 동작하지만, 역할 기반이 변경에 강합니다.
📜 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 (4)
packages/design-system/src/components/index.ts(1 hunks)packages/design-system/src/components/progress/Progress.stories.tsx(1 hunks)packages/design-system/src/components/progress/Progress.tsx(1 hunks)packages/design-system/src/components/switch/Switch.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/design-system/src/components/index.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/design-system/src/components/progress/Progress.tsx (1)
packages/design-system/src/lib/utils.ts (1)
cn(108-110)
packages/design-system/src/components/progress/Progress.stories.tsx (1)
packages/design-system/src/components/progress/Progress.tsx (1)
Progress(38-61)
🔇 Additional comments (7)
packages/design-system/src/components/switch/Switch.tsx (2)
14-14: 높이 2rem로 통일한 변경 좋아요컨테이너 h-[2rem]과 Thumb size-[1.6rem] 조합이 자연스러운 여백(각 0.2rem)을 만들어 이전 크기 불일치(overflow 가능성) 우려를 해소합니다. 접근성 측면에서도 터치 타깃이 커져 이점 있습니다.
14-14: 스위치 높이 변경에 따른 사용처 검증 필요전체 코드베이스에서 자동 스크립트로는 Switch 사용처를 확인할 수 없으므로, 아래 항목을 수동으로 점검해 주세요.
- 디자인 시스템 외 앱/패키지 레벨에서
Switch컴포넌트 import 및 사용 여부className으로h-/w-/size-유틸을 오버라이드한 케이스 유무- 텍스트 라인높이(line-height)나 아이콘과 병렬 배치된 곳의 vertical align(align-middle, leading) 및 간격(ml, mr, gap) 조정 필요성
- 고정된
translate-x값(Thumb)이 오프셋을 유발하는지 여부위 점검 후 레이아웃/정렬 문제 발생 시 추가 조율이 필요합니다.
packages/design-system/src/components/progress/Progress.tsx (3)
44-45: 입력값 클램핑으로 안전성 확보 — LGTM0–100 범위로 고정해 예기치 않은 입력에도 안전합니다. 그대로 가시죠.
6-14: CVA 설계 깔끔 — 확장 포인트 명확track의 높이/배경, indicator의 색·그라데이션을 variant로만 분기하는 구조가 명확합니다. 추후 디자인 추가도 용이해 보입니다.
38-61: forwardRef 적용으로 사용성 개선 및 일관성 확보ref 전달을 지원하면 포커스/측정/스크롤 연동 등 활용 폭이 넓어집니다.
적용 diff:
-export function Progress({ - className, - variant, - value, - ...props -}: ProgressProps) { - const progressPercent = Math.max(0, Math.min(100, value)); - - return ( - <ProgressPrimitive.Root +export const Progress = React.forwardRef<ProgressElement, ProgressProps>( + function Progress({ className, variant, value, ...props }, ref) { + const progressPercent = Math.max(0, Math.min(100, value)); + + return ( + <ProgressPrimitive.Root + ref={ref} data-slot="progress" - className={cn(track({ variant }), className)} + className={cn(className, track({ variant }))} value={progressPercent} max={100} {...props} > <ProgressPrimitive.Indicator data-slot="progress-indicator" className={indicator({ variant })} style={{ width: `${progressPercent}%` }} /> </ProgressPrimitive.Root> - ); -} + ); + } +);⛔ Skipped due to learnings
Learnt from: jjangminii PR: Pinback-Team/pinback-client#22 File: packages/design-system/src/components/input/Input.tsx:5-9 Timestamp: 2025-08-23T18:37:59.985Z Learning: React 19에서는 함수 컴포넌트가 ref를 일반 prop으로 받을 수 있다. interface에서 ref: Ref<HTMLInputElement>로 정의하고 함수 컴포넌트에서 ({ ref, ...props }) => 형태로 받을 수 있어 forwardRef가 불필요하다. React 19.1.1+ 프로젝트에서는 이 방식을 사용해야 한다.Learnt from: jjangminii PR: Pinback-Team/pinback-client#22 File: packages/design-system/src/components/input/Input.tsx:5-9 Timestamp: 2025-08-23T18:37:59.985Z Learning: React 19부터 함수 컴포넌트에서 ref를 일반 prop으로 받을 수 있어 forwardRef가 더 이상 필요하지 않다. InputProps interface에서 ref: Ref<HTMLInputElement>로 정의하고 함수 컴포넌트에서 ({ ref, ...props }) => 형태로 받을 수 있다.packages/design-system/src/components/progress/Progress.stories.tsx (2)
6-39: 문서/컨트롤/기본 args 구성 좋습니다 — LGTMvalue 범위를 0–100으로 한정하고 variant를 라디오로 노출한 점이 사용 경험에 적합합니다.
1-1: Storybook Vite 환경에서는@storybook/react-vite에서 타입을 가져오는 것이 맞습니다현재 프로젝트는 Vite 기반으로 Storybook을 구성하고 있으며,
@storybook/react-vite패키지에서Meta와StoryObj타입을 제공하고 있습니다. 다른 Stories 파일에서도 동일한 import 경로를 사용하고 있어 일관성을 유지하는 것이 바람직합니다.
(참고: Storybook 패키지별 릴리스 주기가 다르며,react-vite패키지는 9.x 라인에서 안정화되어 있습니다.)Likely an incorrect or invalid review comment.
| export interface ProgressProps | ||
| extends Omit< | ||
| React.ComponentProps<typeof ProgressPrimitive.Root>, | ||
| 'value' | 'max' | ||
| >, | ||
| VariantProps<typeof track> { | ||
| value: number; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ref 전달 누락: Radix Root의 ref를 노출하세요
디자인 시스템 컴포넌트는 보통 루트 요소 ref 전달을 지원합니다. forwardRef로 노출하고 타입도 With/WithoutRef로 정리하면 좋습니다. 또한 구조 고정 의도가 있다면 children을 받아들이지 않도록 차단해두는 편이 안전합니다.
권장 diff(타입 정리):
+type ProgressElement = React.ElementRef<typeof ProgressPrimitive.Root>;
+type ProgressRootProps = React.ComponentPropsWithoutRef<
+ typeof ProgressPrimitive.Root
+>;
+
-export interface ProgressProps
- extends Omit<
- React.ComponentProps<typeof ProgressPrimitive.Root>,
- 'value' | 'max'
- >,
- VariantProps<typeof track> {
+export interface ProgressProps
+ extends Omit<ProgressRootProps, 'value' | 'max' | 'children'>,
+ VariantProps<typeof track> {
value: number;
}이 변경에 맞춰 컴포넌트 본문도 forwardRef로 교체해야 합니다(아래 코멘트 참조).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/design-system/src/components/progress/Progress.tsx around lines
29-36, the ProgressProps currently omit the Radix Root ref and don't prevent
children; update the prop types to support forwarding the Root ref (e.g., use a
WithRef/WithoutRef pattern or include React.ComponentPropsWithRef<typeof
ProgressPrimitive.Root> and remove direct 'value'/'max' conflicts), mark
children as never to prevent passing children, and change the component
implementation to use React.forwardRef to pass the ref to
ProgressPrimitive.Root. Ensure the exported types and component signature align
(props type accepts the ref and the forwardRef generic matches the Root element
type) and update all usages to receive the forwarded ref.
jllee000
left a comment
jllee000
left a comment
There was a problem hiding this comment.
그라데이션 바!! 고생하셨습니다 :) 트리랑 프로필 상황에 맞게 바로 꺼내쓸 수 있어서 직관적이고 좋네요! 어차피 고정값일 max={100}을 props로 넣는 이유? 대한 개인 질문만 확인해주세요!
(shadcn에서 max prop 전달이 필수 사항이여서 그렇게 하신거 같기도하고...)
| import { cn } from '@/lib/utils'; | ||
|
|
||
| const track = cva('relative w-full overflow-hidden rounded-full', { | ||
| variants: { |
There was a problem hiding this comment.
w-를 full로 주면서, 추후 화면 영역에 맞게 반응형으로 맞출 수 있게 관리 가능할 것 같아서 좋네용
| import * as React from 'react'; | ||
| import * as ProgressPrimitive from '@radix-ui/react-progress'; |
There was a problem hiding this comment.
progressPrimitive은 shadCn에서 불러온 요소일까요??
| className={cn(track({ variant }), className)} | ||
| value={progressPercent} | ||
| max={100} |
There was a problem hiding this comment.
just 질문! 여기서 max가 필요한 이유가 있을까요? max의미가 100점만점 이런 의미인 것 같은데, 어차피 최대 100 기준이 고정인거면 그냥 value 값 그대로가 곧 막대기 width이랑 직결되어서 max값이 크게 상관 없어 보여서요!
만약에 max가 100고정이 아니라, 포인트 20개 만점~ 이런 식이면 progressPercent를 (포인트 개수 / max) * 100 이런 식으로 따로 계산해야하니까 유동적인 max가 의미 있지만, max가 100인 게 고정일거라면 이 max 속성의 의미가 있는지?에 대한 궁금증이 있습니다!
There was a problem hiding this comment.
사실 시각적 요소는 width: ${progressPercent}%로 결정되어 max값은 크게 상관 없어요-! progressbar는 value와 함께 max가 있으면 스크린리더가 “40%” 또는 “40/100”처럼 맥락 있는 값을 읽어 주어 접근성에 있어 더 좋다고 판단했어요
3 participants
📌 Related Issues
✅ 체크 리스트
📄 Tasks
⭐ PR Point (To Reviewer)
역할 분리 확정
사용처에서 클래스 반복을 피하고 일관성을 보장하기 위해 스타일 주입/합성 대신 variant="profile" | "tree" 두 가지만 공식 지원하도록 했습니다.
분기처리 선택이유
사용방법
📷 Screenshot
Summary by CodeRabbit