Conversation
WalkthroughCompose 기반의 재사용 가능한 텍스트 필드 컴포넌트 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 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
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt:
- Around line 64-66: The textStyle assignment creates a new TextStyle on every
recomposition by calling inputTextStyle.copy(color = inputTextColor); wrap that
copy call in a remember keyed by inputTextStyle and inputTextColor so a new
object is only created when those values change—update the textStyle usage in
CherrishTextField to use the remembered value derived from inputTextStyle and
inputTextColor.
🧹 Nitpick comments (3)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt (3)
49-55: 중복된 RoundedCornerShape 값을 추출하는 것을 고려하세요.동일한
RoundedCornerShape(10.dp)값이clip과border에서 두 번 사용됩니다. 지역 변수로 추출하면 일관성을 유지하고 향후 수정 시 실수를 줄일 수 있습니다.♻️ 리팩터링 제안
+ val shape = RoundedCornerShape(10.dp) + BasicTextField( value = value, onValueChange = onValueChanged, modifier = modifier - .clip(RoundedCornerShape(10.dp)) + .clip(shape) .background(color = CherrishTheme.colors.gray0) .border( width = 1.dp, color = CherrishTheme.colors.gray500, - shape = RoundedCornerShape(10.dp) + shape = shape ),
56-56: 향후 유연성을 위해singleLine을 파라미터화하는 것을 고려하세요.현재
singleLine = true로 하드코딩되어 있어 여러 줄 입력이 필요한 경우 컴포넌트를 재사용할 수 없습니다. 재사용성을 높이려면singleLine과maxLines를 파라미터로 노출하는 것을 고려하세요.♻️ 파라미터 추가 제안
fun CherrishTextField( value: String, onValueChanged: (String) -> Unit, placeholder: String, placeholderTextStyle: TextStyle, inputTextStyle: TextStyle, inputTextColor: Color, onDoneAction: () -> Unit = {}, paddingValues: PaddingValues, keyboardImeAction: ImeAction = ImeAction.Done, keyboardType: KeyboardType = KeyboardType.Unspecified, placeholderTextColor: Color = CherrishTheme.colors.gray500, + singleLine: Boolean = true, + maxLines: Int = if (singleLine) 1 else Int.MAX_VALUE, modifier: Modifier = Modifier ) { BasicTextField( value = value, onValueChange = onValueChanged, // ... - singleLine = true, + singleLine = singleLine, + maxLines = maxLines,
67-84: 불필요한Column래퍼를 제거하세요.
decorationBox내부의Column은padding을 적용하기 위해서만 사용되고 있습니다.Box에 직접 패딩을 적용하여 불필요한 중첩을 줄이고 컴포지션을 단순화할 수 있습니다.♻️ 단순화 제안
decorationBox = { innerTextField -> - Column( - modifier = Modifier.padding(paddingValues) - ) { - Box( - contentAlignment = Alignment.CenterStart - ) { - if (value.isEmpty()) { - Text( - text = placeholder, - color = placeholderTextColor, - style = placeholderTextStyle - ) - } - innerTextField() + Box( + contentAlignment = Alignment.CenterStart, + modifier = Modifier.padding(paddingValues) + ) { + if (value.isEmpty()) { + Text( + text = placeholder, + color = placeholderTextColor, + style = placeholderTextStyle + ) } + innerTextField() } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: - Jetpack Compose 구조, 상태 관리, recomposition 최적화에 집중
- ViewModel, UiState, 단방향 데이터 흐름 패턴 검토
- 불필요한 recomposition 가능성, remember/derivedStateOf 적절한 사용 확인
- 네이밍 컨벤션, 가독성, Google 권장 Android 아키텍처 준수 여부
Files:
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt (1)
app/src/main/java/com/cherrish/android/core/designsystem/theme/Theme.kt (1)
CherrishTheme(38-61)
⏰ 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: PR Lint Check
- GitHub Check: PR Build Check
🔇 Additional comments (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt (1)
88-104: 프리뷰 구현이 적절합니다.프리뷰 함수가
remember와mutableStateOf를 올바르게 사용하여 로컬 상태를 관리하고 있으며, 컴포넌트의 사용법을 잘 보여주고 있습니다. 다크 배경으로 설정하여 시각적 확인도 용이합니다.
nhyeonii
left a comment
nhyeonii
left a comment
There was a problem hiding this comment.
굿 !!!!!! 텍필 어려울텐데 아주 잘햇고만용 CornerRadius 관련 이슈 반영 부타캐요 !!
| keyboardImeAction: ImeAction = ImeAction.Done, | ||
| keyboardType: KeyboardType = KeyboardType.Unspecified, | ||
| placeholderTextColor: Color = CherrishTheme.colors.gray500, | ||
| modifier: Modifier = Modifier |
There was a problem hiding this comment.
P1 : modifier은 선택인자 순서 중 가장 위로 ~~
| value = value, | ||
| onValueChange = onValueChanged, | ||
| modifier = modifier | ||
| .clip(RoundedCornerShape(10.dp)) |
There was a problem hiding this comment.
P1 : 요거 CornerRadius가 두개가 달라용 !!! 이거 파라미터로 받아오던지 아니면 타입에 따라 다르도록 분기처리 해조야할 것 같슴둥 !!
There was a problem hiding this comment.
헉 !! 파라미터로 분리해두겠습니다
| Column( | ||
| modifier = Modifier.padding(paddingValues) |
There was a problem hiding this comment.
P2 : 요거 Column이 꼭 필요할까용 ?!
이 Column 삭제하고 그냥 바로 Box에 패딩 적용해도 동일하게 구현 가능할거 같아용 !!! Column과 Box를 둘다 사용하신 이유가 잇나용 ?!
decorationBox = { innerTextField ->
Box(
modifier = Modifier.padding(paddingValues),
contentAlignment = Alignment.CenterStart
) {
if (value.isEmpty()) {
Text(
text = placeholder,
color = placeholderTextColor,
style = placeholderTextStyle
)
}
innerTextField()
}
}
There was a problem hiding this comment.
Column으로 구현하려다 보니 오히려 복잡해졌네요 ㅜㅅㅜ 수정하겠습니당!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt:
- Around line 40-62: CherrishTextField currently accepts keyboardImeAction and a
single onDoneAction but only wires KeyboardActions(onDone = ...) so other
ImeAction values (e.g., ImeAction.Search) won't invoke the callback; update the
component to either (a) expose a general callback like onImeAction: (ImeAction)
-> Unit and wire KeyboardActions for all relevant actions (onDone, onSearch,
onGo, onSend, onNext, etc.) to call onImeAction with the corresponding
ImeAction, or (b) keep onDoneAction but map other KeyboardActions
(onSearch/onGo/onSend) to also call onDoneAction so keyboardImeAction values
still trigger the provided callback; adjust the parameters (keyboardImeAction,
onDoneAction) in CherrishTextField and the KeyboardActions wiring accordingly.
🧹 Nitpick comments (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt (1)
32-32: 네이밍 컨벤션:onValueChanged→onValueChange권장Compose의
BasicTextField와TextField는 모두onValueChange를 사용합니다. 일관성을 위해 동일한 네이밍을 사용하는 것이 좋습니다.♻️ 제안하는 수정
fun CherrishTextField( value: String, - onValueChanged: (String) -> Unit, + onValueChange: (String) -> Unit, roundedCornerShape: RoundedCornerShape, ... ) { BasicTextField( value = value, - onValueChange = onValueChanged, + onValueChange = onValueChange, ... ) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: - Jetpack Compose 구조, 상태 관리, recomposition 최적화에 집중
- ViewModel, UiState, 단방향 데이터 흐름 패턴 검토
- 불필요한 recomposition 가능성, remember/derivedStateOf 적절한 사용 확인
- 네이밍 컨벤션, 가독성, Google 권장 Android 아키텍처 준수 여부
Files:
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt (1)
app/src/main/java/com/cherrish/android/core/designsystem/theme/Theme.kt (1)
CherrishTheme(38-61)
⏰ 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: PR Build Check
- GitHub Check: PR Lint Check
🔇 Additional comments (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt (2)
45-82: BasicTextField 구현이 잘 되어 있습니다.
clip→background→border순서의 modifier chain이 올바르게 적용되어 있습니다.textStyle.copy(color = inputTextColor)패턴으로 스타일을 안전하게 확장하고 있습니다.- decorationBox 내부의 placeholder 조건부 렌더링이 깔끔합니다.
85-102: Preview 구현이 적절합니다.
remember { mutableStateOf("") }를 사용한 상태 관리가 올바릅니다.CherrishTheme으로 감싸서 테마 컨텍스트 내에서 컴포넌트를 테스트하고 있습니다.private접근 제한자를 사용하여 preview가 외부에 노출되지 않도록 했습니다.
| onDoneAction: () -> Unit = {}, | ||
| keyboardImeAction: ImeAction = ImeAction.Done, | ||
| keyboardType: KeyboardType = KeyboardType.Unspecified, | ||
| placeholderTextColor: Color = CherrishTheme.colors.gray500 | ||
| ) { | ||
| BasicTextField( | ||
| value = value, | ||
| onValueChange = onValueChanged, | ||
| modifier = modifier | ||
| .clip(roundedCornerShape) | ||
| .background(color = CherrishTheme.colors.gray0) | ||
| .border( | ||
| width = 1.dp, | ||
| color = CherrishTheme.colors.gray500, | ||
| shape = roundedCornerShape | ||
| ), | ||
| singleLine = true, | ||
| keyboardOptions = KeyboardOptions( | ||
| keyboardType = keyboardType, | ||
| imeAction = keyboardImeAction | ||
| ), | ||
| keyboardActions = KeyboardActions( | ||
| onDone = { onDoneAction() } |
There was a problem hiding this comment.
keyboardImeAction과 onDoneAction 간의 불일치 가능성
현재 keyboardImeAction은 모든 ImeAction 값을 받을 수 있지만, KeyboardActions는 onDone만 처리합니다. 만약 사용자가 keyboardImeAction = ImeAction.Search로 설정하면 검색 버튼을 눌러도 onDoneAction이 호출되지 않습니다.
🔧 제안하는 수정 방안
방안 1: 동적으로 IME 액션 처리
- keyboardActions = KeyboardActions(
- onDone = { onDoneAction() }
- ),
+ keyboardActions = KeyboardActions(
+ onDone = { if (keyboardImeAction == ImeAction.Done) onDoneAction() },
+ onSearch = { if (keyboardImeAction == ImeAction.Search) onDoneAction() },
+ onGo = { if (keyboardImeAction == ImeAction.Go) onDoneAction() },
+ onSend = { if (keyboardImeAction == ImeAction.Send) onDoneAction() }
+ ),방안 2: 콜백 이름을 더 일반적인 이름으로 변경
- onDoneAction: () -> Unit = {},
+ onImeAction: () -> Unit = {},🤖 Prompt for AI Agents
In
@app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt
around lines 40 - 62, CherrishTextField currently accepts keyboardImeAction and
a single onDoneAction but only wires KeyboardActions(onDone = ...) so other
ImeAction values (e.g., ImeAction.Search) won't invoke the callback; update the
component to either (a) expose a general callback like onImeAction: (ImeAction)
-> Unit and wire KeyboardActions for all relevant actions (onDone, onSearch,
onGo, onSend, onNext, etc.) to call onImeAction with the corresponding
ImeAction, or (b) keep onDoneAction but map other KeyboardActions
(onSearch/onGo/onSend) to also call onDoneAction so keyboardImeAction values
still trigger the provided callback; adjust the parameters (keyboardImeAction,
onDoneAction) in CherrishTextField and the KeyboardActions wiring accordingly.
hyeminililo
left a comment
hyeminililo
left a comment
There was a problem hiding this comment.
수고하셨습니다 ! 아래 부분 한번 읽어봐주세용 ~~!!
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt (2)
49-68: 커서 색상 고려 제안
BasicTextField에서cursorBrush파라미터가 설정되지 않아 기본 커서 색상이 적용됩니다. 테마와의 일관성을 위해 커서 색상을 명시적으로 지정하는 것을 고려해 보세요.♻️ 커서 색상 추가 예시
+import androidx.compose.ui.graphics.SolidColor BasicTextField( value = value, onValueChange = onValueChanged, + cursorBrush = SolidColor(inputTextColor), modifier = modifier
32-32: Compose 네이밍 컨벤션 확인
onValueChanged파라미터 이름이 Compose의 일반적인 컨벤션인onValueChange와 약간 다릅니다. 프로젝트 내 일관성을 위해 기존 컨벤션을 따르는 것도 고려해 보세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: - Jetpack Compose 구조, 상태 관리, recomposition 최적화에 집중
- ViewModel, UiState, 단방향 데이터 흐름 패턴 검토
- 불필요한 recomposition 가능성, remember/derivedStateOf 적절한 사용 확인
- 네이밍 컨벤션, 가독성, Google 권장 Android 아키텍처 준수 여부
Files:
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt (1)
app/src/main/java/com/cherrish/android/core/designsystem/theme/Theme.kt (1)
CherrishTheme(38-61)
⏰ 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: PR Lint Check
- GitHub Check: PR Build Check
🔇 Additional comments (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/textfield/CherrishTextField.kt (2)
45-47: remember 사용이 적절합니다.
textStyle생성 로직을remember로 분리하고inputTextStyle과inputTextColor를 키로 지정하여 불필요한 recomposition을 방지한 점이 좋습니다. 코딩 가이드라인에 따른 최적화가 잘 적용되었습니다.
87-104: 프리뷰 구현이 잘 되었습니다.로컬 상태 관리에
remember { mutableStateOf }패턴을 올바르게 사용하고,CherrishTheme으로 감싸 테마가 적용된 상태에서 컴포넌트를 확인할 수 있도록 구성한 점이 좋습니다.
sohee6989
left a comment
sohee6989
left a comment
There was a problem hiding this comment.
수고하셨습니다!!
리뷰 한 번만 확인 부탁드립니다!
| imeAction = keyboardImeAction | ||
| ), | ||
| keyboardActions = KeyboardActions( | ||
| onDone = { onDoneAction() } |
There was a problem hiding this comment.
p1: 여기 keyboardActions이 onDone이 항상 아닐수도 있지 않나여??
예를 들어 아래 사진에서 이름 텍필, 날짜의 연도, 월의 텍필은 흐름대로 간다고 했을때 동작이 done이 아니라 next가 맞을 것 같아서요!
There was a problem hiding this comment.
오호 맞는 말이네요 !! Done이랑 Next로 분기 처리하여 수정하겠습니당
|
|
||
| BasicTextField( | ||
| value = value, | ||
| onValueChange = onValueChanged, |
There was a problem hiding this comment.
p3: 여기에서는 아직 동작이 안 일어났으니 파라미터 명을 onValueChange로 해주는거 어떠세요??
4 participants
Related issue 🛠
Work Description ✏️
Screenshot 📸
Uncompleted Tasks 😅
N/A
To Reviewers 📢
배경색과 보더를 제외한 대부분의 속성은 파라미터로 분리해두었습니다.
사용 시 해당 부분 참고해서 활용해주세용 !!
프리뷰 코드도 함께 작성해두었으니 같이 확인 부탁드립니다잉
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.