[FEAT/#6] 버튼 공통 컴포넌트 구현#10
Conversation
Walkthrough재사용 가능한 Compose 버튼 두 가지를 추가합니다: full-width 레이아웃의 Changes
Sequence Diagram(s)(생성 조건 미충족 — 생략) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
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: 3
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.kt:
- Around line 46-72: The remember blocks computing backgroundColor and textColor
use cherrishColor but do not include it in their dependency keys, so theme
changes won't update colors; fix by either adding cherrishColor to the remember
dependency list for both backgroundColor and textColor (i.e., remember(enabled,
style, cherrishColor) { ... }) or remove remember entirely and compute the
colors directly (preferred for this simple logic) inside the composable while
keeping the existing when branches for CherrishButtonStyle and enabled.
- Around line 54-56: The SECONDARY branch in CherrishBasicButton
(CherrishButtonStyle.SECONDARY) currently always returns gray400/gray700; update
both the background-color and text-color logic in the CherrishBasicButton
composable (where the when handles CherrishButtonStyle) to honor the enabled
flag: when enabled use the existing gray400 background and gray700 text, and
when disabled return the same disabled palette used by PRIMARY (background
gray200 and text gray600) so SECONDARY matches PRIMARY’s disabled visual
feedback.
🧹 Nitpick comments (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.kt (2)
76-76: 버튼 너비를 유연하게 사용할 수 있도록 개선을 고려해보세요.현재 버튼에
fillMaxWidth()가 기본적으로 적용되어 항상 전체 너비를 차지합니다. 사용 사례에 따라 버튼이 콘텐츠 너비만큼만 차지해야 할 수도 있습니다.Preview 함수(line 134, 140)에서
Modifier.weight()를 사용하는 것으로 보아 유연한 너비 조절이 필요한 경우가 있을 것으로 보입니다.fillMaxWidth()를 기본 modifier에서 제거하고, 호출하는 쪽에서 필요시 추가하는 방식을 고려해보세요.♻️ 제안하는 수정 방법
Row( modifier = modifier - .fillMaxWidth() .scale(scale = scale) .clip(shape = RoundedCornerShape(12.dp)) // ... ) {그리고 호출하는 쪽에서:
CherrishBasicButton( text = "다음", onClick = {}, modifier = Modifier.fillMaxWidth() // 필요한 경우에만 )
82-82: 리플 효과 제거가 접근성에 미치는 영향을 고려해주세요.
indication = null로 설정하여 Material의 기본 리플 효과를 제거했습니다. 커스텀 스케일 애니메이션(lines 40-44)이 시각적 피드백을 제공하지만, 리플 효과는 표준 Material 접근성 패턴의 일부입니다.현재 구현도 충분한 시각적 피드백을 제공하지만, 추후 접근성 테스트에서 사용자 피드백이 충분한지 확인하는 것이 좋습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.ktapp/src/main/java/com/cherrish/android/core/designsystem/type/CherrishButtonStyle.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/type/CherrishButtonStyle.ktapp/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.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/type/CherrishButtonStyle.kt (1)
3-6: 잘 구현된 enum 클래스입니다.버튼 스타일을 표현하는 간결하고 명확한 enum 정의입니다. PRIMARY와 SECONDARY 스타일 구분이 적절합니다.
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.kt (1)
98-144: Preview 함수들이 잘 작성되었습니다.두 개의 Preview 함수가 다양한 사용 사례를 잘 보여줍니다:
- enabled/disabled 상태 비교
- 스타일 변형 (PRIMARY, SECONDARY)
- weight를 사용한 레이아웃 시나리오
이는 컴포넌트의 사용법을 문서화하고 개발 중 빠른 시각적 확인을 가능하게 합니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/java/com/cherrish/android/presentation/main/component/MainBottomBar.kt (1)
47-68: 레이아웃 구조가 개선되었습니다.단일 Row 구조로 단순화하고
key(tab.route)를 사용하여 안정적인 recomposition을 보장하는 점이 좋습니다.navigationBarsPadding()도 적절히 적용되었습니다.다만
spacedBy(50.dp)의 고정 간격은 다양한 화면 크기에서 일관성이 떨어질 수 있습니다. 필요시 화면 너비에 비례하는 동적 간격을 고려해보세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/com/cherrish/android/presentation/main/MainTab.ktapp/src/main/java/com/cherrish/android/presentation/main/component/MainBottomBar.ktapp/src/main/res/drawable/ic_calendar.xmlapp/src/main/res/drawable/ic_challenge.xmlapp/src/main/res/drawable/ic_home.xmlapp/src/main/res/drawable/ic_mypage.xml
✅ Files skipped from review due to trivial changes (2)
- app/src/main/res/drawable/ic_mypage.xml
- app/src/main/res/drawable/ic_calendar.xml
🧰 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/presentation/main/MainTab.ktapp/src/main/java/com/cherrish/android/presentation/main/component/MainBottomBar.kt
⏰ 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 (5)
app/src/main/res/drawable/ic_challenge.xml (1)
1-18: 챌린지 아이콘 리소스가 올바르게 구현되었습니다.벡터 드로어블이 표준 포맷으로 정의되어 있으며, 24x24 dp 크기와 투명 배경, 두 가지 색상의 스트로크가 적절히 설정되어 있습니다.
app/src/main/res/drawable/ic_home.xml (1)
1-19: 홈 아이콘 리소스가 올바르게 구현되었습니다.벡터 드로어블이 표준 포맷으로 정의되어 있으며, 디자인 시스템의 색상(#FF617B, #1B1D1F)을 적절히 사용하고 있습니다.
app/src/main/java/com/cherrish/android/presentation/main/component/MainBottomBar.kt (2)
78-78: 리플 효과 제거가 의도된 것인지 확인이 필요합니다.
noRippleClickable을 사용하여 탭 클릭 시 리플 효과가 제거되었습니다. Material Design 가이드라인에서는 일반적으로 시각적 피드백을 권장하므로, 디자인 요구사항에 따라 의도된 변경인지 확인하시기 바랍니다.
86-91: 테마 기반 색상 처리가 적절합니다.선택된 탭에서
Color.Unspecified를 사용하여 아이콘의 원본 색상을 유지하고, 미선택 상태에서gray500을 일관되게 적용한 점이 좋습니다. 테마 색상 사용도 적절합니다.app/src/main/java/com/cherrish/android/presentation/main/MainTab.kt (1)
17-35: 아이콘 리소스 업데이트가 올바르게 적용되었습니다.각 탭에 대해 generic launcher 아이콘 대신 구체적인 아이콘 리소스로 변경되어 UI 일관성이 향상되었습니다. PR에서 추가된 drawable 리소스들과 잘 연동됩니다.
This reverts commit 1df3d47. git revert HEAD#
6c2a139 to
ff1b2e0
Compare
hyeminililo
left a comment
hyeminililo
left a comment
There was a problem hiding this comment.
코드 진짜아 깔끔하네요오 ,, 저도 본받아서 컴포넌트 구성해보겠습니다 !
| val cherrishColor = CherrishTheme.colors | ||
| val isPressed by interactionSource.collectIsPressedAsState() | ||
|
|
||
| val scale by animateFloatAsState( |
There was a problem hiding this comment.
이 부분은 임의대로 지정하면 되는건가요 ? 저도 구현하려고 하는데 피그마에는 따로 명시되어있지 않은것 같아 궁금해 여쭤봅니다 :)
There was a problem hiding this comment.
혜민이 애니메이션이 들어가는 부분이 있나요 ?! 버튼 같은 경우는 눌렀을 때의 상태도 피그마에 디자인이 들어가있어서 디자인 애니메이션 스펙 확인하고 적용했습니다! 현재 혜민이 칩 같은 경우에는 애니메이션이 들어가지 않은거 같아요 ~~ 그래서 따로 구현이 필요하지 않을거 같네용 !
usuuhyn
left a comment
usuuhyn
left a comment
There was a problem hiding this comment.
버튼 공통 컴포넌트 깔끔하네용 😻 수고하셨습니다!
| } | ||
| } | ||
|
|
||
| val textColor = remember(enabled, style) { |
There was a problem hiding this comment.
사소한 질문인데용 여기서 remember 없이 when으로 바로 색을 계산해도 괜찮을지 궁금합니다!
색상 정도는 리컴포지션 비용이 크지 않을 것 같아서요 !!!
There was a problem hiding this comment.
움움 단순 색상 참조 반환이라 비용이 크지 않겟지만 저는 remember를 유지하는 게 좋다고 생각합니닿ㅎㅎㅎ !! enabled/style이 변하지 않으면 when 블록 자체를 아예 건너뛸 수 있어서 아무리 가벼워도 0번 실행이 여러 번 실행보다는 낫다고 생각이 들기두 하구요 우하하 …
| style: CherrishButtonStyle = CherrishButtonStyle.PRIMARY, | ||
| interactionSource: MutableInteractionSource = remember { MutableInteractionSource() } | ||
| ) { | ||
| val cherrishColor = CherrishTheme.colors |
There was a problem hiding this comment.
지금 cherrishColor.gray0를 여러 곳에서 쓰고 있어서 타이포랑 다르게 cherrishColor 형태로 한 번 받아서 사용해주신 거 같으신데요..! 개인적으로 이 부분들의 접근 방식을 통일시켜 사용하는게 어떨까 하는 생각이 듭니다 ㅎ.ㅎ
추가로 저는 여러군데 써도 타이포랑 컬러를 CherrishTheme.colors.~, CherrishTheme.typography.~ 이렇게 쓰는 편이어서 이 부분 관련해서 저희 컨벤션을 정해서 다같이 통일시키는게 좋을 것 같다는 생각이 듭니다!
There was a problem hiding this comment.
제가 얘를 따로 빼서 작성한 이유가 잇답니다 후후 !! 바로 remember 블록 안에서 CherrishTheme.colors.~ 이런식의 호출이 불가넝합니다 .. !! CherrishTheme.colors는 @Composable 프로퍼티라서 remember {} 내부처럼 Composable 컨텍스트가 아닌 곳에서 직접 호출할 수 없어용 !! 그래서 Composable 스코프에서 미리 colors를 받아 사용하도록 구조를 분리한거랍니도 !
There was a problem hiding this comment.
아하아하 맞네여 remember안에서 저게 안되네여!!
답변 감사합니당~~
| val cherrishColor = CherrishTheme.colors | ||
| val isPressed by interactionSource.collectIsPressedAsState() | ||
|
|
||
| val scale by animateFloatAsState( |
There was a problem hiding this comment.
이번 솝트에서 애니메이션 자주 만나네. . ... .... ... . .. . ... ..
| label = "button-scale" | ||
| ) | ||
|
|
||
| val backgroundColor = remember(enabled, style) { |
There was a problem hiding this comment.
button을 PRIMARY, SECONDARY으로 나눠서 너무 깔끔하게 잘 만들어주신거 같아요!
근데 피그마 보며 색상, 타이포 등을 더블체크하면서 든 생각과 관련해 나현님이 생각이 궁금해 질문 하나 남깁니다..!!
저는 개인적으로 기본 버튼 답게 추후 버튼 추가 생성시 이 기본 버튼을 기반으로 사용하여 추가 버튼 컴포넌트 구현에 대한 확장이 편해야 될 것 같다는 생각이 드는데요..!
처음 봣을 때 이렇게 나눠주신게 너무 깔끔하게 잘 나눠주셨다고 느꼈는데 한편으로는 이 형태의 기본 컴포넌트일 경우, 배경색이 다른 버튼을 만들어야 할 경우 상위에서 확장해서 사용할 수가 없고 내부 색상, 타이포를 건드려야 하는 경우가 생길수도 있겠더라고요...!
그렇게 되면 기본 버튼이라는 의미가 무색하다는 생각이 들어 이것 관련해서 의견이 궁금합니다!!
There was a problem hiding this comment.
우와앙 이 리뷰 너무 좋아요 .../// 사실 저도 확장성에 대해 고민을 마니 햇어요 !! 현재 PRIMARY / SECONDARY로 나눈 이유는 현 시점에서 디자인 가이드가 명확하게 정의된 버튼 타입이 이 두 가지이고 앱 전반에서 반복적으로 사용되는 버튼의 기본 UX(색상, 타이포, 인터랙션)를 우선 통일하는 것이 목적이엇어요 !! 말씀 주신 것처럼 추후 배경색이나 타이포가 다른 버튼이 추가될 경우
이 컴포넌트를 그대로 상속, 확장하기 어려울 수 있다는 점에는 공감해요 ! 다만 현재 단계에서는 모든 케이스를 포괄하는 베이스 버튼보다는 디자인 시스템에서 합의된 공통 컴포넌트인 버튼 타입을 명시적으로 제공하는 방향이 앱잼에서의 사용성과 일관성 측면에서 더 적합하다고 판단하여 이렇게 코드를 작성햇던건데용 !!
음 생각해보니 그렇게 되면 CherrishBasicButton이라는 이름보다는 그냥 CherrishButton이라고 네이밍을 바꾸는 것도 나쁘지 않은 방법일 것 같아요 ㅎㅎ 소희님의 생각은 어떠신가요 ?!? 아예 BasicButton을 따로 구현하는게 더 좋은 방법일 것 같나용 ?!?!?
There was a problem hiding this comment.
디자인 시스템에서 합의된 공통 컴포넌트를 구현하려고 하신바는 공감합니다!!
이것 관련해서 저는 개인적으로 BasicButton을 하나 만들고 CherrishButton을 만드는게 좋을 것 같습니다..!!ㅎㅎ
그래도 앱잼만 보기보다는 그 이후도 봐서 기본 버튼을 만든 후 기본 버튼 가지고 CherrishButton을 현재 코드처럼 만들어
버튼이 달라져도 BasicButton 가지고 확장을 쉽게 하는 게 좋지 않을까 하는 의견 남겨봅니다..!!ㅎㅎㅎ
There was a problem hiding this comment.
일단 CherrishBasicButton을 구현해두긴 했는데 내부 패딩 값을 파라미터로 받는 부분이 좀 고민되네요. BasicButton의 역할이 기본적인 버튼 구조 제공인데 padding 값까지 외부에서 주입받는 게 과연 맞는 활용인지 의문이 들어요. BasicButton이 너무 많은 책임을 지게 되는 것 같아요이 (구조 + clickable + padding 주입) !! Cherrish 디자인 시스템에서 버튼의 padding은 표준값(10.dp)이 있는데 이걸 매번 커스터마이징 가능하게 하는 게 일관성 측면에서 좋을까라는 고민도 되네유 ~~ 너무 확장성에 대해 생각하다보니 오히려 지금 단계에서 필요한 책임 분리 기준을 넘어서 설계가 복잡해지는 건 아닐지 고민이 드는 것두 사실입니다 ! 한번 확인해보시고 조은 방법이 잇으시다면 추천해쥬세요 ~~
=> 코드를 가만히 보다보니 내부 패딩 값을 인자로 받는 것 보단 체이닝 순서를 바꿔서 .then(modifier)를 사용하여 fillMaxWidth() → clickable() → background() → padding() 이런 순서로 패딩 적용되게 하면 터치 영역도 잘 반영될거 같네여 ~~
| when (style) { | ||
| CherrishButtonStyle.PRIMARY -> { | ||
| when { | ||
| !enabled -> cherrishColor.gray200 |
There was a problem hiding this comment.
요거 사소하지만 enable 기준 분기 처리를 아래처럼 해줘도 괜찮을 것 같아요!
| !enabled -> cherrishColor.gray200 | |
| enabled -> cherrishColor.red700 | |
| else -> cherrishColor.gray200 |
부정 조건보다는 이렇게 쓰는게 더 직관적이여서 가독성 면에서 더 좋은 것 같다는 생각이 들어서요!!ㅎㅎ
There was a problem hiding this comment.
오왕 조아요 !! 반영하겟스비나 ㅎㅎㅎ
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.kt (1)
27-27: 기본 매개변수에서remember사용은 권장되지 않습니다.기본 매개변수 표현식은 호출 시점에 평가되므로, 호출자가
interactionSource를 제공하지 않으면 매 recomposition마다 새로운 인스턴스가 생성될 수 있습니다. 이는 상태 유실 및 불필요한 recomposition을 야기할 수 있습니다.♻️ 권장 수정안
@Composable fun CherrishBasicButton( onClick: () -> Unit, modifier: Modifier = Modifier, enabled: Boolean = true, contentPadding: PaddingValues = PaddingValues(10.dp), - interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, + interactionSource: MutableInteractionSource? = null, content: @Composable () -> Unit ) { + val actualInteractionSource = interactionSource ?: remember { MutableInteractionSource() } Row( modifier = modifier .fillMaxWidth() .clickable( enabled = enabled, indication = null, - interactionSource = interactionSource, + interactionSource = actualInteractionSource, onClick = onClick )app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt (2)
34-34: 기본 매개변수에서remember사용 문제
CherrishBasicButton과 동일한 문제입니다. 호출자가interactionSource를 제공하지 않으면 매 recomposition마다 새 인스턴스가 생성될 수 있습니다.♻️ 권장 수정안
@Composable fun CherrishButton( text: String, onClick: () -> Unit, modifier: Modifier = Modifier, enabled: Boolean = true, style: CherrishButtonStyle = CherrishButtonStyle.PRIMARY, - interactionSource: MutableInteractionSource = remember { MutableInteractionSource() } + interactionSource: MutableInteractionSource? = null ) { val cherrishColor = CherrishTheme.colors + val actualInteractionSource = interactionSource ?: remember { MutableInteractionSource() } val isPressed by actualInteractionSource.collectIsPressedAsState()
45-71:remember키에cherrishColor누락
backgroundColor와textColor계산 시cherrishColor를 사용하지만,remember의 키로 포함되어 있지 않습니다. 테마 색상이 동적으로 변경되는 경우 캐시된 값이 갱신되지 않을 수 있습니다.현재 앱에서 테마 색상이 고정되어 있다면 실제 문제는 발생하지 않겠지만, 다크모드 지원 등 향후 확장성을 고려하면 수정하는 것이 좋습니다.
♻️ 선택적 수정안
remember를 제거하고 매번 계산하거나,cherrishColor를 키에 추가:- val backgroundColor = remember(enabled, style) { + val backgroundColor = remember(enabled, style, cherrishColor) { when (style) { CherrishButtonStyle.PRIMARY -> { when { enabled -> cherrishColor.red700 else -> cherrishColor.gray200 } } CherrishButtonStyle.SECONDARY -> { cherrishColor.gray400 } } } - val textColor = remember(enabled, style) { + val textColor = remember(enabled, style, cherrishColor) { when (style) {또는 간단한
when표현식이므로remember없이 직접 계산해도 성능 차이가 미미합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.ktapp/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.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/button/CherrishButton.ktapp/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.kt
🧬 Code graph analysis (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.kt (1)
CherrishBasicButton(21-45)app/src/main/java/com/cherrish/android/core/designsystem/theme/Theme.kt (1)
CherrishTheme(38-61)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.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 (3)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.kt (1)
21-45: LGTM!전반적인 구조가 잘 설계되어 있습니다.
contentslot 패턴을 활용한 재사용 가능한 기본 버튼 컴포넌트이며, modifier 순서도 적절합니다.app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt (2)
36-43: LGTM!Press 상태 애니메이션 구현이 잘 되어 있습니다.
collectIsPressedAsState()와animateFloatAsState를 활용한 80ms 스케일 애니메이션이 적절합니다.
73-87: LGTM!
CherrishBasicButton을 잘 활용하여 스타일이 적용된 버튼을 구성했습니다. Modifier 순서(scale → clip → background)도 올바릅니다.
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/button/CherrishButton.kt:
- Around line 45-61: The remember blocks for backgroundColor and textColor read
cherrishColor (from CherrishTheme.colors) but do not include it in the remember
keys, so the remembered colors won't update when the theme changes; fix by
removing remember and compute the colors directly (i.e., assign backgroundColor
and textColor from the when(...) expressions without remember) OR include
cherrishColor in the remember keys (e.g., remember(enabled, style,
cherrishColor)), updating the logic in the backgroundColor and textColor
assignments and keeping references to CherrishButtonStyle.PRIMARY/SECONDARY
intact.
🧹 Nitpick comments (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt (1)
105-125: weight 값을 더 의미 있는 비율로 표현하는 것을 고려하세요.
184f와122f는 특정 픽셀 값에서 파생된 것으로 보이지만, 비율 기반 레이아웃에서는 더 간단한 값(예:1.5f,1f또는184f/306f,122f/306f)을 사용하는 것이 의도를 더 명확하게 전달합니다. 다만 디자인 스펙을 정확히 따르기 위한 것이라면 현재 구현도 적절합니다.♻️ 비율 기반 weight로 리팩토링 예시
CherrishButton( text = "다운타임 없이 일정 추가", onClick = {}, style = CherrishButtonStyle.SECONDARY, - modifier = Modifier.weight(184f) + modifier = Modifier.weight(1.5f) // 또는 Modifier.weight(184f / 306f) ) CherrishButton( text = "확인", onClick = {}, - modifier = Modifier.weight(122f) + modifier = Modifier.weight(1f) // 또는 Modifier.weight(122f / 306f) )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.ktapp/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.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/button/CherrishButton.kt
🧬 Code graph analysis (1)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt (2)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishBasicButton.kt (1)
CherrishBasicButton(19-42)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 (4)
app/src/main/java/com/cherrish/android/core/designsystem/component/button/CherrishButton.kt (4)
28-35: 함수 시그니처가 명확하고 잘 설계되었습니다.매개변수 순서와 기본값 설정이 Compose 컨벤션을 따르고 있습니다.
36-43: pressed 상태 애니메이션이 잘 구현되었습니다.
collectIsPressedAsState()와animateFloatAsState를 사용한 스케일 애니메이션이 적절합니다. 80ms 지속 시간도 빠른 피드백을 제공하기에 적합합니다.
81-99: 프리뷰가 잘 구성되었습니다.enabled/disabled 상태를 모두 보여주는 기본 프리뷰가 명확하고 유용합니다.
63-79: CherrishBasicButton의 modifier 체인 동작이 의도된 설계입니다.코드 검토 결과,
fillMaxWidth()→scale(0.95f)적용으로 인한 너비 축소는 의도된 press 피드백 메커니즘입니다. 명시적animateFloatAsState(tween(80ms))설정과 preview에서의 정상 동작으로 보아, 이는 Material Design 표준 상호작용 패턴입니다. 별도 수정이 필요하지 않습니다.
4 participants
Related issue 🛠
Work Description ✏️
Screenshot 📸
2026-01-08.7.22.55.mov
Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.