Skip to content

Commit 52d6909

Browse files
committed
scroll: Show start of latest message if long, instead of end
This makes our first payoff in actual UX from having the message list split into two back-to-back slivers! With this change, if you open a message list and the latest message is very tall, the list starts out scrolled so that you can see the top of that latest message -- plus a bit of context above it (25% of the viewport's height). Previously the list would always start out scrolled to the end, so you'd have to scroll up in order to read even the one latest message from the beginning. In addition to a small UX improvement now, this makes a preview of behavior we'll want to have when the bottom sliver starts at the first unread message, and may have many messages after that. This new behavior is nice already with one message, if the message happens to be very tall; but it'll become critical when the bottom sliver is routinely many screenfuls tall.
1 parent fca651b commit 52d6909

File tree

2 files changed

+65
-21
lines changed

2 files changed

+65
-21
lines changed

lib/widgets/scrolling.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,10 +245,13 @@ class MessageListScrollPosition extends ScrollPositionWithSingleContext {
245245

246246
if (!_hasEverCompletedLayout) {
247247
// The list is being laid out for the first time (its first performLayout).
248-
// Start out scrolled to the end.
248+
// Start out scrolled down so the bottom sliver (the new messages)
249+
// occupies 75% of the viewport,
250+
// or at the in-range scroll position closest to that.
249251
// This also brings [pixels] within bounds, which
250252
// the initial value of 0.0 might not have been.
251-
final target = maxScrollExtent;
253+
final target = clampDouble(0.75 * viewportDimension,
254+
minScrollExtent, maxScrollExtent);
252255
if (!hasPixels || pixels != target) {
253256
correctPixels(target);
254257
changed = true;

test/widgets/scrolling_test.dart

Lines changed: 60 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -64,20 +64,58 @@ void main() {
6464
});
6565

6666
testWidgets('short/long -> scrolls to ends and no farther', (tester) async {
67-
// Starts out scrolled to bottom.
67+
// Starts out scrolled to top (to show top of the bottom sliver).
6868
await prepare(tester, topHeight: 100, bottomHeight: 800);
69-
check(tester.getRect(findBottom)).bottom.equals(600);
69+
check(tester.getRect(findTop)).top.equals(0);
70+
check(tester.getRect(findBottom)).bottom.equals(900);
7071

71-
// Try scrolling down (by dragging up); doesn't move.
72-
await tester.drag(findBottom, Offset(0, -100));
72+
// Try scrolling up (by dragging down); doesn't move.
73+
await tester.drag(findBottom, Offset(0, 100));
7374
await tester.pump();
74-
check(tester.getRect(findBottom)).bottom.equals(600);
75+
check(tester.getRect(findBottom)).bottom.equals(900);
7576

76-
// Try scrolling up (by dragging down); moves only as far as top of list.
77-
await tester.drag(findBottom, Offset(0, 400));
77+
// Try scrolling down (by dragging up); moves only as far as bottom of list.
78+
await tester.drag(findBottom, Offset(0, -400));
7879
await tester.pump();
79-
check(tester.getRect(findBottom)).bottom.equals(900);
80+
check(tester.getRect(findBottom)).bottom.equals(600);
81+
});
82+
83+
testWidgets('starts by showing top of bottom sliver, long/long', (tester) async {
84+
// Both slivers are long; the bottom sliver gets 75% of the viewport.
85+
await prepare(tester, topHeight: 1000, bottomHeight: 3000);
86+
check(tester.getRect(findBottom)).top.equals(150);
87+
});
88+
89+
testWidgets('starts by showing top of bottom sliver, short/long', (tester) async {
90+
// The top sliver is shorter than 25% of the viewport.
91+
// It's shown in full, and the bottom sliver gets the rest (so >75%).
92+
await prepare(tester, topHeight: 50, bottomHeight: 3000);
8093
check(tester.getRect(findTop)).top.equals(0);
94+
check(tester.getRect(findBottom)).top.equals(50);
95+
});
96+
97+
testWidgets('starts by showing top of bottom sliver, short/medium', (tester) async {
98+
// The whole list fits in the viewport. It's pinned to the bottom,
99+
// even when that gives the bottom sliver more than 75%.
100+
await prepare(tester, topHeight: 50, bottomHeight: 500);
101+
check(tester.getRect(findTop))..top.equals(50)..bottom.equals(100);
102+
check(tester.getRect(findBottom)).bottom.equals(600);
103+
});
104+
105+
testWidgets('starts by showing top of bottom sliver, medium/short', (tester) async {
106+
// The whole list fits in the viewport. It's pinned to the bottom,
107+
// even when that gives the top sliver more than 25%.
108+
await prepare(tester, topHeight: 300, bottomHeight: 100);
109+
check(tester.getRect(findTop))..top.equals(200)..bottom.equals(500);
110+
check(tester.getRect(findBottom)).bottom.equals(600);
111+
});
112+
113+
testWidgets('starts by showing top of bottom sliver, long/short', (tester) async {
114+
// The bottom sliver is shorter than 75% of the viewport.
115+
// It's shown in full, and the top sliver gets the rest (so >25%).
116+
await prepare(tester, topHeight: 1000, bottomHeight: 300);
117+
check(tester.getRect(findTop)).bottom.equals(300);
118+
check(tester.getRect(findBottom)).bottom.equals(600);
81119
});
82120

83121
testWidgets('short/short -> starts at bottom, immediately without animation', (tester) async {
@@ -91,43 +129,46 @@ void main() {
91129
check(ys).deepEquals(List.generate(10, (_) => 0.0));
92130
});
93131

94-
testWidgets('short/long -> starts at bottom, immediately without animation', (tester) async {
132+
testWidgets('short/long -> starts at desired start, immediately without animation', (tester) async {
95133
await prepare(tester, topHeight: 100, bottomHeight: 800);
96134

97135
final ys = <double>[];
98136
for (int i = 0; i < 10; i++) {
99-
ys.add(tester.getRect(findBottom).bottom - 600);
137+
ys.add(tester.getRect(findTop).top);
100138
await tester.pump(Duration(milliseconds: 15));
101139
}
102140
check(ys).deepEquals(List.generate(10, (_) => 0.0));
103141
});
104142

105-
testWidgets('starts at bottom, even when bottom underestimated at first', (tester) async {
143+
testWidgets('starts at desired start, even when bottom underestimated at first', (tester) async {
106144
const numItems = 10;
107-
const itemHeight = 300.0;
145+
const itemHeight = 20.0;
108146

109147
// A list where the bottom sliver takes several rounds of layout
110148
// to see how long it really is.
111149
final controller = MessageListScrollController();
112150
await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr,
113151
child: MessageListScrollView(
114152
controller: controller,
153+
// The tiny cacheExtent causes each layout round to only reach
154+
// the first item it expects will go beyond the viewport.
155+
cacheExtent: 1.0, // in (logical) pixels!
115156
center: const ValueKey('center'),
116157
slivers: [
117158
SliverToBoxAdapter(
118-
child: SizedBox(height: 100, child: Text('top'))),
159+
child: SizedBox(height: 300, child: Text('top'))),
119160
SliverList.list(key: const ValueKey('center'),
120161
children: List.generate(numItems, (i) =>
121162
SizedBox(height: (i+1) * itemHeight, child: Text('item $i')))),
122163
])));
123164
await tester.pump();
124165

125-
// Starts out scrolled all the way to the bottom,
126-
// even though it must have taken several rounds of layout to find that.
127-
check(controller.position)
128-
.pixels.equals(itemHeight * numItems * (numItems + 1)/2);
129-
check(tester.getRect(find.text('item ${numItems-1}', skipOffstage: false)))
130-
.bottom.equals(600);
166+
// Starts out with the bottom sliver occupying 75% of the viewport…
167+
check(controller.position).pixels.equals(450);
168+
// … even though it has more height than that.
169+
check(tester.getRect(find.text('item 6'))).bottom.isGreaterThan(600);
170+
// (And even though on the first round of layout, it would have looked
171+
// much shorter so that the view would have tried to scroll to its end.)
131172
});
132173

133174
testWidgets('stick to end of list when it grows', (tester) async {

0 commit comments

Comments
 (0)