Patch create group #717
Patch create group #717
Conversation
Replaces local state and logic in GroupChatDetailsSheet with createGroupProvider for managing group creation, group name, and image upload. Moves contact filtering, error handling, and invite sheet logic into the provider. Updates UI to reflect provider state, including loading and error states, and enables group image selection and upload.
Introduces CreateGroupState using Freezed for state management and CreateGroupNotifier for handling group creation logic, including group name validation, image picking, contact filtering, and group creation. Adds the corresponding provider and generated Freezed code.
Introduced a discardChanges method in CreateGroupNotifier to reset group creation state. Updated GroupChatDetailsSheet to call discardChanges when the sheet is dismissed, ensuring temporary group data is cleared when the user exits.
Replaces direct usage of ImageUtils with wnImageUtilsProvider for obtaining the image MIME type in CreateGroupNotifier. This change improves dependency management and aligns with provider-based architecture.
Refactored the group image upload logic in CreateGroupNotifier to return the upload result and update the group's image data after a successful upload. The method now requires the active public key as a parameter and updates the group with the new image metadata if the upload succeeds. Removed an unused import.
…o patch-create-group
- Introduced method in to manage group description updates. - Updated to include and . - Refactored method to utilize the new group description. - Enhanced to capture and display group description input. - Updated to support multiple lines for group description. - Renamed to for consistency.
…bility and maintain consistency - Removed unnecessary line breaks in getter methods across various state classes. - Standardized formatting in copyWith methods for better clarity. - Ensured consistent use of single-line statements where applicable.
WalkthroughAdds a provider-driven group-creation flow: new CreateGroupState (Freezed) and CreateGroupNotifier with createGroupProvider; UI sheet refactor to use the provider (name/description, image picker, contact key-package filtering, create/upload flow, and error handling). Also adds WnTextField.maxLines, changes bottom-sheet color, and renames EditIconWidget → WnEditIconWidget. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as GroupDetailsSheet (UI)
participant Provider as CreateGroupNotifier
participant Img as ImagePickerService
participant Backend as Backend / Rust Bridge
User->>UI: Open group creation
UI->>Provider: init + filterContactsWithKeyPackage()
Provider->>Backend: classify contacts
Backend-->>Provider: classification result
Provider-->>UI: update contacts lists
User->>UI: Enter name/description, select members
UI->>Provider: updateGroupName/Description(...)
alt Pick image
User->>UI: Tap edit avatar
UI->>Provider: pickGroupImage()
Provider->>Img: pickImage()
Img-->>Provider: image path or error
Provider-->>UI: update selectedImagePath or error
end
User->>UI: Tap "Create"
UI->>Provider: createGroup(onGroupCreated)
Provider->>Backend: create group with contacts
Backend-->>Provider: group created
opt Image selected
Provider->>Backend: uploadGroupImage (requires pubkey)
Backend-->>Provider: imageKey/imageHash
Provider->>Backend: update group metadata
Backend-->>Provider: updated group record
end
Provider-->>UI: clear loading, invoke onGroupCreated
UI-->>User: navigate to new group chat
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ui/contact_list/group_chat_details_sheet.dart (1)
101-105: Dispose the description controllerLine 103:
_groupDescriptionControllerkeeps its listener and controller alive across sheet lifecycles, leaking memory and duplicating callbacks. Mirror the cleanup you already perform for the name controller.void dispose() { _groupNameController.removeListener(_onGroupNameChanged); _groupNameController.dispose(); + _groupDescriptionController.removeListener(_onGroupDescriptionChanged); + _groupDescriptionController.dispose(); super.dispose(); }
📜 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 (8)
lib/config/providers/create_group_provider.dart(1 hunks)lib/config/states/create_group_state.dart(1 hunks)lib/config/states/create_group_state.freezed.dart(1 hunks)lib/ui/contact_list/group_chat_details_sheet.dart(3 hunks)lib/ui/core/ui/wn_bottom_sheet.dart(1 hunks)lib/ui/core/ui/wn_text_field.dart(3 hunks)lib/ui/settings/profile/edit_profile_screen.dart(1 hunks)lib/ui/settings/profile/widgets/edit_icon.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/core/ui/wn_bottom_sheet.dartlib/ui/settings/profile/widgets/edit_icon.dartlib/ui/core/ui/wn_text_field.dartlib/config/states/create_group_state.freezed.dartlib/config/states/create_group_state.dartlib/ui/settings/profile/edit_profile_screen.dartlib/ui/contact_list/group_chat_details_sheet.dartlib/config/providers/create_group_provider.dart
🧠 Learnings (4)
📓 Common learnings
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.962Z
Learning: In the whitenoise_flutter codebase, the _updateGroupInfo method in GroupsNotifier performs optimistic updates that directly modify Group objects in the provider state (groups list and groupsMap). For non-DM groups, the display name comes from group.name, so updating the Group object directly is sufficient to reflect name changes in the UI without needing to refresh the separate display name cache.
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use freezed to manage UI states.
Applied to files:
lib/config/states/create_group_state.freezed.dart
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/ui/contact_list/group_chat_details_sheet.dart
📚 Learning: 2025-09-14T21:22:00.962Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.962Z
Learning: In the whitenoise_flutter codebase, the _updateGroupInfo method in GroupsNotifier performs optimistic updates that directly modify Group objects in the provider state (groups list and groupsMap). For non-DM groups, the display name comes from group.name, so updating the Group object directly is sufficient to reflect name changes in the UI without needing to refresh the separate display name cache.
Applied to files:
lib/config/providers/create_group_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 (4)
lib/ui/core/ui/wn_bottom_sheet.dart (1)
175-176: Neutral surface aligns with palette updateColor swap to
context.colors.neutralkeeps the sheet consistent with the refreshed theme and doesn’t affect behavior. Nice cleanup.lib/ui/settings/profile/edit_profile_screen.dart (1)
167-167: LGTM! Correctly updated widget usage.The widget reference has been properly updated to use the renamed
WnEditIconWidgetclass, maintaining consistency with the changes inedit_icon.dart.lib/ui/settings/profile/widgets/edit_icon.dart (1)
7-10: LGTM – renaming and type change approved
Renaming toWnEditIconWidgetfollows project conventions and usingVoidCallback?is more idiomatic. Verified no remaining references toEditIconWidget.lib/ui/contact_list/group_chat_details_sheet.dart (1)
111-133: Move provider listener out of buildRegistering
ref.listeninsidebuildmeans every rebuild adds another listener, so a single error or invite prompt will fan out into multiple toasts/bottom sheets. Set up the listener once ininitStateinstead.void initState() { super.initState(); _groupNameController.addListener(_onGroupNameChanged); _groupDescriptionController.addListener(_onGroupDescriptionChanged); WidgetsBinding.instance.addPostFrameCallback((_) { ref.read(createGroupProvider.notifier).filterContactsWithKeyPackage(widget.selectedContacts); }); + ref.listen(createGroupProvider, (previous, next) { + if (next.error != null) { + safeShowErrorToast(next.error!); + ref.read(createGroupProvider.notifier).clearError(); + } + + if (next.shouldShowInviteSheet && next.contactsWithoutKeyPackage.isNotEmpty) { + WidgetsBinding.instance.addPostFrameCallback((_) async { + if (mounted) { + try { + await ShareInviteBottomSheet.show( + context: context, + contacts: next.contactsWithoutKeyPackage, + ); + } catch (e) { + safeShowErrorToast('Failed to show share invite bottom sheet: $e'); + } finally { + ref.read(createGroupProvider.notifier).dismissInviteSheet(); + } + } + }); + } + }); } @@ - ref.listen(createGroupProvider, (previous, next) { - if (next.error != null) { - safeShowErrorToast(next.error!); - ref.read(createGroupProvider.notifier).clearError(); - } - - if (next.shouldShowInviteSheet && next.contactsWithoutKeyPackage.isNotEmpty) { - WidgetsBinding.instance.addPostFrameCallback((_) async { - if (mounted) { - try { - await ShareInviteBottomSheet.show( - context: context, - contacts: next.contactsWithoutKeyPackage, - ); - } catch (e) { - safeShowErrorToast('Failed to show share invite bottom sheet: $e'); - } finally { - ref.read(createGroupProvider.notifier).dismissInviteSheet(); - } - } - }); - } - });⛔ Skipped due to learnings
Learnt from: Quwaysim PR: parres-hq/whitenoise_flutter#634 File: lib/ui/chat/chat_info/group_chat_info.dart:130-133 Timestamp: 2025-09-13T19:08:51.873Z Learning: In Flutter Riverpod, ref.listen should be used exclusively within the build method of a widget, as designed. If listening is needed outside of build (such as in initState), use ref.listenManual instead.
…flutter into patch-create-group
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
lib/config/providers/create_group_provider.dart (7)
16-16: Consider using getIt for dependency injection.The static
_imagePickerServicefield violates the dependency injection pattern recommended in the coding guidelines. ImagePickerService should be registered with getIt and injected via constructor or retrieved when needed.As per coding guidelines.
Apply this approach:
Register the service in your service locator setup:
getIt.registerLazySingleton<ImagePickerService>(() => ImagePickerService());Then access it in the notifier:
class CreateGroupNotifier extends StateNotifier<CreateGroupState> { final _logger = Logger('CreateGroupNotifier'); final Ref ref; CreateGroupNotifier(this.ref) : super(const CreateGroupState()); // Access service when needed Future<void> pickGroupImage() async { try { final imagePickerService = getIt<ImagePickerService>(); final imagePath = await imagePickerService.pickProfileImage(); // ... } } }
21-29: Remove blank lines within the function.Lines 26-27 contain blank lines within the function body. The coding guidelines specify: "Don't leave blank lines within a function."
As per coding guidelines.
Apply this diff:
void updateGroupName(String groupName) { final isValid = groupName.trim().isNotEmpty; state = state.copyWith( groupName: groupName, isGroupNameValid: isValid, - error: null, - stackTrace: null, + error: null, stackTrace: null, ); }
31-37: Remove blank lines within the function.Lines 34-35 contain blank lines within the function body.
As per coding guidelines.
Apply this diff:
void updateGroupDescription(String groupDescription) { state = state.copyWith( groupDescription: groupDescription, - error: null, - stackTrace: null, + error: null, stackTrace: null, ); }
39-56: Remove blank lines within the function.The function contains blank lines (45-46, 52-53) that violate the coding guideline: "Don't leave blank lines within a function."
As per coding guidelines.
100-102: Handle missing activePubkey more gracefully.Throwing an exception when
activePubkeyis empty is inconsistent with the error handling pattern used elsewhere in this class. Consider updating the state with an error message instead.Apply this diff:
if (state.selectedImagePath != null && state.selectedImagePath!.isNotEmpty) { final activePubkey = ref.read(activePubkeyProvider) ?? ''; if (activePubkey.isEmpty) { - throw Exception('No active pubkey available'); + state = state.copyWith( + error: 'No active account available for image upload', + isCreatingGroup: false, + ); + return; }
144-155: Remove blank lines within the function.Line 148 contains a blank line within the function body.
As per coding guidelines.
218-221: Define constants for map keys.The magic strings
'withKeyPackage'and'withoutKeyPackage'are used as map keys. The coding guidelines recommend: "Avoid magic numbers and define constants."As per coding guidelines.
Define constants at the class level:
class CreateGroupNotifier extends StateNotifier<CreateGroupState> { static const String _keyWithKeyPackage = 'withKeyPackage'; static const String _keyWithoutKeyPackage = 'withoutKeyPackage'; // ... }Then use them in the map:
return { - 'withKeyPackage': contactsWithKeyPackage, - 'withoutKeyPackage': contactsWithoutKeyPackage, + _keyWithKeyPackage: contactsWithKeyPackage, + _keyWithoutKeyPackage: contactsWithoutKeyPackage, };And in the consumer (lines 63-64):
- final contactsWithKeyPackage = filteredContacts['withKeyPackage']!; - final contactsWithoutKeyPackage = filteredContacts['withoutKeyPackage']!; + final contactsWithKeyPackage = filteredContacts[CreateGroupNotifier._keyWithKeyPackage]!; + final contactsWithoutKeyPackage = filteredContacts[CreateGroupNotifier._keyWithoutKeyPackage]!;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/config/providers/create_group_provider.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/config/providers/create_group_provider.dart
🧠 Learnings (1)
📚 Learning: 2025-09-14T21:22:00.962Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.962Z
Learning: In the whitenoise_flutter codebase, the _updateGroupInfo method in GroupsNotifier performs optimistic updates that directly modify Group objects in the provider state (groups list and groupsMap). For non-DM groups, the display name comes from group.name, so updating the Group object directly is sufficient to reflect name changes in the UI without needing to refresh the separate display name cache.
Applied to files:
lib/config/providers/create_group_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 (5)
lib/config/providers/create_group_provider.dart (5)
224-226: LGTM!Clear and concise error state reset.
228-233: LGTM!Properly dismisses the invite sheet and clears the associated contact list.
235-249: LGTM! Past review comment has been addressed.The
discardChangesmethod now comprehensively resets all creation state including group name, description, contact lists, image path, error state, and UI flags. This resolves the previous concern about incomplete state reset.
252-254: LGTM!The provider is correctly defined using StateNotifierProvider with the appropriate shorthand constructor reference.
157-196: wnImageUtilsProvider reference is valid
The provider is defined in lib/config/providers/active_account_provider.dart and correctly accessible here.Likely an incorrect or invalid review comment.
|
It didn't work for me, Am I doing something wrong? |
No, its partially implemented. there are missing rust fields to be able to get the url of the uploaded image. so this PR isnt really about group image upload, there is a seperate PR comming up for that |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ui/contact_list/group_chat_details_sheet.dart (1)
104-106: Memory leak: Missing disposal of_groupDescriptionController.The
_groupDescriptionControllerlistener and controller are not being disposed, which will cause a memory leak.Apply this diff to fix the disposal:
@override void dispose() { _groupNameController.removeListener(_onGroupNameChanged); _groupNameController.dispose(); + _groupDescriptionController.removeListener(_onGroupDescriptionChanged); + _groupDescriptionController.dispose(); super.dispose(); }
🧹 Nitpick comments (2)
lib/ui/contact_list/group_chat_details_sheet.dart (2)
62-64: Consider moving contact filtering toinitStateif possible.Using
addPostFrameCallbackfor initialization logic adds unnecessary complexity. IffilterContactsWithKeyPackagedoesn't require the widget tree to be built, call it directly ininitStateinstead.Apply this diff if the provider method doesn't require a built context:
@override void initState() { super.initState(); _groupNameController.addListener(_onGroupNameChanged); _groupDescriptionController.addListener(_onGroupDescriptionChanged); - WidgetsBinding.instance.addPostFrameCallback((_) { - ref.read(createGroupProvider.notifier).filterContactsWithKeyPackage(widget.selectedContacts); - }); + ref.read(createGroupProvider.notifier).filterContactsWithKeyPackage(widget.selectedContacts); }
278-287: Localize the "Uploading Image..." string.The button label correctly uses localized strings for group creation states, but 'Uploading Image...' on line 283 is hardcoded.
Apply this diff:
-'Uploading Image...' +'ui.uploadingImage'.tr()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/config/providers/create_group_provider.dart(1 hunks)lib/ui/contact_list/group_chat_details_sheet.dart(3 hunks)lib/ui/settings/profile/edit_profile_screen.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/config/providers/create_group_provider.dart
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/contact_list/group_chat_details_sheet.dartlib/ui/settings/profile/edit_profile_screen.dart
🧠 Learnings (1)
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/ui/contact_list/group_chat_details_sheet.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 (5)
lib/ui/settings/profile/edit_profile_screen.dart (1)
168-168: LGTM! Widget rename aligns with project conventions.The rename from
EditIconWidgettoWnEditIconWidgetfollows the project's naming pattern for custom widgets.lib/ui/contact_list/group_chat_details_sheet.dart (4)
75-77: LGTM! Clean delegation to provider.The image picker method correctly delegates to the provider, keeping the UI layer thin.
113-136: Good error handling and state management.The listener correctly handles errors and clears them after display. The invite sheet logic includes proper error handling and dismissal tracking via
shouldShowInviteSheet.
138-143: LGTM! Proper cleanup on back navigation.The
PopScopewrapper correctly callsdiscardChangeswhen the user navigates back, ensuring clean state management.
233-276: LGTM! Clean member list implementation.The contact list correctly uses provider state and handles text overflow appropriately. The responsive design with proper spacing is well-implemented.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/ui/contact_list/group_chat_details_sheet.dart (1)
79-96: Duplicate: Complex navigation flow with multiple mounted checks.This navigation pattern (pop → navigate to chat with delay and mounted checks) remains complex and fragile. This was previously flagged but not addressed.
As suggested in the previous review, consider simplifying to a single navigation step:
void _createGroupChat() async { await ref.read(createGroupProvider.notifier).createGroup( onGroupCreated: (createdGroup) { if (createdGroup != null && mounted) { // Navigate directly to the chat, closing this sheet context.go(Routes.chat(createdGroup.mlsGroupId)); } }, ); }If the two-step navigation is truly required, add a comment explaining why.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
android/app/src/main/jniLibs/arm64-v8a/librust_lib_whitenoise.sois excluded by!**/*.soandroid/app/src/main/jniLibs/armeabi-v7a/librust_lib_whitenoise.sois excluded by!**/*.soandroid/app/src/main/jniLibs/x86_64/librust_lib_whitenoise.sois excluded by!**/*.so
📒 Files selected for processing (9)
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/ui/contact_list/group_chat_details_sheet.dart(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/locales/tr.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/contact_list/group_chat_details_sheet.dart
🧠 Learnings (1)
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/ui/contact_list/group_chat_details_sheet.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/pt.json (1)
294-299: LGTM! Portuguese translations added for group creation UI.The new locale keys (groupName, invitingMembers, groupNameHint, groupDescriptionHint, uploadingImage) are properly structured and align with the provider-driven group creation flow introduced in this PR. The translations appear appropriate for Portuguese.
lib/locales/fr.json (1)
294-299: LGTM! French translations added for group creation UI.The new locale keys are properly formatted and the translations are appropriate for French. Consistent with the group creation feature additions.
lib/locales/ru.json (1)
294-299: LGTM! Russian translations added for group creation UI.The Russian translations for the new group creation keys are properly structured and appear accurate.
lib/locales/it.json (1)
294-299: LGTM! Italian translations added for group creation UI.The Italian locale strings are properly added and the translations appear appropriate for the group creation feature.
lib/locales/es.json (1)
294-299: LGTM! Spanish translations added for group creation UI.The Spanish locale keys are correctly added and translations are appropriate for the new group creation flow.
lib/locales/de.json (1)
294-299: LGTM! German translations added for group creation UI.The German locale strings are properly structured and translations appear accurate for the group creation feature.
lib/locales/en.json (1)
204-208: LGTM! English locale keys added for group creation UI.The new keys (invitingMembers, groupNameHint, groupDescriptionHint, uploadingImage) are properly added and provide clear English text for the group creation flow. These align with the UI implementation in group_chat_details_sheet.dart.
lib/ui/contact_list/group_chat_details_sheet.dart (2)
7-7: Excellent provider-driven refactor with comprehensive state management.The refactor successfully transitions from local state to provider-driven architecture:
- ✅ Clean integration with
createGroupProviderfor centralized state management- ✅ Proper initialization of contact filtering on mount (lines 62-64)
- ✅ Delegated validation to provider state (canCreateGroup getter)
- ✅ Provider-based error handling with toast notifications (lines 109-132)
- ✅ PopScope implementation for discarding changes (lines 134-139)
- ✅ UI properly reflects provider state for loading, image upload, and creation states
- ✅ Localized strings used throughout ('ui.groupName'.tr(), etc.)
The provider pattern reduces coupling and makes the component more maintainable. The error handling and state management are well-structured.
Also applies to: 62-64, 67-73, 75-77, 79-96, 105-132, 134-287
211-211: WnTextField supports maxLines; usingmaxLines: 5is valid.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/ui/contact_list/group_chat_details_sheet.dart (1)
57-57: Consider the implications of mutable state.The
createdGroupfield is mutable state stored in the widget. While functional for this use case (temporary storage between creation and navigation), this pattern can make state management harder to reason about.If the navigation logic grows more complex, consider handling this entirely within the provider or passing it directly through the callback chain without storing it in widget state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
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/ui/contact_list/group_chat_details_sheet.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- lib/locales/en.json
- lib/locales/it.json
- lib/locales/tr.json
- lib/locales/ru.json
- lib/locales/pt.json
- lib/locales/es.json
- lib/locales/fr.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/contact_list/group_chat_details_sheet.dart
🧠 Learnings (1)
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/ui/contact_list/group_chat_details_sheet.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 (11)
lib/locales/de.json (1)
294-299: LGTM! Localization keys properly added.The new localization keys support the refactored group creation flow. The keys are well-structured and consistently named, aligning with the UI changes in
group_chat_details_sheet.dart.Also applies to: 303-304
lib/ui/contact_list/group_chat_details_sheet.dart (10)
7-8: LGTM! Provider imports properly added.The new imports for
CreateGroupNotifier,CreateGroupState,WnAvatar, andWnEditIconWidgetsupport the refactored provider-driven architecture.Also applies to: 15-15, 19-19
56-65: Good addition of group description with proper initialization.The
_groupDescriptionControlleris correctly initialized with a listener, and the provider'sfilterContactsWithKeyPackageis called at the appropriate time usingaddPostFrameCallback.
124-130: Previous memory leak issue resolved! ✅The
disposemethod now properly cleans up both controllers by removing listeners and callingdispose(). This addresses the critical memory leak flagged in previous reviews.Based on past review comments.
93-109: Good error handling with logging.The
_showInviteSheetmethod properly handles errors with try-catch, logs exceptions with stack traces, shows user-friendly error messages, and ensures cleanup in the finally block.
134-142: Clean error handling with provider listener.The
ref.listenpattern effectively handles errors and invite sheet display. Errors are shown as toasts and immediately cleared, preventing stale error states.
145-151: Navigation flow improved from previous review! ✅The
PopScopeintegration is cleaner than the previous nested navigation pattern. It properly resets the provider state on dismissal and triggers navigation to the created chat.Based on past review comments.
156-181: Good integration of editable group image.The avatar with edit icon overlay provides clear affordance for changing the group image. Using
ValueListenableBuilderensures the avatar updates as the user types the group name.
183-228: Previous localization issues resolved! ✅The group name and description fields now use proper localized strings (
'ui.groupName'.tr(),'ui.groupDescription'.tr()) instead of hardcoded text. ThemaxLines: 5on the description field provides good UX for longer content.Based on past review comments.
231-284: Well-structured member list with localization.The "Inviting Members:" label is properly localized (
'ui.invitingMembers'.tr()), and theWrapwidget provides a responsive layout for member chips. The chip design with avatar and name truncation is clean.
286-295: Excellent button state management.The create button correctly:
- Disables when validation fails (
state.canCreateGroup)- Shows contextual loading labels (uploading vs creating)
- Uses the loading state to prevent double-submission
| void _goToChat() { | ||
| if (createdGroup != null) { | ||
| WidgetsBinding.instance.addPostFrameCallback( | ||
| (_) async { | ||
| if (mounted) { | ||
| Routes.goToChat(context, createdGroup!.mlsGroupId); | ||
| } | ||
| } | ||
|
|
||
| // Navigate to the created group | ||
| if (mounted) { | ||
| context.pop(); | ||
|
|
||
| WidgetsBinding.instance.addPostFrameCallback((_) async { | ||
| if (mounted) { | ||
| context.go(Routes.home); | ||
| // Small delay to ensure navigation completes | ||
| await Future.delayed(const Duration(milliseconds: 150)); | ||
| if (mounted) { | ||
| Routes.goToChat(context, createdGroup.mlsGroupId); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| } else { | ||
| safeShowErrorToast('ui.failedToCreateGroup'.tr()); | ||
| } | ||
| } catch (e) { | ||
| if (mounted) { | ||
| safeShowErrorToast('${'ui.errorCreatingGroup'.tr()}: ${e.toString()}'); | ||
| } | ||
| } finally { | ||
| // Always reset loading state | ||
| if (mounted) { | ||
| setState(() { | ||
| _isCreatingGroup = false; | ||
| }); | ||
| } | ||
| }, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Minor race condition in navigation timing.
The createdGroup null check (line 112) happens outside the post-frame callback, while the actual navigation (line 116) happens inside. If the widget is disposed between these checks, the callback could attempt to navigate with a stale context.
Consider moving the null check inside the callback or consolidating the navigation logic:
void _goToChat() {
- if (createdGroup != null) {
- WidgetsBinding.instance.addPostFrameCallback(
- (_) async {
- if (mounted) {
- Routes.goToChat(context, createdGroup!.mlsGroupId);
- }
- },
- );
- }
+ WidgetsBinding.instance.addPostFrameCallback(
+ (_) async {
+ if (mounted && createdGroup != null) {
+ Routes.goToChat(context, createdGroup!.mlsGroupId);
+ }
+ },
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void _goToChat() { | |
| if (createdGroup != null) { | |
| WidgetsBinding.instance.addPostFrameCallback( | |
| (_) async { | |
| if (mounted) { | |
| Routes.goToChat(context, createdGroup!.mlsGroupId); | |
| } | |
| } | |
| // Navigate to the created group | |
| if (mounted) { | |
| context.pop(); | |
| WidgetsBinding.instance.addPostFrameCallback((_) async { | |
| if (mounted) { | |
| context.go(Routes.home); | |
| // Small delay to ensure navigation completes | |
| await Future.delayed(const Duration(milliseconds: 150)); | |
| if (mounted) { | |
| Routes.goToChat(context, createdGroup.mlsGroupId); | |
| } | |
| } | |
| }); | |
| } | |
| } else { | |
| safeShowErrorToast('ui.failedToCreateGroup'.tr()); | |
| } | |
| } catch (e) { | |
| if (mounted) { | |
| safeShowErrorToast('${'ui.errorCreatingGroup'.tr()}: ${e.toString()}'); | |
| } | |
| } finally { | |
| // Always reset loading state | |
| if (mounted) { | |
| setState(() { | |
| _isCreatingGroup = false; | |
| }); | |
| } | |
| }, | |
| ); | |
| } | |
| } | |
| void _goToChat() { | |
| WidgetsBinding.instance.addPostFrameCallback( | |
| (_) async { | |
| if (mounted && createdGroup != null) { | |
| Routes.goToChat(context, createdGroup!.mlsGroupId); | |
| } | |
| }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In lib/ui/contact_list/group_chat_details_sheet.dart around lines 111 to 121,
the null check for createdGroup occurs before scheduling a post-frame callback
which then uses createdGroup and context; if the widget is disposed before the
callback runs this can cause a race using stale state. Move the createdGroup
null check inside the post-frame callback and re-check mounted before calling
Routes.goToChat, or alternatively schedule the callback unconditionally but
inside it verify both mounted and createdGroup != null and then call
Routes.goToChat with createdGroup!.mlsGroupId; this ensures navigation only
happens when state is still valid.
Description
Refactors group creation flow to use a dedicated state management provider, improving code organization and separation of concerns. The new implementation introduces a CreateGroupNotifier with comprehensive state handling for group creation, including contact filtering, image upload, and error management.
Changes
New State Management: Created CreateGroupState and CreateGroupNotifier to manage group creation flow with proper state tracking
Group Description: Included group description field
Image Upload Integration: Implemented group image upload with proper error handling and state tracking (partial impl, awaiting update group details with uploaded url)
Improved Error Handling: Added comprehensive error states and stack trace tracking for debugging
UI Updates:
Updated group_chat_details_sheet.dart to use the new provider
Enhanced text field and bottom sheet components for better UX
Added edit profile screen improvements
State Validation: Added canCreateGroup getter to ensure valid state before group creation
Discard Changes: Added ability to discard changes and reset group creation state
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
UI
Localization
Bug Fixes