Skip to content

Commit 4b7f923

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 d300fb9 commit 4b7f923

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

334334
if (!_hasEverCompletedLayout) {
335335
// The list is being laid out for the first time (its first performLayout).
336-
// Start out scrolled to the end.
336+
// Start out scrolled down so the bottom sliver (the new messages)
337+
// occupies 75% of the viewport,
338+
// or at the in-range scroll position closest to that.
337339
// This also brings [pixels] within bounds, which
338340
// the initial value of 0.0 might not have been.
339-
final target = maxScrollExtent;
341+
final target = clampDouble(0.75 * viewportDimension,
342+
minScrollExtent, maxScrollExtent);
340343
if (!hasPixels || pixels != target) {
341344
correctPixels(target);
342345
changed = true;

test/widgets/scrolling_test.dart

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

179179
testWidgets('short/long -> scrolls to ends and no farther', (tester) async {
180-
// Starts out scrolled to bottom.
180+
// Starts out scrolled to top (to show top of the bottom sliver).
181181
await prepare(tester, topHeight: 100, bottomHeight: 800);
182-
check(tester.getRect(findBottom)).bottom.equals(600);
182+
check(tester.getRect(findTop)).top.equals(0);
183+
check(tester.getRect(findBottom)).bottom.equals(900);
183184

184-
// Try scrolling down (by dragging up); doesn't move.
185-
await tester.drag(findBottom, Offset(0, -100));
185+
// Try scrolling up (by dragging down); doesn't move.
186+
await tester.drag(findBottom, Offset(0, 100));
186187
await tester.pump();
187-
check(tester.getRect(findBottom)).bottom.equals(600);
188+
check(tester.getRect(findBottom)).bottom.equals(900);
188189

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

196234
testWidgets('short/short -> starts at bottom, immediately without animation', (tester) async {
@@ -204,43 +242,46 @@ void main() {
204242
check(ys).deepEquals(List.generate(10, (_) => 0.0));
205243
});
206244

207-
testWidgets('short/long -> starts at bottom, immediately without animation', (tester) async {
245+
testWidgets('short/long -> starts at desired start, immediately without animation', (tester) async {
208246
await prepare(tester, topHeight: 100, bottomHeight: 800);
209247

210248
final ys = <double>[];
211249
for (int i = 0; i < 10; i++) {
212-
ys.add(tester.getRect(findBottom).bottom - 600);
250+
ys.add(tester.getRect(findTop).top);
213251
await tester.pump(Duration(milliseconds: 15));
214252
}
215253
check(ys).deepEquals(List.generate(10, (_) => 0.0));
216254
});
217255

218-
testWidgets('starts at bottom, even when bottom underestimated at first', (tester) async {
256+
testWidgets('starts at desired start, even when bottom underestimated at first', (tester) async {
219257
const numItems = 10;
220-
const itemHeight = 300.0;
258+
const itemHeight = 20.0;
221259

222260
// A list where the bottom sliver takes several rounds of layout
223261
// to see how long it really is.
224262
final controller = MessageListScrollController();
225263
await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr,
226264
child: MessageListScrollView(
227265
controller: controller,
266+
// The tiny cacheExtent causes each layout round to only reach
267+
// the first item it expects will go beyond the viewport.
268+
cacheExtent: 1.0, // in (logical) pixels!
228269
center: const ValueKey('center'),
229270
slivers: [
230271
SliverToBoxAdapter(
231-
child: SizedBox(height: 100, child: Text('top'))),
272+
child: SizedBox(height: 300, child: Text('top'))),
232273
SliverList.list(key: const ValueKey('center'),
233274
children: List.generate(numItems, (i) =>
234275
SizedBox(height: (i+1) * itemHeight, child: Text('item $i')))),
235276
])));
236277
await tester.pump();
237278

238-
// Starts out scrolled all the way to the bottom,
239-
// even though it must have taken several rounds of layout to find that.
240-
check(controller.position.pixels)
241-
.equals(itemHeight * numItems * (numItems + 1)/2);
242-
check(tester.getRect(find.text('item ${numItems-1}', skipOffstage: false)))
243-
.bottom.equals(600);
279+
// Starts out with the bottom sliver occupying 75% of the viewport…
280+
check(controller.position.pixels).equals(450);
281+
// … even though it has more height than that.
282+
check(tester.getRect(find.text('item 6'))).bottom.isGreaterThan(600);
283+
// (And even though on the first round of layout, it would have looked
284+
// much shorter so that the view would have tried to scroll to its end.)
244285
});
245286

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

0 commit comments

Comments
 (0)