From c8b7672aa2277514990232e10889bcdf18b3de66 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Sat, 7 Nov 2026 16:52:01 -0800 Subject: [PATCH] fix(mobile): check the current picker selection on a toggle A second tap could reach Flutter before the rebuild that reflected the first, and both toggles read the selection captured at build time, so the same person was added twice. The diffed native table traps on a duplicate row. Read the current selection instead, and fail loudly if a shown member row is missing from the state. Co-Authored-By: Claude Opus 5.5 Co-authored-by: studio Signed-off-by: Tom Brow --- mobile/ios/Runner/NativeAddMembersSheet.swift | 9 ++++++--- .../lib/features/channels/add_members_sheet.dart | 6 +++++- .../features/channels/channels_page/sheets.dart | 4 +++- .../native_creation_tests.dart | 16 ++++++++++++++++ 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/mobile/ios/Runner/NativeAddMembersSheet.swift b/mobile/ios/Runner/NativeAddMembersSheet.swift index 84c1c75644..989ed29255 100644 --- a/mobile/ios/Runner/NativeAddMembersSheet.swift +++ b/mobile/ios/Runner/NativeAddMembersSheet.swift @@ -296,7 +296,8 @@ final class NativeAddMembersViewController: UITableViewController, UISearchResul let changed = snapshot.itemIdentifiers.filter { item in drawn[item].map { $0 != next[item] } ?? false } - snapshot.reconfigureItems(changed.filter(previous.itemIdentifiers.contains)) + let shown = Set(previous.itemIdentifiers) + snapshot.reconfigureItems(changed.filter(shown.contains)) drawn = next let animate = view.window != nil && wasChosen != chosen.map(Item.member) && !UIAccessibility.isReduceMotionEnabled @@ -327,7 +328,7 @@ final class NativeAddMembersViewController: UITableViewController, UISearchResul cell.contentView.accessibilityElementsHidden = false cell.isUserInteractionEnabled = !busy var content = UIListContentConfiguration.subtitleCell() - guard case .member(let pubkey) = item, let user = member(pubkey) else { + guard case .member(let pubkey) = item else { content.text = status content.textProperties.color = loadError && !atLimit ? Self.errorColor : .secondaryLabel content.textProperties.numberOfLines = 0 @@ -336,6 +337,7 @@ final class NativeAddMembersViewController: UITableViewController, UISearchResul cell.contentConfiguration = content return } + guard let user = member(pubkey) else { preconditionFailure("A shown member row must be in the state") } let chosen = selected.contains { $0["pubkey"] as? String == pubkey } let key = user["avatarKey"] as? String content.text = user["name"] as? String @@ -378,10 +380,11 @@ final class NativeAddMembersViewController: UITableViewController, UISearchResul override func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { tableView.deselectRow(at: indexPath, animated: true) guard !busy, let item = rows.itemIdentifier(for: indexPath) else { return } - guard case .member(let pubkey) = item, let user = member(pubkey) else { + guard case .member(let pubkey) = item else { if loadError && !atLimit { send("retry", [:]) } return } + guard let user = member(pubkey) else { preconditionFailure("A shown member row must be in the state") } if user["unavailable"] as? Bool == true { return } send("toggle", ["pubkey": pubkey]) // Where asked (New DM), a pick ends its query, as an inserted mention diff --git a/mobile/lib/features/channels/add_members_sheet.dart b/mobile/lib/features/channels/add_members_sheet.dart index b5362d9ec5..5a283b1022 100644 --- a/mobile/lib/features/channels/add_members_sheet.dart +++ b/mobile/lib/features/channels/add_members_sheet.dart @@ -161,7 +161,11 @@ class AddChannelMembersSheet extends HookConsumerWidget { void toggleUser(MentionCandidate user) { if (isSubmitting.value) return; final normalized = user.pubkey.toLowerCase(); - if (selectedPubkeys.contains(normalized)) { + // The current selection, not this build's: a second tap can land + // before the rebuild that reflects the first. + if (selectedUsers.value.any( + (selected) => selected.pubkey.toLowerCase() == normalized, + )) { selectedUsers.value = [ for (final selected in selectedUsers.value) if (selected.pubkey.toLowerCase() != normalized) selected, diff --git a/mobile/lib/features/channels/channels_page/sheets.dart b/mobile/lib/features/channels/channels_page/sheets.dart index e1f511e28c..e96edaf55a 100644 --- a/mobile/lib/features/channels/channels_page/sheets.dart +++ b/mobile/lib/features/channels/channels_page/sheets.dart @@ -461,7 +461,9 @@ class _NewDirectMessageSheet extends HookConsumerWidget { }, onToggle: (pubkey) { if (isSubmitting.value) return; - if (selectedPubkeys.contains(pubkey)) { + // The current selection, not this build's: a second tap can land + // before the rebuild that reflects the first. + if (selectedUsers.value.any((user) => user.pubkey == pubkey)) { selectedUsers.value = selectedUsers.value .where((user) => user.pubkey != pubkey) .toList(); diff --git a/mobile/test/features/channels/channels_page_test/native_creation_tests.dart b/mobile/test/features/channels/channels_page_test/native_creation_tests.dart index f9a62e1a43..d3239866e0 100644 --- a/mobile/test/features/channels/channels_page_test/native_creation_tests.dart +++ b/mobile/test/features/channels/channels_page_test/native_creation_tests.dart @@ -64,6 +64,22 @@ void registerNativeCreationTests(Widget Function(List) buildPage) { expect(found['detail'], isNull); await event('toggle', {'pubkey': _dmKey(2)}); await event('query', {'value': 'Person'}); + // Two taps before Flutter rebuilds still never list a person twice. + for (var tap = 0; tap < 2; tap++) { + await messenger.handlePlatformMessage( + bridge.name, + const StandardMethodCodec().encodeMethodCall( + MethodCall('toggle', {'session': session, 'pubkey': _dmKey(3)}), + ), + (_) {}, + ); + } + await tester.pumpAndSettle(); + final picked = [ + for (final user in states.last['selected'] as List) + (user as Map)['pubkey'], + ]; + expect(picked.toSet(), hasLength(picked.length)); for (final key in people.keys) { await event('toggle', {'pubkey': key}); }