-
Notifications
You must be signed in to change notification settings - Fork 0
fix: 타유저 서재 뷰 상태바 패딩 적용 #745
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
WalkthroughUserStorageActivity를 AppCompatActivity에서 ComponentActivity로 변경하고 Compose 기반 Scaffold로 UI를 구성하면서 시스템바 인셋 패딩을 적용했습니다. LibraryPagingSource의 getRefreshKey가 고정값(0L) 반환으로 변경되었고, Rating enum에 0.5~3.0 구간의 세부 단계가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Activity as UserStorageActivity
participant Compose as Scaffold/Box
participant Screen as LibraryScreen
User->>Activity: Start Activity
Activity->>Compose: setContent(WebsosoTheme + Scaffold)
Compose->>Compose: systemBars insets padding 적용
Compose->>Screen: LibraryScreen(navigate lambdas)
sequenceDiagram
participant UI as UI/Pager
participant PS as LibraryPagingSource
UI->>PS: refresh()
PS-->>UI: getRefreshKey() = DEFAULT_LAST_USER_NOVEL_ID(0L)
UI->>PS: load(key=0L)
PS-->>UI: Page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code Graph Analysis (1)app/src/main/java/com/into/websoso/ui/userStorage/UserStorageActivity.kt (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 4
🧹 Nitpick comments (3)
data/library/src/main/java/com/into/websoso/data/library/paging/LibraryPagingSource.kt (1)
36-36: 리프레시 시작 키가 항상 0L로 설정됨 → 의도 여부 및 UX 영향 확인 필요현재
LibraryPagingSource#getRefreshKey가 항상DEFAULT_LAST_USER_NOVEL_ID(0L)를 반환하여 새로고침 시 무조건 첫 페이지부터 로드됩니다.
대용량 데이터에서 불필요한 네트워크 호출, 레이턴시 증가, 스크롤 위치 복원 실패 등의 UX 저하가 발생할 수 있으니, 변경 의도를 검토해 주세요.점검된 위치:
- data/library/src/main/java/com/into/websoso/data/library/paging/LibraryPagingSource.kt:36
override fun getRefreshKey(state: PagingState<Long, NovelEntity>): Long = DEFAULT_LAST_USER_NOVEL_ID- core/database/src/main/java/com/into/websoso/core/database/datasource/library/paging/MapValuePagingSource.kt:34
override fun getRefreshKey(state: PagingState<Key, To>): Key? = targetSource.getRefreshKey(...)앵커 기반 리프레시 키 복원 예시:
override fun getRefreshKey(state: PagingState<Long, NovelEntity>): Long { val anchorPos = state.anchorPosition ?: return DEFAULT_LAST_USER_NOVEL_ID return state.closestItemToPosition(anchorPos)?.userNovelId ?: DEFAULT_LAST_USER_NOVEL_ID }– 의도된 동작인지, 아니면 위 예시처럼 앵커 기반 복원이 필요한지 확인 부탁드립니다.
domain/library/src/main/java/com/into/websoso/domain/library/model/Rating.kt (1)
22-25: Kotlin 1.9 미만 호환성 검증 필요
entriesAPI는 Kotlin 1.9 이상에서만 안정화되었습니다. 프로젝트가 1.9 미만을 사용 중이라면 빌드 오류가 발생할 수 있으므로 아래 사항을 확인해주세요.
- Gradle/Kotlin 플러그인 버전이 1.9 이상인지 검증
- 만약 하위 호환성 유지가 필요하다면
values()호출로 대체 고려- 새 enum 상수 삽입으로
ordinal값이 변경될 수 있으니, 영속화/직렬화 로직에 미치는 영향 검토File:
domain/library/src/main/java/com/into/websoso/domain/library/model/Rating.kt
Lines: 22–25- fun from(value: Float): Rating = - entries.find { + fun from(value: Float): Rating = + values().find { it.value.isCloseTo(value) } ?: DEFAULTapp/src/main/java/com/into/websoso/ui/userStorage/UserStorageActivity.kt (1)
39-39: 요구사항 확인: “상태바 패딩”만 필요하다면 systemBars 대신 statusBars 사용 권장이슈/PR 제목은 상태바 패딩을 지칭합니다. 현재 WindowInsets.systemBars는 네비게이션 바까지 포함해 하단에도 패딩이 들어갑니다. 의도대로 상단만 필요하다면 아래 중 하나로 조정하세요.
- 확장 함수 사용:
- .windowInsetsPadding(WindowInsets.systemBars) + .statusBarsPadding()
- WindowInsets를 명시:
- .windowInsetsPadding(WindowInsets.systemBars) + .windowInsetsPadding(WindowInsets.statusBars)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/into/websoso/ui/userStorage/UserStorageActivity.kt(2 hunks)data/library/src/main/java/com/into/websoso/data/library/paging/LibraryPagingSource.kt(1 hunks)domain/library/src/main/java/com/into/websoso/domain/library/model/Rating.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/main/java/com/into/websoso/ui/userStorage/UserStorageActivity.kt (1)
feature/library/src/main/java/com/into/websoso/feature/library/LibraryScreen.kt (2)
LibraryScreen(48-113)LibraryScreen(115-208)
🔇 Additional comments (2)
domain/library/src/main/java/com/into/websoso/domain/library/model/Rating.kt (1)
9-14: Rating.enum의 ordinal 변경 영향 없음 확인확인 결과
Rating.ordinal은 영속화(Room/SharedPrefs)나 네트워크 직렬화에 사용되지 않으며, 별도의TypeConverter나@Serializable어노테이션도 존재하지 않습니다. 따라서 중간 상수 삽입으로 인한ordinal변경이 다른 로직에 미치는 영향은 없습니다.app/src/main/java/com/into/websoso/ui/userStorage/UserStorageActivity.kt (1)
42-53: LibraryScreen/네비게이션 람다 연결은 적절합니다ViewModel 주입과 네비게이션 람다 위임 모두 자연스럽고 유지보수에 용이합니다.
app/src/main/java/com/into/websoso/ui/userStorage/UserStorageActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/into/websoso/ui/userStorage/UserStorageActivity.kt
Show resolved
Hide resolved
| Scaffold( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .windowInsetsPadding(WindowInsets.systemBars), | ||
| ) { inner -> | ||
| Box(modifier = Modifier.padding(inner)) { | ||
| LibraryScreen( | ||
| libraryViewModel = libraryViewModel, |
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.
🛠️ Refactor suggestion
Scaffold 기본 인셋 + windowInsetsPadding 동시 적용으로 중복 패딩 가능성
Material3 Scaffold는 기본적으로 contentWindowInsets를 통해 인셋을 컨텐츠 패딩(inner)로 전달합니다. 현재는
- Modifier.windowInsetsPadding(WindowInsets.systemBars) +
- Box(modifier = Modifier.padding(inner))
가 함께 적용되어 상하(특히 상단) 패딩이 두 번 들어갈 수 있습니다. 하나만 사용하세요.
권장안 A(간단): windowInsetsPadding 제거
- Scaffold(
- modifier = Modifier
- .fillMaxSize()
- .windowInsetsPadding(WindowInsets.systemBars),
- ) { inner ->
+ Scaffold(
+ modifier = Modifier
+ .fillMaxSize(),
+ ) { inner ->대안 B(명시적 인셋 제어): windowInsetsPadding을 유지하고, Scaffold의 contentWindowInsets를 0으로 설정
- Scaffold(
+ Scaffold(
+ contentWindowInsets = WindowInsets(0),
modifier = Modifier
.fillMaxSize()
.windowInsetsPadding(WindowInsets.systemBars),
) { inner ->📝 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.
| Scaffold( | |
| modifier = Modifier | |
| .fillMaxSize() | |
| .windowInsetsPadding(WindowInsets.systemBars), | |
| ) { inner -> | |
| Box(modifier = Modifier.padding(inner)) { | |
| LibraryScreen( | |
| libraryViewModel = libraryViewModel, | |
| Scaffold( | |
| modifier = Modifier | |
| .fillMaxSize(), | |
| ) { inner -> | |
| Box(modifier = Modifier.padding(inner)) { | |
| LibraryScreen( | |
| libraryViewModel = libraryViewModel, | |
| // … | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/into/websoso/ui/userStorage/UserStorageActivity.kt
around lines 36 to 43, Scaffold is supplying contentWindowInsets to the inner
content and you also apply Modifier.windowInsetsPadding(WindowInsets.systemBars)
on the Scaffold, causing duplicate top/bottom padding; remove the duplicate by
either A) deleting the Modifier.windowInsetsPadding(WindowInsets.systemBars) so
Scaffold's passed inner padding (used by Box(modifier =
Modifier.padding(inner))) is the sole inset source, or B) keep the
windowInsetsPadding but explicitly disable Scaffold's contentWindowInsets (set
it to zero) so the Box padding(inner) no longer double-counts—pick one approach
and remove the other to eliminate the duplicated inset.
📌𝘐𝘴𝘴𝘶𝘦𝘴
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
.windowInsetsPadding(WindowInsets.systemBars)지정해줬습니다.Summary by CodeRabbit