Feat/#21 travel follow shop info#23
Conversation
|
아 그리고 이제 이 작업후에 목요일 오후까지는 작업이 좀 어려워요 .. |
Walkthrough가게 상세(PlaceDetail) 피처를 새로 추가하고 Follow 관련 에러를 API별 세분화했으며, FollowServiceProtocol에 장소 상세·사진 조회 메서드를 추가했습니다. FollowDetail에서 예산 UI/로직을 제거하고 PlaceDetail을 RIBs로 통합해 라우팅/뷰/인터랙터를 연결했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant FollowVC as FollowDetailViewController
participant FollowInteractor as FollowDetailInteractor
participant FollowRouter as FollowDetailRouter
participant PlaceDetailBuilder as PlaceDetailBuilder
participant PlaceDetailInteractor as PlaceDetailInteractor
participant FollowService as FollowService
participant PlaceDetailVC as PlaceDetailViewController
User->>FollowVC: 하단 시트 chevron 탭
FollowVC->>FollowInteractor: listener.didTapPlaceDetailChevron(place)
FollowInteractor->>FollowRouter: routeToPlaceDetail(travelPlace, youtuberName)
FollowRouter->>PlaceDetailBuilder: build(withListener, travelPlace, youtuberName)
PlaceDetailBuilder->>PlaceDetailInteractor: init(presenter, followService, travelPlace, youtuberName)
PlaceDetailBuilder->>PlaceDetailVC: viewController 생성 및 반환
FollowRouter->>FollowRouter: attach & present PlaceDetailRouter
PlaceDetailInteractor->>FollowService: fetchPlaceDetail(googlePlaceId)
FollowService-->>PlaceDetailInteractor: Result<PlaceDetail, PlaceDetailError>
PlaceDetailInteractor->>FollowService: fetchPlacePhotos(googlePlaceId)
FollowService-->>PlaceDetailInteractor: Result<[PlacePhoto], PlacePhotosError>
PlaceDetailInteractor->>PlaceDetailVC: updatePlaceDetail(...) / updatePhotos(...)
PlaceDetailVC-->>User: 가게 상세 표시
User->>PlaceDetailVC: 뒤로 버튼 탭
PlaceDetailVC->>PlaceDetailInteractor: didTapBackButton()
PlaceDetailInteractor->>FollowRouter: listener.placeDetailDidTapBack() -> detachPlaceDetail()
FollowRouter->>FollowRouter: dismiss & detach
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 6
🤖 Fix all issues with AI agents
In `@Projects/Domain/Sources/Model/Follow/Error/PlaceDetailError.swift`:
- Around line 12-19: Add a serverError case to the PlaceDetailError enum and
update its mapping logic: modify the PlaceDetailError declaration (enum
PlaceDetailError) to include case serverError(message: String) alongside
missingParameter, notFound, and unknown, then update the error-to-API-code
mapping (the file/function that maps response codes to PlaceDetailError
instances) to map the COMM-08-001 server error to
PlaceDetailError.serverError(message:). Ensure any switch statements or
initializers that pattern-match PlaceDetailError handle the new serverError case
to maintain exhaustive handling.
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/PlaceDetailViewController.swift`:
- Around line 470-486: In PlaceDetailViewController update the handling of
travelPlace.estimatedDuration so a value of 0 is treated as "no duration" and
the durationInfoView is hidden: change the optional binding/branch around
estimatedDuration (used to compute hours/minutes and build durationText) to only
proceed when let duration = travelPlace.estimatedDuration and duration > 0, and
keep durationInfoView.isHidden = true for the else case; ensure the existing
durationText construction and durationInfoView.configure(text:) remain unchanged
inside the positive-duration branch.
- Around line 365-406: segmentChanged currently toggles planBHeaderLabel and
planBCollectionView visibility based only on isInfoSelected, which overwrites
the data-driven state set in updatePlaceDetail; change segmentChanged to also
check whether Plan B data exists (or a dedicated Boolean property you add, e.g.,
hasPlanBData) before setting planBHeaderLabel.isHidden and
planBCollectionView.isHidden so they remain hidden when no Plan B items are
present; reference the existing updatePlaceDetail logic to determine the correct
condition and update segmentChanged (and any new hasPlanBData property)
accordingly.
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PhotoCollectionView.swift`:
- Around line 93-118: calculateTotalHeight() currently sums heights per column
(waterfall) which mismatches the FlowLayout; change it to compute height
row-by-row using calculateCellWidth() and photoItems: compute each item's
cellHeight = cellWidth * (CGFloat(item.height) / CGFloat(item.width)), then
iterate photoItems in pairs (index 0&1, 2&3, ...) and for each row add
max(leftCellHeight, rightCellHeight) plus spacing (handle a single trailing item
by using its height), finally subtract the extra spacing at the end and return
max(totalHeight, 0); update references to spacing, cellWidth, photoItems and
calculateCellWidth() in the calculateTotalHeight() method.
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PlanBCollectionView.swift`:
- Around line 154-160: configure(with:) doesn't set thumbnailImageView and
PlanBInfo has no image; either add an image property (e.g., thumbnailURL or
UIImage) to PlanBInfo and update configure(with:) to load/assign it to
thumbnailImageView (use your image loader/async fetching and set placeholder),
or remove thumbnailImageView from the view/layout if images are not needed;
update references in PlanBCollectionView (configure(with:), thumbnailImageView,
nameLabel) and any layout code to keep UI consistent.
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/Views/TipCollectionView.swift`:
- Around line 111-127: In scrollViewWillEndDragging, clamp the computed page
index so it never goes below 0 or above totalTipsCount - 1: after computing let
index = round(offset / cellWidthWithSpacing) convert to an Int, clamp it (e.g.
max(0, min(Int(index), totalTipsCount - 1))), and then recompute offset using
that clamped index before setting targetContentOffset.pointee and calling
tipDelegate?.tipCollectionView(self, didScrollToPage: clampedIndex); ensure you
reference the same totalTipsCount (or the backing tips.count) and replace uses
of the raw rounded index with the clamped value.
🧹 Nitpick comments (16)
Projects/Features/FollowFeature/Sources/Views/Cells/PlaceCell.swift (1)
95-103: 탭 제스처가containerView에만 적용되어 있습니다.
sequenceView와travelTimeContainerView영역 탭은 무시됩니다. 의도된 동작이라면 괜찮지만, 사용자가 셀 전체를 탭 가능하다고 기대할 수 있으므로 확인 부탁드립니다.Projects/Modules/Networks/Sources/ErrorMapping/Follow/PlacePhotosError+Mapping.swift (1)
13-23:errors파라미터가 사용되지 않고 있습니다.
errors: [ErrorResponse.ErrorDetail]파라미터가 전달만 받고 매핑 로직에서 사용되지 않습니다. 다른 매핑 이니셜라이저들과 시그니처를 통일하기 위한 의도라면 괜찮지만, 필요 시_ errors로 명시하거나, 추후 상세 에러 정보가 필요한 케이스(예:missingParameter)에서 활용을 검토해 보세요.Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PlaceInfoRowView.swift (1)
44-59: 아이콘의 하단 제약 조건이 없습니다.
iconImageView에top제약만 있고bottom제약이 없어서, 텍스트가 한 줄이고 높이가 20pt 미만인 경우 아이콘이 뷰 영역 밖으로 나갈 수 있습니다. 현재bodyMR폰트 사이즈와numberOfLines: 2설정상 실질적 문제가 될 가능성은 낮지만, 안전을 위해 아이콘 하단 제약을 추가하는 것을 고려해 보세요.🛠️ 아이콘 하단 제약 추가 제안
iconImageView.snp.makeConstraints { $0.leading.equalToSuperview() $0.top.equalToSuperview().offset(2) $0.width.height.equalTo(20) + $0.bottom.lessThanOrEqualToSuperview() }Projects/Features/FollowFeature/Sources/Views/CollectionViews/PlaceListCollectionView.swift (1)
84-86: 빈UICollectionViewDelegate확장이 남아 있습니다.
didSelectItemAt을 제거하면서 빈 extension이 되었습니다.UICollectionViewDelegateFlowLayout이UICollectionViewDelegate를 상속하므로, 이 빈 extension을 제거하고 아래UICollectionViewDelegateFlowLayoutextension에 통합하는 것도 고려해 보세요.♻️ 빈 extension 제거 제안
-// MARK: - UICollectionViewDelegate - -extension PlaceListCollectionView: UICollectionViewDelegate { } - // MARK: - UICollectionViewDelegateFlowLayout -extension PlaceListCollectionView: UICollectionViewDelegateFlowLayout { +extension PlaceListCollectionView: UICollectionViewDelegate, UICollectionViewDelegateFlowLayout {Projects/Features/FollowFeature/Sources/Views/PlaceDetailBottomSheetView.swift (1)
228-232: View에서 직접UIApplication.shared.open호출은 관심사 분리 원칙에 위배됩니다.
chevronButtonTapped은 콜백 패턴(onChevronTapped)을 사용하지만,findRouteButtonTapped은 View 내에서 직접 URL을 여는 방식으로 일관성이 없습니다. 길찾기 동작도 콜백으로 위임하면 테스트 용이성과 아키텍처 일관성이 향상됩니다.♻️ 콜백 패턴 적용 제안
// Properties private var googleMapsUri: String? var onChevronTapped: (() -> Void)? +var onFindRouteTapped: ((String) -> Void)? `@objc` private func findRouteButtonTapped() { - guard let urlString = googleMapsUri, - let url = URL(string: urlString) else { return } - UIApplication.shared.open(url) + guard let urlString = googleMapsUri else { return } + onFindRouteTapped?(urlString) }Projects/Modules/Networks/Sources/TargetType/FollowAPI.swift (1)
74-76: GET 요청에Content-Type: application/json헤더 설정이 불필요할 수 있습니다.GET 요청에는 일반적으로 request body가 없으므로
Content-Type헤더가 의미 없습니다. 기존 케이스도 동일하게 설정되어 있으므로 현재 PR 범위 밖이지만, 향후 정리 시 참고하시기 바랍니다.Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PlanBCollectionView.swift (1)
72-76: 셀 높이 매직 넘버70이 두 곳에 중복됩니다.
calculateHeight와sizeForItemAt모두70을 사용하고 있어 유지보수 시 불일치 위험이 있습니다. 상수로 추출하는 것을 권장합니다.♻️ 상수 추출 제안
final class PlanBCollectionView: UICollectionView { + private enum Constants { + static let cellHeight: CGFloat = 70 + static let cellSpacing: CGFloat = 10 + } // ... func calculateHeight(for itemCount: Int) -> CGFloat { - let cellHeight: CGFloat = 70 - let spacing: CGFloat = 10 + let cellHeight = Constants.cellHeight + let spacing = Constants.cellSpacing return CGFloat(itemCount) * cellHeight + CGFloat(max(0, itemCount - 1)) * spacing }Also applies to: 83-89
Projects/Modules/Networks/Sources/ErrorMapping/Follow/ItineraryError+Mapping.swift (1)
13-22:errors파라미터가 사용되지 않고 있습니다.
init(code:message:errors:)에서errors: [ErrorResponse.ErrorDetail]파라미터가 매핑 로직에서 전혀 사용되지 않습니다. 의도적으로 무시하는 것이라면_로 명시하거나, 향후 활용 계획이 없다면 제거를 고려해 주세요.Projects/Features/FollowFeature/Sources/FollowDetailInteractor.swift (1)
113-133: 캐시 히트 시 MainActor 디스패치 누락
loadPlaces(for:)에서 캐시된 데이터가 있을 때(Line 115)presenter.updatePlaces를 직접 호출하지만, 캐시 미스 경로(Line 127-131)에서는MainActor.run으로 감싸고 있습니다. 현재는 UI 스레드에서 호출되어 문제가 없지만, 향후 호출 컨텍스트가 변경되면 메인 스레드 위반이 발생할 수 있습니다. 일관성을 위해 동일한 패턴 적용을 권장합니다.Projects/Features/FollowFeature/Sources/PlaceDetail/PlaceDetailInteractor.swift (3)
74-120: 비정형 Task가 Interactor 비활성화 시 취소되지 않음
fetchPlaceData()에서 생성된Task는 Interactor가 비활성화(willResignActive)되어도 계속 실행됩니다.[weak self]guard로 크래시는 방지되지만, 불필요한 네트워크 요청이 계속 진행될 수 있습니다. Task를 저장하고willResignActive()에서 cancel하는 것이 좋습니다.♻️ 제안하는 수정
// MARK: - State private var placeDetail: PlaceDetail? private var placePhotos: [PlacePhoto] = [] private var currentTipPage: Int = 0 +private var fetchTask: Task<Void, Never>? ... override func willResignActive() { super.willResignActive() + fetchTask?.cancel() + fetchTask = nil } private func fetchPlaceData() { let googlePlaceId = travelPlace.place.googlePlaceId - Task { [weak self] in + fetchTask = Task { [weak self] in guard let self else { return } ... } }
87-115: 에러 처리가API 실패 시 콘솔에 로그만 출력하고 있으며, 사용자에게는 빈 화면만 보이게 됩니다. 최소한 Presenter에 에러 상태를 전달하여 사용자에게 재시도 안내 또는 에러 메시지를 표시하는 것을 권장합니다. 당장 구현이 어렵다면 TODO 코멘트라도 남겨두시면 좋겠습니다.
135-137:currentTipPage는 저장만 되고 읽히지 않음
didScrollToTipPage에서 값을 업데이트하지만, 어디에서도 읽히지 않습니다. 추후 사용 예정이라면 TODO 코멘트를 남기거나, 현재 불필요하다면 제거를 고려해 주세요.Projects/Features/FollowFeature/Sources/PlaceDetail/Views/TipCollectionView.swift (1)
102-109: 셀 너비 계산에 매직 넘버가 반복 사용됨
scrollViewWillEndDragging과sizeForItemAt양쪽에서37,10,27등 동일한 값이 반복됩니다. 상수로 추출하면 한쪽만 수정하고 다른 쪽은 빠뜨리는 실수를 방지할 수 있습니다.Projects/Modules/Networks/Sources/DTO/Follow/PlaceDTO.swift (1)
39-41:photos배열이 non-optional이어서 API 응답에 해당 키가 없으면 디코딩 실패 가능API가 사진이 없는 경우
photos키를 생략하거나null을 반환하면Decodable디코딩이 실패합니다. 같은 DTO 모듈 내 다른 배열 필드들(예:regularOpeningHours,transportation,youtubeTips)은 optional로 정의된 패턴과 일치시키기 위해,photos를[PlacePhotoResponse]?로 변경하거나 기본값을 설정하는 것을 권장합니다.Projects/Features/FollowFeature/Sources/PlaceDetail/PlaceDetailViewController.swift (2)
544-643:updateBottomConstraints의 4개 분기에서 대량의 제약 조건 중복각 분기가 거의 동일한
remakeConstraints호출을 반복하고 있어 유지보수가 어렵고 실수 가능성이 높습니다. 공통 제약 조건을 추출하고 조건부로bottom과height만 변경하는 방식으로 리팩토링을 고려해 주세요.
58-115:segmentedControl과stickySegmentedControl의 동일한 설정 코드 중복두 segmented control의 스타일 설정이 완전히 동일합니다. 팩토리 메서드나 헬퍼 함수로 추출하면 유지보수가 용이합니다.
예시
private static func makeStyledSegmentedControl() -> UISegmentedControl { let control = UISegmentedControl(items: ["정보", "사진"]) control.selectedSegmentIndex = 0 control.setBackgroundImage(UIImage(), for: .normal, barMetrics: .default) control.setBackgroundImage(UIImage(), for: .selected, barMetrics: .default) control.setDividerImage(UIImage(), forLeftSegmentState: .normal, rightSegmentState: .normal, barMetrics: .default) control.setTitleTextAttributes([ .foregroundColor: UIColor(hexCode: "#999999"), .font: UIFont.NDGL.bodyLM.font ], for: .normal) control.setTitleTextAttributes([ .foregroundColor: UIColor(hexCode: "#1E1E1E"), .font: UIFont.NDGL.bodyLSB.font ], for: .selected) return control }
| public enum PlaceDetailError: Error, Sendable { | ||
| /// 필수 요청 파라미터가 존재하지 않음 (COMM-01-006) | ||
| case missingParameter(message: String) | ||
| /// 장소를 찾을 수 없음 (PLACE-02-001) | ||
| case notFound(message: String) | ||
| /// 알 수 없는 에러 | ||
| case unknown(code: String, message: String) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'serverError' --type=swift -C2Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 6052
🏁 Script executed:
fd -e swift | xargs grep -l 'PlacePhotosError' | head -5Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 431
🏁 Script executed:
fd -e swift -path "*/Error/*" | head -20Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 243
🏁 Script executed:
fd -e swift -path "*PlaceDetailError*"Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 243
🏁 Script executed:
cat -n "Projects/Domain/Sources/Model/Follow/Error/PlacePhotosError.swift"Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 764
🏁 Script executed:
fd PlaceDetailErrorRepository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 225
🏁 Script executed:
rg -l PlaceDetailErrorRepository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 431
🏁 Script executed:
cat -n "Projects/Modules/Networks/Sources/ErrorMapping/Follow/PlaceDetailError+Mapping.swift"Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 806
🏁 Script executed:
rg -A 20 -B 5 'PlaceDetailError' "Projects/Modules/Networks/Sources/Service/FollowService.swift" | head -50Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 1231
serverError 케이스 누락 - 추가 필요
PlaceDetailError에는 PlacePhotosError, HomeError, ItineraryError, ContentCardError 등 프로젝트 내 다른 에러 타입들처럼 서버 에러(COMM-08-001) 케이스가 없습니다. 모든 API 엔드포인트에서 발생할 수 있는 표준 서버 에러 코드이므로, 일관성 있는 에러 처리를 위해 serverError(message: String) 케이스를 추가하고 매핑 파일을 함께 수정하세요.
🤖 Prompt for AI Agents
In `@Projects/Domain/Sources/Model/Follow/Error/PlaceDetailError.swift` around
lines 12 - 19, Add a serverError case to the PlaceDetailError enum and update
its mapping logic: modify the PlaceDetailError declaration (enum
PlaceDetailError) to include case serverError(message: String) alongside
missingParameter, notFound, and unknown, then update the error-to-API-code
mapping (the file/function that maps response codes to PlaceDetailError
instances) to map the COMM-08-001 server error to
PlaceDetailError.serverError(message:). Ensure any switch statements or
initializers that pattern-match PlaceDetailError handle the new serverError case
to maintain exhaustive handling.
| @objc private func segmentChanged(_ sender: UISegmentedControl) { | ||
| let isInfoSelected = sender.selectedSegmentIndex == 0 | ||
|
|
||
| infoContainerView.isHidden = !isInfoSelected | ||
| separatorView.isHidden = !isInfoSelected | ||
| tipCollectionView.isHidden = !isInfoSelected | ||
| tipPageControl.isHidden = !isInfoSelected | ||
| planBHeaderLabel.isHidden = !isInfoSelected | ||
| planBCollectionView.isHidden = !isInfoSelected | ||
|
|
||
| photoCollectionView.isHidden = isInfoSelected | ||
|
|
||
| if isInfoSelected { | ||
| photoCollectionView.snp.remakeConstraints { | ||
| $0.top.equalTo(underlineView.snp.bottom).offset(16) | ||
| $0.leading.trailing.equalToSuperview() | ||
| $0.height.equalTo(photoCollectionView.calculateTotalHeight()) | ||
| } | ||
| } else { | ||
| // Photo tab: add bottom constraint to photoCollectionView | ||
| photoCollectionView.snp.remakeConstraints { | ||
| $0.top.equalTo(underlineView.snp.bottom).offset(16) | ||
| $0.leading.trailing.equalToSuperview() | ||
| $0.height.equalTo(photoCollectionView.calculateTotalHeight()) | ||
| $0.bottom.equalToSuperview().offset(-40) | ||
| } | ||
| } | ||
|
|
||
| segmentedControl.selectedSegmentIndex = sender.selectedSegmentIndex | ||
| stickySegmentedControl.selectedSegmentIndex = sender.selectedSegmentIndex | ||
|
|
||
| UIView.animate(withDuration: 0.2) { | ||
| let offset = isInfoSelected ? 0 : self.view.frame.width / 2 | ||
| self.underlineView.snp.updateConstraints { | ||
| $0.leading.equalToSuperview().offset(offset) | ||
| } | ||
| self.stickyUnderlineView.snp.updateConstraints { | ||
| $0.leading.equalToSuperview().offset(offset) | ||
| } | ||
| self.view.layoutIfNeeded() | ||
| } | ||
| } |
There was a problem hiding this comment.
planBHeaderLabel과 planBCollectionView가 Plan B 데이터 없이도 표시될 수 있는 버그
segmentChanged에서 planBHeaderLabel.isHidden과 planBCollectionView.isHidden을 탭 선택 여부만으로 설정하고 있습니다. 정보 탭 선택 시 (isInfoSelected == true) Plan B 데이터가 비어있어도 두 뷰가 무조건 isHidden = false로 설정됩니다.
updatePlaceDetail에서 비어있을 때 isHidden = true로 설정하지만, 탭 전환 시 이 상태가 덮어씌워집니다.
🐛 Plan B 데이터 존재 여부를 함께 확인하도록 수정 제안
`@objc` private func segmentChanged(_ sender: UISegmentedControl) {
let isInfoSelected = sender.selectedSegmentIndex == 0
infoContainerView.isHidden = !isInfoSelected
separatorView.isHidden = !isInfoSelected
tipCollectionView.isHidden = !isInfoSelected
tipPageControl.isHidden = !isInfoSelected
- planBHeaderLabel.isHidden = !isInfoSelected
- planBCollectionView.isHidden = !isInfoSelected
+ planBHeaderLabel.isHidden = !isInfoSelected || planBCollectionView.isHidden
+ planBCollectionView.isHidden = !isInfoSelected || planBCollectionView.isHidden또는 현재 Plan B 데이터 존재 여부를 별도 프로퍼티로 관리하는 것이 더 명확합니다.
🤖 Prompt for AI Agents
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/PlaceDetailViewController.swift`
around lines 365 - 406, segmentChanged currently toggles planBHeaderLabel and
planBCollectionView visibility based only on isInfoSelected, which overwrites
the data-driven state set in updatePlaceDetail; change segmentChanged to also
check whether Plan B data exists (or a dedicated Boolean property you add, e.g.,
hasPlanBData) before setting planBHeaderLabel.isHidden and
planBCollectionView.isHidden so they remain hidden when no Plan B items are
present; reference the existing updatePlaceDetail logic to determine the correct
condition and update segmentChanged (and any new hasPlanBData property)
accordingly.
| // Duration | ||
| if let duration = travelPlace.estimatedDuration { | ||
| let hours = duration / 60 | ||
| let minutes = duration % 60 | ||
| let durationText: String | ||
| if hours > 0 && minutes > 0 { | ||
| durationText = "\(hours)시간 \(minutes)분 체류 예상" | ||
| } else if hours > 0 { | ||
| durationText = "\(hours)시간 체류 예상" | ||
| } else { | ||
| durationText = "\(minutes)분 체류 예상" | ||
| } | ||
| durationInfoView.configure(text: durationText) | ||
| durationInfoView.isHidden = false | ||
| } else { | ||
| durationInfoView.isHidden = true | ||
| } |
There was a problem hiding this comment.
estimatedDuration이 0일 때 "0분 체류 예상"이 표시됩니다
duration이 0이면 hours와 minutes 모두 0이 되어 else 분기에서 "0분 체류 예상"이 표시됩니다. 0분 체류는 의미 있는 정보가 아니므로 숨기는 것이 적절합니다.
제안
- if let duration = travelPlace.estimatedDuration {
+ if let duration = travelPlace.estimatedDuration, duration > 0 {🤖 Prompt for AI Agents
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/PlaceDetailViewController.swift`
around lines 470 - 486, In PlaceDetailViewController update the handling of
travelPlace.estimatedDuration so a value of 0 is treated as "no duration" and
the durationInfoView is hidden: change the optional binding/branch around
estimatedDuration (used to compute hours/minutes and build durationText) to only
proceed when let duration = travelPlace.estimatedDuration and duration > 0, and
keep durationInfoView.isHidden = true for the else case; ensure the existing
durationText construction and durationInfoView.configure(text:) remain unchanged
inside the positive-duration branch.
| func calculateTotalHeight() -> CGFloat { | ||
| guard !photoItems.isEmpty else { return 0 } | ||
|
|
||
| let cellWidth = calculateCellWidth() | ||
| guard cellWidth > 0 else { return 0 } | ||
|
|
||
| var leftColumnHeight: CGFloat = 0 | ||
| var rightColumnHeight: CGFloat = 0 | ||
| let spacing: CGFloat = 8 | ||
|
|
||
| for (index, item) in photoItems.enumerated() { | ||
| // 0으로 나누기 방지 | ||
| guard item.width > 0 else { continue } | ||
| let aspectRatio = CGFloat(item.height) / CGFloat(item.width) | ||
| let cellHeight = cellWidth * aspectRatio | ||
|
|
||
| if index % 2 == 0 { | ||
| leftColumnHeight += cellHeight + spacing | ||
| } else { | ||
| rightColumnHeight += cellHeight + spacing | ||
| } | ||
| } | ||
|
|
||
| let totalHeight = max(leftColumnHeight, rightColumnHeight) - spacing | ||
| return max(totalHeight, 0) | ||
| } |
There was a problem hiding this comment.
calculateTotalHeight()의 계산 방식이 UICollectionViewFlowLayout의 실제 레이아웃과 불일치
calculateTotalHeight()는 좌/우 열에 각 아이템 높이를 누적하는 워터폴(masonry) 방식으로 계산하지만, UICollectionViewFlowLayout은 행(row) 기반으로 배치합니다. 즉, 같은 행의 두 아이템 중 더 높은 쪽에 맞춰 행 높이가 결정됩니다.
예시:
- 아이템 0: 높이 100, 아이템 1: 높이 200
- 아이템 2: 높이 200, 아이템 3: 높이 100
- 실제 FlowLayout: row0 = 200, row1 = 200 → 총 408
- calculateTotalHeight: left = 300, right = 300 → 총 292
이 차이로 인해 컬렉션 뷰의 높이 제약이 실제 콘텐츠보다 작게 설정되어 아이템이 잘릴 수 있습니다.
해결 방법:
- 행 단위로
max(leftHeight, rightHeight)를 합산하는 방식으로 수정하거나, - 워터폴 레이아웃이 의도된 것이라면
UICollectionViewFlowLayout대신 커스텀UICollectionViewLayout을 사용하세요.
♻️ 행 기반 계산으로 수정 (FlowLayout 유지 시)
func calculateTotalHeight() -> CGFloat {
guard !photoItems.isEmpty else { return 0 }
let cellWidth = calculateCellWidth()
guard cellWidth > 0 else { return 0 }
- var leftColumnHeight: CGFloat = 0
- var rightColumnHeight: CGFloat = 0
let spacing: CGFloat = 8
+ var totalHeight: CGFloat = 0
+ var index = 0
- for (index, item) in photoItems.enumerated() {
- // 0으로 나누기 방지
- guard item.width > 0 else { continue }
- let aspectRatio = CGFloat(item.height) / CGFloat(item.width)
- let cellHeight = cellWidth * aspectRatio
-
- if index % 2 == 0 {
- leftColumnHeight += cellHeight + spacing
- } else {
- rightColumnHeight += cellHeight + spacing
+ while index < photoItems.count {
+ let leftItem = photoItems[index]
+ let leftHeight: CGFloat = leftItem.width > 0
+ ? cellWidth * CGFloat(leftItem.height) / CGFloat(leftItem.width)
+ : cellWidth
+
+ var rowHeight = leftHeight
+ if index + 1 < photoItems.count {
+ let rightItem = photoItems[index + 1]
+ let rightHeight: CGFloat = rightItem.width > 0
+ ? cellWidth * CGFloat(rightItem.height) / CGFloat(rightItem.width)
+ : cellWidth
+ rowHeight = max(leftHeight, rightHeight)
}
+
+ totalHeight += rowHeight + spacing
+ index += 2
}
- let totalHeight = max(leftColumnHeight, rightColumnHeight) - spacing
- return max(totalHeight, 0)
+ return max(totalHeight - spacing, 0)
}🤖 Prompt for AI Agents
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PhotoCollectionView.swift`
around lines 93 - 118, calculateTotalHeight() currently sums heights per column
(waterfall) which mismatches the FlowLayout; change it to compute height
row-by-row using calculateCellWidth() and photoItems: compute each item's
cellHeight = cellWidth * (CGFloat(item.height) / CGFloat(item.width)), then
iterate photoItems in pairs (index 0&1, 2&3, ...) and for each row add
max(leftCellHeight, rightCellHeight) plus spacing (handle a single trailing item
by using its height), finally subtract the extra spacing at the end and return
max(totalHeight, 0); update references to spacing, cellWidth, photoItems and
calculateCellWidth() in the calculateTotalHeight() method.
| func configure(with planB: PlanBInfo) { | ||
| let attributedText = NSAttributedString( | ||
| string: planB.name, | ||
| attributes: UIFont.NDGL.bodyMM.attributes | ||
| ) | ||
| nameLabel.attributedText = attributedText | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find PlanBInfo struct definition
fd -t f "\.swift$" | xargs rg -l "struct PlanBInfo|class PlanBInfo"Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Search for PlanBInfo definition with context
rg "struct PlanBInfo|class PlanBInfo" -A 20Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 1676
🏁 Script executed:
#!/bin/bash
# Check the complete PlanBCollectionView file
cat -n "Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PlanBCollectionView.swift"Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 5767
configure(with:) 메서드에서 thumbnailImageView를 구성하지 않습니다.
thumbnailImageView가 레이아웃에 포함되어 있고 스타일링되어 있지만, configure 메서드에서 이미지를 설정하지 않습니다. 더 근본적으로, PlanBInfo 모델에는 이미지 데이터가 전혀 없습니다. PlanBInfo에 이미지 속성을 추가할 계획이 있다면 그에 맞춰 구성해야 하고, 그렇지 않다면 레이아웃에서 thumbnailImageView를 제거하는 것을 고려하세요.
🤖 Prompt for AI Agents
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PlanBCollectionView.swift`
around lines 154 - 160, configure(with:) doesn't set thumbnailImageView and
PlanBInfo has no image; either add an image property (e.g., thumbnailURL or
UIImage) to PlanBInfo and update configure(with:) to load/assign it to
thumbnailImageView (use your image loader/async fetching and set placeholder),
or remove thumbnailImageView from the view/layout if images are not needed;
update references in PlanBCollectionView (configure(with:), thumbnailImageView,
nameLabel) and any layout code to keep UI consistent.
| func scrollViewWillEndDragging( | ||
| _ scrollView: UIScrollView, | ||
| withVelocity velocity: CGPoint, | ||
| targetContentOffset: UnsafeMutablePointer<CGPoint> | ||
| ) { | ||
| let cellWidth = scrollView.frame.width - 37 - 10 - 27 | ||
| let spacing: CGFloat = 10 | ||
| let cellWidthWithSpacing = cellWidth + spacing | ||
|
|
||
| var offset = targetContentOffset.pointee.x + 37 | ||
| let index = round(offset / cellWidthWithSpacing) | ||
| offset = index * cellWidthWithSpacing - 37 | ||
|
|
||
| targetContentOffset.pointee = CGPoint(x: offset, y: 0) | ||
|
|
||
| tipDelegate?.tipCollectionView(self, didScrollToPage: Int(index)) | ||
| } |
There was a problem hiding this comment.
스냅 페이징에서 페이지 인덱스가 유효 범위로 제한되지 않음
round(offset / cellWidthWithSpacing)의 결과가 음수이거나 totalTipsCount를 초과할 수 있습니다 (사용자가 바운스 영역까지 스크롤하는 경우). 유효 범위로 클램핑하는 것이 안전합니다.
♻️ 페이지 인덱스 클램핑 제안
func scrollViewWillEndDragging(
_ scrollView: UIScrollView,
withVelocity velocity: CGPoint,
targetContentOffset: UnsafeMutablePointer<CGPoint>
) {
let cellWidth = scrollView.frame.width - 37 - 10 - 27
let spacing: CGFloat = 10
let cellWidthWithSpacing = cellWidth + spacing
var offset = targetContentOffset.pointee.x + 37
- let index = round(offset / cellWidthWithSpacing)
+ let maxIndex = max(totalTipsCount - 1, 0)
+ let index = min(max(round(offset / cellWidthWithSpacing), 0), CGFloat(maxIndex))
offset = index * cellWidthWithSpacing - 37
targetContentOffset.pointee = CGPoint(x: offset, y: 0)
tipDelegate?.tipCollectionView(self, didScrollToPage: Int(index))
}📝 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.
| func scrollViewWillEndDragging( | |
| _ scrollView: UIScrollView, | |
| withVelocity velocity: CGPoint, | |
| targetContentOffset: UnsafeMutablePointer<CGPoint> | |
| ) { | |
| let cellWidth = scrollView.frame.width - 37 - 10 - 27 | |
| let spacing: CGFloat = 10 | |
| let cellWidthWithSpacing = cellWidth + spacing | |
| var offset = targetContentOffset.pointee.x + 37 | |
| let index = round(offset / cellWidthWithSpacing) | |
| offset = index * cellWidthWithSpacing - 37 | |
| targetContentOffset.pointee = CGPoint(x: offset, y: 0) | |
| tipDelegate?.tipCollectionView(self, didScrollToPage: Int(index)) | |
| } | |
| func scrollViewWillEndDragging( | |
| _ scrollView: UIScrollView, | |
| withVelocity velocity: CGPoint, | |
| targetContentOffset: UnsafeMutablePointer<CGPoint> | |
| ) { | |
| let cellWidth = scrollView.frame.width - 37 - 10 - 27 | |
| let spacing: CGFloat = 10 | |
| let cellWidthWithSpacing = cellWidth + spacing | |
| var offset = targetContentOffset.pointee.x + 37 | |
| let maxIndex = max(totalTipsCount - 1, 0) | |
| let index = min(max(round(offset / cellWidthWithSpacing), 0), CGFloat(maxIndex)) | |
| offset = index * cellWidthWithSpacing - 37 | |
| targetContentOffset.pointee = CGPoint(x: offset, y: 0) | |
| tipDelegate?.tipCollectionView(self, didScrollToPage: Int(index)) | |
| } |
🤖 Prompt for AI Agents
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/Views/TipCollectionView.swift`
around lines 111 - 127, In scrollViewWillEndDragging, clamp the computed page
index so it never goes below 0 or above totalTipsCount - 1: after computing let
index = round(offset / cellWidthWithSpacing) convert to an Int, clamp it (e.g.
max(0, min(Int(index), totalTipsCount - 1))), and then recompute offset using
that clamped index before setting targetContentOffset.pointee and calling
tipDelegate?.tipCollectionView(self, didScrollToPage: clampedIndex); ensure you
reference the same totalTipsCount (or the backing tips.count) and replace uses
of the raw rounded index with the clamped value.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Projects/Features/HomeFeature/Sources/HomeInteractor.swift (1)
128-142: 🛠️ Refactor suggestion | 🟠 Major하드코딩된 테스트 ID를 실제 모델의 ID로 교체해야 합니다.
두 메서드 모두
PopularTrip과Recommendation모델에서 사용 가능한id프로퍼티를 무시하고 하드코딩된2를 사용 중입니다. 두 모델 모두 공개된id: Int프로퍼티를 가지고 있으므로, 이를 직접 사용할 수 있습니다.♻️ 수정 제안
func didSelectPopularTrip(at index: Int, in section: Int) { guard section < categories.count else { return } let category = categories[section] guard let trips = tripsByCategory[category], index < trips.count else { return } - // TODO: 실제 API 연동 시 trip.id 사용 - // 현재는 테스트를 위해 항상 id 2로 이동 - router?.routeToFollowDetail(with: 2) + router?.routeToFollowDetail(with: trips[index].id) } func didSelectRecommendation(at index: Int) { guard index < recommendations.count else { return } - // TODO: 실제 API 연동 시 recommendation.id 사용 - // 현재는 테스트를 위해 항상 id 2로 이동 - router?.routeToFollowDetail(with: 2) + router?.routeToFollowDetail(with: recommendations[index].id) }Projects/Features/FollowFeature/Sources/FollowDetailInteractor.swift (1)
93-98:⚠️ Potential issue | 🟡 MinorAPI 실패 시 사용자에게 피드백이 없습니다.
loadTravelDetail에서detailResult실패 시 로딩만 숨기고 에러를 무시합니다(Line 93-98).loadPlaces에서도(try? result.get()) ?? []로 실패를 빈 배열로 대체합니다(Line 125). 사용자에게 에러 상태(토스트, 재시도 버튼 등)를 표시하는 것을 고려해 주세요.Also applies to: 124-125
🤖 Fix all issues with AI agents
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/PlaceDetailInteractor.swift`:
- Around line 65-72: The Task started in didBecomeActive by fetchPlaceData()
isn't being cancelled when the interactor deactivates; create a stored property
(e.g., fetchTask: Task<Void, Never>?) on PlaceDetailInteractor, assign the Task
returned from fetchPlaceData() to that property inside didBecomeActive (or
inside fetchPlaceData()), and cancel and nil-out fetchTask in willResignActive
(call fetchTask?.cancel(); fetchTask = nil). Also ensure fetchPlaceData's Task
clears the stored fetchTask when it completes to avoid dangling references.
🧹 Nitpick comments (16)
Projects/Features/FollowFeature/Sources/Views/CollectionViews/PlaceListCollectionView.swift (1)
84-86: 빈UICollectionViewDelegateconformance는 제거 가능합니다.
UICollectionViewDelegateFlowLayout이UICollectionViewDelegate를 상속하므로, Line 90-94의UICollectionViewDelegateFlowLayout확장만으로 delegate 준수가 충족됩니다. 빈 extension은 불필요합니다.♻️ 제안
-// MARK: - UICollectionViewDelegate - -extension PlaceListCollectionView: UICollectionViewDelegate { } - // MARK: - UICollectionViewDelegateFlowLayoutProjects/Domain/Sources/Model/Follow/Error/ItineraryError.swift (1)
12-19:ContentCardError와 구조 및 에러 코드가 동일합니다.
ItineraryError와ContentCardError가 동일한 케이스(notFound—TRAVEL-02-001,serverError—COMM-08-001,unknown)를 가지고 있습니다. API별 타입 안전성을 위해 의도적으로 분리한 것이라면 괜찮지만, 향후 에러 케이스가 변경될 때 두 곳을 동시에 수정해야 하는 유지보수 부담이 생길 수 있습니다.공통 케이스를 프로토콜이나 제네릭 베이스 타입으로 추출하는 것도 고려해 볼 수 있습니다.
Projects/Modules/Networks/Sources/ErrorMapping/Follow/PlacePhotosError+Mapping.swift (1)
12-25:errors파라미터가 모든 매핑 이니셜라이저에서 사용되지 않고 있습니다.
PlacePhotosError+Mapping,PlaceDetailError+Mapping,ItineraryError+Mapping모두errors: [ErrorResponse.ErrorDetail]파라미터를 받지만 실제로 사용하지 않습니다. 공통 프로토콜/인터페이스 요구사항이라면 괜찮지만, 그렇지 않다면_로 표시하거나 제거하는 것을 고려해 주세요.Projects/Modules/Networks/Sources/ErrorMapping/Follow/ContentCardError+Mapping.swift (1)
12-23:errors파라미터가 사용되지 않음
errors: [ErrorResponse.ErrorDetail]파라미터가 전달만 되고 내부에서 전혀 사용되지 않습니다. 다른 에러 매핑 이니셜라이저와 시그니처 일관성을 위한 것이라면_ errors로 명시하거나, 향후 활용 계획이 없다면 제거를 고려해 주세요.Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PlaceInfoRowView.swift (1)
23-27:numberOfLines = 2제한 확인텍스트가 2줄을 초과하면 잘려서 표시됩니다. 디자인 스펙에 맞는 의도적 제한이라면 괜찮지만, 긴 영업시간 등 가변 길이 텍스트가 들어올 수 있다면
0(무제한)으로 변경하거나lineBreakMode를.byTruncatingTail로 명시하는 것을 고려해 주세요.Projects/Features/FollowFeature/Sources/Views/PlaceDetailBottomSheetView.swift (1)
228-232: View 레이어에서UIApplication.shared.open직접 호출RIBs 아키텍처에서는 URL 열기와 같은 사이드 이펙트를 View가 아닌 Interactor/Router를 통해 처리하는 것이 일반적입니다. 현재 구현은 동작하지만, 테스트 가능성과 아키텍처 일관성을 위해 콜백 패턴(
onChevronTapped처럼)으로 위임하는 것을 권장합니다.Projects/Features/FollowFeature/Sources/PlaceDetail/Views/TipCollectionView.swift (2)
38-61: 매직 넘버를 상수로 추출하세요
37,10,27등의 값이contentInset,sizeForItemAt,scrollViewWillEndDragging에서 반복 사용되고 있습니다. 값 불일치 위험을 줄이기 위해 상수로 추출하는 것을 권장합니다.♻️ 상수 추출 예시
final class TipCollectionView: UICollectionView { + private enum Layout { + static let leadingInset: CGFloat = 37 + static let trailingInset: CGFloat = 27 + static let itemSpacing: CGFloat = 10 + }Also applies to: 100-128
230-253:PaddingLabel을 별도 파일로 분리하는 것을 고려하세요
PaddingLabel은 범용 유틸리티 컴포넌트입니다. 다른 곳에서도 재사용될 가능성이 높으므로 DSKit 또는 공통 뷰 모듈로 이동하면 좋겠습니다.Projects/Modules/Networks/Sources/Service/FollowService.swift (1)
20-82: 네 가지 메서드의 반복 패턴을 제네릭 헬퍼로 추출하는 것을 고려해보세요.
fetchTravelDetail,fetchPlaces,fetchPlaceDetail,fetchPlacePhotos모두 동일한request → switch success/failure/networkFailure구조를 따르고 있습니다.toDomain()호출과.unknown(code: "NETWORK", ...)매핑이 반복됩니다.♻️ 제네릭 헬퍼 예시
private func performRequest<Response: DomainConvertible, DomainError>( _ target: FollowAPI, errorMapper: `@escaping` (String, String) -> DomainError, networkErrorMapper: `@escaping` (String) -> DomainError ) async -> Result<Response.DomainType, DomainError> { let result: NetworkResult<Response, DomainError> = await provider.request(target, errorMapper: errorMapper) switch result { case .success(let response): return .success(response.toDomain()) case .failure(let error): return .failure(error) case .networkFailure(let error): return .failure(networkErrorMapper(error.message)) } }Projects/Features/FollowFeature/Sources/FollowDetailViewController.swift (1)
245-268:present(bottomSheet, animated: false)— 의도된 동작인지 확인하세요.Line 267에서 바텀시트를
animated: false로 표시하고 있으나, dismiss 시에는animated: true를 사용합니다. 바텀시트 등장 시 애니메이션 없이 나타나는 것이 디자인 의도와 일치하는지 확인해 주세요.Projects/Features/FollowFeature/Sources/FollowDetailInteractor.swift (1)
85-133: 비구조적Task가 인터랙터 생명주기에 바인딩되지 않습니다.
loadTravelDetail()과loadPlaces(for:)에서 생성된Task가 저장되지 않아,willResignActive()시점에 취소할 수 없습니다. 인터랙터가 비활성화된 후에도MainActor.run블록이 실행되어 presenter를 업데이트할 수 있습니다.♻️ Task 취소 지원 예시
+ private var activeTasks: [Task<Void, Never>] = [] + override func willResignActive() { super.willResignActive() + activeTasks.forEach { $0.cancel() } + activeTasks.removeAll() } private func loadTravelDetail() { - Task { + let task = Task { ... } + activeTasks.append(task) }Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PhotoCollectionView.swift (1)
120-127:calculateCellWidth()가 레이아웃 전에 호출될 경우 부정확한 값을 반환합니다.
frame.width가 0일 때 기본값 100을 반환하는데,sizeForItemAt이나calculateTotalHeight()에서 이 값을 기반으로 셀 크기를 계산하면 초기 레이아웃 패스에서 잘못된 크기가 적용될 수 있습니다.PlaceDetailViewController.updatePhotos에서layoutIfNeeded()이후 높이를 재계산하고 있어 최종적으로는 보정되지만,collectionView.bounds.width를 사용하거나 레이아웃이 확정된 시점에만 호출되도록 보장하는 것이 더 안전합니다.Projects/Features/FollowFeature/Sources/PlaceDetail/PlaceDetailInteractor.swift (1)
87-115: API 실패 시 사용자에게 아무런 피드백이 없습니다.현재 에러 처리가
프로덕션 환경에서는
os_log)을 사용하는 것도 권장됩니다.Projects/Features/FollowFeature/Sources/PlaceDetail/PlaceDetailViewController.swift (3)
58-73: Segmented control 설정 코드가stickySegmentedControl과segmentedControl에서 중복됩니다.두 segmented control의 스타일 설정(배경 이미지, 디바이더, 텍스트 속성)이 완전히 동일합니다. 헬퍼 메서드나 팩토리 메서드로 추출하면 유지보수성이 향상됩니다.
♻️ 팩토리 메서드 추출 제안
+ private static func makeSegmentedControl() -> UISegmentedControl { + let control = UISegmentedControl(items: ["정보", "사진"]) + control.selectedSegmentIndex = 0 + control.setBackgroundImage(UIImage(), for: .normal, barMetrics: .default) + control.setBackgroundImage(UIImage(), for: .selected, barMetrics: .default) + control.setDividerImage(UIImage(), forLeftSegmentState: .normal, rightSegmentState: .normal, barMetrics: .default) + control.setTitleTextAttributes([ + .foregroundColor: UIColor(hexCode: "#999999"), + .font: UIFont.NDGL.bodyLM.font + ], for: .normal) + control.setTitleTextAttributes([ + .foregroundColor: UIColor(hexCode: "#1E1E1E"), + .font: UIFont.NDGL.bodyLSB.font + ], for: .selected) + return control + }그런 다음:
private let segmentedControl = makeSegmentedControl() private let stickySegmentedControl = makeSegmentedControl()Also applies to: 100-115
546-643:updateBottomConstraints의 4개 분기가 매우 장황하며 제약 조건 코드가 반복됩니다.
hasTips와hasPlanB조합에 따라 4개 분기를 모두remakeConstraints로 처리하고 있어 유지보수가 어렵고 실수가 발생하기 쉽습니다.separatorView제약 조건은 모든 분기에서 동일하게 재설정되고 있고, tip/planB가 없는 경우 높이를 0으로 설정하는 패턴이 반복됩니다.조건별로 높이와 bottom anchor만 분기하는 방식으로 단순화하면 가독성과 유지보수성이 크게 개선될 수 있습니다.
527-539:updatePhotos에서DispatchQueue.main.async사용이 불필요할 수 있습니다.이 메서드는 이미
MainActor.run블록 내에서 호출되므로 메인 스레드에서 실행됩니다.DispatchQueue.main.async는 다음 run loop까지 레이아웃 업데이트를 지연시키려는 의도로 보이지만,applySnapshot직후layoutIfNeeded()를 호출하면 같은 효과를 얻을 수 있습니다. 현재 방식이 동작에 문제를 일으키지는 않지만, 의도를 명확히 하는 주석이 있으면 좋겠습니다.
| override func didBecomeActive() { | ||
| super.didBecomeActive() | ||
| fetchPlaceData() | ||
| } | ||
|
|
||
| override func willResignActive() { | ||
| super.willResignActive() | ||
| } |
There was a problem hiding this comment.
비활성화 시 진행 중인 Task가 취소되지 않습니다.
didBecomeActive()에서 시작된 Task가 willResignActive()에서 취소되지 않아, 사용자가 빠르게 뒤로 가기했을 때 이미 해제된 화면에 대해 presenter 업데이트가 시도될 수 있습니다.
🛡️ Task 참조를 저장하고 비활성화 시 취소하는 수정 제안
+ private var fetchTask: Task<Void, Never>?
+
override func didBecomeActive() {
super.didBecomeActive()
fetchPlaceData()
}
override func willResignActive() {
super.willResignActive()
+ fetchTask?.cancel()
+ fetchTask = nil
}그리고 fetchPlaceData()에서:
private func fetchPlaceData() {
let googlePlaceId = travelPlace.place.googlePlaceId
- Task { [weak self] in
+ fetchTask = Task { [weak self] in
guard let self else { return }🤖 Prompt for AI Agents
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/PlaceDetailInteractor.swift`
around lines 65 - 72, The Task started in didBecomeActive by fetchPlaceData()
isn't being cancelled when the interactor deactivates; create a stored property
(e.g., fetchTask: Task<Void, Never>?) on PlaceDetailInteractor, assign the Task
returned from fetchPlaceData() to that property inside didBecomeActive (or
inside fetchPlaceData()), and cancel and nil-out fetchTask in willResignActive
(call fetchTask?.cancel(); fetchTask = nil). Also ensure fetchPlaceData's Task
clears the stored fetchTask when it completes to avoid dangling references.
| // Copyright © 2026 NDGL-iOS. All rights reserved. | ||
| // | ||
|
|
||
| import Domain |
There was a problem hiding this comment.
Networks 모듈이 Domain 모듈을 모르는 게 맞지 않나요?
There was a problem hiding this comment.
Data는 Domain을 알고, Network는 Domain을 모르게
public func toDomain() -> TripCategory {
.init(id: self.id, creator: self.name, viedoType: VideoType(rawValue: self.type))
}
이런식으로 해서 toDomain을 Data 모듈에 추가해주셨는데 저도 이렇게 수정해보면 어떨까요?
There was a problem hiding this comment.
넵 /Data/Sources/Transform에서 각 DTO extension을 만들어서 domain으로 래핑해주면 좋을 거 같아요!
There was a problem hiding this comment.
그리고 public일 필요는 없을거 같아요
There was a problem hiding this comment.
따로 논의했던 내용처럼 approve 해주신 뒤 머지하고 수정하는 방향으로 하겠습니다 !
코드를 다르게 구현한 부분이 있네요..!
| imageView.contentMode = .scaleAspectFill | ||
| imageView.clipsToBounds = true | ||
| imageView.layer.cornerRadius = 8 | ||
| imageView.backgroundColor = UIColor(hexCode: "#F5F5F5") |
There was a problem hiding this comment.
| imageView.backgroundColor = UIColor(hexCode: "#F5F5F5") | |
| imageView.backgroundColor = DSKitAsset.Colors.white.color |
DSKit에 등록된 color 적용해주세요
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PhotoCollectionView.swift`:
- Around line 120-127: calculateCellWidth() currently returns a hard-coded 100
when frame.width is 0 which leads to incorrect height from
calculateTotalHeight(); instead, ensure the view has a valid width by using
self.bounds.width (which reflects actual layout) and, if it's zero, call
layoutIfNeeded() on the collection view (or the view itself) to force layout
before measuring, then compute width = (measuredWidth - horizontalPadding -
interItemSpacing) / 2 and return max(width, 1). Update references in
PlaceDetailViewController.updatePhotos that rely on DispatchQueue.main.async +
layoutIfNeeded() by removing the implicit timing dependency (or explicitly call
collectionView.layoutIfNeeded() before calling calculateTotalHeight()) so the
measurement uses the real bounds rather than the temporary default.
🧹 Nitpick comments (4)
Projects/Features/FollowFeature/Sources/PlaceDetail/PlaceDetailViewController.swift (4)
58-115: 세그먼트 컨트롤 설정 코드 중복
stickySegmentedControl(Line 58-73)과segmentedControl(Line 100-115)의 설정 코드가 동일합니다. 팩토리 메서드로 추출하면 유지보수가 편해집니다.♻️ 팩토리 메서드 추출 제안
+ private static func makeSegmentedControl() -> UISegmentedControl { + let control = UISegmentedControl(items: ["정보", "사진"]) + control.selectedSegmentIndex = 0 + control.setBackgroundImage(UIImage(), for: .normal, barMetrics: .default) + control.setBackgroundImage(UIImage(), for: .selected, barMetrics: .default) + control.setDividerImage(UIImage(), forLeftSegmentState: .normal, rightSegmentState: .normal, barMetrics: .default) + control.setTitleTextAttributes([ + .foregroundColor: UIColor(hexCode: "#999999"), + .font: UIFont.NDGL.bodyLM.font + ], for: .normal) + control.setTitleTextAttributes([ + .foregroundColor: UIColor(hexCode: "#1E1E1E"), + .font: UIFont.NDGL.bodyLSB.font + ], for: .selected) + return control + } - private let stickySegmentedControl: UISegmentedControl = { ... }() + private let stickySegmentedControl = makeSegmentedControl() - private let segmentedControl: UISegmentedControl = { ... }() + private let segmentedControl = makeSegmentedControl()
546-643:updateBottomConstraints복잡도가 높고 반복 코드가 많음4개 분기에서 동일한 뷰들의
remakeConstraints를 반복하고 있어 가독성과 유지보수성이 낮습니다. SwiftLint의type_body_length경고(415줄 > 400줄 제한)도 이 메서드의 길이가 주요 원인입니다.
hasTips/hasPlanB조합에 따라 높이와 오프셋만 달라지므로, 조건별 파라미터를 먼저 결정한 뒤 한 번만 constraint를 설정하는 방식으로 단순화할 수 있습니다.♻️ 단순화 예시 (의사코드)
func updateBottomConstraints(hasTips: Bool, hasPlanB: Bool, planBHeight: CGFloat) { let tipHeight: CGFloat = hasTips ? 218 : 0 let tipTopOffset: CGFloat = hasTips ? 20 : 0 let planBTopOffset: CGFloat = hasPlanB ? 32 : 0 let planBItemHeight: CGFloat = hasPlanB ? planBHeight : 0 // separatorView constraints (always same) separatorView.snp.remakeConstraints { ... } // tipCollectionView tipCollectionView.snp.remakeConstraints { $0.top.equalTo(separatorView.snp.bottom).offset(tipTopOffset) $0.leading.trailing.equalToSuperview() $0.height.equalTo(tipHeight) if hasTips && !hasPlanB { $0.bottom.equalToSuperview().offset(-40) } } // tipPageControl, planBHeaderLabel, planBCollectionView 등도 동일 패턴 // ... view.layoutIfNeeded() }
531-538:DispatchQueue.main.async사용이 불필요할 수 있음
updatePhotos는 Interactor에서 메인 스레드로 호출될 가능성이 높습니다.DispatchQueue.main.async는 레이아웃 패스 이후로 높이 계산을 지연시키려는 의도로 보이지만,layoutIfNeeded()를 직접 호출하므로 동기적으로 처리해도 동일한 결과를 얻을 수 있습니다. 만약 레이아웃 지연이 의도라면 주석으로 명시하는 것을 권장합니다.
189-194:viewDidDisappear에서isMovingFromParent사용 시 주의RIBs 아키텍처에서
isMovingFromParent를 통한 뒤로가기 감지는 일반적인 패턴이지만, 프로그래매틱 dismiss 시에도 트리거될 수 있습니다. 현재 내비게이션 기반이라면 문제없으나, 향후 모달 전환 등이 추가될 경우 의도치 않은 동작이 발생할 수 있습니다.
| private func calculateCellWidth() -> CGFloat { | ||
| let totalWidth = frame.width | ||
| guard totalWidth > 0 else { return 100 } // 레이아웃 전 기본값 | ||
| let horizontalPadding: CGFloat = 24 * 2 | ||
| let interItemSpacing: CGFloat = 8 | ||
| let width = (totalWidth - horizontalPadding - interItemSpacing) / 2 | ||
| return max(width, 1) // 음수 방지 | ||
| } |
There was a problem hiding this comment.
frame.width가 0일 때 기본값 100 반환 — 높이 계산 정확도에 영향
레이아웃 전에 calculateTotalHeight()가 호출되면 calculateCellWidth()가 100을 반환하여 부정확한 높이가 계산됩니다. PlaceDetailViewController.updatePhotos에서 DispatchQueue.main.async + layoutIfNeeded()로 타이밍을 맞추고 있지만, 이 의존 관계가 암묵적이라 깨지기 쉽습니다.
🤖 Prompt for AI Agents
In
`@Projects/Features/FollowFeature/Sources/PlaceDetail/Views/PhotoCollectionView.swift`
around lines 120 - 127, calculateCellWidth() currently returns a hard-coded 100
when frame.width is 0 which leads to incorrect height from
calculateTotalHeight(); instead, ensure the view has a valid width by using
self.bounds.width (which reflects actual layout) and, if it's zero, call
layoutIfNeeded() on the collection view (or the view itself) to force layout
before measuring, then compute width = (measuredWidth - horizontalPadding -
interItemSpacing) / 2 and return max(width, 1). Update references in
PlaceDetailViewController.updatePhotos that rely on DispatchQueue.main.async +
layoutIfNeeded() by removing the implicit timing dependency (or explicitly call
collectionView.layoutIfNeeded() before calling calculateTotalHeight()) so the
measurement uses the real bounds rather than the temporary default.
2 participants
🔗 연결된 이슈
📄 작업 내용
💻 주요 코드 설명
PlaceDetailInteractor
Summary by CodeRabbit
새로운 기능
개선 사항