Show create group dialog when no groups exist #641
Conversation
Introduces a CreateGroupDialog that prompts users to create a new group if none exist when adding a contact to a group. The dialog integrates with NewGroupChatSheet, allowing pre-selection of the contact to be added. Updates NewGroupChatSheet to support pre-selected contacts and refactors related logic in add_to_group_screen.dart.
WalkthroughWhen adding to a group and no groups exist, the UI shows CreateGroupDialog. On create it closes two routes, resolves a contact (cached follows → user profile → fallback) and opens NewGroupChatSheet with that contact pre-selected; on cancel it closes dialogs and loading/errors are handled. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as AddToGroupScreen
participant D as CreateGroupDialog
participant F as followsProvider
participant P as userProfileDataProvider
participant G as NewGroupChatSheet
U->>S: Tap "Add to Group"
alt No groups exist
S->>D: show CreateGroupDialog
opt User cancels
U-->>D: Cancel
D-->>S: pop two routes
end
opt User creates group
U-->>D: New Group Chat
D-->>S: pop two routes
S->>F: try find contact in cached follows
alt Found
F-->>S: contact metadata
S->>S: build ContactModel
else Not found
S->>P: fetch user profile
alt Profile found
P-->>S: profile data
S->>S: build ContactModel
else Not found
S->>S: build fallback ContactModel
end
end
S->>G: show NewGroupChatSheet(preSelectedContacts: [contact])
end
else Existing groups
S-->>U: proceed with existing flow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 (2)
lib/ui/chat/chat_management/add_to_group_screen.dart (1)
175-183: Fix: missing ScreenUtil import and invalid Row.spacing (compile blockers).
- This file uses 16.h/24.w/18.sp but doesn’t import flutter_screenutil.
- Row has no spacing parameter; this won’t compile.
Apply:
@@ import 'package:whitenoise/ui/core/ui/wn_image.dart'; +import 'package:flutter_screenutil/flutter_screenutil.dart'; @@ - Padding( - padding: EdgeInsets.symmetric(horizontal: 16.w), - child: Row( - spacing: 8.w, - children: [ + Padding( + padding: EdgeInsets.symmetric(horizontal: 16.w), + child: Row( + children: [ IconButton( icon: WnImage( AssetsPaths.icChevronLeft, size: 24.w, color: context.colors.primary, ), onPressed: () => Navigator.of(context).pop(), ), + SizedBox(width: 8.w), Text( 'Add to Group',lib/ui/contact_list/new_group_chat_sheet.dart (1)
47-56: Make ContactModel equality/hashCode use the unique id (publicKey) only.ContactModel already overrides operator== and hashCode (lib/domain/models/contact_model.dart — operator== ≈ line 81, hashCode ≈ line 97) but both include multiple fields (publicKey, imagePath, displayName, about, website, nip05, lud16). Because selection uses Set, equality should be based on the unique identifier (publicKey) to avoid duplicate entries and selection/contains() glitches — update operator== and hashCode to compare/hash only publicKey (or ensure preSelectedContacts use the exact same instances).
🧹 Nitpick comments (7)
lib/ui/chat/chat_management/add_to_group_screen.dart (3)
41-76: Avoid setState after await without mounted checks.Add mounted guards before setState calls that occur after awaits.
@@ Future<void> _loadGroups() async { setState(() { _isLoading = true; }); await ref.read(groupsProvider.notifier).loadGroups(); + if (!mounted) return; @@ if (regularGroups.isEmpty) { setState(() { _isLoading = false; }); // Show dialog when no groups exist if (mounted) { _showCreateGroupDialog(); } return; } @@ if (loadTasks.isNotEmpty) { await Future.wait(loadTasks); } + if (!mounted) return; @@ setState(() { _regularGroups = regularGroups; _isLoading = false; }); } @@ setState(() { _isLoading = false; });Also applies to: 118-121
36-41: Prefer Set for selected group IDs to prevent duplicates.Replace the List with a Set; no other logic changes required.
- final List<String> _groupsToAddUserTo = []; + final Set<String> _groupsToAddUserTo = {}; @@ - value: _groupsToAddUserTo.contains(group.mlsGroupId) || isContactInGroup, + value: _groupsToAddUserTo.contains(group.mlsGroupId) || isContactInGroup, @@ - if (value == true) { - _groupsToAddUserTo.add(group.mlsGroupId); - } else { - _groupsToAddUserTo.remove(group.mlsGroupId); - } + if (value == true) { + _groupsToAddUserTo.add(group.mlsGroupId); + } else { + _groupsToAddUserTo.remove(group.mlsGroupId); + }Also applies to: 244-251
190-199: Localize user-facing strings.Strings like “Add to Group”, “(X members)”, “No groups selected”, success/error toasts should use AppLocalizations per guidelines.
Example:
- 'Add to Group', + AppLocalizations.of(context)!.addToGroup,Apply similarly for other user-facing text.
Also applies to: 236-241, 269-272, 80-109
lib/ui/contact_list/new_group_chat_sheet.dart (1)
30-40: Localize user-facing strings.“New group chat”, “Search contact or public key…”, empty/error texts, “Go Back”, “Continue” should come from AppLocalizations.
Also applies to: 157-161, 170-191, 197-209
lib/ui/chat/chat_management/widgets/create_group_dialog.dart (3)
50-57: Use IconButton for accessibility and semantics.Replace GestureDetector with IconButton and add a tooltip/semantic label.
- GestureDetector( - onTap: onCancel ?? () => Navigator.of(context).pop(), - child: Icon( - Icons.close, - size: 16.w, - color: context.colors.mutedForeground, - ), - ), + IconButton( + onPressed: onCancel ?? () => Navigator.of(context).pop(), + icon: Icon(Icons.close, size: 16.w, color: context.colors.mutedForeground), + tooltip: 'Close', + ),
43-49: Localize dialog copy and buttons.Move “Create a Group to Continue”, body text, “Cancel”, “New Group Chat” to AppLocalizations.
Also applies to: 62-71, 75-95
10-19: Trim unused API surface: drop contactToAdd until it’s used.The contactToAdd field/param isn’t used. Removing it keeps the public API lean.
class CreateGroupDialog extends StatelessWidget { final VoidCallback? onCreateGroup; final VoidCallback? onCancel; - final ContactModel? contactToAdd; @@ const CreateGroupDialog({ super.key, this.onCreateGroup, this.onCancel, - this.contactToAdd, }); @@ static Future<bool?> show( BuildContext context, { VoidCallback? onCreateGroup, VoidCallback? onCancel, - ContactModel? contactToAdd, }) { return showDialog<bool>( context: context, barrierDismissible: true, builder: (context) => CreateGroupDialog( onCreateGroup: onCreateGroup, onCancel: onCancel, - contactToAdd: contactToAdd, ), ); }Also applies to: 105-121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/ui/chat/chat_management/add_to_group_screen.dart(3 hunks)lib/ui/chat/chat_management/widgets/create_group_dialog.dart(1 hunks)lib/ui/contact_list/new_group_chat_sheet.dart(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/chat/chat_management/widgets/create_group_dialog.dartlib/ui/contact_list/new_group_chat_sheet.dartlib/ui/chat/chat_management/add_to_group_screen.dart
🧠 Learnings (6)
📚 Learning: 2025-09-14T21:22:00.952Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.952Z
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/ui/chat/chat_management/add_to_group_screen.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/chat/chat_management/add_to_group_screen.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/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-09-08T17:39:13.711Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#599
File: test/ui/contact_list/start_chat_bottom_sheet_test.dart:173-176
Timestamp: 2025-09-08T17:39:13.711Z
Learning: In the whitenoise Flutter app, the followProvider uses followsProvider behind the scenes, so mocking followsProvider with MockFollowsNotifier effectively controls what followProvider sees in tests.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (3)
lib/ui/contact_list/new_group_chat_sheet.dart (1)
16-21: LGTM: preSelectedContacts is plumbed correctly and applied in initState.Also applies to: 25-40, 53-56
lib/ui/chat/chat_management/add_to_group_screen.dart (2)
143-146: Incorrect — getUserProfileData already returns ContactModel. The provider declaresFuture<ContactModel> getUserProfileData(String pubkey)and maps User.metadata → ContactModel, so assigning its result to a ContactModel is correct (see lib/config/providers/user_profile_data_provider.dart).Likely an incorrect or invalid review comment.
20-22: No action required — pubkey encoding is already normalized. follows_provider.findFollowByPubkey normalizes the input to hex (PubkeyFormatter.toHex); GroupsProvider.addToGroup converts membersNpubs → hex before calling the API; ContactModel.fromMetadata stores npub via toNpub and operator== compares hex via toHex. See lib/config/providers/follows_provider.dart, lib/config/providers/group_provider.dart, lib/domain/models/contact_model.dart.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
lib/ui/chat/chat_management/add_to_group_screen.dart (2)
127-133: Nice: fixed the double-pop-before-show bug.Popping only the dialog, then showing the sheet from a still-mounted context is correct.
157-163: Bug: possible null in preSelectedContacts list.contactToAdd can be null; [ContactModel?] is not assignable to List.
Apply:
- await NewGroupChatSheet.show( - context, - preSelectedContacts: [contactToAdd], - ); + await NewGroupChatSheet.show( + context, + preSelectedContacts: contactToAdd != null ? [contactToAdd] : null, + );
🧹 Nitpick comments (4)
lib/ui/chat/chat_management/add_to_group_screen.dart (4)
49-55: Guard setState after await to avoid setState on disposed State.If this widget is disposed before these lines run, setState will throw.
Apply:
- setState(() { - _isLoading = false; - }); - // Show dialog when no groups exist - if (mounted) { - _showCreateGroupDialog(); - } + if (!mounted) return; + setState(() { + _isLoading = false; + }); + // Show dialog when no groups exist + _showCreateGroupDialog();
165-167: Cancel pops two routes — confirm UX; prefer context.pop if available.If the intent is to leave this screen on cancel, fine; otherwise only pop the dialog. Team preference (per prior learnings) favors context.pop().
Option A (leave screen): pop only dialog.
- Navigator.of(context).pop(); - Navigator.of(context).pop(); + Navigator.of(context).pop(); // close dialog onlyOption B (exit screen too), switch to context.pop for consistency:
- Navigator.of(context).pop(); - Navigator.of(context).pop(); + context.pop(); // dialog + context.pop(); // this screen
123-169: Keep functions short; extract contact resolution into a helper._showsCreateGroupDialog mixes UI control flow with data resolution. Extracting improves readability and testability.
Example:
Future<ContactModel?> _resolveContactForNpub(WidgetRef ref, String npub) async { try { final follows = ref.read(followsProvider.notifier); final f = follows.findFollowByPubkey(npub); if (f != null) { return ContactModel.fromMetadata(pubkey: f.pubkey, metadata: f.metadata); } final userProfile = await ref.read(userProfileDataProvider.notifier).getUserProfileData(npub); if (userProfile != null) { return ContactModel.fromMetadata(pubkey: npub, metadata: userProfile.metadata); } } catch (_) {} return ContactModel(displayName: 'Unknown User', publicKey: npub); }Then call:
final contactToAdd = await _resolveContactForNpub(ref, widget.contactNpub);
123-169: Style: remove blank lines inside function per repo guideline.Tighten vertical whitespace in _showCreateGroupDialog.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ui/chat/chat_management/add_to_group_screen.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/chat/chat_management/add_to_group_screen.dart
🧠 Learnings (7)
📚 Learning: 2025-09-14T21:22:00.952Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.952Z
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/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-09-15T20:51:15.280Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#639
File: lib/ui/auth_flow/login_screen.dart:140-165
Timestamp: 2025-09-15T20:51:15.280Z
Learning: In the whitenoise_flutter codebase, prefer simple context.pop() for back actions rather than more complex Navigator.maybePop() with fallback logic, as indicated by the maintainer Quwaysim.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.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/chat/chat_management/add_to_group_screen.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/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-09-08T17:39:13.711Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#599
File: test/ui/contact_list/start_chat_bottom_sheet_test.dart:173-176
Timestamp: 2025-09-08T17:39:13.711Z
Learning: In the whitenoise Flutter app, the followProvider uses followsProvider behind the scenes, so mocking followsProvider with MockFollowsNotifier effectively controls what followProvider sees in tests.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (3)
lib/ui/chat/chat_management/add_to_group_screen.dart (3)
5-11: LGTM on new imports.All added imports are used and align with the new flow.
136-144: Good reuse of cached follow → ContactModel conversion.Using ContactModel.fromMetadata keeps UI types consistent.
145-148: Incorrect — getUserProfileData already returns ContactModel; no conversion needed. lib/config/providers/user_profile_data_provider.dart declaresFuture<ContactModel> getUserProfileData(String pubkey)(line 24), socontactToAdd = await userProfileDataNotifier.getUserProfileData(widget.contactNpub)is type-correct.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
lib/ui/chat/chat_management/add_to_group_screen.dart (3)
175-178: Cancel should close only the dialog (avoid double-pop).Double‑popping here exits the screen as well. Previous guidance favored keeping this screen on cancel.
- onCancel: () { - Navigator.of(context).pop(); - Navigator.of(context).pop(); - }, + onCancel: () { + Navigator.of(context).pop(); // Close dialog only + },
147-159: Type mismatch: convert UserProfileData → ContactModel before preselecting.
getUserProfileDatalikely returns a profile type, notContactModel. Convert toContactModel(as in past review) before using it. Also add explicit types for locals per guidelines.- final followsNotifier = ref.read(followsProvider.notifier); - final existingFollow = followsNotifier.findFollowByPubkey(widget.contactNpub); + final FollowsNotifier followsNotifier = ref.read(followsProvider.notifier); + final FollowModel? existingFollow = + followsNotifier.findFollowByPubkey(widget.contactNpub); if (existingFollow != null) { contactToAdd = ContactModel.fromMetadata( pubkey: existingFollow.pubkey, metadata: existingFollow.metadata, ); } else { - // If not in follows, fetch from user profile data provider - final userProfileDataNotifier = ref.read(userProfileDataProvider.notifier); - contactToAdd = await userProfileDataNotifier.getUserProfileData(widget.contactNpub); + // If not in follows, fetch from user profile data provider and convert + final UserProfileDataNotifier userProfileDataNotifier = + ref.read(user_profile_data_provider.notifier); + final UserProfileData? profile = + await userProfileDataNotifier.getUserProfileData(widget.contactNpub); + if (profile != null) { + contactToAdd = ContactModel.fromMetadata( + pubkey: widget.contactNpub, + metadata: profile.metadata, + ); + } }#!/bin/bash # Verify the expected types in the codebase rg -nP --type=dart 'class\s+FollowsNotifier\b' rg -nP --type=dart 'findFollowByPubkey\s*\(' -C2 rg -nP --type=dart 'class\s+UserProfileDataNotifier\b' rg -nP --type=dart 'getUserProfileData\s*\(' -C2
168-174: Fix null-safety: don't pass a nullable contact inside a listNewGroupChatSheet.preSelectedContacts is declared as List? (lib/ui/contact_list/new_group_chat_sheet.dart:16–18). Passing [contactToAdd] can produce [null] and cause a type error — replace with a conditional null check:
- await NewGroupChatSheet.show( - context, - preSelectedContacts: [contactToAdd], - ); + await NewGroupChatSheet.show( + context, + preSelectedContacts: contactToAdd != null ? [contactToAdd] : null, + );
🧹 Nitpick comments (6)
lib/ui/chat/chat_management/add_to_group_screen.dart (6)
49-57: Add explicit typing for localregularGroups.The guidelines require explicit types. Declare
regularGroupsasList<Group>instead of relying on inference.- final regularGroups = await ref.read(groupsProvider.notifier).getRegularGroups(); + final List<Group> regularGroups = + await ref.read(groupsProvider.notifier).getRegularGroups();
58-69: Type locals and loop variable explicitly.
- Give
loadTasksan explicitList<Future<void>>type.- Type the loop variable as
Group.- Consider typing
existingMembersas the correct collection type for clarity.- final loadTasks = <Future<void>>[]; + final List<Future<void>> loadTasks = <Future<void>>[]; - for (final group in regularGroups) { + for (final Group group in regularGroups) { final existingMembers = ref.read(groupsProvider).groupMembers?[group.mlsGroupId];
46-57: Good: early exit to show dialog; loading state guarded by try/finally.Flow reads well and avoids doing extra work when there are no groups.
Per guidelines, remove blank lines within the function to keep a tight layout.
75-79: Don’t surface raw exceptions to users; localize the message.Showing
$ein a toast can leak internals. Log the error and display a localized, user‑friendly message via AppLocalizations.- ref.showErrorToast('Failed to load groups: $e'); + debugPrint('Failed to load groups: $e'); + // Replace with your l10n accessor + ref.showErrorToast(context.l10n.errorLoadingGroups);
138-142: Verify you’re popping the same navigator used to show the dialog.If
CreateGroupDialog.showuses the root navigator,Navigator.of(context).pop()might pop this page instead of the dialog. Prefer passingrootNavigator: trueconsistently or popping via the same navigator used to present the dialog.Would you like me to adjust
CreateGroupDialog.showto expose whether it used the root navigator and pop accordingly?
134-181: Optional: extract contact resolution into a helper for single-responsibility.
_showCreateGroupDialogmixes UI and data resolution. Consider extracting a privateFuture<ContactModel?> _resolveContactToAdd()to simplify and improve testability.I can generate the helper and unit tests if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ui/chat/chat_management/add_to_group_screen.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/chat/chat_management/add_to_group_screen.dart
🧠 Learnings (7)
📚 Learning: 2025-09-14T21:22:00.952Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.952Z
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/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-09-15T20:51:15.280Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#639
File: lib/ui/auth_flow/login_screen.dart:140-165
Timestamp: 2025-09-15T20:51:15.280Z
Learning: In the whitenoise_flutter codebase, prefer simple context.pop() for back actions rather than more complex Navigator.maybePop() with fallback logic, as indicated by the maintainer Quwaysim.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.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/chat/chat_management/add_to_group_screen.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/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-09-08T17:39:13.711Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#599
File: test/ui/contact_list/start_chat_bottom_sheet_test.dart:173-176
Timestamp: 2025-09-08T17:39:13.711Z
Learning: In the whitenoise Flutter app, the followProvider uses followsProvider behind the scenes, so mocking followsProvider with MockFollowsNotifier effectively controls what followProvider sees in tests.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (1)
lib/ui/chat/chat_management/add_to_group_screen.dart (1)
5-5: LGTM: necessary imports.New imports are relevant to the new flow and are used in this file.
Also applies to: 7-11
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/ui/chat/chat_management/widgets/create_group_dialog.dart (1)
23-90: Reuse WnDialog’s standard API instead of a fully custom child.We already have WnDialog that handles header/title/actions consistently across the app; prefer its non-custom variant to avoid bespoke layout here.
🧹 Nitpick comments (5)
lib/ui/chat/chat_management/widgets/create_group_dialog.dart (5)
40-47: Use IconButton instead of GestureDetector for accessibility and semantics.IconButton provides focus, semantics, and splash by default.
- GestureDetector( - onTap: onCancel ?? () => Navigator.of(context).pop(), - child: Icon( - Icons.close, - size: 16.w, - color: context.colors.mutedForeground, - ), - ), + IconButton( + icon: Icon(Icons.close, size: 16.w, color: context.colors.mutedForeground), + tooltip: AppLocalizations.of(context)!.close, + onPressed: onCancel ?? () => Navigator.of(context).pop(), + ),
93-108: Align the dialog API: return void or emit an explicit result.
show()returnsFuture<bool?>but no path sets a bool onpop(). Either switch toFuture<void>or wire buttons topop(true/false). The current signature can mislead callers.- static Future<bool?> show( + static Future<void> show( @@ - }) { - return showDialog<bool>( + }) { + return showDialog<void>( context: context, builder: (context) => CreateGroupDialog( onCreateGroup: onCreateGroup, onCancel: onCancel, - contactToAdd: contactToAdd, ), ); }Please confirm no call sites depend on a boolean result before changing the signature; otherwise, prefer popping
trueon “New Group Chat” andfalseon cancel.
41-41: Prefer AutoRoute navigation helpers for consistency.Project guideline says “Use AutoRoute to manage routes.” Use
context.router.pop()(ormaybePop) and setbarrierDismissibleexplicitly if needed.+import 'package:auto_route/auto_route.dart'; @@ - onTap: onCancel ?? () => Navigator.of(context).pop(), + onTap: onCancel ?? () => context.router.pop(), @@ - onPressed: onCancel ?? () => Navigator.of(context).pop(), + onPressed: onCancel ?? () => context.router.pop(), @@ - return showDialog<bool>( + return showDialog<void>( context: context, + barrierDismissible: false,If barrier dismissal should be allowed, set
barrierDismissible: trueintentionally; current behavior is implicit.Also applies to: 67-67, 99-103
12-19: Remove unused contactToAdd from this widget or surface it meaningfully.The field is accepted and threaded through but never used; it expands the public API without benefit. Either display context (“Add {name} to a new group”) or drop the parameter here and let the callback own it.
I can generate a follow-up diff to remove it here and adjust the call site in add_to_group_screen.dart if it’s unused.
Also applies to: 97-98, 105-106
66-86: Place the primary action before the secondary for visual hierarchy.Consider showing “New Group Chat” above “Cancel” to emphasize the recommended action.
- WnFilledButton( - onPressed: onCancel ?? () => Navigator.of(context).pop(), - label: 'Cancel', - visualState: WnButtonVisualState.secondary, - size: WnButtonSize.small, - labelTextStyle: TextStyle( - fontSize: 16.sp, - fontWeight: FontWeight.w600, - ), - ), - Gap(12.h), WnFilledButton( onPressed: onCreateGroup, label: 'New Group Chat', size: WnButtonSize.small, labelTextStyle: TextStyle( fontSize: 16.sp, fontWeight: FontWeight.w600, ), ), + Gap(12.h), + WnFilledButton( + onPressed: onCancel ?? () => Navigator.of(context).pop(), + label: 'Cancel', + visualState: WnButtonVisualState.secondary, + size: WnButtonSize.small, + labelTextStyle: TextStyle( + fontSize: 16.sp, + fontWeight: FontWeight.w600, + ), + ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ui/chat/chat_management/widgets/create_group_dialog.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/chat/chat_management/widgets/create_group_dialog.dart
🔇 Additional comments (1)
lib/ui/chat/chat_management/widgets/create_group_dialog.dart (1)
77-79: Good: null onPressed disables the primary action.Using a nullable
onCreateGroupcleanly disables the “New Group Chat” button via the button’s semantics.Confirm
WnFilledButtonindeed treatsonPressed == nullas disabled across all themes.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
Sorry to block this, but I am testing it in my iphone and after I create a group with a user (both new users created in whitenoise) an select Continue, it brings me back to a blank sheet with the title "Add to Group"
|
Steps to reproduce
|
|
PS: I first started following the user, otherwise I couldn't see it in the "New group chat" from step 3 in the table above |
|
For the flow for a new whitenoise user that I don't follow, it's confusing cause the new group only shows other people I follow, but not the user that I initially wanted to add to the group. I think this is just because the group creation is now restricted in the frontend so that we can only add people we follow to a group. Maybe, while group creation works this way, the "Add to group" option should be visible only if we follow that user? WDYT? 💡 Edit: this could of course be separate issue, not for this PR |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ui/chat/chat_management/add_to_group_screen.dart (1)
247-249: Normalize key formats before comparing (use PubkeyFormatter.toHex()).members.any((member) => member.publicKey == widget.contactNpub) can false‑negative when one value is npub and the other hex — compare normalized keys instead, e.g. compare PubkeyFormatter(pubkey: member.publicKey).toHex() to PubkeyFormatter(pubkey: widget.contactNpub).toHex() (use
?? ''fallback if needed).
Location: lib/ui/chat/chat_management/add_to_group_screen.dart:247-249
♻️ Duplicate comments (2)
lib/ui/chat/chat_management/add_to_group_screen.dart (2)
194-196: Cancel flow: double-pop UX — confirm intent (dialog only vs. leave screen).This mirrors a previous comment. If the desired UX is to stay on this screen when canceling, pop only the dialog.
Option A (stay on screen):
- onCancel: () { - Navigator.of(context).pop(); - Navigator.of(context).pop(); - }, + onCancel: () { + context.pop(); // close only the dialog + },Option B (leave screen): consider rootNavigator for consistency, or document the choice.
157-160: Compile-time type bug: assigning UserProfileData? to ContactModel?.
getUserProfileDatalikely returnsUserProfileData?, notContactModel. Convert toContactModelfrom metadata.Apply:
- final userProfileDataNotifier = ref.read(userProfileDataProvider.notifier); - contactToAdd = await userProfileDataNotifier.getUserProfileData(widget.contactNpub); + final userProfileDataNotifier = ref.read(userProfileDataProvider.notifier); + final profile = await userProfileDataNotifier.getUserProfileData(widget.contactNpub); + if (profile != null) { + contactToAdd = ContactModel.fromMetadata( + pubkey: widget.contactNpub, + metadata: profile.metadata, + ); + }
🧹 Nitpick comments (4)
lib/ui/chat/chat_management/add_to_group_screen.dart (4)
71-86: Don’t surface raw exceptions to users; keep details in logs and localize the message.Show a generic error in the toast and send
e(and stack) to logs.Example (adjust to your logger/localizations):
- ref.showErrorToast('Failed to load groups: $e'); + ref.showErrorToast(AppLocalizations.of(context)!.groupsLoadFailed); + // log(e, s) elsewhere
49-49: Add explicit types per repo guidelines.Declare local variable and loop element types.
Apply:
- final regularGroups = await ref.read(groupsProvider.notifier).getRegularGroups(); + final List<Group> regularGroups = await ref.read(groupsProvider.notifier).getRegularGroups();- for (final group in regularGroups) { + for (final Group group in regularGroups) {Also applies to: 60-61
141-141: Prefer context.pop() for navigation (consistency with codebase).Keeps navigation concise and aligned with maintainers’ preference.
- if (Navigator.of(context).canPop()) { - Navigator.of(context).pop(); - } + if (context.canPop()) { + context.pop(); + }- Navigator.of(context).pop(); + context.pop();Add import:
import 'package:auto_route/auto_route.dart';Also applies to: 183-183
163-173: Localize user-visible strings (“Unknown User”, error toasts).Use AppLocalizations keys instead of hardcoded strings.
Example:
- displayName: 'Unknown User', + displayName: AppLocalizations.of(context)!.unknownUser,And similarly for error toasts.
Also applies to: 77-78, 189-190
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ui/chat/chat_management/add_to_group_screen.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/chat/chat_management/add_to_group_screen.dart
🧠 Learnings (7)
📚 Learning: 2025-09-14T21:22:00.952Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.952Z
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/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-09-15T20:51:15.280Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#639
File: lib/ui/auth_flow/login_screen.dart:140-165
Timestamp: 2025-09-15T20:51:15.280Z
Learning: In the whitenoise_flutter codebase, prefer simple context.pop() for back actions rather than more complex Navigator.maybePop() with fallback logic, as indicated by the maintainer Quwaysim.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.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/chat/chat_management/add_to_group_screen.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/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-09-08T17:39:13.711Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#599
File: test/ui/contact_list/start_chat_bottom_sheet_test.dart:173-176
Timestamp: 2025-09-08T17:39:13.711Z
Learning: In the whitenoise Flutter app, the followProvider uses followsProvider behind the scenes, so mocking followsProvider with MockFollowsNotifier effectively controls what followProvider sees in tests.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (4)
lib/ui/chat/chat_management/add_to_group_screen.dart (4)
5-11: Imports look correct for new flow.New dependencies align with the dialog/sheet and data lookups.
46-57: Good: defensive loading with try/finally and early return when no groups.Solid pattern; prevents stuck loading and triggers the dialog only when needed.
58-69: Good: batched member loads via Future.wait.Avoids n+1 awaits and keeps the UI responsive.
134-187: Navigation sequence on create: solid fix (pop dialog, await sheet, then pop screen).This avoids using a disposed context and matches prior guidance.
Please confirm NewGroupChatSheet.show returns a Future that resolves after the sheet closes (so awaiting it is correct). If not, we should move the post-sheet pop into onGroupCreated only.
josefinalliende
left a comment
josefinalliende
left a comment
There was a problem hiding this comment.
I tried it again in my iPhone and now it worked perfect! 💯 🎉
I don't know, maybe we should open a new issue 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/chat/chat_management/add_to_group_screen.dart (1)
243-245: Normalize pubkey formats before comparing (fix false-negative check)member.publicKey is typically hex while widget.contactNpub is bech32; direct equality will miss matches and allow duplicate adds.
Location: lib/ui/chat/chat_management/add_to_group_screen.dart (around lines 243–245).
Fix: normalize both sides to the same format (hex). Example:
final targetHex = PubkeyFormatter(pubkey: widget.contactNpub).toHex();
final isContactInGroup = members.any((m) => PubkeyFormatter(pubkey: m.publicKey).toHex() == targetHex);
♻️ Duplicate comments (1)
lib/ui/chat/chat_management/add_to_group_screen.dart (1)
189-192: Cancel flow: double-pop can be fragile; prefer explicit root pop for the dialog and a single page pop.This mirrors the earlier feedback. Keep this screen on cancel unless UX demands exit.
Apply:
- onCancel: () { - Navigator.of(context).pop(); - Navigator.of(context).pop(); - }, + onCancel: () { + final rootNav = Navigator.of(context, rootNavigator: true); + if (rootNav.canPop()) rootNav.pop(); // close dialog + // Option A (recommended): keep this screen so user can retry + // Option B: if UX wants to leave, then: + // if (mounted) Navigator.of(context).pop(); + },
🧹 Nitpick comments (3)
lib/ui/chat/chat_management/add_to_group_screen.dart (3)
139-143: Pop the dialog via the root navigator to avoid popping this page.Using the page navigator here can pop this screen instead of the dialog depending on how the dialog was shown.
Apply:
- // Close the dialog only - if (Navigator.of(context).canPop()) { - Navigator.of(context).pop(); - } + // Close the dialog only (use the same navigator used by showDialog) + final rootNav = Navigator.of(context, rootNavigator: true); + if (rootNav.canPop()) { + rootNav.pop(); + }
271-279: Use a Set for selected group ids to avoid duplicates and O(1) lookups.Minor but safer for toggling.
Apply (and adjust the field declaration):
- value: _groupsToAddUserTo.contains(group.mlsGroupId) || isContactInGroup, + value: _groupsToAddUserTo.contains(group.mlsGroupId) || isContactInGroup, onChanged: (bool? value) { setState(() { if (value == true) { - _groupsToAddUserTo.add(group.mlsGroupId); + _groupsToAddUserTo.add(group.mlsGroupId); } else { _groupsToAddUserTo.remove(group.mlsGroupId); } }); },And at field declaration (outside this hunk):
- final List<String> _groupsToAddUserTo = []; + final Set<String> _groupsToAddUserTo = <String>{};
117-131: Guard setState after async and prevent double-submit while loading.Avoid setState on a disposed State; also disable the button while loading.
Apply:
- setState(() { - _isLoading = false; - }); + if (!mounted) return; + setState(() { + _isLoading = false; + });And for the button (later in the file):
- onPressed: _groupsToAddUserTo.isEmpty ? null : _addUserToGroups, + onPressed: (_isLoading || _groupsToAddUserTo.isEmpty) ? null : _addUserToGroups,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ui/chat/chat_management/add_to_group_screen.dart(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/chat/chat_management/add_to_group_screen.dart
🧠 Learnings (7)
📚 Learning: 2025-09-14T21:22:00.952Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#634
File: lib/config/providers/group_provider.dart:1130-1132
Timestamp: 2025-09-14T21:22:00.952Z
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/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-09-15T20:51:15.280Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#639
File: lib/ui/auth_flow/login_screen.dart:140-165
Timestamp: 2025-09-15T20:51:15.280Z
Learning: In the whitenoise_flutter codebase, prefer simple context.pop() for back actions rather than more complex Navigator.maybePop() with fallback logic, as indicated by the maintainer Quwaysim.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.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/chat/chat_management/add_to_group_screen.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/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-09-08T17:39:13.711Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#599
File: test/ui/contact_list/start_chat_bottom_sheet_test.dart:173-176
Timestamp: 2025-09-08T17:39:13.711Z
Learning: In the whitenoise Flutter app, the followProvider uses followsProvider behind the scenes, so mocking followsProvider with MockFollowsNotifier effectively controls what followProvider sees in tests.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/chat/chat_management/add_to_group_screen.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (1)
lib/ui/chat/chat_management/add_to_group_screen.dart (1)
46-56: Loading flow refactor looks good.Early exit to dialog, parallel member loads, and finally-reset with mounted checks are solid.
Also applies to: 58-69, 71-86
| ref.showErrorToast('Failed to load groups: $e'); | ||
| } |
There was a problem hiding this comment.
Localize user-facing strings with AppLocalizations.
Several strings are hard-coded; move them to l10n.
Example:
- ref.showErrorToast('Failed to load groups: $e');
+ ref.showErrorToast(AppLocalizations.of(context)!.groupsLoadFailed(e.toString()));Similarly:
- 'No groups selected'
- 'Successfully added user to ...'
- 'Add to Group' (title and button)
- '(... members)'
- 'Unknown User'
Define keys in ARB and replace usages.
Also applies to: 91-92, 118-120, 220-221, 263-264, 296-297
🤖 Prompt for AI Agents
lib/ui/chat/chat_management/add_to_group_screen.dart lines ~77-78, 91-92,
118-120, 220-221, 263-264, 296-297: Several user-facing strings are hard-coded
and must be localized via AppLocalizations. Add ARB entries for keys such as
failedLoadGroups, noGroupsSelected, userAddedToGroup (with placeholders),
addToGroupTitle, addToGroupButton, membersCount (e.g. "({count} members)"), and
unknownUser; update the Dart code to import AppLocalizations and replace each
hard-coded string with the corresponding AppLocalizations.<key>.<key> call
(using Intl placeholders where needed), pass any variables (like group/user
names and counts) through the localization methods, and run the Flutter gen-l10n
build to regenerate localization classes; ensure tests/usage compile and update
any widget tests expecting raw strings.
| // First try to get from follows (cached contacts) | ||
| final followsNotifier = ref.read(followsProvider.notifier); | ||
| final existingFollow = followsNotifier.findFollowByPubkey(widget.contactNpub); | ||
|
|
||
| if (existingFollow != null) { | ||
| contactToAdd = ContactModel.fromMetadata( | ||
| pubkey: existingFollow.pubkey, | ||
| metadata: existingFollow.metadata, | ||
| ); | ||
| } else { | ||
| // If not in follows, fetch from user profile data provider | ||
| final userProfileDataNotifier = ref.read(userProfileDataProvider.notifier); | ||
| contactToAdd = await userProfileDataNotifier.getUserProfileData(widget.contactNpub); | ||
| } | ||
| } catch (e) { |
There was a problem hiding this comment.
Type mismatch and null-safety: ensure ContactModel and never pass [null] to preSelectedContacts.
getUserProfileData likely returns a profile, not ContactModel. Also, if it returns null, [contactToAdd] becomes [null] at runtime.
Apply:
- // Get contact information for the user to be added
- ContactModel? contactToAdd;
+ // Get contact information for the user to be added
+ ContactModel? contactToAdd;
try {
// First try to get from follows (cached contacts)
final followsNotifier = ref.read(followsProvider.notifier);
final existingFollow = followsNotifier.findFollowByPubkey(widget.contactNpub);
if (existingFollow != null) {
contactToAdd = ContactModel.fromMetadata(
pubkey: existingFollow.pubkey,
metadata: existingFollow.metadata,
);
} else {
- // If not in follows, fetch from user profile data provider
- final userProfileDataNotifier = ref.read(userProfileDataProvider.notifier);
- contactToAdd = await userProfileDataNotifier.getUserProfileData(widget.contactNpub);
+ // If not in follows, fetch from user profile data provider and convert
+ final userProfileDataNotifier = ref.read(userProfileDataProvider.notifier);
+ final profile = await userProfileDataNotifier.getUserProfileData(widget.contactNpub);
+ if (profile != null) {
+ contactToAdd = ContactModel.fromMetadata(
+ pubkey: widget.contactNpub,
+ metadata: profile.metadata,
+ );
+ }
}
} catch (e) {
- // Create a basic contact model with just the public key
- contactToAdd = ContactModel(
- displayName: 'Unknown User',
- publicKey: widget.contactNpub,
- );
+ // Swallow and fallback below
}
- // Ensure we always have a contact (in case getUserProfileData returns null)
+ // Ensure we always have a contact (in case getUserProfileData returns null)
+ contactToAdd ??= ContactModel(
+ displayName: 'Unknown User',
+ publicKey: widget.contactNpub,
+ );
if (!mounted) return;
await NewGroupChatSheet.show(
context,
- preSelectedContacts: [contactToAdd],
+ preSelectedContacts: [contactToAdd!],
onGroupCreated: (group) {Also applies to: 162-167, 169-176
🤖 Prompt for AI Agents
In lib/ui/chat/chat_management/add_to_group_screen.dart around lines 147-161
(and also apply to 162-167 and 169-176), you are assigning the result of
getUserProfileData (which returns a user profile or nullable) directly to a
ContactModel variable and then passing it into preSelectedContacts, which can
lead to a type mismatch or passing null. Fix by converting the fetched profile
to a ContactModel (e.g., call ContactModel.fromMetadata/fromProfile with the
profile's pubkey/metadata) and guard against null: only add to
preSelectedContacts when the created ContactModel is non-null (or handle the
null case explicitly, e.g., show an error or skip adding). Ensure types match
the ContactModel constructor and that preSelectedContacts never receives a null
entry.
Description
Introduces a CreateGroupDialog that prompts users to create a new group if none exist when adding a contact to a group. The dialog integrates with NewGroupChatSheet, allowing pre-selection of the contact to be added. Updates NewGroupChatSheet to support pre-selected contacts and refactors related logic in add_to_group_screen.dart.
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)Fixes #545
Summary by CodeRabbit
New Features
Improvements