Conversation
- 하루 종일 UILabel을 눌러도 하루 종일 버튼이 눌리도록 수정
Walkthrough이번 변경에서는 루틴 생성 화면에서 "하루종일" 실행 방식 토글을 위한 새로운 버튼과 하단 시트 형태의 시간 선택기(DatePickerView)가 도입되었습니다. 실행 시간의 내부 표현이 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RoutineCreationView
participant DatePickerView
participant RoutineCreationViewModel
User->>RoutineCreationView: "하루종일" 버튼/라벨 클릭
RoutineCreationView->>RoutineCreationViewModel: configureExecutionTime(.allDay)
RoutineCreationViewModel-->>RoutineCreationView: executionTimePublisher("하루종일")
User->>RoutineCreationView: 시간 선택 버튼 클릭
RoutineCreationView->>DatePickerView: present (bottom sheet)
User->>DatePickerView: 시간 선택 후 "등록하기" 클릭
DatePickerView->>RoutineCreationView: datePickerView(_:didSelectTime:)
RoutineCreationView->>RoutineCreationViewModel: configureExecutionTime(.time(selectedDate))
RoutineCreationViewModel-->>RoutineCreationView: executionTimePublisher("오전 9:00" 등)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
Projects/Presentation/Sources/RoutineCreation/View/RoutineCreationView.swift (1)
446-453: 하드코딩된 문자열 비교 개선 필요
executionType == "하루종일"비교에서 하드코딩된 문자열을 사용하고 있습니다. 이는 로컬라이제이션이나 텍스트 변경 시 오류를 발생시킬 수 있습니다.다음과 같이 상수를 사용하거나 enum 비교를 하는 것이 좋겠습니다:
// 상수 정의 private enum Constants { static let allDayText = "하루종일" } // 또는 ViewModel에서 boolean 값을 함께 제공하는 방식 고려
🧹 Nitpick comments (2)
Projects/Presentation/Sources/DatePicker/View/DatePickerView.swift (1)
1-7: 파일 이름과 클래스 이름 불일치 수정 필요파일명은
DatePickerViewController.swift이지만 실제 클래스명은DatePickerView입니다. 일관성을 위해 파일명을DatePickerView.swift로 변경하거나 클래스명을DatePickerViewController로 변경해주세요.Projects/Presentation/Sources/RoutineCreation/View/RoutineCreationView.swift (1)
206-206: allDayButton2의 레이아웃과 역할
allDayButton2가allDayLabel과 같은 영역에 배치되어 있어, 라벨 영역 전체를 터치 가능하게 만드는 목적으로 보입니다. 이는 UX 개선을 위한 좋은 접근입니다.하지만 코드 가독성을 위해 더 명확한 이름을 사용하는 것을 고려해보세요:
- private let allDayButton2 = UIButton() + private let allDayLabelTouchArea = UIButton()Also applies to: 347-350
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Projects/Presentation/Sources/DatePicker/View/DatePickerView.swift(1 hunks)Projects/Presentation/Sources/RoutineCreation/View/RoutineCreationView.swift(7 hunks)Projects/Presentation/Sources/RoutineCreation/ViewModel/RoutineCreationViewModel.swift(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: choijungp
PR: YAPP-Github/Bitnagil-iOS#19
File: Projects/Presentation/Sources/Common/PresentationDependencyAssembler.swift:0-0
Timestamp: 2025-07-16T10:57:16.559Z
Learning: choijungp는 추후 CustomAlertView를 만들어서 에러 처리를 UI로 해결하도록 수정할 계획을 가지고 있음.
🔇 Additional comments (8)
Projects/Presentation/Sources/DatePicker/View/DatePickerView.swift (1)
47-52: 메모리 관리 확인 - weak self 사용UIAction 클로저에서
weak self를 적절히 사용하고 있어 retain cycle을 방지하고 있습니다. 좋은 구현입니다.Projects/Presentation/Sources/RoutineCreation/View/RoutineCreationView.swift (3)
507-513: 배열을 사용한 중복 코드 제거여러 버튼에 동일한 액션을 추가하는 방식이 깔끔합니다. 코드 중복을 효과적으로 제거했습니다.
517-521: DatePickerView 프레젠테이션 로직커스텀 바텀시트를 사용한 DatePickerView 프레젠테이션이 잘 구현되어 있습니다. 델리게이트 설정도 적절합니다.
627-631: DatePickerViewDelegate 구현델리게이트 메서드 구현이 적절하며, 선택된 시간을 ViewModel에 올바르게 전달하고 있습니다.
Projects/Presentation/Sources/RoutineCreation/ViewModel/RoutineCreationViewModel.swift (4)
8-8: Foundation 임포트 추가Date 타입 사용을 위한 Foundation 임포트가 적절히 추가되었습니다.
24-24: ExecutionType.time의 Date 타입 변경String에서 Date로 타입을 변경한 것은 훌륭한 개선사항입니다. 타입 안전성과 시간 처리의 정확성이 향상되었습니다.
61-61: 출력 타입 변경
executionTimePublisher의 타입을AnyPublisher<String, Never>로 변경한 것이 UI 요구사항에 맞게 잘 설계되었습니다.
79-81: Publisher 매핑 로직
executionTimeSubject를 description으로 매핑하는 로직이 명확하고 적절합니다.
| func configureLayout() { | ||
| let safeArea = view.safeAreaLayoutGuide | ||
| view.addSubview(datePicker) | ||
| view.addSubview(registerButton) | ||
|
|
||
| datePicker.snp.makeConstraints { make in | ||
| make.top.horizontalEdges.equalToSuperview() | ||
| make.height.equalTo(Layout.datePickerHeight) | ||
| } | ||
|
|
||
| registerButton.snp.makeConstraints { make in | ||
| make.horizontalEdges.equalToSuperview().inset(Layout.horizontalSpacing) | ||
| make.top.equalTo(datePicker.snp.bottom).offset(Layout.reigisterButtonVerticalSpacing) | ||
| make.bottom.equalTo(safeArea.snp.bottom).offset(-Layout.reigisterButtonVerticalSpacing) | ||
| make.height.equalTo(Layout.registerButtonHeight) | ||
| } | ||
| } |
There was a problem hiding this comment.
Auto Layout 제약 조건 검토
레이아웃 구성에서 타이포로 인한 오류가 발생할 수 있습니다. 67번 라인에서 수정된 상수명을 사용해야 합니다.
- make.top.equalTo(datePicker.snp.bottom).offset(Layout.reigisterButtonVerticalSpacing)
- make.bottom.equalTo(safeArea.snp.bottom).offset(-Layout.reigisterButtonVerticalSpacing)
+ make.top.equalTo(datePicker.snp.bottom).offset(Layout.registerButtonVerticalSpacing)
+ make.bottom.equalTo(safeArea.snp.bottom).offset(-Layout.registerButtonVerticalSpacing)🤖 Prompt for AI Agents
In Projects/Presentation/Sources/DatePicker/View/DatePickerView.swift between
lines 55 and 71, there is a typo in the constant name used for the
registerButton constraints. Replace all instances of
"reigisterButtonVerticalSpacing" with the correctly spelled
"registerButtonVerticalSpacing" to fix the layout constraint errors.
| let dateFormatter = DateFormatter() | ||
| dateFormatter.locale = Locale(identifier: "ko_KR") | ||
| dateFormatter.dateFormat = "a h:mm" | ||
|
|
||
| return dateFormatter.string(from: time) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
DateFormatter 성능 최적화 필요
현재 description computed property에서 매번 새로운 DateFormatter를 생성하고 있습니다. 이는 성능에 부정적인 영향을 줄 수 있습니다.
static 프로퍼티로 DateFormatter를 캐시하는 것을 권장합니다:
+ private static let timeFormatter: DateFormatter = {
+ let formatter = DateFormatter()
+ formatter.locale = Locale(identifier: "ko_KR")
+ formatter.dateFormat = "a h:mm"
+ return formatter
+ }()
+
var description: String {
switch self {
case .time(let time):
- let dateFormatter = DateFormatter()
- dateFormatter.locale = Locale(identifier: "ko_KR")
- dateFormatter.dateFormat = "a h:mm"
-
- return dateFormatter.string(from: time)
+ return Self.timeFormatter.string(from: time)🤖 Prompt for AI Agents
In
Projects/Presentation/Sources/RoutineCreation/ViewModel/RoutineCreationViewModel.swift
around lines 31 to 35, the DateFormatter is being created every time the
description property is accessed, which hurts performance. To fix this, define a
static DateFormatter property on the type to cache and reuse the formatter
instance instead of creating a new one each time. Update the description
property to use this static cached formatter.
There was a problem hiding this comment.
요 부분 Presentation 모듈에 Date+ 파일 확인해보면 case 추가해서 재사용할 수 있을 것 같아요 !!!
but, DateFormatter()를 재사용하고 있진 않음 .. 함수 호출할 때마다 인스턴스 생성되긴 함 ...
There was a problem hiding this comment.
dateformatter를 매번 생성하는 작업이 가벼운 작업이라 생각해서 이렇게 구현했습니다. 관련해서 찾아보니 dateformatter를 생성하는 것이 꽤 expensive 하다는 레퍼런스를 많이 접할 수 있었습니다.
왜 생성에 많은 비용이 드는지는 좀 더 공부해봐야겠네요! 코드는 수정하겠습니다. 감사합니다~
There was a problem hiding this comment.
@taipaise 좋은 학습 자세네요! 🎉
DateFormatter가 expensive한 이유는 다음과 같습니다:
- 로케일 데이터 로딩: 각 로케일의 날짜/시간 형식, 달력 시스템, 언어별 월/요일 이름 등 방대한 데이터를 메모리에 로드
- 포맷 패턴 파싱: "a h:mm" 같은 포맷 문자열을 내부 규칙으로 변환하는 복잡한 파싱 과정
- ICU 라이브러리 초기화: Apple의 국제화 라이브러리인 ICU의 복잡한 초기화 과정
그래서 Apple 공식 문서에서도 DateFormatter를 재사용하라고 권장하고 있어요. 성능 최적화에 대한 호기심이 좋은 개발 습관으로 이어질 것 같습니다!
🧠 Learnings used
Learnt from: taipaise
PR: YAPP-Github/Bitnagil-iOS#26
File: Projects/Presentation/Sources/DatePicker/View/DatePickerView.swift:33-53
Timestamp: 2025-07-27T13:07:40.427Z
Learning: DatePickerView에서 locale을 "en_US"로 설정하는 것은 한국어 앱이지만 시간을 AM/PM 형식으로 표시하기 위한 의도적인 UX 설계 결정이다. 한국 로케일은 기본적으로 24시간 형식을 사용하기 때문이다.
Learnt from: choijungp
PR: YAPP-Github/Bitnagil-iOS#19
File: Projects/Presentation/Sources/Common/PresentationDependencyAssembler.swift:0-0
Timestamp: 2025-07-16T10:57:16.559Z
Learning: choijungp는 추후 CustomAlertView를 만들어서 에러 처리를 UI로 해결하도록 수정할 계획을 가지고 있음.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@choijungp 그럼 우선 case에 추가시키고, 인스턴스를 재활용하는 건 데스 노트에 추가해서 추후 개선할까요?
There was a problem hiding this comment.
동의 동의 동의 !! 완전 동의 !!!!!!!!!!
| let dateFormatter = DateFormatter() | ||
| dateFormatter.locale = Locale(identifier: "ko_KR") | ||
| dateFormatter.dateFormat = "a h:mm" | ||
|
|
||
| return dateFormatter.string(from: time) |
There was a problem hiding this comment.
요 부분 Presentation 모듈에 Date+ 파일 확인해보면 case 추가해서 재사용할 수 있을 것 같아요 !!!
but, DateFormatter()를 재사용하고 있진 않음 .. 함수 호출할 때마다 인스턴스 생성되긴 함 ...
| // | ||
| // DatePickerViewController.swift | ||
| // Presentation | ||
| // | ||
| // Created by 이동현 on 7/27/25. | ||
| // |
There was a problem hiding this comment.
이 파일을 Sources 밑에 DatePicker로 위치 시킨 이유가 있으실까요 ??
루틴 등록 외에 또 어디서 사용할지 모르겠어서욤 ...
만약 재사용된다고 해도 Common으로 옮기는게 어떠신지 ?? 재사용 안된다면 RoutineCreation 폴더 밑에 위치시키는건 어떠신지 ?!?!
There was a problem hiding this comment.
재사용 여지가 없는거 같아서 RoutineCreation 아래가 적절해 보이네요! 이동시키겠습니다~!
| allDayButton2.snp.makeConstraints { make in | ||
| make.edges.equalTo(allDayLabel) | ||
| } |
There was a problem hiding this comment.
-
매우매우 주관적인 생각이지만, 사용자들은 하루종일 label을 눌러도 체크가 되길 원하지 않을까라는 생각을 해서 투명 버튼을 위에 덧씌웠습니다! 체크 박스 영역이 생각보다 작다고 느껴져서요.!.!
-
버튼 자체를 low하게 커스텀 하는 방법도 있지만, 공수가 많이 들고 코드가 훨씬 복잡해진다고 생각합니다. 또 label 위에 투명 버튼을 올리는 방법은 실제 현업에서도 많이 사용하는 방법으로 알고 있습니다! 버튼을 합쳐서 하나로 만든다 하더라도, (버튼 1개, 체크 박스 이미지 뷰 1개, 하루종을 label 1개)로 생성되는 UI 컴포넌트 수는 같을 거 같다 판단했습니다. 따라서 버튼을 하나로 합치고, UIImageView를 하나 추가하는 것이 아니라, 기존에 잡아둔 layout에 투명 버튼을 하나 덧씌우는 방법을 선택했습니다!
-
체크 박스와 label 사이 공간은 굉장히 작기 때문에 손가락의 두께를 고려했을 때 크게 상관 없지 않을까 하는데, 수정하는게 좋을까요???
There was a problem hiding this comment.
노오 ~~ 사실 진짜 진짜 악의적인 경우를 고려했을 때이긴 함니다 !!!! 지금두 괜찮다구 생각해욤 !!!!
지금은 아무툰간에 .. 빠른 개발이 중요하긴 하니까유 ........
완벽한 PR이라 좀 꼬집어봤슴니다 !!!! 😈
근데 지이이이인짜 나중에 만약에 시간이 많이많이 남는다면 최최최최최최종 마지막으로 요 부분 다시 생각해부아요 ㅎㅎ
choijungp
left a comment
There was a problem hiding this comment.
코멘트 다 확인했고 파일 위치 옮기기랑 Date+ 만 반영하면 될 것 같습니다 !!!!!
믿고 띵푸루부 하겠습니다 !!!!!!!!
1일 2PR이라니 ... ... 🔥🔥🔥
대단띵동 ㄷㄷㄷㄷ !!!!!!!!!!!!!!!!!!! 굿굿굿
|
@choijungp 초 경량 pr이라 🥹 리뷰 내용은 377cf97 에서 반영하였습니다. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Projects/Presentation/Sources/RoutineCreation/View/Component/DatePickerView.swift (1)
2-2: 파일명 불일치 수정 필요헤더 코멘트의 파일명이 "DatePickerViewController.swift"로 되어 있지만 실제 파일명은 "DatePickerView.swift"입니다.
-// DatePickerViewController.swift +// DatePickerView.swiftProjects/Presentation/Sources/RoutineCreation/View/RoutineCreationView.swift (1)
446-453: 문자열 비교 방식의 잠재적 취약성
executionType == "하루종일"문자열 비교는 현재 작동하지만, 향후 로컬라이제이션이나 텍스트 변경 시 문제가 될 수 있습니다. enum 기반 비교가 더 안전할 것 같습니다.더 안전한 구현을 위해 문자열 대신 타입 기반 비교를 고려해보세요:
// ViewModel에서 ExecutionType을 함께 제공하거나 // 별도의 isAllDay 프로퍼티를 추가하는 방법
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Projects/Presentation/Sources/Common/Extension/Date+.swift(2 hunks)Projects/Presentation/Sources/RoutineCreation/View/Component/DatePickerView.swift(1 hunks)Projects/Presentation/Sources/RoutineCreation/View/RoutineCreationView.swift(7 hunks)Projects/Presentation/Sources/RoutineCreation/ViewModel/RoutineCreationViewModel.swift(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Projects/Presentation/Sources/RoutineCreation/ViewModel/RoutineCreationViewModel.swift
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: taipaise
PR: YAPP-Github/Bitnagil-iOS#26
File: Projects/Presentation/Sources/DatePicker/View/DatePickerView.swift:33-53
Timestamp: 2025-07-27T13:07:40.469Z
Learning: DatePickerView에서 locale을 "en_US"로 설정하는 것은 한국어 앱이지만 시간을 AM/PM 형식으로 표시하기 위한 의도적인 UX 설계 결정이다. 한국 로케일은 기본적으로 24시간 형식을 사용하기 때문이다.
Learnt from: choijungp
PR: YAPP-Github/Bitnagil-iOS#19
File: Projects/Presentation/Sources/Common/PresentationDependencyAssembler.swift:0-0
Timestamp: 2025-07-16T10:57:16.559Z
Learning: choijungp는 추후 CustomAlertView를 만들어서 에러 처리를 UI로 해결하도록 수정할 계획을 가지고 있음.
Projects/Presentation/Sources/Common/Extension/Date+.swift (1)
Learnt from: taipaise
PR: #26
File: Projects/Presentation/Sources/DatePicker/View/DatePickerView.swift:33-53
Timestamp: 2025-07-27T13:07:40.469Z
Learning: DatePickerView에서 locale을 "en_US"로 설정하는 것은 한국어 앱이지만 시간을 AM/PM 형식으로 표시하기 위한 의도적인 UX 설계 결정이다. 한국 로케일은 기본적으로 24시간 형식을 사용하기 때문이다.
Projects/Presentation/Sources/RoutineCreation/View/Component/DatePickerView.swift (1)
Learnt from: taipaise
PR: #26
File: Projects/Presentation/Sources/DatePicker/View/DatePickerView.swift:33-53
Timestamp: 2025-07-27T13:07:40.469Z
Learning: DatePickerView에서 locale을 "en_US"로 설정하는 것은 한국어 앱이지만 시간을 AM/PM 형식으로 표시하기 위한 의도적인 UX 설계 결정이다. 한국 로케일은 기본적으로 24시간 형식을 사용하기 때문이다.
Projects/Presentation/Sources/RoutineCreation/View/RoutineCreationView.swift (2)
Learnt from: taipaise
PR: #26
File: Projects/Presentation/Sources/DatePicker/View/DatePickerView.swift:33-53
Timestamp: 2025-07-27T13:07:40.469Z
Learning: DatePickerView에서 locale을 "en_US"로 설정하는 것은 한국어 앱이지만 시간을 AM/PM 형식으로 표시하기 위한 의도적인 UX 설계 결정이다. 한국 로케일은 기본적으로 24시간 형식을 사용하기 때문이다.
Learnt from: choijungp
PR: #19
File: Projects/Presentation/Sources/Common/PresentationDependencyAssembler.swift:0-0
Timestamp: 2025-07-16T10:57:16.559Z
Learning: choijungp는 추후 CustomAlertView를 만들어서 에러 처리를 UI로 해결하도록 수정할 계획을 가지고 있음.
🧬 Code Graph Analysis (2)
Projects/Presentation/Sources/RoutineCreation/View/Component/DatePickerView.swift (1)
Projects/Presentation/Sources/RoutineCreation/View/RoutineCreationView.swift (1)
datePickerView(628-630)
Projects/Presentation/Sources/RoutineCreation/View/RoutineCreationView.swift (3)
Projects/Presentation/Sources/RoutineCreation/View/Component/RoutineTimePickerButton.swift (1)
configure(64-66)Projects/Presentation/Sources/RoutineCreation/ViewModel/RoutineCreationViewModel.swift (1)
action(81-104)Projects/Presentation/Sources/Common/Extension/UIViewController+.swift (1)
presentCustomBottomSheet(79-82)
🔇 Additional comments (11)
Projects/Presentation/Sources/Common/Extension/Date+.swift (2)
25-25: 새로운 날짜 형식 케이스 추가가 적절합니다.
amPmTimeShort케이스가 잘 추가되었습니다. DatePickerView에서 선택된 시간을 간결하게 표시하는 용도로 사용되는 것으로 보입니다.
34-34: 올바른 시간 형식 문자열입니다."a h:mm" 형식은 AM/PM과 함께 시간을 간결하게 표시하는 데 적합합니다. 단일 'h'를 사용하여 앞자리 0 없이 1-12시간을 표시하는 것이 한국어 UX에 잘 맞습니다.
Projects/Presentation/Sources/RoutineCreation/View/Component/DatePickerView.swift (3)
11-13: 델리게이트 프로토콜 설계가 적절합니다.
DatePickerViewDelegate프로토콜이 잘 정의되었습니다. 메소드명과 파라미터가 명확하고, 시간 선택 결과를Date타입으로 전달하는 것이 적절합니다.
33-38: 날짜 피커 속성 설정이 올바릅니다.
en_US로케일 설정은 한국어 앱에서 AM/PM 형식을 표시하기 위한 의도적인 UX 설계 결정으로, 이전 학습 내용과 일치합니다. 휠 스타일과 시간 모드 설정도 적절합니다.
46-53: 버튼 액션 구현이 안전합니다.weak self 패턴을 사용하여 메모리 누수를 방지하고, 델리게이트 호출 후 뷰 컨트롤러를 dismiss하는 플로우가 적절합니다.
Projects/Presentation/Sources/RoutineCreation/View/RoutineCreationView.swift (6)
47-47: 바텀 시트 높이 상수 추가가 적절합니다.DatePickerView를 위한 바텀 시트 높이 상수가 Layout enum에 잘 정의되었습니다.
74-74: 변수명 개선이 잘 반영되었습니다.이전 리뷰 피드백을 반영하여
allDayButton2에서allDayLabelButton으로 변경한 것이 좋습니다. 변수명이 기능을 명확히 나타내어 코드 가독성이 향상되었습니다.
347-349: 레이블 터치 영역 확장 구현이 적절합니다.
allDayLabelButton이allDayLabel과 동일한 영역을 갖도록 제약조건을 설정한 것이 UX 향상에 도움이 됩니다.
507-513: 두 버튼에 동일한 액션 적용이 좋습니다.
allDayButton과allDayLabelButton모두에 동일한 액션을 적용하여 일관된 사용자 경험을 제공하는 것이 적절합니다.
515-521: DatePickerView 통합이 잘 구현되었습니다.커스텀 바텀 시트를 통해 DatePickerView를 표시하고 델리게이트를 설정하는 플로우가 깔끔합니다.
627-631: DatePickerViewDelegate 구현이 완벽합니다.델리게이트 메소드에서 선택된 시간을 뷰모델에 전달하는 플로우가 적절하게 구현되었습니다.

🌁 Background
📱 Screenshot
👩💻 Contents
📝 Review Note
Summary by CodeRabbit
신규 기능
개선 사항
버그 수정