Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough북마크 및 리마인드 페이지의 삭제 흐름을 즉시 삭제에서 확인 모달을 거치는 2단계로 변경했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 사용자
participant M as OptionsMenu
participant P as 페이지(MyBookmark/Remind)
participant D as PopupContainer(확인 모달)
participant S as DeleteMutation
participant Q as QueryCache
U->>M: "삭제" 선택
M->>P: onDelete(id)
P->>P: set deleteTargetId, isDeleteOpen = true
P->>D: 확인 모달 렌더
alt 취소
U-->>D: "취소"
D->>P: close()
P->>P: reset deleteTargetId, isDeleteOpen = false
else 삭제 확정
U-->>D: "삭제"
D->>P: confirm(deleteTargetId)
P->>S: mutate(deleteTargetId)
S-->>P: onSuccess
P->>Q: invalidate related queries (bookmark*, categoryBookmarkArticles, remindArticles, arcons)
P->>P: reset deleteTargetId, isDeleteOpen = false, close menu
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 chromatic 배포 확인: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/client/src/pages/myBookmark/MyBookmark.tsx (2)
65-75: 쿼리 무효화 및 상태 리셋 로직 개선삭제 성공 후 모달 상태와 타겟 ID를 리셋하고 메뉴를 닫는 로직이 올바르게 구현되어 있습니다. TODO 주석대로 쿼리키 팩토리 패턴을 적용하면 더 좋을 것 같습니다.
쿼리키 팩토리 패턴을 적용하여 중복 코드를 제거할 수 있습니다:
// utils/queryKeys.ts (새 파일) export const queryKeys = { bookmarkReadArticles: ['bookmarkReadArticles'] as const, bookmarkUnreadArticles: ['bookmarkUnreadArticles'] as const, categoryBookmarkArticles: ['categoryBookmarkArticles'] as const, arcons: ['arcons'] as const, } as const;그러면 다음과 같이 사용할 수 있습니다:
- queryClient.invalidateQueries({ queryKey: ['bookmarkReadArticles'] }); - queryClient.invalidateQueries({ queryKey: ['bookmarkUnreadArticles'] }); + queryClient.invalidateQueries({ queryKey: queryKeys.bookmarkReadArticles }); + queryClient.invalidateQueries({ queryKey: queryKeys.bookmarkUnreadArticles });
193-221: 모달 접근성 및 구현 검토삭제 확인 모달의 기본 구조는 좋지만, 접근성 측면에서 몇 가지 개선이 필요합니다.
모달이 열렸을 때 포커스가 모달 내부로 이동되고 트랩되어야 합니다. 또한 모달이 닫힐 때 포커스가 트리거 요소로 돌아가야 합니다.
다음과 같이 개선할 수 있습니다:
{isDeleteOpen && ( - <div className="fixed inset-0" aria-modal="true" role="dialog"> + <div className="fixed inset-0" aria-modal="true" role="alertdialog" aria-labelledby="delete-modal-title"> <div className="absolute inset-0 bg-black/60" onClick={() => setIsDeleteOpen(false)} /> <div className="absolute inset-0 z-[100] flex items-center justify-center p-4"> <PopupContainer isOpen type="subtext" title="정말 삭제하시겠어요?" + titleId="delete-modal-title" subtext="저장된 내용이 모두 사라지게 돼요." left="취소" right="삭제" onLeftClick={() => setIsDeleteOpen(false)} onRightClick={() => { if (deleteTargetId != null) { handleDeleteArticle(deleteTargetId); } else { setIsDeleteOpen(false); } }} onClose={() => setIsDeleteOpen(false)} + autoFocus /> </div> </div> )}주요 개선사항:
role="alertdialog"사용 (삭제라는 중요한 작업을 위해)aria-labelledby속성으로 제목과 연결autoFocus속성 추가로 포커스 관리 개선apps/client/src/pages/remind/Remind.tsx (2)
160-186: 모달 접근성 개선 필요기본적인 삭제 확인 모달 구조는 좋지만, MyBookmark.tsx와 마찬가지로 접근성 측면에서 개선이 필요합니다.
MyBookmark.tsx에 제안한 것과 동일한 접근성 개선사항을 적용해주세요:
{isDeleteOpen && ( - <div className="fixed inset-0" aria-modal="true" role="dialog"> + <div className="fixed inset-0" aria-modal="true" role="alertdialog" aria-labelledby="delete-modal-title"> <div className="absolute inset-0 bg-black/60" onClick={() => setIsDeleteOpen(false)} /> <div className="absolute inset-0 z-[100] flex items-center justify-center p-4"> <PopupContainer isOpen type="subtext" title="정말 삭제하시겠어요?" + titleId="delete-modal-title" subtext="저장된 내용이 모두 사라지게 돼요." left="취소" right="삭제" onLeftClick={() => setIsDeleteOpen(false)} onRightClick={() => { if (deleteTargetId != null) { handleDeleteArticle(deleteTargetId); } else { setIsDeleteOpen(false); } }} onClose={() => setIsDeleteOpen(false)} + autoFocus /> </div> </div> )}
160-186: 코드 중복 문제두 파일에서 거의 동일한 삭제 확인 모달 코드가 중복되고 있습니다. 공통 컴포넌트로 추출하는 것을 고려해보세요.
다음과 같이 재사용 가능한 컴포넌트로 추출할 수 있습니다:
// components/common/DeleteConfirmModal.tsx interface DeleteConfirmModalProps { isOpen: boolean; onConfirm: () => void; onCancel: () => void; title?: string; subtext?: string; } export const DeleteConfirmModal: React.FC<DeleteConfirmModalProps> = ({ isOpen, onConfirm, onCancel, title = "정말 삭제하시겠어요?", subtext = "저장된 내용이 모두 사라지게 돼요." }) => { if (!isOpen) return null; return ( <div className="fixed inset-0" aria-modal="true" role="alertdialog"> <div className="absolute inset-0 bg-black/60" onClick={onCancel} /> <div className="absolute inset-0 z-[100] flex items-center justify-center p-4"> <PopupContainer isOpen type="subtext" title={title} subtext={subtext} left="취소" right="삭제" onLeftClick={onCancel} onRightClick={onConfirm} onClose={onCancel} /> </div> </div> ); };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/client/src/pages/myBookmark/MyBookmark.tsx(4 hunks)apps/client/src/pages/remind/Remind.tsx(4 hunks)
🔇 Additional comments (8)
apps/client/src/pages/myBookmark/MyBookmark.tsx (4)
1-1: PopupContainer 컴포넌트 추가 - LGTM!디자인 시스템에서 PopupContainer를 가져와서 삭제 확인 모달에 사용하는 것은 좋은 접근입니다.
28-30: 삭제 확인 플로우를 위한 상태 관리 - LGTM!
isDeleteOpen과deleteTargetId상태를 추가하여 2단계 삭제 플로우를 구현한 것은 UX 측면에서 적절합니다. 사용자의 실수로 인한 데이터 손실을 방지할 수 있습니다.
185-189: 삭제 확인 플로우 구현 - LGTM!즉시 삭제에서 확인 모달을 거치는 2단계 플로우로 변경한 것은 사용자 안전성을 고려한 좋은 개선입니다.
226-226: 편집 모달의 블러 효과 제거 - LGTM!편집 모달에서
backdrop-blur-[2px]를 제거한 것은 일관성을 위한 좋은 변경입니다.apps/client/src/pages/remind/Remind.tsx (4)
2-2: PopupContainer 컴포넌트 추가 - LGTM!디자인 시스템에서 PopupContainer를 일관성 있게 사용하는 것은 좋은 접근입니다.
23-25: 삭제 확인 플로우 상태 관리 - LGTM!MyBookmark.tsx와 동일한 패턴으로 삭제 확인 상태를 구현한 것이 일관성 있고 좋습니다.
56-61: arcons 쿼리 무효화 추가 및 상태 리셋
arcons쿼리 무효화를 추가하고 삭제 모달 상태를 적절히 리셋하는 로직이 잘 구현되어 있습니다. Line 60의close()호출도 있는 것으로 보아 추가적인 정리 작업이 포함되어 있습니다.
152-156: 일관된 삭제 확인 플로우 구현 - LGTM!MyBookmark.tsx와 동일한 패턴으로 삭제 확인 플로우를 구현한 것이 일관성 있고 좋습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/client/src/pages/myBookmark/MyBookmark.tsx (3)
28-30: 삭제 모달 상태 분리 OK, 닫힘 시 일관 초기화 권장모달을 어떤 경로로 닫아도
deleteTargetId를 함께 초기화하는 공용 클로저를 두면 안정적입니다. 아래 모달 블록 코멘트에 제안 diff 포함했습니다.
193-221: 삭제 모달: 배경 딤 및 z-index 불일치(편집 모달 대비)·닫힘 처리 일관화
- 현재 오버레이가 투명(
bg-*없음)이라 배경이 어두워지지 않습니다. 편집 모달(라인 224~)과 동일하게bg-black/60적용 권장.- 래퍼
div에z-[1000]이 없어 헤더/포털보다 아래에 깔릴 수 있습니다. 편집 모달과 동일 z 계층으로 통일 권장.- 모달 닫힘 경로(오버레이 클릭, 왼쪽 버튼, onClose)에서
deleteTargetId초기화 누락. 공용closeDeleteModal로 일관 처리 권장.아래 diff를 적용하면 일관성과 접근성이 개선됩니다.
- {isDeleteOpen && ( - <div className="fixed inset-0" aria-modal="true" role="dialog"> + {isDeleteOpen && ( + <div className="fixed inset-0 z-[1000]" aria-modal="true" role="dialog"> <div - className="absolute inset-0" - onClick={() => setIsDeleteOpen(false)} + className="absolute inset-0 bg-black/60" + onClick={closeDeleteModal} /> <div className="absolute inset-0 z-[100] flex items-center justify-center p-4"> <PopupContainer isOpen type="subtext" title="정말 삭제하시겠어요?" subtext="저장된 내용이 모두 사라지게 돼요." left="취소" right="삭제" - onLeftClick={() => setIsDeleteOpen(false)} + onLeftClick={closeDeleteModal} onRightClick={() => { if (deleteTargetId != null) { handleDeleteArticle(deleteTargetId); } else { - setIsDeleteOpen(false); + closeDeleteModal(); } }} - onClose={() => setIsDeleteOpen(false)} + onClose={closeDeleteModal} /> </div> </div> )}컴포넌트 외곽(상단 상태 선언 인근)에 아래 유틸을 추가하세요:
// 제안: 모달 닫힘 공용 처리 const closeDeleteModal = () => { setIsDeleteOpen(false); setDeleteTargetId(null); };
222-227: 편집 모달 오버레이 딤 적용 LGTM, z-index 일관 유지 권장편집 모달은
z-[1000]+bg-black/60로 정리되어 좋습니다. 삭제 모달도 동일 계층으로 통일(위 diff 참고)하면 레이어링 이슈 방지됩니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/client/src/pages/myBookmark/MyBookmark.tsx(4 hunks)apps/client/src/pages/remind/Remind.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/client/src/pages/remind/Remind.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-17T09:18:13.818Z
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#102
File: apps/extension/src/components/modalPop/ModalPop.tsx:166-172
Timestamp: 2025-07-17T09:18:13.818Z
Learning: In apps/extension/src/components/modalPop/ModalPop.tsx, the categories array should include "안 읽은 정보" (Unread Information) as the first default category that cannot be deleted. This default category is used consistently across the client-side dashboard and should be protected from deletion in the extension as well.
Applied to files:
apps/client/src/pages/myBookmark/MyBookmark.tsx
🔇 Additional comments (3)
apps/client/src/pages/myBookmark/MyBookmark.tsx (3)
1-1: PopupContainer 도입 LGTM디자인시스템 컴포넌트 사용 적절합니다.
186-189: 메뉴→모달 전환 흐름 적절삭제 타깃 설정 후 모달 오픈, 메뉴 닫기 순서 자연스럽습니다.
65-75: 확인 결과 — 'arcons'는 유효, 삭제 훅 재사용 여부 확인 필요
- 'arcons'는 존재하며 apps/client/src/shared/apis/queries.ts의 useGetArcons에서 queryKey ['arcons']로 정의되어 있고 Remind와 MyBookmark에서 invalidateQueries로 사용 중(오타 아님).
- MyBookmark가 사용 중인 useDeleteRemindArticle은 apps/client/src/shared/apis/queries.ts에 정의되어 있고(내부적으로 deleteRemindArticle 사용) 레포에서 useDeleteBookmarkArticle 같은 별도 훅은 발견되지 않음. 이 재사용이 의도된 것인지 확인하거나, 의도와 다르면 북마크 전용 삭제 훅/엔드포인트로 분리하세요. (참고: apps/client/src/pages/myBookmark/MyBookmark.tsx).
- 중복된 invalidateQueries는 현재 TODO와 동일하게 Query Key Factory로 통일해 정합성/유지보수성 개선 권장.
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
📷 Screenshot
Summary by CodeRabbit