Conversation
*close, triangle-alert icon 추가
*WithdrawDialog에서 사용
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthrough새로운 Radix 기반 Dialog 컴포넌트를 추가하고 NavigationBar의 일부 버튼을 Dialog 트리거로 교체했습니다. Logo에 사이즈 옵션과 스타일 매핑(iconSizeMap)을 도입했고, 아이콘 맵에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant NavBar as NavigationBar
participant DialogComp as Dialog
participant Portal as Portal
participant Action as ActionButton
User->>NavBar: 트리거 버튼 클릭
NavBar->>DialogComp: trigger 전달 / 렌더링
User->>DialogComp: 트리거 작동 (open)
DialogComp->>Portal: Portal 생성
Portal->>DialogComp: Overlay + Content 렌더링
DialogComp->>DialogComp: 아이콘/제목/설명/액션 렌더
alt LoginDialog
DialogComp->>Action: 카카오 로그인 버튼 표시
User->>Action: 로그인 클릭
Action-->>User: 로그인 처리
else WithdrawDialog
DialogComp->>Action: 취소/탈퇴 버튼 표시
User->>Action: 선택 (취소/탈퇴)
Action-->>User: 선택 처리
end
User->>DialogComp: Close 클릭
DialogComp-->>NavBar: 모달 종료
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
![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)
src/components/NavigationBar/NavigationBar.tsx (1)
7-13: 컴포넌트를 NavigationBar 외부로 이동 고려
AddPromptButton과LoginButton이 NavigationBar 내부에 정의되어 있어 매 렌더링마다 새로 생성됩니다. 성능 최적화를 위해 컴포넌트 외부로 이동하는 것을 권장합니다.🔎 수정 제안
+const AddPromptButton = () => ( + <Button icon="addLine" iconSize="md"> + 프롬프트 등록 + </Button> +); + +const LoginButton = () => <Button variant="solid">로그인</Button>; + const NavigationBar = () => { - const AddPromptButton = () => ( - <Button icon="addLine" iconSize="md"> - 프롬프트 등록 - </Button> - ); - - const LoginButton = () => <Button variant="solid">로그인</Button>; - return ( // ... ); };src/components/Logo/Icon.styles.ts (1)
1-4: LGTM!
iconSizeMap이 깔끔하게 정의되었습니다. 타입 안전성을 위해as const를 추가하면 더 좋습니다.🔎 타입 안전성 향상을 위한 제안
export const iconSizeMap = { md: 'w-8 h-8 rounded-lg', xl: 'w-16 h-16 rounded-2xl', -}; +} as const;src/components/Dialog/Dialog.tsx (1)
42-42: 불필요한 코드를 제거하세요.
?? undefined는 중복입니다. JSX에서 falsy 값은 자동으로 렌더링되지 않습니다.🔎 제안된 수정사항
- {secondaryAction ?? undefined} + {secondaryAction}src/components/NavigationBar/_components/Dialog.tsx (2)
37-50: primaryAction과 secondaryAction의 명명이 혼란스러울 수 있습니다.현재 "취소" 버튼이
primaryAction으로, "탈퇴하기" 버튼이secondaryAction으로 전달되고 있습니다. 시각적 스타일은 올바르지만(취소는 회색, 탈퇴는 빨강), 의미상으로는 반대로 보일 수 있습니다.기본 Dialog 컴포넌트가 단순히 순서대로 렌더링하는 것이라면 문제없지만, 향후 primary/secondary에 다른 의미(예: 포커스 순서, 키보드 단축키)가 부여될 경우 혼란을 야기할 수 있습니다.
다음 중 하나를 고려해보세요:
- 명명을
leftAction/rightAction또는cancelAction/confirmAction으로 변경- 현재 방식을 유지하되 주석으로 의도를 명확히 표시
- Dialog 컴포넌트에서 actions 배열을 받아 순서를 명시적으로 제어
29-29: TODO 주석이 명확합니다.향후 필요한 개선사항이 잘 문서화되어 있습니다.
Callout이나 Checkbox 컴포넌트 구현이 필요하시면 새 이슈를 생성하도록 도와드릴 수 있습니다. 필요하신가요?
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/assets/icons/close.svgis excluded by!**/*.svgsrc/assets/icons/triangle-alert.svgis excluded by!**/*.svgsrc/components/Icon/generated/Close.tsxis excluded by!**/generated/**src/components/Icon/generated/TriangleAlert.tsxis excluded by!**/generated/**src/components/Icon/generated/index.tsxis excluded by!**/generated/**
📒 Files selected for processing (13)
src/components/Card/Card.tsxsrc/components/Dialog/Dialog.tsxsrc/components/Dialog/Dialog.types.tssrc/components/Icon/iconMap.tssrc/components/Label/Label.styles.tssrc/components/Label/Label.tsxsrc/components/Label/Label.types.tssrc/components/Logo/Icon.styles.tssrc/components/Logo/Logo.tsxsrc/components/Logo/Logo.types.tssrc/components/NavigationBar/NavigationBar.tsxsrc/components/NavigationBar/_components/Dialog.tsxsrc/components/NavigationBar/_components/WithdrawIcon.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
src/components/Icon/iconMap.ts (1)
src/components/Icon/Icon.tsx (1)
Icon(4-18)
src/components/NavigationBar/NavigationBar.tsx (2)
src/components/Logo/Logo.tsx (1)
Logo(34-37)src/components/Dialog/Dialog.tsx (1)
Dialog(5-51)
src/components/NavigationBar/_components/Dialog.tsx (2)
src/components/Dialog/Dialog.types.ts (1)
DialogProps(12-12)src/components/Logo/Logo.tsx (1)
Logo(34-37)
src/components/Logo/Logo.tsx (3)
src/components/Logo/Logo.types.ts (2)
IconLogoProps(3-5)BasicLogoProps(7-7)src/components/Logo/Icon.styles.ts (1)
iconSizeMap(1-4)src/components/Icon/Icon.tsx (1)
Icon(4-18)
src/components/Dialog/Dialog.tsx (2)
src/components/Dialog/Dialog.types.ts (1)
DialogProps(12-12)src/components/Icon/Icon.tsx (1)
Icon(4-18)
src/components/NavigationBar/_components/WithdrawIcon.tsx (1)
src/components/Icon/Icon.tsx (1)
Icon(4-18)
🔇 Additional comments (11)
src/components/Card/Card.tsx (1)
1-1: LGTM!Import 경로가 디렉토리 구조에 맞게 올바르게 수정되었습니다.
src/components/Label/Label.types.ts (1)
2-2: LGTM!상대 경로가 올바르게 수정되었습니다.
src/components/Icon/iconMap.ts (1)
19-23: LGTM!Dialog 닫기 버튼(
close)과 탈퇴 아이콘(triangleAlert)에 필요한 아이콘이 iconMap에 올바르게 추가되었습니다.src/components/NavigationBar/NavigationBar.tsx (1)
21-22: "프롬프트 등록" 버튼도 Login Dialog를 표시하는 것이 의도된 동작인지 확인 필요현재
AddPromptButton과LoginButton모두Dialog.Login을 사용하고 있습니다. TODO 주석에서 언급된 것처럼 향후 로그인 여부에 따른 분기가 필요하다면, 비로그인 상태에서 프롬프트 등록 시 로그인 유도는 합리적인 UX입니다. 다만, 로그인된 사용자에게는 다른 다이얼로그(프롬프트 작성)가 표시되어야 할 것으로 보입니다.src/components/Logo/Logo.types.ts (1)
3-7: LGTM!Logo 컴포넌트 변형별로 타입이 명확하게 분리되었습니다.
sizeprop이iconSizeMap의 키와 일치하며,HTMLAttributes확장으로 네이티브 HTML 속성도 지원됩니다.src/components/NavigationBar/_components/WithdrawIcon.tsx (1)
5-7: stroke/fill props로 SVG 속성 오버라이드 작동 중 - 추가 수정 불필요The
strokeandfillprops are correctly passed through the Icon component to the SVG element and properly override the inline SVG attributes. However, the suggestion to use Tailwind stroke utilities (stroke-red-600) would not work in this context because inline SVG attributes take precedence over CSS classes.The current implementation is correct. For improved consistency with the design system, consider using the Icon component's
colorprop instead:<Icon name="triangleAlert" size="xl" color="#e7000b" />, which leverages the SVG'scurrentColorvalues.Likely an incorrect or invalid review comment.
src/components/Dialog/Dialog.tsx (2)
5-51: 전체적인 Dialog 구현이 잘 되어 있습니다.Radix UI 기반의 접근성을 고려한 Dialog 구조, Portal을 통한 올바른 렌더링, 그리고 확장 가능한 컴포넌트 설계가 잘 구현되어 있습니다.
1-1: 코드가 올바릅니다. 추가 조치가 필요하지 않습니다.
radix-ui는 Radix Primitives의 공식 메타 패키지이며,import { Dialog as DialogPrimitive } from 'radix-ui';는 유효한 import 문법입니다. Dialog를 가져오는 두 가지 공식 방식이 모두 지원됩니다:
- 메타 패키지 사용 (현재 코드):
import { Dialog } from "radix-ui";- 개별 패키지 사용:
import * as DialogPrimitive from "@radix-ui/react-dialog";Likely an incorrect or invalid review comment.
src/components/NavigationBar/_components/Dialog.tsx (1)
8-25: LoginDialog 구현이 깔끔합니다.카카오 로그인을 위한 적절한 색상과 레이아웃이 잘 구현되어 있습니다. 캡션을 통한 약관 동의 안내도 UX 관점에서 좋습니다.
src/components/Logo/Logo.tsx (2)
3-4: import 구조가 명확합니다.타입과 스타일을 분리하여 import하는 구조가 좋습니다.
6-14: size 속성 구현이 정확하고 완벽합니다.기본값 설정, 타입 안전성, 그리고 iconSizeMap을 통한 스타일링이 깔끔하게 구현되어 있습니다. Icon 컴포넌트의 sizeMap에는 'md'(20px)와 'xl'(32px)이 모두 포함되어 있으며, Logo 컴포넌트에서 size 속성을 올바르게 전달하고 있습니다.
| <button | ||
| className="absolute top-4 right-4 p-2 text-gray-400 hover:text-gray-600 rounded-lg hover:bg-gray-100 transition-colors" | ||
| aria-label="닫기"> | ||
| <Icon name="close" size="md" aria-hidden /> |
There was a problem hiding this comment.
aria-hidden 속성 문법을 수정하세요.
React에서 boolean 속성은 명시적으로 값을 지정해야 합니다.
🔎 제안된 수정사항
- <Icon name="close" size="md" aria-hidden />
+ <Icon name="close" size="md" aria-hidden="true" />📝 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.
| <Icon name="close" size="md" aria-hidden /> | |
| <Icon name="close" size="md" aria-hidden="true" /> |
🤖 Prompt for AI Agents
In src/components/Dialog/Dialog.tsx around line 26, the JSX uses the boolean
attribute aria-hidden without an explicit value; update the attribute to use
explicit boolean syntax (e.g., aria-hidden={true} or aria-hidden={false} as
appropriate) so React treats it as a boolean prop, and ensure the chosen value
reflects whether the icon should be exposed to assistive technologies.
*렌더링마다 생성 방지
*추후 props 네이밍 변경을 위함
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/Dialog/Dialog.tsx (1)
27-27: aria-hidden 속성 문법을 수정하세요.이 문제는 이전 리뷰에서 이미 지적되었습니다. React에서 boolean 속성은 명시적으로 값을 지정해야 합니다.
🔎 제안된 수정사항
- <Icon name="close" size="md" aria-hidden /> + <Icon name="close" size="md" aria-hidden="true" />
🧹 Nitpick comments (3)
src/components/Dialog/Dialog.tsx (3)
5-5: TODO 해결을 도와드릴 수 있습니다.액션 버튼의 네이밍에 대한 고민이 남아있네요. 일반적인 대안으로는
confirmAction/cancelAction,onConfirm/onCancel, 또는submitButton/cancelButton등이 있습니다.네이밍 개선안 코드를 생성하거나 이 작업을 추적할 이슈를 열어드릴까요?
21-22: DialogContent 래핑 구조를 재검토하세요.DialogPrimitive.Content를 별도의
div로 감싸는 것은 Radix UI의 표준 패턴이 아닙니다. 일반적으로 DialogContent는 Portal의 직접 자식으로 배치되며, 자체적으로 포지셔닝을 처리합니다. 이 추가 래퍼는 Radix의 내장 포지셔닝, 포커스 트래핑 또는 접근성 동작을 방해할 수 있습니다.센터링이 필요하다면 DialogContent의 className에 직접 적용하는 것을 권장합니다.
🔎 권장 구조 수정안
<DialogPrimitive.Portal> <DialogPrimitive.Overlay className="fixed inset-0 bg-black/50 z-50" /> - <div className="fixed inset-0 flex items-center justify-center z-50 p-4"> - <DialogPrimitive.Content className="bg-white rounded-xl max-w-md w-full p-8 relative focus:outline-none"> + <DialogPrimitive.Content className="fixed left-1/2 top-1/2 -translate-x-1/2 -translate-y-1/2 bg-white rounded-xl max-w-md w-full p-8 relative focus:outline-none z-50"> <DialogPrimitive.Close asChild> <button className="absolute top-4 right-4 p-2 text-gray-400 hover:text-gray-600 rounded-lg hover:bg-gray-100 transition-colors" aria-label="닫기"> <Icon name="close" size="md" aria-hidden /> </button> </DialogPrimitive.Close> <div className="flex flex-col gap-6"> {/* ... content ... */} </div> </DialogPrimitive.Content> - </div> </DialogPrimitive.Portal>
41-44: 모바일 환경에서 버튼 레이아웃을 고려하세요.현재 버튼들이
flex gap-3로 가로 배치되어 있습니다. 좁은 화면에서는 버튼이 잘리거나 너무 작아질 수 있으니, 반응형 처리(flex-col sm:flex-row등)를 고려해보세요.🔎 반응형 레이아웃 예시
- <div className="flex gap-3"> + <div className="flex flex-col sm:flex-row gap-3"> {primaryAction} {secondaryAction} </div>
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Dialog/Dialog.tsxsrc/components/NavigationBar/NavigationBar.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/NavigationBar/NavigationBar.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/Dialog/Dialog.tsx (2)
src/components/Dialog/Dialog.types.ts (1)
DialogProps(12-12)src/components/Icon/Icon.tsx (1)
Icon(4-18)
🔇 Additional comments (2)
src/components/Dialog/Dialog.tsx (2)
33-38: 의미론적 Dialog 구조가 올바르게 구현되었습니다.DialogPrimitive.Title과 DialogPrimitive.Description을 적절히 사용하여 스크린 리더 사용자에게 올바른 컨텍스트를 제공하고 있습니다. 닫기 버튼의
aria-label도 적절합니다.
1-1: import 경로는 프로젝트의 실제 dependencies와 일치합니다.package.json에서
"radix-ui": "^1.4.3"이 정의되어 있으며, 현재 import 문from 'radix-ui'는 이와 정확히 일치합니다. 이 프로젝트는 표준 @radix-ui/react-* 패키지 대신 "radix-ui" 패키지를 사용하고 있으므로 현재 import 경로는 올바르고 변경할 필요가 없습니다.Likely an incorrect or invalid review comment.
1 participant
✅ 체크리스트
📝 작업 상세 내용
아래와 같이 사용 가능합니다.
🔎 후속 작업 (선택 사항)
📸 스크린샷 (선택 사항)
✅ 셀프 체크리스트
이슈 번호: #14
Summary by CodeRabbit
새로운 기능
리팩토링
✏️ Tip: You can customize this high-level summary in your review settings.