Skip to content

Commit 9ded2aa

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 9e6624b commit 9ded2aa

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
@@ -523,10 +523,13 @@ class MessageListScrollPosition extends ScrollPositionWithSingleContext {
523523

524524
if (!_hasEverCompletedLayout) {
525525
// The list is being laid out for the first time (its first performLayout).
526-
// Start out scrolled to the end.
526+
// Start out scrolled down so the bottom sliver (the new messages)
527+
// occupies 75% of the viewport,
528+
// or at the in-range scroll position closest to that.
527529
// This also brings [pixels] within bounds, which
528530
// the initial value of 0.0 might not have been.
529-
final target = maxScrollExtent;
531+
final target = clampDouble(0.75 * viewportDimension,
532+
minScrollExtent, maxScrollExtent);
530533
if (!hasPixels || pixels != target) {
531534
correctPixels(target);
532535
changed = true;

test/widgets/scrolling_test.dart

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

187187
testWidgets('short/long -> scrolls to ends and no farther', (tester) async {
188-
// Starts out scrolled to bottom.
188+
// Starts out scrolled to top (to show top of the bottom sliver).
189189
await prepare(tester, topHeight: 100, bottomHeight: 800);
190-
check(tester.getRect(findBottom)).bottom.equals(600);
190+
check(tester.getRect(findTop)).top.equals(0);
191+
check(tester.getRect(findBottom)).bottom.equals(900);
191192

192-
// Try scrolling down (by dragging up); doesn't move.
193-
await tester.drag(findBottom, Offset(0, -100));
193+
// Try scrolling up (by dragging down); doesn't move.
194+
await tester.drag(findBottom, Offset(0, 100));
194195
await tester.pump();
195-
check(tester.getRect(findBottom)).bottom.equals(600);
196+
check(tester.getRect(findBottom)).bottom.equals(900);
196197

197-
// Try scrolling up (by dragging down); moves only as far as top of list.
198-
await tester.drag(findBottom, Offset(0, 400));
198+
// Try scrolling down (by dragging up); moves only as far as bottom of list.
199+
await tester.drag(findBottom, Offset(0, -400));
199200
await tester.pump();
200-
check(tester.getRect(findBottom)).bottom.equals(900);
201+
check(tester.getRect(findBottom)).bottom.equals(600);
202+
});
203+
204+
testWidgets('starts by showing top of bottom sliver, long/long', (tester) async {
205+
// Both slivers are long; the bottom sliver gets 75% of the viewport.
206+
await prepare(tester, topHeight: 1000, bottomHeight: 3000);
207+
check(tester.getRect(findBottom)).top.equals(150);
208+
});
209+
210+
testWidgets('starts by showing top of bottom sliver, short/long', (tester) async {
211+
// The top sliver is shorter than 25% of the viewport.
212+
// It's shown in full, and the bottom sliver gets the rest (so >75%).
213+
await prepare(tester, topHeight: 50, bottomHeight: 3000);
201214
check(tester.getRect(findTop)).top.equals(0);
215+
check(tester.getRect(findBottom)).top.equals(50);
216+
});
217+
218+
testWidgets('starts by showing top of bottom sliver, short/medium', (tester) async {
219+
// The whole list fits in the viewport. It's pinned to the bottom,
220+
// even when that gives the bottom sliver more than 75%.
221+
await prepare(tester, topHeight: 50, bottomHeight: 500);
222+
check(tester.getRect(findTop))..top.equals(50)..bottom.equals(100);
223+
check(tester.getRect(findBottom)).bottom.equals(600);
224+
});
225+
226+
testWidgets('starts by showing top of bottom sliver, medium/short', (tester) async {
227+
// The whole list fits in the viewport. It's pinned to the bottom,
228+
// even when that gives the top sliver more than 25%.
229+
await prepare(tester, topHeight: 300, bottomHeight: 100);
230+
check(tester.getRect(findTop))..top.equals(200)..bottom.equals(500);
231+
check(tester.getRect(findBottom)).bottom.equals(600);
232+
});
233+
234+
testWidgets('starts by showing top of bottom sliver, long/short', (tester) async {
235+
// The bottom sliver is shorter than 75% of the viewport.
236+
// It's shown in full, and the top sliver gets the rest (so >25%).
237+
await prepare(tester, topHeight: 1000, bottomHeight: 300);
238+
check(tester.getRect(findTop)).bottom.equals(300);
239+
check(tester.getRect(findBottom)).bottom.equals(600);
202240
});
203241

204242
testWidgets('short/short -> starts at bottom, immediately without animation', (tester) async {
@@ -212,43 +250,46 @@ void main() {
212250
check(ys).deepEquals(List.generate(10, (_) => 0.0));
213251
});
214252

215-
testWidgets('short/long -> starts at bottom, immediately without animation', (tester) async {
253+
testWidgets('short/long -> starts at desired start, immediately without animation', (tester) async {
216254
await prepare(tester, topHeight: 100, bottomHeight: 800);
217255

218256
final ys = <double>[];
219257
for (int i = 0; i < 10; i++) {
220-
ys.add(tester.getRect(findBottom).bottom - 600);
258+
ys.add(tester.getRect(findTop).top);
221259
await tester.pump(Duration(milliseconds: 15));
222260
}
223261
check(ys).deepEquals(List.generate(10, (_) => 0.0));
224262
});
225263

226-
testWidgets('starts at bottom, even when bottom underestimated at first', (tester) async {
264+
testWidgets('starts at desired start, even when bottom underestimated at first', (tester) async {
227265
const numItems = 10;
228-
const itemHeight = 300.0;
266+
const itemHeight = 20.0;
229267

230268
// A list where the bottom sliver takes several rounds of layout
231269
// to see how long it really is.
232270
final controller = MessageListScrollController();
233271
await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr,
234272
child: MessageListScrollView(
235273
controller: controller,
274+
// The tiny cacheExtent causes each layout round to only reach
275+
// the first item it expects will go beyond the viewport.
276+
cacheExtent: 1.0, // in (logical) pixels!
236277
center: const ValueKey('center'),
237278
slivers: [
238279
SliverToBoxAdapter(
239-
child: SizedBox(height: 100, child: Text('top'))),
280+
child: SizedBox(height: 300, child: Text('top'))),
240281
SliverList.list(key: const ValueKey('center'),
241282
children: List.generate(numItems, (i) =>
242283
SizedBox(height: (i+1) * itemHeight, child: Text('item $i')))),
243284
])));
244285
await tester.pump();
245286

246-
// Starts out scrolled all the way to the bottom,
247-
// even though it must have taken several rounds of layout to find that.
248-
check(controller.position.pixels)
249-
.equals(itemHeight * numItems * (numItems + 1)/2);
250-
check(tester.getRect(find.text('item ${numItems-1}', skipOffstage: false)))
251-
.bottom.equals(600);
287+
// Starts out with the bottom sliver occupying 75% of the viewport…
288+
check(controller.position.pixels).equals(450);
289+
// … even though it has more height than that.
290+
check(tester.getRect(find.text('item 6'))).bottom.isGreaterThan(600);
291+
// (And even though on the first round of layout, it would have looked
292+
// much shorter so that the view would have tried to scroll to its end.)
252293
});
253294

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

0 commit comments

Comments
 (0)