Conversation
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
SDK Size Comparison 📏
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
gpunto
force-pushed
the
redesign/avatar-group
branch
from
2a03e87 to
7ced2e1
Compare
WalkthroughThis PR introduces stacked avatar rendering with overflow badges for multi-member channels, adds new color and typography theme properties, and updates UI components to use these new design tokens. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stream-chat-android-compose/api/stream-chat-android-compose.api (1)
4078-4124:⚠️ Potential issue | 🟡 MinorDocument the StreamTypography expansion in the changelog.
The
numericLargeproperty and updated constructor signature expand the public API. While the factory methoddefaultTypography()ensures backward compatibility for typical usage, this change should be documented inCHANGELOG.mdto inform downstream consumers. Consider adding an entry under the "Added" section for stream-chat-android-compose to explain the new text style property.
🤖 Fix all issues with AI agents
In `@stream-chat-android-compose/api/stream-chat-android-compose.api`:
- Around line 1628-1633: The API dump exposes ComposableSingletons$CountBadgeKt
(with getLambda-1$stream_chat_android_compose_release and lambda-1) as public
while CountBadge and CountBadgeSize are intended internal; either make the
CountBadge/CountBadgeSize public if that was intended or revert their visibility
to internal and regenerate the API dump so ComposableSingletons$CountBadgeKt is
not public. Locate the symbols ComposableSingletons$CountBadgeKt, CountBadge,
CountBadgeSize and the function getLambda-1$stream_chat_android_compose_release
in the codebase, adjust the Kotlin visibility modifiers to be consistent
(internal for implementation-only types or public for intended API), then re-run
the API dump generation to update the .api file so it matches the corrected
visibilities.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/avatar/ChannelAvatar.kt`:
- Around line 159-214: StackedGroupAvatar declares a showBorder parameter but
never uses or forwards it, misleading callers; either remove the parameter from
StackedGroupAvatar (and its callers such as ChannelAvatar) if stacked avatars
should not support borders, or thread showBorder into the stacked children by
adjusting how the base modifier is created—e.g., update StackedGroupAvatar to
pass showBorder into StackedGroupAvatarSpecs.baseModifier (or into UserAvatar
calls) so borders are applied consistently (refer to StackedGroupAvatar,
StackedGroupAvatarSpecs.baseModifier, and UserAvatar for where to propagate or
remove the flag).
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/common/CountBadge.kt`:
- Around line 49-67: The Text composable is incorrectly hardcoding style =
ChatTheme.typography.numericLarge instead of using the computed style from
size.textStyle(typography), causing wrong lineHeight/other text metrics for
Small badges; update the Text call to use the local style variable (style) and
keep the separately computed fontSize override (fontSize = fontSize) so metrics
come from size.textStyle(typography) while font size respects fixedFontSize
logic; refer to the computed variables style, fontSize,
size.textStyle(typography), and the Text composable to locate the change.
🧹 Nitpick comments (10)
stream-chat-android-previewdata/src/main/kotlin/io/getstream/chat/android/previewdata/PreviewChannelData.kt (1)
96-103: Add KDoc to this public function.The existing coding guidelines require public APIs to be documented with KDoc. A brief description of the function's purpose and its parameter would suffice.
Also, consider guarding against non-positive values of
howMany, or at minimum documenting the expected range, sinceList(-1) { ... }would throw anIllegalArgumentExceptionat runtime.📝 Suggested KDoc and guard
+ /** + * Creates a [Channel] populated with [howMany] generated [Member] entries. + * + * `@param` howMany The number of members to include. Must be positive. + */ public fun makeChannelWithMembers(howMany: Int): Channel = Channel( + require(howMany > 0) { "howMany must be positive" }Or, if you prefer keeping it simple for preview-only code, just the KDoc is fine.
As per coding guidelines:
**/*.kt: "Document public APIs with KDoc, including thread expectations and state notes."stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamTokens.kt (1)
59-59: Naming inconsistency with otherlineHeighttokens.The other line-height tokens use semantic names (
lineHeightTightest,lineHeightTighter, etc.), while this one uses a raw numeric suffix. Since 12.sp sits betweenlineHeightTightest(10.sp) andlineHeightTighter(14.sp), consider a semantic name (e.g.,lineHeightTighterSmallor similar) to stay consistent.This is internal so the impact is limited — just a nit for consistency.
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamTypography.kt (1)
50-52: Missing@paramKDoc fornumericLarge.The class-level KDoc documents
numericMediumandnumericExtraLargebut omitsnumericLarge. Add it for consistency. As per coding guidelines, public APIs should be documented with KDoc.📝 Suggested addition after line 51
* `@param` numericMedium Style for medium-sized numeric indicators, like the unread count. + * `@param` numericLarge Style for large-sized numeric indicators, like the avatar overflow count. * `@param` numericExtraLarge Style for extra-large numeric indicators.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/avatar/UserAvatar.kt (2)
131-138: Redundant.background(background)onIcon.The parent
BoxWithConstraintsat line 128 already applies.background(background). Applying it again on theIconat line 136 is unnecessary since the Icon is centered within the parent and the parent fills its space with that background color.♻️ Suggested fix
Icon( painter = painterResource(R.drawable.stream_compose_ic_user), contentDescription = null, tint = foreground, modifier = Modifier - .background(background) .size(this.maxWidth.toPlaceholderIconSize()), )
152-153: Consider using@StreamPreviewinstead of@Preview.The coding guidelines specify that Compose previews should use
@StreamPreviewhelpers. This preview uses the standard@Previewannotation instead.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/common/CountBadge.kt (1)
76-86: Consider using@StreamPreviewinstead of@Preview.As per coding guidelines, Compose previews should use
@StreamPreviewhelpers.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamColors.kt (2)
176-176: Missing KDoc@paramentries for several new public properties.The following new public properties are not documented in the class-level KDoc:
avatarBgPlaceholderavatarTextPlaceholdertextOnDarkbadgeBgDefaultbadgeTextInversecontrolRadioCheckBgSelectedcontrolRadioCheckIconSelectedThe existing KDoc documents most other properties; these should be added for consistency. As per coding guidelines, public APIs should be documented with KDoc.
Also applies to: 187-187, 213-213, 223-228, 258-259
494-495:neutral900is declared beforeneutral800, breaking the numerical ordering.All other primitive colors follow ascending numerical order. Swap these two lines for consistency.
♻️ Suggested fix
- val neutral900 = Color(0xFF1C1C1C) val neutral800 = Color(0xFF323232) + val neutral900 = Color(0xFF1C1C1C)stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/avatar/ChannelAvatar.kt (2)
137-157:alignmentsByIndexlist-index mapping is subtle — consider a clarifying comment.The list uses positional indexing to map
membersCount→ alignments, with index 0 holding anemptyList()and indices 1–2 both mapping toalignments2. This works, but the implicit coupling between the list index and thewhenbranches inStackedGroupAvatar(e.g.,membersCount == 1usesalignments[0]andalignments[1]fromalignments2) is easy to misread. A brief inline comment would help future readers.
194-211:showIndicatoron every stacked avatar could be visually crowded at small sizes.Each
UserAvatarin the loop receivesshowIndicator, which means up to 4 small avatars could each render their own online indicator badge simultaneously. AtAvatarSize.ExtraSmall(the Medium dimension tier), the indicators might overlap or be hard to see. If this is intentional per design, no action needed — just flagging in case it wasn't considered.
...ompose/src/main/java/io/getstream/chat/android/compose/ui/components/avatar/ChannelAvatar.kt
Show resolved
Hide resolved
...d-compose/src/main/java/io/getstream/chat/android/compose/ui/components/common/CountBadge.kt
Show resolved
Hide resolved
gpunto
force-pushed
the
redesign/avatar-group
branch
2 times, most recently
from
9ba648d to
bef5003
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
...main/java/io/getstream/chat/android/compose/ui/components/attachments/images/ImagesPicker.kt
Show resolved
Hide resolved
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/StreamTypography.kt`:
- Around line 216-220: The numericLarge TextStyle is missing an explicit
lineHeight and should match its siblings; update the numericLarge TextStyle (in
StreamTypography) to include lineHeight = StreamTokens.lineHeightTighter so it
uses the same tighter line spacing as numericMedium and numericExtraLarge
instead of inheriting the platform default.
🧹 Nitpick comments (7)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamTypography.kt (1)
78-78: Missing@param numericLargein the class KDoc.The KDoc block (lines 50–52) documents
numericMediumandnumericExtraLargebut skips the newnumericLargeproperty. As per coding guidelines, public APIs should be documented with KDoc.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/common/CountBadge.kt (1)
76-86: Consider using@StreamPreviewhelper instead of raw@Preview.As per coding guidelines, Compose previews in this module should use
@StreamPreviewhelpers.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamColors.kt (2)
176-228: Several new public properties lack@paramKDoc entries.
avatarBgPlaceholder,avatarTextPlaceholder,textOnDark,badgeBgDefault,badgeTextInverse,controlRadioCheckBgSelected, andcontrolRadioCheckIconSelectedare all public properties without corresponding@paramentries in the class-level KDoc (lines 26–141). As per coding guidelines, public APIs should be documented with KDoc.
494-495: Nit:neutral900is declared beforeneutral800, breaking numeric order.The other neutral primitives follow ascending numeric order. Consider reordering for consistency.
Suggested reorder
val neutral700 = Color(0xFF4A4A4A) - val neutral900 = Color(0xFF1C1C1C) val neutral800 = Color(0xFF323232) + val neutral900 = Color(0xFF1C1C1C)stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/avatar/UserAvatar.kt (2)
121-140: Redundant.background(background)on theIconmodifier.The
BoxWithConstraintson line 128 already sets.background(background). The duplicate on theIcon(line 136) is harmless but unnecessary.Suggested cleanup
Icon( painter = painterResource(R.drawable.stream_compose_ic_user), contentDescription = null, tint = foreground, modifier = Modifier - .background(background) .size(this.maxWidth.toPlaceholderIconSize()), )
152-186: Consider using@StreamPreviewhelper instead of raw@Preview.As per coding guidelines, Compose previews in this module should use
@StreamPreviewhelpers.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/avatar/ChannelAvatar.kt (1)
297-326: Consider using@StreamPreviewhelper instead of raw@Preview.As per coding guidelines, Compose previews in this module should use
@StreamPreviewhelpers. This applies to the other new previews in this PR as well (AvatarPreviewinUserAvatar.kt,CountBadgePreviewinCountBadge.kt).
...android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/StreamTypography.kt
Show resolved
Hide resolved
gpunto
force-pushed
the
redesign/avatar-group
branch
from
bef5003 to
c5dc6c4
Compare
andremion
left a comment
andremion
left a comment
There was a problem hiding this comment.
Good job!
Can you also check why the avatar and the channel text look inconsistent?
e.g. the avatar shows 2 members + 11, whereas the channel texts shows 2 members + 10
...ompose/src/main/java/io/getstream/chat/android/compose/ui/components/avatar/ChannelAvatar.kt
Show resolved
Hide resolved
gpunto
force-pushed
the
redesign/avatar-group
branch
from
dd1b897 to
a492732
Compare
Good point, it's because we're excluding the current user in the channel title count. I've just asked in Figma because, as far as I understand, we're not excluding it for the avatar and we don't have examples for this case of the channel title yet. |
gpunto
force-pushed
the
redesign/avatar-group
branch
from
a492732 to
0fbeb6c
Compare
gpunto
force-pushed
the
redesign/avatar-group
branch
from
0fbeb6c to
5ea3eea
Compare
|
So, Jurgen update channel list design examples so that the title fallback logic matches the avatar logic, so we're good to go here! We'll address the inconsistency when updating the channel list design. |
2 participants
Goal
The group avatars have a new design, where we stack user avatars depending on how many there are.
Implementation
The implementation uses the channel image if there's one, otherwise it checks the number of users in the channel and switches to the appropriate layout where we display 2, 3, 4 avatars or 2 avatars + a count badge.
🎨 UI Changes
Preview
Testing
Can be checked in the sample
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor