Conversation
Walkthrough새로운 ProgressBar React 컴포넌트와 Storybook 스토리 추가, 색상 팔레트에 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
Update: 2026년 02월 05일 18시 20분 52초 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/shared/lib/colors.ts`:
- Line 51: The declared type ProgressBarColor = keyof typeof colors is
incorrect/mismatched and unused; update types to match the component or remove
it: either delete ProgressBarColor if unused, or change its definition to
reflect the flattened palette used by the ProgressBar component (e.g., use keyof
typeof COLORS) so it matches the keys the component expects, and update any
references in ProgressBar to use the corrected type; ensure symbols referenced
are ProgressBarColor, colors, COLORS, and the ProgressBar component.
In `@src/shared/ui/ProgressBar/ProgressBar.tsx`:
- Around line 16-17: The ProgressBar component currently wraps its JSX return in
an unnecessary Flex component; remove the Flex wrapper and return the inner
elements directly (or use a fragment) to simplify the DOM and avoid unused flex
styles—update the JSX in ProgressBar (remove the surrounding <Flex>...</Flex>
that encloses the component output) and do the same for the other identical
occurrence in this file so no redundant Flex remains.
- Line 24: ProgressBar currently hardcodes the track backgroundColor as
'#F3F4F6'; update the ProgressBar component to import the design system color
(e.g., colors.gray[100]) and use that value instead of the literal string for
the backgroundColor style, and add the import for colors from your design system
module (ensure the identifier matches your existing exports) so the style
references colors.gray[100] in the ProgressBar implementation.
| width: '264px', | ||
| height: `${height}px`, | ||
| borderRadius: `${height / 2}px`, | ||
| backgroundColor: '#F3F4F6', |
There was a problem hiding this comment.
width, height, borderRadius는 className으로 옮기고
런타임 값(percent, color)만 inline style로 분리하면 더 읽기 쉬울 것 같아요!
src/shared/lib/colors.ts
Outdated
| export type Colors = keyof typeof colors; | ||
| export type IconColor = keyof typeof COLORS; | ||
| export type SwitchColor = keyof typeof COLORS; | ||
| export type ProgressBarColor = keyof typeof colors; |
There was a problem hiding this comment.
코드래빗 리뷰 참고해주시면 좋을 것 같습니다.
| }; | ||
|
|
||
| export function ProgressBar({ color = 'primary', percent, className }: Props) { | ||
| const height = 10; |
There was a problem hiding this comment.
해당 상수는 고정된 값이라 컴포넌트 외부에 위치시켜주시면 좋을 것 같아요. 또. 상수임을 나타내기 위해 대문자로 작성해주시면 판별하는데 도움이 될 것 같네요~
There was a problem hiding this comment.
감사합니다! 현재는 tailwindCSS를 적용시켰습니다! a5c8483
| style={{ | ||
| width: '264px', | ||
| height: `${height}px`, | ||
| borderRadius: `${height / 2}px`, | ||
| backgroundColor: '#F3F4F6', |
There was a problem hiding this comment.
요 영역들을 tailwind를 사용하지 않고, 따로 style 내장 css를 사용하신 이유가 있으신가요?
There was a problem hiding this comment.
초기에 너비와 높이를 동적으로 변경할 수 있게하기 위해서 내장 css를 사용했었습니다. 모바일에서만 사용된다는 얘기를 전달 받은 이후 93d455a 수정 했습니다.
| }} | ||
| > | ||
| <div | ||
| className="h-full transition-[width] duration-300 ease-in-out" |
There was a problem hiding this comment.
transition-[width] 요 값이 실제로 적용이 되나요?
There was a problem hiding this comment.
네 실제 적용이 됩니다! transition 먼저 적용해서 작업을 한 다음duration 속성을 추가했는데 혹시 중복되거나 문제가 되는 부분이 있을까요?
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/shared/ui/ProgressBar/ProgressBar.tsx`:
- Around line 5-35: The ProgressBar component is missing ARIA attributes; update
the rendered container (the outer or the inner bar in ProgressBar) to include
role="progressbar" and set aria-valuenow to the clampedPercent (or a derived
integer), and aria-valuemin="0" and aria-valuemax="100"; ensure you use the
clampedPercent variable (from ProgressBar) for the value attributes and preserve
existing className/cn, COLORS and Flex usage so screen readers can announce the
progress correctly.
- Line 31: The ProgressBar uses an unsupported Tailwind class "duration-600" in
the className on the ProgressBar component; replace "duration-600" with a
supported duration utility (for example "duration-700" or "duration-500") in the
className string inside ProgressBar.tsx, or alternatively add a custom
transitionDuration value for 600 in your tailwind.config.ts and rebuild so
"duration-600" becomes valid.
| type Props = { | ||
| /** | ||
| * defined in percentage (0-100). | ||
| */ | ||
| percent: number; | ||
| /** | ||
| * color of the switch. | ||
| * @default 'primary' | ||
| */ | ||
| color?: keyof typeof COLORS; | ||
| className?: string; | ||
| }; | ||
|
|
||
| export function ProgressBar({ color = 'primary', percent, className }: Props) { | ||
| const clampedPercent = Math.min(100, Math.max(0, percent)); | ||
| const barColor = COLORS[color]; | ||
|
|
||
| return ( | ||
| <Flex> | ||
| <div | ||
| className={cn('h-[0.625rem] w-[16.5rem] overflow-hidden rounded-full', className)} | ||
| style={{ | ||
| backgroundColor: colors.gray[100], | ||
| }} | ||
| > | ||
| <div | ||
| className="h-full rounded-full transition-[width] duration-600 ease-in-out" | ||
| style={{ | ||
| width: `${clampedPercent}%`, | ||
| backgroundColor: barColor, | ||
| }} |
There was a problem hiding this comment.
접근성 속성(ARIA) 추가가 필요합니다.
진행 상태를 전달하는 컴포넌트인데 role="progressbar" 및 aria-valuenow/min/max가 없어 스크린리더가 의미를 인지하기 어렵습니다.
♿ 제안 수정안
type Props = {
/**
* defined in percentage (0-100).
*/
percent: number;
+ /**
+ * 접근성 라벨
+ */
+ ariaLabel?: string;
/**
* color of the switch.
* `@default` 'primary'
*/
color?: keyof typeof COLORS;
className?: string;
};
-export function ProgressBar({ color = 'primary', percent, className }: Props) {
+export function ProgressBar({
+ color = 'primary',
+ percent,
+ className,
+ ariaLabel = 'progress',
+}: Props) {
const clampedPercent = Math.min(100, Math.max(0, percent));
const barColor = COLORS[color];
return (
<Flex>
<div
className={cn('h-[0.625rem] w-[16.5rem] overflow-hidden rounded-full', className)}
+ role="progressbar"
+ aria-valuemin={0}
+ aria-valuemax={100}
+ aria-valuenow={clampedPercent}
+ aria-label={ariaLabel}
style={{
backgroundColor: colors.gray[100],
}}
>🤖 Prompt for AI Agents
In `@src/shared/ui/ProgressBar/ProgressBar.tsx` around lines 5 - 35, The
ProgressBar component is missing ARIA attributes; update the rendered container
(the outer or the inner bar in ProgressBar) to include role="progressbar" and
set aria-valuenow to the clampedPercent (or a derived integer), and
aria-valuemin="0" and aria-valuemax="100"; ensure you use the clampedPercent
variable (from ProgressBar) for the value attributes and preserve existing
className/cn, COLORS and Flex usage so screen readers can announce the progress
correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/shared/ui/ProgressBar/ProgressBar.tsx`:
- Around line 9-12: The JSDoc for the ProgressBar component mistakenly says
"color of the switch"; update the comment associated with the ProgressBar (look
for the JSDoc above the ProgressBar prop or component definition in
ProgressBar.tsx) to correctly read "color of the progress bar" (and keep the
existing `@default` 'primary' documentation intact) so the description reflects
this component.
배경 색상이 고정된 값이라 className 을 통해 값을 고정시켰습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/shared/ui/ProgressBar/ProgressBar.tsx`:
- Line 1: Remove the unused named import "colors" from the ProgressBar module's
import statement; keep the needed "COLORS" symbol if still used by the component
(i.e., change the import from "import { COLORS, colors } ..." to only import
COLORS) and ensure no other references to the "colors" identifier remain in the
component (search for "colors" and delete or replace any leftover usage).
🔥 연관 이슈
🚀 작업 내용
2026-02-01.11.54.13.mov
🤔 고민했던 내용
퍼센트의 상승 단위를 25 단위로 할지 1 단위로 할지 고민하다가 추후 게임의 스테이지가 늘어나거나 줄어들 가능성이 있어 1단위로 했습니다.
프롭스로 전달하되 기본값을 설정하는게 좋을지 사이즈를 고민시킬지 고민하다가 이번 행사 외 다시 쓰일 가능성의 여부를 잘 모르겠어서 크기를 고정시켰습니다.
💬 리뷰 중점사항
Summary by CodeRabbit