Skip to content

Commit

Permalink
msglist [nfc]: Let ScrollView count forward, and sliver handle reverse
Browse files Browse the repository at this point in the history
This means that the overall scroll view has AxisDirection.down
as the scroll direction, rather than AxisDirection.up, and the
SliverStickyHeader is passed GrowthDirection.reverse instead
of GrowthDirection.forward in order to get the same effect.

This makes the scrolling a little easier to think about when focused
on MessageList's code, because for example (as seen in this diff)
`scrollMetrics.extentBefore` now refers to the messages that are
older than the ones on screen, rather than those that are newer.

This also brings us closer to the setup we'll want for zulip#80 and zulip#82,
opening the message list at an anchor other than the end: the empty
placeholder sliver at the bottom will become the list of messages
after the starting anchor, while the sliver at the top will remain
the list of messages above the anchor.
  • Loading branch information
gnprice authored and chrisbobbe committed Jan 30, 2024
1 parent 03e5f07 commit bdac26f
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 14 deletions.
22 changes: 11 additions & 11 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -212,13 +212,13 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
}

void _adjustButtonVisibility(ScrollMetrics scrollMetrics) {
if (scrollMetrics.extentBefore == 0) {
if (scrollMetrics.extentAfter == 0) {
_scrollToBottomVisibleValue.value = false;
} else {
_scrollToBottomVisibleValue.value = true;
}

if (scrollMetrics.extentAfter < kFetchMessagesBufferPixels) {
if (scrollMetrics.extentBefore < kFetchMessagesBufferPixels) {
// TODO: This ends up firing a second time shortly after we fetch a batch.
// The result is that each time we decide to fetch a batch, we end up
// fetching two batches in quick succession. This is basically harmless
Expand Down Expand Up @@ -278,6 +278,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat

Widget _buildListView(context) {
final length = model!.items.length;
const centerSliverKey = ValueKey('center sliver');
return CustomScrollView(
// TODO: Offer `ScrollViewKeyboardDismissBehavior.interactive` (or
// similar) if that is ever offered:
Expand All @@ -292,18 +293,12 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat

controller: scrollController,
semanticChildCount: length + 2,

// Setting reverse: true means the scroll starts at the bottom.
// Flipping the indexes (in the SliverChildBuilderDelegate callback)
// means the start/bottom has the latest messages.
// This works great when we want to start from the latest.
// TODO handle scroll starting at first unread, or link anchor
// TODO on new message when scrolled up, anchor scroll to what's in view
reverse: true,
anchor: 1.0,
center: centerSliverKey,

slivers: [
SliverStickyHeaderList(
headerPlacement: HeaderPlacement.scrollingEnd,
headerPlacement: HeaderPlacement.scrollingStart,
delegate: SliverChildBuilderDelegate(
// To preserve state across rebuilds for individual [MessageItem]
// widgets as the size of [MessageListView.items] changes we need
Expand Down Expand Up @@ -337,6 +332,11 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
final data = model!.items[length - 1 - (i - 2)];
return _buildItem(data, i);
})),

// This is a trivial placeholder that occupies no space. Its purpose is
// to have the key that's passed to [ScrollView.center], and so to cause
// the above [SliverStickyHeaderList] to run from bottom to top.
const SliverToBoxAdapter(key: centerSliverKey),
]);
}

Expand Down
6 changes: 3 additions & 3 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ void main() {
// Initial state should be not visible, as the message list renders with latest message in view
check(isButtonVisible(tester)).equals(false);

scrollController.jumpTo(600);
scrollController.jumpTo(-600);
await tester.pump();
check(isButtonVisible(tester)).equals(true);

Expand All @@ -165,7 +165,7 @@ void main() {
// Initial state should be not visible, as the message list renders with latest message in view
check(isButtonVisible(tester)).equals(false);

scrollController.jumpTo(600);
scrollController.jumpTo(-600);
await tester.pump();
check(isButtonVisible(tester)).equals(true);

Expand All @@ -187,7 +187,7 @@ void main() {
// Initial state should be not visible, as the message list renders with latest message in view
check(isButtonVisible(tester)).equals(false);

scrollController.jumpTo(600);
scrollController.jumpTo(-600);
await tester.pump();
check(isButtonVisible(tester)).equals(true);

Expand Down

0 comments on commit bdac26f

Please sign in to comment.