mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
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.  --------- Signed-off-by: Tom Brow <tomb@block.xyz>
This commit is contained in:
@@ -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),
|
||||
),
|
||||
const SliverToBoxAdapter(child: SizedBox(height: Grid.xs)),
|
||||
Builder(
|
||||
key: const Key('new-dm-results'),
|
||||
builder: (context) {
|
||||
if (selectedUsers.value.length >= 8) {
|
||||
return const SizedBox(
|
||||
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 ||
|
||||
if (directoryAsync.isLoading && directoryAsync.asData == null ||
|
||||
isSearchTransitionPending) {
|
||||
return const SizedBox(
|
||||
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(
|
||||
return SliverToBoxAdapter(
|
||||
child: SizedBox(
|
||||
height: 280,
|
||||
child: Center(
|
||||
child: Column(
|
||||
@@ -719,10 +726,12 @@ class _NewDirectMessageSheet extends HookConsumerWidget {
|
||||
],
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
if (availableResults.isEmpty) {
|
||||
return SizedBox(
|
||||
return SliverToBoxAdapter(
|
||||
child: SizedBox(
|
||||
height: 280,
|
||||
child: Center(
|
||||
child: Text(
|
||||
@@ -731,11 +740,10 @@ class _NewDirectMessageSheet extends HookConsumerWidget {
|
||||
: 'No matching users.',
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
return ListView.builder(
|
||||
physics: const NeverScrollableScrollPhysics(),
|
||||
shrinkWrap: true,
|
||||
return SliverList.builder(
|
||||
itemCount: availableResults.length,
|
||||
itemBuilder: (context, index) {
|
||||
final user = availableResults[index];
|
||||
@@ -793,16 +801,17 @@ class _NewDirectMessageSheet extends HookConsumerWidget {
|
||||
},
|
||||
),
|
||||
if (submitError.value case final error?) ...[
|
||||
const SizedBox(height: Grid.xxs),
|
||||
Text(
|
||||
const SliverToBoxAdapter(child: SizedBox(height: Grid.xxs)),
|
||||
SliverToBoxAdapter(
|
||||
child: Text(
|
||||
error,
|
||||
style: context.textTheme.bodySmall?.copyWith(
|
||||
color: context.colors.error,
|
||||
),
|
||||
),
|
||||
],
|
||||
],
|
||||
),
|
||||
],
|
||||
],
|
||||
),
|
||||
),
|
||||
);
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
part of '../channels_page_test.dart';
|
||||
|
||||
void dmRecipientLayoutTests(Widget Function(List<Override>) 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<RenderRepaintBoundary>()!;
|
||||
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<String> &&
|
||||
(widget.key! as ValueKey<String>).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);
|
||||
},
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user