Conversation
WalkthroughRadix UI의 Switch 의존성을 제거하고, 네이티브 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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년 01월 06일 17시 08분 01초 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @src/shared/ui/Switch/Switch.tsx:
- Around line 39-57: The button using role="switch" in the Switch component is
missing the required aria-checked attribute; update the <button> in Switch.tsx
(the element that uses role="switch" and data-state based on isChecked) to
include aria-checked={isChecked} (or aria-checked={isChecked ? 'true' : 'false'}
if you prefer strings) so screen readers receive the current on/off state—keep
the rest of the props/handlers (disabled, onClick/handleClick, data-state)
unchanged.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
package.jsonsrc/shared/ui/Switch/Switch.tsxvite.config.ts
💤 Files with no reviewable changes (2)
- vite.config.ts
- package.json
🧰 Additional context used
🧬 Code graph analysis (1)
src/shared/ui/Switch/Switch.tsx (1)
src/shared/lib/colors.ts (2)
SwitchColor(48-48)COLORS(37-44)
🪛 Biome (2.1.2)
src/shared/ui/Switch/Switch.tsx
[error] 42-42: The element with the switch ARIA role does not have the required ARIA attributes.
Missing ARIA prop(s):
(lint/a11y/useAriaPropsForRole)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: preview-storybook
🔇 Additional comments (4)
src/shared/ui/Switch/Switch.tsx (4)
1-3: LGTM!필요한 의존성만 import하고 있으며, 모두 컴포넌트 내에서 올바르게 사용되고 있습니다.
5-14: LGTM!Props 타입 정의가 올바르며, controlled/uncontrolled 패턴을 위한
checked,defaultChecked,onCheckedChangeprops가 적절하게 정의되어 있습니다.onClick을 제외한 것도 내부에서 클릭 처리를 관리하므로 타당합니다.
25-26: LGTM!Controlled/uncontrolled 패턴이 React 모범 사례에 따라 올바르게 구현되어 있습니다.
checkedprop이 제공되면 controlled로, 없으면 내부 상태를 사용하는 uncontrolled로 동작합니다.
28-36: LGTM!클릭 핸들러가 올바르게 구현되어 있습니다. Disabled 상태를 먼저 확인하고, controlled/uncontrolled 상태를 적절히 구분하여 처리하며, 콜백을 항상 호출하는 점이 controlled 컴포넌트의 올바른 패턴입니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @src/shared/ui/Switch/Switch.tsx:
- Line 57: The switch knob span in the Switch component is missing a transition
class so the knob jumps instead of animating; update the span (the element with
className containing "duration-100" in Switch.tsx) to also include "transition"
or "transition-transform" (e.g., add transition-transform) so the duration takes
effect and the group-data-[state=checked]:translate-x-3.5 animates smoothly.
- Around line 47-55: The switch root rendering builds className and inline style
using the className prop, the 'group ...' base classes, and COLORS[color];
update this to include explicit disabled and focus-visible utility classes
(e.g., add disabled:opacity-50 disabled:cursor-not-allowed and
focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2) so
the component no longer relies solely on browser defaults; modify the className
array in Switch.tsx (where className and style are set) to append these
focus/disabled classes and ensure they coexist with the existing
data-[state=...] and COLORS[color] usage.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/shared/ui/Switch/Switch.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/shared/ui/Switch/Switch.tsx (1)
src/shared/lib/colors.ts (2)
SwitchColor(48-48)COLORS(37-44)
🔇 Additional comments (4)
src/shared/ui/Switch/Switch.tsx (4)
1-14: Props 타입 설계가 우수합니다!
Omit<React.ComponentProps<'button'>, 'onClick'>를 사용하여 외부에서onClick을 오버라이드하지 못하도록 막은 점이 좋습니다. 새로 추가된checked,defaultChecked,onCheckedChangeprops는 React의 표준 제어/비제어 컴포넌트 패턴을 따르고 있어 일관성이 있습니다.
25-26: 제어/비제어 상태 관리가 올바르게 구현되었습니다.
checked ?? uncontrolledChecked패턴을 사용하여 제어 모드와 비제어 모드를 모두 지원하는 것이 적절합니다.defaultChecked가 초기 상태로만 사용되는 것도 React의 표준 동작과 일치합니다.
28-36: 클릭 핸들러 로직이 정확합니다.비제어 모드일 때만 내부 상태를 업데이트하고 (
checked === undefined), 제어 모드에서는onCheckedChange콜백만 호출하는 구현이 올바릅니다.disabled체크로 비활성 상태를 적절히 처리하고 있습니다.
45-45: 이전 리뷰의 중요 이슈가 해결되었습니다.
aria-checked속성이 추가되어 이전 리뷰에서 지적된 접근성 문제가 수정되었습니다. 스크린 리더가 이제 스위치의 상태를 올바르게 전달할 수 있습니다.
| className={[ | ||
| 'group flex h-3.5 w-7 items-center rounded-full shadow-lg', | ||
| 'data-[state=checked]:bg-[var(--switch-color)]', | ||
| 'data-[state=unchecked]:bg-gray-500', | ||
| className, | ||
| ] | ||
| .filter(Boolean) | ||
| .join(' ')} | ||
| style={{ '--switch-color': COLORS[color] } as React.CSSProperties} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
선택적 개선: 명시적인 disabled 및 focus 스타일 고려.
현재는 네이티브 disabled 속성과 브라우저 기본 포커스 스타일에 의존하고 있습니다. 디자인 시스템의 일관성을 위해 명시적인 스타일을 추가하는 것을 고려해보세요:
disabled:opacity-50 disabled:cursor-not-allowed- 비활성 상태 시각적 피드백focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2- 접근성 향상을 위한 포커스 표시
🔎 선택적 스타일링 개선 제안
className={[
'group flex h-3.5 w-7 items-center rounded-full shadow-lg',
'data-[state=checked]:bg-[var(--switch-color)]',
'data-[state=unchecked]:bg-gray-500',
+ 'disabled:opacity-50 disabled:cursor-not-allowed',
+ 'focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary-300',
className,
]🤖 Prompt for AI Agents
In @src/shared/ui/Switch/Switch.tsx around lines 47 - 55, The switch root
rendering builds className and inline style using the className prop, the 'group
...' base classes, and COLORS[color]; update this to include explicit disabled
and focus-visible utility classes (e.g., add disabled:opacity-50
disabled:cursor-not-allowed and focus-visible:outline focus-visible:outline-2
focus-visible:outline-offset-2) so the component no longer relies solely on
browser defaults; modify the className array in Switch.tsx (where className and
style are set) to append these focus/disabled classes and ensure they coexist
with the existing data-[state=...] and COLORS[color] usage.
| > | ||
| <SwitchPrimitives.Thumb className="h-5 w-5 -translate-x-1.5 rounded-full bg-white shadow-inner drop-shadow-md duration-100 group-data-[state=checked]:translate-x-3.5" /> | ||
| </SwitchPrimitives.Root> | ||
| <span className="h-5 w-5 -translate-x-1.5 rounded-full bg-white shadow-inner drop-shadow-md duration-100 group-data-[state=checked]:translate-x-3.5" /> |
There was a problem hiding this comment.
transition 클래스 누락으로 애니메이션이 동작하지 않습니다.
duration-100 클래스가 있지만 transition 또는 transition-transform 클래스가 없어서 스위치 노브가 부드럽게 슬라이드되지 않고 즉시 점프합니다. Tailwind에서는 duration-* 클래스만으로는 트랜지션이 활성화되지 않습니다.
🔎 트랜지션 수정 제안
- <span className="h-5 w-5 -translate-x-1.5 rounded-full bg-white shadow-inner drop-shadow-md duration-100 group-data-[state=checked]:translate-x-3.5" />
+ <span className="h-5 w-5 -translate-x-1.5 rounded-full bg-white shadow-inner drop-shadow-md transition-transform duration-100 group-data-[state=checked]:translate-x-3.5" />📝 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.
| <span className="h-5 w-5 -translate-x-1.5 rounded-full bg-white shadow-inner drop-shadow-md duration-100 group-data-[state=checked]:translate-x-3.5" /> | |
| <span className="h-5 w-5 -translate-x-1.5 rounded-full bg-white shadow-inner drop-shadow-md transition-transform duration-100 group-data-[state=checked]:translate-x-3.5" /> |
🤖 Prompt for AI Agents
In @src/shared/ui/Switch/Switch.tsx at line 57, The switch knob span in the
Switch component is missing a transition class so the knob jumps instead of
animating; update the span (the element with className containing "duration-100"
in Switch.tsx) to also include "transition" or "transition-transform" (e.g., add
transition-transform) so the duration takes effect and the
group-data-[state=checked]:translate-x-3.5 animates smoothly.
🔥 연관 이슈
🚀 작업 내용
fe 레포지토리 테스트 동작시 발생한 에러를 해결하기 위해
switch 컴포넌트에서 radix의존성을 제거했습니다
🤔 고민했던 내용
💬 리뷰 중점사항
Summary by CodeRabbit
새로운 기능
Chores
✏️ Tip: You can customize this high-level summary in your review settings.