Skip to content

Commit

Permalink
msglist: Fetch older messages on scrolling up
Browse files Browse the repository at this point in the history
Fixes: #78
  • Loading branch information
gnprice committed Aug 11, 2023
1 parent 18bb0c6 commit bccaead
Show file tree
Hide file tree
Showing 4 changed files with 303 additions and 33 deletions.
98 changes: 86 additions & 12 deletions lib/model/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ class MessageListMessageItem extends MessageListItem {
MessageListMessageItem(this.message, this.content);
}

/// Indicates the app is loading more messages at the top or bottom.
class MessageListLoadingItem extends MessageListItem {
final MessageListDirection direction;

const MessageListLoadingItem(this.direction);
}

enum MessageListDirection { older, newer }

/// Indicates we've reached the oldest message in the narrow.
class MessageListHistoryStartItem extends MessageListItem {
const MessageListHistoryStartItem();
Expand Down Expand Up @@ -55,6 +64,10 @@ mixin _MessageSequence {
bool get haveOldest => _haveOldest;
bool _haveOldest = false;

/// Whether we are currently fetching the next batch of older messages.
bool get fetchingOlder => _fetchingOlder;
bool _fetchingOlder = false;

/// The parsed message contents, as a list parallel to [messages].
///
/// The i'th element is the result of parsing the i'th element of [messages].
Expand All @@ -70,7 +83,7 @@ mixin _MessageSequence {
/// before, between, or after the messages.
///
/// This information is completely derived from [messages] and
/// the flag [haveOldest].
/// the flags [haveOldest] and [fetchingOlder].
/// It exists as an optimization, to memoize that computation.
final QueueList<MessageListItem> items = QueueList();

Expand All @@ -86,6 +99,11 @@ mixin _MessageSequence {
static int _compareItemToMessageId(MessageListItem item, int messageId) {
switch (item) {
case MessageListHistoryStartItem(): return -1;
case MessageListLoadingItem():
switch (item.direction) {
case MessageListDirection.older: return -1;
case MessageListDirection.newer: return 1;
}
case MessageListMessageItem(:var message): return message.id.compareTo(messageId);
}
}
Expand Down Expand Up @@ -115,6 +133,19 @@ mixin _MessageSequence {
_processMessage(messages.length - 1);
}

void _insertAllMessages(int index, Iterable<Message> toInsert) {
// TODO parse/process messages in smaller batches, to not drop frames.
// On a Pixel 5, a batch of 100 messages takes ~15-20ms in _insertAllMessages.
// (Before that, ~2-5ms in jsonDecode and 0ms in fromJson,
// so skip worrying about those steps.)
assert(contents.length == messages.length);
messages.insertAll(index, toInsert);
contents.insertAll(index, toInsert.map(
(message) => parseContent(message.content)));
assert(contents.length == messages.length);
_reprocessAll();
}

/// Redo all computations from scratch, based on [messages].
void _recompute() {
assert(contents.length == messages.length);
Expand All @@ -137,12 +168,22 @@ mixin _MessageSequence {

/// Update [items] to include markers at start and end as appropriate.
void _updateEndMarkers() {
switch ((items.firstOrNull, haveOldest)) {
case (MessageListHistoryStartItem(), true): break;
case (MessageListHistoryStartItem(), _ ): items.removeFirst();

case (_, true): items.addFirst(const MessageListHistoryStartItem());
case (_, _): break;
assert(!(haveOldest && fetchingOlder));
final startMarker = switch ((fetchingOlder, haveOldest)) {
(true, _) => const MessageListLoadingItem(MessageListDirection.older),
(_, true) => const MessageListHistoryStartItem(),
(_, _) => null,
};
final hasStartMarker = switch (items.firstOrNull) {
MessageListLoadingItem() => true,
MessageListHistoryStartItem() => true,
_ => false,
};
switch ((startMarker != null, hasStartMarker)) {
case (true, true): items[0] = startMarker!;
case (true, _ ): items.addFirst(startMarker!);
case (_, true): items.removeFirst();
case (_, _ ): break;
}
}

Expand All @@ -164,13 +205,12 @@ mixin _MessageSequence {
/// Lifecycle:
/// * Create with [init].
/// * Add listeners with [addListener].
/// * Fetch messages with [fetch]. When the fetch completes, this object
/// * Fetch messages with [fetchInitial]. When the fetch completes, this object
/// will notify its listeners (as it will any other time the data changes.)
/// * Fetch more messages as needed with [fetchOlder].
/// * On reassemble, call [reassemble].
/// * When the object will no longer be used, call [dispose] to free
/// resources on the [PerAccountStore].
///
/// TODO support fetching another batch
class MessageListView with ChangeNotifier, _MessageSequence {
MessageListView._({required this.store, required this.narrow});

Expand All @@ -190,10 +230,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
final PerAccountStore store;
final Narrow narrow;

Future<void> fetch() async {
/// Fetch messages, starting from scratch.
Future<void> fetchInitial() async {
// TODO(#80): fetch from anchor firstUnread, instead of newest
// TODO(#82): fetch from a given message ID as anchor
assert(!fetched && !haveOldest);
assert(!fetched && !haveOldest && !fetchingOlder);
assert(messages.isEmpty && contents.isEmpty);
// TODO schedule all this in another isolate
final result = await getMessages(store.connection,
Expand All @@ -211,6 +252,39 @@ class MessageListView with ChangeNotifier, _MessageSequence {
notifyListeners();
}

/// Fetch the next batch of older messages, if applicable.
Future<void> fetchOlder() async {
if (haveOldest) return;
if (fetchingOlder) return;
assert(fetched);
assert(messages.isNotEmpty);
_fetchingOlder = true;
_updateEndMarkers();
notifyListeners();
try {
final result = await getMessages(store.connection,
narrow: narrow.apiEncode(),
anchor: NumericAnchor(messages[0].id),
includeAnchor: false,
numBefore: kMessageListFetchBatchSize,
numAfter: 0,
);

if (result.messages.isNotEmpty
&& result.messages.last.id == messages[0].id) {
// TODO(server-6): includeAnchor should make this impossible
result.messages.removeLast();
}

_insertAllMessages(0, result.messages);
_haveOldest = result.foundOldest;
} finally {
_fetchingOlder = false;
_updateEndMarkers();
notifyListeners();
}
}

/// Add [message] to this view, if it belongs here.
///
/// Called in particular when we get a [MessageEvent].
Expand Down
31 changes: 30 additions & 1 deletion lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,19 @@ class MessageListAppBarTitle extends StatelessWidget {
}
}

/// The approximate height of a short message in the message list.
const _kShortMessageHeight = 80;

/// The point at which we fetch more history, in pixels from the start or end.
///
/// When the user scrolls to within this distance of the start (or end) of the
/// history we currently have, we make a request to fetch the next batch of
/// older (or newer) messages.
//
// When the user reaches this point, they're at least halfway through the
// previous batch.
const kFetchMessagesBufferPixels = (kMessageListFetchBatchSize / 2) * _kShortMessageHeight;

class MessageList extends StatefulWidget {
const MessageList({super.key, required this.narrow});

Expand Down Expand Up @@ -160,7 +173,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
void _initModel(PerAccountStore store) {
model = MessageListView.init(store: store, narrow: widget.narrow);
model!.addListener(_modelChanged);
model!.fetch();
model!.fetchInitial();
}

void _modelChanged() {
Expand All @@ -176,6 +189,17 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
} else {
_scrollToBottomVisibleValue.value = true;
}

final extentRemainingAboveViewport = scrollMetrics.maxScrollExtent - scrollMetrics.pixels;
if (extentRemainingAboveViewport < 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
// but makes things a bit more complicated to reason about.
// The cause seems to be that this gets called again with maxScrollExtent
// still not yet updated to account for the newly-added messages.
model?.fetchOlder();
}
}

void _scrollChanged() {
Expand Down Expand Up @@ -251,6 +275,11 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
child: Padding(
padding: EdgeInsets.symmetric(vertical: 16.0),
child: Text("No earlier messages."))), // TODO use an icon
MessageListLoadingItem() =>
const Center(
child: Padding(
padding: EdgeInsets.symmetric(vertical: 16.0),
child: CircularProgressIndicator())), // TODO perhaps a different indicator
MessageListMessageItem(:var message, :var content) =>
MessageItem(
trailing: i == 0 ? const SizedBox(height: 8) : const SizedBox(height: 11),
Expand Down
Loading

0 comments on commit bccaead

Please sign in to comment.