fix(sdds-acore/uikit-compose): ListItem, Text and Icon#793
Merged
Conversation
…y. Icon and Text now respects color and brush specified by user.
📝 WalkthroughWalkthroughAdds content-start/end color properties and gravity alignment across cell/list styling, refactors Text API to use Color/Brush overloads, extends motion and theme-builder support, updates fixtures, and bumps multiple package patch versions. ChangesContent Display and Styling Enhancements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 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📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (2)
sdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/CellStyle.kt (1)
856-857: ⚡ Quick winDocument implicit fallback to title brush.
When
contentStartColorandcontentEndColorare not explicitly configured, they fall back todefaultTitleBrush. This coupling creates an implicit dependency where content colors track title color by default. While this may be intentional for visual consistency, the behavior is not documented in the interface or builder.Suggested documentation addition
Add KDoc to the interface properties:
/** - * Кисть контента в начале + * Кисть контента в начале. + * По умолчанию использует кисть тайтла, если не задана явно. */ val contentStartColor: StatefulValue<Brush> /** - * Кисть контента в конце + * Кисть контента в конце. + * По умолчанию использует кисть тайтла, если не задана явно. */ val contentEndColor: StatefulValue<Brush>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/CellStyle.kt` around lines 856 - 857, Document the implicit fallback where contentStartColor and contentEndColor default to defaultTitleBrush by adding KDoc on the CellStyle interface/builder properties (contentStartColor, contentEndColor) explaining that when these are null they will track the title color via defaultTitleBrush; mention the intent (visual consistency), the coupling/side-effect, and expected override behavior so callers know this implicit dependency (refer to the CellStyle builder/implementation that sets contentStartColor = contentStartColor ?: defaultTitleBrush and contentEndColor = contentEndColor ?: defaultTitleBrush).integration-core/uikit-compose-fixtures/src/main/kotlin/com/sdds/compose/uikit/fixtures/stories/list/ListStory.kt (1)
143-195: ⚡ Quick winPrefer explicit
Nonebranches instead ofelsein enumwhenblocks.Both
whenexpressions currently useelse -> null; this drops exhaustiveness checks for future enum additions. MapNoneexplicitly in each block.♻️ Suggested change
- else -> null + ListItemStartContent.None -> null @@ - else -> null + ListItemEndContent.None -> null🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration-core/uikit-compose-fixtures/src/main/kotlin/com/sdds/compose/uikit/fixtures/stories/list/ListStory.kt` around lines 143 - 195, The when branches in getStartContent (handling ListItemStartContent) and getEndContent (handling ListItemEndContent) use else -> null which disables exhaustiveness checking; replace the else branches with explicit ListItemStartContent.None -> null and ListItemEndContent.None -> null respectively so the when remains exhaustive and future enum additions will trigger compiler warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@sdds-core/plugin_theme_builder/src/main/kotlin/com/sdds/plugin/themebuilder/internal/components/list/item/compose/ListItemComposeVariationGenerator.kt`:
- Around line 70-74: The current when-expression in
ListItemComposeVariationGenerator that assigns enumValue silently coerces
unknown gravity tokens to "Top"; change it to treat unrecognized values as the
style default "Center" (or log/warn about invalid tokens) instead of defaulting
to "Top" so configuration mistakes aren't hidden—locate the when block that sets
enumValue and replace the else branch to return "Center" (and optionally emit a
warning including the original token) to preserve expected defaults.
- Around line 166-171: ListItemProperties.hasColors() currently returns true
only if backgroundColor, titleColor, disclosureIconColor, contentStartColor, or
contentEndColor are non-null, so subtitleColor and labelColor are omitted and
colorsCall() may be skipped; update hasColors() to also check subtitleColor and
labelColor so that when either is set the function returns true and the .colors
{ ... } block is emitted. Locate the hasColors() function and add null checks
for subtitleColor and labelColor alongside the existing checks to ensure color
configuration is preserved.
In `@sdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/CellStyle.kt`:
- Around line 580-588: Rename the two properties contentStartColor and
contentEndColor to contentStartBrush and contentEndBrush (type remains
StatefulValue<Brush>) so they follow the existing Brush suffix convention used
by titleBrush, labelBrush, subtitleBrush, disclosureTextBrush, and
disclosureIconBrush; keep any public builder/factory methods named
contentStartColor(...) and contentEndColor(...) if you want to preserve the
existing API surface for callers, but update all internal references and
implementations (DefaultCellColors, BaseCell usage, CellConfig, theme builder
generator, generated theme code, and any motion style properties) to use the new
property names contentStartBrush/contentEndBrush to ensure consistency across
the codebase.
In `@sdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/Icon.kt`:
- Around line 42-46: The current brushProducer logic wrongly gates using the
explicit brush on tint equality; update the Icon.kt logic so brushProducer uses
the provided brush whenever it is non-null and only falls back to BrushProducer
{ tint.asBrush() } when brush == null (do not check tint == LocalTint.current to
decide precedence). Concretely, change the construction of brushProducer
(symbol: brushProducer) to prefer the non-null parameter brush (wrap it in
BrushProducer if needed) and otherwise produce a brush from tint
(tint.asBrush()); then add a regression test verifying the path tint !=
LocalTint.current && brush != null returns/uses the provided brush.
---
Nitpick comments:
In
`@integration-core/uikit-compose-fixtures/src/main/kotlin/com/sdds/compose/uikit/fixtures/stories/list/ListStory.kt`:
- Around line 143-195: The when branches in getStartContent (handling
ListItemStartContent) and getEndContent (handling ListItemEndContent) use else
-> null which disables exhaustiveness checking; replace the else branches with
explicit ListItemStartContent.None -> null and ListItemEndContent.None -> null
respectively so the when remains exhaustive and future enum additions will
trigger compiler warnings.
In `@sdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/CellStyle.kt`:
- Around line 856-857: Document the implicit fallback where contentStartColor
and contentEndColor default to defaultTitleBrush by adding KDoc on the CellStyle
interface/builder properties (contentStartColor, contentEndColor) explaining
that when these are null they will track the title color via defaultTitleBrush;
mention the intent (visual consistency), the coupling/side-effect, and expected
override behavior so callers know this implicit dependency (refer to the
CellStyle builder/implementation that sets contentStartColor = contentStartColor
?: defaultTitleBrush and contentEndColor = contentEndColor ?:
defaultTitleBrush).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ecfc8d7c-216c-4e25-8393-140a837e715d
⛔ Files ignored due to path filters (2)
tokens/plasma.homeds.compose/screenshots-compose/testCellMAvatarIcon_dark.pngis excluded by!**/*.pngtokens/plasma.homeds.compose/screenshots-compose/testCellMAvatarIcon_light.pngis excluded by!**/*.png
📒 Files selected for processing (27)
gradle/libs.versions.tomlintegration-core/uikit-compose-fixtures/src/main/kotlin/com/sdds/compose/uikit/fixtures/stories/list/ListStory.ktplayground/sandbox-integration-test/src/main/kotlin/com/sdds/playground/integrationtest/components/scenario/ScenarioTabs.ktsdds-core/plugin_theme_builder/gradle.propertiessdds-core/plugin_theme_builder/src/main/kotlin/com/sdds/plugin/themebuilder/internal/components/cell/CellConfig.ktsdds-core/plugin_theme_builder/src/main/kotlin/com/sdds/plugin/themebuilder/internal/components/cell/compose/CellComposeVariationGenerator.ktsdds-core/plugin_theme_builder/src/main/kotlin/com/sdds/plugin/themebuilder/internal/components/list/item/ListItemConfig.ktsdds-core/plugin_theme_builder/src/main/kotlin/com/sdds/plugin/themebuilder/internal/components/list/item/compose/ListItemComposeVariationGenerator.ktsdds-core/uikit-compose/gradle.propertiessdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/CellStyle.ktsdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/Icon.ktsdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/ListItem.ktsdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/ListItemStyle.ktsdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/ListStyle.ktsdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/Text.ktsdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/internal/cell/BaseCell.ktsdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/motion/components/cell/CellMotionStyle.ktsdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/motion/components/list/ListItemMotionStyle.kttokens/plasma-stards-compose/gradle.propertiestokens/plasma.giga.compose/gradle.propertiestokens/plasma.homeds.compose/gradle.propertiestokens/plasma.homeds.compose/src/main/kotlin/com/sdds/plasma/homeds/styles/cell/CellStyles.kttokens/plasma.homeds.compose/src/main/kotlin/com/sdds/plasma/homeds/styles/listitem/ListItemStyles.kttokens/plasma.homeds.compose/src/main/kotlin/com/sdds/plasma/homeds/styles/listitem/ListNumberedItemStyles.kttokens/plasma.sd.service.compose/gradle.propertiestokens/sdds-sbcom-compose/gradle.propertiestokens/sdds.serv.compose/gradle.properties
Collaborator
📦 Собранные Release APK
📲 Как установить APK на Android-устройство:
💻 Установка через USB (ADB):
|
Collaborator
|
📘Артефакты документации опубликованы: |
raininforest
previously approved these changes
Jun 1, 2026
luizasok
approved these changes
Jun 1, 2026
There was a problem hiding this comment.
♻️ Duplicate comments (1)
sdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/Icon.kt (1)
43-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPast review issue remains unresolved: equality check is unreliable for parameter precedence.
Updating the KDoc to match the implementation does not fix the underlying design flaw flagged in the past review. The condition
tint == LocalTint.currentcannot reliably distinguish "user explicitly passed a tint value" from "tint defaulted to LocalTint.current":
- If a caller writes
Icon(painter, ..., tint = Color.Gray)andLocalTint.currenthappens to beColor.Gray, the condition evaluates totrueeven though tint was explicitly provided.- If
LocalTint.currentis customized, a caller omitting the tint parameter will gettint != LocalTint.currentand unintentionally bypass the brush.This creates unpredictable behavior when both
tintandbrushare provided.🔧 Recommended fix: use simple null-based precedence
- val brushProducer = if (tint == LocalTint.current) { - brush ?: BrushProducer { tint.asBrush() } - } else { - BrushProducer { tint.asBrush() } - } + val brushProducer = brush ?: BrushProducer { tint.asBrush() }Update the KDoc to reflect simple precedence: "brush takes priority when non-null; otherwise tint is used."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/Icon.kt` around lines 43 - 47, The equality check tint == LocalTint.current in the brushProducer selection is unreliable for determining whether callers passed an explicit tint; change the logic to a null-based precedence: if brush != null use brush, otherwise fall back to tint.asBrush(); update the brushProducer assignment (and any related code using BrushProducer and the tint parameter) to implement this null-first precedence and adjust the KDoc to state "brush takes priority when non-null; otherwise tint is used."
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@sdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/Icon.kt`:
- Around line 43-47: The equality check tint == LocalTint.current in the
brushProducer selection is unreliable for determining whether callers passed an
explicit tint; change the logic to a null-based precedence: if brush != null use
brush, otherwise fall back to tint.asBrush(); update the brushProducer
assignment (and any related code using BrushProducer and the tint parameter) to
implement this null-first precedence and adjust the KDoc to state "brush takes
priority when non-null; otherwise tint is used."
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 29ef2407-5775-4754-9d33-7fd1d2cc46cb
📒 Files selected for processing (3)
sdds-core/plugin_theme_builder/src/main/kotlin/com/sdds/plugin/themebuilder/internal/components/list/item/compose/ListItemComposeVariationGenerator.ktsdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/Icon.ktsdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/ListItemStyle.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- sdds-core/uikit-compose/src/main/kotlin/com/sdds/compose/uikit/ListItemStyle.kt
- sdds-core/plugin_theme_builder/src/main/kotlin/com/sdds/plugin/themebuilder/internal/components/list/item/compose/ListItemComposeVariationGenerator.kt
angirb
approved these changes
Jun 1, 2026
raininforest
approved these changes
Jun 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sdds-uikit-compose
List
ListItem
Text
Icon
plasma-homeds-compose
ListNumbered
What/why changed
Это обязательный блок и заголовок!
Более подробное описание решаемой проблемы.
Summary by CodeRabbit
New Features
Chores