[Feat] 온보딩 UI 개편#201
Merged
Hidden character warning
The head ref may contain hidden characters: "FLT-12-\uc628\ubcf4\ub529-UI-\uac1c\ud3b8"
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough온보딩 콘텐츠 화면에 장르 선택 상태와 UI를 추가하고, 라우트 콜백을 Changes온보딩 장르 선택 기능
Sequence DiagramsequenceDiagram
participant UI as "OnboardingContentScreen\n(Composables)"
participant VM as "OnboardingViewModel"
participant Nav as "NavController"
participant State as "OnboardingContentUiState"
UI->>VM: onGenreClick(genre)
VM->>State: toggle selectedGenres -> emit new state
State-->>UI: recomposition with updated selectedGenres
UI->>Nav: onNextClick -> navigateToOnboardingDone()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/src/main/java/com/flint/presentation/onboarding/navigation/OnboardingNavigation.kt (1)
14-14: ⚡ Quick winOTT 화면 관련 코드가 사용되지 않는 상태로 남아 있음
PR 목표에 따르면 OTT 화면이 온보딩 플로우에서 제거되었지만, 아래 세 곳에 연관 코드가 그대로 남아 있습니다:
- Line 14:
import com.flint.presentation.onboarding.OnboardingOttRoute- Lines 29–31:
fun NavController.navigateToOnboardingOtt()— 더 이상 호출되지 않음- Lines 67–76:
composable<Route.OnboardingOtt> { ... }— 그래프에 등록되어 있지만 도달할 수 없는 목적지
OnboardingOttRoute자체가 아직 남아있다면 별도로 삭제가 필요하며,Route.OnboardingOtt도 마찬가지로 정리가 필요합니다.♻️ 제거 제안
-import com.flint.presentation.onboarding.OnboardingOttRoute-fun NavController.navigateToOnboardingOtt() { - navigate(Route.OnboardingOtt) -}- composable<Route.OnboardingOtt> { backStackEntry -> - val sharedViewModel = backStackEntry.sharedViewModel<OnboardingViewModel>(navController) - - OnboardingOttRoute( - paddingValues = paddingValues, - navigateUp = navController::navigateUp, - navigateToDone = navController::navigateToOnboardingDone, - viewModel = sharedViewModel, - ) - }Also applies to: 29-31, 67-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/flint/presentation/onboarding/navigation/OnboardingNavigation.kt` at line 14, Remove the unused OTT onboarding artifacts: delete the import OnboardingOttRoute, remove the NavController extension function navigateToOnboardingOtt(), and remove the composable registration for Route.OnboardingOtt (the composable { ... } block). Also check for and delete the OnboardingOttRoute class/object and the Route.OnboardingOtt entry if they remain unused elsewhere to avoid dangling references; make sure to run build to catch any remaining usages and update navigation tests if needed.app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.kt (2)
383-388: 💤 Low value프리뷰에서 완전 한정 이름(FQCN) 사용
Line 386의
kotlinx.collections.immutable.persistentListOf(...)대신 파일 상단에 import를 추가하고persistentListOf()를 사용하거나,toImmutableList()로 단순화할 수 있습니다.♻️ 개선 제안
+import kotlinx.collections.immutable.toImmutableList -selectedGenres = selectedGenres.toList().let { - kotlinx.collections.immutable.persistentListOf(*it.toTypedArray()) -}, +selectedGenres = selectedGenres.toList().toImmutableList(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.kt` around lines 383 - 388, In OnboardingContentScreen where OnboardingContentUiState is constructed (the selectedGenres mapping), replace the fully-qualified call kotlinx.collections.immutable.persistentListOf(...) with a top-level import and a shorter call (persistentListOf(...)) or simplify by converting the list to an immutable list via a helper like toImmutableList(); update the import section of the file and change the expression that builds selectedGenres (referencing selectedGenres and OnboardingContentUiState) accordingly.
156-169: ⚡ Quick win동일한
Modifier.layout사이드 패딩 로직이 중복됨장르 칩
LazyRow(Line 159–169)와 선택된 콘텐츠LazyRow(Line 211–221)에 완전히 동일한Modifier.layout블록이 두 번 등장합니다. Modifier 확장 함수로 추출하면 수정이 용이해집니다.♻️ 확장 함수로 추출 제안
공통 위치에 다음 확장 함수를 추가합니다 (파일 상단 또는 별도 유틸 파일):
private fun Modifier.overflowHorizontalPadding(padding: Dp): Modifier = this.layout { measurable, constraints -> val sidePadding = padding.roundToPx() val placeable = measurable.measure( constraints.copy(maxWidth = constraints.maxWidth + sidePadding * 2) ) layout(constraints.maxWidth, placeable.height) { placeable.place(-sidePadding, 0) } }그런 다음 두
LazyRow의 modifier를 다음과 같이 교체합니다:-modifier = Modifier - .height(48.dp) - .layout { measurable, constraints -> - val sidePadding = 16.dp.roundToPx() - val placeable = measurable.measure( - constraints.copy(maxWidth = constraints.maxWidth + sidePadding * 2) - ) - layout(constraints.maxWidth, placeable.height) { - placeable.place(-sidePadding, 0) - } - }, +modifier = Modifier + .height(48.dp) + .overflowHorizontalPadding(16.dp),-modifier = Modifier.layout { measurable, constraints -> - val sidePadding = 16.dp.roundToPx() - val placeable = measurable.measure( - constraints.copy(maxWidth = constraints.maxWidth + sidePadding * 2) - ) - layout(constraints.maxWidth, placeable.height) { - placeable.place(-sidePadding, 0) - } -}, +modifier = Modifier.overflowHorizontalPadding(16.dp),Also applies to: 209-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.kt` around lines 156 - 169, Extract the duplicated Modifier.layout block into a private extension function named overflowHorizontalPadding(padding: Dp): Modifier and use it from both LazyRow usages (the genre-chip LazyRow and the selected-content LazyRow) instead of repeating the layout lambda; implement overflowHorizontalPadding to round the padding to pixels, measure with constraints.copy(maxWidth = constraints.maxWidth + sidePadding * 2), and place the child at -sidePadding, then replace the inline Modifier.layout(...) in both places with .overflowHorizontalPadding(16.dp).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/flint/presentation/onboarding/OnboardingViewModel.kt`:
- Around line 114-126: The selectGenre currently triggers getSearchContentList
but the repository does not accept genres, so remove the
getSearchContentList(...) call from selectGenre and instead compute a
client-side filtered result using the updated _contentUiState.selectedGenres:
after updating selectedGenres in selectGenre, derive a filtered list from the
existing search results (e.g., currentState.searchResults or contentList) by
keeping items whose genre intersects selectedGenres and update _contentUiState
with that filtered list (or expose a new filteredResults property);
alternatively ensure OnboardingContentScreen reads selectedGenres and filters
the displayed list locally. Update references: selectGenre,
getSearchContentList, _contentUiState, selectedGenres, and
OnboardingContentScreen.
---
Nitpick comments:
In
`@app/src/main/java/com/flint/presentation/onboarding/navigation/OnboardingNavigation.kt`:
- Line 14: Remove the unused OTT onboarding artifacts: delete the import
OnboardingOttRoute, remove the NavController extension function
navigateToOnboardingOtt(), and remove the composable registration for
Route.OnboardingOtt (the composable { ... } block). Also check for and delete
the OnboardingOttRoute class/object and the Route.OnboardingOtt entry if they
remain unused elsewhere to avoid dangling references; make sure to run build to
catch any remaining usages and update navigation tests if needed.
In
`@app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.kt`:
- Around line 383-388: In OnboardingContentScreen where OnboardingContentUiState
is constructed (the selectedGenres mapping), replace the fully-qualified call
kotlinx.collections.immutable.persistentListOf(...) with a top-level import and
a shorter call (persistentListOf(...)) or simplify by converting the list to an
immutable list via a helper like toImmutableList(); update the import section of
the file and change the expression that builds selectedGenres (referencing
selectedGenres and OnboardingContentUiState) accordingly.
- Around line 156-169: Extract the duplicated Modifier.layout block into a
private extension function named overflowHorizontalPadding(padding: Dp):
Modifier and use it from both LazyRow usages (the genre-chip LazyRow and the
selected-content LazyRow) instead of repeating the layout lambda; implement
overflowHorizontalPadding to round the padding to pixels, measure with
constraints.copy(maxWidth = constraints.maxWidth + sidePadding * 2), and place
the child at -sidePadding, then replace the inline Modifier.layout(...) in both
places with .overflowHorizontalPadding(16.dp).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25b9312f-a2cb-4a37-b590-a5141e89d77f
📒 Files selected for processing (4)
app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.ktapp/src/main/java/com/flint/presentation/onboarding/OnboardingUiState.ktapp/src/main/java/com/flint/presentation/onboarding/OnboardingViewModel.ktapp/src/main/java/com/flint/presentation/onboarding/navigation/OnboardingNavigation.kt
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
2 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.
📮 관련 이슈
📌 작업 내용
📸 스크린샷
Screen_Recording_20260503_171803_Flint.mp4
😅 미구현
🫛 To. 리뷰어
Summary by CodeRabbit