From 93114b4279a619613a6df8ef740472216e1d4cfb Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Tue, 20 Oct 2026 15:17:52 -0700 Subject: [PATCH] fix(mobile): lazily lay out new-DM recipients (#8435) The new-message picker laid out every recipient inside an outer scroll view. Searching or changing selection could therefore mount the whole directory in one frame. Use a bounded sliver viewport so offscreen recipients stay lazy while the search field and selection chips still scroll above the keyboard. ### Validation - Current-head CI passed, including all Flutter test shards, Android, and native/release Swift jobs. Code and security reviews found no issues. After merging main and retaining both test imports, all 81 channel-page tests and the full mobile check/test suite passed. - Added a 200-person widget regression: the original mounts all 200 rows; the fix keeps fewer than 30 mounted through open, search, selection and removal, and can scroll to the final recipient. Covers no-match and closing too. - Strengthened the keyboard/chip test to verify the eight-recipient limit and that removing a chip makes the ninth recipient reachable. - All 79 channel-page tests passed; the strengthened limit test and Flutter analysis passed. - Native iOS simulator build passed. On a separately cloned paired simulator, verified populated directory/search, eight selected recipients, limit message, removal, no-match, scrolling to the final recipient and close without creating a conversation. Debug/AX instrumentation and a newer Flutter baseline make this functional validation, not a controlled release-speed benchmark. - Normal pre-push mobile checks passed, including formatting, analysis and the full Flutter test suite. Required CI exposed existing formatting in `message_carousel_preview_cases.dart`; a separate formatter-only commit corrects it. - The full local repository gate hit an unrelated Rust failure in `buzz-acp::signals_cancel_startup_turn_and_pending_stdin_without_hanging` (exit status `None` rather than `Some(143)`). No Rust code changed. The gate stopped at unit tests, so later build stages did not run. ### Related issue No duplicate recipient-layout fix found in the existing issue/PR search. ### Before and after The synthetic 200-person picker renders identically before and after (with a reserved keyboard inset). One image represents both; mounted-row assertions establish the performance mechanism separately. ![Recipient picker before and after](https://raw.githubusercontent.com/block/buzz/c9980118b75e64ee8ba0b559b7cc981ee59f809b/pr-8435--dm-directory.png) --------- Signed-off-by: Tom Brow --- .../channels/channels_page/sheets.dart | 191 +++++++++--------- .../features/channels/channels_page_test.dart | 40 +++- .../dm_recipient_layout_tests.dart | 110 ++++++++++ 3 files changed, 248 insertions(+), 93 deletions(-) create mode 100644 mobile/test/features/channels/channels_page_test/dm_recipient_layout_tests.dart diff --git a/mobile/lib/features/channels/channels_page/sheets.dart b/mobile/lib/features/channels/channels_page/sheets.dart index 0072958c5b..0c08ff6bfb 100644 --- a/mobile/lib/features/channels/channels_page/sheets.dart +++ b/mobile/lib/features/channels/channels_page/sheets.dart @@ -535,12 +535,14 @@ class _NewDirectMessageSheet extends HookConsumerWidget { ), child: SafeArea( top: false, - child: SingleChildScrollView( - child: Column( - mainAxisSize: MainAxisSize.min, - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - GestureDetector( + // The sheet bounds this viewport even with the keyboard visible. + // Keep rows as slivers so search and chip updates do not lay out the + // entire directory. The field still scrolls with long chip selections. + child: CustomScrollView( + shrinkWrap: true, + slivers: [ + SliverToBoxAdapter( + child: GestureDetector( behavior: HitTestBehavior.translucent, onTap: isSubmitting.value ? null : queryFocusNode.requestFocus, child: SizedBox( @@ -664,24 +666,27 @@ class _NewDirectMessageSheet extends HookConsumerWidget { ), ), ), - const SizedBox(height: Grid.xs), - Builder( - key: const Key('new-dm-results'), - builder: (context) { - if (selectedUsers.value.length >= 8) { - return const SizedBox( + ), + const SliverToBoxAdapter(child: SizedBox(height: Grid.xs)), + Builder( + key: const Key('new-dm-results'), + builder: (context) { + if (selectedUsers.value.length >= 8) { + return const SliverToBoxAdapter( + child: SizedBox( height: 96, child: Center( child: Text( 'DMs support up to nine people, including you.', ), ), - ); - } - if (directoryAsync.isLoading && - directoryAsync.asData == null || - isSearchTransitionPending) { - return const SizedBox( + ), + ); + } + if (directoryAsync.isLoading && directoryAsync.asData == null || + isSearchTransitionPending) { + return const SliverToBoxAdapter( + child: SizedBox( height: 280, child: Center( child: BuzzLoadingIndicator( @@ -689,10 +694,12 @@ class _NewDirectMessageSheet extends HookConsumerWidget { semanticLabel: 'Loading people', ), ), - ); - } - if (directoryAsync.hasError) { - return SizedBox( + ), + ); + } + if (directoryAsync.hasError) { + return SliverToBoxAdapter( + child: SizedBox( height: 280, child: Center( child: Column( @@ -719,10 +726,12 @@ class _NewDirectMessageSheet extends HookConsumerWidget { ], ), ), - ); - } - if (availableResults.isEmpty) { - return SizedBox( + ), + ); + } + if (availableResults.isEmpty) { + return SliverToBoxAdapter( + child: SizedBox( height: 280, child: Center( child: Text( @@ -731,78 +740,78 @@ class _NewDirectMessageSheet extends HookConsumerWidget { : 'No matching users.', ), ), - ); - } - return ListView.builder( - physics: const NeverScrollableScrollPhysics(), - shrinkWrap: true, - itemCount: availableResults.length, - itemBuilder: (context, index) { - final user = availableResults[index]; - return AppListCardItem( - index: index, - itemCount: availableResults.length, - dividerIndent: Grid.xs + 40 + Grid.xs, - child: ListTile( - key: Key('new-dm-person-${user.pubkey}'), - leading: AvatarImage( - imageUrl: user.avatarUrl, - radius: 20, - backgroundColor: context.controlSurface, - fallback: Text( - user.initial, - style: context.textTheme.labelLarge?.copyWith( - color: context.colors.onPrimaryContainer, - fontWeight: FontWeight.w600, - ), - ), - isAgent: user.isAgent, - ), - title: Text( - names.labelFor(user.pubkey), - maxLines: 1, - overflow: TextOverflow.ellipsis, - ), - subtitle: Text( - user.secondaryLabel, - maxLines: 1, - overflow: TextOverflow.ellipsis, - ), - trailing: Icon( - BuzzIcons.plus, - size: 18, - color: context.colors.onSurfaceVariant, - ), - onTap: isSubmitting.value - ? null - : () { - selectedUsers.value = [ - ...selectedUsers.value, - user, - ]; - queryController.clear(); - query.value = ''; - debouncedQuery.value = ''; - submitError.value = null; - queryFocusNode.requestFocus(); - }, - ), - ); - }, + ), ); - }, - ), - if (submitError.value case final error?) ...[ - const SizedBox(height: Grid.xxs), - Text( + } + return SliverList.builder( + itemCount: availableResults.length, + itemBuilder: (context, index) { + final user = availableResults[index]; + return AppListCardItem( + index: index, + itemCount: availableResults.length, + dividerIndent: Grid.xs + 40 + Grid.xs, + child: ListTile( + key: Key('new-dm-person-${user.pubkey}'), + leading: AvatarImage( + imageUrl: user.avatarUrl, + radius: 20, + backgroundColor: context.controlSurface, + fallback: Text( + user.initial, + style: context.textTheme.labelLarge?.copyWith( + color: context.colors.onPrimaryContainer, + fontWeight: FontWeight.w600, + ), + ), + isAgent: user.isAgent, + ), + title: Text( + names.labelFor(user.pubkey), + maxLines: 1, + overflow: TextOverflow.ellipsis, + ), + subtitle: Text( + user.secondaryLabel, + maxLines: 1, + overflow: TextOverflow.ellipsis, + ), + trailing: Icon( + BuzzIcons.plus, + size: 18, + color: context.colors.onSurfaceVariant, + ), + onTap: isSubmitting.value + ? null + : () { + selectedUsers.value = [ + ...selectedUsers.value, + user, + ]; + queryController.clear(); + query.value = ''; + debouncedQuery.value = ''; + submitError.value = null; + queryFocusNode.requestFocus(); + }, + ), + ); + }, + ); + }, + ), + if (submitError.value case final error?) ...[ + const SliverToBoxAdapter(child: SizedBox(height: Grid.xxs)), + SliverToBoxAdapter( + child: Text( error, style: context.textTheme.bodySmall?.copyWith( color: context.colors.error, ), ), - ], + ), ], - ), + ], ), ), ); diff --git a/mobile/test/features/channels/channels_page_test.dart b/mobile/test/features/channels/channels_page_test.dart index 61fe9c746d..6a79458c1c 100644 --- a/mobile/test/features/channels/channels_page_test.dart +++ b/mobile/test/features/channels/channels_page_test.dart @@ -59,6 +59,7 @@ import 'package:buzz/shared/widgets/skeleton.dart'; import '../../helpers/thread_ids.dart'; part 'channels_page_test/presence_tests.dart'; +part 'channels_page_test/dm_recipient_layout_tests.dart'; part 'channels_page_test/discovery_journey_tests.dart'; part 'channels_page_test/unread_readiness_tests.dart'; @@ -3360,6 +3361,16 @@ void main() { ); }); + dmRecipientLayoutTests( + (overrides) => buildTestable( + keyboardInset: 300, + overrides: [ + channelsProvider.overrideWith(() => _FakeNotifier(testChannels)), + ...overrides, + ], + ), + ); + testWidgets('new message sheet lists and selects relay members', ( tester, ) async { @@ -3537,7 +3548,7 @@ void main() { addTearDown(tester.view.resetDevicePixelRatio); final directoryUsers = [ - for (var index = 0; index < 8; index++) + for (var index = 0; index < 9; index++) DirectoryUser( pubkey: 'person-$index', displayName: 'Long recipient name number $index', @@ -3562,7 +3573,7 @@ void main() { await tester.tap(find.text('New direct message')); await tester.pumpAndSettle(); - for (final user in directoryUsers) { + for (final user in directoryUsers.take(8)) { final result = find.byKey(Key('new-dm-person-${user.pubkey}')); await tester.ensureVisible(result); await tester.pumpAndSettle(); @@ -3579,6 +3590,31 @@ void main() { find.text('DMs support up to nine people, including you.'), findsOneWidget, ); + expect(find.byKey(const Key('new-dm-person-person-8')), findsNothing); + final lastChip = find.byKey(const Key('new-dm-selected-person-7')); + await tester.ensureVisible(lastChip); + await tester.pumpAndSettle(); + await tester.tap(lastChip); + await tester.pumpAndSettle(); + expect(find.byKey(const Key('new-dm-selected-person-7')), findsNothing); + final ninthRecipient = find.byKey(const Key('new-dm-person-person-8')); + final listScroll = find + .descendant( + of: find.byType(BottomSheet).last, + matching: find.byWidgetPredicate( + (widget) => + widget is Scrollable && + widget.axisDirection == AxisDirection.down, + ), + ) + .last; + await tester.scrollUntilVisible( + ninthRecipient, + 100, + scrollable: listScroll, + ); + await tester.pumpAndSettle(); + expect(ninthRecipient.hitTestable(), findsOneWidget); expect(tester.takeException(), isNull); }); diff --git a/mobile/test/features/channels/channels_page_test/dm_recipient_layout_tests.dart b/mobile/test/features/channels/channels_page_test/dm_recipient_layout_tests.dart new file mode 100644 index 0000000000..d3a4334a3a --- /dev/null +++ b/mobile/test/features/channels/channels_page_test/dm_recipient_layout_tests.dart @@ -0,0 +1,110 @@ +part of '../channels_page_test.dart'; + +void dmRecipientLayoutTests(Widget Function(List) harness) { + testWidgets( + 'recipient directory lays out only visible rows and reaches end', + (tester) async { + tester.view.physicalSize = const Size(390, 800); + tester.view.devicePixelRatio = 1; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + final users = [ + for (var i = 0; i < 200; i++) + DirectoryUser(pubkey: 'person-$i', displayName: 'Person $i'), + ]; + await tester.pumpWidget( + harness([ + relayDirectoryUsersProvider.overrideWith((ref) async => users), + relayDirectorySearchProvider.overrideWith( + (ref, query) async => users, + ), + ]), + ); + await tester.pumpAndSettle(); + await tester.tap(find.byTooltip('Create or start conversation')); + await tester.pumpAndSettle(); + await tester.tap(find.text('New direct message')); + await tester.pumpAndSettle(); + + if (Platform.environment['COMMUNITY_SCREENSHOTS'] case final directory?) { + final boundary = tester + .element(find.byType(BottomSheet).last) + .findAncestorRenderObjectOfType()!; + await tester.runAsync(() async { + final image = await boundary.toImage(pixelRatio: 2); + final bytes = await image.toByteData(format: ui.ImageByteFormat.png); + await Directory(directory).create(recursive: true); + await File( + '$directory/dm-directory.png', + ).writeAsBytes(bytes!.buffer.asUint8List()); + image.dispose(); + }); + } + + final rows = find.byWidgetPredicate( + (widget) => + widget is ListTile && + widget.key is ValueKey && + (widget.key! as ValueKey).value.startsWith( + 'new-dm-person-', + ), + ); + void expectBoundedRows() { + expect(rows.evaluate().length, inExclusiveRange(0, 30)); + expect(find.byKey(const Key('new-dm-person-person-199')), findsNothing); + } + + expectBoundedRows(); + final search = find.byKey(const Key('new-dm-search')); + await tester.enterText(search, 'Person'); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pumpAndSettle(); + expectBoundedRows(); + await tester.tap(find.byKey(const Key('new-dm-person-person-0'))); + await tester.pumpAndSettle(); + expect(find.bySemanticsLabel('Remove Person 0'), findsOneWidget); + expectBoundedRows(); + await tester.tap(find.bySemanticsLabel('Remove Person 0')); + await tester.pumpAndSettle(); + expectBoundedRows(); + + await tester.enterText(search, 'no such recipient'); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pumpAndSettle(); + expect(find.text('No matching users.'), findsOneWidget); + expect(rows, findsNothing); + await tester.enterText(search, ''); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pumpAndSettle(); + expectBoundedRows(); + + final scrollable = find + .descendant( + of: find.byType(BottomSheet).last, + matching: find.byWidgetPredicate( + (widget) => + widget is Scrollable && + widget.axisDirection == AxisDirection.down, + ), + ) + .last; + await tester.scrollUntilVisible( + find.byKey(const Key('new-dm-person-person-199')), + 300, + scrollable: scrollable, + maxScrolls: 100, + ); + await tester.pumpAndSettle(); + expect( + find.byKey(const Key('new-dm-person-person-199')).hitTestable(), + findsOneWidget, + ); + expect(rows.evaluate().length, lessThan(30)); + expect(tester.takeException(), isNull); + + await tester.tap(find.byTooltip('Close sheet')); + await tester.pumpAndSettle(); + expect(find.text('New message'), findsNothing); + }, + ); +}