Conversation
…to feat/#44-collectioncreate-toggle
chanmi1125
added
🧩 Component
📝 Walkthrough워크스루새로운 공개 컴포저블 변경사항
Sequence Diagram(s)(생성 조건에 부합하지 않아 생략) 예상 코드 리뷰 시간🎯 2 (Simple) | ⏱️ ~12 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
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: 0
🧹 Nitpick comments (2)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateToggle.kt (2)
25-43: 접근성(Accessibility) 고려 필요
onCheckedChange = null로 설정하면 Switch의 내장 접근성 기능(TalkBack 상태 알림, 포커스 처리 등)이 비활성화됩니다.noRippleClickable은 시맨틱 정보를 제공하지 않아 스크린 리더 사용자가 토글 상태를 인식하기 어려울 수 있습니다.리플 효과 없이 접근성을 유지하려면
Modifier.toggleable과indication = null을 조합하거나,semanticsmodifier를 추가하는 것을 권장합니다.♻️ 접근성을 유지하는 대안
+import androidx.compose.foundation.selection.toggleable +import androidx.compose.foundation.interaction.MutableInteractionSource +import androidx.compose.ui.semantics.Role @Composable fun CollectionCreateToggle( checked: Boolean, onCheckedChange: (Boolean) -> Unit, modifier: Modifier = Modifier ) { Switch( checked = checked, onCheckedChange = null, - modifier = modifier.noRippleClickable { onCheckedChange(!checked) }, + modifier = modifier.toggleable( + value = checked, + onValueChange = onCheckedChange, + role = Role.Switch, + interactionSource = remember { MutableInteractionSource() }, + indication = null + ), thumbContent = { Box( modifier = Modifier.size(28.dp) ) },
20-42: Trailing comma 일관성이 PR의 다른 파일들은 trailing comma를 일관되게 추가했는데, 이 파일에서는 누락되었습니다. 스타일 일관성을 위해 추가를 권장합니다.
♻️ Trailing comma 추가
fun CollectionCreateToggle( checked: Boolean, onCheckedChange: (Boolean) -> Unit, - modifier: Modifier = Modifier + modifier: Modifier = Modifier, ) { Switch( checked = checked, onCheckedChange = null, modifier = modifier.noRippleClickable { onCheckedChange(!checked) }, thumbContent = { Box( - modifier = Modifier.size(28.dp) + modifier = Modifier.size(28.dp), ) }, colors = SwitchDefaults.colors( checkedThumbColor = FlintTheme.colors.white, checkedTrackColor = FlintTheme.colors.secondary400, uncheckedThumbColor = FlintTheme.colors.white, uncheckedTrackColor = FlintTheme.colors.gray200, checkedBorderColor = Color.Transparent, - uncheckedBorderColor = Color.Transparent - ) + uncheckedBorderColor = Color.Transparent, + ), ) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/com/flint/core/designsystem/component/topappbar/FlintBackTopAppbar.ktapp/src/main/java/com/flint/core/designsystem/component/topappbar/FlintBasicTopAppbar.ktapp/src/main/java/com/flint/core/designsystem/component/topappbar/FlintLogoTopAppbar.ktapp/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateToggle.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateToggle.kt (1)
app/src/main/java/com/flint/core/designsystem/theme/Theme.kt (1)
FlintTheme(8-16)
⏰ 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 (5)
app/src/main/java/com/flint/core/designsystem/component/topappbar/FlintLogoTopAppbar.kt (1)
16-36: LGTM!포맷팅 변경(trailing comma 추가)이 Kotlin 스타일 가이드와 일관성 있게 적용되었습니다.
app/src/main/java/com/flint/core/designsystem/component/topappbar/FlintBackTopAppbar.kt (1)
26-84: LGTM!포맷팅 변경(trailing comma 추가)이 일관성 있게 적용되었습니다. 기존 로직에 영향 없이 코드 스타일만 개선되었습니다.
app/src/main/java/com/flint/core/designsystem/component/topappbar/FlintBasicTopAppbar.kt (1)
29-84: LGTM!Modifier 체인 포맷팅과 trailing comma 추가가 코드 가독성을 향상시킵니다. 로직 변경 없이 스타일만 개선되었습니다.
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateToggle.kt (2)
29-33: Thumb 크기 확인 필요PR 설명에서 디자이너와 32.dp 높이로 합의했다고 언급되어 있는데, thumbContent의 Box 크기는 28.dp입니다. 의도된 설정인지 확인해 주세요. (예: 28.dp thumb + 패딩 = 32.dp 전체 높이)
46-57: LGTM!Preview 구현이 적절합니다.
FlintTheme으로 래핑하고remember와mutableStateOf를 사용하여 상태를 관리하는 패턴이 올바르게 적용되었습니다.
|
|
||
| @Composable | ||
| fun CollectionCreateToggle( | ||
| checked: Boolean, |
There was a problem hiding this comment.
p1
Boolean 타입은 앞에 is를 붙여서 isChecked로 해주시면 좋을 것 같아요!
| import com.flint.core.designsystem.theme.FlintTheme | ||
|
|
||
| @Composable | ||
| fun CollectionCreateToggle( |
There was a problem hiding this comment.
p1
토글 스위치는 피그마 디자인 시스템에 정의된 컴포넌트라, 위치를 core > designsystem > toggle로 옮겨주시고, 명칭도 FlintBasicToggle로 바꿔주시면 좋을 것 같습니다!
CollectionCreate 화면에서만 쓰일 거라는 보장이 없는 컴포넌트기도 하니까!
| Box( | ||
| modifier = Modifier.size(28.dp) | ||
| ) |
There was a problem hiding this comment.
p3
이거 Shape 지정 안해줘도 thumbContent에서 동그라미로 잘 들어가나 보네요
There was a problem hiding this comment.
네!! 기본 모양이 원이더라구용
| ) { | ||
| Switch( | ||
| checked = checked, | ||
| onCheckedChange = null, |
There was a problem hiding this comment.
p3
오 이거 리플 효과 제거하려고 자체 onCheckedChange를 null로 두신 건가영
There was a problem hiding this comment.
넵 ㅠㅠ Switch에는 리플 효과를 제거할 수 있는 방법이 없다고 해서 클릭을 자체적으로 만들어줬어요! 혹시 다른 방법이 있을까요?!
There was a problem hiding this comment.
CompositionLocal 이라는 친구 사용해서 리플 제거해볼 수도 있을 거예요!
https://developer.android.com/develop/ui/compose/compositionlocal?hl=ko
참고해 보시면 좋을 것 같습니다:)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateToggle.kt:
- Around line 25-28: The Switch currently sets onCheckedChange = null which
disables built-in accessibility semantics; instead preserve accessibility by
either keeping onCheckedChange non-null (forwarding to the passed
onCheckedChange while disabling ripple via a ripple-less indication) or replace
the custom modifier.noRippleClickable with Modifier.toggleable(checked =
isChecked, onValueChange = onCheckedChange, role = Role.Switch) so TalkBack and
keyboard navigation still get proper semantics; update the Switch instantiation
(and any related modifiers) to use one of these approaches referencing the
Switch, isChecked, onCheckedChange, modifier.noRippleClickable,
Modifier.toggleable and Role.Switch symbols.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateToggle.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/flint/presentation/collectioncreate/component/CollectionCreateToggle.kt (1)
app/src/main/java/com/flint/core/designsystem/theme/Theme.kt (1)
FlintTheme(8-16)
⏰ 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/flint/presentation/collectioncreate/component/CollectionCreateToggle.kt (2)
29-42: LGTM!FlintTheme 색상 활용과 커스텀 thumb 크기 설정이 적절합니다. 투명 테두리 처리도 깔끔합니다.
46-57: LGTM!FlintTheme으로 감싸고 remember 기반 상태 관리를 사용한 Preview 구현이 적절합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/flint/core/designsystem/component/toggle/FlintBasicToggle.kt:
- Around line 25-28: The Switch currently sets onCheckedChange = null and routes
taps via modifier.noRippleClickable, which makes accessibility services treat
the control as non-interactive; change Switch to provide a real onCheckedChange
handler (e.g., pass onCheckedChange) and remove noRippleClickable for core
activation, or keep the modifier for visuals but also supply the onCheckedChange
callback to Switch; additionally use an explicit InteractionSource and set
indication = null on the Switch (or onClickable) to remove ripple without
disabling accessibility, referencing the Switch composable, its onCheckedChange
parameter, modifier.noRippleClickable, interactionSource, and indication to
implement the fix.
🧹 Nitpick comments (1)
app/src/main/java/com/flint/core/designsystem/component/toggle/FlintBasicToggle.kt (1)
20-28: 파일명과 컴포넌트명 불일치파일명이
FlintBasicToggle.kt인데 컴포넌트명은CollectionCreateToggle입니다. 범용 토글로 사용할 계획이라면FlintBasicToggle로 이름을 변경하거나, 특정 용도라면 파일명을CollectionCreateToggle.kt로 변경하는 것을 권장합니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/flint/core/designsystem/component/toggle/FlintBasicToggle.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/flint/core/designsystem/component/toggle/FlintBasicToggle.kt (1)
app/src/main/java/com/flint/core/designsystem/theme/Theme.kt (1)
FlintTheme(8-16)
⏰ 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/flint/core/designsystem/component/toggle/FlintBasicToggle.kt (3)
1-17: LGTM!패키지 선언과 import 구문이 적절합니다.
29-33: thumb 크기와 디자인 스펙 확인 필요PR 설명에 디자이너와 32.dp 높이로 합의했다고 되어 있습니다. 현재 thumb 크기가 28.dp로 설정되어 있는데, 이것이 의도한 스펙과 일치하는지 확인해 주세요. Switch 전체 높이를 명시적으로 지정해야 할 수도 있습니다.
46-57: LGTM!Preview 구현이 적절합니다.
FlintTheme으로 감싸고mutableStateOf를 사용하여 인터랙티브한 미리보기가 가능합니다.
| Switch( | ||
| checked = isChecked, | ||
| onCheckedChange = null, | ||
| modifier = modifier.noRippleClickable { onCheckedChange(!isChecked) }, |
There was a problem hiding this comment.
접근성(Accessibility) 관련 검토 필요
onCheckedChange = null로 설정하면 TalkBack 등 접근성 서비스에서 Switch를 비활성화 상태로 인식할 수 있습니다. noRippleClickable로 클릭 이벤트를 처리하고 있지만, 스크린 리더 사용자가 토글을 조작하지 못할 수 있습니다.
접근성을 유지하면서 리플 효과를 제거하려면 onCheckedChange를 직접 처리하고, interactionSource와 indication = null을 활용하는 방식을 고려해 주세요.
♻️ 접근성을 유지하는 대안
+import androidx.compose.foundation.interaction.MutableInteractionSource
+
@Composable
fun CollectionCreateToggle(
isChecked: Boolean,
onCheckedChange: (Boolean) -> Unit,
modifier: Modifier = Modifier
) {
Switch(
checked = isChecked,
- onCheckedChange = null,
- modifier = modifier.noRippleClickable { onCheckedChange(!isChecked) },
+ onCheckedChange = onCheckedChange,
+ modifier = modifier,
+ interactionSource = remember { MutableInteractionSource() },
thumbContent = {
Box(
modifier = Modifier.size(28.dp)
)
},
colors = SwitchDefaults.colors(
// ... colors remain the same
)
)
}🤖 Prompt for AI Agents
In
@app/src/main/java/com/flint/core/designsystem/component/toggle/FlintBasicToggle.kt
around lines 25 - 28, The Switch currently sets onCheckedChange = null and
routes taps via modifier.noRippleClickable, which makes accessibility services
treat the control as non-interactive; change Switch to provide a real
onCheckedChange handler (e.g., pass onCheckedChange) and remove
noRippleClickable for core activation, or keep the modifier for visuals but also
supply the onCheckedChange callback to Switch; additionally use an explicit
InteractionSource and set indication = null on the Switch (or onClickable) to
remove ripple without disabling accessibility, referencing the Switch
composable, its onCheckedChange parameter, modifier.noRippleClickable,
interactionSource, and indication to implement the fix.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@app/src/main/java/com/flint/core/designsystem/component/toggle/FlintBasicToggle.kt:
- Around line 25-43: The current Switch sets onCheckedChange = null and uses
modifier.noRippleClickable which breaks accessibility; instead restore the
Switch's onCheckedChange to call the provided onCheckedChange and remove
noRippleClickable, or replace the clickable with a proper Modifier.toggleable
that supplies the same onCheckedChange but sets indication = null and provides a
custom MutableInteractionSource to suppress ripples while preserving semantics
and keyboard/ TalkBack behavior; update usages of Switch, onCheckedChange,
modifier.noRippleClickable, indication and interactionSource accordingly so the
Switch remains semantically interactive and accessible.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/com/flint/core/designsystem/component/toggle/FlintBasicToggle.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/flint/core/designsystem/component/toggle/FlintBasicToggle.kt (1)
app/src/main/java/com/flint/core/designsystem/theme/Theme.kt (1)
FlintTheme(8-16)
⏰ 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). (1)
- GitHub Check: PR Build Check
🔇 Additional comments (2)
app/src/main/java/com/flint/core/designsystem/component/toggle/FlintBasicToggle.kt (2)
1-17: LGTM!임포트가 올바르게 구성되어 있으며, 필요한 Compose 및 FlintTheme 관련 의존성이 적절히 포함되어 있습니다.
46-57: LGTM!Preview 함수가
FlintTheme으로 올바르게 래핑되어 있고,remember와mutableStateOf를 사용한 상태 관리가 적절합니다.
| Switch( | ||
| checked = isChecked, | ||
| onCheckedChange = null, | ||
| modifier = modifier.noRippleClickable { onCheckedChange(!isChecked) }, | ||
| thumbContent = { | ||
| Box( | ||
| modifier = Modifier.size(28.dp) | ||
| ) | ||
| }, | ||
| colors = SwitchDefaults.colors( | ||
| checkedThumbColor = FlintTheme.colors.white, | ||
| checkedTrackColor = FlintTheme.colors.secondary400, | ||
| uncheckedThumbColor = FlintTheme.colors.white, | ||
| uncheckedTrackColor = FlintTheme.colors.gray200, | ||
|
|
||
| checkedBorderColor = Color.Transparent, | ||
| uncheckedBorderColor = Color.Transparent | ||
| ) | ||
| ) |
There was a problem hiding this comment.
접근성(Accessibility) 문제: onCheckedChange = null 설정
onCheckedChange를 null로 설정하고 noRippleClickable로 클릭을 처리하면, TalkBack 등 스크린 리더에서 토글 가능한 요소로 인식되지 않아 접근성이 저하됩니다. Switch의 시맨틱 동작(semantic action)과 키보드 네비게이션도 정상 작동하지 않습니다.
리플 효과를 제거하면서 접근성을 유지하려면 interactionSource를 커스터마이징하는 방식을 권장합니다.
♻️ 접근성을 유지하면서 리플을 제거하는 방법
+import androidx.compose.foundation.interaction.MutableInteractionSource
+
@Composable
fun FlintBasicToggle(
isChecked: Boolean,
onCheckedChange: (Boolean) -> Unit,
modifier: Modifier = Modifier
) {
+ val interactionSource = remember { MutableInteractionSource() }
+
Switch(
checked = isChecked,
- onCheckedChange = null,
- modifier = modifier.noRippleClickable { onCheckedChange(!isChecked) },
+ onCheckedChange = onCheckedChange,
+ modifier = modifier,
+ interactionSource = interactionSource,
thumbContent = {
Box(
modifier = Modifier.size(28.dp)
)
},
colors = SwitchDefaults.colors(
checkedThumbColor = FlintTheme.colors.white,
checkedTrackColor = FlintTheme.colors.secondary400,
uncheckedThumbColor = FlintTheme.colors.white,
uncheckedTrackColor = FlintTheme.colors.gray200,
-
checkedBorderColor = Color.Transparent,
uncheckedBorderColor = Color.Transparent
)
)
}리플 효과 제거가 필수 요구사항이라면, indication = null을 사용하는 커스텀 Modifier.toggleable을 적용하되 시맨틱 정보는 유지하는 방식을 검토해 주세요.
🤖 Prompt for AI Agents
In
@app/src/main/java/com/flint/core/designsystem/component/toggle/FlintBasicToggle.kt
around lines 25 - 43, The current Switch sets onCheckedChange = null and uses
modifier.noRippleClickable which breaks accessibility; instead restore the
Switch's onCheckedChange to call the provided onCheckedChange and remove
noRippleClickable, or replace the clickable with a proper Modifier.toggleable
that supplies the same onCheckedChange but sets indication = null and provides a
custom MutableInteractionSource to suppress ripples while preserving semantics
and keyboard/ TalkBack behavior; update usages of Switch, onCheckedChange,
modifier.noRippleClickable, indication and interactionSource accordingly so the
Switch remains semantically interactive and accessible.
2 participants
📮 관련 이슈
📌 작업 내용
📸 스크린샷
😅 미구현
🫛 To. 리뷰어
Summary by CodeRabbit
새로운 기능
✏️ Tip: You can customize this high-level summary in your review settings.