zulip / zulip-flutter

Future Zulip client using Flutter
Apache License 2.0
161 stars 134 forks source link

Control scroll position on new and newly-fetched messages #83

Open gnprice opened 1 year ago

gnprice commented 1 year ago

Currently if you're looking at the message list and then a new message comes in, you'll see the messages on the screen jump. This is exactly the right thing (*) if you're scrolled to the bottom — the new message appears at the bottom, and the existing messages all move up to make room. But if you're scrolled further up, then we should keep steady on the screen the messages you're looking at.

Instead, the current behavior is that you see all the messages jump up.

Fundamentally what's happening is that we use a (variant of a) ListView, which models the contents as a sequence of items indexed 0, 1, 2, 3, etc.; and then we pass reverse: true so that item 0 is shown at the bottom, and interpret item index i as "the i'th message in the list, counting zero-based from the end". So if we're looking around item 37 (the message 37 slots from the end), and then a new message arrives, then as far as the ListView knows we should still be looking around item 37 — but that's now the message that was item 36 a moment ago.

A good 90% solution here would be to build the message list out of two list slivers facing back to back, rather than one list sliver facing upward. This is a standard Flutter solution for having a list that grows in both directions; see example in the CustomScrollView doc.

(The list sliver class in that example is SliverList, which corresponds to a basic ListView. We'd use our SliverStickyHeaderList, which corresponds to our StickyHeaderListView.)

(*) Well, possibly it'd be better to have some kind of sliding animation rather than a sudden jump. But that'd be its own issue. This issue is about the new state of things that would be the endpoint of any animation.


Notes on possible followups:

That two-sliver solution would completely solve the problem for adding more messages (or removing messages) at the very top or bottom, which covers (a) message events and (b) fetching messages on scroll, #78. We'd still have some jumps on (c) deleting messages, or moving them away, when they're not the very oldest or latest; (d) moving messages into the middle of the list; (e) likely on some cases of editing messages so that their height changes; (f) similarly on a message going from no reactions to some reactions or back, or other changes that affect its displayed height.

For (c) and (d), I think the Flutter API has the hooks we'd need (we'd make our own implementation of RenderSliverBoxChildManager), but fundamentally it's a subtle problem; in particular we'd need a richer data structure for the list of messages than a List or Queue.

For (e) and (f), and for that matter as a possible alternative solution for (c) and (d) or even (a) and (b), I believe we could use a ScrollController. When something changes, we'd calculate what the right scroll position is in the new state of things, and call jumpTo to go there. Or perhaps we'd override ScrollController.createScrollPosition? The example of PageController, mentioned in that doc, sounds potentially promising as a model.

gnprice commented 1 month ago

This 90% solution:

A good 90% solution here would be to build the message list out of two list slivers facing back to back, rather than one list sliver facing upward.

will happen as part of #82. So this issue tracks going further than that.