[Fix] 감정등록 화면 QA 대응 (무한 스크롤 끊김, 스크롤 가능 영역 확장)#79
Merged
Conversation
- 구슬 collectionView 하단의 안내 문구에서도 collectionView의 스크롤이 가능하도록 수정 - 무한 스크롤이 끊기는 현상 개선 - 구슬 스크롤 시 진동 피드백으로 사용자경험 향상
|
Caution Review failedThe pull request is closed. 개요감정 컬렉션 레이아웃에 Layout 열거형과 fetchcurrentIndex()를 추가하고, EmotionRegistrationViewController에서 컬렉션뷰 높이·아이템 집합 크기 변경(3→7), 제스처 처리 및 햅틱, infoSwipeOverlayView 통합과 스크롤 관련 로직을 갱신했습니다. (≤50단어) 변경 사항
예상 코드 리뷰 노력🎯 3 (중간) | ⏱️ ~20분 관련 가능성 있는 PR들
시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 2
🤖 Fix all issues with AI agents
In
`@Projects/Presentation/Sources/EmotionRegister/View/EmotionRegistrationViewController.swift`:
- Around line 515-517: The current recenter condition wrongly uses itemCount /
itemSetCount and triggers for most indices; update the check in
EmotionRegistrationViewController so recentering only happens when the
tapped/visible index is in the very first or very last repeated set: use if
indexPath.row < itemCount || indexPath.row >= itemCount * (itemSetCount - 1) {
scrollToIndex(index: index) } (use the existing index variable and the itemCount
and itemSetCount symbols) so only the first set (indices < itemCount) or the
last set (indices >= itemCount * (itemSetCount - 1)) cause scrollToIndex to be
called.
- Around line 131-134: infoSwipeOverlayView에 대해 중복된 backgroundColor 설정과 사용되지 않는
변수 infoSwipeOverlayViewPanGesturePanGesture를 제거하고, infoSwipeOverlayView의 의도(터치
차단용 오버레이인지 제스처 수신용인지)를 명확히 하여 처리하세요: 만약 터치 차단 목적이면 isUserInteractionEnabled =
true로 두고 실제 터치 처리(예: 빈 UITapGestureRecognizer 또는 hitTest override)를 추가하여 터치를
블록하도록 구현하고, 제스처가 불필요하면 모든 제스처 관련 코드(예:
infoSwipeOverlayViewPanGesturePanGesture)와 isUserInteractionEnabled 설정을 false로
정리하세요; 또한 Layout.infoViewSize가 0으로 되어 있는 점을 주석으로 명시해 의도를 설명하거나 크기 변경 시 동작을
재검증하세요.
🧹 Nitpick comments (1)
Projects/Presentation/Sources/EmotionRegister/View/EmotionRegistrationViewController.swift (1)
287-304: itemSetCount 상수 활용 + 디버그 출력 정리 권장
하드코딩된7은 상수 변경 시 누락 위험이 있고,♻️ 정리 제안
- let multipliedEmotions = (0..<7).flatMap { i in + let multipliedEmotions = (0..<itemSetCount).flatMap { i in i == centerIndex ? emotions : emotions.map { $0.copy() } } @@ - print(lastMarbleCellIndex)
Projects/Presentation/Sources/EmotionRegister/View/EmotionRegistrationViewController.swift
Show resolved
Hide resolved
Comment on lines
+515
to
517
| if indexPath.row < itemCount / itemSetCount || indexPath.row >= itemCount * (itemSetCount / 2 + 1) / itemSetCount { | ||
| scrollToIndex(index: index) | ||
| } |
There was a problem hiding this comment.
재센터 조건이 대부분의 인덱스에서 항상 참이 됩니다.
itemCount / itemSetCount 기준은 임계값이 너무 작아 대부분의 스크롤에서 scrollToIndex가 호출될 수 있습니다. 총 아이템 수 기준으로 첫/마지막 세트에서만 재센터하도록 조정하는 편이 안전합니다.
🛠️ 수정 제안
- if indexPath.row < itemCount / itemSetCount || indexPath.row >= itemCount * (itemSetCount / 2 + 1) / itemSetCount {
+ let totalItemCount = itemCount * itemSetCount
+ if indexPath.row < itemCount || indexPath.row >= totalItemCount - itemCount {
scrollToIndex(index: index)
}📝 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
| if indexPath.row < itemCount / itemSetCount || indexPath.row >= itemCount * (itemSetCount / 2 + 1) / itemSetCount { | |
| scrollToIndex(index: index) | |
| } | |
| let totalItemCount = itemCount * itemSetCount | |
| if indexPath.row < itemCount || indexPath.row >= totalItemCount - itemCount { | |
| scrollToIndex(index: index) | |
| } |
🤖 Prompt for AI Agents
In
`@Projects/Presentation/Sources/EmotionRegister/View/EmotionRegistrationViewController.swift`
around lines 515 - 517, The current recenter condition wrongly uses itemCount /
itemSetCount and triggers for most indices; update the check in
EmotionRegistrationViewController so recentering only happens when the
tapped/visible index is in the very first or very last repeated set: use if
indexPath.row < itemCount || indexPath.row >= itemCount * (itemSetCount - 1) {
scrollToIndex(index: index) } (use the existing index variable and the itemCount
and itemSetCount symbols) so only the first set (indices < itemCount) or the
last set (indices >= itemCount * (itemSetCount - 1)) cause scrollToIndex to be
called.
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
1 participant
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.
🌁 Background
📱 Screenshot
👩💻 Contents
📬 Reference
Summary by CodeRabbit
릴리즈 노트
새로운 기능
개선 사항
✏️ Tip: You can customize this high-level summary in your review settings.