Skip to content

Commit 6a8cf5c

Browse files
gnpricechrisbobbe
authored andcommitted
msglist [nfc]: Unpack to a CustomScrollView of a single sliver
This is effectively what StickyHeaderListView.builder was abbreviating. This expanded form will then give us flexibility to introduce a second sliver.
1 parent c8242dd commit 6a8cf5c

File tree

2 files changed

+46
-39
lines changed

2 files changed

+46
-39
lines changed

lib/widgets/message_list.dart

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
279279

280280
Widget _buildListView(context) {
281281
final length = model!.items.length;
282-
return StickyHeaderListView.builder(
282+
return CustomScrollView(
283283
// TODO: Offer `ScrollViewKeyboardDismissBehavior.interactive` (or
284284
// similar) if that is ever offered:
285285
// https://github.com/flutter/flutter/issues/57609#issuecomment-1355340849
@@ -291,46 +291,54 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
291291
_ => ScrollViewKeyboardDismissBehavior.manual,
292292
},
293293

294-
// To preserve state across rebuilds for individual [MessageItem]
295-
// widgets as the size of [MessageListView.items] changes we need
296-
// to match old widgets by their key to their new position in
297-
// the list.
298-
//
299-
// The keys are of type [ValueKey] with a value of [Message.id]
300-
// and here we use a O(log n) binary search method. This could
301-
// be improved but for now it only triggers for materialized
302-
// widgets. As a simple test, flinging through All Messages in
303-
// CZO on a Pixel 5, this only runs about 10 times per rebuild
304-
// and the timing for each call is <100 microseconds.
305-
//
306-
// Non-message items (e.g., start and end markers) that do not
307-
// have state that needs to be preserved have not been given keys
308-
// and will not trigger this callback.
309-
findChildIndexCallback: (Key key) {
310-
final valueKey = key as ValueKey;
311-
final index = model!.findItemWithMessageId(valueKey.value);
312-
if (index == -1) return null;
313-
return length - 1 - (index - 2);
314-
},
315294
controller: scrollController,
316-
itemCount: length + 2,
295+
semanticChildCount: length + 2,
296+
317297
// Setting reverse: true means the scroll starts at the bottom.
318-
// Flipping the indexes (in itemBuilder) means the start/bottom
319-
// has the latest messages.
298+
// Flipping the indexes (in the SliverChildBuilderDelegate callback)
299+
// means the start/bottom has the latest messages.
320300
// This works great when we want to start from the latest.
321301
// TODO handle scroll starting at first unread, or link anchor
322302
// TODO on new message when scrolled up, anchor scroll to what's in view
323303
reverse: true,
324-
itemBuilder: (context, i) {
325-
// To reinforce that the end of the feed has been reached:
326-
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680603
327-
if (i == 0) return const SizedBox(height: 36);
328304

329-
if (i == 1) return MarkAsReadWidget(narrow: widget.narrow);
330-
331-
final data = model!.items[length - 1 - (i - 2)];
332-
return _buildItem(data, i);
333-
});
305+
slivers: [
306+
SliverStickyHeaderList(
307+
headerPlacement: HeaderPlacement.scrollingEnd,
308+
delegate: SliverChildBuilderDelegate(
309+
// To preserve state across rebuilds for individual [MessageItem]
310+
// widgets as the size of [MessageListView.items] changes we need
311+
// to match old widgets by their key to their new position in
312+
// the list.
313+
//
314+
// The keys are of type [ValueKey] with a value of [Message.id]
315+
// and here we use a O(log n) binary search method. This could
316+
// be improved but for now it only triggers for materialized
317+
// widgets. As a simple test, flinging through All Messages in
318+
// CZO on a Pixel 5, this only runs about 10 times per rebuild
319+
// and the timing for each call is <100 microseconds.
320+
//
321+
// Non-message items (e.g., start and end markers) that do not
322+
// have state that needs to be preserved have not been given keys
323+
// and will not trigger this callback.
324+
findChildIndexCallback: (Key key) {
325+
final valueKey = key as ValueKey;
326+
final index = model!.findItemWithMessageId(valueKey.value);
327+
if (index == -1) return null;
328+
return length - 1 - (index - 2);
329+
},
330+
childCount: length + 2,
331+
(context, i) {
332+
// To reinforce that the end of the feed has been reached:
333+
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680603
334+
if (i == 0) return const SizedBox(height: 36);
335+
336+
if (i == 1) return MarkAsReadWidget(narrow: widget.narrow);
337+
338+
final data = model!.items[length - 1 - (i - 2)];
339+
return _buildItem(data, i);
340+
})),
341+
]);
334342
}
335343

336344
Widget _buildItem(MessageListItem data, int i) {

test/widgets/message_list_test.dart

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import 'package:zulip/model/store.dart';
1717
import 'package:zulip/widgets/content.dart';
1818
import 'package:zulip/widgets/icons.dart';
1919
import 'package:zulip/widgets/message_list.dart';
20-
import 'package:zulip/widgets/sticky_header.dart';
2120
import 'package:zulip/widgets/store.dart';
2221

2322
import '../api/fake_api.dart';
@@ -76,13 +75,13 @@ void main() {
7675
}
7776

7877
ScrollController? findMessageListScrollController(WidgetTester tester) {
79-
final stickyHeaderListView = tester.widget<StickyHeaderListView>(find.byType(StickyHeaderListView));
80-
return stickyHeaderListView.controller;
78+
final scrollView = tester.widget<CustomScrollView>(find.byType(CustomScrollView));
79+
return scrollView.controller;
8180
}
8281

8382
group('fetch older messages on scroll', () {
8483
int? itemCount(WidgetTester tester) =>
85-
tester.widget<StickyHeaderListView>(find.byType(StickyHeaderListView)).semanticChildCount;
84+
tester.widget<CustomScrollView>(find.byType(CustomScrollView)).semanticChildCount;
8685

8786
testWidgets('basic', (tester) async {
8887
await setupMessageListPage(tester, foundOldest: false,
@@ -527,7 +526,7 @@ void main() {
527526
testWidgets('animation state persistence', (WidgetTester tester) async {
528527
// Check that _UnreadMarker maintains its in-progress animation
529528
// as the number of items changes in MessageList. See
530-
// `findChildIndexCallback` passed into [StickyHeaderListView.builder]
529+
// `findChildIndexCallback` passed into [SliverStickyHeaderList]
531530
// at [_MessageListState._buildListView].
532531
final message = eg.streamMessage(flags: []);
533532
await setupMessageListPage(tester, messages: [message]);

0 commit comments

Comments
 (0)