feat: media upload ui #721
Conversation
WalkthroughAdds a Riverpod ChatInputNotifier and Freezed ChatInputState for chat draft persistence, media selection, and UI state; converts DraftMessageService methods from static to instance methods; adds ImagePickerService.pickMultipleImages; introduces several chat-input UI widgets; updates AssetsPaths icons; adds "photos" translations; adds a borderless text-field option; and updates/adds tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as ChatInput Widget
participant Notifier as ChatInputNotifier
participant Draft as DraftMessageService
Note over UI,Notifier: Init & load draft
UI->>Notifier: loadDraft(chatId)
Notifier->>Draft: loadDraft(chatId)
Draft-->>Notifier: draftText?
Notifier-->>UI: state(updated with draft)
Note over User,Notifier: Typing -> scheduled save
User->>UI: type text
UI->>Notifier: scheduleDraftSave(text)
Notifier-->>Notifier: cancel prior timer / start timer
Notifier->>Draft: saveDraft(chatId, text) [on timer]
sequenceDiagram
autonumber
actor User
participant UI as ChatInput Widget
participant Notifier as ChatInputNotifier
participant ImgSvc as ImagePickerService
User->>UI: tap "Photos"
UI->>Notifier: toggleMediaSelector()
UI->>ImgSvc: pickMultipleImages()
ImgSvc-->>UI: [paths]
UI->>Notifier: handleImagesSelected(paths)
Notifier-->>UI: state(selectedImages updated, showMediaSelector=false)
User->>UI: remove image (index)
UI->>Notifier: removeImage(index)
Notifier-->>UI: state(selectedImages updated)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
josefinalliende
force-pushed
the
feat/media-upload-ui
branch
from
fec0d1e to
0b30f00
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/ui/chat/widgets/chat_input_media_selector.dart (1)
18-21: Consider flattening the nested containers.The outer
Containerat line 18 only provides padding, while the innerContainerat line 20 provides decoration. You can simplify this by applying padding directly to the inner container.Apply this diff to simplify the structure:
- return Container( - padding: EdgeInsets.only(left: 16.w, right: 16.w), - child: Container( + return Container( + margin: EdgeInsets.symmetric(horizontal: 16.w), decoration: BoxDecoration(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
assets/svgs/ic_image.svgis excluded by!**/*.svgios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
lib/config/providers/chat_input_provider.dart(1 hunks)lib/domain/services/draft_message_service.dart(4 hunks)lib/domain/services/image_picker_service.dart(1 hunks)lib/locales/de.json(1 hunks)lib/locales/en.json(1 hunks)lib/locales/es.json(1 hunks)lib/locales/fr.json(1 hunks)lib/locales/it.json(1 hunks)lib/locales/pt.json(1 hunks)lib/locales/ru.json(1 hunks)lib/locales/tr.json(1 hunks)lib/models/relay_status.dart(1 hunks)lib/ui/auth_flow/qr_scanner_bottom_sheet.dart(1 hunks)lib/ui/chat/states/chat_input_state.dart(1 hunks)lib/ui/chat/states/chat_input_state.freezed.dart(1 hunks)lib/ui/chat/widgets/chat_input.dart(4 hunks)lib/ui/chat/widgets/chat_input_media_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_media_selector.dart(1 hunks)lib/ui/chat/widgets/chat_input_reply_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_send_button.dart(1 hunks)lib/ui/chat/widgets/media_thumbnail.dart(1 hunks)lib/ui/core/themes/assets.dart(1 hunks)lib/ui/core/ui/wn_text_form_field.dart(3 hunks)lib/ui/settings/general_settings_screen.dart(1 hunks)test/config/providers/chat_input_provider_test.dart(1 hunks)test/domain/services/draft_message_service_test.dart(13 hunks)test/domain/services/image_picker_service_test.dart(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
test/domain/services/image_picker_service_test.dartlib/ui/auth_flow/qr_scanner_bottom_sheet.dartlib/ui/chat/states/chat_input_state.dartlib/models/relay_status.dartlib/ui/chat/widgets/chat_input_reply_preview.dartlib/ui/core/ui/wn_text_form_field.dartlib/domain/services/image_picker_service.dartlib/ui/chat/widgets/chat_input.dartlib/config/providers/chat_input_provider.dartlib/ui/chat/widgets/media_thumbnail.dartlib/ui/chat/widgets/chat_input_media_selector.darttest/config/providers/chat_input_provider_test.dartlib/ui/chat/states/chat_input_state.freezed.dartlib/ui/settings/general_settings_screen.dartlib/domain/services/draft_message_service.dartlib/ui/chat/widgets/chat_input_media_preview.dartlib/ui/chat/widgets/chat_input_send_button.darttest/domain/services/draft_message_service_test.dartlib/ui/core/themes/assets.dart
test/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
test/**/*.dart: Follow Arrange-Act-Assert in tests
Name test variables clearly using inputX, mockX, actualX, expectedX
Write unit tests for each public function; use test doubles for dependencies (except cheap third-party)
Use standard Flutter widget testing
Files:
test/domain/services/image_picker_service_test.darttest/config/providers/chat_input_provider_test.darttest/domain/services/draft_message_service_test.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/auth_flow/qr_scanner_bottom_sheet.dartlib/ui/chat/states/chat_input_state.dartlib/models/relay_status.dartlib/ui/chat/widgets/chat_input_reply_preview.dartlib/ui/core/ui/wn_text_form_field.dartlib/domain/services/image_picker_service.dartlib/ui/chat/widgets/chat_input.dartlib/config/providers/chat_input_provider.dartlib/ui/chat/widgets/media_thumbnail.dartlib/ui/chat/widgets/chat_input_media_selector.dartlib/ui/chat/states/chat_input_state.freezed.dartlib/ui/settings/general_settings_screen.dartlib/domain/services/draft_message_service.dartlib/ui/chat/widgets/chat_input_media_preview.dartlib/ui/chat/widgets/chat_input_send_button.dartlib/ui/core/themes/assets.dart
🧠 Learnings (1)
📚 Learning: 2025-10-12T22:44:49.592Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-10-12T22:44:49.592Z
Learning: Applies to lib/**/*.dart : Use freezed to model/manage UI states
Applied to files:
lib/ui/chat/states/chat_input_state.dartlib/ui/chat/states/chat_input_state.freezed.dart
🔇 Additional comments (23)
lib/ui/core/ui/wn_text_form_field.dart (1)
53-53: LGTM! Clean feature addition with proper defaults.The
hideBorderparameter is well-implemented with a sensible default (false) that maintains backward compatibility. The conditional border logic is clear and follows Flutter conventions by usingInputBorder.nonefor hidden borders andnullforfocusedBorderto preserve default behavior.Also applies to: 88-88, 165-183
lib/locales/es.json (1)
204-205: LGTM! Proper locale addition.The translation entries are correctly formatted and the Spanish translation "Fotos" is appropriate.
lib/ui/auth_flow/qr_scanner_bottom_sheet.dart (1)
77-77: LGTM! Asset constant update.The asset reference has been correctly updated to align with the new asset API naming conventions.
lib/models/relay_status.dart (1)
73-73: LGTM! Consistent asset constant update.The asset reference update maintains consistency with the new asset API naming conventions.
lib/locales/pt.json (1)
204-205: LGTM! Proper locale addition.The Portuguese translation "Fotos" is correctly added with proper JSON formatting.
lib/locales/it.json (1)
204-205: LGTM! Proper locale addition.The Italian translation "Foto" is correctly formatted and appropriate.
lib/locales/ru.json (1)
204-205: LGTM! Proper locale addition.The Russian translation "Фото" is correctly added with proper JSON formatting.
lib/locales/tr.json (1)
204-205: LGTM! Proper locale addition.The Turkish translation "Fotoğraflar" is correctly formatted and appropriate.
lib/ui/settings/general_settings_screen.dart (1)
138-138: LGTM! Correctly updated to instance-based service call.The migration from static to instance-based
DraftMessageServiceis properly reflected here.lib/locales/de.json (1)
204-205: LGTM! Translation added correctly.The "photos" translation is properly added with correct JSON syntax.
lib/locales/en.json (1)
204-205: LGTM! New translation key added correctly.The "photos" key is properly added to support the new media selection UI.
lib/domain/services/image_picker_service.dart (1)
26-39: LGTM! Well-implemented multiple image selection.The method follows the existing pattern from
pickProfileImageand uses appropriate dimensions for chat media (1920x1920 vs 512x512 for profile images). Error handling is consistent.test/domain/services/image_picker_service_test.dart (1)
77-121: LGTM! Comprehensive test coverage for multiple image selection.The test group covers all key scenarios: single image, multiple images, and picker failure. Tests follow the Arrange-Act-Assert pattern correctly.
lib/locales/fr.json (1)
204-205: LGTM! French translation added correctly.The "photos" translation is properly added with correct JSON syntax.
lib/ui/chat/states/chat_input_state.dart (1)
1-14: LGTM! Clean Freezed state implementation.The
ChatInputStatecorrectly uses Freezed for immutable UI state management, following the coding guidelines. Field names use appropriate verbs (isLoadingDraft,showMediaSelector) and all types are properly declared.Based on coding guidelines
lib/ui/chat/widgets/chat_input_send_button.dart (1)
9-78: LGTM! Well-structured send button widget.The widget is properly scoped with a single responsibility, manages the text controller listener lifecycle correctly, and uses smooth animations for state transitions. The implementation follows coding guidelines for small, focused components.
lib/ui/chat/widgets/media_thumbnail.dart (1)
50-51: Fix invalid Color API usage in overlay.
Colors.black.withValuesis not a valid Flutter Color API, so this widget will not compile. UsewithOpacity(0.5)(or an equivalent alpha setter) to achieve the intended semi-transparent overlay.- color: Colors.black.withValues(alpha: 0.5), + color: Colors.black.withOpacity(0.5),⛔ Skipped due to learnings
Learnt from: Quwaysim PR: parres-hq/whitenoise_flutter#640 File: lib/ui/core/ui/wn_bottom_sheet.dart:0-0 Timestamp: 2025-09-16T06:11:58.094Z Learning: In Flutter 3.27.0 and later, `withOpacity()` is deprecated in favor of `withValues(alpha: value)` to avoid precision loss. The `withValues()` method maintains floating-point precision while `withOpacity()` converts to integers internally, causing precision loss.lib/ui/chat/widgets/chat_input_media_selector.dart (1)
8-55: LGTM! Clean widget structure with proper typing.The widget follows Dart and Flutter best practices with proper type declarations, const constructors, and a shallow widget tree. The structure with a Column and Divider suggests the design is prepared for future expansion to support additional media types.
lib/ui/chat/widgets/chat_input_media_preview.dart (1)
27-35: LGTM! Solid state management and interaction handling.The widget properly manages its internal state with ScrollController disposal, handles edge cases (empty list early return), and implements intuitive thumbnail interactions (tap to activate, tap again to remove). The active thumbnail scrolling enhances UX.
Also applies to: 37-49, 61-124
lib/config/providers/chat_input_provider.dart (1)
8-112: LGTM! Excellent provider implementation with robust error handling.The notifier demonstrates best practices throughout:
- Dependency injection with sensible defaults enables testability
- Debounced draft saving with
scheduleDraftSavereduces I/O- Bounds checking at lines 77-80 prevents index errors
mountedcheck at line 33 prevents state updates after disposal- Proper resource cleanup in
dispose- Clear separation of concerns with focused, single-purpose methods
The implementation aligns well with the coding guidelines for Riverpod state management.
lib/ui/chat/widgets/chat_input_reply_preview.dart (1)
10-87: LGTM! Clean implementation with proper null handling.The widget elegantly handles optional reply/edit states with:
- Early return for null case at line 24
- Graceful fallback chain for sender name (lines 46-48) and content (line 74)
- Proper use of nullable types throughout
- Responsive sizing with ScreenUtil
- Consistent theming via context extensions
The implementation follows Flutter and Dart best practices.
lib/domain/services/draft_message_service.dart (1)
7-63: LGTM! Good refactor for testability.Converting from static to instance methods enables proper dependency injection and improves testability, aligning with the PR objectives. The optional
storageparameter in each method allows test code to inject mock storage implementations.The
_defaultStorageand_draftPrefixremaining as static is acceptable since they're constants and don't hinder testability of the instance methods.lib/ui/core/themes/assets.dart (1)
6-82: LGTM! Comprehensive icon catalog expansion.The additions and reorganization provide a well-structured icon catalog with:
- Consistent naming conventions (
ic<Name>pattern)- Clear path construction using
_svgsDir- Alphabetically organized for easy navigation
- Support for new media features (e.g.,
icImageat line 43)The structure makes it easy to maintain and locate icons.
josefinalliende
force-pushed
the
feat/media-upload-ui
branch
from
0b30f00 to
9fdc780
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
lib/ui/chat/widgets/chat_input.dart(4 hunks)lib/ui/chat/widgets/chat_input_media_preview.dart(1 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 dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/ui/chat/widgets/chat_input_media_preview.dartlib/ui/chat/widgets/chat_input.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/chat/widgets/chat_input_media_preview.dartlib/ui/chat/widgets/chat_input.dart
🧠 Learnings (1)
📚 Learning: 2025-10-12T22:44:49.592Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-10-12T22:44:49.592Z
Learning: Applies to **/*.dart : Avoid magic numbers and define constants
Applied to files:
lib/ui/chat/widgets/chat_input_media_preview.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
josefinalliende
force-pushed
the
feat/media-upload-ui
branch
from
9fdc780 to
027b88c
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
lib/ui/chat/widgets/chat_input.dart (1)
65-78: Focus border repaint fix is in place.setState is now called on focus changes; the border updates correctly. Matches prior suggestion.
🧹 Nitpick comments (13)
lib/ui/core/ui/wn_text_form_field.dart (1)
165-182: Extract repeated border configuration to reduce duplication.The same
OutlineInputBorderconfiguration is duplicated for bothborderandenabledBorder. Consider extracting this to a local variable to improve maintainability.Apply this diff to extract the common border:
+ final standardBorder = OutlineInputBorder( + borderRadius: BorderRadius.zero, + borderSide: BorderSide( + color: context.colors.input, + ), + ); + final decoration = (widget.decoration ?? const InputDecoration()).copyWith( constraints: isMultiline ? null : BoxConstraints.tightFor(height: targetHeight), suffixIcon: suffixIcon, labelText: widget.labelText, hintText: widget.hintText, hintStyle: TextStyle( fontSize: 14.sp, fontWeight: FontWeight.w600, color: context.colors.mutedForeground, ), suffixIconColor: context.colors.primary, fillColor: context.colors.avatarSurface, contentPadding: EdgeInsets.symmetric(horizontal: 12.w, vertical: isSmall ? 13.5.h : 19.5.h), prefixIconConstraints: BoxConstraints.tightFor(height: targetHeight), suffixIconConstraints: BoxConstraints.tightFor(height: targetHeight), - border: - widget.hideBorder - ? InputBorder.none - : OutlineInputBorder( - borderRadius: BorderRadius.zero, - borderSide: BorderSide( - color: context.colors.input, - ), - ), - enabledBorder: - widget.hideBorder - ? InputBorder.none - : OutlineInputBorder( - borderRadius: BorderRadius.zero, - borderSide: BorderSide( - color: context.colors.input, - ), - ), + border: widget.hideBorder ? InputBorder.none : standardBorder, + enabledBorder: widget.hideBorder ? InputBorder.none : standardBorder, focusedBorder: widget.hideBorder ? InputBorder.none : null, filled: true, );lib/ui/chat/widgets/chat_input_send_button.dart (1)
46-76: Ignore whitespace-only input when showing the send button.Use trim to avoid showing the button for spaces/newlines.
- final hasContent = widget.textController.text.isNotEmpty || widget.hasImages; + final hasContent = + widget.textController.text.trim().isNotEmpty || widget.hasImages;test/config/providers/chat_input_provider_test.dart (2)
82-91: Dispose the ad-hoc ProviderContainer.Avoid leaking listeners by disposing the container created inside the test.
test('has expected initial state', () { final testContainer = ProviderContainer(); + addTearDown(testContainer.dispose); final state = testContainer.read(chatInputProvider('test-group-123')); expect(state, isA<ChatInputState>()); expect(state.isLoadingDraft, false);
268-286: Use fake timers instead of real delays for draft-save tests.Makes tests faster and deterministic.
Example with FakeAsync (from flutter_test’s fake_async):
import 'package:fake_async/fake_async.dart'; test('saves draft after delay', () { mockDraftMessageService.reset(); fakeAsync((fa) { notifier.scheduleDraftSave('test draft'); expect(mockDraftMessageService.savedDrafts, isEmpty); fa.elapse(testDraftSaveDelay); }); expect(mockDraftMessageService.savedDrafts, ['test draft']); });lib/ui/chat/widgets/chat_input.dart (3)
127-135: Avoid re-saving the draft immediately after loading it.Programmatically setting the controller after load triggers _onTextChanged and schedules a save of the same content. Suppress the next text-change callback when applying a loaded draft.
class _ChatInputState extends ConsumerState<ChatInput> with WidgetsBindingObserver { final _textController = TextEditingController(); final _focusNode = FocusNode(); final _inputKey = GlobalKey(); + bool _suppressNextTextChange = false; @@ Future<void> _loadDraftMessage() async { final chatState = ref.read(chatProvider); final isEditing = chatState.editingMessage[widget.groupId] != null; if (!isEditing) { final chatInputNotifier = ref.read(chatInputProvider(widget.groupId).notifier); final draft = await chatInputNotifier.loadDraft(); - if (draft != null && draft.isNotEmpty && mounted) { - _textController.text = draft; - } + if (draft != null && draft.isNotEmpty && mounted) { + _suppressNextTextChange = true; + _textController.text = draft; + } } } void _onTextChanged() { - final chatInputState = ref.read(chatInputProvider(widget.groupId)); + if (_suppressNextTextChange) { + _suppressNextTextChange = false; + return; + } + final chatInputState = ref.read(chatInputProvider(widget.groupId)); if (chatInputState.isLoadingDraft) return;Also applies to: 139-148
80-87: Minor: clarify media toggling logic.Compute the post-toggle visibility explicitly for readability.
void _toggleMediaSelector() { final chatInputNotifier = ref.read(chatInputProvider(widget.groupId).notifier); - final chatInputState = ref.read(chatInputProvider(widget.groupId)); - chatInputNotifier.toggleMediaSelector(); - if (!chatInputState.showMediaSelector) { - _focusNode.unfocus(); - } + final willShow = !ref.read(chatInputProvider(widget.groupId)).showMediaSelector; + chatInputNotifier.toggleMediaSelector(); + if (willShow) _focusNode.unfocus(); }
184-189: Avoid unnecessary rebuilds by not watching the notifier.Use ref.read for chatProvider.notifier; watching the notifier rarely provides value and can cause extra rebuilds.
- final chatNotifier = ref.watch(chatProvider.notifier); + final chatNotifier = ref.read(chatProvider.notifier);lib/config/providers/chat_input_provider.dart (1)
64-74: Deduplicate images when appending selections.Avoid duplicate thumbnails if the user picks the same image(s) again.
Future<void> handleImagesSelected() async { final imagePaths = await _imagePickerService.pickMultipleImages(); - if (imagePaths.isNotEmpty) { - state = state.copyWith( - showMediaSelector: false, - selectedImages: [...state.selectedImages, ...imagePaths], - ); - } else { - state = state.copyWith(showMediaSelector: false); - } + final existing = state.selectedImages; + final newOnes = imagePaths.where((p) => !existing.contains(p)).toList(); + state = state.copyWith( + showMediaSelector: false, + selectedImages: newOnes.isEmpty ? existing : [...existing, ...newOnes], + ); }lib/ui/chat/widgets/chat_input_media_selector.dart (1)
66-95: Add semantics for accessibility.Announce the action to screen readers.
- return InkWell( - onTap: onTap, - child: Container( + return Semantics( + button: true, + label: label, + onTap: onTap, + child: InkWell( + onTap: onTap, + child: Container( width: double.infinity, padding: EdgeInsets.symmetric(horizontal: 16.w, vertical: 16.h), decoration: BoxDecoration( color: context.colors.surface, ), child: Row( mainAxisAlignment: MainAxisAlignment.spaceBetween, children: [ Text( label, style: context.textTheme.bodyMedium?.copyWith( fontWeight: FontWeight.w600, color: context.colors.primary, ), ), WnImage( icon, size: 24.w, color: context.colors.primary, ), ], ), - ), - ), + ), + ), );lib/ui/chat/widgets/chat_input_reply_preview.dart (1)
29-30: Extract magic numbers to named constants.Multiple hardcoded dimension values appear throughout the widget (14.w, 8.h, 24.w, 16.w, 12.sp, 4.h). These should be extracted to named constants for maintainability and consistency.
Based on learnings
Define constants at the top of the class:
class ChatInputReplyPreview extends StatelessWidget { static const double _horizontalMargin = 14.0; static const double _verticalMarginTop = 14.0; static const double _verticalMarginBottom = 8.0; static const double _padding = 8.0; static const double _closeButtonSize = 24.0; static const double _closeIconSize = 16.0; static const double _textFontSize = 12.0; static const double _contentGap = 4.0; const ChatInputReplyPreview({Then update the usages:
return Container( - margin: EdgeInsets.all(14.w).copyWith(bottom: 8.h), - padding: EdgeInsets.all(8.w), + margin: EdgeInsets.symmetric( + horizontal: _horizontalMargin.w, + vertical: _verticalMarginTop.h, + ).copyWith(bottom: _verticalMarginBottom.h), + padding: EdgeInsets.all(_padding.w), decoration: BoxDecoration(style: TextStyle( color: context.colors.mutedForeground, - fontSize: 12.sp, + fontSize: _textFontSize.sp, fontWeight: FontWeight.w600,child: Container( - width: 24.w, - height: 24.w, + width: _closeButtonSize.w, + height: _closeButtonSize.w, alignment: Alignment.center, child: WnImage( AssetsPaths.icClose, - width: 16.w, - height: 16.w, + width: _closeIconSize.w, + height: _closeIconSize.w, color: context.colors.mutedForeground,- Gap(4.h), + Gap(_contentGap.h), Text(style: TextStyle( color: context.colors.primary, - fontSize: 12.sp, + fontSize: _textFontSize.sp, fontWeight: FontWeight.w600,Also applies to: 51-51, 58-64, 72-72, 77-77
lib/ui/chat/widgets/chat_input_media_preview.dart (2)
70-70: Extract remaining magic numbers to constants.While the core image dimensions have been properly extracted (great job!), several padding and margin values remain hardcoded (14.w, 8.h, 16.h at line 70; 12.h, 12.w at lines 93-94; 32.h at line 97; 32.w, 8.w at lines 107-108). For consistency and maintainability, these should also be extracted to constants.
Based on learnings
Add these constants alongside the existing ones:
class _ChatInputMediaPreviewState extends State<ChatInputMediaPreview> { static const double _imageWidth = 285.0; static const double _imageHeight = 250.0; static const double _imageSpacing = 8.0; static const double _thumbnailSpacing = 12.0; + static const double _horizontalPadding = 14.0; + static const double _verticalPaddingReply = 8.0; + static const double _verticalPaddingNormal = 16.0; + static const double _thumbnailBottomPosition = 12.0; + static const double _thumbnailLeftPosition = 12.0; + static const double _thumbnailHeight = 32.0; + static const double _addButtonSize = 32.0; + static const double _addButtonPadding = 8.0; final _scrollController = ScrollController();Then update the usages:
return Container( - padding: EdgeInsets.symmetric(horizontal: 14.w, vertical: widget.isReply ? 8.h : 16.h), + padding: EdgeInsets.symmetric( + horizontal: _horizontalPadding.w, + vertical: widget.isReply ? _verticalPaddingReply.h : _verticalPaddingNormal.h, + ), child: SizedBox(Positioned( - bottom: 12.h, - left: 12.w, + bottom: _thumbnailBottomPosition.h, + left: _thumbnailLeftPosition.w, right: 0, child: SizedBox( - height: 32.h, + height: _thumbnailHeight.h, child: ListView.separated(return WnIconButton( onTap: widget.onAddMore, iconPath: AssetsPaths.icAdd, - size: 32.w, - padding: 8.w, + size: _addButtonSize.w, + padding: _addButtonPadding.w, buttonColor: context.colors.secondary,Also applies to: 93-94, 97-97, 107-108
22-24: Add blank line before @OverRide for consistency.Minor formatting: add a blank line before the
@overrideannotation to follow Dart style conventions.Apply this diff:
final bool isReply; + @override State<ChatInputMediaPreview> createState() => _ChatInputMediaPreviewState();lib/ui/core/themes/assets.dart (1)
6-8: Alphabetical ordering is incomplete.The constants are not fully alphabetically sorted as suggested by the commit message. For example:
- Lines 6-8:
icAddshould come beforeicAddNewChatandicAddUser- Lines 15-18:
icCheckmarkshould come beforeicCheckmarkFilledandicCheckmarkSolidWhile this doesn't affect functionality, maintaining strict alphabetical order improves maintainability and makes it easier to locate constants.
Apply this diff to correct the ordering:
//SVGS - static const String icAddNewChat = '$_svgsDir/ic_add_new_chat.svg'; - static const String icAddUser = '$_svgsDir/ic_add_user.svg'; static const String icAdd = '$_svgsDir/ic_add.svg'; + static const String icAddNewChat = '$_svgsDir/ic_add_new_chat.svg'; + static const String icAddUser = '$_svgsDir/ic_add_user.svg'; static const String icArrowRight = '$_svgsDir/ic_arrow_right.svg';static const String icChatInvite = '$_svgsDir/ic_chat_invite.svg'; static const String icChat = '$_svgsDir/ic_chat.svg'; + static const String icCheckmark = '$_svgsDir/ic_checkmark.svg'; static const String icCheckmarkFilled = '$_svgsDir/ic_checkmark_filled.svg'; static const String icCheckmarkSolid = '$_svgsDir/ic_checkmark_solid.svg'; - static const String icCheckmark = '$_svgsDir/ic_checkmark.svg'; static const String icChevronDown = '$_svgsDir/ic_chevron_down.svg';Also applies to: 15-18
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
assets/svgs/ic_image.svgis excluded by!**/*.svgios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
lib/config/providers/chat_input_provider.dart(1 hunks)lib/domain/services/draft_message_service.dart(4 hunks)lib/domain/services/image_picker_service.dart(1 hunks)lib/locales/de.json(1 hunks)lib/locales/en.json(1 hunks)lib/locales/es.json(1 hunks)lib/locales/fr.json(1 hunks)lib/locales/it.json(1 hunks)lib/locales/pt.json(1 hunks)lib/locales/ru.json(1 hunks)lib/locales/tr.json(1 hunks)lib/models/relay_status.dart(1 hunks)lib/ui/auth_flow/qr_scanner_bottom_sheet.dart(1 hunks)lib/ui/chat/states/chat_input_state.dart(1 hunks)lib/ui/chat/states/chat_input_state.freezed.dart(1 hunks)lib/ui/chat/widgets/chat_input.dart(5 hunks)lib/ui/chat/widgets/chat_input_media_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_media_selector.dart(1 hunks)lib/ui/chat/widgets/chat_input_reply_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_send_button.dart(1 hunks)lib/ui/chat/widgets/media_thumbnail.dart(1 hunks)lib/ui/core/themes/assets.dart(1 hunks)lib/ui/core/ui/wn_text_form_field.dart(3 hunks)lib/ui/settings/general_settings_screen.dart(1 hunks)test/config/providers/chat_input_provider_test.dart(1 hunks)test/domain/services/draft_message_service_test.dart(13 hunks)test/domain/services/image_picker_service_test.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- lib/locales/es.json
- lib/domain/services/image_picker_service.dart
- lib/locales/de.json
- lib/ui/settings/general_settings_screen.dart
- lib/ui/chat/widgets/media_thumbnail.dart
- lib/ui/chat/states/chat_input_state.dart
- lib/locales/pt.json
- lib/locales/it.json
- lib/ui/chat/states/chat_input_state.freezed.dart
- lib/locales/tr.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
test/domain/services/image_picker_service_test.darttest/domain/services/draft_message_service_test.dartlib/ui/chat/widgets/chat_input_send_button.dartlib/ui/chat/widgets/chat_input_reply_preview.dartlib/domain/services/draft_message_service.dartlib/ui/chat/widgets/chat_input_media_selector.dartlib/ui/core/ui/wn_text_form_field.dartlib/ui/auth_flow/qr_scanner_bottom_sheet.dartlib/models/relay_status.dartlib/config/providers/chat_input_provider.dartlib/ui/chat/widgets/chat_input.dartlib/ui/core/themes/assets.darttest/config/providers/chat_input_provider_test.dartlib/ui/chat/widgets/chat_input_media_preview.dart
test/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
test/**/*.dart: Follow Arrange-Act-Assert in tests
Name test variables clearly using inputX, mockX, actualX, expectedX
Write unit tests for each public function; use test doubles for dependencies (except cheap third-party)
Use standard Flutter widget testing
Files:
test/domain/services/image_picker_service_test.darttest/domain/services/draft_message_service_test.darttest/config/providers/chat_input_provider_test.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/chat/widgets/chat_input_send_button.dartlib/ui/chat/widgets/chat_input_reply_preview.dartlib/domain/services/draft_message_service.dartlib/ui/chat/widgets/chat_input_media_selector.dartlib/ui/core/ui/wn_text_form_field.dartlib/ui/auth_flow/qr_scanner_bottom_sheet.dartlib/models/relay_status.dartlib/config/providers/chat_input_provider.dartlib/ui/chat/widgets/chat_input.dartlib/ui/core/themes/assets.dartlib/ui/chat/widgets/chat_input_media_preview.dart
🧠 Learnings (1)
📚 Learning: 2025-10-12T22:44:49.592Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-10-12T22:44:49.592Z
Learning: Applies to **/*.dart : Avoid magic numbers and define constants
Applied to files:
lib/ui/chat/widgets/chat_input_media_preview.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 (10)
lib/locales/ru.json (1)
204-205: LGTM!The localization key addition is properly formatted and consistent with similar additions across other locale files in this PR.
lib/locales/en.json (1)
204-205: LGTM!The localization key is properly added and will support the new chat input media selector UI component.
test/domain/services/draft_message_service_test.dart (1)
19-27: LGTM!The test updates correctly align with the service refactor from static to instance methods. Creating a new instance for each test call ensures proper test isolation and follows unit testing best practices.
Also applies to: 36-44, 47-55, 67-75, 86-92, 97-103, 115-121, 133-140, 143-150, 155-162, 175-179, 182-186, 191-195, 208-212, 215-219
test/domain/services/image_picker_service_test.dart (2)
23-39: LGTM!The multiline formatting improves readability while preserving the existing test behavior.
77-121: LGTM!Excellent test coverage for the new
pickMultipleImagesmethod. The tests comprehensively cover single image selection, multiple images, and error scenarios.lib/domain/services/draft_message_service.dart (2)
7-23: LGTM!Converting from static to instance method improves testability and enables dependency injection. The method logic and error handling remain unchanged.
25-36: LGTM!The conversion to instance methods is consistent across all service methods and maintains the existing behavior while enabling better testing and mocking capabilities.
Also applies to: 38-49, 51-63
lib/models/relay_status.dart (1)
73-73: LGTM!The asset reference update aligns with the asset set modernization described in the PR summary.
lib/ui/auth_flow/qr_scanner_bottom_sheet.dart (1)
77-77: LGTM!The asset reference update is consistent with the broader asset reorganization in this PR.
lib/locales/fr.json (1)
204-205: LGTM!The French localization follows the same pattern as other locale files and is properly formatted.
josefinalliende
force-pushed
the
feat/media-upload-ui
branch
from
876fbe5 to
82cf58c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
lib/ui/chat/widgets/media_thumbnail.dart (1)
35-40: Consider adding error handling for file loading.While the image paths should be valid since they come from the ImagePickerService, adding an error widget (e.g.,
errorBuilderparameter on Image.file) would improve robustness in case of unexpected file system issues.Example:
Image.file( File(path), height: 32.h, width: 32.w, fit: BoxFit.cover, errorBuilder: (context, error, stackTrace) => Container( height: 32.h, width: 32.w, color: context.colors.mutedForeground, child: Icon(Icons.error, size: 16.w), ), )lib/ui/chat/widgets/chat_input_media_preview.dart (1)
42-54: Consider resetting active state when images change externally.If the image list changes externally (e.g., via undo or external removal),
_activeThumbIndexcould point to an invalid or different image. Consider addingdidUpdateWidgetto reset_activeThumbIndexif the list length changes.Example:
@override void didUpdateWidget(ChatInputMediaPreview oldWidget) { super.didUpdateWidget(oldWidget); if (oldWidget.imagePaths.length != widget.imagePaths.length) { if (_activeThumbIndex != null && _activeThumbIndex! >= widget.imagePaths.length) { setState(() { _activeThumbIndex = null; }); } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
assets/svgs/ic_image.svgis excluded by!**/*.svgios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
lib/config/providers/chat_input_provider.dart(1 hunks)lib/domain/services/draft_message_service.dart(4 hunks)lib/domain/services/image_picker_service.dart(1 hunks)lib/locales/de.json(1 hunks)lib/locales/en.json(1 hunks)lib/locales/es.json(1 hunks)lib/locales/fr.json(1 hunks)lib/locales/it.json(1 hunks)lib/locales/pt.json(1 hunks)lib/locales/ru.json(1 hunks)lib/locales/tr.json(1 hunks)lib/models/relay_status.dart(1 hunks)lib/ui/auth_flow/qr_scanner_bottom_sheet.dart(1 hunks)lib/ui/chat/states/chat_input_state.dart(1 hunks)lib/ui/chat/states/chat_input_state.freezed.dart(1 hunks)lib/ui/chat/widgets/chat_input.dart(5 hunks)lib/ui/chat/widgets/chat_input_media_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_media_selector.dart(1 hunks)lib/ui/chat/widgets/chat_input_reply_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_send_button.dart(1 hunks)lib/ui/chat/widgets/media_thumbnail.dart(1 hunks)lib/ui/core/themes/assets.dart(1 hunks)lib/ui/core/ui/wn_text_form_field.dart(3 hunks)lib/ui/settings/general_settings_screen.dart(1 hunks)test/config/providers/chat_input_provider_test.dart(1 hunks)test/domain/services/draft_message_service_test.dart(13 hunks)test/domain/services/image_picker_service_test.dart(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/ui/auth_flow/qr_scanner_bottom_sheet.dart
🚧 Files skipped from review as they are similar to previous changes (17)
- lib/locales/pt.json
- lib/ui/settings/general_settings_screen.dart
- lib/ui/chat/states/chat_input_state.dart
- lib/locales/es.json
- lib/ui/chat/states/chat_input_state.freezed.dart
- test/domain/services/draft_message_service_test.dart
- lib/domain/services/draft_message_service.dart
- lib/domain/services/image_picker_service.dart
- lib/locales/tr.json
- lib/config/providers/chat_input_provider.dart
- lib/locales/it.json
- test/domain/services/image_picker_service_test.dart
- lib/ui/chat/widgets/chat_input_reply_preview.dart
- lib/ui/chat/widgets/chat_input_send_button.dart
- lib/locales/ru.json
- lib/locales/fr.json
- lib/locales/en.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/ui/chat/widgets/chat_input_media_preview.dartlib/ui/core/ui/wn_text_form_field.darttest/config/providers/chat_input_provider_test.dartlib/ui/chat/widgets/chat_input.dartlib/models/relay_status.dartlib/ui/chat/widgets/chat_input_media_selector.dartlib/ui/chat/widgets/media_thumbnail.dartlib/ui/core/themes/assets.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/chat/widgets/chat_input_media_preview.dartlib/ui/core/ui/wn_text_form_field.dartlib/ui/chat/widgets/chat_input.dartlib/models/relay_status.dartlib/ui/chat/widgets/chat_input_media_selector.dartlib/ui/chat/widgets/media_thumbnail.dartlib/ui/core/themes/assets.dart
test/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
test/**/*.dart: Follow Arrange-Act-Assert in tests
Name test variables clearly using inputX, mockX, actualX, expectedX
Write unit tests for each public function; use test doubles for dependencies (except cheap third-party)
Use standard Flutter widget testing
Files:
test/config/providers/chat_input_provider_test.dart
🔇 Additional comments (8)
lib/locales/de.json (1)
203-204: LGTM!The German translation for "photos" is correct and properly integrated into the localization structure.
lib/ui/core/ui/wn_text_form_field.dart (1)
53-53: LGTM!The
isBorderHiddenparameter follows the verb naming convention for booleans and is correctly implemented throughout the widget. The conditional border logic is clear and straightforward.Also applies to: 88-88, 165-183
lib/ui/chat/widgets/chat_input_media_selector.dart (1)
8-53: LGTM!The
ChatInputMediaSelectorwidget is well-structured with clear separation of concerns. The use of localization and responsive sizing follows best practices.test/config/providers/chat_input_provider_test.dart (1)
51-305: LGTM!The test suite is comprehensive and well-structured. It covers all major state transitions, edge cases, and follows the Arrange-Act-Assert pattern as per coding guidelines.
lib/ui/chat/widgets/chat_input.dart (1)
7-12: LGTM!The refactor to a provider-based architecture is well-executed. The code follows Riverpod patterns, properly separates concerns with extracted widgets, and addresses all past review feedback including the focus-driven border updates. The draft management, media handling, and send logic are all correctly implemented.
Also applies to: 44-49, 51-78, 80-98, 127-159, 161-182, 184-314
lib/ui/chat/widgets/chat_input_media_preview.dart (1)
65-130: LGTM!The media preview widget is well-structured with proper constant usage (addressing past feedback), clean layout, and intuitive interaction patterns. The ScrollController is properly disposed.
lib/models/relay_status.dart (1)
73-73: Asset constant verified
AssetsPaths.icCheckmarkFilledis defined inlib/ui/core/themes/assets.dartand references the existing fileassets/svgs/ic_checkmark_filled.svg.lib/ui/core/themes/assets.dart (1)
1-89: LGTM! Clean asset constants refactoring.The reorganization is well-structured with proper alphabetical ordering, consistent naming conventions (
icNamepattern), and appropriate use ofstatic const Stringdeclarations. The addition oficImage(line 43) supports the new media upload functionality.
josefinalliende
force-pushed
the
feat/media-upload-ui
branch
from
f33e2bb to
a0f3052
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
lib/ui/chat/widgets/chat_input.dart (1)
65-78: Focus change handling correctly addresses previous feedback.The
setStatecall at the end ensures theAnimatedPadding's border color updates on every focus transition. This properly addresses the previous review feedback about restoring focus-driven border updates.lib/ui/chat/widgets/chat_input_reply_preview.dart (1)
28-87: Localization properly addressed.The fallback user name now uses
'shared.unknownUser'.tr()which addresses the previous localization feedback. All text is properly localized, and the layout provides good UX with ellipsis for long content.lib/ui/chat/widgets/chat_input_media_preview.dart (1)
10-40: Constants properly extracted.The magic numbers have been properly extracted to named constants as per previous feedback. Good practice with the static const declarations. Controller disposal is properly implemented.
🧹 Nitpick comments (2)
lib/ui/chat/widgets/chat_input_media_selector.dart (1)
36-50: Consider simplifying the widget tree.The
Columnwraps a single child followed by aDivider. If only photos will ever be supported, this structure works fine. However, if you plan to add more media types (videos, files, etc.), consider making the widget more extensible by accepting a list of options rather than hardcoding a single_ChatInputMediaSelectorOption.lib/ui/chat/widgets/media_thumbnail.dart (1)
43-64: Consider simplifying the active state overlay.The nested
ContainerinsideCenterinsideContainerat lines 46-62 can be simplified while maintaining the same visual effect. This is purely a readability improvement.Apply this diff to simplify:
if (isActive) Positioned.fill( - child: Center( - child: Container( - width: 32.w, - height: 32.h, - decoration: BoxDecoration( - color: Colors.black.withValues(alpha: 0.5), - ), - child: Center( - child: SizedBox( - width: 18.w, - height: 18.h, - child: WnImage( - AssetsPaths.icTrashCan, - color: context.colors.solidNeutralWhite, - ), - ), - ), - ), - ), + child: Container( + width: 32.w, + height: 32.h, + decoration: BoxDecoration( + color: Colors.black.withValues(alpha: 0.5), + ), + child: Center( + child: WnImage( + AssetsPaths.icTrashCan, + size: 18.w, + color: context.colors.solidNeutralWhite, + ), + ), + ), ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
assets/svgs/ic_image.svgis excluded by!**/*.svgios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
lib/config/providers/chat_input_provider.dart(1 hunks)lib/domain/services/draft_message_service.dart(4 hunks)lib/domain/services/image_picker_service.dart(1 hunks)lib/locales/de.json(1 hunks)lib/locales/en.json(1 hunks)lib/locales/es.json(1 hunks)lib/locales/fr.json(1 hunks)lib/locales/it.json(1 hunks)lib/locales/pt.json(1 hunks)lib/locales/ru.json(1 hunks)lib/locales/tr.json(1 hunks)lib/models/relay_status.dart(1 hunks)lib/ui/auth_flow/qr_scanner_bottom_sheet.dart(1 hunks)lib/ui/chat/states/chat_input_state.dart(1 hunks)lib/ui/chat/states/chat_input_state.freezed.dart(1 hunks)lib/ui/chat/widgets/chat_input.dart(5 hunks)lib/ui/chat/widgets/chat_input_media_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_media_selector.dart(1 hunks)lib/ui/chat/widgets/chat_input_reply_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_send_button.dart(1 hunks)lib/ui/chat/widgets/media_thumbnail.dart(1 hunks)lib/ui/core/themes/assets.dart(1 hunks)lib/ui/core/ui/wn_text_form_field.dart(3 hunks)lib/ui/settings/general_settings_screen.dart(1 hunks)test/config/providers/chat_input_provider_test.dart(1 hunks)test/domain/services/draft_message_service_test.dart(13 hunks)test/domain/services/image_picker_service_test.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- lib/ui/auth_flow/qr_scanner_bottom_sheet.dart
- lib/locales/tr.json
- lib/ui/core/ui/wn_text_form_field.dart
- lib/ui/chat/states/chat_input_state.freezed.dart
- lib/ui/settings/general_settings_screen.dart
- lib/locales/ru.json
- lib/ui/chat/widgets/chat_input_send_button.dart
- lib/locales/fr.json
- lib/locales/en.json
- lib/ui/chat/states/chat_input_state.dart
- lib/config/providers/chat_input_provider.dart
- lib/locales/es.json
- lib/locales/de.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
test/domain/services/image_picker_service_test.darttest/domain/services/draft_message_service_test.dartlib/models/relay_status.dartlib/ui/chat/widgets/chat_input_reply_preview.dartlib/ui/chat/widgets/chat_input_media_selector.dartlib/ui/chat/widgets/chat_input_media_preview.dartlib/domain/services/image_picker_service.dartlib/domain/services/draft_message_service.dartlib/ui/chat/widgets/media_thumbnail.darttest/config/providers/chat_input_provider_test.dartlib/ui/core/themes/assets.dartlib/ui/chat/widgets/chat_input.dart
test/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
test/**/*.dart: Follow Arrange-Act-Assert in tests
Name test variables clearly using inputX, mockX, actualX, expectedX
Write unit tests for each public function; use test doubles for dependencies (except cheap third-party)
Use standard Flutter widget testing
Files:
test/domain/services/image_picker_service_test.darttest/domain/services/draft_message_service_test.darttest/config/providers/chat_input_provider_test.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/models/relay_status.dartlib/ui/chat/widgets/chat_input_reply_preview.dartlib/ui/chat/widgets/chat_input_media_selector.dartlib/ui/chat/widgets/chat_input_media_preview.dartlib/domain/services/image_picker_service.dartlib/domain/services/draft_message_service.dartlib/ui/chat/widgets/media_thumbnail.dartlib/ui/core/themes/assets.dartlib/ui/chat/widgets/chat_input.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 (38)
lib/domain/services/image_picker_service.dart (1)
26-39: LGTM! Multi-image picker implementation is solid.The new
pickMultipleImagesmethod correctly mirrors the structure ofpickProfileImagewith appropriate error handling and logging. The higher resolution constraints (1920x1920) are suitable for chat media versus the lower resolution for profile images (512x512).test/domain/services/image_picker_service_test.dart (2)
23-39: LGTM! Consistent mock setup formatting.The reformatted
when()wrapping is consistent and improves readability without changing behavior.
77-121: LGTM! Comprehensive test coverage for multi-image picking.The new test group covers all essential scenarios: single image selection, multiple image selection, and error handling. The test structure follows the AAA pattern and aligns with the coding guidelines.
lib/domain/services/draft_message_service.dart (4)
7-23: LGTM! Instance method conversion enables better testability.Converting
saveDraftfrom static to instance method aligns with the PR objective to make the service easier to mock in tests and supports dependency injection patterns.
25-36: LGTM! Instance method conversion is consistent.The
loadDraftmethod conversion maintains identical behavior while enabling better testability.
38-49: LGTM! Instance method conversion is consistent.The
clearDraftmethod conversion maintains identical behavior while enabling better testability.
51-63: LGTM! Instance method conversion is consistent.The
clearAllDraftsmethod conversion maintains identical behavior while enabling better testability.lib/ui/core/themes/assets.dart (1)
6-82: LGTM! Icon constants expanded and organized.The new icon constants are well-organized and follow the existing naming convention. The expanded icon surface supports the new media upload UI components introduced in this PR.
lib/models/relay_status.dart (1)
73-73: LGTM! Icon reference updated to match renamed constant.The change from
icCheckmarkFilledSvgtoicCheckmarkFilledaligns with the icon reorganization inassets.dart.lib/locales/pt.json (1)
203-204: LGTM! Portuguese translation added for photos feature.The new "photos" translation entry is correctly placed and provides localization for the new image selection UI.
lib/locales/it.json (1)
203-204: LGTM! Italian translation added for photos feature.The new "photos" translation entry is correctly placed and provides localization for the new image selection UI.
test/domain/services/draft_message_service_test.dart (5)
19-27: LGTM! Test updated to use instance methods.The test correctly instantiates
DraftMessageService()and calls the instance method, aligning with the service refactor.
36-44: LGTM! Test cases updated consistently.All test cases for
saveDraftcorrectly use the instance method pattern.Also applies to: 47-55, 67-75
86-92: LGTM! Test cases updated consistently.All test cases for
loadDraftcorrectly use the instance method pattern.Also applies to: 97-103, 115-121
133-140: LGTM! Test cases updated consistently.All test cases for
clearDraftcorrectly use the instance method pattern.Also applies to: 143-150, 155-162
175-179: LGTM! Test cases updated consistently.All test cases for
clearAllDraftscorrectly use the instance method pattern.Also applies to: 182-186, 191-195, 208-212, 215-219
lib/ui/chat/widgets/chat_input_media_selector.dart (2)
1-14: LGTM!The imports are well-organized, and the class declaration follows Flutter and project conventions. The constructor properly declares types and uses the
requiredkeyword for mandatory parameters.
55-96: LGTM!The private option widget is well-structured with proper tap handling via
InkWell, responsive sizing, and theme integration. The layout clearly presents the option with both label and icon.test/config/providers/chat_input_provider_test.dart (9)
9-49: LGTM!The mock implementations are well-structured, properly override the service methods, and provide configurable return values for testing. The
reset()method onMockDraftMessageServiceis a good practice for test isolation.
51-91: LGTM!The test setup properly isolates each test with fresh mocks and container instances. The initial state test thoroughly verifies all expected default values. Good use of
setUpandtearDownfor test hygiene.
93-116: LGTM!The media selector toggle and hide tests clearly verify the state transitions. Good coverage of both toggling on/off and explicit hiding.
118-171: LGTM!The image selection tests comprehensively cover various scenarios including empty selections, new selections, and appending to existing selections. Good use of nested groups to organize related test cases.
173-204: LGTM!The image removal tests properly cover edge cases including invalid and negative indices. Good verification that the operation only succeeds for valid indices.
206-228: LGTM!The clear tests thoroughly verify that all state fields are reset and that the draft service's
clearDraftmethod is invoked. Good practice resetting the mock before the test.
230-245: LGTM!The setter tests clearly verify that the state fields are updated correctly. Simple and effective.
247-266: LGTM!The draft loading tests cover both the null and non-null draft scenarios with clear assertions. Well-organized using nested groups.
268-303: LGTM!Excellent coverage of the draft scheduling and saving behavior. The tests properly verify timer debouncing, cancellation of previous scheduled saves, and the immediate save flow. Good use of
Future.delayedwith appropriate timing.lib/ui/chat/widgets/chat_input.dart (7)
39-63: LGTM!The lifecycle methods are properly structured.
initStatecorrectly registers listeners and schedules the height measurement.didUpdateWidgetproperly syncs the editing message content using the provider state, with a comparison to prevent unnecessary updates. Good state management pattern.
80-98: LGTM!The media selector toggle logic correctly unfocuses the input when opening the selector (to hide the keyboard), and the image handling methods properly delegate to the provider. The flow is well-structured.
100-109: LGTM!The single-line height measurement is properly implemented with safety checks for mounted state and context. The height is correctly stored via the provider for use by the send button.
119-159: LGTM!The draft loading and saving logic is well-structured with proper guards to prevent recursion (
isLoadingDraftcheck) and to avoid interfering with message editing mode. Good lifecycle handling with immediate save on app pause/inactive.
161-182: LGTM!The send logic correctly validates that either text content or selected images are present before sending. The state cleanup properly clears the provider state and cancels reply/edit modes. The validation at line 169 is forward-looking, allowing image-only messages in preparation for the upcoming media upload functionality.
184-195: LGTM!The build method structure is clean with appropriate
SafeAreausage and minimal column sizing. Good foundation for the animated layout.
195-312: LGTM!The widget composition is well-structured with proper integration of the new provider-backed widgets. The animations are smooth, and the conditional rendering (add button only when no images, media selector visibility) provides good UX. The padding adjustment based on media selector state creates a clean transition.
lib/ui/chat/widgets/chat_input_reply_preview.dart (1)
10-27: LGTM!The widget constructor and early return logic are well-structured. Proper handling of the optional message parameters with a clean early return when neither is present.
lib/ui/chat/widgets/chat_input_media_preview.dart (3)
42-63: LGTM!The thumbnail tap logic is intuitive: tap once to activate and scroll to the image, tap again to remove it. The scroll animation properly uses the extracted constants for position calculation. Clean implementation.
65-68: LGTM!Good optimization with the early return when no images are present.
92-129: LGTM!The thumbnail strip is well-implemented with the add button at the start followed by image thumbnails. Proper index adjustment and active state management. Clean integration of the
MediaThumbnailwidgets.
josefinalliende
force-pushed
the
feat/media-upload-ui
branch
from
a0f3052 to
fa70b6a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/ui/chat/widgets/chat_input_media_preview.dart (1)
83-89: Error handling could be more user-friendly.While an
errorBuilderwas added (line 88), it just returnsSizedBox.shrink(), causing failed images to silently disappear. Users won't know if an image failed to load or was removed.Consider showing a placeholder to indicate the error:
child: Image.file( File(imagePath), height: _imageHeight.h, width: _imageWidth.w, fit: BoxFit.cover, - errorBuilder: (context, error, stackTrace) => const SizedBox.shrink(), + errorBuilder: (context, error, stackTrace) => Container( + height: _imageHeight.h, + width: _imageWidth.w, + color: context.colors.mutedForeground.withValues(alpha: 0.1), + child: Center( + child: Icon( + Icons.broken_image, + size: 48.w, + color: context.colors.mutedForeground, + ), + ), + ), ),
🧹 Nitpick comments (1)
lib/ui/chat/widgets/chat_input_media_preview.dart (1)
27-54: Consider making active state more robust to image list changes.The
_activeThumbIndexcould become stale ifimagePathsis modified by the parent (e.g., images added/removed from outside this widget). Currently, the widget only clears_activeThumbIndexwhen removing via_handleThumbnailTap.Consider adding
didUpdateWidgetto validate or clear the active index whenimagePathschanges:@override void didUpdateWidget(ChatInputMediaPreview oldWidget) { super.didUpdateWidget(oldWidget); if (widget.imagePaths != oldWidget.imagePaths) { // Clear active state if the list changed if (_activeThumbIndex != null && _activeThumbIndex! >= widget.imagePaths.length) { setState(() { _activeThumbIndex = null; }); } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
lib/ui/chat/widgets/chat_input.dart(5 hunks)lib/ui/chat/widgets/chat_input_media_preview.dart(1 hunks)lib/ui/chat/widgets/media_thumbnail.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ui/chat/widgets/media_thumbnail.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 dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/ui/chat/widgets/chat_input.dartlib/ui/chat/widgets/chat_input_media_preview.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/chat/widgets/chat_input.dartlib/ui/chat/widgets/chat_input_media_preview.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/chat/widgets/chat_input.dart (2)
161-182: LGTM! Good improvement for image-only messages.The send logic now correctly allows sending messages with images even when text content is empty (line 169), and properly clears the provider state after sending (line 174).
184-313: LGTM! Well-structured provider-driven UI.The build method has been successfully refactored to use provider state and new modular widgets. The layout properly responds to media selector visibility, focus state, and selected images. The animations enhance the user experience.
lib/ui/chat/widgets/chat_input_media_preview.dart (1)
65-131: LGTM! Clean widget structure.The build method has a clear layout with the main image gallery and thumbnail strip properly positioned. The use of constants for dimensions (addressed from past review) makes the code maintainable.
| import 'package:whitenoise/domain/services/image_picker_service.dart'; | ||
| import 'package:whitenoise/ui/chat/states/chat_input_state.dart'; | ||
|
|
||
| class ChatInputNotifier extends StateNotifier<ChatInputState> { |
There was a problem hiding this comment.
Let's always prefer Notifiers/NotifierProviders (2.0+) over StateNotifiers/StateNotifierProviders (1.x).
There was a problem hiding this comment.
After rebase now the sha changed... f089764
| width: 32.w, | ||
| height: 32.h, | ||
| decoration: BoxDecoration( | ||
| color: Colors.black.withValues(alpha: 0.5), |
There was a problem hiding this comment.
Prefer to use the color from context so it'll behave well with light/dark mode
There was a problem hiding this comment.
same, after rebase sha changed to aa120b1
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/config/providers/chat_input_provider.dart (1)
68-78: Optional: Simplify redundant state update.Both branches set
showMediaSelector: false, so the else block is redundant.Apply this diff to simplify:
Future<void> handleImagesSelected() async { final imagePaths = await _imagePickerService.pickMultipleImages(); - if (imagePaths.isNotEmpty) { - state = state.copyWith( - showMediaSelector: false, - selectedImages: [...state.selectedImages, ...imagePaths], - ); - } else { - state = state.copyWith(showMediaSelector: false); - } + state = state.copyWith( + showMediaSelector: false, + selectedImages: imagePaths.isNotEmpty + ? [...state.selectedImages, ...imagePaths] + : state.selectedImages, + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/config/providers/chat_input_provider.dart(1 hunks)lib/ui/chat/widgets/media_thumbnail.dart(1 hunks)test/config/providers/chat_input_provider_test.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/ui/chat/widgets/media_thumbnail.dart
- test/config/providers/chat_input_provider_test.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 dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/config/providers/chat_input_provider.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/config/providers/chat_input_provider.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 (6)
lib/config/providers/chat_input_provider.dart (6)
8-15: LGTM! Clean dependency injection design.The use of
FamilyNotifierwith injected dependencies follows Riverpod 2.0+ best practices and makes testing straightforward.
24-31: LGTM! Proper lifecycle management.The timer cleanup in
onDisposeprevents resource leaks, and the use ofconstfor the initial state is efficient.
80-88: LGTM! Defensive bounds checking with logging.The bounds check prevents out-of-range errors, and the warning log aids debugging. Creating a new list maintains immutability.
90-94: LGTM! Optimization to prevent unnecessary rebuilds.The equality check before updating state avoids triggering listeners when the value hasn't changed.
112-114: LGTM! Follows Riverpod 2.0+ best practices.The use of
NotifierProvider.familyaligns with the preference stated in past review comments for Notifier/NotifierProvider (2.0+) over StateNotifier/StateNotifierProvider (1.x).
100-109: No action required—error handling already exists at the service layer.The
DraftMessageService.clearDraftmethod (lines 43–47 inlib/domain/services/draft_message_service.dart) wraps the storage deletion in a try-catch block and silently returns on failure. This design ensures the provider'sclear()method will always complete normally, allowing the state reset to proceed. Error handling is delegated to the service as intended.
| state = state.copyWith(showMediaSelector: !state.showMediaSelector); | ||
| } | ||
|
|
||
| Future<void> handleImagesSelected() async { |
There was a problem hiding this comment.
Consider adding error handling incase something goes wrong
There was a problem hiding this comment.
There is in the builder if the UI, if an error happens, the image is not shown. Already checked that case in my iphone by hardcoding local paths that don't exist and it works fine
There was a problem hiding this comment.
errorBuilder in ChatInputMediaPreview and in MediaThumbnail
There was a problem hiding this comment.
But now giving a second thought, something else could go wrong before getting to the widget right? I will add error handling here. Thanks
josefinalliende
force-pushed
the
feat/media-upload-ui
branch
from
706e8bf to
c28a784
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
lib/ui/chat/widgets/chat_input_media_preview.dart (1)
83-90: Optional: show a fixed-size placeholder instead of shrinking on load error.Shrinking collapses layout; a placeholder preserves scroll positions/spacing. You previously chose shrink—keeping this as optional.
🧹 Nitpick comments (14)
test/domain/services/image_picker_service_test.dart (1)
77-121: Good test coverage for the new method. Consider adding an empty list scenario.The tests for
pickMultipleImagescover the essential scenarios well. However, consider adding a test case for when the user cancels the picker and no images are selected (empty list return), which mirrors the "no image picked" scenario in thepickProfileImagetests.Add this test group after line 91:
group('when no images are picked', () { setUp(() { when( mockImagePicker.pickMultiImage(maxWidth: 1920, maxHeight: 1920, imageQuality: 85), ).thenAnswer((_) async => []); }); test('returns empty list', () async { final result = await service.pickMultipleImages(); expect(result, isEmpty); }); });test/domain/services/draft_message_service_test.dart (1)
198-219: Add explicit assertion for thedraft_message_empty-suffix key.You already cover “exact prefix” vs “similar but invalid” keys. Add a check that the
draft_message_key is also removed to lock behavior down.test('removes keys with exact prefix', () async { await DraftMessageService().clearAllDrafts(storage: storage); final cleared = await storage.read(key: 'draft_message_valid_chat_id'); expect(cleared, isNull); + final clearedEmptySuffix = await storage.read(key: 'draft_message_'); + expect(clearedEmptySuffix, isNull); });test/config/providers/chat_input_provider_test.dart (3)
85-94: Dispose the extra ProviderContainer to avoid leaks.Add disposal to keep tests tidy and prevent hidden side effects.
- test('has expected initial state', () { - final testContainer = ProviderContainer(); + test('has expected initial state', () { + final testContainer = ProviderContainer(); final state = testContainer.read(chatInputProvider('test-group-123')); expect(state, isA<ChatInputState>()); expect(state.isLoadingDraft, false); expect(state.showMediaSelector, false); expect(state.selectedImages, isEmpty); expect(state.singleLineHeight, isNull); expect(state.previousEditingMessageContent, isNull); + testContainer.dispose(); });
1-8: Make timer-based tests deterministic (avoid 5–10ms delays).Use fake_async to elapse the scheduled delay reliably and prevent CI flakiness.
@@ -import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:fake_async/fake_async.dart'; @@ - test('saves draft after delay', () async { - mockDraftMessageService.reset(); - notifier.scheduleDraftSave('test draft'); - expect(mockDraftMessageService.savedDrafts, isEmpty); - - await Future.delayed(const Duration(milliseconds: 10)); - expect(mockDraftMessageService.savedDrafts, ['test draft']); - }); + test('saves draft after delay', () { + mockDraftMessageService.reset(); + fakeAsync((async) { + notifier.scheduleDraftSave('test draft'); + expect(mockDraftMessageService.savedDrafts, isEmpty); + async.elapse(testDraftSaveDelay); + expect(mockDraftMessageService.savedDrafts, ['test draft']); + }); + }); @@ - test('cancels previous timer when called multiple times', () async { - mockDraftMessageService.reset(); - notifier.scheduleDraftSave('first draft'); - notifier.scheduleDraftSave('second draft'); - - await Future.delayed(const Duration(milliseconds: 10)); - expect(mockDraftMessageService.savedDrafts, ['second draft']); - }); + test('cancels previous timer when called multiple times', () { + mockDraftMessageService.reset(); + fakeAsync((async) { + notifier.scheduleDraftSave('first draft'); + notifier.scheduleDraftSave('second draft'); + async.elapse(testDraftSaveDelay); + expect(mockDraftMessageService.savedDrafts, ['second draft']); + }); + }); @@ - test('cancels pending scheduled save', () async { - mockDraftMessageService.reset(); - notifier.scheduleDraftSave('scheduled draft'); - await notifier.saveDraftImmediately('immediate draft'); - - await Future.delayed(const Duration(milliseconds: 10)); - expect(mockDraftMessageService.savedDrafts, ['immediate draft']); - }); + test('cancels pending scheduled save', () { + mockDraftMessageService.reset(); + fakeAsync((async) { + notifier.scheduleDraftSave('scheduled draft'); + // immediate save should cancel the pending timer + async.run((_) async => await notifier.saveDraftImmediately('immediate draft')); + async.elapse(testDraftSaveDelay); + expect(mockDraftMessageService.savedDrafts, ['immediate draft']); + }); + });Also applies to: 302-319, 329-336
22-41: Capture and assert chatIds in mocks to verify correct routing.Record
chatIdin the mock and assert it in save/load/clear interactions.class MockDraftMessageService extends DraftMessageService { String? draftToReturn; final List<String> savedDrafts = []; final List<String> clearedChats = []; + final List<String> savedChatIds = []; @@ @override Future<void> saveDraft({ required String chatId, required String message, FlutterSecureStorage? storage, }) async { savedDrafts.add(message); + savedChatIds.add(chatId); draftToReturn = message; } @@ - test('saves draft after delay', () async { + test('saves draft after delay', () async { mockDraftMessageService.reset(); notifier.scheduleDraftSave('test draft'); expect(mockDraftMessageService.savedDrafts, isEmpty); await Future.delayed(const Duration(milliseconds: 10)); expect(mockDraftMessageService.savedDrafts, ['test draft']); + expect(mockDraftMessageService.savedChatIds, [testGroupId]); });Also applies to: 302-310
lib/ui/chat/widgets/chat_input_media_selector.dart (1)
66-95: Improve accessibility with explicit semantics.Expose the tap target as a button to screen readers.
@override Widget build(BuildContext context) { - return InkWell( - onTap: onTap, - child: Container( + return Semantics( + button: true, + label: label, + child: InkWell( + onTap: onTap, + child: Container( width: double.infinity, padding: EdgeInsets.symmetric(horizontal: 16.w, vertical: 16.h), decoration: BoxDecoration( color: context.colors.surface, ), child: Row( @@ - ), - ), + ), + ), ); }lib/domain/services/draft_message_service.dart (2)
7-23: LGTM on instance migration; prefer DI for storage.Consider injecting storage via constructor to simplify call sites and testing (still allow overrides per call if needed).
-class DraftMessageService { - static const FlutterSecureStorage _defaultStorage = FlutterSecureStorage(); +class DraftMessageService { + const DraftMessageService({FlutterSecureStorage? storage}) + : _storage = storage ?? const FlutterSecureStorage(); static const String _draftPrefix = 'draft_message_'; + final FlutterSecureStorage _storage; @@ - final secureStorage = storage ?? _defaultStorage; + final secureStorage = storage ?? _storage;
25-36: Avoid swallowing exceptions silently; add lightweight logging.Catching and returning is fine for UX, but add debug logging to aid diagnostics.
@@ - } catch (e) { - return; + } catch (e, st) { + assert(() { + // Visible in debug builds only + // ignore: avoid_print + print('DraftMessageService.saveDraft error: $e\n$st'); + return true; + }()); + return; } @@ - } catch (e) { - return null; + } catch (e, st) { + assert(() { + // ignore: avoid_print + print('DraftMessageService.loadDraft error: $e\n$st'); + return true; + }()); + return null; } @@ - } catch (e) { - return; + } catch (e, st) { + assert(() { + // ignore: avoid_print + print('DraftMessageService.clearDraft error: $e\n$st'); + return true; + }()); + return; } @@ - } catch (e) { - return; + } catch (e, st) { + assert(() { + // ignore: avoid_print + print('DraftMessageService.clearAllDrafts error: $e\n$st'); + return true; + }()); + return; }Also applies to: 38-49, 51-63
lib/ui/chat/widgets/chat_input_media_preview.dart (2)
33-35: Prefer explicit type annotation for private field.Declare the type to follow the “always declare types” guideline.
- final _scrollController = ScrollController(); + final ScrollController _scrollController = ScrollController();
80-92: Stabilize list items with keys to preserve state on insert/remove.Keys avoid rebuild artifacts when removing images or reordering.
- return ClipRRect( + return ClipRRect( + key: ValueKey(imagePath), child: Image.file( File(imagePath), height: _imageHeight.h, width: _imageWidth.w, fit: BoxFit.cover, errorBuilder: (context, error, stackTrace) => const SizedBox.shrink(), ), );- return MediaThumbnail( + return MediaThumbnail( + key: ValueKey(imagePath), path: imagePath, isActive: _activeThumbIndex == imageIndex, onTap: () => _handleThumbnailTap(imageIndex), );Also applies to: 116-121
lib/ui/chat/widgets/chat_input.dart (3)
35-37: Add explicit types for controllers and key.Follow the “always declare types” rule for clarity and consistency.
- final _textController = TextEditingController(); - final _focusNode = FocusNode(); - final _inputKey = GlobalKey(); + final TextEditingController _textController = TextEditingController(); + final FocusNode _focusNode = FocusNode(); + final GlobalKey _inputKey = GlobalKey();
51-63: React to editingMessage changes with ref.listen instead of didUpdateWidget.didUpdateWidget won’t fire on provider state changes; listen to the provider and sync the text controller.
@override - void didUpdateWidget(ChatInput oldWidget) { - super.didUpdateWidget(oldWidget); - final editingMessage = ref.read(chatProvider).editingMessage[widget.groupId]; - final chatInputNotifier = ref.read(chatInputProvider(widget.groupId).notifier); - final chatInputState = ref.read(chatInputProvider(widget.groupId)); - - if (editingMessage != null && - editingMessage.content != chatInputState.previousEditingMessageContent) { - chatInputNotifier.setPreviousEditingMessageContent(editingMessage.content); - _textController.text = editingMessage.content ?? ''; - } - } + void didUpdateWidget(ChatInput oldWidget) { + super.didUpdateWidget(oldWidget); + // No-op: handled via ref.listen in initState. + }Add in initState (after existing callbacks):
@override void initState() { super.initState(); WidgetsBinding.instance.addObserver(this); _loadDraftMessage(); _focusNode.addListener(_handleFocusChange); _textController.addListener(_onTextChanged); WidgetsBinding.instance.addPostFrameCallback((_) { _measureSingleLineHeight(); }); + ref.listen( + chatProvider, + (prev, next) { + final editingMessage = next.editingMessage[widget.groupId]; + if (editingMessage == null) return; + final content = editingMessage.content; + final chatInputState = ref.read(chatInputProvider(widget.groupId)); + if (content != chatInputState.previousEditingMessageContent) { + ref.read(chatInputProvider(widget.groupId).notifier) + .setPreviousEditingMessageContent(content); + _textController.text = content ?? ''; + } + }, + ); }
100-108: Recompute single-line height on metrics/orientation changes.Height may change with orientation or text scale; re-measure in didChangeMetrics.
class _ChatInputState extends ConsumerState<ChatInput> with WidgetsBindingObserver { + @override + void didChangeMetrics() { + super.didChangeMetrics(); + WidgetsBinding.instance.addPostFrameCallback((_) => _measureSingleLineHeight()); + }lib/ui/core/themes/assets.dart (1)
1-83: Naming consistency (optional): drop “Svg” suffix for constant names.Consider renaming icWhiteNoiseSvg → icWhiteNoise to match the rest of the API. Low priority unless you plan bulk renames.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
assets/svgs/ic_image.svgis excluded by!**/*.svgios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
lib/config/providers/chat_input_provider.dart(1 hunks)lib/domain/services/draft_message_service.dart(4 hunks)lib/domain/services/image_picker_service.dart(1 hunks)lib/locales/de.json(1 hunks)lib/locales/en.json(1 hunks)lib/locales/es.json(1 hunks)lib/locales/fr.json(1 hunks)lib/locales/it.json(1 hunks)lib/locales/pt.json(1 hunks)lib/locales/ru.json(1 hunks)lib/locales/tr.json(1 hunks)lib/models/relay_status.dart(1 hunks)lib/ui/auth_flow/qr_scanner_bottom_sheet.dart(1 hunks)lib/ui/chat/states/chat_input_state.dart(1 hunks)lib/ui/chat/states/chat_input_state.freezed.dart(1 hunks)lib/ui/chat/widgets/chat_input.dart(5 hunks)lib/ui/chat/widgets/chat_input_media_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_media_selector.dart(1 hunks)lib/ui/chat/widgets/chat_input_reply_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_send_button.dart(1 hunks)lib/ui/chat/widgets/media_thumbnail.dart(1 hunks)lib/ui/core/themes/assets.dart(1 hunks)lib/ui/core/ui/wn_text_form_field.dart(3 hunks)lib/ui/settings/general_settings_screen.dart(1 hunks)test/config/providers/chat_input_provider_test.dart(1 hunks)test/domain/services/draft_message_service_test.dart(13 hunks)test/domain/services/image_picker_service_test.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- lib/locales/en.json
- lib/locales/it.json
- lib/ui/core/ui/wn_text_form_field.dart
- lib/ui/settings/general_settings_screen.dart
- lib/config/providers/chat_input_provider.dart
- lib/ui/chat/states/chat_input_state.dart
- lib/ui/chat/widgets/chat_input_send_button.dart
- lib/locales/ru.json
- lib/locales/tr.json
- lib/ui/chat/widgets/chat_input_reply_preview.dart
- lib/locales/pt.json
- lib/locales/fr.json
- lib/domain/services/image_picker_service.dart
- lib/ui/auth_flow/qr_scanner_bottom_sheet.dart
- lib/ui/chat/widgets/media_thumbnail.dart
- lib/locales/de.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
test/domain/services/image_picker_service_test.dartlib/domain/services/draft_message_service.dartlib/ui/chat/states/chat_input_state.freezed.darttest/config/providers/chat_input_provider_test.dartlib/ui/chat/widgets/chat_input_media_selector.dartlib/ui/chat/widgets/chat_input.darttest/domain/services/draft_message_service_test.dartlib/models/relay_status.dartlib/ui/chat/widgets/chat_input_media_preview.dartlib/ui/core/themes/assets.dart
test/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
test/**/*.dart: Follow Arrange-Act-Assert in tests
Name test variables clearly using inputX, mockX, actualX, expectedX
Write unit tests for each public function; use test doubles for dependencies (except cheap third-party)
Use standard Flutter widget testing
Files:
test/domain/services/image_picker_service_test.darttest/config/providers/chat_input_provider_test.darttest/domain/services/draft_message_service_test.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/domain/services/draft_message_service.dartlib/ui/chat/states/chat_input_state.freezed.dartlib/ui/chat/widgets/chat_input_media_selector.dartlib/ui/chat/widgets/chat_input.dartlib/models/relay_status.dartlib/ui/chat/widgets/chat_input_media_preview.dartlib/ui/core/themes/assets.dart
🧠 Learnings (2)
📚 Learning: 2025-10-12T22:44:49.606Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-10-12T22:44:49.606Z
Learning: Applies to lib/**/*.dart : Use freezed to model/manage UI states
Applied to files:
lib/ui/chat/states/chat_input_state.freezed.dart
📚 Learning: 2025-10-15T02:08:41.843Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#721
File: lib/ui/chat/widgets/chat_input.dart:80-87
Timestamp: 2025-10-15T02:08:41.843Z
Learning: In `lib/ui/chat/widgets/chat_input.dart`, the `_toggleMediaSelector()` method correctly reads `chatInputState` before toggling and checks `!chatInputState.showMediaSelector` to unfocus when opening the selector. This matches the design requirement where the input should be unfocused when the media selector is shown.
Applied to files:
lib/ui/chat/widgets/chat_input_media_selector.dartlib/ui/chat/widgets/chat_input.dartlib/ui/chat/widgets/chat_input_media_preview.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 (6)
lib/locales/es.json (1)
203-204: Localization addition looks good.Spanish translation "Fotos" is correct and appropriately placed in the chats section to support the new image selection UI. JSON syntax is valid with proper trailing comma handling.
test/domain/services/image_picker_service_test.dart (1)
20-75: LGTM! Well-structured tests with good coverage.The refactored mock setup using wrapped
when(...)style improves readability. The three test scenarios (image picked, no image picked, picker fails) provide comprehensive coverage for thepickProfileImagemethod.test/domain/services/draft_message_service_test.dart (1)
19-27: Nice migration to instance methods; tests read clearly and follow AAA.Coverage looks solid across save/load/clear/clearAll, including whitespace and special chars. No blocking issues.
Also applies to: 35-41, 66-75, 86-92, 97-103, 115-121, 133-140, 143-150, 155-162, 175-186, 191-195, 208-219
lib/models/relay_status.dart (1)
72-74: Asset rename verification passed — no issues found.The new constant
AssetsPaths.icCheckmarkFilledis properly defined inlib/ui/core/themes/assets.dart:16and the oldicCheckmarkFilledSvghas been completely removed. All usages across the codebase (inrelay_status.dart,qr_scanner_bottom_sheet.dart, andmessage_model.dart) correctly reference the new constant. No runtime missing-asset errors will occur.lib/ui/chat/states/chat_input_state.freezed.dart (1)
1-277: Generated Freezed state looks good.Immutability, unmodifiable list view, and copyWith are correctly generated.
lib/ui/core/themes/assets.dart (1)
54-57: Constant correctly references the existing asset file.Verification confirms
ic_mute_notfication.svgexists atassets/svgs/and the constant declaration matches the actual filename. No mismatch between the constant and the asset file.
…sier to mock in tests
josefinalliende
force-pushed
the
feat/media-upload-ui
branch
from
c28a784 to
561a675
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/ui/core/themes/assets.dart (1)
6-82: Consider completing alphabetical sorting for consistency.While most icons are sorted, a few are out of order:
icAdd(line 8) should come beforeicAddNewChat(line 6)icChat(line 15) should come beforeicChatInvite(line 14)icCheckmark(line 18) should come beforeicCheckmarkFilled(line 16)icEye(line 35) should come beforeicEyeOff(line 34)icUser(line 77) should come beforeicUserFollow(line 76)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
assets/svgs/ic_image.svgis excluded by!**/*.svgios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
lib/config/providers/chat_input_provider.dart(1 hunks)lib/domain/services/draft_message_service.dart(4 hunks)lib/domain/services/image_picker_service.dart(1 hunks)lib/locales/de.json(1 hunks)lib/locales/en.json(1 hunks)lib/locales/es.json(1 hunks)lib/locales/fr.json(1 hunks)lib/locales/it.json(1 hunks)lib/locales/pt.json(1 hunks)lib/locales/ru.json(1 hunks)lib/locales/tr.json(1 hunks)lib/models/relay_status.dart(1 hunks)lib/ui/auth_flow/qr_scanner_bottom_sheet.dart(1 hunks)lib/ui/chat/states/chat_input_state.dart(1 hunks)lib/ui/chat/states/chat_input_state.freezed.dart(1 hunks)lib/ui/chat/widgets/chat_input.dart(5 hunks)lib/ui/chat/widgets/chat_input_media_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_media_selector.dart(1 hunks)lib/ui/chat/widgets/chat_input_reply_preview.dart(1 hunks)lib/ui/chat/widgets/chat_input_send_button.dart(1 hunks)lib/ui/chat/widgets/media_thumbnail.dart(1 hunks)lib/ui/core/themes/assets.dart(1 hunks)lib/ui/core/ui/wn_text_form_field.dart(3 hunks)lib/ui/settings/general_settings_screen.dart(1 hunks)test/config/providers/chat_input_provider_test.dart(1 hunks)test/domain/services/draft_message_service_test.dart(13 hunks)test/domain/services/image_picker_service_test.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- lib/locales/fr.json
- lib/locales/it.json
- lib/domain/services/image_picker_service.dart
- lib/locales/es.json
- lib/locales/de.json
- lib/locales/tr.json
- lib/models/relay_status.dart
- lib/locales/ru.json
- lib/ui/chat/widgets/media_thumbnail.dart
- lib/ui/chat/widgets/chat_input_reply_preview.dart
- test/domain/services/image_picker_service_test.dart
- lib/ui/auth_flow/qr_scanner_bottom_sheet.dart
- lib/ui/chat/widgets/chat_input_media_selector.dart
- lib/ui/chat/widgets/chat_input.dart
- lib/ui/chat/widgets/chat_input_media_preview.dart
- lib/ui/settings/general_settings_screen.dart
🧰 Additional context used
📓 Path-based instructions (3)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value)
Avoid using dynamic and Object without justification
Create necessary types instead of overusing primitives or dynamic
One export per file
Use PascalCase for classes
Use camelCase for variables, functions, and methods
Avoid magic numbers and define constants
Start each function name with a verb
Use verbs for boolean variables (e.g., isLoading, hasError, canDelete)
Write short functions with a single purpose (under ~20 instructions)
Name functions with a verb plus context; for booleans use isX/hasX/canX; for void use executeX/saveX
Avoid deep nesting via early returns and extraction to utility functions
Use higher-order functions (map, where/filter, reduce) to avoid nesting
Use arrow functions for simple functions (under ~3 statements); use named functions otherwise
Use default parameter values instead of null checks
Reduce function parameters using RO-RO: pass/return parameter objects with declared types
Maintain a single level of abstraction within functions
Encapsulate data in composite types; avoid overusing primitives
Prefer validating data within classes rather than in functions
Prefer immutability; use final for runtime constants and const for compile-time constants
Use const constructors and const literals where possible
Follow SOLID principles
Prefer composition over inheritance
Declare interfaces (abstract classes) to define contracts
Write small classes with a single purpose (under ~200 instructions, <10 public methods, <10 properties)
Use exceptions for unexpected errors
Only catch exceptions to fix expected problems or add context; otherwise use a global handler
Files:
lib/ui/core/ui/wn_text_form_field.dartlib/domain/services/draft_message_service.dartlib/ui/chat/states/chat_input_state.dartlib/ui/chat/states/chat_input_state.freezed.darttest/config/providers/chat_input_provider_test.dartlib/ui/chat/widgets/chat_input_send_button.dartlib/config/providers/chat_input_provider.dartlib/ui/core/themes/assets.darttest/domain/services/draft_message_service_test.dart
lib/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
lib/**/*.dart: Use flutter_rust_bridge to access core app functionality
Use Riverpod for state management; prefer StreamProviders for Rust API streams; use keepAlive if needed
Use freezed to model/manage UI states
Controllers should expose methods as inputs and update UI state that drives the UI
Use AutoRoute for navigation and use extras to pass data between pages
Use Dart extensions to manage reusable code
Use ThemeData to manage themes
Use AppLocalizations for translations
Use constants to manage constant values
Avoid deeply nested widget trees; aim for a flatter widget structure for performance and readability
Break down large widgets into smaller, focused, reusable components
Keep the widget tree shallow to simplify state management and data flow
Utilize const constructors and const widgets wherever possible to reduce rebuilds
Files:
lib/ui/core/ui/wn_text_form_field.dartlib/domain/services/draft_message_service.dartlib/ui/chat/states/chat_input_state.dartlib/ui/chat/states/chat_input_state.freezed.dartlib/ui/chat/widgets/chat_input_send_button.dartlib/config/providers/chat_input_provider.dartlib/ui/core/themes/assets.dart
test/**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
test/**/*.dart: Follow Arrange-Act-Assert in tests
Name test variables clearly using inputX, mockX, actualX, expectedX
Write unit tests for each public function; use test doubles for dependencies (except cheap third-party)
Use standard Flutter widget testing
Files:
test/config/providers/chat_input_provider_test.darttest/domain/services/draft_message_service_test.dart
🧠 Learnings (4)
📚 Learning: 2025-10-12T22:44:49.606Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-10-12T22:44:49.606Z
Learning: Applies to lib/**/*.dart : Use freezed to model/manage UI states
Applied to files:
lib/ui/chat/states/chat_input_state.dartlib/ui/chat/states/chat_input_state.freezed.dart
📚 Learning: 2025-10-15T02:08:41.843Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#721
File: lib/ui/chat/widgets/chat_input.dart:80-87
Timestamp: 2025-10-15T02:08:41.843Z
Learning: In `lib/ui/chat/widgets/chat_input.dart`, the `_toggleMediaSelector()` method correctly reads `chatInputState` before toggling and checks `!chatInputState.showMediaSelector` to unfocus when opening the selector. This matches the design requirement where the input should be unfocused when the media selector is shown.
Applied to files:
lib/ui/chat/states/chat_input_state.dartlib/ui/chat/widgets/chat_input_send_button.dart
📚 Learning: 2025-09-07T02:15:31.931Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.931Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/config/providers/chat_input_provider.dart
📚 Learning: 2025-09-07T02:08:09.199Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/domain/models/contact_model.dart:36-41
Timestamp: 2025-09-07T02:08:09.199Z
Learning: The issue of ContactModel.publicKey potentially being set to an empty string when pubkey.toNpub() conversion fails (causing collision/lookup issues) is being addressed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/config/providers/chat_input_provider.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 (9)
lib/locales/en.json (1)
203-204: Localization addition looks good.The new "photos" translation key is appropriately placed in the "chats" section and aligns with the media selection UI component. Trailing comma on line 203 is correctly positioned to maintain valid JSON syntax. Coordinates well with companion locale file updates.
lib/locales/pt.json (1)
204-204: Translation addition is correct and properly formatted.The new "photos" entry is accurately translated, properly positioned in the chats section, and follows the established JSON structure. This aligns with the media selection UI feature and is consistent with corresponding entries in other locale files.
lib/ui/core/ui/wn_text_form_field.dart (1)
53-53: LGTM! Border hiding feature implemented correctly.The
isBorderHiddenparameter follows the verb naming convention and the conditional border logic is implemented correctly across all border properties (border, enabledBorder, focusedBorder).Also applies to: 88-88, 165-183
lib/ui/chat/states/chat_input_state.dart (1)
1-14: LGTM! Freezed state class is well-structured.The state class correctly uses Freezed for immutable UI state, all boolean fields follow verb naming conventions (isLoadingDraft, showMediaSelector), and fields are properly typed with appropriate defaults.
Based on learnings.
test/domain/services/draft_message_service_test.dart (1)
19-19: LGTM! Test updates correctly reflect API changes.All tests properly instantiate
DraftMessageService()and call instance methods, maintaining comprehensive coverage of draft persistence behavior including edge cases (empty strings, whitespace, special characters, multiple drafts).Also applies to: 36-36, 47-47, 67-67, 86-86, 97-97, 115-115, 133-133, 143-143, 155-155, 175-175, 182-182, 208-208, 215-215
test/config/providers/chat_input_provider_test.dart (1)
1-339: LGTM! Comprehensive and well-structured test suite.The test coverage is excellent, with clean mock implementations and thorough scenarios including:
- Initial state verification
- Media selector toggling and hiding
- Image selection with success, empty, error, and append scenarios
- Image removal with valid/invalid/negative indices
- State management (clear, setters)
- Draft persistence (load, scheduled save, immediate save with timer cancellation)
The test follows best practices: clear Arrange-Act-Assert structure, explicit type declarations, descriptive test names, and proper use of ProviderContainer overrides.
lib/config/providers/chat_input_provider.dart (1)
1-119: LGTM! Well-designed Riverpod Notifier with proper patterns.The implementation demonstrates best practices:
- Uses Riverpod 2.0
FamilyNotifierpattern (per past feedback)- Dependency injection via constructor for testability
- Proper resource cleanup (timer cancellation in onDispose)
- Bounds checking and logging in
removeImage()- Error handling with logging in
handleImagesSelected()- All methods follow verb naming convention
- Functions are short and focused (all under 20 instructions)
- Proper state immutability using
copyWithlib/domain/services/draft_message_service.dart (1)
7-7: LGTM! Instance method conversion enables better testability.Converting these methods from static to instance methods is an architectural improvement that:
- Enables dependency injection (as used in
ChatInputNotifier)- Facilitates mocking in tests (as seen in
MockDraftMessageService)- Maintains backward compatibility via default storage parameter
- Preserves all existing error handling and behavior
The change aligns with the provider-driven architecture introduced in this PR.
Also applies to: 25-25, 38-38, 51-51
lib/ui/chat/states/chat_input_state.freezed.dart (1)
1-277: LGTM! Standard Freezed generated boilerplate.This is properly generated Freezed code for the
ChatInputStatedata class. The implementation correctly provides:
- Immutable state with proper unmodifiable wrappers for collections (lines 192-194)
- Deep equality comparison for the
selectedImageslist (lines 216-218)- Type-safe
copyWithfunctionality with proper null-safety handling- Appropriate default values for all fields
As generated code, this file should not be manually modified and follows the expected Freezed patterns for state management.
Description
We want to send media in groups #707 . First step is to select an preview images in the chat input #722 . This PR adds the widgets to show images before sending them, according to this Figma
As the chat input widget was already loong and more logic related to images needed to be added, I refactored a bit the component, extracting logic to a tested provider 🧪 and also little widgets to its own files.
Next step: upload them to blossom server and send the message with the media tags.
Commit by commit:
short-select-images.mp4
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
Style
Chores
Tests