-
Notifications
You must be signed in to change notification settings - Fork 13
Consolidate avatar components #510
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
Conversation
WalkthroughReplaces ContactAvatar/ProfileAvatar with WnAvatar across UI screens and widgets. Updates imports accordingly, adds/uses an optional showBorder parameter in many call sites, and removes profile_avatar.dart. Renames the avatar class in wn_avatar.dart to WnAvatar. No changes to business logic or control flow. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration 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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
lib/ui/core/ui/wn_avatar.dart (1)
84-96: Handle potential StringIndexOutOfBoundsException.The
substring(0, 1)call assumes the trimmed string has at least one character, but an empty string after trimming could cause an exception.// Show first letter of displayName if available - if (displayName != null && displayName!.trim().isNotEmpty) { + if (displayName != null && displayName!.trim().isNotEmpty) { + final trimmed = displayName!.trim(); return Center( child: Text( - displayName!.trim().substring(0, 1).toUpperCase(), + trimmed.substring(0, 1).toUpperCase(), style: TextStyle( fontSize: size * 0.4, fontWeight: FontWeight.bold, color: context.colors.primary, ), ), ); }
🧹 Nitpick comments (14)
lib/ui/core/ui/wn_avatar.dart (1)
60-68: Consider adding error handling for file system operations.The
File(imageUrl).existsSync()call could potentially throw exceptions on certain platforms or with invalid paths. Consider wrapping this in a try-catch block.// Try local file image - if (File(imageUrl).existsSync()) { + try { + if (File(imageUrl).existsSync()) { return Image.file( File(imageUrl), width: size, height: size, fit: BoxFit.cover, errorBuilder: (context, error, stackTrace) => _buildFallbackAvatar(context), ); + } + } catch (e) { + // Fall through to asset image attempt }lib/ui/contact_list/widgets/contact_list_tile.dart (1)
221-221: Consider increasing animation duration for better UX.The animation duration of 100ms might be too fast for a smooth shimmer effect. Consider increasing it to 1000-1500ms for a more polished loading experience.
_controller = AnimationController( - duration: const Duration(milliseconds: 100), + duration: const Duration(milliseconds: 1200), vsync: this, );lib/ui/settings/profile/share_profile_screen.dart (2)
97-103: Align border behavior with other screens (conditional border when no image)Other call sites often show a border only when there's no image (e.g., edit_profile_screen uses imageUrl.isEmpty). For consistency and to avoid unintended visual changes, consider making the border conditional on picture presence.
Apply this diff:
WnAvatar( imageUrl: profile.picture ?? '', displayName: profile.displayName ?? '', size: 96.w, - showBorder: true, + showBorder: (profile.picture ?? '').isEmpty, ),
1-216: No leftover legacy avatar imports/usages; address inconsistent showBorder usage✅ No
ContactAvatar/ProfileAvatarsymbols orprofile_avatar.dart/chat_contact_avatar.dartfiles found.
✅WnAvataris adopted throughout the codebase.
⚠️ However, theshowBorderparameter is omitted in several places. Please review and addshowBorderas needed for visual consistency:
- lib/ui/contact_list/chat_list_screen.dart:253
- lib/ui/contact_list/group_welcome_invitation_sheet.dart:62, 193, 271
- lib/ui/chat/chat_management/add_to_group_screen.dart:169
- lib/ui/chat/chat_info/group_chat_info.dart:127
- lib/ui/chat/chat_info/dm_chat_info.dart:148
- lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart:202
- lib/ui/contact_list/widgets/welcome_tile.dart:44
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (1)
202-206: Consider conditional border when the member has no imageMany screens add a border only when there’s no avatar image. Using the same rule here improves visual consistency across the app.
WnAvatar( imageUrl: widget.member.imagePath ?? '', displayName: widget.member.displayName, size: 96.w, + showBorder: (widget.member.imagePath ?? '').isEmpty, ),lib/ui/chat/chat_info/group_chat_info.dart (1)
127-131: Match header border behavior with ChatHeader (use showBorder: true here too)In chat_header_widget.dart, group headers pass showBorder: true. For a consistent header look between chat header and chat info, mirror that here.
WnAvatar( imageUrl: '', displayName: groupDetails?.name ?? 'Unknown Group', size: 96.w, + showBorder: true, ),lib/ui/contact_list/group_welcome_invitation_sheet.dart (3)
62-66: Provide displayName and showBorder for reliable fallback renderingWithout displayName, WnAvatar may not render initials when imageUrl is empty. Also, explicitly enabling the border for a group placeholder keeps the visual consistent with other invite headers.
Apply this diff:
- WnAvatar( - imageUrl: '', - size: 96.w, - ), + WnAvatar( + imageUrl: '', + displayName: welcomeData.groupName, + size: 96.w, + showBorder: true, + ),
193-196: Add displayName and conditional border for inviter avatarCover the no-picture case by passing displayName for initials and showing a border only when the URL is empty.
Apply this diff:
- WnAvatar( - imageUrl: snapshot.data?.picture ?? '', - size: 18.w, - ), + WnAvatar( + imageUrl: snapshot.data?.picture ?? '', + displayName: displayName, + size: 18.w, + showBorder: (snapshot.data?.picture ?? '').isEmpty, + ),
271-274: Strengthen DM avatar fallback (displayName + conditional border)Same rationale: initials when no image and a subtle border only when needed.
Apply this diff:
- return WnAvatar( - imageUrl: profileImageUrl, - size: 96.w, - ); + return WnAvatar( + imageUrl: profileImageUrl, + displayName: metadata?.displayName ?? metadata?.name ?? '', + size: 96.w, + showBorder: profileImageUrl.isEmpty, + );lib/ui/contact_list/widgets/welcome_tile.dart (1)
44-48: Toggle border when avatar image is missingSeveral other call sites enable a border for placeholders. Mirror that here for consistency.
Apply this diff:
WnAvatar( imageUrl: welcomerImageUrl, displayName: welcomerName, size: 56.r, + showBorder: welcomerImageUrl.isEmpty, ),lib/ui/chat/invite/chat_invite_screen.dart (1)
265-270: DM header avatar usage looks goodIncludes displayName and showBorder. If design wants the border only on missing images, consider toggling it based on URL emptiness; otherwise this is fine as-is.
Optionally:
- WnAvatar( - imageUrl: welcomerImageUrl, - displayName: welcomerName, - size: 96.r, - showBorder: true, - ), + WnAvatar( + imageUrl: welcomerImageUrl, + displayName: welcomerName, + size: 96.r, + showBorder: welcomerImageUrl.isEmpty, + ),lib/ui/chat/chat_management/add_to_group_screen.dart (1)
169-173: Enable border for group placeholders to align with invite headersConsistent look for group avatars across the app when there’s no image.
Apply this diff:
secondary: WnAvatar( imageUrl: '', displayName: group.name, size: 56.w, + showBorder: true, ),lib/ui/chat/chat_info/dm_chat_info.dart (1)
148-152: Unify size scaling and optionally show a border for empty imagesFor consistent circular avatar scaling across screens, prefer .r over .w. Also consider showing a border when the image is missing (as done in ChatListItemTile) to keep visuals consistent.
Apply this diff:
- WnAvatar( - imageUrl: dmChatData?.displayImage ?? '', - displayName: dmChatData?.displayName ?? 'Unknown', - size: 96.w, - ), + WnAvatar( + imageUrl: dmChatData?.displayImage ?? '', + displayName: dmChatData?.displayName ?? 'Unknown', + size: 96.r, + showBorder: (dmChatData?.displayImage ?? '').isEmpty, + ),lib/ui/contact_list/chat_list_screen.dart (1)
253-257: Optional: add a border when no profile image is setIf the previous header avatar showed a ring/edge when missing an image, add showBorder to preserve parity.
Apply this diff:
- child: WnAvatar( - imageUrl: profileImagePath, - displayName: currentDisplayName, - size: 36.r, - ), + child: WnAvatar( + imageUrl: profileImagePath, + displayName: currentDisplayName, + size: 36.r, + showBorder: profileImagePath.isEmpty, + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
lib/ui/auth_flow/create_profile_screen.dart(2 hunks)lib/ui/chat/chat_info/chat_info_screen.dart(1 hunks)lib/ui/chat/chat_info/dm_chat_info.dart(1 hunks)lib/ui/chat/chat_info/group_chat_info.dart(2 hunks)lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart(2 hunks)lib/ui/chat/chat_management/add_to_group_screen.dart(2 hunks)lib/ui/chat/invite/chat_invite_screen.dart(3 hunks)lib/ui/chat/widgets/chat_header_widget.dart(3 hunks)lib/ui/chat/widgets/contact_info.dart(2 hunks)lib/ui/contact_list/chat_list_screen.dart(2 hunks)lib/ui/contact_list/group_welcome_invitation_sheet.dart(4 hunks)lib/ui/contact_list/widgets/chat_list_item_tile.dart(2 hunks)lib/ui/contact_list/widgets/contact_list_tile.dart(2 hunks)lib/ui/contact_list/widgets/profile_avatar.dart(0 hunks)lib/ui/contact_list/widgets/user_profile.dart(2 hunks)lib/ui/contact_list/widgets/welcome_tile.dart(2 hunks)lib/ui/core/ui/wn_avatar.dart(1 hunks)lib/ui/settings/profile/edit_profile_screen.dart(2 hunks)lib/ui/settings/profile/share_profile_screen.dart(2 hunks)
💤 Files with no reviewable changes (1)
- lib/ui/contact_list/widgets/profile_avatar.dart
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit Inference Engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/chat/chat_info/dm_chat_info.dartlib/ui/chat/chat_info/chat_info_screen.dartlib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/chat/widgets/contact_info.dartlib/ui/core/ui/wn_avatar.dartlib/ui/contact_list/widgets/contact_list_tile.dartlib/ui/chat/chat_info/group_chat_info.dartlib/ui/auth_flow/create_profile_screen.dartlib/ui/contact_list/widgets/welcome_tile.dartlib/ui/settings/profile/share_profile_screen.dartlib/ui/contact_list/widgets/user_profile.dartlib/ui/chat/chat_management/add_to_group_screen.dartlib/ui/contact_list/chat_list_screen.dartlib/ui/settings/profile/edit_profile_screen.dartlib/ui/contact_list/group_welcome_invitation_sheet.dartlib/ui/contact_list/widgets/chat_list_item_tile.dartlib/ui/chat/widgets/chat_header_widget.dartlib/ui/chat/invite/chat_invite_screen.dart
🧠 Learnings (2)
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
lib/ui/contact_list/widgets/welcome_tile.dartlib/ui/contact_list/group_welcome_invitation_sheet.dartlib/ui/contact_list/widgets/chat_list_item_tile.dartlib/ui/chat/invite/chat_invite_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/contact_list/widgets/chat_list_item_tile.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (29)
lib/ui/core/ui/wn_avatar.dart (2)
7-16: LGTM! Clean class rename and parameter addition.The rename from
ContactAvatartoWnAvatarfollows proper naming conventions, and the addition of theshowBorderparameter with a sensible default value maintains backward compatibility while extending functionality.
31-37: LGTM! Border implementation follows Flutter best practices.The conditional border implementation using the
showBorderparameter is well-structured and properly uses theme colors with fallback values.lib/ui/contact_list/widgets/contact_list_tile.dart (2)
10-10: LGTM! Import updated correctly.The import has been properly updated to use the new
WnAvatarlocation.
55-60: LGTM! Avatar usage updated correctly with appropriate border logic.The migration to
WnAvataris correctly implemented with theshowBorderparameter logically set totruewhen no image is available, which provides a good visual indicator.lib/ui/contact_list/widgets/user_profile.dart (2)
8-8: LGTM! Import updated correctly.The import statement has been properly updated to reference the new
WnAvatarwidget location.
40-45: LGTM! Avatar migration implemented correctly.The migration to
WnAvataris properly implemented with appropriate parameter mapping. TheshowBorderlogic makes sense - showing a border when no image is provided gives good visual feedback to the user.lib/ui/auth_flow/create_profile_screen.dart (2)
9-9: LGTM! Import updated correctly.The import has been properly updated to use the consolidated
WnAvatarwidget.
97-102: LGTM! Avatar migration simplifies the code.The migration to
WnAvataris well-implemented. Passing the fulldisplayTextasdisplayNamesimplifies the logic since the avatar widget now handles the first letter extraction internally.lib/ui/chat/widgets/contact_info.dart (2)
4-4: LGTM! Import updated correctly.The import statement has been properly updated to reference the consolidated
WnAvatarwidget.
56-61: LGTM! Avatar usage updated correctly.The migration to
WnAvataris properly implemented withshowBorder: trueproviding consistent visual styling for contact information displays.lib/ui/settings/profile/share_profile_screen.dart (1)
14-14: Import path update to WnAvatar looks correctCore UI import is aligned with the new location/name.
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (1)
14-14: Import updated to core WnAvatar — OKMatches the new shared widget location.
lib/ui/settings/profile/edit_profile_screen.dart (2)
13-13: Import updated to WnAvatar — OKConsistent with the consolidation effort.
152-157: LGTM: Correct conditional border and argsBorder follows image presence; arguments match the new WnAvatar API.
lib/ui/chat/chat_info/group_chat_info.dart (1)
235-239: LGTM: Member list tile avatar migrationCorrect use of WnAvatar with a small size and explicit border matches the intended list style.
lib/ui/chat/widgets/chat_header_widget.dart (3)
11-11: Core avatar import is correctImport path aligns with the new consolidated widget location.
67-72: LGTM: Group chat header avatar migrationArguments and showBorder usage look consistent.
164-169: LGTM: Direct message header avatar migrationConsistent border usage and parameter mapping.
lib/ui/contact_list/group_welcome_invitation_sheet.dart (1)
11-11: Import swap to WnAvatar looks correct and library-wide for partsMaking WnAvatar available at this library root ensures any part files needing it also compile. No issues spotted.
lib/ui/contact_list/widgets/welcome_tile.dart (1)
10-10: Import updated to WnAvatar — OKThe centralized avatar component import aligns with the consolidation effort.
lib/ui/chat/invite/chat_invite_screen.dart (2)
15-15: Import replacement is appropriateSwitching to WnAvatar at the screen level is consistent with the migration and exposes it to sub-widgets here.
164-170: Group header avatar usage looks goodPassing displayName for initials, size at 96.r, and enabling a border matches the intended presentation for group placeholders.
lib/ui/chat/chat_management/add_to_group_screen.dart (1)
10-10: Import updated to WnAvatar — OKAll good.
lib/ui/chat/chat_info/chat_info_screen.dart (1)
20-20: Library-level import of WnAvatar is correct for part filesThis ensures DMChatInfo and GroupChatInfo (parts) can reference WnAvatar without their own imports. Nice catch.
lib/ui/chat/chat_info/dm_chat_info.dart (2)
148-152: Avatar refactor applied correctlySwapping to WnAvatar with equivalent props preserves behavior. No logic changes introduced here.
148-152: Parent file already importswn_avatar.dart, no action required
- lib/ui/chat/chat_info/chat_info_screen.dart (line 20):
import 'package:whitenoise/ui/core/ui/wn_avatar.dart';lib/ui/contact_list/widgets/chat_list_item_tile.dart (2)
15-15: Import updated to new avatar componentCorrectly points to lib/ui/core/ui/wn_avatar.dart.
95-101: LGTM: WnAvatar usage with conditional borderPassing showBorder: displayImageUrl.isEmpty matches the new API and keeps empty-image cases visually distinct.
lib/ui/contact_list/chat_list_screen.dart (1)
25-25: All old avatar references have been removedNo imports or files for
ProfileAvatar/ContactAvatar,chat_contact_avatar.dart, orprofile_avatar.dartremain—onlylib/ui/core/ui/wn_avatar.dartis present.
josefinalliende
left a 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.
✅ LGTM!!! 🚀
| imageUrl: ref.watch(accountProvider).selectedImagePath ?? '', | ||
| displayName: firstLetter, | ||
| displayName: displayText, | ||
| size: 96.w, |
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.
Related to size prop, but not for the scope of this PR... what do you think about WnButtonSize way of handling sizes?
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.
I'd seen that in some places no size is needed (so default is used, in others is passed a size with .w and in others with .r
| return WnAvatar( | ||
| imageUrl: ref.watch(accountProvider).selectedImagePath ?? '', | ||
| displayName: firstLetter, | ||
| displayName: displayText, |
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.
This change was a bit confusing to me, but it seems to be working fine!
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.
We can now pass the full string to WnAvatar. the naming here has changed because WnAvatar does the trimming.
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.
Thanks for the explanation @untreu2 🫶
Description
ContactAvatar→WnAvatarlib/ui/chat/widgets/tolib/ui/core/ui/ProfileAvatarwidget (replaced withWnAvatar)Everything looks exactly the same.
Type of Change
Checklist
just precommitto ensure that formatting and linting are correctCHANGELOG.mdfile with your changesFixes #489