Replace SvgPicture with unified WnImage Component #568
Conversation
…-chat-content-on-keyboard-open' of github.com:parres-hq/whitenoise_flutter into 498-push-chat-content-on-keyboard-open
|
Caution Review failedThe pull request is closed. WalkthroughReplaced direct SVG/asset image renderings with a unified WnImage across UI components; extended WnImage to handle local files and a configurable fallbackWidget; refactored WnAvatar to use WnImage and added a public FallbackAvatar; removed flutter_svg imports and updated tests and minor mock signatures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI
participant WnImage
participant Net as NetworkLoader
participant File as LocalFileLoader
participant Asset as AssetLoader
participant Fallback
UI->>WnImage: build(src, size, color, fallbackWidget)
alt src is network
WnImage->>Net: request (SVG or raster)
alt success
Net-->>UI: rendered image
else failure
WnImage->>Fallback: _buildFallback(ctx)
Fallback-->>UI: fallback widget
end
else src is local file
WnImage->>File: load (SVG or raster)
alt success
File-->>UI: rendered image
else failure
WnImage->>Fallback: _buildFallback(ctx)
Fallback-->>UI: fallback widget
end
else treat as asset
WnImage->>Asset: load (SVG or raster)
alt success
Asset-->>UI: rendered image
else failure
WnImage->>Fallback: _buildFallback(ctx)
Fallback-->>UI: fallback widget
end
end
sequenceDiagram
autonumber
actor UI
participant WnAvatar
participant WnImage
participant FallbackAvatar
UI->>WnAvatar: build(displayName, size, src)
WnAvatar->>WnImage: build(src, size, fallbackWidget: (ctx)=>FallbackAvatar)
alt image loads
WnImage-->>UI: image widget
else error/missing
WnImage->>FallbackAvatar: instantiate(displayName, size)
FallbackAvatar-->>UI: rendered fallback avatar
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
lib/ui/chat/chat_info/dm_chat_info.dart (1)
216-223: WnImage migration looks good; prefersizefor square icons.Using only
widthcan lead to inconsistent aspect ifheightdefaults differently. Align with other files by usingsize: 14.w.- suffixIcon: WnImage( - isFollow ? AssetsPaths.icRemoveUser : AssetsPaths.icAddUser, - width: 14.w, - color: + suffixIcon: WnImage( + isFollow ? AssetsPaths.icRemoveUser : AssetsPaths.icAddUser, + size: 14.w, + color: isFollow ? context.colors.secondaryForeground : context.colors.primaryForeground, ),lib/ui/chat/widgets/chat_input.dart (1)
259-264: Good swap to WnImage; collapse width/height intosize.Reduces duplication and keeps icons square.
- child: WnImage( - AssetsPaths.icClose, - width: 16.w, - height: 16.w, - color: context.colors.mutedForeground, - ), + child: WnImage( + AssetsPaths.icClose, + size: 16.w, + color: context.colors.mutedForeground, + ),lib/ui/contact_list/start_chat_bottom_sheet.dart (1)
269-273: Icon color mismatches secondary buttons; also avoid repeated_isFollow()calls.
- For
WnButtonVisualState.secondary, usesecondaryForeground(matches dm_chat_info.dart and design tokens).- Compute
isFollowedonce to avoid multiple provider reads in the same build.- label: _isFollow() ? 'Remove Contact' : 'Add Contact', + label: _isFollow() ? 'Remove Contact' : 'Add Contact', suffixIcon: WnImage( _isFollow() ? AssetsPaths.icRemoveUser : AssetsPaths.icAddUser, - size: 18.w, - color: context.colors.primary, + size: 18.w, + color: context.colors.secondaryForeground, ),- suffixIcon: WnImage( - AssetsPaths.icChatInvite, - size: 18.w, - color: context.colors.primary, - ), + suffixIcon: WnImage( + AssetsPaths.icChatInvite, + size: 18.w, + color: context.colors.secondaryForeground, + ),Outside the shown hunk, just before building the buttons block:
// Add near the top of the "buttons" Column build scope final bool isFollowed = _isFollow();Then reuse
isFollowedfor label and icon path:label: isFollowed ? 'Remove Contact' : 'Add Contact', suffixIcon: WnImage(isFollowed ? AssetsPaths.icRemoveUser : AssetsPaths.icAddUser, ...)Also applies to: 281-284
lib/ui/core/ui/wn_icon_button.dart (1)
37-41: Migration to WnImage LGTM; consider scaling icon size with button size.Right now the icon is fixed at 16.w. Optionally derive it from the button’s
size/paddingso the visual ratio stays consistent.Example:
final double containerSize = size ?? 40.h; final double pad = padding ?? 12.w; final double iconSize = (containerSize - 2 * pad).clamp(12.w, 24.w); WnImage(iconPath, size: iconSize, color: iconColor ?? context.colors.primary)lib/ui/chat/widgets/chat_search_widget.dart (1)
77-83: WnImage replacements look solid; minor consistency nits.
- Prefer
size:over separate width/height.- Optional: add
tooltipto IconButtons for a11y (e.g., “Previous match”, “Next match”, “Clear search”).- child: WnImage( - AssetsPaths.icSearch, - width: 20.w, - height: 20.w, - color: context.colors.mutedForeground, - ), + child: WnImage( + AssetsPaths.icSearch, + size: 20.w, + color: context.colors.mutedForeground, + ),- icon: WnImage( - AssetsPaths.icClose, - width: 20.w, - height: 20.w, - color: context.colors.mutedForeground, - ), + icon: WnImage( + AssetsPaths.icClose, + size: 20.w, + color: context.colors.mutedForeground, + ),- icon: WnImage( - AssetsPaths.icChevronUp, - height: 16.w, - width: 16.w, - color: context.colors.primarySolid, - ), + icon: WnImage( + AssetsPaths.icChevronUp, + size: 16.w, + color: context.colors.primarySolid, + ),- icon: WnImage( - AssetsPaths.icChevronDown, - height: 16.w, - width: 16.w, - color: context.colors.primarySolid, - ), + icon: WnImage( + AssetsPaths.icChevronDown, + size: 16.w, + color: context.colors.primarySolid, + ),Also applies to: 85-91, 131-136, 165-169
lib/ui/core/ui/wn_avatar.dart (1)
64-93: Unicode-safe initial + replace magic numbers with named constants.substring(0, 1) can split grapheme clusters (e.g., emoji, accented chars). Use Characters.first. Also replace 0.4/0.25 with named constants for clarity.
@override Widget build(BuildContext context) { - // Show first letter of displayName if available + // Show first letter of displayName if available if (displayName != null && displayName!.trim().isNotEmpty) { return Center( child: Text( - displayName!.trim().substring(0, 1).toUpperCase(), + displayName!.trim().characters.first.toUpperCase(), style: TextStyle( - fontSize: size * 0.4, + fontSize: size * _fontScale, fontWeight: FontWeight.bold, color: context.colors.primary, ), ), ); } // Final fallback: Default avatar asset with proper padding return Padding( - padding: EdgeInsets.all(size * 0.25), + padding: EdgeInsets.all(size * _paddingScale), child: Center( child: WnImage( AssetsPaths.icUser, - width: size * 0.4, - height: size * 0.4, + width: size * _iconScale, + height: size * _iconScale, color: context.colors.primary, ), ), ); }Add these inside FallbackAvatar (top of class body):
// Requires: import 'package:characters/characters.dart'; static const double _fontScale = 0.4; static const double _iconScale = 0.4; static const double _paddingScale = 0.25;And add at file imports:
import 'package:characters/characters.dart';lib/ui/core/ui/wn_image.dart (1)
67-70: Avoid synchronous I/O in build paths when possible.existsSync() is called during build; consider deferring to the rendering attempt and letting errorBuilder handle failures, or cache the existence check externally if this rebuilds frequently.
Example direction:
- Treat non-network sources as asset first, and if that fails, retry as file (may require a small loader widget to avoid repeat attempts), or
- Accept an explicit “isLocalFile” hint from call sites that know the source.
Also applies to: 123-149
📜 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 (8)
lib/ui/chat/chat_info/chat_info_screen.dart(0 hunks)lib/ui/chat/chat_info/dm_chat_info.dart(1 hunks)lib/ui/chat/widgets/chat_input.dart(2 hunks)lib/ui/chat/widgets/chat_search_widget.dart(4 hunks)lib/ui/contact_list/start_chat_bottom_sheet.dart(1 hunks)lib/ui/core/ui/wn_avatar.dart(1 hunks)lib/ui/core/ui/wn_icon_button.dart(2 hunks)lib/ui/core/ui/wn_image.dart(11 hunks)
💤 Files with no reviewable changes (1)
- lib/ui/chat/chat_info/chat_info_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/chat/chat_info/dm_chat_info.dartlib/ui/core/ui/wn_icon_button.dartlib/ui/chat/widgets/chat_input.dartlib/ui/chat/widgets/chat_search_widget.dartlib/ui/contact_list/start_chat_bottom_sheet.dartlib/ui/core/ui/wn_image.dartlib/ui/core/ui/wn_avatar.dart
🧠 Learnings (7)
📚 Learning: 2025-08-23T11:02:28.308Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#527
File: lib/ui/core/ui/wn_avatar.dart:1-6
Timestamp: 2025-08-23T11:02:28.308Z
Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.
Applied to files:
lib/ui/core/ui/wn_icon_button.dartlib/ui/chat/widgets/chat_input.dartlib/ui/chat/widgets/chat_search_widget.dartlib/ui/contact_list/start_chat_bottom_sheet.dartlib/ui/core/ui/wn_avatar.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/start_chat_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 flutter_rust_bridge to access core functionality of the app.
Applied to files:
lib/ui/contact_list/start_chat_bottom_sheet.dartlib/ui/core/ui/wn_avatar.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/core/ui/wn_avatar.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/core/ui/wn_avatar.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 freezed to manage UI states.
Applied to files:
lib/ui/core/ui/wn_avatar.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:
lib/ui/core/ui/wn_avatar.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 (3)
lib/ui/core/ui/wn_image.dart (3)
13-16: Docs updated — good clarity on local-file support.
32-33: Unified fallback flow looks solid.Adding fallbackWidget with a sensible precedence (fallback -> errorWidget -> default) is a clean extension.
Also applies to: 57-58, 216-227
89-90: Error propagation to fallback is consistent across renderers — nice.Routing all errors to _buildFallback keeps behavior predictable.
Also applies to: 106-107, 119-120, 151-176
| suffixIcon: WnImage( | ||
| _isFollow() ? AssetsPaths.icRemoveUser : AssetsPaths.icAddUser, | ||
| width: 18.w, | ||
| height: 18.w, | ||
| colorFilter: ColorFilter.mode( | ||
| context.colors.primary, | ||
| BlendMode.srcIn, | ||
| ), | ||
| size: 18.w, | ||
| color: context.colors.primary, | ||
| ), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Repo-wide check for leftover SvgPicture/Image usages.
Run this to confirm objective #567 is fully met and that flutter_svg imports are gone.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for leftover SvgPicture usages..."
rg -nP --type=dart '\bSvgPicture\b|flutter_svg' || echo "No SvgPicture references found."
echo "Searching for direct Image constructors likely needing WnImage..."
rg -nP --type=dart '\bImage\.(asset|network|file)\s*\(' || echo "No direct Image.asset/network/file references found."
echo "Scanning for legacy colorFilter on icons (post-migration smell)..."
rg -nP --type=dart 'colorFilter\s*:' || echo "No colorFilter occurrences found."
echo "Done."Length of output: 2780
Remove leftover SvgPicture imports and test assertions
- In tests (e.g. test/ui/contact_list/widgets/user_profile_test.dart, start_chat_bottom_sheet_test.dart, share_invite_bottom_sheet_test.dart, ui/core/ui/wn_callout_test.dart): replace
with assertions against WnImage.
import 'package:flutter_svg/flutter_svg.dart'; find.byType(SvgPicture)
- In lib/ui/chat/chat_info/group_chat_info.dart (lines 164–179): delete the commented-out
SvgPicture.assetblocks. - Direct
Image.asset/Image.filecalls within lib/ui/core/ui/wn_image.dart are internal and may remain.
🤖 Prompt for AI Agents
In lib/ui/contact_list/start_chat_bottom_sheet.dart around lines 269–273, there
is usage of WnImage (and elsewhere leftover SvgPicture references/tests and
commented blocks); remove any unused flutter_svg imports and update tests to
assert against WnImage instead of SvgPicture (replace import
'package:flutter_svg/flutter_svg.dart' and find.byType(SvgPicture) with WnImage
assertions in test/ui/contact_list/widgets/user_profile_test.dart,
start_chat_bottom_sheet_test.dart, share_invite_bottom_sheet_test.dart,
ui/core/ui/wn_callout_test.dart), delete the commented-out SvgPicture.asset
blocks in lib/ui/chat/chat_info/group_chat_info.dart (lines 164–179), and leave
direct Image.asset/Image.file calls inside lib/ui/core/ui/wn_image.dart as-is;
ensure all references and imports to SvgPicture are removed project-wide.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
I tried it locally and changed svgs look fine! ✅
👀 Code rabbit made some context about shadowing context and passing context to another widget that might make sense, also I left a question related to that
|
My bad about the context @josefinalliende. I extracted the build method widget to a class widget using my IDE shortcut and forgot to remove the original BuildContext that was passed to it. AFK now, I'll fix that in like 30mins when I get back to my machine. Thanks for catching this. |
No hurry! Just ping me when you’d like me to review again. Tomorrow is fine too , not sure what time it is for you, probably late haha. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/ui/core/ui/wn_avatar.dart (1)
44-44: Avoid shadowing the surrounding BuildContext in the callback.Rename the callback param to reduce confusion with the
buildmethod’scontext.- fallbackWidget: (context) => FallbackAvatar(displayName: displayName, size: size), + fallbackWidget: (ctx) => FallbackAvatar(displayName: displayName, size: size),
🧹 Nitpick comments (1)
lib/ui/core/ui/wn_avatar.dart (1)
61-89: Use grapheme-safe first letter and avoid double trim.Handle multi-codepoint user initials (emoji, combined marks) and compute
trimonce.- // Show first letter of displayName if available - if (displayName != null && displayName!.trim().isNotEmpty) { - return Center( - child: Text( - displayName!.trim().substring(0, 1).toUpperCase(), + // Show first letter of displayName if available + final String trimmed = displayName?.trim() ?? ''; + if (trimmed.isNotEmpty) { + final String firstGrapheme = trimmed.characters.first.toUpperCase(); + return Center( + child: Text( + firstGrapheme, style: TextStyle( fontSize: size * 0.4, fontWeight: FontWeight.bold, color: context.colors.primary, ), ), ); }Add import (outside this hunk):
import 'package:characters/characters.dart';
📜 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/chat/widgets/chat_input.dart(2 hunks)lib/ui/chat/widgets/chat_search_widget.dart(4 hunks)lib/ui/core/ui/wn_avatar.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/ui/chat/widgets/chat_search_widget.dart
- lib/ui/chat/widgets/chat_input.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/core/ui/wn_avatar.dart
🧠 Learnings (8)
📚 Learning: 2025-08-23T11:02:28.308Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#527
File: lib/ui/core/ui/wn_avatar.dart:1-6
Timestamp: 2025-08-23T11:02:28.308Z
Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.
Applied to files:
lib/ui/core/ui/wn_avatar.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/core/ui/wn_avatar.dart
📚 Learning: 2025-09-01T14:56:50.988Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#568
File: lib/ui/core/ui/wn_image.dart:1-2
Timestamp: 2025-09-01T14:56:50.988Z
Learning: The whitenoise_flutter project does not target web platforms and any future web version would be in a separate repository, so dart:io imports and file system operations are acceptable in this codebase.
Applied to files:
lib/ui/core/ui/wn_avatar.dart
📚 Learning: 2025-09-01T14:56:47.293Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#568
File: lib/ui/core/ui/wn_image.dart:1-2
Timestamp: 2025-09-01T14:56:47.293Z
Learning: The whitenoise_flutter project does not target web platforms - any web version would be in a separate repository. Therefore, dart:io usage and web compatibility concerns can be ignored in this codebase.
Applied to files:
lib/ui/core/ui/wn_avatar.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/core/ui/wn_avatar.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 freezed to manage UI states.
Applied to files:
lib/ui/core/ui/wn_avatar.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 flutter_rust_bridge to access core functionality of the app.
Applied to files:
lib/ui/core/ui/wn_avatar.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:
lib/ui/core/ui/wn_avatar.dart
🔇 Additional comments (3)
lib/ui/core/ui/wn_avatar.dart (3)
40-45: Migration to WnImage looks solid.Unified loading + fallback via
fallbackWidgetreads clean and aligns with the PR goal.
51-56: Nice extraction and public API.
FallbackAvatarisconst,StatelessWidget, and fields are typed/final. Good.
77-86: No change needed—AssetsPaths.icUseris an SVG, so thecolor:tint will apply correctly.
|
Do Rabbit Change request. |
Quwaysim
dismissed stale reviews from codeswot and josefinalliende
via
3930790
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/ui/contact_list/share_invite_bottom_sheet_test.dart (1)
70-71: Use endsWith for a stricter, less flaky match.
Using contains can yield false positives if the path changes or is prefixed; endsWith asserts the exact asset target.- widget is WnImage && - widget.src.contains('assets/svgs/ic_copy.svg'), + widget is WnImage && + widget.src.endsWith('assets/svgs/ic_copy.svg'),test/ui/core/ui/wn_callout_test.dart (1)
56-57: Prefer endsWith over contains for exact asset verification.
Reduces false positives and keeps the assertion resilient to path prefixes.- widget is WnImage && - widget.src.contains('assets/svgs/ic_information.svg'), + widget is WnImage && + widget.src.endsWith('assets/svgs/ic_information.svg'),
📜 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 (2)
test/ui/contact_list/share_invite_bottom_sheet_test.dart(2 hunks)test/ui/core/ui/wn_callout_test.dart(2 hunks)
🧰 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:
test/ui/contact_list/share_invite_bottom_sheet_test.darttest/ui/core/ui/wn_callout_test.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/core/ui/wn_callout_test.dart
🧠 Learnings (3)
📚 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/core/ui/wn_callout_test.dart
📚 Learning: 2025-08-23T11:02:28.308Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#527
File: lib/ui/core/ui/wn_avatar.dart:1-6
Timestamp: 2025-08-23T11:02:28.308Z
Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.
Applied to files:
test/ui/contact_list/share_invite_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 **/*_test.dart : Write unit tests for each public function. Use test doubles to simulate dependencies, except for third-party dependencies that are not expensive to execute.
Applied to files:
test/ui/core/ui/wn_callout_test.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)
test/ui/contact_list/share_invite_bottom_sheet_test.dart (1)
5-5: Remove lingering SvgPicture.asset references
Remove the commented-outSvgPicture.assetusages inlib/ui/chat/chat_info/group_chat_info.dart(lines 164 and 176). Ensure allpackage:flutter_svg/flutter_svg.dartimports are removed.⛔ Skipped due to learnings
Learnt from: Quwaysim PR: parres-hq/whitenoise_flutter#527 File: lib/ui/core/ui/wn_avatar.dart:1-6 Timestamp: 2025-08-23T11:02:28.308Z Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.test/ui/core/ui/wn_callout_test.dart (1)
5-5: Import switch to WnImage matches the component migration.
…h WnImage in tests
…with-wnImage-class
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
test/ui/contact_list/start_chat_bottom_sheet_test.dart (5)
137-141: Target the copy icon deterministically instead of byType + first.
Use WnImage.src (or a Key) to avoid tapping the wrong image if more WnImage widgets appear.Apply:
- final copyButton = find.byType(WnImage); - expect(copyButton, findsWidgets); - await tester.tap(copyButton.first); + final copyButton = find.byWidgetPredicate( + (w) => w is WnImage && w.src == 'assets/svgs/ic_copy.svg', + ); + expect(copyButton, findsOneWidget); + await tester.tap(copyButton);Alternative: assign a Key to the copy button in StartChatBottomSheet and use find.byKey.
151-151: Assert the main loader specifically to avoid false positives.
Currently this passes even if only button spinners are present. Match the 32-size loader explicitly.Apply:
- expect(find.byType(CircularProgressIndicator), findsWidgets); + expect( + find.byWidgetPredicate( + (w) => + w is SizedBox && + (w.width == 32.0 || w.height == 32.0) && + find.descendant( + of: find.byWidget(w), + matching: find.byType(CircularProgressIndicator), + ).evaluate().isNotEmpty, + ), + findsOneWidget, + );Tip: add a Key to the main loader to make this more robust.
170-191: Simplify “main loader hidden” checks; current ancestor + .first is brittle.
Directly assert that no 32-size loader with a CircularProgressIndicator exists.Apply to each block:
- // Check that any CircularProgressIndicator found is from ButtonLoadingIndicator (18.w size) - // and not the main loading indicator (32.w size) - final indicators = find.byType(CircularProgressIndicator); - final indicatorWidgets = tester.widgetList<CircularProgressIndicator>(indicators); - for (final indicator in indicatorWidgets) { - final sizedBox = tester.widget<SizedBox>( - find - .ancestor( - of: find.byWidget(indicator), - matching: find.byType(SizedBox), - ) - .first, - ); - // Main loading indicator uses 32.w, button indicators use 18.w - expect( - sizedBox.width != 32.0, - isTrue, - reason: 'Found main loading indicator, should be hidden', - ); - } + expect( + find.byWidgetPredicate( + (w) => + w is SizedBox && + (w.width == 32.0 || w.height == 32.0) && + find.descendant( + of: find.byWidget(w), + matching: find.byType(CircularProgressIndicator), + ).evaluate().isNotEmpty, + ), + findsNothing, + reason: 'Found main loading indicator, should be hidden', + );Optional: expose a Key on the main loader and assert find.byKey(...), findsNothing for stronger guarantees.
Also applies to: 232-253, 368-387
302-311: Reduce duplication by reusing mockUser.
Same pubkey; avoid re-declaring User inline.Apply:
- followsProvider.overrideWith( - () => MockFollowsNotifier([ - User( - pubkey: contact.publicKey, - metadata: const FlutterMetadata(custom: {}), - createdAt: DateTime.now(), - updatedAt: DateTime.now(), - ), - ]), - ), + followsProvider.overrideWith( + () => MockFollowsNotifier([mockUser]), + ),
329-337: Repeat of inline User creation — prefer mockUser for consistency.Apply:
- followsProvider.overrideWith( - () => MockFollowsNotifier([ - User( - pubkey: contact.publicKey, - metadata: const FlutterMetadata(custom: {}), - createdAt: DateTime.now(), - updatedAt: DateTime.now(), - ), - ]), - ), + followsProvider.overrideWith( + () => MockFollowsNotifier([mockUser]), + ),
📜 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 (2)
lib/ui/contact_list/start_chat_bottom_sheet.dart(2 hunks)test/ui/contact_list/start_chat_bottom_sheet_test.dart(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ui/contact_list/start_chat_bottom_sheet.dart
🧰 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:
test/ui/contact_list/start_chat_bottom_sheet_test.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/start_chat_bottom_sheet_test.dart
🧠 Learnings (4)
📚 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/start_chat_bottom_sheet_test.dart
📚 Learning: 2025-08-23T11:02:28.308Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#527
File: lib/ui/core/ui/wn_avatar.dart:1-6
Timestamp: 2025-08-23T11:02:28.308Z
Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.
Applied to files:
test/ui/contact_list/start_chat_bottom_sheet_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 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:
test/ui/contact_list/start_chat_bottom_sheet_test.dart
📚 Learning: 2025-09-01T14:56:50.988Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#568
File: lib/ui/core/ui/wn_image.dart:1-2
Timestamp: 2025-09-01T14:56:50.988Z
Learning: The whitenoise_flutter project does not target web platforms and any future web version would be in a separate repository, so dart:io imports and file system operations are acceptable in this codebase.
Applied to files:
test/ui/contact_list/start_chat_bottom_sheet_test.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 (8)
test/ui/contact_list/start_chat_bottom_sheet_test.dart (8)
10-10: WnImage import aligns tests with the widget migration. Good change.
167-168: Manual pump after setup is fine here.
Prevents pumpAndSettle hangs with perpetual animations.
229-230: Extra pump after setup: looks good.
290-291: Extra pump after overrides: looks good.
315-315: Pump after widget build: good.
341-341: Pump after widget build: good.
363-364: Pump after setup: good.
29-29: No return type change required: ActiveAccountNotifier extends AsyncNotifier, so Future build() is correct.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
✅ For next time, I’d prefer skipping a test over keeping an expect with a reason that explains the opposite of what we actually want 😅 At least that way we still get a warning when running tests. In this case, the loading issue will be solved soon (in #599), so this problematic test will be removed anyway.
| expect( | ||
| sizedBox.width != 32.0, | ||
| isTrue, | ||
| reason: 'Found main loading indicator, should be hidden', |
There was a problem hiding this comment.
I dont' think this is an actual fix to checks passing...
There was a problem hiding this comment.
Actually, that loading is a bug, so it’s a good thing the tests were failing. I agree it should be hidden. The strange part is why these tests weren’t failing on master as well.
codeswot
dismissed stale reviews from untreu2, josefinalliende, and themself
via
4e27068
Description
Fixes #567 to replace left over SvgPicture and Image widgets with the new WnImage widget.
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
Bug Fixes
Style
Refactor
Tests