Remove CarbonIcons package and use SVGs for all icons #487
Conversation
WalkthroughReplaced CarbonIcons/IconData usage across the app with SVG asset-based icons, added many AssetsPaths constants, updated UI widgets to render SvgPicture with ColorFilter, adjusted APIs (RelayStatus.getIcon→getIconAsset, WnStickyHeadsUp/WnHeadingType icon→iconAsset, MenuItem.icon→assetPath, SettingsListTile icon→assetPath), removed supa_carbon_icons, and updated tests accordingly. Changes
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. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
✨ 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
🔭 Outside diff range comments (4)
lib/ui/settings/nostr_keys/nostr_keys_screen.dart (1)
64-66: Dispose all TextEditingControllers to prevent leaks_publicKeyController is never disposed. This can leak listeners and resources in a StatefulWidget.
Apply this diff:
@override void dispose() { + _publicKeyController.dispose(); _privateKeyController.dispose(); super.dispose(); }lib/ui/chat/chat_management/add_to_group_screen.dart (1)
41-44: Loading flag remains true on early return in _loadGroupsIf groups is null/empty, you return while _isLoading is still true, leaving the UI in a perpetual loading state.
Apply this diff:
- final groups = ref.read(groupsProvider).groups; - if (groups == null || groups.isEmpty) { - return; - } + final groups = ref.read(groupsProvider).groups; + if (groups == null || groups.isEmpty) { + setState(() { + _isLoading = false; + }); + return; + }Alternatively, wrap the method body in try/finally and reset _isLoading in finally.
lib/ui/settings/general_settings_screen.dart (2)
247-251: Logic bug: accounts.length > 2 should be > 1“Multiple accounts” means two or more. Using > 2 skips the 2-accounts case and alters logout flow.
- final hasMultipleAccounts = accounts.length > 2; + final hasMultipleAccounts = accounts.length > 1;
104-111: Guard setState after async work with mounted check_loadAccounts awaits multiple futures; the widget may unmount before setState, causing exceptions.
} - setState(() { + if (!mounted) return; + setState(() { _accounts = accounts; _currentAccount = currentAccount; _accountContactModels = contactModels; });
🧹 Nitpick comments (43)
lib/ui/core/themes/assets.dart (1)
49-71: Asset constants look good; consider adding a private constructor and documenting name collisions
- The additions are consistent with existing naming. One exception is icCheckmarkFilledSvg, which is intentionally suffixed to avoid collision with the PNG icCheckmarkFilled—this is fine but worth documenting to avoid confusion later.
- Prevent accidental instantiation of this static-only class with a private constructor.
Add a private constructor near the top of the file:
class AssetsPaths { const AssetsPaths._(); // Prevent instantiation static const String _svgsDir = 'assets/svgs'; // ... }Optionally add a short comment above icCheckmarkFilledSvg explaining the PNG/SVG name collision to guide future contributors.
test/ui/contact_list/share_invite_bottom_sheet_test.dart (1)
68-69: Make the finder robust: avoid byType(SvgPicture) which is brittleMatching by widget type can fail if multiple SvgPictures exist. Prefer a stable Key (or semantics label) for the specific copy button.
Apply this diff:
- final copyButton = find.byType(SvgPicture); + final copyButton = find.byKey(const Key('copyButton'));Then, in the production widget (ShareInviteBottomSheet), set a key on the copy button (example):
// inside ShareInviteBottomSheet where the copy button is built CustomIconButton( key: const Key('copyButton'), onTap: () => /* copy logic */, iconPath: AssetsPaths.icCopy, // ... )Alternatively (if you can’t add a Key), use a predicate on the asset name:
final copyButton = find.byWidgetPredicate( (w) => w is SvgPicture && w.bytesLoader is SvgAssetLoader && (w.bytesLoader as SvgAssetLoader).assetName.endsWith('ic_copy.svg'), );lib/ui/settings/donate/donate_screen.dart (1)
58-66: Prefer IconButton (with tooltip) for accessibility and better hit testingUsing GestureDetector with a raw SVG loses built-in semantics, focus, and a11y affordances. IconButton also provides a consistent tap target and ripple.
Apply this diff:
- GestureDetector( - onTap: () => context.pop(), - child: SvgPicture.asset( - AssetsPaths.icChevronLeft, - width: 24.w, - height: 24.w, - colorFilter: ColorFilter.mode( - context.colors.primary, - BlendMode.srcIn, - ), - ), - ), + IconButton( + onPressed: () => context.pop(), + tooltip: 'Back', + icon: SvgPicture.asset( + AssetsPaths.icChevronLeft, + width: 24.w, + height: 24.w, + colorFilter: ColorFilter.mode( + context.colors.primary, + BlendMode.srcIn, + ), + ), + ),test/ui/contact_list/widgets/user_profile_test.dart (1)
103-105: Stabilize the copy-button finder to avoid false positivesAsserting by SvgPicture type may match unrelated SVGs. Use a Key (preferred) or an asset-based predicate.
Apply this diff:
- final copyButton = find.byType(SvgPicture); + final copyButton = find.byKey(const Key('copyButton'));Production change (UserProfile) to support the test:
CustomIconButton( key: const Key('copyButton'), onTap: () => /* copy logic */, iconPath: AssetsPaths.icCopy, // ... )Alternate (no Key):
final copyButton = find.byWidgetPredicate( (w) => w is SvgPicture && w.bytesLoader is SvgAssetLoader && (w.bytesLoader as SvgAssetLoader).assetName.endsWith('ic_copy.svg'), );test/ui/contact_list/start_chat_bottom_sheet_test.dart (1)
172-176: Use a specific finder for the copy control; tapping byType(SvgPicture) can be ambiguousIf multiple SvgPictures are present, this will fail or tap the wrong widget. Prefer a Key or an asset-based predicate.
Apply this diff:
- final copyButton = find.byType(SvgPicture); + final copyButton = find.byKey(const Key('copyButton')); expect(copyButton, findsOneWidget); await tester.tap(copyButton);Production change (StartChatBottomSheet) to add the key:
CustomIconButton( key: const Key('copyButton'), onTap: () => /* copy logic */, iconPath: AssetsPaths.icCopy, // ... )Alternate (no Key):
final copyButton = find.byWidgetPredicate( (w) => w is SvgPicture && w.bytesLoader is SvgAssetLoader && (w.bytesLoader as SvgAssetLoader).assetName.endsWith('ic_copy.svg'), );lib/ui/settings/nostr_keys/nostr_keys_screen.dart (1)
96-104: Increase back button tap target and add semantics for accessibilityThe 24x24 tap area is small for touch. Also, SVGs don’t provide semantics by default. Consider enlarging the tap target and adding a semantics label.
Apply this diff to wrap the icon and add a label:
- child: SvgPicture.asset( - AssetsPaths.icChevronLeft, - colorFilter: ColorFilter.mode( - context.colors.primary, - BlendMode.srcIn, - ), - width: 24.w, - height: 24.w, - ), + child: SizedBox( + width: 48.w, + height: 48.w, + child: Center( + child: SvgPicture.asset( + AssetsPaths.icChevronLeft, + colorFilter: ColorFilter.mode( + context.colors.primary, + BlendMode.srcIn, + ), + width: 24.w, + height: 24.w, + semanticsLabel: 'Back', + ), + ), + ),Alternatively, replace GestureDetector with an IconButton to get ripple and built-in a11y.
lib/ui/chat/chat_management/add_to_group_screen.dart (1)
124-132: Use layout units (w/r) instead of sp for icon size; add semantics labelUsing .sp for icon dimensions ties icon size to text scale, which is unintended. Prefer .w or .r for icons. Also, add a semantics label for screen readers.
Apply this diff:
- icon: SvgPicture.asset( - AssetsPaths.icChevronLeft, - width: 24.sp, - height: 24.sp, - colorFilter: ColorFilter.mode( - context.colors.primary, - BlendMode.srcIn, - ), - ), + icon: SvgPicture.asset( + AssetsPaths.icChevronLeft, + width: 24.w, + height: 24.w, + colorFilter: ColorFilter.mode( + context.colors.primary, + BlendMode.srcIn, + ), + semanticsLabel: 'Back', + ),Optional: add a tooltip to IconButton for better a11y on web/desktop:
tooltip: 'Back',lib/ui/contact_list/widgets/user_profile.dart (1)
85-93: Use .w/.r for icon size instead of .sp; add semantics labelIcons should not scale with text size. Also, adding a semantics label improves a11y for the copy action.
Apply this diff:
- icon: SvgPicture.asset( - AssetsPaths.icCopy, - width: 24.sp, - height: 24.sp, - colorFilter: ColorFilter.mode( - context.colors.primary, - BlendMode.srcIn, - ), - ), + icon: SvgPicture.asset( + AssetsPaths.icCopy, + width: 24.w, + height: 24.w, + colorFilter: ColorFilter.mode( + context.colors.primary, + BlendMode.srcIn, + ), + semanticsLabel: 'Copy public key', + ),Optional: add
tooltip: 'Copy public key',to IconButton.lib/ui/contact_list/widgets/profile_ready_card.dart (3)
62-70: Enlarge tap target and add semantics for the dismiss (close) affordanceCurrent GestureDetector wraps a 20x20 icon which is a small hit area. Consider enlarging the tap target and labeling for screen readers.
Apply this diff:
- child: SvgPicture.asset( - AssetsPaths.icClose, - width: 20.w, - height: 20.w, - colorFilter: ColorFilter.mode( - context.colors.mutedForeground, - BlendMode.srcIn, - ), - ), + child: SizedBox( + width: 48.w, + height: 48.w, + child: Center( + child: SvgPicture.asset( + AssetsPaths.icClose, + width: 20.w, + height: 20.w, + colorFilter: ColorFilter.mode( + context.colors.mutedForeground, + BlendMode.srcIn, + ), + semanticsLabel: 'Dismiss', + ), + ), + ),Alternatively, switch to an IconButton for built-in semantics and splash.
103-111: Mark decorative icon as excluded from semanticsThere is adjacent text; excluding the icon avoids double announcements by screen readers.
Apply this diff:
SvgPicture.asset( AssetsPaths.icQrCode, width: 16.w, height: 16.w, colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), + excludeFromSemantics: true, ),
134-142: Mark decorative icon as excluded from semanticsSame rationale: text already conveys meaning; keep a11y concise.
Apply this diff:
SvgPicture.asset( AssetsPaths.icUserFollow, width: 16.w, height: 16.w, colorFilter: ColorFilter.mode( context.colors.primaryForeground, BlendMode.srcIn, ), + excludeFromSemantics: true, ),lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (1)
122-130: Add semantics label (and optional tooltip) to close buttonImprove a11y by labeling the close icon. Tooltips help on web/desktop.
Apply this diff:
IconButton( - icon: SvgPicture.asset( + icon: SvgPicture.asset( AssetsPaths.icClose, width: 24.w, height: 24.w, colorFilter: ColorFilter.mode( context.colors.mutedForeground, BlendMode.srcIn, ), + semanticsLabel: 'Close', ), onPressed: () => Navigator.pop(context), ),Optional: add
tooltip: 'Close',to the IconButton.lib/ui/contact_list/widgets/contact_list_tile.dart (1)
180-188: Add accessibility label to the trash iconExpose a semanticsLabel so TalkBack/VoiceOver can announce the action when swiping.
Apply this diff:
child: SvgPicture.asset( AssetsPaths.icTrashCan, colorFilter: const ColorFilter.mode( Colors.white, BlendMode.srcIn, ), width: 24.w, height: 24.w, + semanticsLabel: 'Delete', ),lib/ui/contact_list/chat_list_screen.dart (2)
360-364: Tint search icon to match theme colorsThe previous Icon inherited color from the theme. Without a colorFilter, the SVG may not adapt to light/dark themes. Apply a colorFilter consistent with your input styling.
Apply this diff:
- prefixIcon: SvgPicture.asset( - AssetsPaths.icSearch, - width: 24.w, - height: 24.w, - ), + prefixIcon: SvgPicture.asset( + AssetsPaths.icSearch, + width: 24.w, + height: 24.w, + colorFilter: ColorFilter.mode( + context.colors.mutedForeground, + BlendMode.srcIn, + ), + ),
373-377: Also tint the clear (close) iconEnsure consistent theming for the suffixIcon.
Apply this diff:
- child: SvgPicture.asset( - AssetsPaths.icClose, - width: 24.w, - height: 24.w, - ), + child: SvgPicture.asset( + AssetsPaths.icClose, + width: 24.w, + height: 24.w, + colorFilter: ColorFilter.mode( + context.colors.mutedForeground, + BlendMode.srcIn, + ), + ),lib/ui/settings/network/widgets/relay_expansion_tile.dart (3)
99-107: Use width/height scaling (.w/.h) instead of .sp for icons.sp is intended for text. For icon dimensions, prefer .w/.h to keep responsiveness consistent with other widgets.
Apply this diff:
- child: SvgPicture.asset( + child: SvgPicture.asset( AssetsPaths.icHelp, colorFilter: ColorFilter.mode( context.colors.mutedForeground, BlendMode.srcIn, ), - width: 18.sp, - height: 18.sp, + width: 18.w, + height: 18.w, ),
99-107: Increase tap target for accessibilityWrap the icon with padding to approach the 48dp recommended minimum tap area.
Apply this diff:
- child: SvgPicture.asset( - AssetsPaths.icHelp, - colorFilter: ColorFilter.mode( - context.colors.mutedForeground, - BlendMode.srcIn, - ), - width: 18.sp, - height: 18.sp, - ), + child: Padding( + padding: EdgeInsets.all(8.w), + child: SvgPicture.asset( + AssetsPaths.icHelp, + colorFilter: ColorFilter.mode( + context.colors.mutedForeground, + BlendMode.srcIn, + ), + width: 18.w, + height: 18.w, + semanticsLabel: 'About this section', + ), + ),
112-120: Align add icon sizing with .w/.h and improve tap targetSame rationale as the help icon—use .w and provide padding for better usability.
Apply this diff:
- child: SvgPicture.asset( - AssetsPaths.icAdd, - colorFilter: ColorFilter.mode( - context.colors.primary, - BlendMode.srcIn, - ), - width: 23.sp, - height: 23.sp, - ), + child: Padding( + padding: EdgeInsets.all(8.w), + child: SvgPicture.asset( + AssetsPaths.icAdd, + colorFilter: ColorFilter.mode( + context.colors.primary, + BlendMode.srcIn, + ), + width: 23.w, + height: 23.w, + semanticsLabel: 'Add relay', + ), + ),lib/ui/chat/widgets/reaction/reaction_menu_item.dart (1)
6-6: Typo in comment: contsructor → constructorMinor nit to keep comments clean.
Apply this diff:
- // contsructor + // constructorlib/ui/contact_list/new_chat_bottom_sheet.dart (1)
385-393: Prefer .w/.h for icon sizing and confirm color choice
- Use .w/.h instead of .sp for non-text dimensions.
- If the prefix icon should be less prominent than primary, consider mutedForeground for parity with other search fields.
Apply this diff:
- prefixIcon: SvgPicture.asset( - AssetsPaths.icSearch, - colorFilter: ColorFilter.mode( - context.colors.primary, - BlendMode.srcIn, - ), - width: 20.sp, - height: 20.sp, - ), + prefixIcon: SvgPicture.asset( + AssetsPaths.icSearch, + colorFilter: ColorFilter.mode( + context.colors.mutedForeground, + BlendMode.srcIn, + ), + width: 20.w, + height: 20.w, + ),lib/ui/chat/widgets/chat_contact_avatar.dart (1)
102-109: Fallback avatar: good switch to SVG; mark decorative to improve a11y.Since this is a non-interactive, purely decorative fallback, exclude it from semantics to avoid screen readers announcing “image”.
- child: SvgPicture.asset( + child: SvgPicture.asset( AssetsPaths.icUser, width: size * 0.4, height: size * 0.4, + excludeFromSemantics: true, colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), ),lib/ui/settings/network/add_relay_bottom_sheet.dart (1)
214-221: Warning icon: mark decorative to avoid duplicate screen reader output.The adjacent text conveys the error; mark the icon decorative to prevent it from being read twice.
- SvgPicture.asset( + SvgPicture.asset( AssetsPaths.icWarningFilled, colorFilter: ColorFilter.mode( context.colors.destructive, BlendMode.srcIn, ), width: 16.w, height: 16.w, + excludeFromSemantics: true, ),lib/ui/chat/widgets/chat_audio_item.dart (2)
62-73: Add tooltip for play/pause button to improve accessibility.IconButton without a tooltip provides minimal semantics. Adding a tooltip helps screen readers and long-press hints.
child: IconButton( icon: SvgPicture.asset( isThisPlaying ? AssetsPaths.icPauseFilled : AssetsPaths.icPlayFilled, colorFilter: ColorFilter.mode( isMe ? context.colors.primary : context.colors.primaryForeground, BlendMode.srcIn, ), width: 20.sp, height: 20.sp, ), - onPressed: () => notifier.togglePlayback(), + tooltip: isThisPlaying ? 'Pause' : 'Play', + onPressed: () => notifier.togglePlayback(), ),
120-130: Microphone icon: mark decorative to avoid noisy semantics.The icon is purely visual; exclude it from semantics.
- SvgPicture.asset( + SvgPicture.asset( AssetsPaths.icMicrophone, colorFilter: ColorFilter.mode( (isMe ? context.colors.primaryForeground : context.colors.primary).withValues( alpha: 0.7, ), BlendMode.srcIn, ), width: 16.sp, height: 16.sp, + excludeFromSemantics: true, ),lib/ui/chat/widgets/swipe_to_reply_widget.dart (1)
89-96: Reply icon: consider RTL support and mark decorative.
- Add matchTextDirection so arrows mirror in RTL.
- Exclude from semantics to avoid redundant announcements.
Please verify the reply asset points to the correct direction in LTR; with matchTextDirection it will auto-mirror for RTL.
- child: SvgPicture.asset( + child: SvgPicture.asset( AssetsPaths.icReply, colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), width: 16.w, height: 16.w, + matchTextDirection: true, + excludeFromSemantics: true, ),lib/ui/settings/app_settings/app_settings_screen.dart (4)
218-226: Back chevron: add RTL support and semantics label.
- matchTextDirection ensures the chevron mirrors in RTL.
- semanticsLabel improves screen reader guidance. For best results, also consider wrapping the GestureDetector with a Semantics(button: true, label: 'Back') widget in a follow-up.
- child: SvgPicture.asset( + child: SvgPicture.asset( AssetsPaths.icChevronLeft, width: 24.w, height: 24.w, colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), + matchTextDirection: true, + semanticsLabel: 'Back', ),
279-286: Trash icon inside labeled button: exclude from semantics.The button already has a clear text label; skip announcing the icon.
- SvgPicture.asset( + SvgPicture.asset( AssetsPaths.icTrashCan, width: 20.w, height: 20.w, colorFilter: ColorFilter.mode( context.colors.solidNeutralWhite, BlendMode.srcIn, ), + excludeFromSemantics: true, ),
364-371: Theme dropdown chevron: decorative icon should be excluded from semantics.Prevents screen readers from redundantly announcing non-informative images.
- SvgPicture.asset( + SvgPicture.asset( isExpanded ? AssetsPaths.icChevronUp : AssetsPaths.icChevronDown, width: 20.w, height: 20.w, colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), + excludeFromSemantics: true, ),
36-36: Typo in dialog title.“Delete app app data” has a duplicated word.
- title: 'Delete app app data', + title: 'Delete app data',lib/ui/settings/general_settings_screen.dart (4)
290-301: Add tooltip and semantics label to the back button for accessibilityIconButton doesn’t automatically convey a textual label when the icon is a custom widget. Provide a tooltip and semanticsLabel so screen readers announce “Back”.
Apply this diff:
IconButton( onPressed: () => context.pop(), + tooltip: 'Back', icon: SvgPicture.asset( AssetsPaths.icChevronLeft, width: 24.w, height: 24.w, + semanticsLabel: 'Back', colorFilter: ColorFilter.mode( context.colors.primarySolid, BlendMode.srcIn, ), ), ),
327-335: Mark decorative QR icon as excluded from semanticsThis trailing icon is decorative; exclude it from semantics to avoid noisy announcements.
SvgPicture.asset( AssetsPaths.icQrCode, width: 20.w, height: 20.w, + excludeFromSemantics: true, colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), ),
357-365: Exclude decorative “switch” icon from semanticsAvoid redundant semantic output; the button’s text already conveys purpose.
SvgPicture.asset( AssetsPaths.icArrowsVertical, width: 16.w, height: 16.w, + excludeFromSemantics: true, colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), ),
464-473: Exclude decorative leading icons from semantics in SettingsListTileThe row’s text represents the actionable label; exclude the icon from semantics.
SvgPicture.asset( assetPath, width: 24.w, height: 24.w, + excludeFromSemantics: true, colorFilter: ColorFilter.mode( foregroundColor ?? context.colors.primary, BlendMode.srcIn, ), ),Additionally, consider replacing GestureDetector with InkWell to get focus/hover/ink ripple and better built-in semantics.
Outside the selected lines, change the tappable wrapper:
// Before return GestureDetector( onTap: onTap, child: Padding( ... ), ); // After return InkWell( onTap: onTap, child: Padding( ... ), );lib/ui/core/ui/wn_bottom_sheet.dart (2)
128-136: Add semantics label and tooltip to back buttonImprove accessibility with a descriptive label and tooltip.
IconButton( onPressed: () => Navigator.pop(context), + tooltip: 'Back', icon: SvgPicture.asset( AssetsPaths.icChevronLeft, + semanticsLabel: 'Back', colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), width: 24.w, height: 24.w, ), ),
156-166: Use IconButton for the close action with tooltip and semanticsGestureDetector lacks semantics and focus handling. IconButton improves accessibility.
- if (showCloseButton) - GestureDetector( - onTap: () => Navigator.pop(context), - child: SvgPicture.asset( - AssetsPaths.icClose, - colorFilter: ColorFilter.mode( - context.colors.primary, - BlendMode.srcIn, - ), - width: 24.w, - height: 24.w, - ), - ), + if (showCloseButton) + IconButton( + onPressed: () => Navigator.pop(context), + tooltip: 'Close', + icon: SvgPicture.asset( + AssetsPaths.icClose, + semanticsLabel: 'Close', + colorFilter: ColorFilter.mode( + context.colors.primary, + BlendMode.srcIn, + ), + width: 24.w, + height: 24.w, + ), + ),test/ui/core/ui/wn_callout_test.dart (1)
53-60: Avoid brittle assertions on SvgPicture.bytesLoader.toString()Comparing toString() is fragile. Prefer a semantics-based assertion that doesn’t rely on internal loader details. Pair this with a semanticsLabel on the icon.
Apply this diff to the test:
- expect( - find.byWidgetPredicate( - (widget) => - widget is SvgPicture && - widget.bytesLoader.toString().contains('assets/svgs/ic_info_filled.svg'), - ), - findsOneWidget, - ); + expect(find.bySemanticsLabel('Information'), findsOneWidget);And in WnCallout (see separate comment), add semanticsLabel: 'Information' to the icon.
lib/ui/settings/network/widgets/network_section.dart (1)
179-187: Exclude tiny status dot from semanticsThe 8px status glyph is purely visual; exclude it from semantics to reduce verbosity.
SvgPicture.asset( relay.status.getIconAsset(), width: 8.w, height: 8.w, + excludeFromSemantics: true, colorFilter: ColorFilter.mode( relay.status.getColor(context), BlendMode.srcIn, ), ),lib/ui/core/ui/wn_callout.dart (1)
29-36: Add semantics label to the information icon for testability and accessibilityThis enables robust tests and improves screen reader support.
SvgPicture.asset( AssetsPaths.icInformation, + semanticsLabel: 'Information', colorFilter: ColorFilter.mode( context.colors.primary, BlendMode.srcIn, ), width: 18.w, height: 18.w, ),lib/models/relay_status.dart (1)
73-88: Normalize asset constant naming (avoid mixing “...Svg” suffix with others)
icCheckmarkFilledSvgincludes a “Svg” suffix, while others likeicInProgress,icIconsTime, etc., do not. Consider standardizing naming inAssetsPaths(e.g., all without file-format suffix) to improve consistency and readability.lib/ui/chat/widgets/reaction/reaction_default_data.dart (1)
22-41: Localize user-facing labelsLabels like 'Reply', 'Copy', 'Edit', and 'Delete' are user-visible. Consider sourcing them from AppLocalizations instead of hard-coding to keep UI translatable. Since
DefaultDatais const, one approach is to build these lists whereBuildContextis available or provide a factory that returns localized items.If helpful, I can draft a small helper (e.g.,
ReactionMenuDefaults.of(context)) that returns the localized lists.lib/ui/chat/widgets/reaction/reactions_dialog_widget.dart (1)
136-146: Use .w/.h instead of .sp for SVG sizesUsing
spties icon size to text scaling. For icons, preferw/hto stay consistent with layout-based scaling.Apply this diff:
- child: SvgPicture.asset( - widget.menuItems[index].assetPath, - width: 20.sp, - height: 20.sp, + child: SvgPicture.asset( + widget.menuItems[index].assetPath, + width: 20.w, + height: 20.w, colorFilter: ColorFilter.mode( widget.menuItems[index].isDestructive ? context.colors.destructive : context.colors.primary, BlendMode.srcIn, ), ),lib/ui/settings/network/widgets/relay_tile.dart (1)
139-141: Prefer layout units for SVG icon sizingUse
w/hinstead ofspso icon dimensions don’t track text scaling.Apply this diff:
- width: 23.sp, - height: 23.sp, + width: 23.w, + height: 23.w,lib/ui/core/ui/wn_heads_up.dart (1)
39-47: Optional: Introduce a reusable WnSvgIcon to reduce duplicationThe same “tinted SVG with width/height” pattern is repeated across the app. A tiny wrapper improves consistency and reduces boilerplate.
Apply this local change:
- SvgPicture.asset( - iconAsset ?? type.iconAsset, - width: 24.w, - height: 24.w, - colorFilter: ColorFilter.mode( - color, - BlendMode.srcIn, - ), - ), + WnSvgIcon( + path: iconAsset ?? type.iconAsset, + size: 24.w, + color: color, + ),And add a small helper widget somewhere common (e.g., lib/ui/core/ui/wn_svg_icon.dart):
import 'package:flutter/material.dart'; import 'package:flutter_screenutil/flutter_screenutil.dart'; import 'package:flutter_svg/flutter_svg.dart'; class WnSvgIcon extends StatelessWidget { const WnSvgIcon({ super.key, required this.path, this.size, this.width, this.height, this.color, this.semanticLabel, }) : assert(size != null || (width != null && height != null)); final String path; final double? size; final double? width; final double? height; final Color? color; final String? semanticLabel; @override Widget build(BuildContext context) { final double w = size ?? width!; final double h = size ?? height!; return SvgPicture.asset( path, width: w, height: h, colorFilter: color != null ? ColorFilter.mode(color!, BlendMode.srcIn) : null, semanticsLabel: semanticLabel, ); } }This enables consistent sizing (size/width/height) and color usage across the codebase.
📜 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 (27)
assets/svgs/ic_arrows_vertical.svgis excluded by!**/*.svgassets/svgs/ic_checkmark_filled.svgis excluded by!**/*.svgassets/svgs/ic_chevron_down.svgis excluded by!**/*.svgassets/svgs/ic_chevron_up.svgis excluded by!**/*.svgassets/svgs/ic_close.svgis excluded by!**/*.svgassets/svgs/ic_data_vis3.svgis excluded by!**/*.svgassets/svgs/ic_development.svgis excluded by!**/*.svgassets/svgs/ic_favorite.svgis excluded by!**/*.svgassets/svgs/ic_icons_time.svgis excluded by!**/*.svgassets/svgs/ic_in_progress.svgis excluded by!**/*.svgassets/svgs/ic_information.svgis excluded by!**/*.svgassets/svgs/ic_information_filled.svgis excluded by!**/*.svgassets/svgs/ic_locked.svgis excluded by!**/*.svgassets/svgs/ic_logout.svgis excluded by!**/*.svgassets/svgs/ic_microphone.svgis excluded by!**/*.svgassets/svgs/ic_moon.svgis excluded by!**/*.svgassets/svgs/ic_password.svgis excluded by!**/*.svgassets/svgs/ic_pause_filled.svgis excluded by!**/*.svgassets/svgs/ic_play_filled.svgis excluded by!**/*.svgassets/svgs/ic_radio_button.svgis excluded by!**/*.svgassets/svgs/ic_reply.svgis excluded by!**/*.svgassets/svgs/ic_settings.svgis excluded by!**/*.svgassets/svgs/ic_trash_can.svgis excluded by!**/*.svgassets/svgs/ic_user.svgis excluded by!**/*.svgassets/svgs/ic_user_follow.svgis excluded by!**/*.svgios/Podfile.lockis excluded by!**/*.lockpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
lib/models/relay_status.dart(2 hunks)lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart(1 hunks)lib/ui/chat/chat_management/add_to_group_screen.dart(2 hunks)lib/ui/chat/widgets/chat_audio_item.dart(3 hunks)lib/ui/chat/widgets/chat_contact_avatar.dart(2 hunks)lib/ui/chat/widgets/reaction/reaction_default_data.dart(2 hunks)lib/ui/chat/widgets/reaction/reaction_menu_item.dart(1 hunks)lib/ui/chat/widgets/reaction/reactions_dialog_widget.dart(1 hunks)lib/ui/chat/widgets/swipe_to_reply_widget.dart(2 hunks)lib/ui/contact_list/chat_list_screen.dart(2 hunks)lib/ui/contact_list/new_chat_bottom_sheet.dart(1 hunks)lib/ui/contact_list/widgets/contact_list_tile.dart(1 hunks)lib/ui/contact_list/widgets/profile_ready_card.dart(4 hunks)lib/ui/contact_list/widgets/user_profile.dart(2 hunks)lib/ui/core/themes/assets.dart(2 hunks)lib/ui/core/ui/wn_bottom_sheet.dart(3 hunks)lib/ui/core/ui/wn_callout.dart(2 hunks)lib/ui/core/ui/wn_heads_up.dart(4 hunks)lib/ui/settings/app_settings/app_settings_screen.dart(5 hunks)lib/ui/settings/donate/donate_screen.dart(2 hunks)lib/ui/settings/general_settings_screen.dart(10 hunks)lib/ui/settings/network/add_relay_bottom_sheet.dart(2 hunks)lib/ui/settings/network/widgets/network_section.dart(1 hunks)lib/ui/settings/network/widgets/relay_expansion_tile.dart(2 hunks)lib/ui/settings/network/widgets/relay_tile.dart(4 hunks)lib/ui/settings/nostr_keys/nostr_keys_screen.dart(2 hunks)pubspec.yaml(0 hunks)test/ui/contact_list/share_invite_bottom_sheet_test.dart(2 hunks)test/ui/contact_list/start_chat_bottom_sheet_test.dart(3 hunks)test/ui/contact_list/widgets/user_profile_test.dart(2 hunks)test/ui/core/ui/wn_callout_test.dart(2 hunks)
💤 Files with no reviewable changes (1)
- pubspec.yaml
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/widgets/contact_list_tile.dartlib/ui/chat/widgets/chat_contact_avatar.dartlib/ui/settings/donate/donate_screen.dartlib/ui/settings/network/add_relay_bottom_sheet.darttest/ui/contact_list/share_invite_bottom_sheet_test.dartlib/ui/contact_list/widgets/profile_ready_card.dartlib/ui/settings/nostr_keys/nostr_keys_screen.dartlib/ui/chat/widgets/swipe_to_reply_widget.darttest/ui/contact_list/widgets/user_profile_test.dartlib/ui/settings/network/widgets/relay_expansion_tile.darttest/ui/contact_list/start_chat_bottom_sheet_test.dartlib/ui/contact_list/widgets/user_profile.dartlib/ui/contact_list/new_chat_bottom_sheet.dartlib/ui/contact_list/chat_list_screen.dartlib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/core/ui/wn_callout.dartlib/ui/core/ui/wn_bottom_sheet.dartlib/ui/chat/chat_management/add_to_group_screen.dartlib/ui/chat/widgets/chat_audio_item.dartlib/models/relay_status.dartlib/ui/settings/general_settings_screen.darttest/ui/core/ui/wn_callout_test.dartlib/ui/chat/widgets/reaction/reaction_menu_item.dartlib/ui/core/themes/assets.dartlib/ui/settings/app_settings/app_settings_screen.dartlib/ui/core/ui/wn_heads_up.dartlib/ui/settings/network/widgets/relay_tile.dartlib/ui/settings/network/widgets/network_section.dartlib/ui/chat/widgets/reaction/reactions_dialog_widget.dartlib/ui/chat/widgets/reaction/reaction_default_data.dart
**/*_test.dart
📄 CodeRabbit Inference Engine (.cursor/rules/flutter.mdc)
**/*_test.dart: Follow the Arrange-Act-Assert convention for tests.
Name test variables clearly. Follow the convention: inputX, mockX, actualX, expectedX, etc.
Write unit tests for each public function. Use test doubles to simulate dependencies, except for third-party dependencies that are not expensive to execute.
Write acceptance tests for each module. Follow the Given-When-Then convention.
Use the standard widget testing for Flutter.
Files:
test/ui/contact_list/share_invite_bottom_sheet_test.darttest/ui/contact_list/widgets/user_profile_test.darttest/ui/contact_list/start_chat_bottom_sheet_test.darttest/ui/core/ui/wn_callout_test.dart
🧠 Learnings (6)
📚 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 flutter_rust_bridge to access core functionality of the app.
Applied to files:
lib/ui/contact_list/widgets/contact_list_tile.dartlib/ui/settings/donate/donate_screen.dartlib/ui/contact_list/widgets/profile_ready_card.dartlib/ui/settings/nostr_keys/nostr_keys_screen.dartlib/ui/settings/network/widgets/relay_expansion_tile.dartlib/ui/chat/chat_management/add_to_group_screen.dartlib/ui/settings/general_settings_screen.dartlib/ui/settings/app_settings/app_settings_screen.dartlib/ui/settings/network/widgets/relay_tile.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 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/contact_list_tile.dartlib/ui/chat/chat_management/add_to_group_screen.dartlib/ui/settings/general_settings_screen.dartlib/ui/settings/app_settings/app_settings_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/settings/donate/donate_screen.dartlib/ui/settings/nostr_keys/nostr_keys_screen.dartlib/ui/settings/network/widgets/relay_expansion_tile.dartlib/ui/settings/general_settings_screen.dartlib/ui/settings/app_settings/app_settings_screen.dartlib/ui/settings/network/widgets/relay_tile.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 **/*_test.dart : Use the standard widget testing for Flutter.
Applied to files:
test/ui/contact_list/share_invite_bottom_sheet_test.darttest/ui/contact_list/widgets/user_profile_test.darttest/ui/contact_list/start_chat_bottom_sheet_test.darttest/ui/core/ui/wn_callout_test.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 extensions to manage reusable code.
Applied to files:
lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.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 ThemeData to manage themes.
Applied to files:
lib/ui/settings/app_settings/app_settings_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 (32)
lib/ui/core/themes/assets.dart (1)
33-33: Asset ‘ic_information_filled.svg’ is present and covered by pubspec.yaml
The fileassets/svgs/ic_information_filled.svgexists, and theassets/svgs/directory is declared inpubspec.yaml, so no runtime errors will occur.test/ui/contact_list/share_invite_bottom_sheet_test.dart (1)
2-2: LGTM: flutter_svg import aligns tests with SvgPicture usagelib/ui/settings/donate/donate_screen.dart (1)
5-5: LGTM: flutter_svg import required for SvgPicture.assettest/ui/contact_list/widgets/user_profile_test.dart (1)
3-3: LGTM: flutter_svg import aligns tests with SvgPicture usagetest/ui/contact_list/start_chat_bottom_sheet_test.dart (1)
2-2: LGTM: flutter_svg import aligns tests with SvgPicture usagelib/ui/settings/nostr_keys/nostr_keys_screen.dart (1)
5-5: SVG import addition looks goodImporting flutter_svg aligns with the PR objective and enables asset-based icon rendering.
lib/ui/chat/chat_management/add_to_group_screen.dart (1)
3-3: Imports for SVG assets are appropriateAdding flutter_svg and assets.dart matches the migration away from CarbonIcons.
Also applies to: 9-9
lib/ui/contact_list/widgets/user_profile.dart (1)
4-4: SVG migration imports look goodImporting flutter_svg and assets paths is consistent with the new icon strategy.
Also applies to: 7-7
lib/ui/contact_list/widgets/profile_ready_card.dart (1)
4-4: Imports match the SVG asset migrationBringing in flutter_svg and assets.dart is correct for the new icon approach.
Also applies to: 10-10
lib/ui/contact_list/widgets/contact_list_tile.dart (1)
180-188: SVG migration for swipe-to-delete looks goodUsing SvgPicture with white colorFilter against a red background mirrors the previous Icon behavior and keeps contrast high.
lib/ui/settings/network/widgets/relay_expansion_tile.dart (1)
4-4: Imports for flutter_svg and AssetsPaths are appropriateThese imports align with the SVG asset migration.
Also applies to: 8-8
lib/ui/chat/widgets/reaction/reaction_menu_item.dart (1)
3-3: No remaining MenuItem.icon usages foundA repository-wide search for both
MenuItem(icon: ...)and.iconreferences returned no matches, confirming all call sites have been updated for the constructor change.lib/ui/chat/widgets/chat_contact_avatar.dart (1)
3-4: Imports updated for SVG assets — LGTM.Switch to flutter_svg and AssetsPaths aligns with the repo-wide migration.
lib/ui/settings/network/add_relay_bottom_sheet.dart (1)
8-8: SVG import — LGTM.Consistent with the migration away from CarbonIcons.
lib/ui/chat/widgets/chat_audio_item.dart (1)
4-5: SVG imports — LGTM.Aligns with asset-based icon usage.
lib/ui/chat/widgets/swipe_to_reply_widget.dart (1)
4-4: SVG import — LGTM.Migration step looks correct.
lib/ui/settings/app_settings/app_settings_screen.dart (3)
5-5: SVG import — LGTM.Consistent with SVG assets across the app.
20-20: Assets import — LGTM.Correctly references AssetsPaths for icon assets.
5-5: All CarbonIcons references successfully removedThe repository scan returned no instances of
CarbonIconsorsupa_carbon_icons. No further action is needed.lib/ui/settings/general_settings_screen.dart (3)
4-4: Migration to flutter_svg and centralized AssetsPaths looks goodUsing SvgPicture with centralized AssetsPaths is consistent and maintainable. ColorFilter with BlendMode.srcIn preserves theming correctly.
Also applies to: 20-20
378-399: Settings tiles migration looks consistentAll tiles now pass assetPath and use SvgPicture with colorFilter. Consistent with the app-wide migration.
4-4: No remaining CarbonIcons references found—migration completeRan a repo-wide search for any lingering
CarbonIconsorsupa_carbon_iconsusages and found none. Verified that:
flutter_svg: ^2.1.0is declared inpubspec.yamlassets/svgs/(andassets/pngs/) are registered underassets:All CarbonIcons references have been removed and the SVG assets pipeline is fully configured.
lib/ui/core/ui/wn_bottom_sheet.dart (1)
4-4: Migration to SVG assets is consistentImports updated to flutter_svg and asset paths centralized. Header icon rendering matches the rest of the app.
Also applies to: 7-7
lib/ui/settings/network/widgets/network_section.dart (1)
179-187: Nice switch to asset-based relay status iconUsing getIconAsset() with colorFilter keeps theme colors consistent and removes icon font dependency.
lib/ui/core/ui/wn_callout.dart (1)
3-6: Asset-based icon migration looks correctImports updated to flutter_svg and AssetsPaths; colorization matches the previous Icon usage.
lib/models/relay_status.dart (2)
70-89: API change to asset-path icons looks consistentSwitching from IconData to asset-paths via
getIconAsset()is coherent with the SVG migration. The switch covers all enum cases and keeps color logic separate. LGTM.
70-89: AllgetIcon→getIconAssetusages updated – no residual references foundRan the scripted searches across the repo and confirmed:
- No lingering
getIcon(calls- No
WnStickyHeadsUp(icon: …)parameters- No leftover
CarbonIconsorsupa_carbon_iconsreferences- All asset constants point to existing files
This public‐API rename is fully propagated and safe to merge.
lib/ui/chat/widgets/reaction/reaction_default_data.dart (1)
22-41: LGTM: MenuItem migration to assetPathReplacing IconData with
assetPathacross the default items is clean and matches the new rendering path.lib/ui/settings/network/widgets/relay_tile.dart (2)
56-64: LGTM: Dialog close icon migrated to SVGThe new
SvgPicture.asset(AssetsPaths.icClose)with colorFilter and sizing is consistent with the migration.
114-122: LGTM: Status icon now uses asset path + colorUsing
status.getIconAsset()withcolorFilterpreserves the previous color semantics while aligning with SVG assets.lib/ui/core/ui/wn_heads_up.dart (2)
39-47: LGTM: Switched to SvgPicture with tintRendering via
SvgPicture.asset(iconAsset ?? type.iconAsset, ...)withColorFilter.mode(..., BlendMode.srcIn)is consistent with the new asset approach.
13-19: All usages of the oldiconAPI have been updated
No instances of the oldWnStickyHeadsUp(icon: …)parameter orWnHeadingType.icongetter remain in thelibdirectory. The breaking change toiconAssetis fully applied.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
Great job! 🚀 🎉 Thank you for doing this one!! Reviewed it locally and overall it is working really well!
💡 I think that the idea mentioned in the signal chat of having an icon component is great, but definitely would leave that for other issue. It could be mayb e a subissue of the issue you are solving now (and someone else can take it, I am not saying thise for you to take it if you maybe need/have/want to do something else now). If you are ok with this just let me know and I can create the sub-issue.
Last but not least, I caught a little bug in some sizes and colors, left screenshots of the problems I found.
There was a problem hiding this comment.
By reviewing this code, I notice that this file it is used in lots of places as ContactAvatar, I think that maybe (for another issue) we should move this one to the core/ui folder and add the Wn prefix to it. WDYT?
| import 'package:whitenoise/ui/core/themes/assets.dart'; | ||
| import 'package:whitenoise/ui/core/themes/src/app_theme.dart'; | ||
|
|
||
| class WnStickyHeadsUp extends StatelessWidget { |
There was a problem hiding this comment.
Oopps probably my bad here when adding Wn prefix to existing components, but this file name does not match the widget name. Can be fixed in other issue, just commenting it here to not forget to fix it.
|
You can create a sub-issue and assign me. And let me fix the bugs that you found so we can merge this first. |
|
@josefinalliende I fixed the sizing/coloring bugs and removed audio-related files. 
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/ui/settings/network/network_screen.dart (1)
156-157: Add semantics label (and optional tooltip) for accessibility.Screen readers will announce a meaningful label. Minor but worthwhile.
Apply this diff within the SvgPicture.asset to add a semantics label:
- width: 24.w, - height: 24.w, + semanticsLabel: 'Back', + width: 24.w, + height: 24.w,Optionally, add a tooltip to IconButton (outside the selected lines) for web/desktop hover:
IconButton( tooltip: 'Back', onPressed: () => Navigator.of(context).pop(), icon: SvgPicture.asset( // ... ), )
📜 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 (3)
lib/ui/contact_list/chat_list_screen.dart(2 hunks)lib/ui/contact_list/new_chat_bottom_sheet.dart(1 hunks)lib/ui/settings/network/network_screen.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/ui/contact_list/new_chat_bottom_sheet.dart
- lib/ui/contact_list/chat_list_screen.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/settings/network/network_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 (1)
lib/ui/settings/network/network_screen.dart (1)
156-157: Explicit SVG sizing is correct and consistent with the migration. LGTM.Adding width and height avoids relying on IconButton.iconSize with a custom child and keeps layout stable. This aligns with the PR’s SVG migration pattern.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
Amaziiing!! 🎉 ✅ 🎉 Tried it locally again and it looks perfect!
Description
Removes the CarbonIcons package and implements the direct use of SVG files from Figma for all icons. This change streamlines icon management and improves the workflow between design and development.
Type of Change
Checklist
just precommitto ensure that formatting and linting are correctCHANGELOG.mdfile with your changesFixes #474
Summary by CodeRabbit
Style
Refactor
Tests
Chores