Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
SDK Size Comparison 📏
|
WalkthroughThis pull request introduces a new RadioCheck UI component for poll interactions, expands the StreamColors theme with poll-progress and radio-check color properties, refactors PollMessageContent to use a new PollStyle theming system, replaces string resources with pluralization support for vote counts, and renames a poll constants value for clarity. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/MessageStyling.kt`:
- Around line 116-130: The incoming PollStyle branch in MessageStyling.kt
incorrectly uses colors.chatTextOutgoing for the textColor; update the else
branch where PollStyle is constructed to use colors.chatTextIncoming instead of
colors.chatTextOutgoing so incoming polls respect the incoming text color
customization (locate the PollStyle construction in the incoming/outgoing
conditional in MessageStyling.kt).
🧹 Nitpick comments (8)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/poll/PollMoreOptionsDialog.kt (1)
240-250: Misleading lambda parameter nameenabled.The
onCheckedChangecallback receives the new checked state (i.e.,!checked), not an enabled/disabled flag. Naming itenabledis confusing given thatRadioCheckalso has anenabledproperty with a different meaning. Consider renaming tonewCheckedorisCheckedfor clarity. Same naming issue also appears inPollMessageContent.ktline 413.✏️ Suggested rename
- RadioCheck( - modifier = Modifier.padding(end = 8.dp), - checked = checked, - onCheckedChange = { enabled -> - if (enabled && checkedCount < poll.maxVotesAllowed && !checked) { - onCastVote.invoke() - } else if (!enabled) { - onRemoveVote.invoke() - } - }, - ) + RadioCheck( + modifier = Modifier.padding(end = 8.dp), + checked = checked, + onCheckedChange = { newChecked -> + if (newChecked && checkedCount < poll.maxVotesAllowed && !checked) { + onCastVote.invoke() + } else if (!newChecked) { + onRemoveVote.invoke() + } + }, + )stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/common/RadioCheck.kt (2)
40-47: Consider adding KDoc for this internal component.While
RadioCheckisinternal, it's used by multiple poll-related composables across the module. A brief KDoc documenting the parameters (especially the nullableonCheckedChangesemantics for read-only mode) would help maintainability.
82-83: Preview should use@StreamPreviewhelpers.Per repository coding guidelines, Compose previews in stream-chat-android-compose should use
@StreamPreviewhelpers instead of the plain@Previewannotation. Based on learnings: "Compose previews should useStreamPreviewhelpers".stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/PollMessageContent.kt (4)
261-272: Consider avoiding passingMutableStatedirectly to composables.
PollButtonsreceivesshowDialog: MutableState<Boolean>andshowAddAnswerDialog: MutableState<Boolean>as parameters. The idiomatic Compose approach is to pass the value and a callback (state hoisting), e.g.,onSuggestOption: () -> UnitandonAddAnswer: () -> Unit, keeping the state management in the parent. This is a private composable so the blast radius is limited, but it couplesPollButtonsto the specific state mechanism.
410-421: Same misleadingenabledparameter name as inPollMoreOptionsDialog.The
onCheckedChangelambda parameterenabledon line 413 represents the new checked state, not whether the control is enabled. See the comment onPollMoreOptionsDialog.ktline 243 for the root cause — renaming tonewCheckedin both locations would improve clarity.
223-227: Bottom padding condition only applies when all options are visible.Line 226:
index == poll.options.size - 1evaluates against the total option count, not the number of displayed (taken) options. When options exceedMAX_NUMBER_OF_VISIBLE_OPTIONS, this condition is never true for any visible option, so the last visible item getsbottom = 0.dp. This appears intentional (the "See All" button follows immediately), but the asymmetry is subtle — a brief inline comment would clarify the intent.
453-458: Minor:animateFloatAsStatemissing a label.
animateFloatAsStateon line 453 could benefit from alabelparameter for animation inspection/debugging tooling. This is purely optional.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamColors.kt (1)
242-245: Missing@paramKDoc for the seven new public properties.The existing class-level KDoc documents every other property. The new
chatPollProgressFill*,chatPollProgressTrack*,controlRadioCheckBgSelected,controlRadioCheckBorder, andcontrolRadioCheckIconSelectedproperties should be documented similarly for consistency.Also applies to: 257-259
...t-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/MessageStyling.kt
Show resolved
Hide resolved
|


Goal
Update polls to the new design
Implementation
Nothing special here, just updating the poll to the new design. In the process, I also introduced the
RadioCheckcomponent according to Figma specs.🎨 UI Changes
Add relevant screenshots
Testing
You can test it in the sample
Summary by CodeRabbit
Release Notes
New Features
Improvements