[Fix] #728 - homeFeature 출석 버튼 탭 시 화면 중첩 네비게이션 버그 해결 #729
Conversation
|
Summary by CodeRabbit릴리스 노트
Walkthrough
Changes
Sequence Diagram(s)sequenceDiagram
participant Provider as ItemProvider
participant Cell as CalendarCardCVC
Note over Provider,Cell #E8F6EF: 셀 생성/등록 흐름 (변경됨)
Provider->>Cell: dequeue/register cell
Provider->>Cell: call cancel existing subscriptions
Cell->>Cell: resetSubscription() -- cancels & reinitializes CancelBag
Provider->>Cell: configureCell(item, userType)
Cell-->>Provider: ready/display
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/Cells/Calendar/CalendarCardCVC.swift(2 hunks)SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/VC/HomeForMemberVC.swift(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/VC/HomeForMemberVC.swift (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/Cells/Calendar/CalendarCardCVC.swift (1)
resetSubscription(147-150)
🔇 Additional comments (2)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/Cells/Calendar/CalendarCardCVC.swift (1)
87-90: 구독 리셋 로직의 중앙화가 잘 되었습니다.
resetSubscription()메서드로 구독 취소 로직을 추출하여 재사용 가능하게 만든 것은 좋은 리팩토링입니다. 이를 통해:
prepareForReuse()에서 재사용- 외부(HomeForMemberVC)에서도 호출 가능
- 코드 중복 제거 및 유지보수성 향상
Also applies to: 147-150
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/VC/HomeForMemberVC.swift (1)
387-391: prepareForReuse()에서 이미 구독이 해제됩니다
CalendarCardCVC의 prepareForReuse()가 cancelBag을 초기화하므로, HomeForMemberVC의 visibleCells 기반 resetCellSubscription() 메서드와 호출부는 중복이거나 불필요합니다. 사용하지 않는다면 해당 메서드와 호출 구문을 제거하세요.Likely an incorrect or invalid review comment.
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/VC/HomeForMemberVC.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/VC/HomeForMemberVC.swift (1)
387-391: 구현이 올바르며 문제를 효과적으로 해결합니다.visibleCells 중
CalendarCardCVC타입만 필터링하여 구독을 정리하는 로직이 정확하게 구현되었습니다.compactMap을 사용한 안전한 타입 캐스팅도 적절합니다.선택적 개선사항:
- 메서드명을
resetCellSubscriptions(복수형) 또는resetVisibleCalendarCellSubscriptions로 변경하면 여러 셀을 처리한다는 의미가 더 명확해집니다.- 메서드에 간단한 주석을 추가하여 CalendarCardCVC만 대상으로 하는 이유를 설명하면 유지보수에 도움이 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/VC/HomeForMemberVC.swift(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/VC/HomeForMemberVC.swift (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/Cells/Calendar/CalendarCardCVC.swift (1)
resetSubscription(147-150)
| stopLatestPostAnimationLoop() | ||
| cancelTasks() | ||
| stopPopularPostsAnimationLoop() | ||
| resetCellSubscription() |
There was a problem hiding this comment.
구독 정리 로직이 올바르게 구현되었습니다.
뷰 컨트롤러가 사라질 때 캘린더 셀의 구독을 정리하는 로직이 적절히 추가되었습니다. 이전 리뷰에서 지적된 메서드명 불일치 문제도 해결되었습니다.
참고: PR 설명에는 viewWillDisappear에서 구독을 reset한다고 명시되어 있으나, 실제 구현은 viewDidDisappear를 사용하고 있습니다. 두 메서드 모두 정리 작업에 적합하지만, 문서와 코드 간 일관성을 위해 PR 설명을 업데이트하는 것을 권장합니다.
🤖 Prompt for AI Agents
In
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/VC/HomeForMemberVC.swift
around line 91, the subscription reset is implemented in viewDidDisappear
(resetCellSubscription()) but the PR description states it runs in
viewWillDisappear; update the PR description to state that subscriptions are
reset in viewDidDisappear to keep docs consistent with code, or if you prefer
the cleanup to happen earlier change the implementation to call
resetCellSubscription() from viewWillDisappear instead and ensure method naming
and lifecycle choice are documented accordingly.
| func resetSubscription() { | ||
| self.cancelBag.cancel() | ||
| self.cancelBag = CancelBag() | ||
| } |
| private func resetCellSubscription() { | ||
| collectionView.visibleCells | ||
| .compactMap { $0 as? CalendarCardCVC } | ||
| .forEach { $0.resetSubscription() } | ||
| } | ||
| } |
There was a problem hiding this comment.
여기보단 HomeForMemberCellProvider의 메소드 내에서 cell.cancelBag.cancel()을 호출하는 것이 로직상 더 자연스러울 것 같아요.
🌱 제안 이유
CalendarCardCVC는 prepareForReuse()에서 구독을 초기화하고 있기 때문에, 스크롤되지 않는 상황에서는 구독이 해제되지 않는 것이 원인으로 보여요.
📌 의문점
여기서 한 가지 궁금했던 부분이 있었는데요. HomeForMemberVC에서 사용하는 다른 셀들(DefaultPostCVC, HomeDefaultHeaderView)도 똑같이 prepareForReuse()에서 구독을 초기화해주고 있는데, 왜 이 셀들은 스크롤하지 않아도 문제가 없었는지 의문이 들었습니다.
코드를 살펴보니 setItemsNeedUpdate()의 동작 차이가 원인이었던 것으로 보여요.
CalendarCVC의 경우
- setItemsNeedUpdate 호출마다 갱신
- 그 때마다
createCalendarCellRegistration()클로저가 다시 실행 - 해당 메소드 내에서 새로운 구독이
cell.cancelBag에 추가 - 스크롤이 없는 경우
prepareForReuse()가 호출되지 않음 - cancelBag이 초기화되지 않은 채로 구독이 쌓이는 문제 발생
HomeDefaultHeaderView와 DefaultPostCVC
- setItemsNeedUpdate 에서 갱신하지 않음
- Provider의 메소드가 재호출되지 않음
- 구독이 그대로 유지
- 스크롤하지 않았을 때 구독 해제 되지 않는 것은 같음
📝 제안 방안
그래서 이 부분보다는 createCalendarCellRegistration() 메소드에서 cell.cancelBag.cancel()을 먼저 해주는 것이 더 적절하다고 생각합니다.
func createCalendarCellRegistration() -> CalendarCellRegistration {
collectionView.createCellRegistration { [weak self] cell, _, item in
guard let self else { return }
// 새 구독 추가 전에 기존 구독 정리
cell.cancelBag.cancel()
// 이후 설정 진행...
}
}또한 VC에서 타입 캐스팅을 통해 특정 셀의 구독을 관리하는 것보다 셀을 설정하는 메소드에서 구독을 초기화하는 것이 흐름상으로도 더 자연스럽다고 생각해요!
저도 미처 발견하지 못했던 부분인데 덕분에 알고 갑니다👍
There was a problem hiding this comment.
우와 !! diffable data source를 사용해본 적이 없어서 cell 관리를 어떻게 해주는지 제대로 파악하지 못한 것 같네요 !!!!
덕분에 공부도 하고 많이 배워갑니다 !!!!!!
This reverts commit 17f6be1.
|
구웃! 수고햇어요 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/CollectionView/Registration/HomeForMemberItemProvider.swift (1)
26-28:resetSubscription()메서드를 사용하여 일관성 개선 필요현재 구현은
cell.cancelBag.cancel()만 호출하지만,CalendarCardCVC의prepareForReuse()메서드(89-90줄)와resetSubscription()메서드(148-151줄)는 취소 후CancelBag을 재초기화합니다. 일관성을 위해 다음과 같이 수정하세요:-cell.cancelBag.cancel() +cell.resetSubscription()참고: 다른 셀들의 구독 취소 패턴
PopularPostCellRegistration,LatestPostCellRegistration,SurveyRegistration도cancelBag을 사용하지만 구독 취소를 하지 않습니다. 이들도 동일한 메모리 누수 위험이 있으나, 이는 별도 PR에서 처리하는 것을 추천합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/CollectionView/Registration/HomeForMemberItemProvider.swift(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/CollectionView/Registration/HomeForMemberItemProvider.swift (1)
SOPT-iOS/Projects/Features/HomeFeature/Sources/HomeScene/Cells/Calendar/CalendarCardCVC.swift (1)
configureCell(136-146)
3 participants
🌴 PR 요약
🌱 작업한 브랜치
🌱 PR Point
문제상황 -> 출석하고 뒤로가기를 했을 때 cell이 다시 그려지지 않아서 구독 취소가 되지 않음 (prepareForeReuse가 호출되는 시점은 cell이 다시 그려질 때라서 !!)
해결 -> viewDidDisappear에서 구독을 reset하는 로직 추가
📌 참고 사항
생략
📸 스크린샷
생략
📮 관련 이슈