-
Notifications
You must be signed in to change notification settings - Fork 2
[REFACTOR/#366] 전체 화면 스크롤되게 UI 수정하기 #367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthrough레이아웃을 NestedScrollView로 변경해 전체 화면 스크롤을 지원하도록 하고, Fragment에서 RecyclerView 초기화 순서를 앞당겨 setupRecyclerView()를 먼저 호출하도록 조정했으며, 캘린더 월 변경 시 RecyclerView를 숨기고 어댑터 목록을 초기화하도록 추가했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @app/src/main/res/layout/fragment_friend_teum_request.xml:
- Around line 28-40: The TextView with id textView2 uses
android:textSize="20dp"; change the unit to sp so system font scaling is
respected by replacing the dp unit with sp (e.g., set android:textSize="20sp")
on the TextView with id textView2 and ensure no other textSize attributes nearby
use dp.
🧹 Nitpick comments (1)
app/src/main/res/layout/fragment_friend_teum_request.xml (1)
128-142: RecyclerView의 중첩 스크롤 설정이 올바릅니다.Line 138의
nestedScrollingEnabled="false"설정은 NestedScrollView 내부에서 RecyclerView가 올바르게 작동하도록 하는 필수 설정입니다. 이를 통해 부모와 자식 간의 스크롤 충돌을 방지합니다.Line 137에서
visibility="visible"로 설정되어 있지만, Fragment 코드(line 260)에서는 초기에isVisible = false로 설정하고 있습니다. XML에서도android:visibility="gone"으로 설정하면 더 일관성 있는 코드가 됩니다.♻️ 선택적 개선 사항
- android:visibility="visible" + android:visibility="gone"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/example/teumteum/ui/friend/FriendTeumRequestFragment.ktapp/src/main/res/layout/fragment_friend_teum_request.xml
🧰 Additional context used
📓 Path-based instructions (2)
app/src/**/ui/**
⚙️ CodeRabbit configuration file
app/src/**/ui/**: [UI(Kotlin) 점검]
- ViewBinding 생명주기: Fragment는 onDestroyView()에서 binding=null 처리, 외부에 뷰 참조 보관 금지.
- 수명주기 안전 수집: repeatOnLifecycle + viewLifecycleOwner 사용, GlobalScope 금지, cancel 처리 확인.
- 리스트/어댑터: DiffUtil 사용, notifyDataSetChanged 지양, ViewHolder 바인딩 최소화.
- 상태 반영: 중복 네비게이션/중복 클릭 방지, 단발성 이벤트는 SharedFlow/Channel로 처리.
Files:
app/src/main/java/com/example/teumteum/ui/friend/FriendTeumRequestFragment.kt
app/src/main/res/layout/**/*.xml
⚙️ CodeRabbit configuration file
app/src/main/res/layout/**/*.xml: [Layout 점검]
- 구조: include/merge 적절 활용으로 중첩 최소화, 재사용성 확보.
- ConstraintLayout: 제약 조건 충돌/경고 여부, 체인/가이드라인/Barrier 등으로 가로세로 정렬 일관성.
- 성능: 중첩 뷰 최소화, 불필요한 weights 제거, list item 레이아웃은 경량 설계.
- 미리보기용 tools: 속성 적극 활용(visibility, text 등), runtime 빌드에 영향 없게 유지.
- 이미지: 벡터 드로어블 우선, 큰 비트맵은 적절한 크기/압축 사용.
Files:
app/src/main/res/layout/fragment_friend_teum_request.xml
🧬 Code graph analysis (1)
app/src/main/java/com/example/teumteum/ui/friend/FriendTeumRequestFragment.kt (4)
app/src/main/java/com/example/teumteum/ui/friend/FriendTodoListFragment.kt (2)
setupRecyclerView(238-242)circleFill(268-273)app/src/main/java/com/example/teumteum/ui/friend/FriendPromiseFragment.kt (2)
setupRecyclerView(239-245)circleFill(282-287)app/src/main/java/com/example/teumteum/ui/main/HomeFragment.kt (1)
circleFill(846-851)app/src/main/java/com/example/teumteum/ui/friend/FriendRoommateDateFragment.kt (1)
circleFill(296-301)
🔇 Additional comments (4)
app/src/main/java/com/example/teumteum/ui/friend/FriendTeumRequestFragment.kt (3)
78-78: 초기화 순서 변경이 적절합니다.RecyclerView 설정을 캘린더 설정보다 먼저 호출하도록 순서를 변경한 것은 전체 화면 스크롤 구조에 맞게 리팩토링한 것으로 보입니다. 두 컴포넌트 간 의존성이 없으므로 안전한 변경입니다.
136-138: 월 변경 시 리스트 초기화 로직이 좋습니다.달이 변경될 때 RecyclerView를 숨기고 기존 데이터를 비우는 것은 사용자가 이전 월의 데이터를 보지 않도록 하는 좋은 UX 패턴입니다.
submitList(emptyList())를 사용하여 DiffUtil을 통해 효율적으로 처리하고 있습니다.
256-261: RecyclerView 설정이 올바릅니다.RecyclerView 초기화 로직이 다른 Fragment들(FriendTodoListFragment, FriendPromiseFragment)과 일관된 패턴을 따르고 있으며, 초기에는 숨김 처리하고 데이터가 있을 때만 표시하는 방식이 적절합니다.
app/src/main/res/layout/fragment_friend_teum_request.xml (1)
2-9: NestedScrollView 설정이 전체 화면 스크롤에 적합합니다.루트 레이아웃을 NestedScrollView로 변경하고
fillViewport="true"및overScrollMode="never"속성을 추가한 것은 PR 목표인 전체 화면 스크롤을 구현하기에 적절한 구조입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @app/src/main/res/layout/fragment_friend_teum_request.xml:
- Around line 53-83: The previous/next calendar ImageViews
(calendar_previous_date_iv and calendar_next_date_iv) lack accessibility and
proper touch feedback; replace them with ImageButton or make them accessible by
adding meaningful contentDescription attributes, set clickable="true" and
focusable="true", ensure a visible touch/ripple feedback (e.g., use
?attr/selectableItemBackgroundBorderless as foreground/background) and guarantee
minimum touch target (minWidth/minHeight or padding to reach 48dp). Update the
layout for both calendar_previous_date_iv and calendar_next_date_iv accordingly
so screen readers announce their purpose and users get visual/touch feedback.
- Around line 127-143: The RecyclerView with id requestHistoryRecyclerView uses
wrap_content and nestedScrollingEnabled="false" which is acceptable for
single-date loads via getTeumRequestsByDate(date) but may suffer if one date
returns many items; update the adapter for requestHistoryRecyclerView to use a
DiffUtil.ItemCallback and ListAdapter (or implement DiffUtil.DiffResult updates)
so updates are efficient for large single-day result sets, and keep
nestedScrollingEnabled as needed or switch to match_parent if layout/scroll
behavior requires it.
🧹 Nitpick comments (3)
app/src/main/res/layout/fragment_friend_teum_request.xml (3)
2-13: 하드코딩 색상(#FFFFFF) 대신 테마/리소스 사용 권장다크모드/테마 대응을 위해
android:background="#FFFFFF"는?attr/colorBackground또는@color/...로 교체하는 게 안전합니다. (구분선#EAEAEA도 동일)
15-40: 상단 여백(70dp) 하드코딩은 인셋 환경에서 깨질 수 있음 + 문자열 리소스화 권장
layout_marginTop="70dp"는 상태바/노치/폰트 스케일에 따라 어긋날 수 있어, 인셋 기반 처리(또는 공통 툴바/앱바 사용)로 맞추는 쪽이 안정적입니다. 또한android:text,android:contentDescription의 리터럴은@string/...로 빼는 게 좋습니다.
53-65: 런타임 텍스트/색상 하드코딩은 tools 속성/리소스로 분리 권장
selected_date_tv의"2025년 6월"은 실제 값이 코드에서 세팅되는 성격이면android:text대신tools:text로 미리보기만 주는 편이 안전합니다. 구분선 색상도 리소스화 권장입니다.Also applies to: 115-125
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/res/layout/fragment_friend_teum_request.xml
🧰 Additional context used
📓 Path-based instructions (1)
app/src/main/res/layout/**/*.xml
⚙️ CodeRabbit configuration file
app/src/main/res/layout/**/*.xml: [Layout 점검]
- 구조: include/merge 적절 활용으로 중첩 최소화, 재사용성 확보.
- ConstraintLayout: 제약 조건 충돌/경고 여부, 체인/가이드라인/Barrier 등으로 가로세로 정렬 일관성.
- 성능: 중첩 뷰 최소화, 불필요한 weights 제거, list item 레이아웃은 경량 설계.
- 미리보기용 tools: 속성 적극 활용(visibility, text 등), runtime 빌드에 영향 없게 유지.
- 이미지: 벡터 드로어블 우선, 큰 비트맵은 적절한 크기/압축 사용.
Files:
app/src/main/res/layout/fragment_friend_teum_request.xml
| <TextView | ||
| android:id="@+id/selected_date_tv" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginStart="20dp" | ||
| android:layout_marginTop="38dp" | ||
| android:fontFamily="@font/pretendard_medium" | ||
| android:text="2025년 6월" | ||
| android:textColor="@color/text_primary" | ||
| android:textSize="20sp" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toTopOf="parent" /> | ||
|
|
||
| <LinearLayout | ||
| android:id="@+id/calendar_weekdays_row" | ||
| android:layout_marginStart="5dp" | ||
| android:layout_marginEnd="5dp" | ||
| android:layout_marginTop="20dp" | ||
| <ImageView | ||
| android:id="@+id/calendar_previous_date_iv" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginEnd="20dp" | ||
| android:src="@drawable/previous" | ||
| app:layout_constraintEnd_toStartOf="@id/calendar_next_date_iv" | ||
| app:layout_constraintTop_toTopOf="@id/selected_date_tv" /> | ||
|
|
||
| <ImageView | ||
| android:id="@+id/calendar_next_date_iv" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginEnd="20dp" | ||
| android:src="@drawable/next" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| app:layout_constraintTop_toTopOf="@id/selected_date_tv" /> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
캘린더 이전/다음 버튼 접근성(a11y) 누락 + 클릭 피드백/터치 타깃 보장 필요
calendar_previous_date_iv, calendar_next_date_iv에 contentDescription이 없고 ImageView라 클릭 요소로 인지되지 않을 수 있습니다(코드에서 클릭을 달아도 a11y가 약해짐). ImageButton으로 전환하거나 최소한 clickable/focusable/foreground 및 contentDescription을 추가해 주세요.
Proposed diff
- <ImageView
+ <ImageButton
android:id="@+id/calendar_previous_date_iv"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginEnd="20dp"
android:src="@drawable/previous"
+ android:background="?attr/selectableItemBackgroundBorderless"
+ android:contentDescription="@string/cd_calendar_prev_month"
+ android:padding="8dp"
app:layout_constraintEnd_toStartOf="@id/calendar_next_date_iv"
app:layout_constraintTop_toTopOf="@id/selected_date_tv" />
- <ImageView
+ <ImageButton
android:id="@+id/calendar_next_date_iv"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginEnd="20dp"
android:src="@drawable/next"
+ android:background="?attr/selectableItemBackgroundBorderless"
+ android:contentDescription="@string/cd_calendar_next_month"
+ android:padding="8dp"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toTopOf="@id/selected_date_tv" />Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @app/src/main/res/layout/fragment_friend_teum_request.xml around lines 53 -
83, The previous/next calendar ImageViews (calendar_previous_date_iv and
calendar_next_date_iv) lack accessibility and proper touch feedback; replace
them with ImageButton or make them accessible by adding meaningful
contentDescription attributes, set clickable="true" and focusable="true", ensure
a visible touch/ripple feedback (e.g., use
?attr/selectableItemBackgroundBorderless as foreground/background) and guarantee
minimum touch target (minWidth/minHeight or padding to reach 48dp). Update the
layout for both calendar_previous_date_iv and calendar_next_date_iv accordingly
so screen readers announce their purpose and users get visual/touch feedback.
| <!-- RecyclerView: 틈 요청 기록 리스트 --> | ||
| <androidx.recyclerview.widget.RecyclerView | ||
| android:id="@+id/requestHistoryRecyclerView" | ||
| android:layout_width="0dp" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginTop="5dp" | ||
| app:cv_dayViewResource="@layout/calendar_day_layout" | ||
| app:cv_orientation="horizontal" | ||
| app:cv_outDateStyle="endOfRow" | ||
| android:layout_marginStart="20dp" | ||
| android:layout_marginEnd="20dp" | ||
| android:layout_marginTop="38dp" | ||
| android:clipToPadding="false" | ||
| android:paddingBottom="24dp" | ||
| android:visibility="visible" | ||
| android:nestedScrollingEnabled="false" | ||
| app:layout_constraintTop_toBottomOf="@id/divider2" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| app:layout_constraintTop_toBottomOf="@id/calendar_weekdays_row" /> | ||
| tools:listitem="@layout/item_request_history" /> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd "fragment_friend_teum_request.xml" --type fRepository: UMC8-TeumTeum/FE
Length of output: 116
🏁 Script executed:
cat -n app/src/main/res/layout/fragment_friend_teum_request.xml | head -160Repository: UMC8-TeumTeum/FE
Length of output: 7566
🏁 Script executed:
rg "requestHistoryRecyclerView|RequestHistoryAdapter" --type java --type kotlin -A 5Repository: UMC8-TeumTeum/FE
Length of output: 2748
🏁 Script executed:
rg "NestedScrollView" app/src/main/res/layout/fragment_friend_teum_request.xml -B 5 -A 10Repository: UMC8-TeumTeum/FE
Length of output: 803
🏁 Script executed:
rg "TeumRequestAdapter|RequestHistoryAdapter" --type java --type kotlin -lRepository: UMC8-TeumTeum/FE
Length of output: 216
🏁 Script executed:
rg "class.*Adapter.*TeumRequest|class.*Adapter.*RequestHistory" --type java --type kotlin -A 10Repository: UMC8-TeumTeum/FE
Length of output: 42
🏁 Script executed:
rg "submitList|TeumRequestDateResult" --type java --type kotlin -B 3 -A 3Repository: UMC8-TeumTeum/FE
Length of output: 20878
🏁 Script executed:
cat -n app/src/main/java/com/example/teumteum/ui/friend/adapter/TeumRequestAdapter.ktRepository: UMC8-TeumTeum/FE
Length of output: 7489
🏁 Script executed:
cat -n app/src/main/java/com/example/teumteum/ui/friend/FriendTeumRequestFragment.kt | grep -A 20 "loadTeumRequestsByDate"Repository: UMC8-TeumTeum/FE
Length of output: 840
RecyclerView의 wrap_content 크기 설정은 최적이 아니나, 일일 단위 데이터 로딩이 성능 리스크를 완화함
현재 구조에서는 캘린더에서 날짜를 선택할 때마다 해당 날짜의 요청만 API로 조회하므로(getTeumRequestsByDate(date)), 전체 달의 데이터를 한 번에 로드하는 상황이 아닙니다. 따라서 wrap_content + nestedScrollingEnabled=false의 성능 영향이 월간 대량 데이터보다는 제한적입니다.
다만 개선 가능한 점:
- 단일 날짜에 요청이 많은 경우에 대비해
DiffUtil기반 어댑터 업데이트 사용 검토 - 단일 날짜 요청 개수가 보장된다면 현재 구조는 수용 가능
🤖 Prompt for AI Agents
In @app/src/main/res/layout/fragment_friend_teum_request.xml around lines 127 -
143, The RecyclerView with id requestHistoryRecyclerView uses wrap_content and
nestedScrollingEnabled="false" which is acceptable for single-date loads via
getTeumRequestsByDate(date) but may suffer if one date returns many items;
update the adapter for requestHistoryRecyclerView to use a DiffUtil.ItemCallback
and ListAdapter (or implement DiffUtil.DiffResult updates) so updates are
efficient for large single-day result sets, and keep nestedScrollingEnabled as
needed or switch to match_parent if layout/scroll behavior requires it.
🔗 관련 이슈 (선택)
closes #366
✨ 작업 내용
✅ 코드 리뷰어 체크리스트
리뷰어가 확인해야할 부분
📸 스크린샷 (선택)
💬 기타 참고 사항
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.