Fix/#151 QA 반영 #155
Merged
Fix/#151 QA 반영 #155
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wotjs020708
requested review from
soseoyo12,
sum130 and
y-eonee
and removed request for
a team
Closed
1 task
📝 WalkthroughWalkthrough캘린더 관련 치료 선택/필터 UI의 레이아웃 및 프레임 높이 조정, 치료 선택 토글 로직 추가, SelectedTreatmentSheetView의 ScrollViewReader 기반 자동 스크롤과 시트 표시 위치 변경, DdayState에 새 케이스 추가 등 프레젠테이션 중심의 변경입니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant FilterView as TreatmentFilterView
participant SheetView as SelectedTreatmentSheetView
participant ViewModel as ViewModel
User->>FilterView: 치료 항목 선택(탭)
FilterView->>ViewModel: addTreatment(treatment) or removeTreatment(treatment) (토글)
ViewModel-->>SheetView: selectedTreatments 업데이트
Note right of SheetView: ScrollViewReader 감지(onChange)
SheetView->>SheetView: proxy.scrollTo("scrollBottom", animated)
SheetView-->>User: 스크롤 애니메이션(새 항목 표시)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/TargetDdaySettingView.swift (1)
10-24: 버튼 순서가 PR 목표와 일치하지 않을 수 있습니다.PR 목표에 "버튼 순서("네, 있어요" / "아직 없어요") 교정"이라고 명시되어 있습니다. 현재
DdayStateenum에서no가 먼저 선언되어 있어ForEach(DdayState.allCases, ...)가 "아직 없어요"를 먼저 표시합니다.의도된 순서가 "네, 있어요" → "아직 없어요"라면 enum case 순서를 변경해야 합니다:
🔧 수정 제안
enum DdayState: CaseIterable { - case no case yes + case no var id: Self { self }Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift (1)
67-71: 중첩된 Task 제거 및 에러 처리 개선 필요.
.task수정자는 이미 async 컨텍스트를 제공하므로 내부Task는 불필요합니다. 또한try로 던져진 에러가 처리되지 않고 무시됩니다.🐛 수정 제안
.task { - Task { - try await viewModel.fetchTreatments() - } + do { + try await viewModel.fetchTreatments() + } catch { + CherrishLogger.error(error) + } }
🤖 Fix all issues with AI agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/SelectTreatment/SelectTreatmentView.swift`:
- Around line 72-90: The diff shows unnecessary blank lines inside the
SelectTreatmentView SwiftUI layout (inside the HStack/VStack block) which reduce
readability; remove the extra empty lines between the VStack children (around
the TypographyText and Spacer entries) so the VStack contents are compact and
consistently formatted, leaving the existing frames, alignment, and spacers
(references: HStack, VStack, TypographyText calls and Spacer) untouched.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/NoTreatment/NoTreatmentFilterView.swift`:
- Line 95: Remove the unnecessary extra whitespace inside the frame modifier
call in NoTreatmentFilterView (the line using .frame(height: 105.adjustedH
)), i.e., fix the .frame(height: ...) invocation so there are no trailing spaces
between the value and the closing parenthesis; update the .frame call in
NoTreatmentFilterView.swift accordingly.
- Around line 17-18: Remove the unused commented Spacer lines in
NoTreatmentFilterView.swift (the two lines "// Spacer()" and "//
.frame(height: 18.adjustedH)") to clean up dead/commented-out code; simply
delete these commented lines from the NoTreatmentFilterView Swift view (e.g.,
around where Spacer() was commented) so the file contains only active, relevant
UI code.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/SelectedTreatmentSheetView.swift`:
- Line 27: The `@State` variable scrollTargetID in SelectedTreatmentSheetView is
declared but never used; either remove the declaration or implement the intended
scrolling behavior: if unused, delete the line declaring scrollTargetID to avoid
dead state, otherwise wire it into your ScrollViewReader (or SwiftUI scrolling
logic) by giving list items identifiable IDs, update scrollTargetID when
selecting an item, and call proxy.scrollTo(scrollTargetID) (e.g., inside
onChange(of: scrollTargetID) or when presenting the sheet) so the view actually
scrolls to the targeted UUID; reference the scrollTargetID symbol and the
SelectedTreatmentSheetView body/ScrollViewReader when making the change.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/Treatment/TreatmentRowView.swift`:
- Around line 74-76: Duplicate downtime formatting exists in
TreatmentSummaryView and DownTimeLabel; extract it into a computed property on
TreatmentEntity named downtimeDisplayText. Add an extension or property on
TreatmentEntity that returns "다운타임* 0일" when downtimeMin == 0 && downtimeMax ==
0, otherwise "다운타임* \(downtimeMin)-\(downtimeMax)일", then replace the inline
logic in TreatmentSummaryView and DownTimeLabel to use
treatmentEntity.downtimeDisplayText.
Comment on lines
+72
to
90
| HStack(spacing: 0) { | ||
| VStack(alignment: .leading, spacing: 0){ | ||
|
|
||
| TypographyText("시술 일정을 추가해볼게요.", style: .title1_sb_18, color: .gray1000) | ||
| .frame(height: 27.adjustedH) | ||
| TypographyText("이미 생각해둔 시술이 있나요?", style: .title1_sb_18, color: .gray1000) | ||
| .frame(height: 27.adjustedH) | ||
|
|
||
|
|
||
|
|
||
| Spacer() | ||
| .frame(height: 4.adjustedH) | ||
| TypographyText("시술을 선택하셨는지 확인할게요.", style: .body1_r_14, color: .gray700) | ||
|
|
||
| .frame(height: 20.adjustedH) | ||
| } | ||
| .frame(height: 78.adjustedH) | ||
| Spacer() | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
레이아웃 구조가 명확합니다
명시적인 높이 프레임을 사용하여 일관된 레이아웃을 구현했습니다. 코드 내 불필요한 빈 줄(74, 79-80행)을 정리하면 가독성이 향상됩니다.
♻️ 코드 정리 제안
private var titleView: some View {
HStack(spacing: 0) {
VStack(alignment: .leading, spacing: 0){
-
TypographyText("시술 일정을 추가해볼게요.", style: .title1_sb_18, color: .gray1000)
.frame(height: 27.adjustedH)
TypographyText("이미 생각해둔 시술이 있나요?", style: .title1_sb_18, color: .gray1000)
.frame(height: 27.adjustedH)
-
-
-
Spacer()
.frame(height: 4.adjustedH)
TypographyText("시술을 선택하셨는지 확인할게요.", style: .body1_r_14, color: .gray700)
.frame(height: 20.adjustedH)
}
.frame(height: 78.adjustedH)
Spacer()
}
}📝 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.
Suggested change
| HStack(spacing: 0) { | |
| VStack(alignment: .leading, spacing: 0){ | |
| TypographyText("시술 일정을 추가해볼게요.", style: .title1_sb_18, color: .gray1000) | |
| .frame(height: 27.adjustedH) | |
| TypographyText("이미 생각해둔 시술이 있나요?", style: .title1_sb_18, color: .gray1000) | |
| .frame(height: 27.adjustedH) | |
| Spacer() | |
| .frame(height: 4.adjustedH) | |
| TypographyText("시술을 선택하셨는지 확인할게요.", style: .body1_r_14, color: .gray700) | |
| .frame(height: 20.adjustedH) | |
| } | |
| .frame(height: 78.adjustedH) | |
| Spacer() | |
| } | |
| } | |
| HStack(spacing: 0) { | |
| VStack(alignment: .leading, spacing: 0){ | |
| TypographyText("시술 일정을 추가해볼게요.", style: .title1_sb_18, color: .gray1000) | |
| .frame(height: 27.adjustedH) | |
| TypographyText("이미 생각해둔 시술이 있나요?", style: .title1_sb_18, color: .gray1000) | |
| .frame(height: 27.adjustedH) | |
| Spacer() | |
| .frame(height: 4.adjustedH) | |
| TypographyText("시술을 선택하셨는지 확인할게요.", style: .body1_r_14, color: .gray700) | |
| .frame(height: 20.adjustedH) | |
| } | |
| .frame(height: 78.adjustedH) | |
| Spacer() | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/SelectTreatment/SelectTreatmentView.swift`
around lines 72 - 90, The diff shows unnecessary blank lines inside the
SelectTreatmentView SwiftUI layout (inside the HStack/VStack block) which reduce
readability; remove the extra empty lines between the VStack children (around
the TypographyText and Spacer entries) so the VStack contents are compact and
consistently formatted, leaving the existing frames, alignment, and spacers
(references: HStack, VStack, TypographyText calls and Spacer) untouched.
...ish-iOS/Presentation/Feature/Calendar/Treatment/View/NoTreatment/NoTreatmentFilterView.swift
Outdated
Show resolved
Hide resolved
| .gray500() | ||
| } | ||
| .frame(height: 105) | ||
| .frame(height: 105.adjustedH ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
불필요한 공백 제거
frame 수정자 내에 불필요한 공백이 있습니다.
♻️ 제안된 수정
- .frame(height: 105.adjustedH )
+ .frame(height: 105.adjustedH)🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/NoTreatment/NoTreatmentFilterView.swift`
at line 95, Remove the unnecessary extra whitespace inside the frame modifier
call in NoTreatmentFilterView (the line using .frame(height: 105.adjustedH
)), i.e., fix the .frame(height: ...) invocation so there are no trailing spaces
between the value and the closing parenthesis; update the .frame call in
NoTreatmentFilterView.swift accordingly.
...S/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/SelectedTreatmentSheetView.swift
Outdated
Show resolved
Hide resolved
Comment on lines
+74
to
+76
| treatmentEntity.downtimeMin == 0 && treatmentEntity.downtimeMax == 0 ? | ||
| "다운타임* 0일" : | ||
| "다운타임* \(treatmentEntity.downtimeMin)-\(treatmentEntity.downtimeMax)일", |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
다운타임 표시 로직 중복 — 헬퍼 함수 추출 권장.
동일한 다운타임 문자열 포맷팅 로직이 TreatmentSummaryView(라인 74-76)와 DownTimeLabel(라인 188-190)에 중복되어 있습니다. TreatmentEntity의 computed property로 추출하면 유지보수성이 향상됩니다.
♻️ TreatmentEntity extension 추가 제안
// TreatmentEntity+Extension.swift 또는 해당 모델 파일에 추가
extension TreatmentEntity {
var downtimeDisplayText: String {
if downtimeMin == 0 && downtimeMax == 0 {
return "다운타임* 0일"
}
return "다운타임* \(downtimeMin)-\(downtimeMax)일"
}
}사용 예시:
TypographyText(
- treatmentEntity.downtimeMin == 0 && treatmentEntity.downtimeMax == 0 ?
- "다운타임* 0일" :
- "다운타임* \(treatmentEntity.downtimeMin)-\(treatmentEntity.downtimeMax)일",
+ treatmentEntity.downtimeDisplayText,
style: .body1_r_14,
color: .gray700
)🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/Treatment/TreatmentRowView.swift`
around lines 74 - 76, Duplicate downtime formatting exists in
TreatmentSummaryView and DownTimeLabel; extract it into a computed property on
TreatmentEntity named downtimeDisplayText. Add an extension or property on
TreatmentEntity that returns "다운타임* 0일" when downtimeMin == 0 && downtimeMax ==
0, otherwise "다운타임* \(downtimeMin)-\(downtimeMax)일", then replace the inline
logic in TreatmentSummaryView and DownTimeLabel to use
treatmentEntity.downtimeDisplayText.
sum130
approved these changes
Jan 22, 2026
Comment on lines
17
to
18
| // Spacer() | ||
| // .frame(height: 18.adjustedH) |
...S/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/SelectedTreatmentSheetView.swift
Outdated
Show resolved
Hide resolved
| .allowsHitTesting(false) | ||
| } | ||
|
|
||
| // 스크롤 앵커용 더미 뷰 |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift (1)
69-73:.task내부의 중복Task래퍼 제거 권장
.task수정자는 이미 비동기 컨텍스트를 제공하므로 내부에Task { }래퍼가 불필요합니다. 또한 에러가 발생해도 처리되지 않습니다.♻️ 제안된 수정
.task { - Task { - try await viewModel.fetchTreatments() - } + do { + try await viewModel.fetchTreatments() + } catch { + // 에러 처리 로직 추가 필요 + } }
🤖 Fix all issues with AI agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/NoTreatment/NoTreatmentFilterView.swift`:
- Around line 38-39: Replace the magic number 198.adjustedH used in the Spacer
frame with a named constant (e.g., bottomSheetSpacing or keyboardInset) declared
near the top of NoTreatmentFilterView or in a dedicated ViewConstants struct and
add a one-line comment explaining the intent (e.g., "spacing to account for
bottom sheet/keyboard inset"); update the Spacer to use that constant (preserve
.adjustedH application if needed) so the purpose is clear and maintainable.
- Around line 26-33: The action closure contains an unnecessary blank line after
viewModel.addTreatment(treatment); remove that extra empty line so the toggle
logic using viewModel.isSelected(treatment),
viewModel.removeTreatment(treatment), and viewModel.addTreatment(treatment) is
compact and consistent; update the closure in NoTreatmentFilterView's action
block to eliminate the stray blank line.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/NoTreatment/NoTreatmentView.swift`:
- Around line 82-90: Remove the redundant state check inside NoTreatmentView's
overlay: since this block is already in the .treatmentFilter case, drop the
`viewModel.state == .treatmentFilter` condition and only check
`!viewModel.selectedTreatments.isEmpty` before showing
SelectedTreatmentSheetView; locate the overlay code in NoTreatmentView.swift
where SelectedTreatmentSheetView is created (references: viewModel.state,
.treatmentFilter, SelectedTreatmentSheetView, selectedTreatments,
removeTreatment) and simplify the conditional accordingly.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/SelectedTreatmentSheetView.swift`:
- Around line 70-71: Move the horizontal padding from the ForEach container to
each row: remove the .padding(.horizontal, 24.5.adjustedW) applied after the
ForEach in SelectedTreatmentSheetView and instead apply .padding(.horizontal,
24.5.adjustedW) directly to each TreatmentRowView instance inside the ForEach
(same pattern as NoTreatmentFilterView); update the ForEach content where
TreatmentRowView is created so each row receives the padding individually and
ensure no leftover container padding remains.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift`:
- Around line 64-65: 해당 매직 넘버는 TreatmentFilterView의 Spacer 높이(198.adjustedH)로
SelectedTreatmentSheetView 오버레이 공간을 예약하는 용도이므로 198.adjustedH를 하드코딩된 리터럴로 두지 말고
상수로 추출하거나 SelectedTreatmentSheetView의 시트 높이와 연동하세요; 예를 들어 TreatmentFilterView 내에
private enum Layout { static let selectedSheetHeight = ... } 또는
SelectedTreatmentSheetView에서 public static let sheetHeight을 제공한 뒤
Spacer().frame(height: SelectedTreatmentSheetView.sheetHeight.adjustedH)처럼 참조하도록
변경해 유지보수성을 개선하세요.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentView.swift`:
- Around line 77-85: 현재 TreatmentView contains an overlay that checks
viewModel.state == .treatmentFilter even though this view is already inside the
.treatmentFilter case, causing a redundant condition and duplicating the same
overlay pattern found in NoTreatmentView; remove the redundant state check and
extract the overlay into a shared component (e.g., a ViewModifier or a small
reusable view) that wraps SelectedTreatmentSheetView (use
viewModel.selectedTreatments and viewModel.removeTreatment(_:) as the public
API) so both TreatmentView and NoTreatmentView can reuse the same overlay logic.
♻️ Duplicate comments (1)
Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/NoTreatment/NoTreatmentFilterView.swift (1)
95-95: 불필요한 공백 제거 필요
frame수정자 내에 불필요한 공백이 여전히 존재합니다. 이전 리뷰에서 지적된 사항입니다.♻️ 제안된 수정
- .frame(height: 105.adjustedH ) + .frame(height: 105.adjustedH)
Comment on lines
+26
to
+33
| action: { | ||
| if viewModel.isSelected(treatment) { | ||
| viewModel.removeTreatment(treatment) | ||
| } else { | ||
| viewModel.addTreatment(treatment) | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
토글 로직 구현이 적절합니다.
ViewModel의 isSelected, addTreatment, removeTreatment 메서드를 활용한 선택/해제 토글 로직이 올바르게 구현되어 있습니다.
다만, 31번 줄에 불필요한 빈 줄이 있어 정리하면 좋겠습니다.
♻️ 제안된 수정
action: {
if viewModel.isSelected(treatment) {
viewModel.removeTreatment(treatment)
} else {
viewModel.addTreatment(treatment)
-
}
}📝 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.
Suggested change
| action: { | |
| if viewModel.isSelected(treatment) { | |
| viewModel.removeTreatment(treatment) | |
| } else { | |
| viewModel.addTreatment(treatment) | |
| } | |
| } | |
| action: { | |
| if viewModel.isSelected(treatment) { | |
| viewModel.removeTreatment(treatment) | |
| } else { | |
| viewModel.addTreatment(treatment) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/NoTreatment/NoTreatmentFilterView.swift`
around lines 26 - 33, The action closure contains an unnecessary blank line
after viewModel.addTreatment(treatment); remove that extra empty line so the
toggle logic using viewModel.isSelected(treatment),
viewModel.removeTreatment(treatment), and viewModel.addTreatment(treatment) is
compact and consistent; update the closure in NoTreatmentFilterView's action
block to eliminate the stray blank line.
Comment on lines
+38
to
+39
| Spacer() | ||
| .frame(height: 198.adjustedH) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
매직 넘버에 대한 설명 추가 권장
198.adjustedH 값의 의도가 명확하지 않습니다. 이 값이 특정 UI 요소(예: 하단 시트, 키보드 등)와의 간격을 위한 것이라면 상수로 추출하거나 주석을 추가하여 의도를 명확히 하면 유지보수에 도움이 됩니다.
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/NoTreatment/NoTreatmentFilterView.swift`
around lines 38 - 39, Replace the magic number 198.adjustedH used in the Spacer
frame with a named constant (e.g., bottomSheetSpacing or keyboardInset) declared
near the top of NoTreatmentFilterView or in a dedicated ViewConstants struct and
add a one-line comment explaining the intent (e.g., "spacing to account for
bottom sheet/keyboard inset"); update the Spacer to use that constant (preserve
.adjustedH application if needed) so the purpose is clear and maintainable.
Comment on lines
+82
to
+90
| .overlay(alignment: .bottom) { | ||
| if viewModel.state == .treatmentFilter, !viewModel.selectedTreatments.isEmpty { | ||
| SelectedTreatmentSheetView( | ||
| selectedTreatments: viewModel.selectedTreatments, | ||
| removeTreatment: viewModel.removeTreatment(_:) | ||
| ) | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
중복 상태 체크 제거 권장
case .treatmentFilter: 내에서 viewModel.state == .treatmentFilter 조건은 항상 true이므로 불필요합니다.
♻️ 제안된 수정
NoTreatmentFilterView(viewModel: viewModel)
.overlay(alignment: .bottom) {
- if viewModel.state == .treatmentFilter, !viewModel.selectedTreatments.isEmpty {
+ if !viewModel.selectedTreatments.isEmpty {
SelectedTreatmentSheetView(
selectedTreatments: viewModel.selectedTreatments,
removeTreatment: viewModel.removeTreatment(_:)
)
-
}
}🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/NoTreatment/NoTreatmentView.swift`
around lines 82 - 90, Remove the redundant state check inside NoTreatmentView's
overlay: since this block is already in the .treatmentFilter case, drop the
`viewModel.state == .treatmentFilter` condition and only check
`!viewModel.selectedTreatments.isEmpty` before showing
SelectedTreatmentSheetView; locate the overlay code in NoTreatmentView.swift
where SelectedTreatmentSheetView is created (references: viewModel.state,
.treatmentFilter, SelectedTreatmentSheetView, selectedTreatments,
removeTreatment) and simplify the conditional accordingly.
Comment on lines
+70
to
+71
| } | ||
| .padding(.horizontal, 24.5.adjustedW) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
find . -name "SelectedTreatmentSheetView.swift" -type fRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 175
🏁 Script executed:
# Check the context around lines 70-71 in the file
sed -n '60,80p' Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/SelectedTreatmentSheetView.swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1132
🏁 Script executed:
# Run the verification script to check padding patterns with TreatmentRowView
rg -n "TreatmentRowView" --type swift -A 5 -B 5Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 8805
🏁 Script executed:
# Check padding application in DownTimeSettingView
sed -n '30,50p' Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/DownTimeSettingView.swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1016
🏁 Script executed:
# Check padding application in TreatmentFilterView
sed -n '45,65p' Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1036
🏁 Script executed:
# Check padding application in NoTreatmentFilterView
sed -n '15,35p' Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/NoTreatment/NoTreatmentFilterView.swiftRepository: TEAM-Cherrish/Cherrish-iOS
Length of output: 1042
TreatmentRowView에 개별 패딩 적용 권장
.padding(.horizontal, 24.5.adjustedW)를 ForEach 전체에 적용하는 대신, NoTreatmentFilterView에서 사용하는 패턴처럼 각 TreatmentRowView에 직접 적용하는 것이 더 명확합니다. 이렇게 하면 각 행이 독립적으로 패딩을 받으며 코드의 의도가 더 분명해집니다.
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/SelectedTreatmentSheetView.swift`
around lines 70 - 71, Move the horizontal padding from the ForEach container to
each row: remove the .padding(.horizontal, 24.5.adjustedW) applied after the
ForEach in SelectedTreatmentSheetView and instead apply .padding(.horizontal,
24.5.adjustedW) directly to each TreatmentRowView instance inside the ForEach
(same pattern as NoTreatmentFilterView); update the ForEach content where
TreatmentRowView is created so each row receives the padding individually and
ensure no leftover container padding remains.
Comment on lines
+64
to
+65
| Spacer() | ||
| .frame(height: 198.adjustedH) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
매직 넘버 상수화 권장
198.adjustedH는 SelectedTreatmentSheetView 오버레이 공간을 위한 것으로 보입니다. 이 값을 상수로 추출하거나 시트 높이와 연동하면 유지보수성이 향상됩니다.
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift`
around lines 64 - 65, 해당 매직 넘버는 TreatmentFilterView의 Spacer 높이(198.adjustedH)로
SelectedTreatmentSheetView 오버레이 공간을 예약하는 용도이므로 198.adjustedH를 하드코딩된 리터럴로 두지 말고
상수로 추출하거나 SelectedTreatmentSheetView의 시트 높이와 연동하세요; 예를 들어 TreatmentFilterView 내에
private enum Layout { static let selectedSheetHeight = ... } 또는
SelectedTreatmentSheetView에서 public static let sheetHeight을 제공한 뒤
Spacer().frame(height: SelectedTreatmentSheetView.sheetHeight.adjustedH)처럼 참조하도록
변경해 유지보수성을 개선하세요.
Comment on lines
+77
to
+85
| .overlay(alignment: .bottom) { | ||
| if viewModel.state == .treatmentFilter, !viewModel.selectedTreatments.isEmpty { | ||
| SelectedTreatmentSheetView( | ||
| selectedTreatments: viewModel.selectedTreatments, | ||
| removeTreatment: viewModel.removeTreatment(_:) | ||
| ) | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
중복 상태 체크 및 코드 중복 개선 권장
case .treatmentFilter:내에서viewModel.state == .treatmentFilter조건이 중복됩니다.NoTreatmentView.swift의 Lines 82-90과 동일한 오버레이 패턴이 반복됩니다.
♻️ 중복 조건 제거
TreatmentFilterView(viewModel: viewModel)
.overlay(alignment: .bottom) {
- if viewModel.state == .treatmentFilter, !viewModel.selectedTreatments.isEmpty {
+ if !viewModel.selectedTreatments.isEmpty {
SelectedTreatmentSheetView(
selectedTreatments: viewModel.selectedTreatments,
removeTreatment: viewModel.removeTreatment(_:)
)
-
}
}향후 오버레이 로직을 ViewModifier 또는 공통 컴포넌트로 추출하면 TreatmentView와 NoTreatmentView 간 중복을 줄일 수 있습니다.
📝 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.
Suggested change
| .overlay(alignment: .bottom) { | |
| if viewModel.state == .treatmentFilter, !viewModel.selectedTreatments.isEmpty { | |
| SelectedTreatmentSheetView( | |
| selectedTreatments: viewModel.selectedTreatments, | |
| removeTreatment: viewModel.removeTreatment(_:) | |
| ) | |
| } | |
| } | |
| .overlay(alignment: .bottom) { | |
| if !viewModel.selectedTreatments.isEmpty { | |
| SelectedTreatmentSheetView( | |
| selectedTreatments: viewModel.selectedTreatments, | |
| removeTreatment: viewModel.removeTreatment(_:) | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentView.swift`
around lines 77 - 85, 현재 TreatmentView contains an overlay that checks
viewModel.state == .treatmentFilter even though this view is already inside the
.treatmentFilter case, causing a redundant condition and duplicating the same
overlay pattern found in NoTreatmentView; remove the redundant state check and
extract the overlay into a shared component (e.g., a ViewModifier or a small
reusable view) that wraps SelectedTreatmentSheetView (use
viewModel.selectedTreatments and viewModel.removeTreatment(_:) as the public
API) so both TreatmentView and NoTreatmentView can reuse the same overlay logic.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔗 연결된 이슈
📄 작업 내용