[Feat/#1585] SP2 솝레터 온보딩 및 이름 설정 UI#1590
Merged
Merged
Conversation
- `SopletterOnboardingNavigation`을 추가하여 온보딩 화면으로의 경로 정의 및 이동 함수 구현 - `SopletterOnboardingScreen` 및 `SopletterOnboardingRoute` 구현 - `OnboardingInfoHolder`와 `SopletterButton`을 포함한 온보딩 UI 레이아웃 구성
- `SopletterOnboardingScreen` 및 `OnboardingInfoHolder` 컴포넌트 추가 - `SopletterNameScreen` 및 `NameInfoHolder` 닉네임 확인 UI 추가 - `SopletterNameViewModel`, `NameState`, `NameSideEffect` 등 MVI 기반 로직 구현 - `SopletterNavigation`을 통한 온보딩 네비게이션 그래프 정의
- 공통 버튼 컴포넌트 `SopletterButton` 구현 - 화면에 사용되는 이미지 리소스 추가 (`img_sopletter_mailbox`, `img_sopletter_onboarding_title`, `img_sopletter_name_letter`)
seungjunGong
approved these changes
May 25, 2026
seungjunGong
left a comment
seungjunGong
left a comment
Member
There was a problem hiding this comment.
고생하셨습니다 ~~ 탑바 공통 컴포로 분리하는거 좋습니다!
| import org.sopt.official.sopletter.name.component.NameInfoHolder | ||
|
|
||
| @Composable | ||
| fun NameRoute( |
Member
There was a problem hiding this comment.
p3: 여기에는 Sopletter prefix를 따로 붙이지 않으신 이유가 있을까요?
다른 Sopletter 관련 컴포넌트들과 네이밍을 맞추는 관점에서 prefix를 붙이는 것도 좋을 것 같아서 여쭤봅니다!
Member
Author
There was a problem hiding this comment.
엇 이름 변경하면서 Route는 깜빡한 것 같습니다! 반영하겠습니다 감사합니다 😆
…into feat/#1585-sopletter-onboarding
Summary by CodeRabbitNew Features
WalkthroughSopletter 기능 모듈에 공유 버튼 컴포넌트( ChangesSopletter 온보딩 & 닉네임 화면 구현
Sequence Diagram(s)sequenceDiagram
rect rgba(99, 179, 237, 0.5)
Note over SopletterNameRoute,SopletterNameScreen: 닉네임 화면 상태 흐름
end
participant SopletterNameRoute
participant SopletterNameViewModel
participant SopletterNameScreen
SopletterNameRoute->>SopletterNameViewModel: state 구독 (collectAsStateWithLifecycle)
SopletterNameViewModel-->>SopletterNameRoute: NameState (name, generation)
SopletterNameRoute->>SopletterNameScreen: NameState 전달
SopletterNameRoute->>SopletterNameViewModel: sideEffect 수집 (flowWithLifecycle)
SopletterNameViewModel-->>SopletterNameRoute: NavigateToSopletterMain (TODO)
SopletterNameScreen->>SopletterNameRoute: 버튼 클릭 → navigateToSopletterMain()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
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}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
feature/sopletter/src/main/java/org/sopt/official/sopletter/component/SopletterButton.kt (2)
24-29: ⚡ Quick win
fillMaxWidth()가 컴포넌트 내부에 하드코딩되어 재사용성이 제한됩니다.현재
fillMaxWidth()가 내부modifier체인에 포함되어 있어, 호출 측에서 버튼 너비를 제어할 수 없습니다. 호출 측에서 전달한modifier를 먼저 적용한 후 내부 스타일을 적용하도록 수정하거나,fillMaxWidth()를 제거하여 호출 측이 너비를 결정하도록 하는 것이 권장됩니다.♻️ 제안하는 수정안
Box( modifier = modifier - .fillMaxWidth() .clip(shape = RoundedCornerShape(12.dp)) .background(color = SoptTheme.colors.primary) .clickable(onClick = onClick),호출 측에서 필요에 따라
.fillMaxWidth()를 추가하도록 변경합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/sopletter/src/main/java/org/sopt/official/sopletter/component/SopletterButton.kt` around lines 24 - 29, The SopletterButton component has fillMaxWidth() hardcoded in its internal modifier chain on the Box, which prevents callers from controlling the button's width and limits reusability. Remove fillMaxWidth() from the modifier chain in the Box that includes clip(RoundedCornerShape(12.dp)), background(color = SoptTheme.colors.primary), and clickable(onClick = onClick), allowing the caller to pass any width modifier they need through the modifier parameter.
24-29: ⚡ Quick win
clickable대신 Material 인터랙션을 제공하는 패턴을 고려하세요.현재
Box + clickable을 사용하고 있으나, Material Design에서는Surface의onClick파라미터를 사용하면 자동으로 리플 효과와 접근성 시맨틱(Role.Button)이 적용됩니다.♻️ Surface 기반 대안
+import androidx.compose.material3.Surface + `@Composable` internal fun SopletterButton( buttonText: String, onClick: () -> Unit, modifier: Modifier = Modifier ) { - Box( + Surface( modifier = modifier - .clip(shape = RoundedCornerShape(12.dp)) - .background(color = SoptTheme.colors.primary) - .clickable(onClick = onClick), + .fillMaxWidth(), + onClick = onClick, + shape = RoundedCornerShape(12.dp), + color = SoptTheme.colors.primary, contentAlignment = Alignment.Center ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/sopletter/src/main/java/org/sopt/official/sopletter/component/SopletterButton.kt` around lines 24 - 29, Replace the Box composable in the SopletterButton with Surface and use its onClick parameter instead of the clickable modifier to follow Material Design patterns. Surface will automatically provide ripple effects and accessibility semantics (Role.Button) without needing to explicitly add the clickable modifier. Remove the clickable modifier from the modifier chain and pass the onClick lambda as a parameter to the Surface composable, keeping the other styling properties like fillMaxWidth, clip, and background as needed for the design.feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/SopletterOnboadingScreen.kt (1)
40-40: 탑바 구현을 위한 TODO 주석이 남아있습니다.PR 설명에서 언급하신 것처럼 탑바는 공유 컴포넌트로 개발 후 API 구현 시 통합 예정인 것으로 이해됩니다.
탑바 컴포넌트 구현이나 통합 작업을 추적하기 위한 별도 이슈 생성이 필요하시면 도움을 드릴 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/SopletterOnboadingScreen.kt` at line 40, The TODO comment regarding the top bar implementation exists in the SopletterOnboadingScreen.kt file at the specified location. Since the top bar is planned to be developed as a shared component and integrated when the API is implemented (as described in the PR), remove this TODO comment from the code. If tracking of this work is necessary, create a separate issue to track the top bar implementation and integration task instead of leaving it as a comment in the code.feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/component/OnboardingInfoHolder.kt (1)
28-40: 💤 Low value이미지의
contentDescription이null로 설정되어 있습니다.현재 두 이미지 모두
contentDescription = null로 설정되어 있는데, 이는 장식용 이미지로 의도된 것으로 보입니다. 만약 이미지가 콘텐츠 이해에 중요한 정보를 담고 있다면 적절한 설명을 추가하는 것이 접근성 측면에서 권장됩니다. 현재 온보딩 맥락에서는 장식용으로 판단되어 문제없어 보입니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/component/OnboardingInfoHolder.kt` around lines 28 - 40, The `contentDescription` fields for both Image composables (img_sopletter_mailbox and img_sopletter_onboarding_title) are set to null, which is appropriate since these images serve as decorative elements in the onboarding context and do not convey essential content information. No changes are required; the current implementation correctly handles accessibility for decorative images by using null contentDescription.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/sopletter/src/main/java/org/sopt/official/sopletter/name/SopletterNameScreen.kt`:
- Line 68: Fix the typo in the buttonText string where "바로가가" should be "바로가기".
In the buttonText assignment that includes the state.generation interpolation,
change the trailing Korean characters from "가가" to "가기" to display the correct
phrase "솝레터 바로가기".
- Around line 24-43: The navigation to SopletterMain is not connected because
the NameSideEffect.NavigateToSopletterMain side effect in the LaunchedEffect
block is just a TODO comment without actual navigation logic, and the
navigateToSopletterMain callback is not passed to the NameScreen composable.
Replace the TODO comment in the when branch that handles
NameSideEffect.NavigateToSopletterMain with actual navigation logic to the main
screen, and pass the navigateToSopletterMain callback parameter to the
NameScreen composable so the button click and side effect both properly trigger
navigation.
In
`@feature/sopletter/src/main/java/org/sopt/official/sopletter/navigation/SopletterNavigation.kt`:
- Around line 16-20: The `sopletterGraph` function has two unused parameters:
`navController` and `navigateUp`. Either remove these parameters from the
function signature if they are not needed, or add a TODO comment in the function
body explaining their planned future use. Ensure the function signature only
includes parameters that are actively used within the function implementation.
- Line 27: The navigateToNickname lambda in the SopletterNavigation is currently
empty, preventing navigation to the nickname screen when users interact with the
"솝레터 시작하기" button. You need to add a nickname input screen destination to the
sopletterGraph navigation graph and then replace the empty navigateToNickname
lambda with an actual navigation function that directs users to that newly added
nickname screen destination using the appropriate navigation action or route.
In
`@feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/SopletterOnboadingScreen.kt`:
- Around line 1-72: The filename contains a typo: "SopletterOnboadingScreen.kt"
has "Onboading" misspelled. Rename the file to "SopletterOnboardingScreen.kt" to
correct the spelling to "Onboarding". This will ensure consistency and prevent
confusion during code searches and maintenance.
---
Nitpick comments:
In
`@feature/sopletter/src/main/java/org/sopt/official/sopletter/component/SopletterButton.kt`:
- Around line 24-29: The SopletterButton component has fillMaxWidth() hardcoded
in its internal modifier chain on the Box, which prevents callers from
controlling the button's width and limits reusability. Remove fillMaxWidth()
from the modifier chain in the Box that includes
clip(RoundedCornerShape(12.dp)), background(color = SoptTheme.colors.primary),
and clickable(onClick = onClick), allowing the caller to pass any width modifier
they need through the modifier parameter.
- Around line 24-29: Replace the Box composable in the SopletterButton with
Surface and use its onClick parameter instead of the clickable modifier to
follow Material Design patterns. Surface will automatically provide ripple
effects and accessibility semantics (Role.Button) without needing to explicitly
add the clickable modifier. Remove the clickable modifier from the modifier
chain and pass the onClick lambda as a parameter to the Surface composable,
keeping the other styling properties like fillMaxWidth, clip, and background as
needed for the design.
In
`@feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/component/OnboardingInfoHolder.kt`:
- Around line 28-40: The `contentDescription` fields for both Image composables
(img_sopletter_mailbox and img_sopletter_onboarding_title) are set to null,
which is appropriate since these images serve as decorative elements in the
onboarding context and do not convey essential content information. No changes
are required; the current implementation correctly handles accessibility for
decorative images by using null contentDescription.
In
`@feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/SopletterOnboadingScreen.kt`:
- Line 40: The TODO comment regarding the top bar implementation exists in the
SopletterOnboadingScreen.kt file at the specified location. Since the top bar is
planned to be developed as a shared component and integrated when the API is
implemented (as described in the PR), remove this TODO comment from the code. If
tracking of this work is necessary, create a separate issue to track the top bar
implementation and integration task instead of leaving it as a comment in the
code.
🪄 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: 112643ee-8962-4078-8fbc-78c8746c8e24
📒 Files selected for processing (12)
feature/sopletter/src/main/java/org/sopt/official/sopletter/component/SopletterButton.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/name/NameContract.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/name/SopletterNameScreen.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/name/SopletterNameViewModel.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/name/component/NameInfoHolder.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/navigation/SopletterNavigation.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/SopletterOnboadingScreen.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/component/OnboardingInfoHolder.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/navigation/SopletterOnboardingNavigation.ktfeature/sopletter/src/main/res/drawable/img_sopletter_mailbox.xmlfeature/sopletter/src/main/res/drawable/img_sopletter_name_letter.xmlfeature/sopletter/src/main/res/drawable/img_sopletter_onboarding_title.xml
- `NameRoute`, `NameScreen` 등 관련 함수명을 `SopletterNameRoute`, `SopletterNameScreen`으로 변경하여 일관성 확보
- `NameInfoHolder` 및 `OnboardingInfoHolder`를 각각 `SopletterNameInfoHolder`, `SopletterOnboardingInfoHolder`로 클래스명 변경 및 파일 리네임
- `SopletterOnboadingScreen.kt` 파일명의 오타를 `SopletterOnboardingScreen.kt`로 수정
- `SopletterNameScreen` 내 버튼 텍스트의 오타 수정 ("바로가가" -> "바로가기")
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/component/SopletterOnboardingInfoHolder.kt (1)
44-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win공통 원인: 온보딩 화면의 사용자 노출 텍스트가 코드에 하드코딩되어 있습니다.
문구 리소스화를 한 번에 적용하면 i18n 대응과 카피 변경 유지보수 비용을 줄일 수 있습니다.
feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/component/SopletterOnboardingInfoHolder.kt#L44-L50: 안내 3줄 문구를strings.xml로 이동하고stringResource로 참조하세요.feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/SopletterOnboardingScreen.kt#L52-L54: CTA 버튼 라벨을strings.xml로 이동하고stringResource로 참조하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/component/SopletterOnboardingInfoHolder.kt` around lines 44 - 50, The hardcoded user-facing text strings in the onboarding UI should be extracted to resources for i18n support and easier maintenance. In SopletterOnboardingInfoHolder.kt (lines 44-50), move the 3-line guidance text from the Text component into strings.xml and reference it using stringResource(). In SopletterOnboardingScreen.kt (lines 52-54), move the CTA button label text into strings.xml and reference it using stringResource(). This allows for consistent string management and supports future internationalization.
♻️ Duplicate comments (1)
feature/sopletter/src/main/java/org/sopt/official/sopletter/name/SopletterNameScreen.kt (1)
24-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick win네비게이션 콜백 미연결로 메인 진입이 동작하지 않습니다.
Line 35에서 사이드이펙트가 TODO로 비워져 있고, Line 41-43에서
navigateToSopletterMain를 전달하지 않아SopletterNameScreen의 기본값{}(Line 49)이 사용됩니다. 현재 Route 경유 시 버튼/사이드이펙트 모두 실제 이동을 수행하지 못합니다.수정 예시
`@Composable` fun SopletterNameRoute( + navigateToSopletterMain: () -> Unit, viewModel: SopletterNameViewModel = hiltViewModel() ) { @@ LaunchedEffect(Unit) { viewModel.sideEffect.flowWithLifecycle(lifeCycleOwner.lifecycle) .collect { sideEffect -> when (sideEffect) { - NameSideEffect.NavigateToSopletterMain -> { /* Todo : SopltterMain 네비게이션 연결 */} + NameSideEffect.NavigateToSopletterMain -> navigateToSopletterMain() } } } @@ SopletterNameScreen( - state = state + state = state, + navigateToSopletterMain = navigateToSopletterMain ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/sopletter/src/main/java/org/sopt/official/sopletter/name/SopletterNameScreen.kt` around lines 24 - 43, The navigation to the main screen is not connected. In the SopletterNameRoute function, the when branch that handles NameSideEffect.NavigateToSopletterMain (currently with a TODO comment) needs to implement the actual navigation logic instead of being empty. Additionally, pass a navigateToSopletterMain callback function as a parameter to the SopletterNameScreen composable call so it can trigger navigation when needed, rather than relying on the default empty lambda implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@feature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/component/SopletterOnboardingInfoHolder.kt`:
- Around line 44-50: The hardcoded user-facing text strings in the onboarding UI
should be extracted to resources for i18n support and easier maintenance. In
SopletterOnboardingInfoHolder.kt (lines 44-50), move the 3-line guidance text
from the Text component into strings.xml and reference it using
stringResource(). In SopletterOnboardingScreen.kt (lines 52-54), move the CTA
button label text into strings.xml and reference it using stringResource(). This
allows for consistent string management and supports future
internationalization.
---
Duplicate comments:
In
`@feature/sopletter/src/main/java/org/sopt/official/sopletter/name/SopletterNameScreen.kt`:
- Around line 24-43: The navigation to the main screen is not connected. In the
SopletterNameRoute function, the when branch that handles
NameSideEffect.NavigateToSopletterMain (currently with a TODO comment) needs to
implement the actual navigation logic instead of being empty. Additionally, pass
a navigateToSopletterMain callback function as a parameter to the
SopletterNameScreen composable call so it can trigger navigation when needed,
rather than relying on the default empty lambda implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b8cb872-dd2c-4107-9f29-cdb1bd3f9226
📒 Files selected for processing (4)
feature/sopletter/src/main/java/org/sopt/official/sopletter/name/SopletterNameScreen.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/name/component/SopletterNameInfoHolder.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/SopletterOnboardingScreen.ktfeature/sopletter/src/main/java/org/sopt/official/sopletter/onboarding/component/SopletterOnboardingInfoHolder.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
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.
Related issue 🛠
Work Description ✏️
Screenshot 📸
To Reviewers 📢
탑바 같은 경우 공통으로 구현되어야 할 수 있어서 탑바를 제외하고 구현하였습니다
나머지는 API 구현 시 붙이면서 반영하겠습니다!