Conversation
Closed
5 tasks
Contributor
📝 WalkthroughWalkthrough모달 그림자 제거 및 테마 색상 적용, 온보딩 화면들에서 헤더 구조와 페이드 효과 추가, 선택 항목 LazyRow 도입, 여러 화면에서 Changes
Sequence Diagram(s)(생략 — 변경이 UI 구성/스타일 중심이며 새 다중 컴포넌트 시퀀스 흐름 시각화 조건을 만족하지 않습니다.) 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
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/flint/presentation/onboarding/OnboardingDoneScreen.kt`:
- Around line 111-119: The FlintLargeButton call is missing the enabled
parameter so ripple remains active even when state = FlintButtonState.Disable;
update the call to pass enabled = !isLoading (so enabled is false when isLoading
is true), and clean up formatting by removing the extra blank line and trailing
space before the closing parenthesis after the modifier; refer to the
FlintLargeButton invocation and the isLoading/state parameters to make this
change.
🧹 Nitpick comments (6)
app/src/main/java/com/flint/presentation/onboarding/OnboardingOttScreen.kt (1)
104-113:onClick내부의 조건문이 불필요합니다.
enabled = ottUiState.canProceed가 설정되어 있으므로, 버튼이 비활성화되면onClick이 호출되지 않습니다. 따라서onClick람다 내부의if (ottUiState.canProceed)조건은 중복됩니다.또한, 112번 줄의 빈 줄은 제거하는 것이 좋습니다.
♻️ 제안된 수정
FlintLargeButton( text = "다음", state = if (ottUiState.canProceed) FlintButtonState.Able else FlintButtonState.Disable, - onClick = {if (ottUiState.canProceed) onNextClick() }, + onClick = onNextClick, enabled = ottUiState.canProceed, modifier = Modifier .fillMaxWidth() .padding(horizontal = 16.dp, vertical = 20.dp), - - ) + )app/src/main/java/com/flint/presentation/splash/SplashScreen.kt (1)
30-36: 잠재적인 이중 네비게이션 이슈 확인 필요현재
navigateToLogin()이 두 번 호출될 수 있는 구조입니다:
SplashScreen의onAnimationFinished콜백 (애니메이션 완료 시)LaunchedEffect의 2초 딜레이 후 호출애니메이션이 2초 이내에 완료되면 네비게이션이 중복 실행될 수 있습니다. 이번 PR 범위는 아니지만, 향후 안정성을 위해 타이머 기반 로직 또는 애니메이션 콜백 중 하나만 사용하는 것을 권장합니다.
♻️ 개선 제안
`@Composable` fun SplashRoute( paddingValues: PaddingValues, navigateToLogin: () -> Unit, navigateToHome: () -> Unit, viewModel: SplashViewModel = hiltViewModel(), ) { + var hasNavigated by remember { mutableStateOf(false) } + val safeNavigate: () -> Unit = { + if (!hasNavigated) { + hasNavigated = true + navigateToLogin() + } + } SplashScreen( - onAnimationFinished = navigateToLogin, + onAnimationFinished = safeNavigate, ) LaunchedEffect(Unit) { delay(2000) - navigateToLogin() + safeNavigate() } }app/src/main/java/com/flint/presentation/onboarding/OnboardingProfileScreen.kt (1)
168-188: 접근성 및 코드 가독성 개선 제안
접근성:
Box+clickable은 버튼 시맨틱스(role)가 없어 스크린 리더 사용자에게 버튼으로 인식되지 않을 수 있습니다.조건 중복:
isValid && isFormatValid조건이 4번 반복됩니다. 가독성을 위해 로컬 변수로 추출하는 것을 권장합니다.♻️ 개선 제안
+ val isButtonEnabled = isValid && isFormatValid Box( modifier = Modifier .fillMaxHeight() .clip(RoundedCornerShape(8.dp)) .background( - if (isValid && isFormatValid) FlintTheme.colors.primary400 + if (isButtonEnabled) FlintTheme.colors.primary400 else FlintTheme.colors.gray700 ) - .clickable(enabled = isValid && isFormatValid) { + .clickable( + enabled = isButtonEnabled, + role = androidx.compose.ui.semantics.Role.Button, + ) { keyboardController?.hide() onCheckNickname() } .padding(horizontal = 16.dp, vertical = 10.dp), contentAlignment = Alignment.Center, ) { Text( text = "확인", - color = if (isValid && isFormatValid) FlintTheme.colors.white else FlintTheme.colors.gray400, - style = if (isValid && isFormatValid) FlintTheme.typography.body1Sb16 else FlintTheme.typography.body1M16, + color = if (isButtonEnabled) FlintTheme.colors.white else FlintTheme.colors.gray400, + style = if (isButtonEnabled) FlintTheme.typography.body1Sb16 else FlintTheme.typography.body1M16, ) }app/src/main/java/com/flint/presentation/onboarding/OnboardingContentScreen.kt (3)
41-43: 사용하지 않는 import를 제거하세요.
FlintBasicButton이FlintLargeButton으로 교체되었으나, line 41의FlintBasicButtonimport가 여전히 남아있습니다.♻️ 제안된 수정
-import com.flint.core.designsystem.component.button.FlintBasicButton import com.flint.core.designsystem.component.button.FlintButtonState import com.flint.core.designsystem.component.button.FlintLargeButton
181-184:Arrangement.spacedBy(0.dp)는 기본값이므로 생략 가능합니다.LazyRow의 기본 horizontalArrangement는 이미 간격 없이 배치되므로, 이 속성을 명시적으로 지정할 필요가 없습니다.
♻️ 제안된 수정
LazyRow( state = lazyListState, - horizontalArrangement = Arrangement.spacedBy(0.dp), ) {
274-283: 비활성화 버튼의 ripple 효과 수정이 잘 적용되었습니다.
enabled파라미터를 통해 PR 목표인 비활성화된 "다음" 버튼의 ripple 효과 문제가 해결되었습니다.다만,
onClick내부의canProceed조건 검사는enabled = contentUiState.canProceed로 이미 처리되므로 중복입니다. 또한 line 282-283의 포맷팅이 다소 비정상적입니다.♻️ 제안된 정리
FlintLargeButton( text = "다음", state = if (contentUiState.canProceed) FlintButtonState.Able else FlintButtonState.Disable, - onClick = { if (contentUiState.canProceed) { onNextClick() } }, + onClick = onNextClick, enabled = contentUiState.canProceed, modifier = Modifier .fillMaxWidth() .padding(horizontal = 16.dp, vertical = 20.dp), - - ) +)
app/src/main/java/com/flint/presentation/onboarding/OnboardingDoneScreen.kt
Show resolved
Hide resolved
giovannijunseokim
approved these changes
Jan 23, 2026
Contributor
giovannijunseokim
left a comment
giovannijunseokim
left a comment
There was a problem hiding this comment.
해결된거죠? Approve
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
3 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_20260123_141630_Flint.1.mp4
Summary by CodeRabbit
릴리스 노트
UI/스타일 개선
기능 개선
기타
✏️ Tip: You can customize this high-level summary in your review settings.