코멘트 작성 및 예외처리 기능 추가 #49
Conversation
…sk-certification-comment
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/yapp/twix/main/MainActivity.kt`:
- Around line 25-26: Move the two Window-level calls out of the Composable block
and into onCreate after enableEdgeToEdge(): remove
WindowCompat.setDecorFitsSystemWindows(window, false) and
WindowInsetsControllerCompat(window,
window.decorView).isAppearanceLightStatusBars = true from inside setContent and
place them directly in onCreate (after calling enableEdgeToEdge()) so they run
once on creation rather than on every recomposition.
- Line 26: Current code sets
WindowInsetsControllerCompat(...).isAppearanceLightStatusBars = true which
forces dark icons and breaks readability in dark theme; update MainActivity.kt
to set the status bar style dynamically: if you can use activity-compose 1.12.0+
prefer using SystemBarStyle.auto() (the modern API) to let the system adapt icon
color automatically, otherwise compute the boolean from Compose's
isSystemInDarkTheme() and set
WindowInsetsControllerCompat(...).isAppearanceLightStatusBars =
!isSystemInDarkTheme() so icons invert correctly for dark vs light theme.
In
`@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt`:
- Around line 44-46: Add a brief comment explaining why CIRCLE_GAP can be
negative (clarify CIRCLE_PADDING_START, CIRCLE_SIZE and that overlapping circles
are intentional) and add an accessibility label to each interactive circle:
locate CIRCLE_PADDING_START, CIRCLE_SIZE and CIRCLE_GAP and insert the comment
describing intent; then update the CommentCircle usage (the Modifier.clickable
block) to include semantics { contentDescription =
stringResource(R.string.comment_circle_description, index + 1) } (or equivalent
localized string) so screen readers get meaningful labels for each circle.
In
`@core/design-system/src/main/java/com/twix/designsystem/components/comment/model/CommentUiModel.kt`:
- Line 23: updateComment에서 comment 텍스트를 COMMENT_COUNT로 잘라낼 때
TextFieldValue.selection을 그대로 복사하면 selection이 잘린 텍스트 길이를 초과하여 런타임 문제가 발생할 수 있으니,
updateComment 함수에서 comment.copy(...)를 만들 때 새 텍스트 길이(newLen =
comment.text.take(COMMENT_COUNT).length)를 기준으로 selection.start와 selection.end를
newLen 범위로 클램프하고(start = min(selection.start, newLen), end = min(selection.end,
newLen) 및 start <= end 보장) 그 클램프된 selection을 사용해 새로운 TextFieldValue를 반환하도록
수정하세요.
In
`@feature/task-certification/src/main/java/com/twix/task_certification/TaskCertificationViewModel.kt`:
- Around line 96-106: The upload() function only handles the canUpload == false
path and lacks the actual upload flow; add the missing upload branch for when
uiState.value.commentUiModel.canUpload is true (e.g., launch a coroutine in
viewModelScope to trigger your upload logic, update state via reduce to show
progress/success/failure and call the existing error handlers on failure), and
extract the hard-coded 1500 delay into a named constant (e.g.,
COMMENT_ERROR_DISPLAY_MS) used by the delay call; refer to upload(),
uiState.value.commentUiModel.canUpload, viewModelScope.launch, reduce {
showCommentError() }, reduce { hideCommentError() } and delay to locate and
implement the change.
🧹 Nitpick comments (10)
feature/task-certification/src/main/res/values/strings.xml (1)
4-4: 에러 메시지 문자열과 상수 동기화에 대한 고려가 필요합니다.현재 "5글자"가 문자열에 하드코딩되어 있고,
CommentUiModel.COMMENT_COUNT = 5와 별도로 관리됩니다. 향후 글자 수 제한이 변경되면 두 곳을 모두 수정해야 하므로 불일치 위험이 있습니다.개선 방안으로는 다음을 고려해 보세요:
- 포맷 문자열 사용:
"코멘트는 %d글자로 입력해주세요!"로 변경하고 코드에서 상수 값을 주입- 또는 현재 방식 유지 시, 상수 변경 시 문자열도 함께 수정해야 함을 주석으로 명시
현재 구현도 동작에는 문제가 없으므로, 향후 유지보수 시 참고하시면 됩니다.
core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentCircle.kt (1)
27-42: Dark Mode 지원을 위한 색상 처리를 고려해 보세요.현재
CommonColor.White가 배경색으로 하드코딩되어 있어, Dark Mode에서도 흰색 원이 표시됩니다. 디자인 의도에 따라 다를 수 있지만, Dark Mode 지원이 필요하다면 테마 기반 색상 사용을 권장합니다.-.background(color = CommonColor.White, shape = CircleShape), +.background(color = MaterialTheme.colorScheme.surface, shape = CircleShape),또한
CursorBar가Box중앙에 배치되어 텍스트와 겹칠 수 있습니다. 커서가 텍스트 다음 위치에 표시되어야 한다면, 텍스트와 커서의 정렬 조정이 필요할 수 있습니다. 현재 디자인 의도가 중앙 커서라면 무시하셔도 됩니다.core/design-system/src/main/java/com/twix/designsystem/components/comment/model/CommentUiModel.kt (1)
17-21:canUpload조건식을 단순화할 수 있습니다.현재 조건:
comment.text.isEmpty() || comment.text.isNotEmpty() && hasMaxCommentLength
hasMaxCommentLength가 true면 이미isNotEmpty()를 의미하므로, 다음과 같이 단순화할 수 있습니다:val canUpload: Boolean get() = - comment.text.isEmpty() || - comment.text.isNotEmpty() && - hasMaxCommentLength + isEmpty || hasMaxCommentLength가독성과 의도 전달이 더 명확해집니다.
core/design-system/src/main/java/com/twix/designsystem/keyboard/KeyBoardAsState.kt (2)
1-1: 파일명 컨벤션 확인이 필요합니다.현재 파일명이
KeyBoardAsState.kt로 "Board"가 분리되어 있습니다. Kotlin 파일명 컨벤션에 따르면KeyboardAsState.kt가 더 적합합니다.
23-33: 키보드 상태 감지의 성능 최적화를 고려해보세요.현재 구현은 기능적으로 동작하고
WindowInsets.ime.getBottom(density)접근 방식도 표준적입니다. 다만 한 가지 개선할 수 있는 부분이 있습니다.왜 개선이 필요한가:
rememberUpdatedState에서 읽는keyboard값이 composition 중에 계산되므로,WindowInsets.ime이 변할 때마다 (IME 애니메이션 중 매 프레임) 리컴포지션이 발생합니다. 이는 불필요한 성능 오버헤드를 유발합니다. Compose 공식 문서에서도 composition 중 WindowInsets 읽기를 피하도록 권장합니다.개선 방안:
snapshotFlow와distinctUntilChanged()를 조합하면 실제 상태 변화(Closed → Opened 또는 Opened → Closed)만 감지하고, 애니메이션 중간 프레임은 무시할 수 있습니다:`@Composable` fun keyboardAsState(): State<Keyboard> { val state = remember { mutableStateOf(Keyboard.Closed) } val density = LocalDensity.current LaunchedEffect(Unit) { snapshotFlow { WindowInsets.ime.getBottom(density) > 0 } .distinctUntilChanged() .map { if (it) Keyboard.Opened else Keyboard.Closed } .collect { state.value = it } } return state }현재 구현도 충분히 동작하지만, 리스트 스크롤이나 복잡한 UI에서 IME 애니메이션 성능이 중요하다면 이 패턴을 고려해볼 가치가 있습니다. 어떤 접근이 이 프로젝트의 성능 요구사항에 더 적합한지 토론해보시겠어요?
feature/task-certification/src/main/java/com/twix/task_certification/component/CameraPreviewBox.kt (1)
33-71: 잘 구현되었습니다! State Hoisting 패턴이 잘 적용되었네요.
CommentUiModel과 관련 콜백들을 상위 컴포넌트로부터 받아CommentBox에 전달하는 방식이 MVI 패턴의 단방향 데이터 플로우와 잘 맞습니다.다만, 한 가지 개선 제안이 있습니다:
- Line 49의
375.66.dp와 Line 54의73.83.dp는 디자인 스펙에서 가져온 값으로 보이는데, 이러한 매직 넘버들을 상수로 추출하면 유지보수성이 향상됩니다.private val CAMERA_PREVIEW_SIZE = 375.66.dp private val CAMERA_PREVIEW_CORNER_RADIUS = 73.83.dpfeature/task-certification/src/main/java/com/twix/task_certification/component/CommentErrorText.kt (1)
24-38:clip()수정자가 중복됩니다.
background(color, shape)는 이미 shape를 적용하므로, 동일한 shape로clip()을 호출하는 것은 불필요합니다.clip()은 자식 컴포넌트가 부모 경계를 넘어갈 때만 필요합니다.또한, 에러 메시지는 스크린 리더 사용자를 위해 접근성 처리를 고려해 보시는 것이 좋습니다.
♻️ 개선 제안
Box( modifier = modifier .width(224.dp) .height(56.dp) - .background(color = GrayColor.C400, shape = RoundedCornerShape(12.dp)) - .clip(RoundedCornerShape(12.dp)), + .background(color = GrayColor.C400, shape = RoundedCornerShape(12.dp)) + .semantics { error("Comment validation error") }, ) {core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt (1)
89-94: TODO 주석이 있습니다.
noClickableRipple로 수정하라는 TODO가 있는데, 이 작업을 이 PR에서 처리하실 건가요? 아니면 별도 이슈로 트래킹하시겠어요?이슈 생성을 도와드릴 수 있습니다. 원하시면 말씀해 주세요.
feature/task-certification/src/main/java/com/twix/task_certification/TaskCertificationScreen.kt (2)
255-305:DimmedScreen컴포저블의 가시성을 확인해 주세요.
DimmedScreen이public으로 선언되어 있는데, 이 컴포넌트가 다른 곳에서 재사용될 예정인가요? 만약TaskCertificationScreen내부에서만 사용된다면private으로 변경하는 것이 적절합니다.또한, 복잡한 오버레이 로직이 잘 구현되어 있습니다!
CompositingStrategy.Offscreen과BlendMode.Clear를 사용하여 원형 구멍을 뚫는 방식이 효과적입니다.♻️ 가시성 수정 제안
`@Composable` -fun DimmedScreen( +private fun DimmedScreen( isFocused: Boolean, textFieldRect: Rect, guideTextRect: Rect, focusManager: FocusManager, ) {
211-221: AnimatedContent 사용이 적절합니다.에러 상태와 타이틀 간의 전환을
AnimatedContent로 처리한 것이 좋습니다.추가 개선 제안:
label파라미터를 추가하면 Compose 도구에서 디버깅할 때 도움이 됩니다.AnimatedContent( targetState = uiState.showCommentError, label = "TitleErrorTransition" ) { isError ->
| WindowCompat.setDecorFitsSystemWindows(window, false) | ||
| WindowInsetsControllerCompat(window, window.decorView).isAppearanceLightStatusBars = true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 먼저 MainActivity.kt 파일을 확인해서 실제 코드 구조를 봅시다
cat -n app/src/main/java/com/yapp/twix/main/MainActivity.kt | head -50Repository: YAPP-Github/Twix-Android
Length of output: 1952
🌐 Web query:
Android Compose WindowCompat.setDecorFitsSystemWindows best practices location onCreate SideEffect
💡 Result:
- Prefer
Activity.enableEdgeToEdge()(AndroidX Activity 1.8.0+), notWindowCompat.setDecorFitsSystemWindows(...), for modern edge-to-edge. Google recommendsenableEdgeToEdge()because it handles backward-compat details (including scrims) for you. [6], [4], [1] - If you do use
WindowCompat.setDecorFitsSystemWindows(window, false), call it in yourActivity.onCreate()(beforesetContent { ... }) to avoid timing/flicker issues and because it’s a window-level configuration. The official “manual edge-to-edge” guide shows it inonCreate. [1] - Avoid doing this from a Compose
SideEffect: window APIs have “must be called at the correct lifecycle time” constraints; wrapping them in Compose effects can violate that expectation (this is one reason Accompanist System UI Controller was deprecated). [6] - Don’t mix with legacy
setSystemUiVisibilityflags: AndroidX explicitly warns these can conflict withsetDecorFitsSystemWindows. [0] - Once
decorFitsSystemWindows=false, you must handle insets in Compose (e.g.,statusBarsPadding(),navigationBarsPadding(),WindowInsets,imePadding()for keyboard), because the system won’t auto-fit content anymore. (This is the intended behavior ofsetDecorFitsSystemWindows(false).) [0]
Typical setup (manual):
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
WindowCompat.setDecorFitsSystemWindows(window, false) // before setContent
setContent { /* handle WindowInsets in Compose */ }
}🏁 Script executed:
# 확인: TwixTheme에서 테마 관련 설정이 있는지 체크
find . -name "TwixTheme*" -o -name "*Theme.kt" | head -20Repository: YAPP-Github/Twix-Android
Length of output: 139
Window 설정을 onCreate에서 setContent 이전으로 이동해주세요.
현재 코드에서 WindowCompat.setDecorFitsSystemWindows와 WindowInsetsControllerCompat 호출이 setContent 블록 내부(Composable 영역)에 있어 매 재구성마다 반복 실행됩니다. Window 레벨 설정은 특정 생명주기 시점에만 호출되어야 하므로, 이를 onCreate에서 enableEdgeToEdge() 다음에 이동하는 것이 안드로이드 공식 가이드라인입니다.
🔧 수정 예시
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
enableEdgeToEdge()
+ WindowCompat.setDecorFitsSystemWindows(window, false)
+ WindowInsetsControllerCompat(window, window.decorView).isAppearanceLightStatusBars = true
setContent {
val toastManager by inject<ToastManager>()
- WindowCompat.setDecorFitsSystemWindows(window, false)
- WindowInsetsControllerCompat(window, window.decorView).isAppearanceLightStatusBars = true
TwixTheme {📝 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.
| WindowCompat.setDecorFitsSystemWindows(window, false) | |
| WindowInsetsControllerCompat(window, window.decorView).isAppearanceLightStatusBars = true | |
| override fun onCreate(savedInstanceState: Bundle?) { | |
| super.onCreate(savedInstanceState) | |
| enableEdgeToEdge() | |
| WindowCompat.setDecorFitsSystemWindows(window, false) | |
| WindowInsetsControllerCompat(window, window.decorView).isAppearanceLightStatusBars = true | |
| setContent { | |
| val toastManager by inject<ToastManager>() | |
| TwixTheme { |
🤖 Prompt for AI Agents
In `@app/src/main/java/com/yapp/twix/main/MainActivity.kt` around lines 25 - 26,
Move the two Window-level calls out of the Composable block and into onCreate
after enableEdgeToEdge(): remove WindowCompat.setDecorFitsSystemWindows(window,
false) and WindowInsetsControllerCompat(window,
window.decorView).isAppearanceLightStatusBars = true from inside setContent and
place them directly in onCreate (after calling enableEdgeToEdge()) so they run
once on creation rather than on every recomposition.
이슈 번호
#38
리뷰/머지 희망 기한 (선택)
2026.02.03
작업내용
결과물
xc.mp4
리뷰어에게 추가로 요구하는 사항 (선택)