Replace png icons with svgs #534
Conversation
WalkthroughMigrates several app icons from PNG to SVG, adds new SVG asset constants, switches image rendering to SvgPicture, changes chat header fallback to the user SVG, and removes the group-image upload toggle (leaving a TODO/no-op). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Data as Image URL (String?)
participant Ext as StringExtension.orDefault
participant UI as ChatHeader
Data->>Ext: provide nullable/empty string
alt null or empty
Ext-->>UI: AssetsPaths.icUser
else has value
Ext-->>UI: original string
end
UI-->>UI: render avatar using provided path (SvgPicture or Image)
sequenceDiagram
autonumber
actor User
participant UI as GroupChatDetailsSheet
Note over UI: Image area always shows camera icon
User->>UI: tap image area
UI-->>UI: no-op / TODO (upload not implemented)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Out-of-scope changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/ui/contact_list/group_chat_details_sheet.dart (1)
219-221: Complete group image upload feature implementation or track as issue.The TODO comment indicates incomplete functionality. Consider either implementing the group image upload feature as part of this refactor or creating a proper issue to track this work.
Would you like me to help create a GitHub issue to track the group image upload feature implementation?
📜 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 ignored due to path filters (17)
assets/pngs/group_logo.pngis excluded by!**/*.pngassets/pngs/hands.pngis excluded by!**/*.pngassets/pngs/ic_add_chat.pngis excluded by!**/*.pngassets/pngs/ic_avatar.pngis excluded by!**/*.pngassets/pngs/ic_checkmark_dashed.pngis excluded by!**/*.pngassets/pngs/ic_checkmark_filled.pngis excluded by!**/*.pngassets/pngs/ic_checkmark_solid.pngis excluded by!**/*.pngassets/pngs/ic_image.pngis excluded by!**/*.pngassets/pngs/ic_notification_muted.pngis excluded by!**/*.pngassets/pngs/ic_off_chat.pngis excluded by!**/*.pngassets/pngs/ic_whitenoise.pngis excluded by!**/*.pngassets/pngs/login_splash.pngis excluded by!**/*.pngassets/pngs/profile_background.pngis excluded by!**/*.pngassets/svgs/ic_add_new_chat.svgis excluded by!**/*.svgassets/svgs/ic_checkmark_solid.svgis excluded by!**/*.svgassets/svgs/ic_off_chat.svgis excluded by!**/*.svgpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
lib/ui/chat/widgets/chat_header_widget.dart(1 hunks)lib/ui/contact_list/group_chat_details_sheet.dart(2 hunks)lib/ui/core/themes/assets.dart(1 hunks)
🧰 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/widgets/chat_header_widget.dartlib/ui/contact_list/group_chat_details_sheet.dartlib/ui/core/themes/assets.dart
🔇 Additional comments (4)
lib/ui/chat/widgets/chat_header_widget.dart (1)
208-208: Updated fallback asset path looks correct.The change from
AssetsPaths.icImagetoAssetsPaths.icUseras the fallback when the string is null or empty appears appropriate for a user-related context. This aligns with the PR objective of migrating from PNG to SVG assets.lib/ui/contact_list/group_chat_details_sheet.dart (1)
230-238: icCamera SVG asset verified
- Confirmed the SVG file exists at
assets/svgs/ic_camera.svg.- The constant is correctly defined in
lib/ui/core/themes/assets.dartasstatic const String icCamera = '$_svgsDir/ic_camera.svg';- Usage in
lib/ui/contact_list/group_chat_details_sheet.dartis valid withSvgPicture.asset(AssetsPaths.icCamera).No further changes required.
lib/ui/core/themes/assets.dart (2)
72-75: All migrated SVG assets verified
- The four new SVG files (ic_add_new_chat.svg, ic_off_chat.svg, ic_checkmark_solid.svg, ic_checkmark_filled.svg) are present under assets/svgs/
- No remaining references to the old PNG assets were found in the Dart code
- pubspec.yaml includes the assets/svgs/ directory, so all SVGs will be bundled
No further changes needed.
77-82: All removed PNG assets are unused or have SVG replacements – safe to remove
- A project-wide search found no references to the old PNG constants (
icImage,groupLogo,loginSplash,profileBackground,avatarPlaceholder,hands,icNotificationMuted,icCheckmarkDashed,icAvatar).- The only match for
icWhiteNoisenow points toAssetsPaths.icWhiteNoiseSvg, confirming migration to SVG.- The only remaining PNGs in
assets.dartare the four intentional ones (blueHoodie,purpleWoman,greenBird,login).No breaking changes detected; removal of the other PNG constants is safe.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/ui/contact_list/chat_list_screen.dart (2)
339-351: Add accessible labels for the icon (a11y/UX)The icon is unlabeled when read by assistive tech since
IconButtonhas notooltipand the SVG lacks asemanticsLabel. Add a label so VO/TalkBack announce it meaningfully.Apply within this block:
- icon: SvgPicture.asset( + icon: SvgPicture.asset( + semanticsLabel: 'New chat', notAllRelayTypesConnected ? AssetsPaths.icOffChat : AssetsPaths.icAddNewChat, width: 21.w, height: 21.w, colorFilter: ColorFilter.mode( context.colors.solidNeutralWhite.withValues( alpha: notAllRelayTypesConnected ? 0.5 : 1.0, ), BlendMode.srcIn, ), ),And (outside this range) consider adding a tooltip on the IconButton:
IconButton( tooltip: notAllRelayTypesConnected ? 'No relays connected' : 'Start a new chat', onPressed: ... icon: ... )
345-349: Consider withOpacity for wider Flutter compatibility
Color.withValues(...)is available on newer Flutter/Dart SDKs. If your minimum Flutter version predates this API, usewithOpacity(...)to avoid build breaks. Functionally equivalent here since only alpha changes.- colorFilter: ColorFilter.mode( - context.colors.solidNeutralWhite.withValues( - alpha: notAllRelayTypesConnected ? 0.5 : 1.0, - ), - BlendMode.srcIn, - ), + colorFilter: ColorFilter.mode( + context.colors.solidNeutralWhite.withOpacity( + notAllRelayTypesConnected ? 0.5 : 1.0, + ), + BlendMode.srcIn, + ),If you’re certain the repo targets a Flutter version that supports
withValues, feel free to keep as-is.
📜 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 (1)
lib/ui/contact_list/chat_list_screen.dart(1 hunks)
🧰 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/contact_list/chat_list_screen.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 (2)
lib/ui/contact_list/chat_list_screen.dart (2)
339-351: Migration to SvgPicture and colorFilter looks correctSwitching to
SvgPicture.assetwithColorFilter.mode(..., BlendMode.srcIn)preserves the previous tint behavior and correctly handles the disabled state via alpha. Asset constants (icOffChat,icAddNewChat) align with the PR’s SVG migration. Nice work.
339-351: The above script will confirm whether any legacy PNG versions of the chat icons remain underassets/pngs. Once you have the results, we can determine if you need to remove those files and update the pubspec accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/ui/core/themes/assets.dart (3)
72-75: Avoid duplicate constants for the same asset; alias and deprecate the old name.You already have icCheckmarkFilledSvg (Line 49) pointing to the same file. Adding icCheckmarkFilled with an identical path creates two source-of-truths and invites drift. Alias the new preferred name to the old one, and deprecate the old constant to guide callers.
Apply this diff to the changed lines:
- static const String icCheckmarkFilled = '$_svgsDir/ic_checkmark_filled.svg'; + // Preferred name (aliases the legacy *Svg name). + static const String icCheckmarkFilled = icCheckmarkFilledSvg;Outside the changed hunk (no diff here, illustrative only), consider marking the legacy constant as deprecated to steer usage:
@Deprecated('Use AssetsPaths.icCheckmarkFilled instead') static const String icCheckmarkFilledSvg = '$_svgsDir/ic_checkmark_filled.svg';
80-85: Remaining PNG assets verified and in use
All four assets (blue_hoodie.png,purple_woman.png,green_bird.png,login.png) are present underassets/pngs/and are referenced in the code as follows:
AssetsPaths.loginused inlib/ui/auth_flow/login_screen.dart(Image.asset call)AssetsPaths.blueHoodie,AssetsPaths.purpleWoman,AssetsPaths.greenBirdused inlib/ui/auth_flow/info_screen.dartviaFeatureItemwidgetsThese files appear to be intentionally raster images, not simple icons or line art. If they truly represent photo-style or complex bitmap content, it’s fine to keep them as PNGs. If any are simpler illustrations that would benefit from being vectorized, consider converting them to SVG and swapping the calls from
Image.assettoSvgPicture.asset.
36-38: Asset filename typo confirmed; consider renaming for clarityThe asset file on disk is indeed named
ic_mute_notfication.svg, matching the code’s reference, so nothing is broken at runtime. However, the word “notfication” is misspelled. To improve clarity and consistency, please rename the file and update the constant:• Rename on disk:
– assets/svgs/ic_mute_notfication.svg
+– assets/svgs/ic_mute_notification.svg• Update constant in
lib/ui/core/themes/assets.dart:- static const String icMutedNotification = '$_svgsDir/ic_mute_notfication.svg'; + static const String icMutedNotification = '$_svgsDir/ic_mute_notification.svg';No changes are necessary in
pubspec.yaml(theassets/svgs/directory is already included).
📜 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 (1)
lib/ui/core/themes/assets.dart(1 hunks)
🧰 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/core/themes/assets.dart
🔇 Additional comments (2)
lib/ui/core/themes/assets.dart (2)
1-85: Sweep completed: no stale PNG references found
A global search for the removed/renamed PNG constants (icImage,groupLogo,loginSplash, non-SVGicWhiteNoise,profileBackground,avatarPlaceholder,hands,icNotificationMuted,icCheckmarkDashed,icAvatar) returned zero matches—everything now points to the updated SVG constants. No further action needed.
72-75: ✅ All SVG assets verified and pubspec declarations present
All four SVGs exist under assets/svgs/, and pubspec.yaml includes the entire assets/svgs/ directory (line 111). No further changes needed.
| static const String icAddNewChat = '$_svgsDir/ic_add_new_chat.svg'; | ||
| static const String icOffChat = '$_svgsDir/ic_off_chat.svg'; | ||
| static const String icCheckmarkSolid = '$_svgsDir/ic_checkmark_solid.svg'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify coexistence of icAddNewChat vs icNewChat.
There are now two “new chat” icons: icNewChat (Line 26) and icAddNewChat (Line 72). If both are intentional variants, add brief doc comments describing when each should be used. If not, consolidate to one name and migrate call sites.
Optionally add short docs on the new constants:
- static const String icAddNewChat = '$_svgsDir/ic_add_new_chat.svg';
+ /// "Add new chat" (plus badge) – use on actions that create a chat.
+ static const String icAddNewChat = '$_svgsDir/ic_add_new_chat.svg';
- static const String icOffChat = '$_svgsDir/ic_off_chat.svg';
+ /// "Chat off/disabled" state icon.
+ static const String icOffChat = '$_svgsDir/ic_off_chat.svg';Run to verify distinct usage:
🏁 Script executed:
#!/bin/bash
# Expect: Both used intentionally in different contexts (review call sites).
rg -nP --type=dart -C2 'AssetsPaths\.(icAddNewChat|icNewChat)\b'Length of output: 529
To confirm whether icNewChat is actually referenced anywhere, let’s search the entire codebase:
🏁 Script executed:
#!/bin/bash
# Check for any usage of the older `icNewChat` constant
rg -nP --type=dart '\bAssetsPaths\.icNewChat\b'Length of output: 49
🏁 Script executed:
#!/bin/bash
# Locate the definition and context of the `icNewChat` constant in the assets file
rg -nP --type=dart 'icNewChat' -C3 lib/uiLength of output: 742
Remove the unused icNewChat constant and document the new icons
Verified that icNewChat is defined in assets.dart (line 26) but is never referenced in the codebase. To avoid confusion and dead code, remove it, and add short doc comments to the remaining new-chat and off-chat icons:
• In lib/ui/core/themes/assets.dart—
– Remove the unused line:
dart static const String icNewChat = '$_svgsDir/ic_new_chat.svg';
• In the same file at lines 72–74—replace with:
diff - static const String icAddNewChat = '$_svgsDir/ic_add_new_chat.svg'; + /// "Add new chat" (plus badge) – use on actions that create a chat. + static const String icAddNewChat = '$_svgsDir/ic_add_new_chat.svg'; - static const String icOffChat = '$_svgsDir/ic_off_chat.svg'; + /// "Chat off/disabled" state icon. + static const String icOffChat = '$_svgsDir/ic_off_chat.svg';
🤖 Prompt for AI Agents
In lib/ui/core/themes/assets.dart around line 26 and lines 72–74, remove the
dead unused constant `static const String icNewChat =
'$_svgsDir/ic_new_chat.svg';` (line ~26) and add short Dart doc comments to the
remaining chat-related icon constants at lines 72–74 (icAddNewChat, icOffChat,
icCheckmarkSolid) describing their purpose (e.g., /// Icon for adding a new
chat, /// Icon for toggling chat off, /// Solid checkmark icon) so the file has
no unused constants and the new icons are documented.
Description
Fixes #491
Replaced all replaceable pngs with their corresponding svgs
Type of Change
Checklist
just precommitto ensure that formatting and linting are correctjust check-flutter-coverageto ensure that flutter coverage rules are passingCHANGELOG.mdfile with your changes (if they affect the user experience)Summary by CodeRabbit
New Features
Refactor
Chores