Design/#38 - Sidebar UI 및 SidebarFeature 구현#40
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
Walkthrough사이드바 필터·프로젝트 선택 기능을 TCA로 구현하고 MainFeature 및 NavigationSplitView에 연결했습니다. 사이드바 UI와 프로젝트 목록을 추가했으며, 카테고리와 프로젝트 컨테이너의 디자인 토큰 및 레이아웃을 조정했습니다. Changes메인 및 사이드바 UI
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SidebarView
participant SidebarFeature
participant MainFeature
User->>SidebarView: 필터 또는 프로젝트 선택
SidebarView->>SidebarFeature: didSelect 전송
SidebarFeature->>SidebarFeature: selection 갱신
SidebarFeature->>MainFeature: selectionChanged delegate 전송
MainFeature-->>SidebarView: 스코프된 상태로 화면 갱신
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
Projects/DVDesign/Sources/Components/DVCategory.swift (1)
98-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value불필요한
isHovered분기 제거
isHovered조건이 참일 때의 반환값과 조건문을 통과한 기본 반환값이 모두gray200으로 동일합니다. 중복되는 조건 분기를 제거하여 코드를 더 간결하게 유지하는 것을 제안합니다.💡 제안하는 수정안
private func backgroundColor(isPressed: Bool) -> Color { if isSelected { return Color.dv(.vaultGreen) } if isPressed { return Color.dv(.gray300) } - if isHovered { return Color.dv(.gray200) } return Color.dv(.gray200) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Projects/DVDesign/Sources/Components/DVCategory.swift` around lines 98 - 103, Remove the redundant isHovered branch from backgroundColor(isPressed:), since it returns the same gray200 color as the default path; preserve the isSelected and isPressed priority and keep gray200 as the fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Projects/DVDesign/Sources/Components/DVCategory.swift`:
- Around line 98-103: Remove the redundant isHovered branch from
backgroundColor(isPressed:), since it returns the same gray200 color as the
default path; preserve the isSelected and isPressed priority and keep gray200 as
the fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cc94e1fd-89d4-4aa9-ba86-7e3f75a636d8
📒 Files selected for processing (11)
Projects/DVDesign/Sources/Components/DVCategory.swiftProjects/DVDesign/Sources/Components/DVProjectContainer.swiftProjects/DVPresentation/Sources/Features/AppFeature.swiftProjects/DVPresentation/Sources/Features/AppView.swiftProjects/DVPresentation/Sources/Features/Main/MainFeature.swiftProjects/DVPresentation/Sources/Features/Main/MainView.swiftProjects/DVPresentation/Sources/Features/Sidebar/SidebarFeature.swiftProjects/DVPresentation/Sources/Features/Sidebar/SidebarView.swiftProjects/DVPresentation/Sources/ProjectSecretRelationDemoView.swiftProjects/DVPresentation/Sources/SecretUseCaseDemoView.swiftProjects/Devault/Sources/ContentView.swift
💤 Files with no reviewable changes (3)
- Projects/Devault/Sources/ContentView.swift
- Projects/DVPresentation/Sources/SecretUseCaseDemoView.swift
- Projects/DVPresentation/Sources/ProjectSecretRelationDemoView.swift
SidebarFilter, SidebarSelection enum 정의 및 SidebarFeature 구현. SidebarSelection으로 filter/project 선택을 단일 상태로 표현해 상호 배타성 보장.
Scope으로 SidebarFeature를 MainFeature에 연결하고 SidebarView를 MainView sidebar 컬럼에 주입.
DVCategory, DVProjectContainer 컴포넌트를 활용한 사이드바 UI 구현. filterGrid, projectSection, bottomBar로 구성.
| } | ||
| .listStyle(.sidebar) | ||
| .scrollContentBackground(.hidden) | ||
| .frame(maxHeight: 200) |
There was a problem hiding this comment.
macOS 앱은 세로로 크게 리사이즈 가능한데, 프로젝트 리스트가 창 높이와 무관하게 최대 200pt로 고정됩니다. 프로젝트가 5개만 넘어가도 즉시 내부 스크롤이 생기고, 아래쪽 Spacer()는 남는 공간을 다 먹습니다. 창이 아무리 커도 UX는 그대로.
제안: .frame(maxHeight: 200) 제거 + 바깥 Spacer() 제거 → List가 남는 세로 공간을 자연스럽게 채우도록. 또는 최소 .frame(minHeight: 100, maxHeight: .infinity) 형태로.
| if isPressed { return Color.dv(.gray300) } | ||
| if isHovered { return Color.dv(.gray200) } | ||
| return Color.dv(.gray100) | ||
| return Color.dv(.gray200) |
There was a problem hiding this comment.
default를 gray100 → gray200으로 올리면서 hover 상태와 색이 동일해졌습니다. .onHover + .animation을 걸어놨지만 시각 변화가 없어 hover 피드백이 죽어있습니다. macOS는 마우스가 항상 존재하므로 hover 피드백이 특히 중요합니다.
| public struct State: Equatable { | ||
| var selection: SidebarSelection = .filter(.all) | ||
| var isProjectSectionExpanded: Bool = true |
There was a problem hiding this comment.
State는 public인데 프로퍼티는 internal. 같은 모듈에서만 접근하니 지금은 문제 없지만, 다른 모듈에서 sidebar의 state를 관찰해야 하는 시점이 오면 걸립니다.
public getter만 노출하거나, 추후 수정.
| private var filterGrid: some View { | ||
| LazyVGrid( | ||
| columns: [GridItem(.flexible()), GridItem(.flexible())], | ||
| spacing: 12 | ||
| ) { | ||
| ForEach(SidebarFilter.allCases, id: \.self) { filter in | ||
| DVCategory( | ||
| title: filter.title, | ||
| count: 0, | ||
| systemImage: filter.icon, | ||
| isSelected: store.selection == .filter(filter) | ||
| ) { | ||
| store.send(.didSelect(.filter(filter))) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
NavigationSplitView의 min: 200, padding.horizontal 122=24를 빼면 그리드는 176pt. 2 flex 컬럼 + spacing 12
→ 각 셀 ~82pt. DVCategory 내부 padding 102=20 → 컨텐츠 62pt. "Expired" 텍스트가 bodyLG로 이 안에 들어가는지 실제로 200pt 폭에서 확인 필요. 위험하면 GridItem(.adaptive(minimum: 90))으로 자동 재배치.
| SidebarView( | ||
| store: Store(initialState: SidebarFeature.State()) { | ||
| SidebarFeature() | ||
| } | ||
| ) | ||
| .frame(width: 240) |
There was a problem hiding this comment.
min: 200과 max: 300도 프리뷰 추가 권장 (Group { ... }로 세 개 나열).
macOS 서비스 특수성 반영해서!
| var icon: String { | ||
| switch self { | ||
| case .all: "square.grid.2x2.fill" | ||
| case .starred: "star.fill" | ||
| case .expired: "exclamationmark" | ||
| case .deleted: "trash" | ||
| } |
There was a problem hiding this comment.
아이콘 전용 버튼들 (gearshape, plus, plus.circle, chevron.up/down)에 accessibilityLabel이 없어 VoiceOver로 "button"으로만 읽힙니다. macOS는 접근성 검사가 App Review에서도 이슈가 되므로 지금 추가 권장
| var title: String { | ||
| switch self { | ||
| case .all: "All" | ||
| case .starred: "Star" | ||
| case .expired: "Expired" | ||
| case .deleted: "Deleted" | ||
| } |
There was a problem hiding this comment.
로컬라이제이션: "All", "Star", "Project", "Rename" 등 모든 사용자 노출 문자열이 하드코딩. 후속 이슈로 관리하고 있는지 확인
2 participants
✨ What’s this PR?
📌 관련 이슈 (Related Issue)
🧶 주요 변경 내용 (Summary)
📸 스크린샷 (Optional)
🧪 테스트 / 검증 내역
💬 기타 공유 사항
SidebarSelection enum 도입
filter 선택과 project 선택은 동시에 존재할 수 없는 상호 배타적 상태.
selectedFilter + selectedProjectIndex두 프로퍼티로 분리하면 둘이 동시에 값을 갖는 잘못된 상태가 모델링 가능하므로,SidebarSelectionenum 하나로 통합해 타입 시스템에서 불가능하게 만들었습니다.List selection Binding 처리
List(selection:)은Binding<Int?>를 요구하지만 store의 selection은SidebarSelection(enum)이라 타입 불일치로$store직접 바인딩이 불가합니다.가이드라인(8.4)를 벗어나지만, 의도적 예외이므로 주석으로 이유를 명시했습니다.
List selection 컬러
macOS List
.sidebar스타일의 selection 하이라이트 색상은.tint()로 변경 불가 — 시스템 Accent Color를 강제 사용합니다.NSAccentColorNameasset 방식도 시도했으나 동작하지 않아 시스템 컬러로 유지합니다.List 가로 패딩
사이드바 전체에
.padding(.horizontal, 12)를 적용하되, List만.padding(.horizontal, -12)로 상쇄해 List가 사이드바 전체 너비를 사용하도록 처리했습니다.아무래도... 다크모드 대응이 필수일 듯 합니다..
🙇🏻♀️ 리뷰 가이드 (선택)
추후 예정 작업
contextMenuAction 구현 (Rename, New Secret, Delete Project)MainFeature.handleSidebarDelegate실제 구현 (현재 모두 .none)Summary by CodeRabbit
새 기능
스타일 개선