Conversation
주제 확정 모달 및 관련 로직을 추가. 사용자가 제안된 주제를 순서대로 선택해 확정할 수 있도록 구현. - ConfirmTopicModal, ConfirmModalTopicCard 컴포넌트 추가 - useConfirmTopics mutation 훅 및 confirmTopics API 추가 - ConfirmTopicsParams, ConfirmTopicsResponse 타입 정의 - TopicHeader에 확정 버튼 활성화 조건 및 클릭 핸들러 연결
Walkthrough제안된 주제들을 선택해 확정하는 모달 기반 기능(카드·모달 컴포넌트, API 엔드포인트·타입, 뮤테이션 훅, 모의 데이터, NumberedCheckbox ref 지원)이 추가되어 클라이언트에서 주제 확정 흐름을 처리합니다. (50단어 이내) Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Modal as ConfirmTopicModal
participant CheckboxGroup as NumberedCheckboxGroup
participant Hook as useConfirmTopics
participant API as Topics API
participant QC as QueryClient
User->>Modal: 모달 열기
User->>CheckboxGroup: 카드 클릭 -> 체크/선택 토글
Modal->>Hook: 확정 요청 (gatheringId, meetingId, topicIds)
Hook->>API: confirmTopics 호출
API-->>Hook: ConfirmTopicsResponse
Hook->>QC: proposedList 캐시 무효화
Hook->>QC: confirmedList 캐시 무효화
QC-->>Modal: UI 갱신 반영
Modal-->>User: 모달 닫기 및 선택 초기화
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/features/topics/components/ConfirmTopicModal.tsx`:
- Around line 47-51: 선택 배열의 순서 불일치를 막기 위해 상태 갱신 로직을 하나로 통일하세요:
NumberedCheckboxGroup.onChange에서 사용하는 배열 생성 로직을 단일 소스(예: 새로운
updateSelectedTopics 함수)로 추출하고 setSelectedTopicIds 호출을 그 함수로만 수행하도록 하며, Card의
onClick(handleToggle)에서는 직접 setSelectedTopicIds를 호출하지 말고 이 공통
함수(updateSelectedTopics 또는 NumberedCheckboxGroup의 onChange 핸들러)를 호출해 토글 동작을
위임하세요; 즉 handleToggle은 내부에서 prev 기반의 수동 토글을 하지 말고
NumberedCheckboxGroup.onChange와 동일한 경로(공통 함수)를 호출하도록 변경해 선택 순서가 항상 동일하게 유지되도록 하고
같은 변경을 파일 내 다른 유사 위치(예: 107-109)에도 적용하세요.
- Around line 39-43: The modal always runs the useProposedTopics query even when
closed; update the ConfirmTopicModal to avoid fetching when open is false by
passing an enabled flag or changing rendering: either add an enabled option to
the hook call (e.g., useProposedTopics({ gatheringId, meetingId, pageSize: 100,
enabled: open })) after extending useProposedTopics to accept and forward an
enabled param to its internal react-query call, or alternatively stop mounting
the component when closed by changing the parent (MeetingDetailPage) to render
ConfirmTopicModal only when open (open && <ConfirmTopicModal .../>). Ensure you
reference useProposedTopics, ConfirmTopicModal, and the open prop when
implementing the change.
In `@src/pages/Meetings/MeetingDetailPage.tsx`:
- Around line 198-203: ConfirmTopicModal retains selectedTopicIds across reopen
because Radix Dialog only hides DOM; add a useEffect inside ConfirmTopicModal
that watches the open prop (the open passed into ConfirmTopicModal) and calls
setSelectedTopicIds([]) when open becomes false to reset selection; reference
the selectedTopicIds state and its setter setSelectedTopicIds and the open prop
in your effect to ensure the modal clears selections on close.
🧹 Nitpick comments (1)
src/features/topics/hooks/useConfirmTopics.ts (1)
43-57: 캐시 무효화가 정상 동작하지만, 더 넓은 범위의 무효화를 고려해볼 수 있습니다.현재
proposedList({ gatheringId, meetingId })로 무효화하면 TanStack Query v5의 partial matching 덕분에pageSize가 포함된 실제 쿼리 키도 매칭됩니다. 동작에는 문제 없습니다.다만, 향후 다른 파라미터가 추가될 경우를 대비해
topicQueryKeys.proposedLists()/topicQueryKeys.confirmedLists()수준으로 무효화하면 유지보수가 더 편할 수 있습니다.♻️ 선택적 리팩터링 제안
onSuccess: (_, variables) => { - // 제안된 주제 무효화 queryClient.invalidateQueries({ - queryKey: topicQueryKeys.proposedList({ - gatheringId: variables.gatheringId, - meetingId: variables.meetingId, - }), + queryKey: topicQueryKeys.proposedLists(), }) - // 확정된 주제 무효화 queryClient.invalidateQueries({ - queryKey: topicQueryKeys.confirmedList({ - gatheringId: variables.gatheringId, - meetingId: variables.meetingId, - }), + queryKey: topicQueryKeys.confirmedLists(), }) },
- NumberedCheckbox에 forwardRef 적용하여 외부 ref 접근 가능하도록 수정 - ConfirmModalTopicCard에서 onClick prop 제거 후 ref 클릭 방식으로 변경 - ConfirmTopicModal 닫기 시 선택 초기화 통합(handleClose), 외부 클릭 방지 추가 - ConfirmTopicModal 조건부 렌더링 적용하여 상태 초기화 보장
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/features/topics/topics.types.ts`:
- Line 38: Fix the typo in the JSDoc TODO comments by changing "없을떄" to "없을때" in
the comment string /** 주제 설명 (Todo : 없을떄 null인지 빈값인지 체크해야함)*/ and the identical
comment at the other occurrence (around line with the same text), updating both
instances to read "없을때".
mgYang53
left a comment
mgYang53
left a comment
There was a problem hiding this comment.
전체적으로 목데이터를 쓰고 계신 것 같은데, 실제 api로 언제 전환하실 예정이신가요??
| const { data: topicsInfiniteData, isLoading } = useProposedTopics({ | ||
| gatheringId, | ||
| meetingId, | ||
| pageSize: 100, |
There was a problem hiding this comment.
여기는 초기로딩 길더라도 한번에 보여줘야 UX가 좋을 것 같아서 한번에 로드했습니다
찔끔찔끔 보이면 순서 정할때 답답할것 같아서요! 다른 아이디어나 의견있으면 말씀주십셔~
There was a problem hiding this comment.
목데이터는 전체 개발 마무리되면 모두 제거하고
실제 api연동하고 테스트 해 볼 예정이었는데
아래 영애님 말대로 목데이터를 어떻게 관리할지 정해봐야겠어요~!
There was a problem hiding this comment.
무한스크롤 훅을 사용하는게 좋을까 했는데 무한 스크롤 훅이 모달에 대응하고 있지 않은 상태네요.
아직 주제는 그렇게 많이 등록되지 않는다는 가정 하에 현재는 이렇게만 하고,
추후 리팩토링 시 적용할 수 있으면 하시죠!
| /** 주제 제목 */ | ||
| title: string | ||
| /** 주제 설명 */ | ||
| /** 주제 설명 (Todo : 없을떄 null인지 빈값인지 체크해야함)*/ |
There was a problem hiding this comment.
description이 없을 때가 있으면 null 타입은 아직 고려 안하시는 이유가 있을까요?
There was a problem hiding this comment.
아 discription이 없으면 null로오는지 ''이렇게 빈값으로 오는지 아직 몰라서
목데이터제거하고 api테스트하면서 수정하려고 적어뒀습니다~
|
목데이터 관련해서는 추후 백엔드 서버가 운영되지 않는 상황도 고려해, 저는 현는 이 부분은 한 번 다 같이 방향을 맞춰보면 좋을 것 같고, |
이 부분은 각자 생각해보고 추후 정하는걸로 하시죠! @haruyam15 대신 현재 파일별로 목데이터 사용 여부 변수가 선언되어 있어 env 파일을 이용해서 공통 관리할 수 있으면 좋겠네요 |
3 participants
🚀 풀 리퀘스트 제안
📋 작업 내용
주제 확정 모달 UI 및 기능 구현
확정할 주제를 순서대로 선택하고 한 번에 확정하는 플로우
TopicHeader에서 주제가 1개 이상일 때만 "주제 확정하기" 버튼 노출
🔧 변경 사항
신규 컴포넌트
ConfirmTopicModal — 주제 확정 모달 (주제 목록 조회 + 순서 선택 + 확정 요청)
ConfirmModalTopicCard — 확정 모달 내 주제 카드 (NumberedCheckbox 포함)
신규 훅
useConfirmTopics — 주제 확정 mutation, 성공 시 제안/확정 주제 쿼리 캐시 무효화
API 레이어
topics.types.ts — ConfirmTopicsParams, ConfirmTopicsResponse 타입 추가
topics.endpoints.ts — CONFIRM 엔드포인트 추가
topics.api.ts — confirmTopics 함수 추가 (목데이터 포함)
topics.mock.ts — getMockConfirmTopics 추가, canConfirm 목데이터 true로 변경
기존 컴포넌트 변경 사항
TopicHeader — proposedTopicsCount, onOpenChange props 추가, 주제 없을 시 버튼 비활성화
MeetingDetailPage — ConfirmTopicModal 연결, isConfirmTopicOpen 상태 관리
📸 스크린샷 (선택 사항)
Summary by CodeRabbit