Conversation
Walkthroughapps/web에 Radix UI 라디오 그룹 의존성을 추가하고, Radio 컴포넌트(타입 및 구현)와 해당 Storybook 스토리를 신규로 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Radio as Radio (Radix RadioGroup)
participant App as Consumer
User->>Radio: 옵션 클릭
Radio->>Radio: 내부 value 업데이트
Radio-->>App: onChange(value)
alt disabled = true
User--xRadio: 입력 차단
end
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
🧪 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. Comment |
|
📚 Storybook is ready for review! |
![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 (5)
apps/web/src/shared/components/Radio/Radio.tsx (3)
2-2: 취약한 상대 경로 대신 alias import 권장프로젝트 루트 기준의 alias(
styled-system/css) 또는 TS path alias 사용을 권장합니다. 리팩터 시:-import { css } from "../../../../styled-system/css"; +import { css } from "styled-system/css";
5-6: id 충돌 방지: useId로 고유 ID 사용 권장옵션 값이 동일하거나 다국어/공백 포함 시 ID 충돌 가능성이 있습니다.
+import { useId } from "react"; @@ -export function Radio({ options, defaultValue, onChange, disabled = false }: RadioProps) { - return ( +export function Radio({ options, defaultValue, onChange, disabled = false }: RadioProps) { + const baseId = useId(); + return ( @@ - id={`radio-${option.value}`} + id={`${baseId}-${option.value}`} @@ - htmlFor={`radio-${option.value}`} + htmlFor={`${baseId}-${option.value}`}Also applies to: 42-43, 69-69
58-58: 중복 인디케이터 제거 또는 스타일 지정커스텀 점(div)으로 체크 상태를 표현하고 있어
<RadioGroupBase.Indicator />는 불필요합니다. 제거하거나 스타일을 부여하세요.- <RadioGroupBase.Indicator />apps/web/src/shared/components/Radio/Radio.stories.tsx (2)
1-1: Storybook 타입 임포트 경로 확인 필요일반적으로 타입은
@storybook/react에서 임포트합니다. 현재 프레임워크 패키지에서 타입이 재노출되는지 확인하거나 아래로 변경을 고려해 주세요.-import type { Meta, StoryObj } from "@storybook/nextjs-vite"; +import type { Meta, StoryObj } from "@storybook/react";
12-33: 컨트롤 UX 개선 제안
- options는 객체 배열이므로 select 대신 object 컨트롤이 적합합니다.
- 변경 이벤트 가시화를 위해 onChange 액션 추가를 권장합니다.
argTypes: { options: { - control: "select", - options: [ - [ - { label: "프로젝트1", value: "프로젝트1" }, - { label: "프로젝트2", value: "프로젝트2" }, - { label: "프로젝트3", value: "프로젝트3" }, - ], - ], + control: "object", description: "radio의 옵션을 지정", }, + onChange: { + action: "change", + description: "선택 값 변경 핸들러", + },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/web/package.json(1 hunks)apps/web/src/shared/components/Radio/Radio.stories.tsx(1 hunks)apps/web/src/shared/components/Radio/Radio.tsx(1 hunks)apps/web/src/shared/components/Radio/Radio.type.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/shared/components/Radio/Radio.stories.tsx (2)
apps/web/src/shared/components/Radio/Radio.tsx (1)
Radio(5-77)apps/web/src/shared/components/Radio/Radio.type.ts (1)
RadioProps(1-9)
apps/web/src/shared/components/Radio/Radio.tsx (1)
apps/web/src/shared/components/Radio/Radio.type.ts (1)
RadioProps(1-9)
🔇 Additional comments (1)
apps/web/package.json (1)
18-18: Radix RadioGroup 의존성 추가 LGTMReact 19/Next 15 조합과도 호환 문제 없어 보입니다. 워크스페이스 전반에서 잠재 중복 설치/버전 상이만 한 번 확인해 주세요.
|
📚 Storybook is ready for review! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/web/src/shared/components/Radio/Radio.tsx (3)
7-16: 키보드 탐색 일관성: orientation="vertical" 지정 권장세로 스택 레이아웃에 맞춰 Radix의 roving focus 동작을 명확히 하려면 orientation을 명시하는 것이 안전합니다.
<RadioGroupBase.Root defaultValue={defaultValue} onValueChange={onChange} disabled={disabled} + orientation="vertical" className={css({ display: "flex", alignItems: "center", flexDirection: "column", gap: "8px", })} >추가로, 폼 통합이 필요하다면 Root에 name 전달을 지원하도록 RadioProps에 name?: string 추가도 고려해주세요.
44-59: 중복된 표시 요소 정리: 커스텀 점(div) 대신 Indicator만 스타일링현재 커스텀 점(div)과 Radix Indicator가 함께 존재합니다. Indicator만 스타일링하면 DOM 중복을 줄이고 상태 연동(checked 시 표시)이 자연스럽습니다.
- > - <div - className={css({ - width: "10px", - height: "10px", - borderRadius: "50%", - position: "absolute", - top: "50%", - left: "50%", - transform: "translate(-50%, -50%)", - backgroundColor: "transparent", - "[data-state=checked] &": { backgroundColor: "#3A8DFF" }, - "[data-disabled] &": { backgroundColor: "#B7C2D0" }, - })} - /> - <RadioGroupBase.Indicator /> + > + <RadioGroupBase.Indicator + className={css({ + width: "10px", + height: "10px", + borderRadius: "50%", + position: "absolute", + top: "50%", + left: "50%", + transform: "translate(-50%, -50%)", + backgroundColor: "#3A8DFF", + "[data-disabled] &": { backgroundColor: "#B7C2D0" }, + })} + />
61-69: label의 aria-disabled 제거 및 그룹 상태 기반 스타일로 대체label에 aria-disabled를 부여해도 의미론적 비활성화가 되지 않습니다. 그룹/아이템의 disabled 상태에 따라 스타일만 반영하면 충분합니다.
<label className={css({ display: "flex", alignItems: "center", cursor: "pointer", userSelect: "none", - '&[aria-disabled="true"]': { opacity: 0.5, cursor: "not-allowed" }, + "[data-disabled] &": { opacity: 0.5, cursor: "not-allowed" }, })} - aria-disabled={disabled} htmlFor={`radio-${option.value}`} >
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/web/package.json(1 hunks)apps/web/src/shared/components/Radio/Radio.stories.tsx(1 hunks)apps/web/src/shared/components/Radio/Radio.tsx(1 hunks)apps/web/src/shared/components/Radio/Radio.type.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/package.json
- apps/web/src/shared/components/Radio/Radio.stories.tsx
- apps/web/src/shared/components/Radio/Radio.type.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/shared/components/Radio/Radio.tsx (1)
apps/web/src/shared/components/Radio/Radio.type.ts (1)
RadioProps(3-11)
🔇 Additional comments (2)
apps/web/src/shared/components/Radio/Radio.tsx (2)
27-40: 포커스 가시성 개선(접근성): _focusVisible 아웃라인 추가키보드 사용자에게 포커스 위치가 명확히 보여야 합니다. Item에 포커스 링을 추가하는 것을 권장합니다.
<RadioGroupBase.Item className={css({ margin: "11px", width: "20px", height: "20px", borderRadius: "50%", border: "1px solid #B7C2D0", cursor: "pointer", position: "relative", flexShrink: 0, _hover: { borderColor: "#3A8DFF" }, '&[data-state="checked"]': { borderColor: "#3A8DFF" }, "&[data-disabled]": { borderColor: "#B7C2D0", cursor: "not-allowed" }, + _focusVisible: { outline: "2px solid #3A8DFF", outlineOffset: "2px" }, })} value={option.value} id={`radio-${option.value}`} >또한 컬러/스페이싱에 테마 토큰(예: 색상 토큰, space/size 토큰) 사용을 검토하면 디자인 일관성과 다크모드 대응성이 좋아집니다.
3-3: 부정확한 지적 — Radio.type.ts에 이미 ReactNode가 import 되어 있습니다.
apps/web/src/shared/components/Radio/Radio.type.ts:1 — import type { ReactNode } from "react";Likely an incorrect or invalid review comment.
| value={option.value} | ||
| id={`radio-${option.value}`} | ||
| > |
There was a problem hiding this comment.
중복 id 위험: useId로 id 안정화
여러 Radio 컴포넌트가 동일 value를 가질 경우 id={radio-${value}}가 페이지 전역에서 중복될 수 있어 라벨 연결이 깨질 수 있습니다. React useId로 고유 prefix를 부여하세요.
- id={`radio-${option.value}`}
+ id={`${baseId}-${option.value}`}- htmlFor={`radio-${option.value}`}
+ htmlFor={`${baseId}-${option.value}`}컴포넌트 내부에 baseId 선언 및 import 추가(파일 외 변경):
import { useId } from "react";
export function Radio(props: RadioProps) {
const baseId = useId();
// ...
}Also applies to: 69-69
🤖 Prompt for AI Agents
In apps/web/src/shared/components/Radio/Radio.tsx around lines 41-43 (and also
at line 69), the component currently builds IDs as `radio-${option.value}` which
can collide across multiple Radio instances; import React's useId, create a
baseId inside the component (e.g. const baseId = useId()), and prefix the
generated id with it (for example `${baseId}-radio-${option.value}`) so each
Radio instance has a unique id; update both the input id and the corresponding
label htmlFor to use the new prefixed id and add the useId import at the top of
the file.
2 participants
📝 PR 유형
📝 PR 설명
Radio 컴포넌트 마이그레이션 작업입니다.
관련된 이슈 넘버
close #58
✅ 작업 목록
MR하기 전에 확인해주세요
📚 논의사항
📚 ETC
Summary by CodeRabbit