-
Notifications
You must be signed in to change notification settings - Fork 17
[Feat] #776 - tooltip tap 바인딩 및 1차 QA 반영 #777
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
base: develop
Are you sure you want to change the base?
Conversation
Summary by CodeRabbit릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings. Summary by CodeRabbit릴리즈 노트
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughSoptlog 메뉴 셀에 툴팁 버튼 탭 퍼블리셔와 취소 백(Publish/Cancellation) 패턴을 추가하고, 뷰컨트롤러·뷰모델 쪽에서 셀 탭 타입을 열거형 기반의 SoptlogCellTapInfo로 전환했으며, 문자열 리터럴과 탭바 관련 로그인 알림 로직을 제거/단순화했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Cell as SoptlogMenuCVC
participant VC as SoptlogVC
participant VM as SoptlogViewModel
Note over Cell: 사용자 툴팁 버튼 탭
Cell->>VC: toolTipButtonTapped (Driver<CGRect>)
VC->>VM: toolTipTap.send(CGRect) %% VC가 퍼블리셔 구독을 통해 뷰모델로 전달
Note right of VM: ViewModel이 이벤트 수신 후 처리/네비게이션 트리거
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, 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 |
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/Cell/SoptlogMenuCVC.swift(2 hunks)SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift(1 hunks)
🔇 Additional comments (1)
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/Cell/SoptlogMenuCVC.swift (1)
21-25: No action needed — asDriver() extension is properly implemented.The code correctly uses the
asDriver()extension method. The extension is defined inSOPT-iOS/Projects/Core/Sources/Extension/Combine+/Publisher+Driver.swiftand is accessible throughout the codebase via theCoremodule import. This pattern is widely used across the project and the implementation is sound.
| cell.toolTipButtonTapped | ||
| .subscribe(toolTipTap) | ||
| .store(in: cancelBag) |
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.
메모리 누수: 셀이 재사용될 때마다 구독이 누적됩니다.
configureMenuCell은 cellForItemAt에서 호출되므로, 셀이 재사용될 때마다 새로운 구독이 생성되어 cancelBag에 추가됩니다. 이전 구독은 취소되지 않고 계속 쌓이게 되어 메모리 누수가 발생하고, 동일한 툴팁 탭 이벤트가 여러 번 발생할 수 있습니다.
다음과 같이 수정하여 문제를 해결할 수 있습니다:
방법 1 (권장): 셀에 CancelBag을 추가하고 prepareForReuse에서 정리
SoptlogMenuCVC.swift에 추가:
+import Combine
+
final class SoptlogMenuCVC: UICollectionViewCell {
+ private var cellCancelBag = CancelBag()
+
// MARK: - Properties+ override func prepareForReuse() {
+ super.prepareForReuse()
+ cellCancelBag.cancel()
+ }SoptlogVC.swift에서 수정:
cell.toolTipButtonTapped
.subscribe(toolTipTap)
- .store(in: cancelBag)
+ .store(in: cell.cellCancelBag)방법 2: 셀 내부에서 구독을 관리하고 클로저로 이벤트 전달
SoptlogMenuCVC에서 구독을 관리하고, VC에서는 클로저를 통해 이벤트를 받는 방식으로 변경하는 것도 고려해보세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift
around lines 265-267, the subscription to cell.toolTipButtonTapped is added to
the view controller's cancelBag each time configureMenuCell is called, causing
subscriptions to accumulate on cell reuse; fix by moving the subscription
ownership into the cell: add a CancelBag (or DisposeBag) property to
SoptlogMenuCVC and clear it in the cell's prepareForReuse(), then in
configureMenuCell subscribe and store the subscription in the cell's bag instead
of the VC cancelBag (alternatively manage the subscription inside the cell and
expose a closure/callback for the VC to handle the event).
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
🧹 Nitpick comments (1)
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/Cell/SoptlogMenuCVC.swift (1)
27-27: 잘 구현되었습니다! 이전 리뷰의 메모리 누수 이슈가 해결되었습니다.
cancelBag과prepareForReuse를 추가하여 셀 재사용 시 구독이 누적되는 문제를 정확히 해결했습니다.선택사항: Best practice로 super 호출 추가
관례상
prepareForReuse에서super.prepareForReuse()를 호출하는 것이 권장됩니다.override func prepareForReuse() { + super.prepareForReuse() cancelBag.cancel() }Also applies to: 103-105
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
SOPT-iOS/Projects/Core/Sources/Literals/StringLiterals.swift(1 hunks)SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/Cell/SoptlogMenuCVC.swift(3 hunks)SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift(1 hunks)
🔇 Additional comments (4)
SOPT-iOS/Projects/Core/Sources/Literals/StringLiterals.swift (1)
478-478: LGTM! 문구가 더 간결해졌습니다."돌아가기"로 변경하여 UI 텍스트가 더 간결하고 명확해졌습니다.
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/Cell/SoptlogMenuCVC.swift (2)
10-10: LGTM! Combine 임포트가 적절합니다.리액티브 프로그래밍을 위한 필수 임포트입니다.
21-25: LGTM! 툴팁 버튼 탭 이벤트 퍼블리셔가 잘 구현되었습니다.
withUnretained(self)로 순환 참조 방지- 버튼 bounds를 윈도우 좌표로 변환하여 툴팁 위치 계산에 활용
- Driver로 메인 스레드 전달 보장
구현이 올바릅니다.
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift (1)
265-267: 완벽합니다! 이전 리뷰의 메모리 누수 이슈가 올바르게 해결되었습니다.구독을
cell.cancelBag에 저장하여 셀이 재사용될 때마다 구독이 누적되는 문제가 해결되었습니다.SoptlogMenuCVC.prepareForReuse()에서cancelBag을 취소하므로, 셀이 재사용될 때마다 이전 구독이 정리됩니다.이전 리뷰 코멘트에서 제안한 해결 방법이 정확히 구현되었습니다.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/CollectionView/CompositionalLayout/SoptlogCompositionalLayout.swift(1 hunks)SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/CollectionView/SectionLayoutKind/SoptlogSectionLayoutKind.swift(1 hunks)SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift(5 hunks)SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/ViewModel/SoptlogViewModel.swift(3 hunks)SOPT-iOS/Projects/Features/TabBarFeature/Sources/Controller/TabBarController.swift(1 hunks)SOPT-iOS/Projects/Features/TabBarFeature/Sources/Coordinator/TabBarCoordinator.swift(0 hunks)SOPT-iOS/Projects/Features/TabBarFeature/Sources/ViewModel/TabBarViewModel.swift(1 hunks)
💤 Files with no reviewable changes (1)
- SOPT-iOS/Projects/Features/TabBarFeature/Sources/Coordinator/TabBarCoordinator.swift
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-23T04:48:57.229Z
Learnt from: dlwogus0128
Repo: sopt-makers/SOPT-iOS PR: 699
File: SOPT-iOS/Projects/Features/RootFeature/Sources/ApplicationCoordinator.swift:576-581
Timestamp: 2025-09-23T04:48:57.229Z
Learning: In SOPT-iOS coordinator refactoring: Legacy coordinators require addDependency() calls for proper lifecycle management, but the new coordinator architecture handles dependency management differently and does not require explicit addDependency() calls.
Applied to files:
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift
📚 Learning: 2025-12-07T07:54:51.207Z
Learnt from: yungu0010
Repo: sopt-makers/SOPT-iOS PR: 769
File: SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift:27-27
Timestamp: 2025-12-07T07:54:51.207Z
Learning: In SoptlogVC.swift, the isPokeEmpty field is intentionally hardcoded to true and not bound to the ViewModel because it's a temporary measure until the poke server 500 error is fixed. Once the server is stabilized, this field and the empty view implementation will be removed.
Applied to files:
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swiftSOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/ViewModel/SoptlogViewModel.swift
🧬 Code graph analysis (3)
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/CollectionView/CompositionalLayout/SoptlogCompositionalLayout.swift (1)
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift (1)
sectionType(150-153)
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift (1)
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/CollectionView/SectionLayoutKind/SoptlogSectionLayoutKind.swift (1)
visibleSections(28-39)
SOPT-iOS/Projects/Features/TabBarFeature/Sources/Controller/TabBarController.swift (1)
SOPT-iOS/Projects/Core/Sources/Extension/UIKit+/UIView+.swift (1)
addSubviews(13-15)
🔇 Additional comments (10)
SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/CollectionView/CompositionalLayout/SoptlogCompositionalLayout.swift (1)
16-19: 섹션 타입 결정 로직 개선이 적절합니다.
sectionType(for:)헬퍼를 사용하여 섹션 인덱스를 타입으로 변환하는 방식이 깔끔하고, nil 처리도 안전합니다.SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/CollectionView/SectionLayoutKind/SoptlogSectionLayoutKind.swift (2)
27-39: 활성 사용자 기반 섹션 가시성 로직이 명확합니다.활동/비활동 회원에 따라 섹션을 필터링하는 로직이 직관적이고 이해하기 쉽습니다.
42-45: 타입 안전성 개선이 좋습니다.
IndexPath대신SoptlogCellTapInfo를 사용하여 섹션과 행 정보를 명시적으로 전달하는 것이 타입 안전성을 높입니다.SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/ViewModel/SoptlogViewModel.swift (2)
31-31:SoptlogCellTapInfo타입 전환이 적절합니다.
IndexPath대신 명시적인 섹션 타입을 포함하는 구조체를 사용하여 가독성과 타입 안전성이 개선되었습니다.
87-104: 임시 비활성화 필터가 명확히 문서화되었습니다.서버 500 에러 해결 대기 중인 상태가 주석으로 잘 표시되어 있습니다. 서버 문제 해결 후
&& false제거를 잊지 마세요.SOPT-iOS/Projects/Features/SoptlogFeature/Sources/SoptlogScene/VC/SoptlogVC.swift (4)
30-36: 활성 사용자 기반 섹션 관리가 잘 구현되었습니다.
visibleSections계산 프로퍼티를 통해 활동/비활동 회원에 따라 보여지는 섹션을 동적으로 관리하는 방식이 깔끔합니다.
149-153: 섹션 인덱스 변환 헬퍼가 안전합니다.경계 검사를 통해 잘못된 섹션 인덱스 접근을 방지하고 있습니다.
160-162:IndexPath대신SoptlogCellTapInfo사용이 우수합니다.enum 기반 섹션 타입을 전달하여 뷰모델에서 타입 안전하게 분기 처리할 수 있습니다.
281-283: 툴팁 구독 관리 개선이 올바르게 구현되었습니다.
SoptlogMenuCVC는private(set) var cancelBag = CancelBag()프로퍼티를 가지고 있으며,prepareForReuse()메서드에서cancelBag.cancel()을 호출하여 셀이 재사용될 때 구독을 올바르게 정리합니다. 메모리 누수 문제가 해결되었습니다.SOPT-iOS/Projects/Features/TabBarFeature/Sources/ViewModel/TabBarViewModel.swift (1)
59-60: Verify this change aligns with intended visitor access permissions for the poke featureThe removed code explicitly blocked visitors (
.visitor) from accessing tab index 1 with an alert and reset to home. In the current architecture, visitors receive three tabs: home(0), poke(1), and soptlog(2), meaning index 1 now points to poke rather than the restricted soptamp tab.This change effectively grants poke access to visitors, which was previously blocked. Confirm whether this is an intentional feature change to the visitor permissions model or if the access control logic should be preserved elsewhere.
| // make.bottom.equalToSuperview().inset(58) | ||
| // make.centerX.equalToSuperview() | ||
| // } | ||
| view.addSubviews(dimmedView, menuCollectionView) |
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.
plusButton 제거가 불완전하여 FAB 메뉴 기능이 작동하지 않습니다.
plusButton을 addSubviews에서 제거했지만, 다음 코드들이 여전히 남아있어 문제가 발생합니다:
- Line 31-37:
plusButton프로퍼티 선언 및 설정 - Line 25:
plusButton.publisher를 사용하는isFABTapped - Line 196:
plusButton.addTarget호출 - Line 210-213, 223:
plusButton을 참조하는 애니메이션 로직
plusButton이 뷰 계층에 추가되지 않아 사용자가 탭할 수 없으므로 FAB 메뉴를 열 수 없습니다.
다음 중 하나를 선택하여 수정하세요:
옵션 1: plusButton을 다시 추가 (의도하지 않은 제거인 경우)
- view.addSubviews(dimmedView, menuCollectionView)
+ view.addSubviews(dimmedView, menuCollectionView, plusButton)
+
+ plusButton.snp.makeConstraints { make in
+ // Add appropriate constraints for plusButton
+ }옵션 2: plusButton 관련 코드 전체 제거 (기능 제거가 의도된 경우)
- private lazy var isFABTapped = plusButton.publisher(for: .touchUpInside).mapVoid().asDriver()
+ // Remove isFABTapped or replace with alternative trigger그리고 Lines 31-37, 196, 204-213의 plusButton 관련 코드도 제거해야 합니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
SOPT-iOS/Projects/Features/TabBarFeature/Sources/Controller/TabBarController.swift
near line 101 and referencing lines 25, 31-37, 196, 204-213, 223: the plusButton
was removed from the view hierarchy but its property, publisher usage, target,
and animation logic remain, breaking the FAB menu; either (A) re-add plusButton
to view.addSubviews(dimmedView, menuCollectionView, plusButton) and ensure its
constraints/initial state match the original behavior, or (B) fully remove the
FAB code by deleting the plusButton property and setup at lines 31-37, the
isFABTapped publisher usage at line 25, the addTarget call at line 196, and all
animation/usage references at lines 204-213 and 223 (and any related
publisher/subscriber cleanup); apply the chosen option consistently and run the
app to verify the FAB/menu behavior.
🌴 PR 요약
🌱 작업한 브랜치
🌱 PR Point
🌿 tooltipTap 퍼블리셔 구독 해제
cell의 publisher를VC의 cancelBag으로 관리-> 누수 가능성이 있는 잘못된 코드이므로
CollectionViewCell이 퍼블리셔를 관리하도록 변경 +prepareForReuse에서 구독 해제🌿 솝트로그 섹션 상수 관리 -> enum 관리
활동 상태에 따라 섹션의 개수가 달라지므로 Int로 분기처리를 하면 안됨
해결 방안
IndexPath가 아닌SoptlogCellTapInfo 커스텀 타입전달📌 참고 사항
📸 스크린샷
📮 관련 이슈