-
Notifications
You must be signed in to change notification settings - Fork 0
20250917 #16 기능개선 ux 로그아웃 버튼 클릭 시 확인 alert 추가 #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
20250917 #16 기능개선 ux 로그아웃 버튼 클릭 시 확인 alert 추가 #17
The head ref may contain hidden characters: "20250917_#16_\uAE30\uB2A5\uAC1C\uC120_UX_\uB85C\uADF8\uC544\uC6C3_\uBC84\uD2BC_\uD074\uB9AD_\uC2DC_\uD655\uC778_Alert_\uCD94\uAC00"
Conversation
Walkthrough로그아웃 확인 다이얼로그 기능을 도입했다. Radix UI AlertDialog 의존성을 추가하고, 공용 AlertDialog UI 래퍼를 생성했다. 로그아웃 전 확인 모달 컴포넌트(LogoutAlertDialog)를 추가하고, AppSidebar에서 직접 로그아웃 호출을 확인 다이얼로그 기반 플로우로 변경했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant S as AppSidebar
participant D as LogoutAlertDialog
participant A as AuthService
participant T as TokenStore
participant R as Router
U->>S: 클릭: 로그아웃 버튼
S->>D: open=true (다이얼로그 표시)
U->>D: 확인(Logout) 클릭
D->>S: onConfirm 호출
S->>A: logout()
activate A
A-->>S: 완료/성공
deactivate A
S->>T: 토큰 삭제/정리
S->>D: open=false (닫기)
S->>R: /login로 이동
alt 취소 선택
U->>D: 취소(Cancel) 클릭
D->>S: onOpenChange(false)
S->>D: open=false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/common/app-sidebar.tsx (1)
56-71: 로그아웃 API 실패 시에도 로컬 세션 종료를 보장하세요.현재는 API 실패 시 토큰이 남아 세션이 유지될 수 있습니다. 토큰 삭제/리다이렉트를 finally로 이동해 항상 로그아웃되도록 하세요.
const onConfirmLogout = useCallback(async () => { if (loading) { return; } setLoading(true); try { - await logout(); - localStorage.removeItem('accessToken'); - localStorage.removeItem('refreshToken'); - setDialogOpen(false); - router.replace('/login'); + await logout(); } finally { - setLoading(false); + // API 성공/실패와 무관하게 로컬 세션 종료 + localStorage.removeItem('accessToken'); + localStorage.removeItem('refreshToken'); + setDialogOpen(false); + router.replace('/login'); + setLoading(false); } }, [loading, router]);
🧹 Nitpick comments (4)
src/components/ui/alert-dialog.tsx (3)
121-131: Action/Cancel에 type="button" 지정 권장 (폼 내 오동작 예방).폼 내부에서 사용될 때 기본 submit을 막기 위해 명시적으로 버튼 타입을 지정해주세요.
function AlertDialogAction({ className, ...props }: React.ComponentProps<typeof AlertDialogPrimitive.Action>) { return ( <AlertDialogPrimitive.Action + type="button" className={cn(buttonVariants(), className)} {...props} /> ) } function AlertDialogCancel({ className, ...props }: React.ComponentProps<typeof AlertDialogPrimitive.Cancel>) { return ( <AlertDialogPrimitive.Cancel + type="button" className={cn(buttonVariants({ variant: "outline" }), className)} {...props} /> ) }Also applies to: 133-143
47-64: Content는 forwardRef로 감싸는 것이 바람직합니다.Radix 프리미티브와 일관되게 ref 전달을 지원하면 포커스 관리/측정에 유리합니다. (Title/Description/Action/Cancel도 동일 패턴 적용 권장)
-function AlertDialogContent({ - className, - ...props -}: React.ComponentProps<typeof AlertDialogPrimitive.Content>) { - return ( - <AlertDialogPortal> - <AlertDialogOverlay /> - <AlertDialogPrimitive.Content - data-slot="alert-dialog-content" - className={cn( - "bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 fixed top-[50%] left-[50%] z-50 grid w-full max-w-[calc(100%-2rem)] translate-x-[-50%] translate-y-[-50%] gap-4 rounded-lg border p-6 shadow-lg duration-200 sm:max-w-lg", - className - )} - {...props} - /> - </AlertDialogPortal> - ) -} +const AlertDialogContent = React.forwardRef< + React.ElementRef<typeof AlertDialogPrimitive.Content>, + React.ComponentPropsWithoutRef<typeof AlertDialogPrimitive.Content> +>(({ className, ...props }, ref) => ( + <AlertDialogPortal> + <AlertDialogOverlay /> + <AlertDialogPrimitive.Content + ref={ref} + data-slot="alert-dialog-content" + className={cn( + "bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 fixed top-[50%] left-[50%] z-50 grid w-full max-w-[calc(100%-2rem)] translate-x-[-50%] translate-y-[-50%] gap-4 rounded-lg border p-6 shadow-lg duration-200 sm:max-w-lg", + className + )} + {...props} + /> + </AlertDialogPortal> +)); +AlertDialogContent.displayName = "AlertDialogContent";
9-13: Root에 부여한 data-slot은 DOM에 출력되지 않습니다.
AlertDialogPrimitive.Root는 DOM을 렌더하지 않아서 테스트 셀렉터로 동작하지 않습니다.Content/Overlay의data-slot을 기준으로 테스트하는 것을 권장합니다.src/app/(with-layout)/_shared/components/logout-alert-dialog.tsx (1)
21-21: 로딩 중에는 다이얼로그가 닫히지 않도록 onOpenChange에 가드 추가 권장.
AlertDialogAction클릭 시 기본적으로 닫힘 이벤트가 발생합니다. 로딩 상태에서 닫힘을 무시하면 UX가 안정적입니다.- <AlertDialog open={props.open} onOpenChange={props.onOpenChange}> + <AlertDialog + open={props.open} + onOpenChange={(next) => { + if (!props.loading) props.onOpenChange(next) + }} + >
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.json(1 hunks)src/app/(with-layout)/_shared/components/logout-alert-dialog.tsx(1 hunks)src/components/common/app-sidebar.tsx(3 hunks)src/components/ui/alert-dialog.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/common/app-sidebar.tsx (1)
src/app/(with-layout)/_shared/components/logout-alert-dialog.tsx (1)
LogoutAlertDialog(19-38)
src/app/(with-layout)/_shared/components/logout-alert-dialog.tsx (1)
src/components/ui/alert-dialog.tsx (8)
AlertDialog(146-146)AlertDialogContent(150-150)AlertDialogHeader(151-151)AlertDialogTitle(153-153)AlertDialogDescription(154-154)AlertDialogFooter(152-152)AlertDialogCancel(156-156)AlertDialogAction(155-155)
src/components/ui/alert-dialog.tsx (1)
src/lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (3)
package.json (1)
13-13: LGTM — 피어 의존성(React 19) 호환 확인됨package.json의 peerDependencies에 'react'/'react-dom'이 ^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc로 선언되어 React 19을 허용하므로 Next 15(React 19 사용)과 호환됩니다.
- PR에 lockfile(pnpm-lock.yaml / yarn.lock / package-lock.json) 업데이트 포함 여부 확인 필요.
src/components/common/app-sidebar.tsx (2)
78-79: 로고 파일(public/icons/logo.svg) 존재 여부 확인 필요src/components/common/app-sidebar.tsx(78–79행)의 Image src가 '/icons/logo.svg'로 설정되어 있습니다. 저장소에 public/icons/logo.svg 파일이 있는지 확인하세요. 누락 시 빌드/런타임에 404가 발생합니다. 실행한 검색 명령에서 결과가 없어 직접 확인이 필요합니다.
64-65: 토큰 키 네이밍 일관성 확인 — 문제 없음. 앱 전역에서 'accessToken' / 'refreshToken' 키를 일관되게 사용합니다 (예: src/app/(without-layout)/login/_shared/components/login-form.tsx, src/app/(without-layout)/login/_shared/services/login-api.ts, src/app/api/login/route.ts, src/middleware.ts, src/components/common/app-sidebar.tsx).
Summary by CodeRabbit