Conversation
WalkthroughRadix UI의 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 사용자
participant T as Dropdown (Trigger)
participant R as Radix DropdownMenu
participant C as 컴포넌트 콜백 (onOpenChange/onClick)
U->>T: 클릭/토글
T->>R: open 요청
R-->>T: open 상태 변경 이벤트
T-->>C: onOpenChange(open)?
U->>R: 항목 선택
R->>T: 선택 이벤트(value)
T->>T: 선택값 업데이트
T-->>C: onClick({label, value})?
note over R,T: ESC 또는 외부 클릭 시 닫힘 처리
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 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! |
|
📚 Storybook is ready for review! |
|
📚 Storybook is ready for review! |
|
📚 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 (6)
apps/web/src/shared/components/Dropdown/Dropdown.type.ts (2)
7-7: 정의된 className prop가 컴포넌트에서 사용되지 않음Dropdown.tsx에서 className을 받아 적용하지 않습니다. API 표면에 존재하는 prop는 실제로 사용되도록 배선해 주세요.
9-9: 이벤트 명칭 정합성: onClick → onSelect 권장Radix DropdownMenu.Item은 선택 이벤트로 onSelect를 권장합니다. 외부 콜백 명도 onSelect(또는 onValueChange)로 맞추면 의미가 명확해집니다.
적용 시 Dropdown.tsx의 내부 핸들러/바인딩도 함께 업데이트해야 합니다.
apps/web/src/shared/components/Dropdown/Dropdown.tsx (2)
8-10: className prop 수용 및 루트 트리거에 전달DropdownProps에 className이 정의되어 있으나 미사용입니다. 트리거 버튼에 합쳐 적용해 커스터마이징 포인트를 노출하세요.
다음과 같이 전달을 제안합니다:
-export const Dropdown = ({ label, items, onOpenChange, onClick }: DropdownProps) => { +export const Dropdown = ({ label, items, className, onOpenChange, onClick }: DropdownProps) => { const [isOpen, setIsOpen] = useState(false);그리고 트리거 버튼 쪽:
- <button + <button type="button" - className={css({ ... })} + className={[css({ /* 기존 스타일 */ }), className].filter(Boolean).join(" ")}
86-116: 렌더 루프 내부 css(...) 호출 최소화itemBaseClass는 정적 스타일입니다. 루프 외부로 hoist하면 재연산을 줄일 수 있습니다. 선택 상태용 itemSelectedClass만 조건부로 합치면 됩니다.
apps/web/src/shared/components/Dropdown/Dropdown.stories.tsx (2)
21-49: items 컨트롤 타입 부적합: select 대신 object 권장배열+객체 구조는 select보다 object 컨트롤이 적합합니다. 현재 options에 단일 옵션 배열만 제공되어 컨트롤 이점이 거의 없습니다.
다음과 같이 조정해 보세요:
- items: { - control: "select", - options: [ [ /* 샘플 아이템들 */ ] ], - description: "dropdown의 아이템을 지정", - }, + items: { + control: "object", + description: "dropdown의 아이템을 지정", + },또한 onOpenChange, onClick(또는 onSelect)에는 action을 연결하면 데모 시 유용합니다:
+ onOpenChange: { action: "onOpenChange" }, + onClick: { action: "onClick" }, // onSelect로 이름 변경 시 같이 수정
81-82: render 타이핑 단순화 가능StoryObj 타입 추론이 되므로 render의 매개변수에 별도 DropdownProps 주석은 생략해도 됩니다. 유지해도 무방합니다.
- render: (args: DropdownProps) => <Dropdown {...args} />, + render: (args) => <Dropdown {...args} />,
📜 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/Dropdown/Dropdown.stories.tsx(1 hunks)apps/web/src/shared/components/Dropdown/Dropdown.tsx(1 hunks)apps/web/src/shared/components/Dropdown/Dropdown.type.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/shared/components/Dropdown/Dropdown.stories.tsx (2)
apps/web/src/shared/components/Dropdown/Dropdown.tsx (1)
Dropdown(8-137)apps/web/src/shared/components/Dropdown/Dropdown.type.ts (1)
DropdownProps(1-10)
apps/web/src/shared/components/Dropdown/Dropdown.tsx (1)
apps/web/src/shared/components/Dropdown/Dropdown.type.ts (1)
DropdownProps(1-10)
🔇 Additional comments (1)
apps/web/package.json (1)
21-21: 확인: @radix-ui/react-dropdown-menu@2.1.16 및 다른 Radix 패키지의 peerDependencies가 React 19를 허용합니다
npm view 결과 react/react-dom에 ^19 포함 — React 19/Next 15과 호환되므로 별도 수정 불필요.
| <DropdownMenu.Trigger asChild className={css({})}> | ||
| <div |
There was a problem hiding this comment.
Trigger 자식 요소는 button 사용 권장 (접근성/키보드 상호작용 개선)
asChild를 쓰는 경우 Trigger의 자식은 button 요소 사용이 권장됩니다. 현재 div를 사용하고 있어 접근성(역할/키보드) 면에서 불리할 수 있습니다.
다음과 같이 수정을 제안합니다:
- <DropdownMenu.Trigger asChild className={css({})}>
- <div
+ <DropdownMenu.Trigger asChild>
+ <button
+ type="button"
className={css({
border: "1px solid rgba(0, 0, 0, 0.2)",
padding: "11px 20px",
minWidth: "170px",
display: "flex",
flexDirection: "row",
alignItems: "center",
justifyContent: "space-between",
gap: "10px",
})}
style={{ borderRadius: isOpen ? "4px 4px 0 0" : "4px" }}
>
...
- </div>
+ </button>
</DropdownMenu.Trigger>📝 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.
| <DropdownMenu.Trigger asChild className={css({})}> | |
| <div | |
| <DropdownMenu.Trigger asChild> | |
| <button | |
| type="button" | |
| className={css({ | |
| border: "1px solid rgba(0, 0, 0, 0.2)", | |
| padding: "11px 20px", | |
| minWidth: "170px", | |
| display: "flex", | |
| flexDirection: "row", | |
| alignItems: "center", | |
| justifyContent: "space-between", | |
| gap: "10px", | |
| })} | |
| style={{ borderRadius: isOpen ? "4px 4px 0 0" : "4px" }} | |
| > | |
| ... | |
| </button> | |
| </DropdownMenu.Trigger> |
🤖 Prompt for AI Agents
In apps/web/src/shared/components/Dropdown/Dropdown.tsx around lines 26-27, the
DropdownMenu.Trigger is using asChild with a div child which harms accessibility
and keyboard interaction; replace the div with a semantic button element
(type="button") that forwards props and retains existing styling/classes, ensure
any aria attributes/handlers are passed through, and keep visual/layout styles
identical so keyboard focus and assistive tech work correctly.
2 participants
📝 PR 유형
📝 PR 설명
Dropdown 컴포넌트 마이그레이션 작업입니다.
관련된 이슈 넘버
close #57
✅ 작업 목록
MR하기 전에 확인해주세요
📚 논의사항
📚 ETC
Summary by CodeRabbit