-
Notifications
You must be signed in to change notification settings - Fork 1
[FEAT] 공통 컴포넌트 구현 #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a design system: multiple vector drawable assets, a centralized Compose theme (colors & typography), numerous reusable Compose UI components, a color utility, and build/config updates to include resources and Kamel image library. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @core/designSystem/src/commonMain/composeResources/drawable/ic_unsaved.xml:
- Around line 6-12: The Icon composable that currently uses tint =
Color.Unspecified for the ic_unsaved drawable should use a theme color instead
(e.g., KusTheme.colors.c_AAAAAA) so the vector's strokeColor isn't hard-coded;
update the Icon's tint argument to KusTheme.colors.c_AAAAAA (or another
appropriate KusTheme color as used by ic_location) to let the Theme control the
icon color.
In
@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusButton.kt:
- Around line 77-92: The passed-in modifier is incorrectly applied to the inner
Row instead of the Button causing caller layout modifiers (e.g.,
Modifier.fillMaxWidth()) to be ignored; update the Button composable call in
KusButton (apply the incoming modifier to the Button instead of the hardcoded
Modifier) and, to keep control over inner content, either create a new parameter
(e.g., contentModifier or innerModifier) or use Modifier for the Row (apply
Modifier or a default) so the Row uses an explicit content modifier while the
Button uses the public modifier parameter.
In
@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusCategoryImageButton.kt:
- Around line 109-111: The empty Column(Modifier.fillMaxSize()) in
KusCategoryImageButton.kt is leftover and should be removed or replaced by
wrapping the KusCategoryImageButton content with it; locate the Column in the
KusCategoryImageButton composable and either delete that empty Column or move
the existing button/content into that Column so it is not an empty container
(ensure you keep any necessary modifiers/state on the composable such as size,
padding or click handlers when moving).
In
@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.kt:
- Around line 62-77: The pointerInput is currently keyed to Unit so the gesture
detector can capture a stale onRatingChange; wrap the callback with
rememberUpdatedState (e.g., val currentOnRating =
rememberUpdatedState(onRatingChange)) and use that inside detectDragGestures,
and change pointerInput to be keyed on the remembered state (e.g.,
pointerInput(currentOnRating) { ... }) so the gesture handler will observe
updated onRatingChange references while keeping the gesture logic the same (use
currentOnRating.value when invoking the callback).
In
@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt:
- Line 48: The onClick parameter of KusRestThumbnail is declared but not used;
either remove it or wire it to the UI by adding a clickable modifier to the
outer Row (or the appropriate composable). Update the Row modifier chain in
KusRestThumbnail to include Modifier.clickable { onClick() } (or
Modifier.clickable(onClick = onClick)) combining it with existing modifiers, and
ensure the androidx.compose.foundation.clickable import is present;
alternatively drop the onClick parameter and any related references if click
behavior is not desired.
- Around line 82-89: The else branch in KusRestThumbnail (when restThumbnail is
not null/blank) is empty because the AsyncImage call is commented out, so no
image or fallback is rendered; restore or replace that code to render the
thumbnail (or a proper placeholder) by re-enabling the AsyncImage logic or
adding an Image/Box showing the loaded image using the restThumbnail value and
the same modifiers (e.g., contentScale = ContentScale.Crop, modifier =
Modifier.size(80.dp)), ensuring the branch that checks restThumbnail (not
null/blank) actually draws the image instead of doing nothing.
In
@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusTopBar.kt:
- Line 21: Update the KDoc for KusTopBar: the @param describing
onRightSecondClicked incorrectly says "우측 오른쪽 첫번째 아이콘" — change "첫번째" to "두번째"
so it accurately documents rightSecondIcon/onRightSecondClicked in the KusTopBar
component.
In @core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Theme.kt:
- Around line 47-48: The theme instances are recreated on every recomposition;
wrap creation of the color and typography objects in remember so they are cached
across recompositions — replace direct instantiation of KusLightColors() and
createKusTypography() (the vals named colors and typography in Theme.kt) with
remembered values using Compose's remember in the composable that provides the
theme, ensuring you call remember { KusLightColors() } and remember {
createKusTypography() } (or remember(themeInputs) when values depend on inputs)
so they aren't reconstructed on each recomposition.
🧹 Nitpick comments (10)
core/designSystem/src/commonMain/composeResources/drawable/ic_filled_star.xml (1)
6-10: Consider applying colors at runtime for better theming support.The hardcoded
fillColorandstrokeColorvalues limit theming flexibility in your design system. Since this is part of a comprehensive theming infrastructure (Typography, Color, Theme), consider applying colors dynamically in Compose usingIconwithtintparameter orImagewithcolorFilter.This approach allows:
- Runtime theme switching (light/dark mode)
- Consistent use of your centralized color palette
- Easier maintenance when design tokens change
💡 Example pattern for runtime color application
In your Compose code where you use this drawable:
Icon( painter = painterResource(Res.drawable.ic_filled_star), contentDescription = "Filled star", tint = MaterialTheme.colorScheme.primary, // or your custom theme color modifier = modifier )For the stroke color, you may need to use the path itself without a stroke and apply the border separately, or create a custom composable that layers the icon.
core/designSystem/src/commonMain/composeResources/drawable/ic_saved.xml (1)
1-13: Consider themeable colors for the saved icon.The icon uses hardcoded colors (fill: #FFB900, stroke: #000000) which may limit flexibility across different themes. Consider whether these colors should be parameterized or if this specific styling is intentionally fixed for the "saved" state branding.
core/designSystem/src/commonMain/composeResources/drawable/ic_like.xml (1)
1-16: LGTM! Well-structured like icon with clip-path.The icon is properly defined with a clip-path and dual-path design. The green color (#43AB38) is appropriate for a positive "like" action. Consider whether the color should be themeable for consistency with other UI components.
core/designSystem/src/commonMain/composeResources/drawable/ic_location.xml (1)
6-12: Consider making this icon tintable for dark theme support.The hardcoded black stroke (
#000000) will be invisible on dark backgrounds. Consider using@android:color/blackor a theme-aware color, or ensure the icon is tinted programmatically when used.♻️ Suggested change for theme compatibility
<path android:strokeWidth="1" android:pathData="M5,14.286C3.149,15.103 2,16.241 2,17.5C2,19.985 6.477,22 12,22C17.523,22 22,19.985 22,17.5C22,16.241 20.851,15.103 19,14.286M18,8C18,12.064 13.5,14 12,17C10.5,14 6,12.064 6,8C6,4.686 8.686,2 12,2C15.314,2 18,4.686 18,8ZM13,8C13,8.552 12.552,9 12,9C11.448,9 11,8.552 11,8C11,7.448 11.448,7 12,7C12.552,7 13,7.448 13,8Z" android:strokeLineJoin="round" android:fillColor="#00000000" - android:strokeColor="#000000" + android:strokeColor="#323232" android:strokeLineCap="round"/>Using
#323232(dark gray) aligns with other icons in this PR and provides better consistency, though tinting at runtime is the preferred approach for theme adaptability.core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Color.kt (1)
9-23: Consider semantic color names for better maintainability.Using hex values as color names (e.g.,
C_43AB38) makes the code less readable. Consider semantic names that convey intent, such asPrimary,Success,Error,TextPrimary,Background, etc. This would make the code more maintainable and easier to understand at a glance.For example:
val Primary = Color(0xFF43AB38) val PrimaryDark = Color(0xFF046B40) val Background = Color(0xFFFFFFFF) // etc.core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusReactionButton.kt (1)
31-34: Consider consistent enum naming:Dislikeinstead ofDisLike.The enum value
DisLikeuses a capital 'L' in the middle, which is non-standard for enum naming. Consider usingDislikefor consistency with Kotlin naming conventions.core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusTopBar.kt (1)
61-64: MissingcontentDescriptionfor accessibility.All
Iconcomponents havecontentDescription = null, which makes them invisible to screen readers. Consider adding meaningful content descriptions or exposing them as parameters for accessibility compliance.♻️ Suggested approach
Add optional content description parameters to the function signature:
fun KusTopBar( modifier: Modifier = Modifier, leftIcon: ImageVector? = null, rightFirstIcon: ImageVector? = null, rightSecondIcon: ImageVector? = null, + leftIconContentDescription: String? = null, + rightFirstIconContentDescription: String? = null, + rightSecondIconContentDescription: String? = null, onLeftClicked: () -> Unit = {},Then use these in the
Iconcomposables:Icon( imageVector = leftIcon, contentDescription = leftIconContentDescription )Also applies to: 78-81, 89-92
core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusCategoryImageButton.kt (1)
75-79: Consider addingcontentDescriptionfor accessibility.The
Imagecomponent hascontentDescription = null. For accessibility, consider passingcategoryNameor a dedicated description so screen readers can describe the category image.Image( imageVector = categoryImage, - contentDescription = null, + contentDescription = categoryName, modifier = Modifier.size(60.dp), )core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt (1)
44-44: PreferList<String>overArrayList<String>.Using
ArrayList<String>in the API exposes implementation details. PreferList<String>for a more idiomatic Kotlin interface.- categories: ArrayList<String>? = null, + categories: List<String>? = null,core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Type.kt (1)
19-37: Consider caching font families to avoid repeated instantiation.Each access to
PretendardBold,PretendardSemiBold, etc. creates a newFontFamilyinstance because they are computed properties. Consider usingrememberto cache them withincreateKusTypography.♻️ Alternative approach
Move font family creation inside
createKusTypographywith caching:@Composable fun createKusTypography(): KusTypography { val pretendardRegular = remember { FontFamily(Font(Res.font.pretendard_regular, FontWeight.Normal)) } val pretendardMedium = remember { FontFamily(Font(Res.font.pretendard_medium, FontWeight.Medium)) } val pretendardSemiBold = remember { FontFamily(Font(Res.font.pretendard_semibold, FontWeight.SemiBold)) } val pretendardBold = remember { FontFamily(Font(Res.font.pretendard_bold, FontWeight.Bold)) } return KusTypography( type20b = KusTextStyle(fontFamily = pretendardBold, fontSize = 20.sp), // ... ) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
core/designSystem/src/commonMain/composeResources/drawable/ic_kus_blank.pngis excluded by!**/*.pngcore/designSystem/src/commonMain/composeResources/drawable/ic_kus_logo.pngis excluded by!**/*.pngcore/designSystem/src/commonMain/composeResources/font/pretendard_black.ttfis excluded by!**/*.ttfcore/designSystem/src/commonMain/composeResources/font/pretendard_bold.ttfis excluded by!**/*.ttfcore/designSystem/src/commonMain/composeResources/font/pretendard_extrabold.ttfis excluded by!**/*.ttfcore/designSystem/src/commonMain/composeResources/font/pretendard_extralight.ttfis excluded by!**/*.ttfcore/designSystem/src/commonMain/composeResources/font/pretendard_light.ttfis excluded by!**/*.ttfcore/designSystem/src/commonMain/composeResources/font/pretendard_medium.ttfis excluded by!**/*.ttfcore/designSystem/src/commonMain/composeResources/font/pretendard_regular.ttfis excluded by!**/*.ttfcore/designSystem/src/commonMain/composeResources/font/pretendard_semibold.ttfis excluded by!**/*.ttf
📒 Files selected for processing (25)
core/designSystem/build.gradle.ktscore/designSystem/src/commonMain/composeResources/drawable/ic_check.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_comment.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_dislike.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_empty_star.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_filled_star.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_half_star.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_launcher_background.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_launcher_foreground.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_like.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_location.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_pencil.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_saved.xmlcore/designSystem/src/commonMain/composeResources/drawable/ic_unsaved.xmlcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusButton.ktcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusCategoryImageButton.ktcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusChip.ktcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.ktcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusReactionButton.ktcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.ktcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusTopBar.ktcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Color.ktcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Theme.ktcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Type.ktcore/designSystem/src/commonMain/kotlin/com/kus/designsystem/util/ColorUtil.kt
🧰 Additional context used
🧬 Code graph analysis (1)
core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Theme.kt (2)
core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Color.kt (2)
update(92-108)KusLightColors(111-143)core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Type.kt (1)
createKusTypography(76-146)
🔇 Additional comments (25)
core/designSystem/src/commonMain/composeResources/drawable/ic_empty_star.xml (1)
1-9: LGTM! Well-defined empty star icon.The vector drawable is properly structured with appropriate dimensions and styling for an empty star state. The light gray fill color (#E0E0E0) provides good visual contrast for unfilled star ratings.
core/designSystem/build.gradle.kts (1)
64-64: LGTM! Correct resource directory configuration.The resources.srcDirs configuration properly enables Gradle to discover the new composeResources directory containing the drawable assets added in this PR.
core/designSystem/src/commonMain/composeResources/drawable/ic_pencil.xml (1)
1-13: Verify the hardcoded white stroke color aligns with usage contexts.The icon uses a white stroke color (#FFFFFF), which will only be visible on dark backgrounds. Ensure this aligns with the intended usage contexts, or consider whether the icon should use a themeable color instead of hardcoded white.
core/designSystem/src/commonMain/composeResources/drawable/ic_check.xml (1)
1-15: LGTM!Clean vector drawable with appropriate dimensions for a check indicator icon. The hardcoded brand color is acceptable as this appears to be a design system asset with specific visual identity.
core/designSystem/src/commonMain/composeResources/drawable/ic_comment.xml (1)
1-27: LGTM!Well-structured comment icon with speech bubble and text line indicators. The neutral gray color is appropriate for a design system icon asset.
core/designSystem/src/commonMain/composeResources/drawable/ic_dislike.xml (1)
1-16: LGTM!Properly structured dislike icon using clip-path for clean masking. The implementation follows Android vector drawable best practices.
core/designSystem/src/commonMain/composeResources/drawable/ic_launcher_background.xml (1)
1-170: LGTM!Standard 108dp adaptive launcher icon background with Android's characteristic green color and grid pattern. Dimensions conform to adaptive icon requirements.
core/designSystem/src/commonMain/composeResources/drawable/ic_half_star.xml (1)
1-21: LGTM!Effective use of clip-path technique to render the half-star state. The layered approach with base stroke and clipped color fills produces a clean rating indicator.
core/designSystem/src/commonMain/composeResources/drawable/ic_launcher_foreground.xml (1)
1-30: LGTM!Standard 108dp adaptive launcher foreground with properly structured gradient shadow and Android robot glyph. The use of
aapt:attrfor inline gradient definition follows Android vector drawable best practices.core/designSystem/src/commonMain/kotlin/com/kus/designsystem/util/ColorUtil.kt (1)
10-13: LGTM!The implementation correctly clamps the percentage to [0, 100] and converts it to an alpha value in the range [0.0, 1.0]. The function is well-documented and handles edge cases appropriately.
core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Color.kt (2)
25-73: LGTM!The
@Stableannotation combined withmutableStateOfproperties is the correct pattern for theme colors in Compose. This enables runtime theme changes while maintaining Compose's recomposition optimizations. The private setters appropriately restrict mutation to theupdate()method.
74-143: LGTM!The
copy()andupdate()methods work together to support theme switching:copy()creates snapshots whileupdate()applies new values. TheKusLightColorsfactory function provides a convenient way to create color schemes with customizable defaults.core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Theme.kt (3)
10-16: LGTM!Using
staticCompositionLocalOffor theme values is appropriate since themes typically don't change frequently during composition. The error messages provide clear feedback when the theme is accessed outside a theme provider.
18-27: LGTM!The
KusThemeobject provides a clean API for accessing theme properties. The@ReadOnlyComposableannotation is correctly applied since these getters only read from composition locals without side effects.
29-41: LGTM!The pattern on line 35 (
remember { colors.copy() }.apply { update(colors) }) is a sophisticated approach for mutable theme state: it creates a stable remembered instance that updates reactively when the input colors change. This ensures proper Compose recomposition semantics while supporting dynamic theming.core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.kt (3)
49-53: Verify the state synchronization behavior matches requirements.The
LaunchedEffectonly syncscurrentRatingwith theratingprop whenisEnabledis false. This means:
- When enabled (interactive mode), external
ratingchanges are ignored- When disabled (read-only mode), it tracks the
ratingpropThis is likely intentional to prevent the component from fighting with user input during drag interactions. However, confirm this behavior aligns with your requirements—especially if the parent might need to programmatically update the rating while the component is enabled.
85-119: LGTM!The star rendering logic correctly calculates fill states for each star position. Using
Color.Unspecifiedas tint preserves the original drawable colors, and the content descriptions provide appropriate accessibility support.
122-142: LGTM!The preview functions effectively demonstrate both interactive and read-only modes, which is helpful for development and documentation.
core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusReactionButton.kt (3)
46-84: LGTM!The component cleanly handles like/dislike selection state and delegates rendering to the private
ReactionButtonItemcomposable. The nullableselectedTypepattern allows for no selection state.
86-134: LGTM!The item composable correctly handles selection state styling and conditional click behavior. Using theme colors ensures consistency across the design system.
136-181: LGTM!The preview demonstrates the component's interactive behavior with realistic state management. The toggle logic correctly handles switching between reactions and canceling selections.
core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt (1)
200-221: Tier badge may clip text due to size constraints.The tier badge uses a 20.dp container with 2.dp padding, leaving ~16.dp for content. However,
type16sb(16sp font) may not fit well, especially with line height. Consider using a smaller font or larger container.core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Type.kt (1)
55-74: LGTM!The
KusTypographyclass is well-structured with@Immutableannotation, clear naming convention encoding size and weight (e.g.,type16sb= 16sp SemiBold), and comprehensive coverage of typography needs.core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusChip.kt (1)
44-56: Modifier order causes touch target to be smaller than visual bounds.The
.clickable()modifier is applied after.padding(), which means the padded area is not clickable. Users tapping near the edges of the chip won't trigger the click.🔧 Suggested fix
Move
clickablebeforepadding:Box( modifier = modifier .background(backgroundColor, roundedCornerShape) .border(1.dp, mainColor, roundedCornerShape) - .padding(horizontal = 16.dp, vertical = 4.dp) - .clickable(onClick = onClick), + .clickable(onClick = onClick) + .padding(horizontal = 16.dp, vertical = 4.dp), ) {Likely an incorrect or invalid review comment.
core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusButton.kt (1)
69-74: No change needed — shadow will be visible.Setting
ambientColor = Color.Transparentonly removes the faint diffuse shadow component. ThespotColor(sharp directional shadow) still defaults to black and is not affected, so the shadow remains visible. The current implementation appears intentional and correct.Likely an incorrect or invalid review comment.
| <path | ||
| android:strokeWidth="1" | ||
| android:pathData="M5,7.8C5,6.12 5,5.28 5.327,4.638C5.615,4.074 6.074,3.615 6.638,3.327C7.28,3 8.12,3 9.8,3H14.2C15.88,3 16.72,3 17.362,3.327C17.927,3.615 18.385,4.074 18.673,4.638C19,5.28 19,6.12 19,7.8V21L12,17L5,21V7.8Z" | ||
| android:strokeLineJoin="round" | ||
| android:fillColor="#00000000" | ||
| android:strokeColor="#000000" | ||
| android:strokeLineCap="round"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find usages of ic_unsaved drawable in Compose code
echo "=== Searching for ic_unsaved usage in Kotlin/Compose ==="
rg -n --type kotlin 'ic_unsaved|Res\.drawable\.ic_unsaved' -B 2 -A 5
echo ""
echo "=== Searching for ic_unsaved in all source files ==="
rg -n 'ic_unsaved' -B 2 -A 5 --type kotlin --type java --type xmlRepository: kustaurant/Android-CMP
Length of output: 4370
🏁 Script executed:
cat -n core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.ktRepository: kustaurant/Android-CMP
Length of output: 10216
Apply theme color via tint parameter instead of Color.Unspecified.
The Icon composable at lines 91–98 uses tint = Color.Unspecified, which falls back to the hard-coded black color in the drawable XML. To integrate with the Theme system, use an appropriate theme color such as KusTheme.colors.c_AAAAAA (as applied to the ic_location Icon on line 134) or another suitable theme color for consistency.
🤖 Prompt for AI Agents
In @core/designSystem/src/commonMain/composeResources/drawable/ic_unsaved.xml
around lines 6 - 12, The Icon composable that currently uses tint =
Color.Unspecified for the ic_unsaved drawable should use a theme color instead
(e.g., KusTheme.colors.c_AAAAAA) so the vector's strokeColor isn't hard-coded;
update the Icon's tint argument to KusTheme.colors.c_AAAAAA (or another
appropriate KusTheme color as used by ic_location) to let the Theme control the
icon color.
| Button( | ||
| onClick = onClick, | ||
| enabled = enabled, | ||
| modifier = Modifier, | ||
| shape = roundedCornerShape, | ||
| colors = ButtonDefaults.buttonColors( | ||
| containerColor = containerColor, | ||
| contentColor = contentColor, | ||
| disabledContainerColor = KusTheme.colors.c_E0E0E0, | ||
| disabledContentColor = KusTheme.colors.c_AAAAAA, | ||
| ), | ||
| border = BorderStroke(1.dp, borderColor), | ||
| contentPadding = contentPadding, | ||
| ) { | ||
| Row( | ||
| modifier = modifier, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modifier applied to inner Row instead of Button may cause unexpected behavior.
The modifier parameter is documented as "내용에 적용되는 수정자" and applied to the inner Row (line 92), but the Button uses a hardcoded Modifier (line 80). This is unconventional and may confuse callers who expect modifier to affect the button's size/layout.
For example, in the preview at line 123, Modifier.fillMaxWidth() is passed but won't make the button fill the width since it's applied to the Row, not the Button.
🔧 Suggested fix
Consider applying the modifier to the Button and using a separate parameter for content modifier if needed:
@Composable
fun KusButton(
enabled: Boolean,
buttonName: String,
roundedCornerShape: RoundedCornerShape,
modifier: Modifier = Modifier,
+ contentModifier: Modifier = Modifier,
// ... other params
) {
// ...
Button(
onClick = onClick,
enabled = enabled,
- modifier = Modifier,
+ modifier = modifier,
shape = roundedCornerShape,
// ...
) {
Row(
- modifier = modifier,
+ modifier = contentModifier,
// ...
)
}
}🤖 Prompt for AI Agents
In
@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusButton.kt
around lines 77 - 92, The passed-in modifier is incorrectly applied to the inner
Row instead of the Button causing caller layout modifiers (e.g.,
Modifier.fillMaxWidth()) to be ignored; update the Button composable call in
KusButton (apply the incoming modifier to the Button instead of the hardcoded
Modifier) and, to keep control over inner content, either create a new parameter
(e.g., contentModifier or innerModifier) or use Modifier for the Row (apply
Modifier or a default) so the Row uses an explicit content modifier while the
Button uses the public modifier parameter.
.../designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusCategoryImageButton.kt
Outdated
Show resolved
Hide resolved
| Modifier.pointerInput(Unit) { | ||
| detectDragGestures( | ||
| onDrag = { change, _ -> | ||
| val x = change.position.x | ||
| if (containerWidth > 0) { | ||
| val newRating = (x / containerWidth * 5).coerceIn(0f, 5f) | ||
| // 0.5 단위 반올림 | ||
| val roundedRating = (round(newRating * 2) / 2).coerceIn(0f, 5f) | ||
| if (roundedRating != currentRating) { | ||
| currentRating = roundedRating | ||
| onRatingChange(roundedRating) | ||
| } | ||
| } | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update pointerInput key to capture callback changes.
Using pointerInput(Unit) means the gesture detector is created once and never recreated. If the onRatingChange callback reference changes, the gesture handler will continue using the stale callback, potentially causing bugs.
🔧 Proposed fix using rememberUpdatedState pattern
+ val currentOnRatingChange by rememberUpdatedState(onRatingChange)
+
Row(
modifier = modifier
.onSizeChanged { size ->
containerWidth = size.width
}
.then(
if (isEnabled) {
- Modifier.pointerInput(Unit) {
+ Modifier.pointerInput(isEnabled) {
detectDragGestures(
onDrag = { change, _ ->
val x = change.position.x
if (containerWidth > 0) {
val newRating = (x / containerWidth * 5).coerceIn(0f, 5f)
// 0.5 단위 반올림
val roundedRating = (round(newRating * 2) / 2).coerceIn(0f, 5f)
if (roundedRating != currentRating) {
currentRating = roundedRating
- onRatingChange(roundedRating)
+ currentOnRatingChange(roundedRating)
}
}
}
)
}Alternatively, you can add the full remember import:
import androidx.compose.runtime.rememberUpdatedState🤖 Prompt for AI Agents
In
@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.kt
around lines 62 - 77, The pointerInput is currently keyed to Unit so the gesture
detector can capture a stale onRatingChange; wrap the callback with
rememberUpdatedState (e.g., val currentOnRating =
rememberUpdatedState(onRatingChange)) and use that inside detectDragGestures,
and change pointerInput to be keyed on the remembered state (e.g.,
pointerInput(currentOnRating) { ... }) so the gesture handler will observe
updated onRatingChange references while keeping the gesture logic the same (use
currentOnRating.value when invoking the callback).
| location: String? = null, | ||
| isSaved: Boolean, | ||
| isEvaluated: Boolean, | ||
| onClick: () -> Unit = {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onClick parameter is declared but never used.
The onClick callback is accepted as a parameter but the component has no clickable modifier attached to the outer Row or any other element. Either wire it up or remove the parameter.
🔧 Suggested fix
Add clickable modifier to the Row:
Row(
modifier = modifier
+ .clickable(onClick = onClick)
.background(
color = KusTheme.colors.c_FFFFFF,
shape = RoundedCornerShape(16.dp)
)
.padding(8.dp)
)🤖 Prompt for AI Agents
In
@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt
at line 48, The onClick parameter of KusRestThumbnail is declared but not used;
either remove it or wire it to the UI by adding a clickable modifier to the
outer Row (or the appropriate composable). Update the Row modifier chain in
KusRestThumbnail to include Modifier.clickable { onClick() } (or
Modifier.clickable(onClick = onClick)) combining it with existing modifiers, and
ensure the androidx.compose.foundation.clickable import is present;
alternatively drop the onClick parameter and any related references if click
behavior is not desired.
...d/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt
Show resolved
Hide resolved
| * @param rightSecondIcon : 우측 오른쪽 두번째 아이콘 | ||
| * @param onLeftClicked : 좌측 아이콘 클릭 로직 | ||
| * @param onRightFirstClicked : 우측 왼쪽 첫번째 아이콘 클릭 로직 | ||
| * @param onRightSecondClicked : 우측 오른쪽 첫번째 아이콘 클릭 로직 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation typo.
Line 21 says "우측 오른쪽 첫번째 아이콘" but describes rightSecondIcon, so it should say "두번째".
- * @param onRightSecondClicked : 우측 오른쪽 첫번째 아이콘 클릭 로직
+ * @param onRightSecondClicked : 우측 오른쪽 두번째 아이콘 클릭 로직📝 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.
| * @param onRightSecondClicked : 우측 오른쪽 첫번째 아이콘 클릭 로직 | |
| * @param onRightSecondClicked : 우측 오른쪽 두번째 아이콘 클릭 로직 |
🤖 Prompt for AI Agents
In
@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusTopBar.kt
at line 21, Update the KDoc for KusTopBar: the @param describing
onRightSecondClicked incorrectly says "우측 오른쪽 첫번째 아이콘" — change "첫번째" to "두번째"
so it accurately documents rightSecondIcon/onRightSecondClicked in the KusTopBar
component.
| val colors = KusLightColors() | ||
| val typography = createKusTypography() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap theme instances in remember to prevent unnecessary recompositions.
The theme objects are recreated on every recomposition, which can cause performance issues and unnecessary downstream recompositions. Since these are stateful objects, they should be remembered.
⚡ Proposed fix to remember theme instances
@Composable
fun KusTheme(
content: @Composable () -> Unit
) {
- val colors = KusLightColors()
- val typography = createKusTypography()
+ val colors = remember { KusLightColors() }
+ val typography = remember { createKusTypography() }
ProvideKusColorsAndTypography(
colors = colors,
typography = typography
) {
MaterialTheme(
content = content
)
}
}📝 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.
| val colors = KusLightColors() | |
| val typography = createKusTypography() | |
| @Composable | |
| fun KusTheme( | |
| content: @Composable () -> Unit | |
| ) { | |
| val colors = remember { KusLightColors() } | |
| val typography = remember { createKusTypography() } | |
| ProvideKusColorsAndTypography( | |
| colors = colors, | |
| typography = typography | |
| ) { | |
| MaterialTheme( | |
| content = content | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In @core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Theme.kt
around lines 47 - 48, The theme instances are recreated on every recomposition;
wrap creation of the color and typography objects in remember so they are cached
across recompositions — replace direct instantiation of KusLightColors() and
createKusTypography() (the vals named colors and typography in Theme.kt) with
remembered values using Compose's remember in the composable that provides the
theme, ensuring you call remember { KusLightColors() } and remember {
createKusTypography() } (or remember(themeInputs) when values depend on inputs)
so they aren't reconstructed on each recomposition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt`:
- Around line 210-231: The tier badge is too tight (Box with size(20.dp) and
padding(2.dp)) while the Text uses KusTheme.typography.type16sb, which can cause
clipping; update the badge layout in KusRestThumbnail by increasing the Box size
(e.g., size(24.dp)), reducing the internal padding (e.g., padding(1.dp)), and/or
switching the Text style to a smaller typography like
KusTheme.typography.type14sb for tierText so the text fits reliably; adjust the
RoundedCornerShape calls as needed to keep the circular appearance.
In `@gradle/libs.versions.toml`:
- Around line 35-36: Update the Kamel version entry in the versions TOML by
changing the dependency value for the key `kamel = "0.9.4"` to the new stable
release `kamel = "1.0.9"` so build scripts pick up Kamel 1.0.9; locate the
`kamel` line in the versions file (the `kamel` version variable) and replace the
version string accordingly.
♻️ Duplicate comments (1)
core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt (1)
50-50:onClickparameter is still not wired to the component.The
onClickcallback is declared but never used. The outerRowhas noclickablemodifier attached. This was flagged in a previous review.🔧 Suggested fix
+import androidx.compose.foundation.clickable Row( modifier = modifier + .clickable(onClick = onClick) .background( color = KusTheme.colors.c_FFFFFF, shape = RoundedCornerShape(16.dp) ) .padding(8.dp) ) {Also applies to: 64-71
🧹 Nitpick comments (6)
core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusCategoryImageButton.kt (2)
9-9: Unused import:fillMaxSizeis no longer needed.After removing the empty
Column(per past review), this import is now unused and should be cleaned up.🧹 Suggested cleanup
-import androidx.compose.foundation.layout.fillMaxSize
90-117: Previews are functional and demonstrate both states correctly.The selectable preview properly demonstrates the toggle behavior. The non-selectable preview shows the default state.
Optional: The
modifier = Modifierargument (lines 97, 113) is redundant since it matches the default parameter value—can be omitted for brevity.gradle.properties (1)
7-7: Consider adding a TODO to re-enable configuration cache.Disabling configuration cache will impact incremental build performance. If this is due to plugin incompatibility (e.g., with KamelImage or other new dependencies), consider adding a comment explaining why and tracking when it can be re-enabled.
-org.gradle.configuration-cache=false +# TODO: Re-enable once [plugin/dependency] supports configuration cache +org.gradle.configuration-cache=falsecore/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt (2)
46-46: PreferList<String>overArrayList<String>for public API.Using
ArrayList<String>in the public API is not idiomatic Kotlin. PreferList<String>which is more flexible and follows Kotlin conventions.♻️ Suggested fix
- categories: ArrayList<String>? = null, + categories: List<String>? = null,
85-99: KamelImage implementation looks good; consider adding a loading state.The
KamelImageimplementation withonFailurefallback properly addresses the previously flagged empty else branch. However, consider adding anonLoadingparameter to display a placeholder or shimmer while the image loads, improving perceived performance.♻️ Optional: Add loading state
KamelImage( resource = asyncPainterResource(restThumbnail), contentDescription = null, contentScale = ContentScale.Crop, modifier = Modifier.size(80.dp), + onLoading = { + Image( + imageVector = vectorResource(Res.drawable.ic_kus_blank), + contentDescription = null, + contentScale = ContentScale.Crop, + modifier = Modifier.size(80.dp), + alpha = 0.5f + ) + }, onFailure = {core/designSystem/build.gradle.kts (1)
63-67: Theresources.srcDirsconfiguration may be redundant.The Compose Multiplatform plugin automatically treats
src/commonMain/composeResourcesas the conventional multiplatform resource root and auto-configures the resource directories accordingly. This explicit configuration is unnecessary and can be safely removed.♻️ Suggested simplification
implementation(libs.kamel.image) } - resources.srcDirs("src/commonMain/composeResources") }
| Box( | ||
| modifier = Modifier | ||
| .align(Alignment.Bottom) | ||
| .size(20.dp) | ||
| .border( | ||
| width = 1.dp, | ||
| color = tierColor, | ||
| shape = RoundedCornerShape(16.dp) | ||
| ) | ||
| .padding(2.dp) | ||
| .background(tierColor, RoundedCornerShape(100.dp)), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| Text( | ||
| text = tierText, | ||
| style = KusTheme.typography.type16sb.copy( | ||
| color = KusTheme.colors.c_FFFFFF, | ||
| lineHeight = androidx.compose.ui.unit.TextUnit.Unspecified | ||
| ), | ||
| textAlign = TextAlign.Center | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tier badge sizing may cause text clipping.
The badge container is 20.dp with 2.dp padding (leaving ~16dp content area), but uses type16sb typography (16sp font). Depending on the device density, the text may not fit properly, especially for double-digit tiers if supported in the future. Consider either increasing the badge size or using a smaller typography.
🤖 Prompt for AI Agents
In
`@core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt`
around lines 210 - 231, The tier badge is too tight (Box with size(20.dp) and
padding(2.dp)) while the Text uses KusTheme.typography.type16sb, which can cause
clipping; update the badge layout in KusRestThumbnail by increasing the Box size
(e.g., size(24.dp)), reducing the internal padding (e.g., padding(1.dp)), and/or
switching the Text style to a smaller typography like
KusTheme.typography.type14sb for tierText so the text fits reliably; adjust the
RoundedCornerShape calls as needed to keep the circular appearance.
| kamel = "0.9.4" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Kamel image library compose multiplatform latest version 2025 2026
💡 Result:
Latest stable Kamel (Compose Multiplatform image library) release is version 1.0.8 (published Sep 18, 2025). [1][2]
Sources:
- Kamel GitHub repository. [1]
- Maven Central / artifact pages for kamel-image 1.0.8 (published Sep 18, 2025). [2]
🌐 Web query:
Kamel 0.9.4 release notes changelog known issues
💡 Result:
Which "Kamel" do you mean? Options I found:
- Apache Camel Kamelets (release 0.9.4) — release page / notes. [1]
- Kamel (media.kamel Kotlin image / library) artifact version 0.9.4 on Maven Central / MVNRepository. [2]
Reply which one (or paste a link) and I’ll fetch the release notes, changelog and known issues.
🌐 Web query:
media.kamel:kamel-image Maven Central latest version
💡 Result:
The latest version on Maven Central is media.kamel:kamel-image:1.0.9. [1]
Maven dependency:
media.kamel
kamel-image
1.0.9
Sources: [1] Maven Central (media.kamel:kamel-image).
Update Kamel library to the latest version.
The Kamel image library is pinned to version 0.9.4, but the latest stable version is 1.0.9 (available on Maven Central). Update kamel = "0.9.4" to kamel = "1.0.9" to benefit from bug fixes, features, and improvements in the 1.0.x release line.
🤖 Prompt for AI Agents
In `@gradle/libs.versions.toml` around lines 35 - 36, Update the Kamel version
entry in the versions TOML by changing the dependency value for the key `kamel =
"0.9.4"` to the new stable release `kamel = "1.0.9"` so build scripts pick up
Kamel 1.0.9; locate the `kamel` line in the versions file (the `kamel` version
variable) and replace the version string accordingly.
…ponent # Conflicts: # gradle/libs.versions.toml # shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusButton.kt # shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusCategoryImageButton.kt # shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusChip.kt # shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.kt # shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusReactionButton.kt # shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt # shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusTopBar.kt # shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/util/ColorUtil.kt # shared/data/firstLaunch/build.gradle.kts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@gradle/libs.versions.toml`:
- Line 36: Update the kamel dependency entry in gradle/libs.versions.toml from
"0.9.4" to a 1.0+ release that targets Kotlin 2.2 (e.g., the latest 1.x Kamel
release), or if you must keep "0.9.4" add a clear comment in the file and
repository docs stating the intentional pin and include verification steps
(build/test matrix or a rebuilt artifact against Kotlin 2.2.20) to prove runtime
compatibility; locate the "kamel" key in libs.versions.toml to make this change
and ensure any build files or dependency BOM references are updated accordingly.
In
`@shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusChip.kt`:
- Around line 44-49: In KusChip's Box modifier chain, the .clickable(onClick =
onClick) is placed after .padding(), which makes the padding non-interactive;
move the .clickable(...) call so it appears before .padding(...) in the modifier
chain (i.e., modifier.background(...).border(...).clickable(onClick =
onClick).padding(...)) so the entire visible chip area becomes the tap target.
In
`@shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.kt`:
- Around line 46-53: The component currently only syncs currentRating from the
rating prop when isEnabled is false, so parent-driven updates while enabled are
ignored; update the LaunchedEffect (or an effect) in KusRatingBar to include
rating as a key and set currentRating = rating.coerceIn(0f, 5f) whenever rating
changes (optionally guard with if (currentRating != rating.coerceIn(...)) to
avoid needless updates), so parent updates are reflected even when isEnabled ==
true.
♻️ Duplicate comments (5)
shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.kt (1)
62-77: Stale callback issue inpointerInput(Unit).This concern was raised in a previous review. Using
Unitas the key means the gesture detector captures the initialonRatingChangereference. Please address the earlier suggestion to userememberUpdatedStateor key on a stable reference.shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusTopBar.kt (1)
21-21: Documentation typo still present.Line 21 describes
onRightSecondClickedbut says "첫번째 아이콘" (first icon) instead of "두번째 아이콘" (second icon).shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusButton.kt (1)
77-92:modifierparameter still applied to innerRowinstead ofButton.The issue flagged in the previous review remains: the
modifierparameter is applied to the innerRow(line 92) while theButtonuses a hardcodedModifier(line 80). This causesModifier.fillMaxWidth()passed in preview at line 123 to not work as expected.shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRestThumbnail.kt (2)
40-71: WireonClickor remove the parameter.
The callback is never invoked, so taps do nothing. Either add a clickable modifier or drop the parameter.🔧 Suggested fix
+import androidx.compose.foundation.clickable ... Row( modifier = modifier + .clickable(onClick = onClick) .background( color = KusTheme.colors.c_FFFFFF, shape = RoundedCornerShape(16.dp) ) .padding(8.dp) )
210-231: Tier badge size is tight fortype16sb.
20.dp with 2.dp padding leaves little room for 16sp text and can clip on some densities. Consider a slightly larger badge (or smaller text).🔧 Suggested fix
Box( modifier = Modifier .align(Alignment.Bottom) - .size(20.dp) + .size(24.dp) .border( width = 1.dp, color = tierColor, shape = RoundedCornerShape(16.dp) ) .padding(2.dp) .background(tierColor, RoundedCornerShape(100.dp)), contentAlignment = Alignment.Center ) {
🧹 Nitpick comments (6)
shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusReactionButton.kt (1)
107-115: Preferclickable(enabled = …)to keep disabled semantics.
This avoids dropping accessibility state when disabled and reduces branching.♻️ Proposed refactor
+import androidx.compose.ui.semantics.Role ... Row( modifier = Modifier - .then( - if (enabled) { - Modifier.clickable { onClick() } - } else { - Modifier - } - ), + .clickable( + enabled = enabled, + role = Role.Button, + onClick = onClick + ), verticalAlignment = Alignment.CenterVertically ) {shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.kt (1)
85-119: Consider adding semantic accessibility for the overall rating.Each star has
contentDescription = "별점 $starIndex"which describes its position, but screen reader users would benefit from knowing the current rating value for the entire component. Consider adding asemanticsmodifier to theRowwithcontentDescription = "별점 $currentRating점"or similar.♿ Example accessibility improvement
+import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semanticsRow( modifier = modifier + .semantics { contentDescription = "별점 ${currentRating}점" } .onSizeChanged { size ->shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusChip.kt (1)
40-42: Consider using a theme color instead of hardcodedColor.White.Using
Color.Whitedirectly may cause issues if dark mode is later supported. For theme consistency, consider defining a background color inKusTheme.colors(e.g.,c_FFFFFFor a semantic name likechipBackground).shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusCategoryImageButton.kt (1)
75-79: Consider adding accessibility support.The
contentDescriptionis set tonull, which means screen readers won't announce the category image. Consider usingcategoryNameas the content description to improve accessibility.♻️ Suggested improvement
Image( imageVector = categoryImage, - contentDescription = null, + contentDescription = categoryName, modifier = Modifier.size(60.dp), )shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusTopBar.kt (2)
61-64: Consider adding accessibility support for icons.All icons in the top bar have
contentDescription = null. For better accessibility, consider addingcontentDescriptionparameters to the composable signature so callers can provide meaningful descriptions for screen readers.♻️ Suggested approach
Add optional content description parameters to the function signature:
fun KusTopBar( // ... existing params leftIconContentDescription: String? = null, rightFirstIconContentDescription: String? = null, rightSecondIconContentDescription: String? = null, // ... )Then use them in the Icon composables:
Icon( imageVector = leftIcon, - contentDescription = null + contentDescription = leftIconContentDescription )
99-130: Consider enabling or removing commented preview code.The commented-out preview functions appear functional and would be useful for visualizing the component during development. Either uncomment them to aid development, or remove them if they're not needed.
| uiautomator = "2.3.0" | ||
| benchmarkMacroJunit4 = "1.4.1" | ||
| napier = "2.7.1" | ||
| kamel = "0.9.4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat gradle/libs.versions.toml | head -100Repository: kustaurant/Android-CMP
Length of output: 6003
🌐 Web query:
Kamel 0.9.4 Kotlin 2.2 Compose Multiplatform 1.9 compatibility
💡 Result:
Short answer: Kamel 0.9.4 (released Mar 8, 2024) was published against Kotlin 1.9-series artifacts and Compose MPP versions around the 1.9-era, so it is not guaranteed to be compatible with Kotlin 2.2 + Compose Multiplatform 1.9 without changes. You should upgrade Kamel to a 1.0+ release that targets newer Kotlin/Compose or test & rebuild against Kotlin 2.2. Sources: Maven listing for Kamel 0.9.4 (shows Kotlin 1.9.x deps), Kamel repo (current 1.x usage examples), and Kotlin/Compose compatibility guidance.
References:
[1] Kamel 0.9.4 (Maven/Central listings).
[2] Kamel GitHub README (current 1.x examples).
[3] Kotlin / Compose Multiplatform compatibility and versioning.
Upgrade Kamel to 1.0+ for Kotlin 2.2 compatibility.
Kamel 0.9.4 was built against Kotlin 1.9.x and is not guaranteed to be compatible with Kotlin 2.2.20. Upgrade to a 1.0+ release that targets Kotlin 2.2, or test and rebuild the current version against the project's Kotlin version. If staying on 0.9.4 is intentional, document the reason and verify runtime behavior.
🤖 Prompt for AI Agents
In `@gradle/libs.versions.toml` at line 36, Update the kamel dependency entry in
gradle/libs.versions.toml from "0.9.4" to a 1.0+ release that targets Kotlin 2.2
(e.g., the latest 1.x Kamel release), or if you must keep "0.9.4" add a clear
comment in the file and repository docs stating the intentional pin and include
verification steps (build/test matrix or a rebuilt artifact against Kotlin
2.2.20) to prove runtime compatibility; locate the "kamel" key in
libs.versions.toml to make this change and ensure any build files or dependency
BOM references are updated accordingly.
| Box( | ||
| modifier = modifier | ||
| .background(backgroundColor, roundedCornerShape) | ||
| .border(1.dp, mainColor, roundedCornerShape) | ||
| .padding(horizontal = 16.dp, vertical = 4.dp) | ||
| .clickable(onClick = onClick), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clickable modifier placed after padding reduces tap target size.
The .clickable() modifier is applied after .padding(), meaning the clickable area excludes the padding. Users must tap the text precisely rather than anywhere within the visible chip bounds, which degrades usability.
Move .clickable() before .padding() to make the entire chip area tappable.
🐛 Proposed fix
Box(
modifier = modifier
.background(backgroundColor, roundedCornerShape)
.border(1.dp, mainColor, roundedCornerShape)
- .padding(horizontal = 16.dp, vertical = 4.dp)
- .clickable(onClick = onClick),
+ .clickable(onClick = onClick)
+ .padding(horizontal = 16.dp, vertical = 4.dp),
) {📝 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.
| Box( | |
| modifier = modifier | |
| .background(backgroundColor, roundedCornerShape) | |
| .border(1.dp, mainColor, roundedCornerShape) | |
| .padding(horizontal = 16.dp, vertical = 4.dp) | |
| .clickable(onClick = onClick), | |
| Box( | |
| modifier = modifier | |
| .background(backgroundColor, roundedCornerShape) | |
| .border(1.dp, mainColor, roundedCornerShape) | |
| .clickable(onClick = onClick) | |
| .padding(horizontal = 16.dp, vertical = 4.dp), | |
| ) { |
🤖 Prompt for AI Agents
In
`@shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusChip.kt`
around lines 44 - 49, In KusChip's Box modifier chain, the .clickable(onClick =
onClick) is placed after .padding(), which makes the padding non-interactive;
move the .clickable(...) call so it appears before .padding(...) in the modifier
chain (i.e., modifier.background(...).border(...).clickable(onClick =
onClick).padding(...)) so the entire visible chip area becomes the tap target.
| var currentRating by remember { mutableFloatStateOf(rating.coerceIn(0f, 5f)) } | ||
| var containerWidth by remember { mutableIntStateOf(0) } | ||
|
|
||
| LaunchedEffect(rating, isEnabled) { | ||
| if (!isEnabled) { | ||
| currentRating = rating.coerceIn(0f, 5f) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parent-driven rating changes are ignored when isEnabled=true.
The currentRating state is initialized once via remember and only synced from the rating prop when isEnabled is false. If the parent updates the rating prop while the component is enabled (e.g., resetting to initial value, or receiving a server response), the component will not reflect that change.
Consider syncing currentRating whenever the rating prop changes, or document that the component is uncontrolled when enabled.
🔧 Proposed fix to sync rating when prop changes
var currentRating by remember { mutableFloatStateOf(rating.coerceIn(0f, 5f)) }
var containerWidth by remember { mutableIntStateOf(0) }
- LaunchedEffect(rating, isEnabled) {
- if (!isEnabled) {
- currentRating = rating.coerceIn(0f, 5f)
- }
+ LaunchedEffect(rating) {
+ currentRating = rating.coerceIn(0f, 5f)
}🤖 Prompt for AI Agents
In
`@shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.kt`
around lines 46 - 53, The component currently only syncs currentRating from the
rating prop when isEnabled is false, so parent-driven updates while enabled are
ignored; update the LaunchedEffect (or an effect) in KusRatingBar to include
rating as a key and set currentRating = rating.coerceIn(0f, 5f) whenever rating
changes (optionally guard with if (currentRating != rating.coerceIn(...)) to
avoid needless updates), so parent updates are reflected even when isEnabled ==
true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shared/core/designSystem/build.gradle.kts (1)
40-52: Moveresources.srcDirs(...)out ofdependenciesand fix the path.
resources.srcDirs(...)is not valid insidedependencies {}; it must be a sibling ofdependencieswithincommonMain. Additionally, the pathsrc/commonMain/designsystem/composeResourcesis incorrect—the directory doesn't exist. The actual composeResources directory is atsrc/commonMain/composeResources.✅ Proposed fix
commonMain { + resources.srcDirs("src/commonMain/composeResources") dependencies { implementation(compose.runtime) implementation(compose.foundation) implementation(compose.ui) implementation(compose.material3) implementation(compose.components.resources) implementation(compose.components.uiToolingPreview) implementation(libs.koin.core) implementation(libs.koin.compose) - - resources.srcDirs("src/commonMain/designsystem/composeResources") } }
🤖 Fix all issues with AI agents
In
`@shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Type.kt`:
- Around line 76-146: createKusTypography() is being recreated on every
recomposition; wrap the creation in remember so the KusTypography instance is
cached (e.g., in KusTheme where typography is currently created) instead of
calling createKusTypography() directly on each compose pass; update the
assignment to use remember when obtaining the typography (or move the remembered
creation into ProvideKusColorsAndTypography where colors are already remembered)
so recreate on recomposition is avoided.
🧹 Nitpick comments (2)
shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Color.kt (1)
92-108: Avoid unnecessary recompositions inupdate().Every assignment triggers state writes even when values are unchanged. Guarding updates reduces recompositions.
♻️ Suggested change
fun update(other: KusColors) { - c_43AB38 = other.c_43AB38 - c_046B40 = other.c_046B40 - c_FFFFFF = other.c_FFFFFF - c_F5F5F5 = other.c_F5F5F5 - c_EAEAEA = other.c_EAEAEA - c_E0E0E0 = other.c_E0E0E0 - c_AAAAAA = other.c_AAAAAA - c_666666 = other.c_666666 - c_323232 = other.c_323232 - c_353535 = other.c_353535 - c_000000 = other.c_000000 - c_0093FF = other.c_0093FF - c_01BAA6 = other.c_01BAA6 - c_FFB900 = other.c_FFB900 - c_9BA5B0 = other.c_9BA5B0 + if (c_43AB38 != other.c_43AB38) c_43AB38 = other.c_43AB38 + if (c_046B40 != other.c_046B40) c_046B40 = other.c_046B40 + if (c_FFFFFF != other.c_FFFFFF) c_FFFFFF = other.c_FFFFFF + if (c_F5F5F5 != other.c_F5F5F5) c_F5F5F5 = other.c_F5F5F5 + if (c_EAEAEA != other.c_EAEAEA) c_EAEAEA = other.c_EAEAEA + if (c_E0E0E0 != other.c_E0E0E0) c_E0E0E0 = other.c_E0E0E0 + if (c_AAAAAA != other.c_AAAAAA) c_AAAAAA = other.c_AAAAAA + if (c_666666 != other.c_666666) c_666666 = other.c_666666 + if (c_323232 != other.c_323232) c_323232 = other.c_323232 + if (c_353535 != other.c_353535) c_353535 = other.c_353535 + if (c_000000 != other.c_000000) c_000000 = other.c_000000 + if (c_0093FF != other.c_0093FF) c_0093FF = other.c_0093FF + if (c_01BAA6 != other.c_01BAA6) c_01BAA6 = other.c_01BAA6 + if (c_FFB900 != other.c_FFB900) c_FFB900 = other.c_FFB900 + if (c_9BA5B0 != other.c_9BA5B0) c_9BA5B0 = other.c_9BA5B0 }shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Type.kt (1)
19-37: Consider caching FontFamily instances withremember.The current implementation creates a new
FontFamilywrapper object each time these properties are accessed. While Compose Multiplatform best practices recommend usingrememberto cache composition-dependent resources likeFontFamily, ensure the result ofcreateKusTypography()is also memoized at the call site withrememberto avoid recreating the entire typography system on every recomposition.♻️ Suggested refactor using remembered font families
`@Composable` private fun rememberPretendardRegular(): FontFamily = remember { FontFamily(Font(Res.font.pretendard_regular, FontWeight.Normal)) } `@Composable` private fun rememberPretendardMedium(): FontFamily = remember { FontFamily(Font(Res.font.pretendard_medium, FontWeight.Medium)) } `@Composable` private fun rememberPretendardSemiBold(): FontFamily = remember { FontFamily(Font(Res.font.pretendard_semibold, FontWeight.SemiBold)) } `@Composable` private fun rememberPretendardBold(): FontFamily = remember { FontFamily(Font(Res.font.pretendard_bold, FontWeight.Bold)) }
| @Composable | ||
| fun createKusTypography() = KusTypography( | ||
| type20b = KusTextStyle( | ||
| fontFamily = PretendardBold, | ||
| fontSize = 20.sp, | ||
| ), | ||
| type16b = KusTextStyle( | ||
| fontFamily = PretendardBold, | ||
| fontSize = 16.sp, | ||
| ), | ||
| type13b = KusTextStyle( | ||
| fontFamily = PretendardBold, | ||
| fontSize = 13.sp, | ||
| ), | ||
| type18sb = KusTextStyle( | ||
| fontFamily = PretendardSemiBold, | ||
| fontSize = 18.sp, | ||
| ), | ||
| type17sb = KusTextStyle( | ||
| fontFamily = PretendardSemiBold, | ||
| fontSize = 17.sp, | ||
| ), | ||
| type16sb = KusTextStyle( | ||
| fontFamily = PretendardSemiBold, | ||
| fontSize = 16.sp, | ||
| ), | ||
| type15sb = KusTextStyle( | ||
| fontFamily = PretendardSemiBold, | ||
| fontSize = 15.sp, | ||
| ), | ||
| type18m = KusTextStyle( | ||
| fontFamily = PretendardMedium, | ||
| fontSize = 18.sp, | ||
| ), | ||
| type16m = KusTextStyle( | ||
| fontFamily = PretendardMedium, | ||
| fontSize = 16.sp, | ||
| ), | ||
| type15m = KusTextStyle( | ||
| fontFamily = PretendardMedium, | ||
| fontSize = 15.sp, | ||
| ), | ||
| type12m = KusTextStyle( | ||
| fontFamily = PretendardMedium, | ||
| fontSize = 12.sp, | ||
| ), | ||
| type11m = KusTextStyle( | ||
| fontFamily = PretendardMedium, | ||
| fontSize = 11.sp, | ||
| ), | ||
| type14r = KusTextStyle( | ||
| fontFamily = PretendardRegular, | ||
| fontSize = 14.sp, | ||
| ), | ||
| type13r = KusTextStyle( | ||
| fontFamily = PretendardRegular, | ||
| fontSize = 13.sp, | ||
| ), | ||
| type12r = KusTextStyle( | ||
| fontFamily = PretendardRegular, | ||
| fontSize = 12.sp, | ||
| ), | ||
| type11r = KusTextStyle( | ||
| fontFamily = PretendardRegular, | ||
| fontSize = 11.sp, | ||
| ), | ||
| type10r = KusTextStyle( | ||
| fontFamily = PretendardRegular, | ||
| fontSize = 10.sp, | ||
| ), | ||
| ) No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
find . -name "Type.kt" -o -name "Theme.kt" | grep -E "Theme|Type" | head -20Repository: kustaurant/Android-CMP
Length of output: 458
🏁 Script executed:
cat -n ./shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Theme.ktRepository: kustaurant/Android-CMP
Length of output: 1978
Wrap createKusTypography() in remember() to avoid recreation on recomposition.
The typography instance is created on every call to KusTheme() without caching, whereas colors are wrapped in remember (line 35 in ProvideKusColorsAndTypography). Add remember to line 48 in KusTheme:
val typography = remember { createKusTypography() }Alternatively, wrap it in ProvideKusColorsAndTypography where colors are already cached.
🤖 Prompt for AI Agents
In
`@shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Type.kt`
around lines 76 - 146, createKusTypography() is being recreated on every
recomposition; wrap the creation in remember so the KusTypography instance is
cached (e.g., in KusTheme where typography is currently created) instead of
calling createKusTypography() directly on each compose pass; update the
assignment to use remember when obtaining the typography (or move the remembered
creation into ProvideKusColorsAndTypography where colors are already remembered)
so recreate on recomposition is avoided.
- 이미지 파일 주석 처리(임시)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusCategoryImageButton.kt`:
- Line 6: The component KusCategoryImageButton currently uses Modifier.clickable
(imported in the file) and passes the isSelected parameter only for visual
styling, so assistive technologies won't get selection state; replace
Modifier.clickable with Modifier.selectable (import/select
androidx.compose.foundation.selection.selectable) and pass selected = isSelected
and onClick = onClick (and optionally role = Role.Button/Role.Tab as
appropriate) so the selection state is exposed to accessibility services; update
the import and ensure the selectable modifier is applied where
Modifier.clickable is used inside the KusCategoryImageButton composable.
In
`@shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusReactionButton.kt`:
- Around line 107-115: The Row modifier currently conditionally adds
Modifier.clickable only when enabled, which removes accessibility semantics when
disabled; instead keep the clickable modifier and pass the enabled flag so the
semantics node remains. Update the Row modifier in KusReactionButton (the block
that uses enabled and onClick) to always apply Modifier.clickable(enabled =
enabled) with the onClick callback, rather than toggling the entire modifier
based on enabled.
♻️ Duplicate comments (4)
shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.kt (2)
46-53: SynccurrentRatingwheneverratingchanges (even when enabled).Parent-driven rating updates are ignored while
isEnabled == true. Line 49 should react toratingchanges regardless of enabled state.🔧 Proposed fix
- LaunchedEffect(rating, isEnabled) { - if (!isEnabled) { - currentRating = rating.coerceIn(0f, 5f) - } - } + LaunchedEffect(rating) { + currentRating = rating.coerceIn(0f, 5f) + }
61-73: UpdatepointerInputkey and avoid staleonRatingChange.
pointerInput(Unit)captures a potentially stale callback; update the key and userememberUpdatedState.🔧 Proposed fix
+ val currentOnRatingChange by rememberUpdatedState(onRatingChange) + Row( modifier = modifier .onSizeChanged { size -> containerWidth = size.width } .then( if (isEnabled) { - Modifier.pointerInput(Unit) { + Modifier.pointerInput(currentOnRatingChange) { detectDragGestures( onDrag = { change, _ -> val x = change.position.x if (containerWidth > 0) { val newRating = (x / containerWidth * 5).coerceIn(0f, 5f) // 0.5 단위 반올림 val roundedRating = (round(newRating * 2) / 2).coerceIn(0f, 5f) if (roundedRating != currentRating) { currentRating = roundedRating - onRatingChange(roundedRating) + currentOnRatingChange(roundedRating) } } } ) }shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Type.kt (1)
76-146: Previous review addressed theremember()caching concern.The prior review correctly identified that
createKusTypography()should be wrapped inremember()at the call site (inKusTheme). No additional concerns with this segment.shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusButton.kt (1)
76-91: Previous review addressed themodifierplacement issue.The prior review correctly identified that applying
modifierto the innerRowinstead of theButtonis unconventional and may confuse callers. Please address the suggested fix from that review.
🧹 Nitpick comments (3)
shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusRatingBar.kt (1)
64-75: Consume drag changes to avoid parent scroll/gesture conflicts.Line 64 should typically consume the pointer change so nested scrolls don’t compete with rating drags.
🔧 Suggested tweak
- onDrag = { change, _ -> + onDrag = { change, _ -> + change.consume() val x = change.position.x if (containerWidth > 0) { val newRating = (x / containerWidth * 5).coerceIn(0f, 5f)shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/theme/Type.kt (1)
19-37: Font families are recreated on each composable getter access.Each
@Composable get()property creates a newFontFamilyinstance every time it's accessed. Since these are called multiple times withincreateKusTypography(), this compounds the allocation issue noted in the previous review about missingremember().Consider caching these as top-level vals or using
rememberat the call site. The fix for wrappingcreateKusTypography()inremember()(as noted in the prior review) will mitigate repeated allocations, but you could also make these lazily-initialized top-level constants for clarity.♻️ Alternative: Use remembered font families within createKusTypography
-private val PretendardRegular: FontFamily - `@Composable` - get() = - FontFamily(Font(Res.font.pretendard_regular, FontWeight.Normal)) - -private val PretendardMedium: FontFamily - `@Composable` - get() = - FontFamily(Font(Res.font.pretendard_medium, FontWeight.Medium)) - -private val PretendardSemiBold: FontFamily - `@Composable` - get() = - FontFamily(Font(Res.font.pretendard_semibold, FontWeight.SemiBold)) - -private val PretendardBold: FontFamily - `@Composable` - get() = - FontFamily(Font(Res.font.pretendard_bold, FontWeight.Bold)) +@Composable +private fun rememberPretendardRegular(): FontFamily = remember { + FontFamily(Font(Res.font.pretendard_regular, FontWeight.Normal)) +} + +@Composable +private fun rememberPretendardMedium(): FontFamily = remember { + FontFamily(Font(Res.font.pretendard_medium, FontWeight.Medium)) +} + +@Composable +private fun rememberPretendardSemiBold(): FontFamily = remember { + FontFamily(Font(Res.font.pretendard_semibold, FontWeight.SemiBold)) +} + +@Composable +private fun rememberPretendardBold(): FontFamily = remember { + FontFamily(Font(Res.font.pretendard_bold, FontWeight.Bold)) +}shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusButton.kt (1)
114-168: Commented preview code left for future use.Per the PR description, showcaseUI was intentionally excluded. Consider adding a TODO comment or tracking issue to enable these previews once the referenced resources (e.g.,
Res.drawable.ic_like) are available.
| import androidx.compose.foundation.Image | ||
| import androidx.compose.foundation.background | ||
| import androidx.compose.foundation.border | ||
| import androidx.compose.foundation.clickable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusCategoryImageButton.ktRepository: kustaurant/Android-CMP
Length of output: 4132
Expose selection state to accessibility services.
Modifier.clickable doesn't surface isSelected to assistive tech. The component has an isSelected parameter used only for visual styling, but screen readers won't announce the selection state. Use Modifier.selectable to properly expose it:
Suggested fix
-import androidx.compose.foundation.clickable
+import androidx.compose.foundation.selection.selectable
+import androidx.compose.ui.semantics.Role
...
- .clickable(onClick = onClick)
+ .selectable(
+ selected = isSelected,
+ onClick = onClick,
+ role = Role.Button,
+ )🤖 Prompt for AI Agents
In
`@shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusCategoryImageButton.kt`
at line 6, The component KusCategoryImageButton currently uses
Modifier.clickable (imported in the file) and passes the isSelected parameter
only for visual styling, so assistive technologies won't get selection state;
replace Modifier.clickable with Modifier.selectable (import/select
androidx.compose.foundation.selection.selectable) and pass selected = isSelected
and onClick = onClick (and optionally role = Role.Button/Role.Tab as
appropriate) so the selection state is exposed to accessibility services; update
the import and ensure the selectable modifier is applied where
Modifier.clickable is used inside the KusCategoryImageButton composable.
| Row( | ||
| modifier = Modifier | ||
| .then( | ||
| if (enabled) { | ||
| Modifier.clickable { onClick() } | ||
| } else { | ||
| Modifier | ||
| } | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
find . -name "KusReactionButton.kt" -type fRepository: kustaurant/Android-CMP
Length of output: 166
🏁 Script executed:
wc -l ./shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusReactionButton.ktRepository: kustaurant/Android-CMP
Length of output: 170
🏁 Script executed:
sed -n '100,120p' ./shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusReactionButton.ktRepository: kustaurant/Android-CMP
Length of output: 580
🌐 Web query:
Jetpack Compose clickable modifier enabled parameter semantics accessibility
💡 Result:
Short answer — how it affects semantics & accessibility:
-
Modifier.clickable(enabled = ...) sets both the runtime/gesture behavior and the semantics node’s enabled state. When enabled = false the onClick won’t be invoked and the element will be presented as disabled to accessibility services (TalkBack, Switch control, etc.). [1][2]
-
clickable merges descendants’ semantics by default (so the clickable becomes a single semantics node for screen readers). Use Modifier.semantics(...) if you need custom merging behavior. [2][3]
-
Use the onClickLabel parameter (or semantics { onClick(label = ...) }) to provide a custom spoken action (“Double tap to …”), and pass a Role when appropriate so assistive tech can describe the control type. [1][2]
References:
[1] clickable modifier API/docs (parameters: enabled, onClickLabel, role).
[2] Jetpack Compose accessibility / semantics guide (merging, click labels, TalkBack guidance).
[3] Compose component API guidelines (semantics merging guidance).
Use clickable(enabled = enabled) to preserve disabled semantics for accessibility.
The conditional modifier approach removes the clickable modifier entirely when disabled, which prevents accessibility services (TalkBack, etc.) from recognizing the control as a disabled button. Using clickable(enabled = enabled) maintains the semantics node and properly communicates the disabled state.
🔧 Suggested change
- Row(
- modifier = Modifier
- .then(
- if (enabled) {
- Modifier.clickable { onClick() }
- } else {
- Modifier
- }
- ),
+ Row(
+ modifier = Modifier.clickable(enabled = enabled) { onClick() },
verticalAlignment = Alignment.CenterVertically
) {📝 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.
| Row( | |
| modifier = Modifier | |
| .then( | |
| if (enabled) { | |
| Modifier.clickable { onClick() } | |
| } else { | |
| Modifier | |
| } | |
| ), | |
| Row( | |
| modifier = Modifier.clickable(enabled = enabled) { onClick() }, |
🤖 Prompt for AI Agents
In
`@shared/core/designSystem/src/commonMain/kotlin/com/kus/designsystem/component/KusReactionButton.kt`
around lines 107 - 115, The Row modifier currently conditionally adds
Modifier.clickable only when enabled, which removes accessibility semantics when
disabled; instead keep the clickable modifier and pass the enabled flag so the
semantics node remains. Update the Row modifier in KusReactionButton (the block
that uses enabled and onClick) to always apply Modifier.clickable(enabled =
enabled) with the onClick callback, rather than toggling the entire modifier
based on enabled.
개요
PR 유형
해당하는 항목에 체크해주세요.
To Reviewers
showcaseUI 는 빼고 일단 올렸습니다!
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.