Skip to content

Commit 17c1fea

Browse files
committed
wip msglist: Split into back-to-back slivers; TODO test?
Thanks to all the preparatory changes we've made in this commit series and those that came before it, this change should have only subtle effects on user-visible behavior. In particular: * The sticky headers should behave exactly as they did before, even when the sliver boundary lies under the sticky header, thanks to several previous commit series. * On first loading a given message list, it should start out scrolled to the end, just as before. * When already scrolled to the end, the message list should stay there when a new message arrives, or a message is edited, etc., even if the (new) latest message is taller than it was. Subtle differences include: * When scrolled up from the bottom and a new message comes in, the behavior is slightly different from before. The current exact behavior is something we probably want to change anyway, and I think the new behavior isn't particularly better or worse. * The behavior upon overscroll might be slightly different; I'm not sure. This therefore serves as a testing ground for all the ways that the message list's scrolling behavior can become more complicated when two (nontrivial) slivers are involved. If we find a situation where the behavior does change noticeably, that will be something to investigate and fix.
1 parent 5ed586c commit 17c1fea

File tree

1 file changed

+54
-17
lines changed

1 file changed

+54
-17
lines changed

lib/widgets/message_list.dart

+54-17
Original file line numberDiff line numberDiff line change
@@ -583,11 +583,18 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
583583
}
584584

585585
Widget _buildListView(BuildContext context) {
586+
const bottomSize = 1;
586587
final length = model!.items.length;
588+
final bottomLength = length <= bottomSize ? length : bottomSize;
589+
final topLength = length - bottomLength;
587590
const centerSliverKey = ValueKey('center sliver');
588591
final zulipLocalizations = ZulipLocalizations.of(context);
589592

590-
Widget sliver = SliverStickyHeaderList(
593+
// TODO(#311) If we have a bottom nav, it will pad the bottom inset,
594+
// and this can be removed; also remove mention in MessageList dartdoc
595+
final needSafeArea = !ComposeBox.hasComposeBox(widget.narrow);
596+
597+
final topSliver = SliverStickyHeaderList(
591598
headerPlacement: HeaderPlacement.scrollingStart,
592599
delegate: SliverChildBuilderDelegate(
593600
// To preserve state across rebuilds for individual [MessageItem]
@@ -609,26 +616,60 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
609616
final valueKey = key as ValueKey<int>;
610617
final index = model!.findItemWithMessageId(valueKey.value);
611618
if (index == -1) return null;
612-
return length - 1 - (index - 3);
619+
final i = length - 1 - (index + bottomLength);
620+
if (i < 0) return null;
621+
return i;
622+
},
623+
childCount: topLength,
624+
(context, i) {
625+
final data = model!.items[length - 1 - (i + bottomLength)];
626+
final item = _buildItem(zulipLocalizations, data);
627+
return item;
628+
}));
629+
630+
Widget bottomSliver = SliverStickyHeaderList(
631+
key: needSafeArea ? null : centerSliverKey,
632+
headerPlacement: HeaderPlacement.scrollingStart,
633+
delegate: SliverChildBuilderDelegate(
634+
// To preserve state across rebuilds for individual [MessageItem]
635+
// widgets as the size of [MessageListView.items] changes we need
636+
// to match old widgets by their key to their new position in
637+
// the list.
638+
//
639+
// The keys are of type [ValueKey] with a value of [Message.id]
640+
// and here we use a O(log n) binary search method. This could
641+
// be improved but for now it only triggers for materialized
642+
// widgets. As a simple test, flinging through All Messages in
643+
// CZO on a Pixel 5, this only runs about 10 times per rebuild
644+
// and the timing for each call is <100 microseconds.
645+
//
646+
// Non-message items (e.g., start and end markers) that do not
647+
// have state that needs to be preserved have not been given keys
648+
// and will not trigger this callback.
649+
findChildIndexCallback: (Key key) {
650+
final valueKey = key as ValueKey<int>;
651+
final index = model!.findItemWithMessageId(valueKey.value);
652+
if (index == -1) return null;
653+
final i = index - topLength;
654+
if (i < 0) return null;
655+
return i;
613656
},
614-
childCount: length + 3,
657+
childCount: bottomLength + 3,
615658
(context, i) {
616659
// To reinforce that the end of the feed has been reached:
617660
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680603
618-
if (i == 0) return const SizedBox(height: 36);
661+
if (i == bottomLength + 2) return const SizedBox(height: 36);
619662

620-
if (i == 1) return MarkAsReadWidget(narrow: widget.narrow);
663+
if (i == bottomLength + 1) return MarkAsReadWidget(narrow: widget.narrow);
621664

622-
if (i == 2) return TypingStatusWidget(narrow: widget.narrow);
665+
if (i == bottomLength) return TypingStatusWidget(narrow: widget.narrow);
623666

624-
final data = model!.items[length - 1 - (i - 3)];
667+
final data = model!.items[topLength + i];
625668
return _buildItem(zulipLocalizations, data);
626669
}));
627670

628-
if (!ComposeBox.hasComposeBox(widget.narrow)) {
629-
// TODO(#311) If we have a bottom nav, it will pad the bottom inset,
630-
// and this can be removed; also remove mention in MessageList dartdoc
631-
sliver = SliverSafeArea(sliver: sliver);
671+
if (needSafeArea) {
672+
bottomSliver = SliverSafeArea(key: centerSliverKey, sliver: bottomSliver);
632673
}
633674

634675
return MessageListScrollView(
@@ -649,12 +690,8 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
649690
paintOrder: SliverPaintOrder.firstIsTop,
650691

651692
slivers: [
652-
sliver,
653-
654-
// This is a trivial placeholder that occupies no space. Its purpose is
655-
// to have the key that's passed to [ScrollView.center], and so to cause
656-
// the above [SliverStickyHeaderList] to run from bottom to top.
657-
const SliverToBoxAdapter(key: centerSliverKey),
693+
topSliver,
694+
bottomSliver,
658695
]);
659696
}
660697

0 commit comments

Comments
 (0)