Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough댓글 입력 필드 배치 로직과 관련 UI 컴포저블들이 리팩토링되었습니다. 주요 변경 사항: 기존 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
chanho0908
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentAnchorFrame.kt (1)
98-115: 가이드 텍스트 구현이 사용자 경험을 향상시킵니다.포커스 상태에서만 가이드 텍스트를 표시하고,
onSizeChanged로 높이를 측정하여 동적으로 위치를 계산하는 방식이 좋습니다. 코멘트 입력창 바로 위에 배치되어 사용자에게 명확한 피드백을 제공합니다.다만,
AnimatedVisibility를 사용하지 않아서 가이드 텍스트가 갑자기 나타나고 사라집니다. 기존 딤 처리에fadeIn()/fadeOut()을 적용한 것처럼 가이드 텍스트에도 애니메이션을 적용하면 더 부드러운 전환이 될 수 있습니다. 이 부분은 선택적 개선 사항입니다.✨ 애니메이션 적용 제안 (선택사항)
- if (uiModel.isFocused) { - AppText( + AnimatedVisibility( + visible = uiModel.isFocused, + enter = fadeIn(), + exit = fadeOut(), + ) { + AppText( text = stringResource(R.string.comment_condition_guide), // ... - ) - } + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentAnchorFrame.kt` around lines 98 - 115, Wrap the existing AppText (the guide text shown when uiModel.isFocused) in an AnimatedVisibility keyed to uiModel.isFocused and apply enter = fadeIn() and exit = fadeOut() so the guide fades in/out instead of appearing abruptly; preserve the current onSizeChanged logic that sets guideTextHeight and the offset calculation using commentTextFieldY and guideTextPaddingBottom inside the AnimatedVisibility content to keep positioning unchanged while adding the animation.core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt (1)
124-135: 접근성(Accessibility) 관련 검토가 필요합니다.숨겨진
TextField(alpha=0, width=0.dp)를 사용하여 커스텀 UI를 구현하는 패턴은 시각적으로는 잘 작동하지만, 스크린 리더(TalkBack) 사용자에게는 문제가 될 수 있습니다. 숨겨진 TextField가 포커스를 받을 때 적절한 콘텐츠 설명을 제공하지 않으면 사용자가 현재 상태를 파악하기 어렵습니다.디자인 시스템 가이드라인에 따른 접근성 고려 여부를 확인해 주세요:
contentDescription또는semanticsmodifier를 통해 현재 입력된 글자 수, 최대 글자 수 등의 정보를 제공하는 것이 좋습니다.♿ 접근성 개선 제안
// TextField에 semantics 정보 추가 예시 Modifier .width(0.dp) .alpha(0f) .semantics { contentDescription = "코멘트 입력 필드, ${internalValue.text.length}/${CommentUiModel.COMMENT_COUNT}자 입력됨" } .focusRequester(focusRequester) // ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt` around lines 124 - 135, The hidden TextField in CommentTextField (the composable using focusRequester, internalValue, and CommentUiModel.COMMENT_COUNT) lacks accessibility semantics; add a semantics modifier on that TextField (or its Modifier chain before focusRequester) that sets a descriptive contentDescription (e.g., "코멘트 입력 필드, X/Y자 입력됨" using internalValue.text.length and CommentUiModel.COMMENT_COUNT) or use clearAndSetSemantics to expose the current char count and purpose to screen readers; ensure this semantics modifier is applied alongside the existing width(0.dp).alpha(0f) so TalkBack users receive the input field label and state when the hidden TextField receives focus.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentAnchorFrame.kt`:
- Around line 98-115: Wrap the existing AppText (the guide text shown when
uiModel.isFocused) in an AnimatedVisibility keyed to uiModel.isFocused and apply
enter = fadeIn() and exit = fadeOut() so the guide fades in/out instead of
appearing abruptly; preserve the current onSizeChanged logic that sets
guideTextHeight and the offset calculation using commentTextFieldY and
guideTextPaddingBottom inside the AnimatedVisibility content to keep positioning
unchanged while adding the animation.
In
`@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt`:
- Around line 124-135: The hidden TextField in CommentTextField (the composable
using focusRequester, internalValue, and CommentUiModel.COMMENT_COUNT) lacks
accessibility semantics; add a semantics modifier on that TextField (or its
Modifier chain before focusRequester) that sets a descriptive contentDescription
(e.g., "코멘트 입력 필드, X/Y자 입력됨" using internalValue.text.length and
CommentUiModel.COMMENT_COUNT) or use clearAndSetSemantics to expose the current
char count and purpose to screen readers; ensure this semantics modifier is
applied alongside the existing width(0.dp).alpha(0f) so TalkBack users receive
the input field label and state when the hidden TextField receives focus.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 70b252f0-9a71-472d-a4ed-ad6b5023258e
📒 Files selected for processing (3)
core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentAnchorFrame.ktcore/design-system/src/main/java/com/twix/designsystem/components/comment/CommentBox.ktcore/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt
💤 Files with no reviewable changes (1)
- core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentBox.kt
| private fun calculateCommentFieldY( | ||
| anchorBottom: Float, | ||
| screenHeight: Float, | ||
| imeBottom: Int, | ||
| navBottom: Int, | ||
| textFieldHeight: Float, | ||
| paddingBottom: Float, | ||
| ): Int { | ||
| // 1. 키보드가 없을 때의 기본 위치: 앵커 하단에서 코멘트창 높이 + 패딩만큼 위 | ||
| val defaultY = anchorBottom - textFieldHeight - paddingBottom | ||
|
|
||
| // 2. 네비게이션 바 인셋을 제외한 키보드 순수 높이 | ||
| // 제스처 내비게이션 환경에서는 navBottom이 0이므로 그대로 imeBottom이 사용됨 | ||
| // 버튼 내비게이션 환경에서는 navBottom만큼 차감하여 실제 키보드 높이만 반영 | ||
| val pureImeHeight = (imeBottom - navBottom).coerceAtLeast(0) | ||
|
|
||
| // 3. 키보드 상단 Y 좌표: 화면 하단에서 키보드 높이만큼 올라간 지점 | ||
| val keyboardTop = screenHeight - pureImeHeight | ||
|
|
||
| // 4. 키보드가 활성화된 경우 코멘트창을 키보드 바로 위에 배치할 Y 좌표 | ||
| val keyboardTopY = keyboardTop - textFieldHeight - paddingBottom | ||
|
|
||
| // 판정: 키보드가 올라와 있고, 기본 위치의 코멘트창 하단이 키보드 상단을 침범하는 경우 | ||
| // 즉 키보드가 조금이라도 코멘트창을 가리면 키보드 위로 즉시 이동 | ||
| val isImeVisible = imeBottom > 0 && (defaultY + textFieldHeight + paddingBottom) > keyboardTop | ||
|
|
||
| return if (isImeVisible) { | ||
| keyboardTopY.toInt() | ||
| } else { | ||
| defaultY.toInt() | ||
| } | ||
| } |
There was a problem hiding this comment.
위치 계산 과정
1. anchorBottom 기준으로 배치
val defaultY = anchorBottom
2. commentTextFieldHeight 만큼 Y축 이동
val defaultY = anchorBottom - textFieldHeight
3. padding 28 만큼 Y축 이동
val defaultY = anchorBottom - textFieldHeight - paddingBottom
🚨 문제: 키보드 활성 시 의도하지 않은 여백 발생
키보드가 올라올 때 screenHeight - imeBottom으로 키보드 상단을 계산하면 아래처럼 예상보다 위쪽에 댓글창이 붙는 현상이 발생했음
val keyboardTop = screenHeight - imeBottom
문제 원인
- WindowInsets.ime의 기준점 오차 발생
키보드 높이(imeBottom)에 시스템 네비게이션 바 인셋의 높이가 포함되어 실제 키보드 상단 좌표가 계산보다 더 높게 잡히는 현상이 발생 !!
문제 해결
navigationBars 인셋을 차감하여 기기별 내비게이션 바 유무와 상관없이 순수 키보드 높이를 구하도록 수정
val pureImeHeight = (imeBottom - navBottom).coerceAtLeast(0)
val keyboardTop = screenHeight - pureImeHeight
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
core/design-system/src/main/res/values/strings.xml (1)
136-136: 버튼 라벨과 화면 타이틀은 string key를 분리하는 편이 안전합니다.왜냐하면 Line 136의
photolog_take_picture는feature/photolog/detail/src/main/java/com/twix/photolog/detail/component/PhotologCardContent.kt:25-43에서는 CTA 버튼 라벨로,feature/photolog/capture/src/main/java/com/twix/photolog/capture/PhotologCaptureScreen.kt:188-193에서는 화면 타이틀로 함께 쓰이고 있어서 이후 한쪽 카피 수정이나 번역 변경이 다른 화면에도 그대로 전파됩니다. 지금처럼 값이"업로드하기"로 바뀌면 key 이름take_picture와 의미도 어긋나서 유지보수 시 혼란이 생깁니다.photolog_cta_upload,photolog_capture_title처럼 역할별 리소스로 분리해 두는 방향은 어떨까요?♻️ 제안하는 리소스 분리 예시
- <string name="photolog_take_picture">업로드하기</string> + <string name="photolog_cta_upload">업로드하기</string> + <string name="photolog_capture_title">업로드하기</string>이후 각 사용처에서 문맥에 맞는 key를 참조하도록 연결하면 됩니다. As per coding guidelines "[리소스 리뷰 가이드] - 다국어 지원을 고려했는가?"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/design-system/src/main/res/values/strings.xml` at line 136, strings.xml에 있는 단일 키 photolog_take_picture이 버튼 라벨(PhotologCardContent.kt)과 화면 타이틀(PhotologCaptureScreen.kt)으로 함께 사용되어 의미가 충돌하므로 역할별로 분리하세요: strings.xml에 photolog_cta_upload(버튼 라벨)와 photolog_capture_title(화면 타이틀) 두 개의 리소스 키를 추가하고 기존 photolog_take_picture 사용처를 각각 PhotologCardContent.kt와 PhotologCaptureScreen.kt에서 새 키로 교체한 뒤 불필요해진 photolog_take_picture 항목은 제거하거나 적절히 대체하세요.core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt (2)
196-204: CursorBar가 깜빡임 애니메이션이 없습니다.일반적인 텍스트 입력 커서는 깜빡이는 애니메이션을 통해 사용자에게 현재 입력 위치를 시각적으로 알려줍니다. 현재 구현은 정적인 막대만 표시되어 UX가 다소 부자연스러울 수 있습니다.
의도적인 디자인 결정인지 확인이 필요합니다. 만약 깜빡임이 필요하다면
rememberInfiniteTransition을 활용할 수 있습니다.💡 깜빡이는 커서 구현 예시
+import androidx.compose.animation.core.RepeatMode +import androidx.compose.animation.core.infiniteRepeatable +import androidx.compose.animation.core.rememberInfiniteTransition +import androidx.compose.animation.core.tween +import androidx.compose.animation.core.animateFloat `@Composable` private fun CursorBar() { + val infiniteTransition = rememberInfiniteTransition(label = "cursor") + val alpha by infiniteTransition.animateFloat( + initialValue = 1f, + targetValue = 0f, + animationSpec = infiniteRepeatable( + animation = tween(500), + repeatMode = RepeatMode.Reverse + ), + label = "cursorAlpha" + ) + Box( modifier = Modifier .width(2.dp) .height(28.dp) - .background(GrayColor.C500), + .background(GrayColor.C500.copy(alpha = alpha)), ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt` around lines 196 - 204, The CursorBar currently renders a static divider; implement a blinking cursor by adding an infinite transition (use rememberInfiniteTransition) inside CursorBar to animate alpha (or visibility) with a repeating tween and apply the animated alpha to the Box via Modifier.alpha(...) or by animating the background color; update the CursorBar function to create the rememberInfiniteTransition, an animateFloat/animateColor with appropriate duration and easing, and use that animated value when building the Box so the cursor blinks.
115-137: 숨겨진 TextField 패턴은 접근성 측면에서 주의가 필요합니다.
width(0.dp)와alpha(0f)로 TextField를 숨기고 커스텀 UI를 표시하는 패턴을 사용하고 있습니다. 이는 커스텀 입력 UI를 구현하는 일반적인 방법이지만, 스크린 리더 사용자에게는 혼란을 줄 수 있습니다.디자인 시스템 가이드라인의 접근성(Accessibility) 고려 사항에 따라,
semanticsmodifier를 통해 적절한 접근성 정보를 제공하는 것을 권장합니다.// 예시: 숨겨진 TextField에 접근성 설명 추가 .semantics { contentDescription = "5글자 코멘트 입력" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt` around lines 115 - 137, The hidden TextField pattern lacks accessibility semantics; update the Modifier chain on the TextField (the one using .width(0.dp).alpha(0f).focusRequester(focusRequester).onFocusChanged { ... }) to include a semantics block that exposes an appropriate contentDescription and current text for screen readers (e.g., .semantics { contentDescription = "Comment input, max ${CommentUiModel.COMMENT_COUNT} characters"; this.text = AnnotatedString(internalValue.text) } or use a string resource), so TextField, internalValue and focusRequester remain but screen readers get meaningful info.core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentFieldBackground.kt (1)
68-74: circleCount가 0 이하일 때 totalWidth 계산에 주의가 필요합니다.
circleCount가 0이면totalWidth = circleSize + circleCenterSpacing * (-1)로 음수가 될 수 있고, 1이면 정상 동작합니다. Canvas 내부에서return@Canvas로 방어하고 있지만, Canvas 자체는 음수 또는 0 너비로 생성될 수 있습니다.현재 사용처(
CommentTextField.kt)에서COMMENT_COUNT = 5로 고정되어 있어 실제 문제가 되진 않지만,internal함수이므로 방어 로직을 함수 초입에 추가하면 더 안전합니다.🛡️ 방어 로직 제안
`@Composable` internal fun CommentFieldBackground( circleCount: Int, circleSize: Dp, circleCenterSpacing: Dp, modifier: Modifier = Modifier, ) { + if (circleCount <= 0) return + val density = LocalDensity.current🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentFieldBackground.kt` around lines 68 - 74, The totalWidth calculation in CommentFieldBackground (using circleSize, circleCenterSpacing and circleCount) can produce a negative or zero width when circleCount <= 0; add a defensive guard at the start of CommentFieldBackground to ensure circleCount is clamped to a minimum of 1 (or compute totalWidth using max(0, circleCount - 1)) before creating the Canvas so width/height passed to Canvas are never non-positive, and keep the existing early return inside the Canvas as an extra safety net.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt`:
- Around line 152-157: The placeholder string indexing in CommentTextField
(inside the char calculation using uiModel, internalValue and index) can throw
IndexOutOfBounds when R.string.comment_text_field_placeholder is shorter than
COMMENT_COUNT; fix by reading the placeholder into a local val (e.g.,
placeholder = stringResource(...)) and use safe access
(placeholder.getOrNull(index) or check length) instead of direct [index], so
char becomes the safe getOrNull result converted toString().orEmpty() for both
focused/unfocused branches.
---
Nitpick comments:
In
`@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentFieldBackground.kt`:
- Around line 68-74: The totalWidth calculation in CommentFieldBackground (using
circleSize, circleCenterSpacing and circleCount) can produce a negative or zero
width when circleCount <= 0; add a defensive guard at the start of
CommentFieldBackground to ensure circleCount is clamped to a minimum of 1 (or
compute totalWidth using max(0, circleCount - 1)) before creating the Canvas so
width/height passed to Canvas are never non-positive, and keep the existing
early return inside the Canvas as an extra safety net.
In
`@core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt`:
- Around line 196-204: The CursorBar currently renders a static divider;
implement a blinking cursor by adding an infinite transition (use
rememberInfiniteTransition) inside CursorBar to animate alpha (or visibility)
with a repeating tween and apply the animated alpha to the Box via
Modifier.alpha(...) or by animating the background color; update the CursorBar
function to create the rememberInfiniteTransition, an animateFloat/animateColor
with appropriate duration and easing, and use that animated value when building
the Box so the cursor blinks.
- Around line 115-137: The hidden TextField pattern lacks accessibility
semantics; update the Modifier chain on the TextField (the one using
.width(0.dp).alpha(0f).focusRequester(focusRequester).onFocusChanged { ... }) to
include a semantics block that exposes an appropriate contentDescription and
current text for screen readers (e.g., .semantics { contentDescription =
"Comment input, max ${CommentUiModel.COMMENT_COUNT} characters"; this.text =
AnnotatedString(internalValue.text) } or use a string resource), so TextField,
internalValue and focusRequester remain but screen readers get meaningful info.
In `@core/design-system/src/main/res/values/strings.xml`:
- Line 136: strings.xml에 있는 단일 키 photolog_take_picture이 버튼
라벨(PhotologCardContent.kt)과 화면 타이틀(PhotologCaptureScreen.kt)으로 함께 사용되어 의미가 충돌하므로
역할별로 분리하세요: strings.xml에 photolog_cta_upload(버튼 라벨)와 photolog_capture_title(화면
타이틀) 두 개의 리소스 키를 추가하고 기존 photolog_take_picture 사용처를 각각 PhotologCardContent.kt와
PhotologCaptureScreen.kt에서 새 키로 교체한 뒤 불필요해진 photolog_take_picture 항목은 제거하거나 적절히
대체하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 6b05593d-e784-4a21-8a4c-29c51891c126
📒 Files selected for processing (4)
core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentCircle.ktcore/design-system/src/main/java/com/twix/designsystem/components/comment/CommentFieldBackground.ktcore/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.ktcore/design-system/src/main/res/values/strings.xml
💤 Files with no reviewable changes (1)
- core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentCircle.kt
core/design-system/src/main/java/com/twix/designsystem/components/comment/CommentTextField.kt
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| package com.twix.designsystem.components.comment | ||
|
|
There was a problem hiding this comment.
Compose의 Stroke는 경계선을 중심으로 그려지기 때문에 2dp stroke를 그리면 안쪽 1dp, 바깥쪽 1dp씩 분배된다고해
기존 구현은 뒤에서 stroke 원을 그리고, 그 위에 같은 크기의 흰 원을 덮는 방식이었기 때문에
안쪽 1dp는 흰 원에 가려지고 바깥쪽 1dp만 보이게 되서 그래서 의도한 2dp border처럼 보이지 않고 실제로는 1dp 정도의 외곽선처럼 보였던 것 같아
이 문제를 해결하기 위해 최종 모양을 하나의 shape로 만든 뒤 그 shape를 직접 그리는 방식으로 수정했어 !
쉽게 말해 기존에는 storke 위에 원이 배치되는, 어떻게 보면 2층짜리 구조였다면 지금은 그걸 하나의 층으로 합쳤어 !
- `TorchButton` 컴포넌트 신규 구현 및 `CameraPreviewBox` 내 적용 - 플래시 관련 이벤트 명칭 변경 (`onClickFlash` -> `onClickTorch`) - 카메라 플래시 On/Off 아이콘 리소스 업데이트 (24dp 규격으로 조정 및 스타일 수정) - `CameraPreviewBox` 내 `TorchIcon`을 신규 구현한 `TorchButton`으로 대체하여 디자인 통일성 강화
- `CommentUiModel`: 댓글 업로드 가능 여부 판단 시 `isEmpty` 대신 `isBlank`를 사용하여 공백만 있는 경우를 필터링하도록 수정 - `CommentTextField`: 입력 값에서 모든 공백 문자를 제거(filterNot)하고 글자 수 제한을 체크하도록 변경하여 비정상적인 입력 방지
1 participant
이슈 번호
#124
작업내용
인증샷 촬영 화면
인증샷 상세 화면
결과물
1. 초기 상태
2. 키보드가 올라왔을 때
3. AppRoundButton 수정
4. CommentTextField Border 수정
5. 리액션바 아이콘 정렬 수정
리뷰어에게 추가로 요구하는 사항 (선택)