-
Notifications
You must be signed in to change notification settings - Fork 2
[REFACTOR/#357] 오버레이 튜토리얼 화면 ui 간격 수정 #360
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
base: dev
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded@sehyeo has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks✅ Passed checks (5 passed)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/res/layout/tutorial_overlay.xml (1)
38-47: 접근성: 닫기 버튼에 contentDescription 추가 필요.
contentDescription="@null"은 스크린 리더 사용자가 이 버튼을 인식할 수 없게 합니다. 튜토리얼 닫기 버튼이므로 적절한 설명을 추가하세요.🔎 제안된 수정
- android:contentDescription="@null" + android:contentDescription="@string/tutorial_close_button_desc"
strings.xml에 해당 문자열 리소스를 추가하세요 (예: "튜토리얼 닫기").
🧹 Nitpick comments (4)
app/src/main/res/layout/tutorial_overlay.xml (4)
17-20: 중첩된 ConstraintLayout 불필요.
contentConstraintLayout은 부모와 동일한 크기(match_parent)로 설정되어 있어 추가적인 레이아웃 계층을 도입하는 것 외에 역할이 없습니다.scrimView와 나머지 자식 뷰들을 루트 ConstraintLayout에 직접 배치하면 중첩을 줄이고 레이아웃 성능을 개선할 수 있습니다.🔎 제안된 수정
- <androidx.constraintlayout.widget.ConstraintLayout - android:id="@+id/content" - android:layout_width="match_parent" - android:layout_height="match_parent"> + <!-- content 래퍼 제거하고 자식 뷰들을 루트에 직접 배치 -->자식 뷰들의 제약 조건을 루트
tutorialOverlay를 기준으로 조정하고, 파일 끝의 닫는 태그도 함께 제거하세요.
111-123: 음수 마진 사용 주의.
android:layout_marginTop="-15dp"은 다양한 화면 크기와 해상도에서 예상치 못한 클리핑이나 레이아웃 문제를 유발할 수 있습니다. ConstraintLayout에서는 제약 조건을 조정하거나translationY를 사용하는 것이 더 안정적입니다.🔎 대안 제안
- android:layout_marginTop="-15dp" + android:translationY="-15dp"또는
dashRightBottom의 위치를 조정하여labelBottom이 음수 오프셋 없이 원하는 위치에 배치되도록 제약 조건을 재설계하는 것을 고려하세요.
210-233: 단일 자식을 가진 LinearLayout 단순화 고려.
hintCalendar와hintTodoLinearLayout은 각각 하나의 TextView만 포함합니다. 배경 스타일을 TextView에 직접 적용하고 래퍼를 제거하면 뷰 계층을 줄일 수 있습니다.🔎 제안된 수정 (hintCalendar 예시)
- <LinearLayout - android:id="@+id/hintCalendar" - android:layout_width="0dp" - android:layout_height="170dp" - android:layout_marginStart="17dp" - android:layout_marginEnd="17dp" - android:layout_marginTop="45dp" - android:background="@drawable/bg_hint_card" - app:layout_constraintTop_toBottomOf="@id/spotlightGroup" - app:layout_constraintStart_toStartOf="parent" - app:layout_constraintEnd_toEndOf="parent"> - - <TextView - android:id="@+id/labelCalendar" - android:layout_width="match_parent" - android:layout_height="wrap_content" - android:layout_marginTop="10dp" - android:layout_marginEnd="14dp" - ... /> - </LinearLayout> + <TextView + android:id="@+id/labelCalendar" + android:layout_width="0dp" + android:layout_height="170dp" + android:layout_marginStart="17dp" + android:layout_marginEnd="17dp" + android:layout_marginTop="45dp" + android:paddingTop="10dp" + android:paddingEnd="14dp" + android:background="@drawable/bg_hint_card" + app:layout_constraintTop_toBottomOf="@id/spotlightGroup" + app:layout_constraintStart_toStartOf="parent" + app:layout_constraintEnd_toEndOf="parent" + ... />
2-9: 미리보기용tools:속성 추가 권장.루트 레이아웃에
android:visibility="gone"이 설정되어 있어 Android Studio 미리보기에서 레이아웃을 확인하기 어렵습니다. 코딩 가이드라인에 따라tools:속성을 활용하면 디자인 타임 프리뷰가 가능합니다.🔎 제안된 수정
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android" xmlns:app="http://schemas.android.com/apk/res-auto" + xmlns:tools="http://schemas.android.com/tools" android:id="@+id/tutorialOverlay" android:layout_width="match_parent" android:layout_height="match_parent" android:clickable="true" android:focusable="true" - android:visibility="gone"> + android:visibility="gone" + tools:visibility="visible">
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/res/layout/tutorial_overlay.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/tutorial_overlay.xml
🔇 Additional comments (1)
app/src/main/res/layout/tutorial_overlay.xml (1)
22-36: LGTM!
spotlightGroup과 내부sample_timetable_iv의 제약 조건 구성이 적절합니다. FrameLayout 래퍼는 향후 뷰 추가나 그룹 애니메이션을 위해 유지하는 것이 합리적입니다.
🔗 관련 이슈 (선택)
closes #357
✨ 작업 내용
✅ 코드 리뷰어 체크리스트
리뷰어가 확인해야할 부분
📸 스크린샷 (선택)
💬 기타 참고 사항
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.