-
Notifications
You must be signed in to change notification settings - Fork 0
[feature/#38][feature/#49] 다시 대화하기 기능 구현 및 폴드 기기 대응 #50
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
Walkthrough
Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 사용자
participant Letter as LetterScreen
participant Nav as Navigator
participant Chat as ChattingScreen
participant VM as ChattingViewModel
User->>Letter: "다시 대화하기" 클릭
Letter->>Nav: 채팅 화면으로 이동
Nav->>Chat: 화면 생성
Chat->>VM: handleIntent(RestartChat)
VM->>VM: 상태 초기화 및 채팅방 재생성
VM-->>Chat: 새 상태 반영
Chat-->>User: 초기화된 채팅 화면 표시
sequenceDiagram
autonumber
actor User as 사용자
participant FairySel as FairySelectionScreen
participant VM as ChattingViewModel
User->>FairySel: 재시도 클릭
FairySel->>VM: handleIntent(RestartChat)
VM->>VM: restartChat() 실행
VM-->>FairySel: 업데이트된 상태
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewersPoem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt (2)
3-3: EmotiaMultiLineTextField import 경로 오류로 컴파일 불가designsystem 컴포넌트를 패키지 없이 임포트하고 있어 unresolved reference가 납니다. 올바른 패키지 경로로 교체하세요.
적용 diff:
-import EmotiaMultiLineTextField +import com.nexters.emotia.core.designsystem.component.EmotiaMultiLineTextField
102-104: IME 높이 기반으로 DP 변환 값을 만들어 두고 재사용하세요하드코딩된 여백(아래 코멘트) 제거를 위해 DP 값으로 변환해두면 재활용이 쉽습니다.
적용 diff:
val density = LocalDensity.current val imeHeight = WindowInsets.ime.getBottom(density) +val imeBottomDp = with(density) { imeHeight.toDp() } val focusManager = LocalFocusManager.current
🧹 Nitpick comments (17)
composeApp/src/androidMain/res/drawable/ic_splash_screen.xml (1)
3-3: 배경색 검정 전환: 테마와 일관성 있음. 아이콘 대비/밀도 리소스 확인 필요
- 검정 배경 전환은 themes.xml 변경과 일관됩니다.
- 단, bitmap src가 밀도 고정 리소스(@mipmap/ic_launcher_xxxhdpi_circled)를 직접 참조합니다. 기기 밀도별 선명도 확보를 위해 밀도 독립 리소스(@mipmap/ic_launcher 또는 전용 벡터/드로어블)를 권장합니다.
- 검정 배경에서 아이콘 대비(특히 라이트/다크 모드, OEM 변형)도 한번 확인 부탁드립니다.
권장 변경 예시(밀도 독립 리소스 사용):
<bitmap android:src="@mipmap/ic_launcher" android:gravity="center" />composeApp/src/androidMain/res/values/themes.xml (1)
5-5: 스플래시 배경 @android:color/black → OK. 리소스화하여 테마 확장성 확보 권장
- 현재 값은 하드코딩된 시스템 색상입니다. DayNight/브랜드 일관성 및 폴드/다양한 폼팩터 대응을 위해 색상을 앱 리소스로 추출하세요.
다음 변경을 제안합니다(해당 색 리소스 신설 전제):
- <item name="windowSplashScreenBackground">@android:color/black</item> + <item name="windowSplashScreenBackground">@color/splash_background</item>colors.xml 예시:
<resources> <color name="splash_background">#000000</color> </resources>또한 검정 배경에서 상태바/네비게이션 아이콘 가독성도 실제 기기에서 확인 부탁드립니다.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyScreen.kt (7)
100-117: 1800f 임계값 매직 넘버 중복 — 상수화하고 화면 크기 기반으로의 동적 계산을 검토해 주세요현재 1800f 비교가 여기와 아래(라인 168)에서 반복됩니다. 상수화로 의도를 명확히 하고, 폴드/대화면에서 연출 일관성을 위해 화면 대각선 기반 임계값으로의 전환을 권장합니다.
적용 예(상수화):
// 파일 상단에 추가 private const val REVEAL_DONE_RADIUS = 1800f해당 범위 내 변경:
- if (animatedRadius >= 1800f) { + if (animatedRadius >= REVEAL_DONE_RADIUS) {동적 임계값 예(참고용): BoxWithConstraints + LocalDensity로 대각선/2 픽셀을 계산해 REVEAL_DONE_RADIUS 대체.
val density = LocalDensity.current BoxWithConstraints { val revealDoneRadius by remember(maxWidth, maxHeight, density) { mutableStateOf(with(density) { val pxW = maxWidth.toPx() val pxH = maxHeight.toPx() kotlin.math.hypot(pxW, pxH) / 2f }) } // animatedRadius 비교에 revealDoneRadius 사용 }폴더블 실제 기기/에뮬레이터에서 애니메이션 끝 지점 일관성 확인 부탁드립니다.
103-104: 배경 색상 일관성 유지root 배경은 theme(colors.black)을 사용하지만, 아래 Canvas 오버레이는 Color.Black를 사용합니다. 테마 일관성을 위해 오버레이도 colors.black 사용을 권장합니다.
범위 밖 참고 변경(라인 209~211 주변):
- drawRect(color = Color.Black, size = size) + drawRect(color = colors.black, size = size)테마 다크톤이 조정될 때 오버레이도 함께 변하도록 하는 효과가 있습니다.
118-126: 콘텐츠 최대 폭(500.dp) 상수화하여 재사용성/일관성 향상동일 값이 본 파일 내 두 곳(여기와 라인 171)에 등장합니다. 상수로 추출해 두 화면(요정/편지) 모두에서 동일하게 사용하면 유지보수성이 좋아집니다.
상수 선언(파일 상단):
private val MaxContentWidth = 500.dp해당 범위 내 변경:
- .widthIn(max = 500.dp) + .widthIn(max = MaxContentWidth)
128-133: 배경 이미지는 장식 요소 → 스크린리더 노이즈 제거배경은 의미 전달 요소가 아니므로 contentDescription을 null로 두어 접근성 잡음을 줄이는 게 좋습니다.
- contentDescription = "Background Image", + contentDescription = null,
136-151: 요정 이미지 접근성/로딩/에러 처리 보강
- contentDescription에 URL을 노출하지 마세요. 사용자 의미 기반(이름 등)으로 제공 권장.
- 로딩/에러 placeholder를 제공해 깜빡임/깨짐을 완화하세요.
model = ImageRequest.Builder(LocalPlatformContext.current) .data(fairyImage) .crossfade(true) .build(), - contentDescription = "$fairyImage image", + contentDescription = "${fairyName} 이미지", contentScale = ContentScale.Inside, modifier = Modifier .size(108.dp) .align(Alignment.BottomCenter) .offset(y = (-49).dp) .clip(RoundedCornerShape(16.dp)), - error = { - // TODO : 에러 이미지 처리 - }, + loading = { + Box( + Modifier + .size(108.dp) + .clip(RoundedCornerShape(16.dp)) + .background(colors.transparencyBlack) + ) + }, + error = { + Image( + painter = painterResource(Res.drawable.img_npc), + contentDescription = "요정 이미지 로드 실패", + modifier = Modifier + .size(108.dp) + .clip(RoundedCornerShape(16.dp)), + contentScale = ContentScale.Crop + ) + },원하시면 공통 placeholder 컴포넌트로 추출해 드리겠습니다.
153-165: 하드코딩 컬러 → 테마 토큰으로 정규화직접 hex(0xFF171E2D, 0xFF1A1A1B)를 쓰기보다 디자인시스템 색상 토큰으로 승격하면 다크테마/접근성 대비 요구 변경 시 대응이 쉬워집니다.
168-185: AnimatedVisibility의 visible=true 하드코딩 제거 + 최대 폭 상수 재사용현재 if 조건에서 이미 가시성 판단을 해 두었는데 내부 visible=true로 고정되어 있어 의미 중복입니다. 애니메이션 트리거를 animatedRadius와 연결하고, 최대 폭 상수도 재사용해 주세요.
- if (stepTexts != null && animatedRadius >= 1800f) { + if (stepTexts != null) { Box( modifier = Modifier - .widthIn(max = 500.dp) + .widthIn(max = MaxContentWidth) .align(Alignment.BottomCenter) ) { AnimatedVisibility( - visible = true, + visible = animatedRadius >= REVEAL_DONE_RADIUS, enter = fadeIn(tween(300)), exit = fadeOut(tween(300)) ) { TypingAnimatedSpeechBubble( fullText = stepTexts, useAlternativeBackground = false ) } } - } + }위(라인 100~117) 코멘트의 REVEAL_DONE_RADIUS 상수와 함께 적용해 주세요.
feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingScreen.kt (1)
436-438: “다시 대화하기” 연타 시 중복 재시작 방지UI단에서도 로딩 중 클릭을 무시해 중복 intent 디스패치를 차단하는 것을 권장합니다.
아래처럼 간단 가드를 추가해 주세요(현재 스코프에서 uiState 접근 가능):
- onRetryClick = { - viewModel.handleIntent(ChattingIntent.RestartChat) - } + onRetryClick = { + if (!uiState.isLoading) { + viewModel.handleIntent(ChattingIntent.RestartChat) + } + }추가로 Text의 rippleClickable에 enabled 옵션이 있다면 enabled = !uiState.isLoading 적용을 고려해 주세요.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt (7)
115-121: 키보드 닫힘 시 스크롤을 0으로 강제 리셋하는 UX 개선사용자가 스크롤한 위치가 초기화됩니다. 키보드가 열릴 때만 하단으로 스크롤하도록 단순화하세요.
적용 diff:
LaunchedEffect(isKeyboardVisible) { if (isKeyboardVisible) { scrollState.animateScrollTo(scrollState.maxValue) - } else { - scrollState.animateScrollTo(0) } }
148-151: 배경 이미지는 스크린리더에서 읽히지 않게 contentDescription = null 권장장식용 이미지는 접근성에서 숨기는 것이 일반적입니다.
적용 diff:
- contentDescription = "Background Image", + contentDescription = null,
155-169: 이미지 로딩 실패/플레이스홀더 처리 보강error 블록이 비어 있습니다. placeholder/error 이미지를 지정하거나 skeleton(Shimmer) 등을 적용해 사용자 경험을 개선하세요. contentDescription도 URL이 아닌 의미 있는 텍스트로 대체하는 것이 좋습니다.
193-207: 하드코딩 문자열 리소스화placeholder 문구는 리소스로 분리해 i18n/문구 관리가 가능하도록 해 주세요. moko-resources를 이미 사용 중이니 stringResource로 치환을 권장합니다.
139-145: max width 500.dp 매직 넘버 상수화폴드 대응값은 디자인 시스템/테마 차원에서 공용 상수로 관리하면 일관성과 유지보수성이 좋아집니다. 예: Dimens.MaxContentWidthFold.
96-99: 에러 처리 TODO 남김 — 사용자 피드백 필요전송 실패/재시작 실패 시 스낵바/다이얼로그 등 피드백을 제공하세요. Orbit SideEffect로 메시지를 내보내고 상위에서 처리해도 됩니다. 필요하면 구현 드릴게요.
12-12: 사용되지 않는 import 정리fillMaxHeight는 사용되지 않는 것으로 보입니다. 정리하면 깔끔합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.gitignore(1 hunks)composeApp/src/androidMain/res/drawable/ic_splash_screen.xml(1 hunks)composeApp/src/androidMain/res/values/themes.xml(1 hunks)feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingScreen.kt(2 hunks)feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingViewModel.kt(2 hunks)feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/contract/ChattingIntent.kt(1 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyScreen.kt(4 hunks)feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/src/commonMain/kotlin/**/*.kt : Use common state management patterns (ViewModel, StateFlow)
Applied to files:
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/src/{commonMain,androidMain,iosMain}/kotlin/**/*.kt : Leverage Compose resources via Res.drawable.* and Res.string.*
Applied to files:
composeApp/src/androidMain/res/drawable/ic_splash_screen.xml
📚 Learning: 2025-07-27T10:25:59.389Z
Learnt from: CR
PR: Nexters/team-ace-client#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-27T10:25:59.389Z
Learning: Applies to composeApp/src/commonMain/kotlin/**/*.kt : Implement navigation logic in common code when possible
Applied to files:
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyScreen.kt
🧬 Code graph analysis (3)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt (2)
core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaMultiLineTextField.kt (1)
EmotiaMultiLineTextField(64-119)core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/EmotiaButton.kt (1)
EmotiaButton(16-42)
feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingViewModel.kt (1)
core/network/src/commonMain/kotlin/com/nexters/emotia/network/service/ChatApiService.kt (1)
createChatRoom(16-23)
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyScreen.kt (1)
core/designsystem/src/commonMain/kotlin/com/nexters/emotia/core/designsystem/component/SpeechBubbleTextField.kt (1)
TypingAnimatedSpeechBubble(42-161)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Firebase App Distribution
🔇 Additional comments (6)
.gitignore (1)
40-42: emotia-credential.json — 인덱스/이력 미포함; .gitignore 패턴 보완 및 유사 비밀파일 검사 재실행 필요
- 검증: 현재 인덱스에 없음; Git 이력(최초 추가)에서도 발견되지 않음.
- 권장: 루트 고정이면 "/emotia-credential.json", 서브폴더 허용이면 "**/emotia-credential.json"으로 패턴 명확화.
- 유사 비밀파일 검사(rg)가 필터로 실패했음 — 아래 명령으로 재실행 후 결과를 올려주세요:
rg -n -S -e '\.env(\.|$)' -e '\.pem$' -e '\.p12$' -e 'credentials?\.json$' || true
- 발견 시 즉시 자격증명 회수(키 롤테이션) 및 히스토리 정리 필요.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/fairy/FairyScreen.kt (1)
21-21: 폴더블 대응을 위한 widthIn 도입 좋습니다폴더블/대화면에서의 가독성 확보 목적에 부합합니다.
feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/contract/ChattingIntent.kt (1)
14-15: RestartChat 인텐트 추가 LGTM의도 명확하고, 사용처(화면/VM)와 일관됩니다.
이 인텐트를 구독하는 다른 레이어(예: Nav, 테스트) 영향 없는지 한 번만 스모크 체크 부탁드립니다.
feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingViewModel.kt (1)
46-47: RestartChat 분기 추가 적절의도 라우팅 문제 없습니다. 아래 restartChat 본문에서 중복 실행 방지 가드만 추가되면 더 안전합니다.
feature/chatting/src/commonMain/kotlin/com/nexters/emotia/feature/chatting/ChattingScreen.kt (1)
118-121: 스포트라이트 축소 비율 조정(70%) LGTM애니메이션 의도에 부합하며 회귀 우려 없어 보입니다.
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt (1)
222-229: '다시 대화하기' 버튼 → ViewModel 인텐트로 재시작 처리하고 클릭 중복 방지 적용현재 onNavigateToChatting()로만 네비게이션하므로, 새 채팅방 생성 요구사항을 만족하려면 ViewModel에 Restart 인텐트(예: LetterIntent.RestartChat)를 보내고 성공 SideEffect에서 화면 전환을 위임하세요. 클릭은 uiState.isLoading 등으로 비활성화하세요.
파일: feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt (약 라인 222–229)
권장 변경(예시):
- .clickable { - onNavigateToChatting() - }, + .clickable(enabled = !uiState.isLoading) { + viewModel.handleIntent(LetterIntent.RestartChat) + },검증: 자동 검색에서 RestartChat/LetterIntent를 찾을 수 없어 존재 여부 미확인 — Intent명 및 ViewModel 핸들러 구현 여부를 수동으로 확인해 주세요.
| private fun restartChat() = intent { | ||
| reduce { ChattingState() } | ||
| createChatRoom() | ||
| } | ||
|
|
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.
중복 탭으로 인한 다중 채팅룸 생성 가능성 — 가드 추가 권장
사용자가 "다시 대화하기"를 연타하면 연속적으로 createRoom 네트워크 호출이 발생할 수 있습니다. 서버/요금제/분석 측면에서 불필요한 룸이 다수 생성될 수 있어 방어 로직이 필요합니다.
다음처럼 VM 레벨에서 최소 가드를 추가해 주세요:
- private fun restartChat() = intent {
- reduce { ChattingState() }
- createChatRoom()
- }
+ private fun restartChat() = intent {
+ // 로딩 중(재시작/생성 포함)에는 추가 재시작 무시
+ if (state.isLoading) return@intent
+ reduce { ChattingState() }
+ createChatRoom()
+ }추가로 권장:
- 상태 초기화 직후 순간적인 깜빡임을 줄이려면 초기화 시 isLoading=true로 세팅(예: ChattingState() 대신 copy로 isLoading=true)해 UI 일관성 확보.
- 연속 클릭 방지는 UI에서도 함께 처리(아래 ChattingScreen 코멘트 참고).
| if (isKeyboardVisible) { | ||
| Spacer(modifier = Modifier.height(210.dp)) | ||
| } |
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
키보드 여백을 하드코딩(210.dp)하지 말고 IME 높이에 맞춰 동적으로 처리
기기/폴드/키보드 높이에 따라 깨질 수 있습니다. 위에서 만든 imeBottomDp를 사용하세요.
적용 diff:
- if (isKeyboardVisible) {
- Spacer(modifier = Modifier.height(210.dp))
- }
+ if (isKeyboardVisible) {
+ Spacer(modifier = Modifier.height(imeBottomDp))
+ }추가로, 전체 섹션에 windowInsetsPadding(WindowInsets.ime)을 적용하는 접근도 고려해 보세요.
📝 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.
| if (isKeyboardVisible) { | |
| Spacer(modifier = Modifier.height(210.dp)) | |
| } | |
| if (isKeyboardVisible) { | |
| Spacer(modifier = Modifier.height(imeBottomDp)) | |
| } |
🤖 Prompt for AI Agents
In
feature/result/src/commonMain/kotlin/com/nexters/emotia/feature/result/letter/LetterScreen.kt
around lines 238 to 240, the keyboard gap is hardcoded with Spacer(height =
210.dp) which breaks across devices; replace the hardcoded value by using the
previously computed imeBottomDp to set the spacer height dynamically (or
alternatively apply windowInsetsPadding(WindowInsets.ime) to the whole section)
so the layout responds to actual IME height instead of a fixed 210.dp.
작업 내용
Summary by CodeRabbit
New Features
Style
Chores