Conversation
soseoyo12
requested review from
sum130,
wotjs020708 and
y-eonee
and removed request for
a team
Closed
1 task
📝 WalkthroughWalkthrough이 PR은 Splash 에셋의 메타데이터 파일을 삭제하고, CherrishPicker 컴포넌트를 수정하여 스크롤 인터랙션을 기반으로 피커 값을 실시간으로 반영하도록 개선합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ScrollView as UIScrollView
participant Coordinator as Coordinator<br/>(UIScrollViewDelegate)
participant Picker as UIPickerView
participant State as State
User->>ScrollView: 피커 드래그 시작
ScrollView->>Coordinator: scrollViewWillBeginDragging()
Coordinator->>State: 초기 상태 저장<br/>(contentOffset, isScrolling=true)
loop 스크롤 진행
User->>ScrollView: 드래그 계속
ScrollView->>Coordinator: scrollViewDidScroll()
Coordinator->>Coordinator: contentOffset 델타 계산
Coordinator->>Coordinator: 대응 행 번호 도출
Coordinator->>State: selectedRow 업데이트
Coordinator->>Picker: 선택 행 업데이트
end
User->>ScrollView: 드래그 종료
ScrollView->>Coordinator: scrollViewDidEndDragging()
Coordinator->>State: isScrolling=false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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: 3
🤖 Fix all issues with AI agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/CherrishPicker.swift`:
- Around line 46-48: 현재 findAndConnectScrollViews(in: picker, coordinator:
context.coordinator)가 UIPickerView 내부에서 UIScrollView를 찾아 연결하는데 내부 구조에 의존하므로, 찾지
못할 때의 폴백을 추가하세요: 변경할 위치는 findAndConnectScrollViews(in:coordinator:) 구현부와
호출부(DispatchQueue.main.async 블록)이며, 함수가 UIScrollView를 찾지 못하면 false를 반환하거나
completion 을 호출하도록 하고, 그 경우 UIPickerView 자체에 UIPanGestureRecognizer 또는
UITapGestureRecognizer를 추가하여 context.coordinator(예:
coordinator.handlePan/handleTap 같은 기존 핸들러)를 연결하도록 구현하세요; 이렇게 하면 내부 서브뷰 탐색에 실패할
때도 사용자 입력을 캡처할 수 있습니다.
- Line 80: There is a duplicated row height value — remove the local
constant/duplicate definition inside pickerView(_:rowHeightForComponent:) and
make that method return the single property rowHeight (private let rowHeight:
CGFloat = 44.adjustedH) so the component uses the central constant; update any
references in pickerView(_:rowHeightForComponent:) to use rowHeight and delete
the redundant duplicate.
- Around line 53-61: 현재 findAndConnectScrollViews(in:coordinator:)가 여러
UIScrollView를 순회할 때 coordinator.originalScrollViewDelegate를 덮어써 첫 발견 델리게이트를 잃을 수
있습니다; 수정 방법은 Coordinator에 원본 델리게이트를 하나가 아닌 스크롤뷰별로 저장하도록 변경하고(예:
originalScrollViewDelegates: [UIScrollView: UIScrollViewDelegate?])
findAndConnectScrollViews에서는 각 scrollView에 대해
coordinator.originalScrollViewDelegates[scrollView] = scrollView.delegate 후
scrollView.delegate = coordinator로 설정하도록 바꾸거나, 단순히 첫번째 UIScrollView를 찾은 뒤 탐색을
중단하도록 로직을 변경하여 덮어쓰기를 방지하세요; 이후 원복 로직(예: disconnect/teardown)도 Coordinator의 매핑을
사용해 각 스크롤뷰의 원본 델리게이트를 복원하도록 업데이트해야 합니다.
Comment on lines
+46
to
+48
| DispatchQueue.main.async { | ||
| self.findAndConnectScrollViews(in: picker, coordinator: context.coordinator) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
내부 UIPickerView 구조에 의존하는 구현
UIPickerView의 내부 서브뷰 계층 구조는 비공개 API이므로, iOS 버전 업데이트 시 UIScrollView를 찾지 못할 수 있습니다. 이 접근 방식은 현재 동작하지만 향후 iOS 업데이트에서 깨질 수 있으므로, 스크롤 뷰를 찾지 못하는 경우에 대한 폴백(fallback) 로직 추가를 권장합니다.
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/CherrishPicker.swift`
around lines 46 - 48, 현재 findAndConnectScrollViews(in: picker, coordinator:
context.coordinator)가 UIPickerView 내부에서 UIScrollView를 찾아 연결하는데 내부 구조에 의존하므로, 찾지
못할 때의 폴백을 추가하세요: 변경할 위치는 findAndConnectScrollViews(in:coordinator:) 구현부와
호출부(DispatchQueue.main.async 블록)이며, 함수가 UIScrollView를 찾지 못하면 false를 반환하거나
completion 을 호출하도록 하고, 그 경우 UIPickerView 자체에 UIPanGestureRecognizer 또는
UITapGestureRecognizer를 추가하여 context.coordinator(예:
coordinator.handlePan/handleTap 같은 기존 핸들러)를 연결하도록 구현하세요; 이렇게 하면 내부 서브뷰 탐색에 실패할
때도 사용자 입력을 캡처할 수 있습니다.
Comment on lines
+53
to
+61
| private func findAndConnectScrollViews(in view: UIView, coordinator: Coordinator) { | ||
| for subview in view.subviews { | ||
| if let scrollView = subview as? UIScrollView { | ||
| coordinator.originalScrollViewDelegate = scrollView.delegate | ||
| scrollView.delegate = coordinator | ||
| } | ||
| findAndConnectScrollViews(in: subview, coordinator: coordinator) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
여러 UIScrollView가 존재할 경우 원본 델리게이트 손실 가능
UIPickerView 내부에 여러 UIScrollView가 있을 경우, originalScrollViewDelegate가 마지막에 발견된 스크롤 뷰의 델리게이트로 덮어씌워집니다. 첫 번째 스크롤 뷰를 찾은 후 탐색을 중단하거나, 각 스크롤 뷰별로 원본 델리게이트를 저장하는 것이 안전합니다.
♻️ 제안된 수정사항
private func findAndConnectScrollViews(in view: UIView, coordinator: Coordinator) {
for subview in view.subviews {
if let scrollView = subview as? UIScrollView {
coordinator.originalScrollViewDelegate = scrollView.delegate
scrollView.delegate = coordinator
+ return // 첫 번째 스크롤 뷰만 연결
}
findAndConnectScrollViews(in: subview, coordinator: coordinator)
}
}🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/CherrishPicker.swift`
around lines 53 - 61, 현재 findAndConnectScrollViews(in:coordinator:)가 여러
UIScrollView를 순회할 때 coordinator.originalScrollViewDelegate를 덮어써 첫 발견 델리게이트를 잃을 수
있습니다; 수정 방법은 Coordinator에 원본 델리게이트를 하나가 아닌 스크롤뷰별로 저장하도록 변경하고(예:
originalScrollViewDelegates: [UIScrollView: UIScrollViewDelegate?])
findAndConnectScrollViews에서는 각 scrollView에 대해
coordinator.originalScrollViewDelegates[scrollView] = scrollView.delegate 후
scrollView.delegate = coordinator로 설정하도록 바꾸거나, 단순히 첫번째 UIScrollView를 찾은 뒤 탐색을
중단하도록 로직을 변경하여 덮어쓰기를 방지하세요; 이후 원복 로직(예: disconnect/teardown)도 Coordinator의 매핑을
사용해 각 스크롤뷰의 원본 델리게이트를 복원하도록 업데이트해야 합니다.
| class Coordinator: NSObject, UIPickerViewDelegate, UIPickerViewDataSource { | ||
| class Coordinator: NSObject, UIPickerViewDelegate, UIPickerViewDataSource, UIScrollViewDelegate { | ||
| var parent: PickerViewRepresentable | ||
| private let rowHeight: CGFloat = 44.adjustedH |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
행 높이 상수 중복
rowHeight가 80번째 줄과 115번째 줄(pickerView(_:rowHeightForComponent:))에서 각각 정의되어 있습니다. 하나의 상수로 통일하면 유지보수성이 향상됩니다.
♻️ 제안된 수정사항
func pickerView(_ pickerView: UIPickerView, rowHeightForComponent component: Int) -> CGFloat {
- 44.adjustedH
+ rowHeight
}🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/CherrishPicker.swift`
at line 80, There is a duplicated row height value — remove the local
constant/duplicate definition inside pickerView(_:rowHeightForComponent:) and
make that method return the single property rowHeight (private let rowHeight:
CGFloat = 44.adjustedH) so the component uses the central constant; update any
references in pickerView(_:rowHeightForComponent:) to use rowHeight and delete
the redundant duplicate.
y-eonee
approved these changes
Jan 20, 2026
Comment on lines
+55
to
+57
| if let scrollView = subview as? UIScrollView { | ||
| coordinator.originalScrollViewDelegate = scrollView.delegate | ||
| scrollView.delegate = coordinator |
| class Coordinator: NSObject, UIPickerViewDelegate, UIPickerViewDataSource { | ||
| class Coordinator: NSObject, UIPickerViewDelegate, UIPickerViewDataSource, UIScrollViewDelegate { | ||
| var parent: PickerViewRepresentable | ||
| private let rowHeight: CGFloat = 44.adjustedH |
| originalScrollViewDelegate?.scrollViewDidEndDecelerating?(scrollView) | ||
| } | ||
|
|
||
| func scrollViewWillEndDragging(_ scrollView: UIScrollView, withVelocity velocity: CGPoint, targetContentOffset: UnsafeMutablePointer<CGPoint>) { |
Comment on lines
+149
to
+153
| let deltaOffset = scrollView.contentOffset.y - initialContentOffset | ||
| let deltaRow = Int(round(deltaOffset / rowHeight)) | ||
| let newRow = initialRow + deltaRow | ||
| let clampedRow = max(0, min(newRow, parent.range.count - 1)) | ||
| let newSelection = parent.range.lowerBound + clampedRow |
wotjs020708
approved these changes
Jan 20, 2026
Contributor
wotjs020708
left a comment
wotjs020708
left a comment
There was a problem hiding this comment.
이 어려운걸.. 고생하셨습니다 리뷰 확인 부탁드려욤~
sum130
approved these changes
Jan 20, 2026
Contributor
sum130
left a comment
sum130
left a comment
There was a problem hiding this comment.
어려운디 수고했어용 감자합니다~~~ 어푸푸!!
Comment on lines
+149
to
+153
| let deltaOffset = scrollView.contentOffset.y - initialContentOffset | ||
| let deltaRow = Int(round(deltaOffset / rowHeight)) | ||
| let newRow = initialRow + deltaRow | ||
| let clampedRow = max(0, min(newRow, parent.range.count - 1)) | ||
| let newSelection = parent.range.lowerBound + clampedRow |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
🔗 연결된 이슈
📄 작업 내용
💻 주요 코드 설명
실시간 감지를 위해 내부 스크롤 뷰를 찾아내서 델리게이트를 가로채는 과정이 추가함으로써 해결했습니다.
if let scrollView = subview as? UIScrollView:UIPickerView는 겉으로는 피커지만, 내부적으로는UIScrollView(또는UITableView)로 구성되어 있습니다. 이 숨겨진 스크롤 뷰를 찾아냅니다.scrollView.delegate = context.coordinator: 원래는 피커 내부 로직이 스크롤을 관리하지만, 스크롤 되는 순간을 포착하기 위해 Coordinator가 스크롤 델리게이트 권한을 가져옵니다. 이렇게 해야scrollViewDidScroll이벤트를 받을 수 있습니다.기존 코드 (
didSelectRow)실시간 코드 (
scrollViewDidScroll)근데….. 이렇게 했을때 실시간 동기화는 해결했지만 UIPicker의 delegate를 뻇어왔기 때문에 손을 놨을때 자동으로 가까운 숫자로 가는 “스냅” 기능이 비활성화되는 문제가 발생했습니다.
그래서,,,,,,
스냅 기능을 아예 만들기….