Skip to content

Commit 26d2d3a

Browse files
committed
WIP msglist: Split into back-to-back slivers; TODO test?; TODO scroll-to-end show, act; TODO weaken test on crossing slivers
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 0f7bc7a commit 26d2d3a

File tree

1 file changed

+54
-17
lines changed

1 file changed

+54
-17
lines changed

lib/widgets/message_list.dart

Lines changed: 54 additions & 17 deletions
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)