[#160] TodoListView에서 정렬 및 필터 옵션을 스티키 헤더로 재구성하고 description에 있는 데로 옵션을 수정한다#165
[#160] TodoListView에서 정렬 및 필터 옵션을 스티키 헤더로 재구성하고 description에 있는 데로 옵션을 수정한다#165
Conversation
opficdev
commented
Mar 3, 2026
- closed TodoListView에서 정렬 및 필터 옵션을 스티키 헤더로 재구성하고 description에 있는 데로 옵션을 수정한다 #160
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the Todo list functionality to introduce a more robust and flexible querying system. It consolidates various todo fetching use cases into a single, comprehensive Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This PR impressively integrates multiple Fetch...UseCase into a single FetchTodosUseCase and introduces TodoQuery to improve data fetching logic, reducing code duplication and enhancing flexibility and scalability. Additionally, the TodoListView's filter and sort options have been reconfigured into sticky headers, improving user experience. However, a critical security concern has been identified in TodoService.swift. Keyword-based searches are executing unbounded Firestore queries, which could lead to resource exhaustion or denial of service for users with large datasets. It is strongly recommended to apply a reasonable limit to these queries or implement a more scalable search solution. Further suggestions for improvement are detailed in the specific comments.
| var appliedFilterCount: Int { | ||
| var count = 0 | ||
| if state.query.sortTarget != .createdAt { count += 1 } | ||
| if state.query.sortOrder != .latest { count += 1 } | ||
| if state.query.isPinned != nil { count += 1 } | ||
| if state.query.completionFilter != .all { count += 1 } | ||
| return count | ||
| } |
There was a problem hiding this comment.
appliedFilterCount의 현재 구현은 동작에는 문제가 없지만, 각 조건을 배열로 만들고 filter와 count를 사용하는 방식으로 더 간결하고 선언적으로 개선할 수 있습니다. 이렇게 하면 가독성이 향상되고 의도를 더 명확하게 표현할 수 있습니다.
| var appliedFilterCount: Int { | |
| var count = 0 | |
| if state.query.sortTarget != .createdAt { count += 1 } | |
| if state.query.sortOrder != .latest { count += 1 } | |
| if state.query.isPinned != nil { count += 1 } | |
| if state.query.completionFilter != .all { count += 1 } | |
| return count | |
| } | |
| var appliedFilterCount: Int { | |
| [ | |
| state.query.sortTarget != .createdAt, | |
| state.query.sortOrder != .latest, | |
| state.query.isPinned != nil, | |
| state.query.completionFilter != .all | |
| ].filter { $0 }.count | |
| } |
| extension TodoQuery.SortTarget { | ||
| var title: String { | ||
| switch self { | ||
| case .createdAt: | ||
| return "생성" | ||
| case .updatedAt: | ||
| return "수정" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| extension TodoQuery.SortOrder { | ||
| var title: String { | ||
| switch self { | ||
| case .latest: | ||
| return "최신순" | ||
| case .oldest: | ||
| return "예전순" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| extension TodoQuery.CompletionFilter { | ||
| var title: String { | ||
| switch self { | ||
| case .all: | ||
| return "완료 + 미완료" | ||
| case .incomplete: | ||
| return "미완료" | ||
| case .completed: | ||
| return "완료" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
TodoQuery의 enum들에 대한 title 연산 프로퍼티 확장은 UI 표현과 관련된 로직입니다. 현재 ViewModel 파일에 위치해 있는데, 이는 Domain 계층의 모델이 Presentation 계층의 구체적인 내용(UI에 표시될 문자열)을 알게 되는 구조입니다. Clean Architecture 원칙에 따라 이러한 UI 관련 확장은 실제 사용되는 View 파일(이 경우 TodoListView.swift) 내부로 옮겨 private extension으로 관리하거나, Presentation 계층의 별도 파일로 분리하는 것이 좋습니다. 이렇게 하면 계층 간의 의존성을 줄이고 코드의 관심사를 명확히 분리할 수 있습니다.
| private var filterBadge: some View { | ||
| let isDark = colorScheme == .dark | ||
| let blue = Color(uiColor: .systemBlue) | ||
| let textColor: Color = isDark ? blue : .white | ||
| let backgroundColor: Color = isDark ? .white : blue | ||
|
|
||
| return Text("\(viewModel.appliedFilterCount)") | ||
| .font(.caption2.weight(.bold)) | ||
| .foregroundColor(textColor) | ||
| .lineLimit(1) | ||
| .minimumScaleFactor(0.6) | ||
| .frame(width: 20, height: 20) | ||
| .background(Circle().fill(backgroundColor)) | ||
| } |
There was a problem hiding this comment.
다크 모드에서 필터 뱃지 배경이 흰색으로 설정되어 있어 시각적으로 너무 두드러져 보일 수 있습니다. 라이트 모드와 일관성을 유지하기 위해, 다크 모드에서도 파란색 배경에 흰색 텍스트를 사용하도록 수정하는 것을 제안합니다. 이렇게 하면 colorScheme에 따라 분기하는 로직을 제거하여 코드를 더 간결하게 만들 수 있습니다.
private var filterBadge: some View {
let blue = Color(uiColor: .systemBlue)
return Text("\(viewModel.appliedFilterCount)")
.font(.caption2.weight(.bold))
.foregroundColor(.white)
.lineLimit(1)
.minimumScaleFactor(0.6)
.frame(width: 20, height: 20)
.background(Circle().fill(blue))
}