-
Notifications
You must be signed in to change notification settings - Fork 374
refactor(ui)!: Redesign Avatar components #2502
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: v10.0.0
Are you sure you want to change the base?
Conversation
This commit introduces a complete redesign of the Avatar components to simplify the API, improve consistency, and align with the new design system. The following components have been removed and replaced: - `StreamUserAvatar` - `StreamGroupAvatar` - `StreamChannelAvatar` They are replaced by a new suite of components: - `StreamUserAvatar`: Displays a single user's avatar. - `StreamUserAvatarGroup`: Displays a grid of user avatars for group channels. - `StreamUserAvatarStack`: A new component for displaying a stack of overlapping user avatars. - `StreamChannelAvatar`: Rebuilt to internally use the new avatar components. Key changes: - **Simplified Sizing**: The `constraints` parameter has been replaced with a `size` enum (e.g., `StreamAvatarSize.lg`) for standardized sizing. - **Tap Handling**: The `onTap` callback has been removed. Avatars should now be wrapped with a `GestureDetector` or `InkWell` for handling interactions. - **Parameter Cleanup**: Parameters like `borderRadius`, `selected`, `selectionColor`, and `selectionThickness` have been removed to simplify the API. Customization is now handled through theming. - **Online Indicator**: The `showOnlineStatus` parameter has been renamed to `showOnlineIndicator`. Its default is now `true` for `StreamUserAvatar` and `false` for avatars inside groups or stacks. - **`StreamGroupAvatar` is now `StreamUserAvatarGroup`**: The component has been renamed and now accepts a list of `User` objects instead of `Member` objects. - **Migration Guide**: A detailed migration guide for the new avatar components has been added under `migrations/redesign/stream_avatar.md`. - **Theming**: Integrated the new `StreamTheme` extension from `stream_core_flutter` for better theme management.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughThis PR replaces legacy avatar widgets with redesigned, component-based avatar implementations (size-based API), updates usages across the SDK and sample app, adds migration docs, introduces a git dependency on stream_core_flutter, and adjusts CI/workflow and map marker sizing to enum-based MarkerSize. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Failure to add the new IP will result in interrupted reviews. 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 |
This commit updates the `stream_core_flutter` dependency to use a version from a git repository. Specifically, it points to the `feat/avatar-group-and-badge-count` branch of the `GetStream/stream-core-flutter` repository.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_chat_flutter/lib/src/poll/interactor/poll_options_list_view.dart (1)
223-249:⚠️ Potential issue | 🟡 MinorRemove or use the unused
radiusparameter.The
radiusfield inOptionVotersaccepts a value but is never used in thebuildmethod. The avatar size is hard-coded to.xsviaStreamUserAvatarStack. Since the only instantiation in the codebase doesn't pass a custom radius, either remove the parameter or map it to control the avatar size.
🤖 Fix all issues with AI agents
In `@analysis_options.yaml`:
- Around line 47-49: Fix the minor grammar in the comment above the disabled
lint rule `avoid_redundant_argument_values`: change "Does not always makes sense
to remove them, Also makes it hard to notice future breaking changes if any." to
a grammatically correct sentence such as "Does not always make sense to remove
them; it also makes it hard to notice future breaking changes." so the comment
reads clearly and correctly.
In
`@packages/stream_chat_flutter/lib/src/components/avatar/stream_channel_avatar.dart`:
- Around line 83-86: The builder in StreamChannelAvatar currently maps members
with members.map((it) => it.user!) which can throw on null users; change the
mapping to filter out null Member.user values (e.g., members.where((m) => m.user
!= null).map((m) => m.user!)) and ensure StreamUserAvatarGroup receives a
non-null list (handle the empty list case by passing an empty list or a fallback
user list) so the UI never crashes when some members have partial payloads.
In
`@packages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar_stack.dart`:
- Around line 66-74: The constructor docs for StreamUserAvatarStack promise "The
[users] list must not be empty" but there's no enforcement; update the API to
enforce this invariant by changing the field and constructor parameter from
Iterable<User> to List<User> and adding assert(users.isNotEmpty, 'users must not
be empty') in the StreamUserAvatarStack constructor (and update all call sites),
or if you cannot change the type, add a runtime check in the build() method (or
constructor) that verifies the iterable is non-empty (e.g., by checking
.isNotEmpty on a collection or using iterator.moveNext()) and throw/assert with
the same message.
🧹 Nitpick comments (4)
packages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar_group.dart (1)
50-58: Consider adding assertion for non-empty users list.The documentation at line 53 states "The [users] list must not be empty", but there's no runtime assertion to enforce this. Consider adding an assertion in the constructor or in the
buildmethod to catch misuse early in development.💡 Suggested assertion
const StreamUserAvatarGroup({ super.key, required this.users, this.size, - }); + }) : assert(users.isNotEmpty, 'users must not be empty');Note: Since
usersis anIterable, callingisNotEmptyin an assertion may iterate the collection. If this is a concern, you could defer the check to thebuildmethod or remove the documentation claim.packages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar.dart (1)
99-104: Consider a stable hash if avatar colors must persist consistently across app sessions.
String.hashCodeis deterministic within a single app session but not guaranteed stable across program executions. The current code achieves within-session color consistency for the same user ID. However, if you need avatar colors to remain consistent when users close and reopen the app, use a stable hash function instead.🔧 Suggested change (stable hash)
- final userHash = user.id.hashCode; // Ensure deterministic colors. + final userHash = _stableHash(user.id); // Ensure deterministic colors. final colorPair = avatarPalette[userHash % avatarPalette.length]; @@ StreamOnlineIndicatorSize _indicatorSizeForAvatarSize( StreamAvatarSize size, ) => switch (size) { .xs || .sm => StreamOnlineIndicatorSize.sm, .md => StreamOnlineIndicatorSize.md, .lg => StreamOnlineIndicatorSize.lg, .xl => StreamOnlineIndicatorSize.xl, }; + + static int _stableHash(String value) { + var hash = 0; + for (final unit in value.codeUnits) { + hash = 31 * hash + unit; + } + return hash & 0x7fffffff; + } }packages/stream_chat_flutter/lib/src/dialogs/channel_info_dialog.dart (1)
52-65: Consider extracting the repeated member lookup.The
firstWherecall with the same predicate is executed twice—once for the avatar (lines 52-56) and once for the name (lines 60-65). Extracting this to a local variable would improve readability and avoid redundant iteration.♻️ Suggested refactor
if (channel.isDistinct && channel.memberCount == 2) Column( children: [ + Builder( + builder: (context) { + final otherMember = members + .firstWhere((e) => e.user?.id != userAsMember.user?.id); + return Column( + children: [ + StreamUserAvatar( + size: .xl, + user: otherMember.user!, + ), + const SizedBox(height: 6), + Text( + otherMember.user?.name ?? '', + style: StreamChatTheme.of(context).textTheme.footnoteBold, + maxLines: 1, + overflow: TextOverflow.ellipsis, + ), + ], + ); + }, + ), - StreamUserAvatar( - size: .xl, - user: members - .firstWhere( - (e) => e.user?.id != userAsMember.user?.id, - ) - .user!, - ), - const SizedBox(height: 6), - Text( - members - .firstWhere( - (e) => e.user?.id != userAsMember.user?.id, - ) - .user - ?.name ?? - '', - style: StreamChatTheme.of(context).textTheme.footnoteBold, - maxLines: 1, - overflow: TextOverflow.ellipsis, - ), ], ),sample_app/lib/pages/new_group_chat_screen.dart (1)
145-148: Consider hiding the online indicator for selected members.Per the PR objectives,
showOnlineIndicatordefaults totrueforStreamUserAvatar. In this context (displaying selected group members), showing the online status may add visual noise. Consider explicitly settingshowOnlineIndicator: falseif the online indicator is not desired here.♻️ Suggested change
StreamUserAvatar( size: .xl, user: user, + showOnlineIndicator: false, ),
packages/stream_chat_flutter/lib/src/components/avatar/stream_channel_avatar.dart
Show resolved
Hide resolved
packages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar_stack.dart
Outdated
Show resolved
Hide resolved
Temporarily skip LFS smudge process on CI workflows to work around an issue with a dependency.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
🤖 Fix all issues with AI agents
In
`@packages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar_stack.dart`:
- Around line 99-112: The build method of StreamUserAvatarStack currently always
constructs a StreamAvatarStack with children from users, but the docstring says
it should return an empty SizedBox when users is empty; update build to check
users.isEmpty and return SizedBox.shrink() (or SizedBox()) when true, otherwise
map users to StreamUserAvatar as before; ensure you reference the existing
symbols (build, users, StreamAvatarStack, StreamUserAvatar, max, size, overlap)
and keep the existing mapping logic for the non-empty case.
🧹 Nitpick comments (2)
.github/workflows/release_publish.yml (1)
10-12: Reasonable temporary workaround; consider tracking the TODO with an issue.The
GIT_LFS_SKIP_SMUDGEenv var is appropriate for avoiding unnecessary LFS downloads when using git-based dependencies. The TODO comment is good practice. However, workflow-level TODOs can be easily overlooked once the PR is merged.Consider creating a tracking issue linked to the
stream_core_flutterpublication milestone to ensure this env block gets removed, preventing it from becoming permanent technical debt.packages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar_stack.dart (1)
84-89: Consider validating overlap bounds.The documentation specifies overlap should be between
0.0and1.0, but there's no assertion preventing invalid values. Negative overlap or values exceeding 1.0 could produce unexpected layout behavior.🛡️ Proposed assertion
const StreamUserAvatarStack({ super.key, required this.users, this.size, this.overlap = 0.33, this.max = 5, - }) : assert(max >= 2, 'max must be at least 2'); + }) : assert(max >= 2, 'max must be at least 2'), + assert(overlap >= 0.0 && overlap <= 1.0, 'overlap must be between 0.0 and 1.0');
packages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar_stack.dart
Show resolved
Hide resolved
packages/stream_chat_flutter/lib/src/components/avatar/stream_channel_avatar.dart
Show resolved
Hide resolved
The migration guide for the redesigned UI components is updated to include a new "Theming" section. This section explains how to use `StreamTheme` to customize the appearance of the new components, providing code examples for both light and dark modes. It also adds a "Need Help?" section directing users to open a GitHub issue if they encounter problems.
Core PR: GetStream/stream-core-flutter#43
Description of the pull request
This PR introduces a complete redesign of the Avatar components to simplify the API, improve consistency, and align with the new design system.
The following components have been removed and replaced:
StreamUserAvatarStreamGroupAvatarStreamChannelAvatarThey are replaced by a new suite of components:
StreamUserAvatar: Displays a single user's avatar.StreamUserAvatarGroup: Displays a grid of user avatars for group channels.StreamUserAvatarStack: A new component for displaying a stack of overlapping user avatars.StreamChannelAvatar: Rebuilt to internally use the new avatar components.Key changes:
constraintsparameter has been replaced with asizeenum (e.g.,StreamAvatarSize.lg) for standardized sizing.onTapcallback has been removed. Avatars should now be wrapped with aGestureDetectororInkWellfor handling interactions.borderRadius,selected,selectionColor, andselectionThicknesshave been removed to simplify the API. Customization is now handled through theming.showOnlineStatusparameter has been renamed toshowOnlineIndicator. Its default is nowtrueforStreamUserAvatarandfalsefor avatars inside groups or stacks.StreamGroupAvataris nowStreamUserAvatarGroup: The component has been renamed and now accepts a list ofUserobjects instead ofMemberobjects.migrations/redesign/stream_avatar.md.StreamThemeextension fromstream_core_flutterfor better theme management.Screenshots / Videos
Summary by CodeRabbit
New Features
Breaking Changes
Documentation