fix: 완료된 모임방 접근 개선 #292
Conversation
모임 메인 페이지 사용성 개선 2차 작업
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough완료된 그룹 모달을 삭제하고 MyGroupModal에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant GroupPage as "Group Page"
participant MyModal as "MyGroupModal"
participant API as "getMyRooms API"
participant Card as "GroupCard"
User->>GroupPage: 왼쪽 버튼 클릭
GroupPage->>MyModal: MyGroupModal 열기
User->>MyModal: '완료' 탭 선택
MyModal->>API: getMyRooms(type='expired', cursor?)
API-->>MyModal: rooms + nextCursor/isLast
MyModal->>Card: 렌더링 (isCompleted=true)
Card->>Card: 마감일 숨김, 참여자 텍스트 조정
alt 스크롤 끝 & 더 있음
User->>MyModal: 스크롤
MyModal->>API: loadMore(cursor)
API-->>MyModal: 추가 rooms
end
User->>Card: 완료된 그룹 클릭
Card->>GroupPage: navigate('joined-detail')
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/components/common/MainHeader.tsx(1 hunks)src/components/feed/TotalFeed.tsx(2 hunks)src/components/group/CompletedGroupModal.tsx(0 hunks)src/components/group/GroupCard.tsx(3 hunks)src/components/group/MyGroupModal.tsx(8 hunks)src/pages/group/Group.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/group/CompletedGroupModal.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/common/MainHeader.tsx (1)
src/components/common/IconButton.tsx (1)
IconButton(3-7)
src/components/group/MyGroupModal.tsx (1)
src/api/rooms/getMyRooms.ts (1)
RoomType(3-3)
🔇 Additional comments (6)
src/components/feed/TotalFeed.tsx (1)
18-24: LGTM!리스트 렌더링에 명시적인 Fragment와 고유한 key를 사용하는 것은 좋은 패턴입니다.
feedId와index를 조합한 key는 안정적이고 고유합니다.src/components/group/GroupCard.tsx (2)
16-16: LGTM!완료된 그룹의 UI를 조정하기 위한
isCompletedprop 추가가 적절합니다. 조건부 렌더링 로직이 명확하며, 완료된 그룹에서는 최대 참여자 수 표시와 마감일 정보를 적절히 숨깁니다.Also applies to: 40-46
149-149: gap 값 변경에 대한 시각적 검증 권장
gap이 6px에서 4px로 줄어들었습니다. 이 변경이 의도된 것이라면 문제없지만, 시각적으로 참여자 수와 아이콘 사이의 간격이 너무 좁지 않은지 확인해보세요.src/components/group/MyGroupModal.tsx (3)
142-143: exhaustive-deps 경고를 비활성화하는 이유 명확화 필요
eslint-disable-next-line react-hooks/exhaustive-deps주석이 있지만,loadMore함수가 의존성 배열에 포함되지 않은 것으로 보입니다.다음을 확인해주세요:
loadMore함수를 의존성 배열에 추가해야 하는지useCallback으로loadMore를 감싸서 안정적인 참조를 유지해야 하는지- 현재 경고를 비활성화하는 것이 정말 안전한지
현재 구조에서는
loadMore가 외부에 선언되어 있고 상태에 의존하므로, effect가 최신 상태를 참조하지 못할 가능성이 있습니다.
154-167: LGTM!완료된 그룹의 네비게이션 로직이 적절하게 구현되었습니다. '완료' 탭에서는 항상 joined 상세 페이지로 이동하고, 다른 상태에서는 그룹의 진행 상태에 따라 적절한 페이지로 라우팅됩니다.
178-186: LGTM!'완료' 탭 추가 및 관련 UI 업데이트가 잘 구현되었습니다:
- 탭 렌더링에 '완료' 추가
GroupCard에isCompletedprop 전달- 완료 상태에 맞는 적절한 empty state 메시지
Also applies to: 198-198, 212-223
| {type === 'home' && ( | ||
| <IconButton onClick={leftButtonClick} src={findUserLogo} alt={'사용자 찾기 아이콘'} /> | ||
| )} |
There was a problem hiding this comment.
Group 페이지에서 왼쪽 버튼이 렌더링되지 않는 문제
type === 'home' 조건으로 인해 왼쪽 버튼이 'home' 타입일 때만 렌더링됩니다. 그러나 Group.tsx(Line 107)에서는 type="group"과 함께 leftButtonClick 핸들러를 전달하고 있어, 버튼이 렌더링되지 않아 핸들러가 실행될 수 없습니다.
다음 중 하나를 적용하세요:
방안 1: Group 페이지에서도 왼쪽 버튼 렌더링
- {type === 'home' && (
+ {(type === 'home' || type === 'group') && (
<IconButton onClick={leftButtonClick} src={findUserLogo} alt={'사용자 찾기 아이콘'} />
)}방안 2: Group.tsx에서 불필요한 leftButtonClick 제거
Group.tsx에서 leftButtonClick prop을 제거하고 MyGroupBox의 클릭만 사용
📝 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.
| {type === 'home' && ( | |
| <IconButton onClick={leftButtonClick} src={findUserLogo} alt={'사용자 찾기 아이콘'} /> | |
| )} | |
| {(type === 'home' || type === 'group') && ( | |
| <IconButton onClick={leftButtonClick} src={findUserLogo} alt={'사용자 찾기 아이콘'} /> | |
| )} |
🤖 Prompt for AI Agents
In src/components/common/MainHeader.tsx around lines 42 to 44, the left
IconButton is only rendered when type === 'home', so pages using type="group"
(like Group.tsx line 107) pass a leftButtonClick handler that never gets
invoked; either render the left button for the group type as well or remove the
unused leftButtonClick prop from Group.tsx. To fix: update the render condition
to include group (e.g., type === 'home' || type === 'group') so the IconButton
is displayed and leftButtonClick will be called, or alternatively remove
leftButtonClick from Group.tsx and rely on MyGroupBox click handling; ensure
props and types are updated accordingly where you choose the approach.
| const roomType: RoomType | 'expired' = | ||
| selected === '진행중' | ||
| ? 'playing' | ||
| : selected === '모집중' | ||
| ? 'recruiting' | ||
| : 'playingAndRecruiting'; | ||
| : selected === '완료' | ||
| ? 'expired' | ||
| : 'playingAndRecruiting'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
RoomType 타입 선언 중복 제거
관련 코드 스니펫(src/api/rooms/getMyRooms.ts Line 2)에 따르면, RoomType은 이미 'expired'를 포함하고 있습니다:
export type RoomType = 'playingAndRecruiting' | 'recruiting' | 'playing' | 'expired';따라서 RoomType | 'expired'는 중복이며 RoomType만 사용하면 됩니다.
다음과 같이 수정하세요:
- const roomType: RoomType | 'expired' =
+ const roomType: RoomType =
selected === '진행중'
? 'playing'
: selected === '모집중'
? 'recruiting'
: selected === '완료'
? 'expired'
: 'playingAndRecruiting';Lines 92-99에도 동일하게 적용하세요.
Also applies to: 92-99
🤖 Prompt for AI Agents
In src/components/group/MyGroupModal.tsx around lines 55-62 (and similarly lines
92-99), the union type "RoomType | 'expired'" is redundant because RoomType
already includes 'expired'; update the variable declarations to use RoomType
only, removing the extra | 'expired' from those type annotations so the code
uses RoomType consistently in both places.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/group/MyGroupModal.tsx (1)
92-92: 이전 리뷰에서 지적된 중복 타입 제거 필요
RoomType에 이미'expired'가 포함되어 있으므로RoomType | 'expired'는 중복입니다. Line 55에서는 수정되었으나 여기서는 아직 반영되지 않았습니다.다음과 같이 수정하세요:
- const roomType: RoomType | 'expired' = + const roomType: RoomType =
🧹 Nitpick comments (2)
src/components/group/MyGroupModal.tsx (2)
142-143: ESLint 규칙 비활성화 검토자동 프리로드 로직에서
loadMore함수가 의존성 배열에 포함되지 않아 exhaustive-deps 규칙이 비활성화되었습니다. 무한 루프를 방지하기 위한 의도적인 선택으로 보이지만,loadMore함수를useCallback으로 감싸고 적절한 의존성을 명시하는 것이 더 안전한 접근일 수 있습니다.
154-167: 내비게이션 로직 정상 작동'완료' 탭 선택 시 joined detail 페이지로 이동하는 로직이 올바르게 추가되었습니다. '진행중'과 '완료' 모두 동일한 경로로 이동하므로 향후 조건을 통합하여 중복을 줄일 수 있습니다.
선택적으로 다음과 같이 리팩토링할 수 있습니다:
const handleGroupCardClick = (group: Group) => { - if (selected === '완료') { - navigate(`/group/detail/joined/${group.id}`); - } else if (selected === '모집중') { + if (selected === '모집중') { navigate(`/group/detail/${group.id}`); - } else if (selected === '진행중') { + } else if (selected === '진행중' || selected === '완료') { navigate(`/group/detail/joined/${group.id}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/group/MyGroupModal.tsx(8 hunks)src/pages/group/Group.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/group/Group.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/group/MyGroupModal.tsx (1)
src/api/rooms/getMyRooms.ts (1)
RoomType(3-3)
🔇 Additional comments (6)
src/components/group/MyGroupModal.tsx (6)
24-24: LGTM!'완료' 탭 추가에 맞춰 state 타입이 올바르게 확장되었습니다.
55-62: LGTM!'완료' 탭에 대한
'expired'roomType 매핑이 올바르게 추가되었고, 타입 선언도 정확합니다.
178-178: LGTM!'완료' 탭이 올바르게 추가되었으며,
as const를 통해 타입 안전성도 확보되었습니다.
198-198: LGTM!
isCompletedprop이 올바르게 전달되어GroupCard가 완료 상태를 인식할 수 있습니다.
212-214: LGTM!'완료' 탭의 빈 상태 메시지가 적절하게 추가되었습니다.
221-223: LGTM!'완료' 탭의 빈 상태 보조 텍스트가 적절하게 추가되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/group/MyGroupModal.tsx (3)
55-62: roomType 계산 로직 중복 제거동일한 roomType 계산 로직이 두 곳(초기 fetch와 loadMore)에서 반복됩니다. 헬퍼 함수로 추출하면 유지보수성이 향상됩니다.
다음과 같이 리팩토링할 수 있습니다:
+ const getRoomTypeFromSelected = (selected: '진행중' | '모집중' | '완료' | ''): RoomType => { + if (selected === '진행중') return 'playing'; + if (selected === '모집중') return 'recruiting'; + if (selected === '완료') return 'expired'; + return 'playingAndRecruiting'; + }; + useEffect(() => { const fetchRooms = async () => { try { setIsLoading(true); setError(null); setNextCursor(null); setIsLast(false); - const roomType: RoomType = - selected === '진행중' - ? 'playing' - : selected === '모집중' - ? 'recruiting' - : selected === '완료' - ? 'expired' - : 'playingAndRecruiting'; + const roomType = getRoomTypeFromSelected(selected); const response = await getMyRooms(roomType, null);그리고 loadMore 함수에서도 동일하게 적용:
const loadMore = async () => { if (isFetchingRef.current || isLast || !nextCursor) return; isFetchingRef.current = true; setIsLoading(true); try { - const roomType: RoomType = - selected === '진행중' - ? 'playing' - : selected === '모집중' - ? 'recruiting' - : selected === '완료' - ? 'expired' - : 'playingAndRecruiting'; + const roomType = getRoomTypeFromSelected(selected); const res = await getMyRooms(roomType, nextCursor);Also applies to: 92-99
154-167: 네비게이션 로직 단순화 고려네비게이션 로직이 중첩된 조건문으로 구성되어 있어 가독성이 다소 떨어집니다. 더 명확한 구조로 개선할 수 있습니다.
다음과 같이 단순화할 수 있습니다:
const handleGroupCardClick = (group: Group) => { - if (selected === '완료') { + // 완료 또는 진행중인 모임방은 joined detail로 이동 + if (selected === '완료' || selected === '진행중') { navigate(`/group/detail/joined/${group.id}`); - } else if (selected === '모집중') { + } else if (selected === '모집중') { navigate(`/group/detail/${group.id}`); - } else if (selected === '진행중') { - navigate(`/group/detail/joined/${group.id}`); } else { + // 전체 탭에서는 진행중 여부에 따라 분기 if (group.isOnGoing) { navigate(`/group/detail/joined/${group.id}`); } else { navigate(`/group/detail/${group.id}`); } } };
207-224: 빈 상태 메시지 가독성 개선 고려중첩된 삼항 연산자가 여러 번 사용되어 가독성이 떨어집니다. 헬퍼 함수나 객체 매핑을 통해 개선할 수 있습니다.
다음과 같이 리팩토링할 수 있습니다:
+ const getEmptyStateMessages = () => { + const messages = { + 진행중: { + title: '진행중인 모임방이 없어요', + subText: '진행중인 모임방에 참여해보세요!', + }, + 모집중: { + title: '모집중인 모임방이 없어요', + subText: '모집중인 모임방에 참여해보세요!', + }, + 완료: { + title: '완료된 모임방이 없어요', + subText: '아직 완료된 모임방이 없습니다.', + }, + '': { + title: '참여중인 모임방이 없어요', + subText: '첫 번째 모임방에 참여해보세요!', + }, + }; + return messages[selected]; + }; + + const emptyMessages = getEmptyStateMessages(); + // ... {!isLoading && convertedGroups.length === 0 && ( <EmptyState> - <EmptyTitle> - {selected === '진행중' - ? '진행중인 모임방이 없어요' - : selected === '모집중' - ? '모집중인 모임방이 없어요' - : selected === '완료' - ? '완료된 모임방이 없어요' - : '참여중인 모임방이 없어요'} - </EmptyTitle> - <EmptySubText> - {selected === '진행중' - ? '진행중인 모임방에 참여해보세요!' - : selected === '모집중' - ? '모집중인 모임방에 참여해보세요!' - : selected === '완료' - ? '아직 완료된 모임방이 없습니다.' - : '첫 번째 모임방에 참여해보세요!'} - </EmptySubText> + <EmptyTitle>{emptyMessages.title}</EmptyTitle> + <EmptySubText>{emptyMessages.subText}</EmptySubText> </EmptyState> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/components/group/MyGroupModal.tsx(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/group/MyGroupModal.tsx (1)
src/pages/searchBook/SearchBook.styled.ts (2)
EmptyTitle(209-214)EmptySubText(216-220)
🔇 Additional comments (2)
src/components/group/MyGroupModal.tsx (2)
142-143: exhaustive-deps 비활성화 사유 확인 필요eslint-disable 주석이 설명 없이 사용되고 있습니다.
loadMore함수가selected상태에 의존하지만 dependency 배열에 포함되지 않았습니다.selected변경 시 line 81의 effect가 데이터를 리셋하므로 의도적인 것으로 보이나, 향후 유지보수를 위해 주석으로 이유를 명시하는 것이 좋습니다.다음과 같이 설명 주석을 추가하는 것을 권장합니다:
tryFill(); - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps + // selected가 변경되면 line 81의 effect에서 데이터를 리셋하므로 dependency에 포함하지 않음 }, [rooms, nextCursor, isLast]);
24-24: '완료' 탭 추가 구현 확인 완료'완료' 상태가 타입, 탭 배열, GroupCard props에 일관되게 추가되어 PR 목표를 정확히 달성했습니다.
Also applies to: 178-178, 198-198
2 participants
#️⃣연관된 이슈
10월 5주차 QA 사항 - 호준
#279
📝작업 내용
모임 메인 페이지 사용성 개선 2차 작업
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
제거