-
Notifications
You must be signed in to change notification settings - Fork 0
[refactor] EmptyView 구조 정리 및 전반적인 사용 방식 개선 #363
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
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughEmptyView를 공통화하도록 리팩토링하고 EmptyDiaryView/DiaryLockView를 제거했으며 FeedList를 FeedListView로 리네임·구조화하고 해당 뷰를 참조하는 뷰컨트롤러들을 업데이트했습니다. 일부 미사용 프리뷰와 공백도 정리했습니다. Changes
Sequence Diagram(s)sequenceDiagram
%%{init: {"themeVariables":{"actorBorder":"#6B7280","actorBackground":"#F8FAFC","noteBorder":"#9CA3AF","noteText":"#111827"}} }%%
participant VC as ViewController
participant FeedListView as FeedListView
participant Table as UITableView
participant Cell as FeedCell
participant Empty as EmptyView
VC->>FeedListView: init / configure()
FeedListView->>Table: register cells, set dataSource/delegate
FeedListView->>Empty: setDefault() / configure(default message/image)
VC->>FeedListView: apply(items:)
FeedListView->>Table: reloadData()
Table->>FeedListView: cellForRow(at:)
FeedListView->>Cell: dequeue & configure(with model)
Cell-->>VC: user action (like/profile/detail/hide/report)
VC-->>FeedListView: handle callback (e.g., onLikeTapped)
FeedListView->>Cell: update UI / close menus
alt no items
FeedListView->>Empty: show
else items present
FeedListView->>Empty: hide
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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: 0
🧹 Nitpick comments (3)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (3)
63-73: 레이아웃 제약 조건 간소화 가능
tableView의 제약 조건이 top, horizontalEdges, bottom을 개별적으로 설정하고 있습니다. 관련 코드 스니펫(FollowListView, NotificationView)의 패턴처럼$0.edges.equalToSuperview()로 간소화할 수 있습니다.🔎 제안된 수정
override func setLayout() { tableView.snp.makeConstraints { - $0.top.equalToSuperview() - $0.horizontalEdges.equalToSuperview() - $0.bottom.equalToSuperview() + $0.edges.equalToSuperview() } noFeedView.snp.makeConstraints { $0.top.equalToSuperview().offset(140) $0.centerX.equalToSuperview() } }
197-201: 중복된reloadData()호출
items프로퍼티의didSet(라인 27-30)에서 이미tableView.reloadData()를 호출하므로, 라인 200의 명시적 호출은 중복입니다.🔎 제안된 수정
func removeDiary(at row: Int) { guard row < items.count else { return } items.remove(at: row) - tableView.reloadData() }
111-122: 두apply메서드 간 empty state 처리 불일치
apply(items:followingState:)는noFeedView의 제약 조건을offset(160)으로 업데이트하지만, 이 메서드는 제약 조건을 업데이트하지 않습니다. 두 메서드가 동일한 뷰에서 사용될 경우, empty state의 위치가 일관되지 않을 수 있습니다.의도된 동작이라면 코멘트로 명시하거나, 일관성을 위해 동일한 처리를 적용하는 것을 권장합니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
HilingualPresentation/Sources/Presentation/Common/Components/EmptyView.swiftHilingualPresentation/Sources/Presentation/Common/Components/Home/CardPreview.swiftHilingualPresentation/Sources/Presentation/Common/Components/Home/CardTopicView.swiftHilingualPresentation/Sources/Presentation/Common/Components/Home/ProfileView.swiftHilingualPresentation/Sources/Presentation/Common/Components/Home/SelectedInfo.swiftHilingualPresentation/Sources/Presentation/Common/Components/HomeView/DiaryLockView.swiftHilingualPresentation/Sources/Presentation/Common/Components/HomeView/EmptyDiaryView.swiftHilingualPresentation/Sources/Presentation/FeedList/FeedListView.swiftHilingualPresentation/Sources/Presentation/FeedList/FeedListViewController.swiftHilingualPresentation/Sources/Presentation/FeedProfile/FeedProfileViewController.swiftHilingualPresentation/Sources/Presentation/FollowList/FollowListView.swiftHilingualPresentation/Sources/Presentation/NotificationHome/NotificationView.swift
💤 Files with no reviewable changes (3)
- HilingualPresentation/Sources/Presentation/FollowList/FollowListView.swift
- HilingualPresentation/Sources/Presentation/Common/Components/HomeView/DiaryLockView.swift
- HilingualPresentation/Sources/Presentation/Common/Components/HomeView/EmptyDiaryView.swift
🧰 Additional context used
🧬 Code graph analysis (2)
HilingualPresentation/Sources/Presentation/Common/Components/EmptyView.swift (2)
HilingualPresentation/Sources/Presentation/FollowList/FollowListView.swift (2)
setUI(51-53)setLayout(55-64)HilingualPresentation/Sources/Presentation/Common/Extensions/UIStackView+.swift (1)
addArrangedSubviews(12-14)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (3)
HilingualPresentation/Sources/Presentation/NotificationHome/NotificationView.swift (2)
setUI(47-50)setLayout(52-58)HilingualPresentation/Sources/Presentation/FollowList/FollowListView.swift (2)
setUI(51-53)setLayout(55-64)HilingualPresentation/Sources/Presentation/Common/Extensions/UIView+.swift (1)
addSubviews(12-14)
🔇 Additional comments (9)
HilingualPresentation/Sources/Presentation/FeedList/FeedListViewController.swift (1)
17-17: LGTM!
FeedList→FeedListView로의 클래스 리네이밍에 맞게 일관되게 업데이트되었습니다.HilingualPresentation/Sources/Presentation/FeedProfile/FeedProfileViewController.swift (1)
29-29: LGTM!
FeedListViewController.swift와 일관되게FeedListView타입으로 업데이트되었습니다.HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (2)
70-73:noFeedView의 offset 값 불일치
setLayout()에서는offset(140)으로 초기화하고,apply(items:followingState:)에서는offset(160)으로 업데이트합니다. 이로 인해 empty state 표시 시 레이아웃이 점프할 수 있습니다.의도된 동작이라면 무시해도 되지만, 그렇지 않다면 offset 값을 통일하는 것이 좋습니다.
Also applies to: 105-107
11-11: 클래스 리네이밍 적절함
FeedList→FeedListView로의 리네이밍은UIView서브클래스에 대한 네이밍 컨벤션을 따르며, PR 목표인EmptyView통합도 잘 반영되어 있습니다.HilingualPresentation/Sources/Presentation/NotificationHome/NotificationView.swift (1)
62-71: LGTM!포맷팅 개선으로 코드 가독성이 향상되었습니다.
configure호출 주변에 공백이 추가되어 로직 블록이 더 명확하게 구분됩니다.HilingualPresentation/Sources/Presentation/Common/Components/Home/SelectedInfo.swift (1)
32-48: 좋은 리팩토링입니다!
EmptyDiaryView와DiaryLockView를 공통EmptyView컴포넌트로 통합하여 코드 중복을 효과적으로 제거했습니다. 클로저 기반 초기화 패턴이 깔끔하고 Swift 관용적입니다.HilingualPresentation/Sources/Presentation/Common/Components/EmptyView.swift (3)
11-36: LGTM!UI 컴포넌트의 시맨틱한 이름 변경(
noFeedLabel→messageLabel,noFeedView→imageView등)이 코드 가독성을 향상시킵니다.FollowListView.swift의setUI()/setLayout()패턴과 일관성 있게 정리되었습니다.
81-94:imageName이 nil일 때 이미지 동작을 검토해주세요.현재 구현에서
imageName이 nil이면 이전 이미지가 유지됩니다. PR 목표에서 "미지정 시 기본 이미지 사용"을 언급했는데, 현재 동작은 "미지정 시 기존 이미지 유지"입니다.의도한 동작이라면 괜찮지만, 기본 이미지로 리셋하려면 아래와 같이 수정할 수 있습니다:
🔎 기본 이미지로 리셋하는 옵션
func configure( message: String, imageName: String? = nil ) { messageLabel.text = message - if let imageName { - imageView.image = UIImage( - named: imageName, - in: .module, - compatibleWith: nil - ) - } + imageView.image = UIImage( + named: imageName ?? "img_diary_empty_ios", + in: .module, + compatibleWith: nil + ) }
72-77: LGTM!
setDefault()메서드가 초기 상태를 명확하게 설정합니다. 초기화 시점에 호출되어 일관된 기본 상태를 보장합니다.
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
🧹 Nitpick comments (3)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (3)
26-28: 전체 테이블 리로드보다 개별 셀 업데이트를 고려하세요.
items배열이 변경될 때마다tableView.reloadData()가 호출되면 스크롤 위치가 손실되고 애니메이션이 끊길 수 있습니다. 특히removeDiary(at:)메서드처럼 단일 항목만 변경되는 경우 비효율적입니다.개별 항목 추가/삭제 시
insertRows/deleteRows를 사용하거나, 전체 교체가 필요한 경우에만reloadData()를 호출하도록 리팩토링을 고려하세요.
166-169: 무한 스크롤 임계값을 상수로 추출하는 것을 고려하세요.하드코딩된
50값을 클래스 상단의 상수로 추출하면 가독성과 유지보수성이 향상됩니다.🔎 선택적 개선안
+ private static let infiniteScrollThreshold: CGFloat = 50 + // ... func scrollViewDidEndDragging( _ scrollView: UIScrollView, willDecelerate decelerate: Bool ) { - let threshold = scrollView.contentSize.height - scrollView.frame.height + 50 + let threshold = scrollView.contentSize.height - scrollView.frame.height + Self.infiniteScrollThreshold if scrollView.contentOffset.y > threshold { onRefresh?() } }
218-222: 애니메이션이 있는 행 삭제를 사용하면 더 나은 UX를 제공할 수 있습니다.
reloadData()를 사용하면 삭제 애니메이션 없이 테이블이 즉시 업데이트됩니다.deleteRows(at:with:)를 사용하면 부드러운 애니메이션과 함께 더 나은 사용자 경험을 제공할 수 있습니다.🔎 개선 제안
func removeDiary(at row: Int) { guard items.indices.contains(row) else { return } items.remove(at: row) - tableView.reloadData() + tableView.deleteRows(at: [IndexPath(row: row, section: 0)], with: .automatic) }참고: 이 변경을 적용하는 경우, Line 27의
didSet에서도tableView.reloadData()호출을 제거하거나 조건부로 처리해야 합니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift
🔇 Additional comments (3)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (3)
78-90: 깔끔한 Public API 설계입니다.서로 다른 사용 사례(팔로잉 상태 기반 vs 커스텀 메시지 기반)를 위한 두 가지
apply메서드 오버로드가 잘 구분되어 있으며,feeds계산 프로퍼티를 통한 읽기 전용 접근도 적절합니다.
96-114: EmptyView 통합이 잘 구현되었습니다.PR 목표에 맞게 공통
EmptyView를 활용하여 빈 상태를 처리하고 있으며,configure(message:)호출 시imageName을 생략하여 기본 이미지가 사용되도록 한 점이 적절합니다. 메시지 선택 로직도 우선순위(customMessage → followingState 기반 → 기본값)가 명확합니다.
193-193: 빈 구현이 의도된 것인지 확인하세요.
didTapMoreButton메서드가 비어있습니다. 이것이 의도된 no-op인지, 아니면 향후 구현이 필요한 부분인지 확인이 필요합니다. 만약 향후 구현이 필요하다면 TODO 주석을 추가하는 것이 좋습니다.
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift
Outdated
Show resolved
Hide resolved
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
HilingualPresentation/Sources/Presentation/FeedList/FeedListViewController.swift (1)
182-195: 사용되지 않는 메서드를 제거해야 합니다.
didTopScrollRefresh메서드는 더 이상 호출되지 않는 것으로 보입니다.UIRefreshControl설정이 제거되었고, 이 메서드를 타겟으로 지정하는 코드가 없습니다. 특히 Line 194의refreshControl?.endRefreshing()호출은refreshControl이 초기화되지 않아 항상 nil입니다.🔎 제거 제안
- @objc private func didTopScrollRefresh() { - if let firstFeed = currentFeeds.first { - AmplitudeManager.shared.logEvent( - "refresh_triggered", - properties: [ - "entry_id": String(firstFeed.diaryID), - "refresh_method": "pull_to_refresh" - ] - ) - } - - self.input.reload.send(()) - feedCellView.tableView.refreshControl?.endRefreshing() - }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swiftHilingualPresentation/Sources/Presentation/FeedList/FeedListViewController.swift
🔇 Additional comments (5)
HilingualPresentation/Sources/Presentation/FeedList/FeedListViewController.swift (1)
17-17: LGTM! 새로운 FeedListView 컴포넌트로의 전환이 적절합니다.타입 변경이 PR의 목표와 일치하며, 코드가 정확합니다.
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (4)
26-28: 자동 리로드 구현이 적절합니다.
didSet에서tableView.reloadData()를 호출하는 방식은 간단하고 명확합니다. 현재 사용 사례에 적합합니다.
68-77: LGTM! 유연한 API 설계입니다.두 개의
apply메서드가 서로 다른 사용 사례(일반 피드 vs 프로필 피드)를 명확하게 지원합니다.
167-175: 하단 스크롤 새로고침 구현을 확인해 주세요.이 구현은 사용자가 목록 하단 근처까지 스크롤할 때 `onRefresh?()를 트리거합니다. PR 목표와 커밋 메시지에서 "상단 끌어당겨서 새로고침(pull-to-refresh)" 제거를 언급했으므로, 이것이 의도된 동작 변경(상단 pull-to-refresh → 하단 infinite scroll)인지 확인이 필요합니다.
- 기존: 상단에서 아래로 당겨서 새로고침
- 현재: 하단 근처 스크롤 시 추가 로드
이 변경이 기획 의도와 일치하는지 확인해 주세요.
11-205: 전반적인 구조가 우수합니다.
FeedListView의 구현이 깔끔하고 책임이 명확하게 분리되어 있습니다:
- 콜백 기반의 이벤트 처리
- 적절한 EmptyView 통합
- 명확한 public API (
apply,closeAllMenus,addTableTapGesture등)코드 품질이 높습니다.
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
🧹 Nitpick comments (1)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (1)
17-17:onRefresh콜백 이름 개선을 고려해보세요.상단 pull-to-refresh가 제거되었지만
onRefresh콜백은 하단 페이지네이션(load more)에 사용되고 있습니다. 기능상 문제는 없으나,onLoadMore로 이름을 변경하면 의도가 더 명확해집니다.Also applies to: 167-175
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift
🧰 Additional context used
🪛 SwiftLint (0.57.0)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift
[Warning] 210-210: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
🔇 Additional comments (3)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (3)
11-12: 클래스명 변경이 적절합니다.
FeedList에서FeedListView로 변경하여 뷰 클래스임을 명확히 나타내고 있습니다. Swift 네이밍 컨벤션에 부합합니다.
26-28:didSet에서 empty state 처리 제거가 적절합니다.
items의didSet에서 테이블 리로드만 수행하고, empty state 업데이트는apply메서드에서updateEmptyView를 통해 처리하도록 관심사를 분리했습니다. 불필요한 중복 체크를 방지하고 호출자가 empty view 업데이트 시점을 제어할 수 있어 더 나은 설계입니다.
106-124: Empty state 처리 통합이 우수합니다.
updateEmptyView를 통해 empty state 로직을 중앙화하여 중복을 제거하고 일관성을 확보했습니다. 커스텀 메시지와 팔로잉 상태에 따른 조건부 메시지 처리가 명확하며, PR 목표와 정확히 일치합니다.
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
♻️ Duplicate comments (1)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (1)
210-214: 삼항 연산자를 if-else로 변경하세요.Void 함수 호출에 삼항 연산자를 사용하면 가독성이 떨어지고 Swift 모범 사례에 어긋납니다. 이전 리뷰에서도 지적된 사항이니 수정 부탁드립니다.
🔎 제안하는 수정
func feedCell(_ cell: FeedCell, didTapMenuItemAt index: Int, isMine: Bool) { - if let row = tableView.indexPath(for: cell)?.row { - isMine ? onHideTapped?(row) : onReportTapped?() + guard let row = tableView.indexPath(for: cell)?.row else { return } + if isMine { + onHideTapped?(row) + } else { + onReportTapped?() } }
🧹 Nitpick comments (1)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (1)
97-101: 테이블뷰 애니메이션을 고려해보세요.현재
reloadData()를 사용하고 있지만,deleteRows(at:with:)를 사용하면 삭제 애니메이션과 함께 더 부드러운 사용자 경험을 제공할 수 있습니다.🔎 제안하는 수정
func removeDiary(at row: Int) { guard items.indices.contains(row) else { return } items.remove(at: row) - tableView.reloadData() + tableView.deleteRows(at: [IndexPath(row: row, section: 0)], with: .automatic) + updateEmptyView(followingState: nil) }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift
🧰 Additional context used
🪛 SwiftLint (0.57.0)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift
[Warning] 212-212: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
🔇 Additional comments (2)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (2)
11-11: 클래스명 변경이 적절합니다.
FeedList를FeedListView로 변경하여 Swift 뷰 네이밍 컨벤션을 따르도록 개선되었습니다. 명확한 네이밍입니다.
108-126: Empty View 로직 통합이 잘 되었습니다.Empty 상태 처리를
updateEmptyView메서드로 일관되게 통합하여 코드 중복을 제거하고 유지보수성을 개선했습니다. 팔로잉 상태와 커스텀 메시지를 모두 지원하는 유연한 구조입니다.
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 (3)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (3)
26-28: 데이터 변경 시 전체 테이블 리로드는 비효율적일 수 있습니다.
items의didSet에서 항상tableView.reloadData()를 호출하면 부분 업데이트(예: 단일 아이템 삭제)에서도 전체 테이블을 다시 그립니다.removeDiary(at:)같은 메서드는 이미 수동으로reloadData()를 호출하므로 중복 리로드가 발생합니다.대안:
didSet을 제거하고 각apply메서드와removeDiary에서 명시적으로 리로드하거나, 더 세밀한 업데이트(deleteRows,insertRows)를 사용하는 것을 권장합니다.🔎 제안하는 수정
-private var items: [FeedModel] = [] { - didSet { tableView.reloadData() } -} +private var items: [FeedModel] = []그리고 각
apply메서드에서 명시적으로 리로드:func apply(items: [FeedModel], followingState haveFollowing: Bool? = nil) { self.items = items + tableView.reloadData() updateEmptyView(followingState: haveFollowing) } func apply(items: [FeedModel], emptyMessage: String?, type: FeedProfileListType) { self.items = items self.type = type + tableView.reloadData() updateEmptyView(customMessage: emptyMessage) }
57-60: 하드코딩된 top offset 값(160)을 상수로 추출하세요.Line 58의
.offset(160)은 매직 넘버입니다. 유지보수성과 가독성을 위해 명명된 상수로 추출하거나, 다양한 화면 크기를 고려한 동적 계산을 권장합니다.🔎 제안하는 수정
+private enum Layout { + static let emptyViewTopOffset: CGFloat = 160 +} + override func setLayout() { tableView.snp.makeConstraints { $0.edges.equalToSuperview() } noFeedView.snp.makeConstraints { - $0.top.equalToSuperview().offset(160) + $0.top.equalToSuperview().offset(Layout.emptyViewTopOffset) $0.centerX.equalToSuperview() } }
169-177: 무한 스크롤 임계값을 상수로 추출하세요.Line 173의
+ 50임계값은 매직 넘버입니다. 유지보수성을 위해 명명된 상수로 추출하는 것을 권장합니다.🔎 제안하는 수정
+private enum ScrollConfig { + static let infiniteScrollThreshold: CGFloat = 50 +} + func scrollViewDidEndDragging( _ scrollView: UIScrollView, willDecelerate decelerate: Bool ) { - let threshold = scrollView.contentSize.height - scrollView.frame.height + 50 + let threshold = scrollView.contentSize.height - scrollView.frame.height + ScrollConfig.infiniteScrollThreshold if scrollView.contentOffset.y > threshold { onRefresh?() } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift
🔇 Additional comments (5)
HilingualPresentation/Sources/Presentation/FeedList/FeedListView.swift (5)
68-101: 공개 API 메서드들이 잘 구현되었습니다.
apply메서드들은 데이터와 empty state를 적절히 업데이트합니다.closeAllMenus()는 visible cells만 순회하여 효율적입니다.removeDiary(at:)는 bounds 체크를 포함하여 안전합니다.addTableTapGesture의cancelsTouchesInView = false설정은 테이블 셀 터치와 제스처가 공존할 수 있도록 합니다.
108-127: Empty state 로직이 명확하고 올바르게 구현되었습니다.메서드는 세 가지 시나리오를 적절히 처리합니다:
- 커스텀 메시지가 있는 경우
- 팔로잉이 없는 경우 전용 메시지
- 기본 피드 empty 메시지
Early return과
bringSubviewToFront호출로 UI 상태가 올바르게 관리됩니다.
137-167: 테이블 뷰 셀 구성이 올바르게 구현되었습니다.
- 안전한 타입 캐스팅과 fallback 처리
- 모든 필요한 데이터가 셀에 전달됨
onLikeToggled클로저에서[weak self]사용으로 메모리 누수 방지- Delegate 패턴과 closure 패턴을 적절히 혼용
184-218: FeedCellDelegate 구현이 올바르고 이전 피드백이 반영되었습니다.
- 모든 delegate 메서드가 안전하게 row를 추출하고 콜백을 호출합니다.
- Lines 211-217의 if-else 구조는 이전 리뷰 피드백을 반영하여 삼항 연산자 대신 명확한 분기 로직을 사용합니다.
- Optional chaining으로 콜백이 설정되지 않은 경우를 안전하게 처리합니다.
64-127: 코드 구조와 조직화가 우수합니다.
- Extension을 활용한 논리적 분리 (공개 API, 비공개 메서드, 프로토콜 준수)
- MARK 주석으로 코드 네비게이션 개선
FeedList→FeedListView리네이밍은 SwiftUI 시대의 네이밍 컨벤션과 일관성 있음- Private extension 활용으로 구현 세부사항 캡슐화
hyeyeonie
left a 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.
수정된 사항 확인완 .ᐟ 고생했어 복복복 ~ 🫳🏻
| private let emptyDiaryView: EmptyView = { | ||
| let view = EmptyView() | ||
| view.configure( | ||
| message: "작성된 일기가 없어요.\n좋은 하루 보내셨기를 바라요!", | ||
| imageName: "img_diary_empty_ios" | ||
| ) | ||
| return view | ||
| }() | ||
|
|
||
| private let diaryLockView: EmptyView = { | ||
| let view = EmptyView() | ||
| view.configure( | ||
| message: "아직 작성 가능한 시간이 아니에요.\n오늘의 일기를 작성해주세요!", | ||
| imageName: "img_diary_lock_ios" | ||
| ) | ||
| return view | ||
| }() |
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.
EmptyView를 재활용하는 방향으로 개선했군요 !
rosejinse
left a 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.
코드도 더 깔끔해지고 컴포넌트 사용하는 것도 이미지가 추가되었는데도 쉽게 정의할 수 있어 좋은 것 같습니둥!! optional 처리까지 최고..🙌
작업하시느라 넘 고생하셧더요 짱
| private let emptyDiaryView: EmptyView = { | ||
| let view = EmptyView() | ||
| view.configure( | ||
| message: "작성된 일기가 없어요.\n좋은 하루 보내셨기를 바라요!", | ||
| imageName: "img_diary_empty_ios" | ||
| ) | ||
| return view | ||
| }() | ||
|
|
||
| private let diaryLockView: EmptyView = { | ||
| let view = EmptyView() | ||
| view.configure( | ||
| message: "아직 작성 가능한 시간이 아니에요.\n오늘의 일기를 작성해주세요!", | ||
| imageName: "img_diary_lock_ios" | ||
| ) | ||
| return view | ||
| }() |
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.
LGTM..👍
| $0.bottom.equalToSuperview() | ||
| } | ||
|
|
||
| tableView.snp.makeConstraints { $0.edges.equalToSuperview() } |
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.
지나쳤을 법한 꼼꼼한 것까지 작업 너무 조아요 !!
| // MARK: - Public API | ||
| // MARK: - Extensions | ||
|
|
||
| extension FeedListView { |
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.
extension 활용의 권위자.
이런 함수들을 따로 뺄 생각을 못 했는데 이렇게 두는 것도 너무 좋은 것 같아요
뺄 때 어떤 기준으로 함수들을 extension으로 빼려고 구분했는지..? 궁금합니다..!!
extension 활용을 하고 싶은데 괜히 작업을 했다가 굳이..? 싶은 부분들이 생길 것 같아서 요렇게 야무지게 잘 빼고 싶네효...🥹
✅ Check List
📌 Related Issue
📎 Work Description
EmptyView컴포넌트를 도입하여 각 뷰에서 개별적으로 정의되어 있던 Empty View 로직을 공통 컴포넌트로 정리했습니다.configure메서드를 통해 문구와 이미지를 모두 변경할 수 있도록 개선했습니다!imageName은 optional로 처리하여, 따로 설정하지 않을 경우 기본 이미지(img_diary_empty_ios)가 사용됩니다.사용 예시
📷 Screenshots
💬 To Reviewers