Skip to content

Commit 46671b4

Browse files
chrisbobbegnprice
authored andcommitted
msglist: Implement mark-read-on-scroll, without yet enabling
When we add the setting for this, coming up, this long-awaited feature will become active. Hooray! This still needs tests. We're tracking that as #1583 for early post-launch. (The launch is coming up very soon.)
1 parent a0241e0 commit 46671b4

File tree

5 files changed

+289
-3
lines changed

5 files changed

+289
-3
lines changed

lib/model/message.dart

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import 'dart:async';
2-
import 'dart:collection';
32
import 'dart:convert';
43

4+
import 'package:collection/collection.dart';
55
import 'package:crypto/crypto.dart';
66
import 'package:flutter/foundation.dart';
77

8+
import '../api/exception.dart';
89
import '../api/model/events.dart';
910
import '../api/model/model.dart';
1011
import '../api/route/messages.dart';
@@ -28,6 +29,8 @@ mixin MessageStore {
2829
void registerMessageList(MessageListView view);
2930
void unregisterMessageList(MessageListView view);
3031

32+
void markReadFromScroll(Iterable<int> messageIds);
33+
3134
Future<void> sendMessage({
3235
required MessageDestination destination,
3336
required String content,
@@ -180,6 +183,67 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMes
180183
_disposed = true;
181184
}
182185

186+
static const _markReadOnScrollBatchSize = 1000;
187+
static const _markReadOnScrollDebounceDuration = Duration(milliseconds: 500);
188+
final _markReadOnScrollQueue = _MarkReadOnScrollQueue();
189+
bool _markReadOnScrollBusy = false;
190+
191+
/// Returns true on success, false on failure.
192+
Future<bool> _sendMarkReadOnScrollRequest(List<int> toSend) async {
193+
assert(toSend.isNotEmpty);
194+
195+
// TODO(#1581) mark as read locally for latency compensation
196+
// (in Unreads and on the message objects)
197+
try {
198+
await updateMessageFlags(connection,
199+
messages: toSend,
200+
op: UpdateMessageFlagsOp.add,
201+
flag: MessageFlag.read);
202+
} on ApiRequestException {
203+
// TODO(#1581) un-mark as read locally?
204+
return false;
205+
}
206+
return true;
207+
}
208+
209+
@override
210+
void markReadFromScroll(Iterable<int> messageIds) async {
211+
assert(!_disposed);
212+
_markReadOnScrollQueue.addAll(messageIds);
213+
if (_markReadOnScrollBusy) return;
214+
215+
_markReadOnScrollBusy = true;
216+
try {
217+
do {
218+
final toSend = <int>[];
219+
int numFromQueue = 0;
220+
for (final messageId in _markReadOnScrollQueue.iterable) {
221+
if (toSend.length == _markReadOnScrollBatchSize) {
222+
break;
223+
}
224+
final message = messages[messageId];
225+
if (message != null && !message.flags.contains(MessageFlag.read)) {
226+
toSend.add(message.id);
227+
}
228+
numFromQueue++;
229+
}
230+
231+
if (toSend.isEmpty || await _sendMarkReadOnScrollRequest(toSend)) {
232+
if (_disposed) return;
233+
_markReadOnScrollQueue.removeFirstN(numFromQueue);
234+
}
235+
if (_disposed) return;
236+
237+
await Future<void>.delayed(_markReadOnScrollDebounceDuration);
238+
if (_disposed) return;
239+
} while (_markReadOnScrollQueue.isNotEmpty);
240+
} finally {
241+
if (!_disposed) {
242+
_markReadOnScrollBusy = false;
243+
}
244+
}
245+
}
246+
183247
@override
184248
Future<void> sendMessage({required MessageDestination destination, required String content}) {
185249
assert(!_disposed);
@@ -517,6 +581,34 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMes
517581
}
518582
}
519583

584+
class _MarkReadOnScrollQueue {
585+
_MarkReadOnScrollQueue();
586+
587+
bool get isNotEmpty => _queue.isNotEmpty;
588+
589+
final _set = <int>{};
590+
final _queue = QueueList<int>();
591+
592+
/// Add [messageIds] to the end of the queue,
593+
/// if they aren't already in the queue.
594+
void addAll(Iterable<int> messageIds) {
595+
for (final messageId in messageIds) {
596+
if (_set.add(messageId)) {
597+
_queue.add(messageId);
598+
}
599+
}
600+
}
601+
602+
Iterable<int> get iterable => _queue;
603+
604+
void removeFirstN(int n) {
605+
for (int i = 0; i < n; i++) {
606+
if (_queue.isEmpty) break;
607+
_set.remove(_queue.removeFirst());
608+
}
609+
}
610+
}
611+
520612
/// The duration an outbox message stays hidden to the user.
521613
///
522614
/// See [OutboxMessageState.waiting].

lib/model/message_list.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,17 @@ mixin _MessageSequence {
222222
return binarySearchByKey(items, messageId, _compareItemToMessageId);
223223
}
224224

225+
Iterable<Message>? getMessagesRange(int firstMessageId, int lastMessageId) {
226+
assert(firstMessageId <= lastMessageId);
227+
final firstIndex = _findMessageWithId(firstMessageId);
228+
final lastIndex = _findMessageWithId(lastMessageId);
229+
if (firstIndex == -1 || lastIndex == -1) {
230+
// TODO(log)
231+
return null;
232+
}
233+
return messages.getRange(firstIndex, lastIndex + 1);
234+
}
235+
225236
static int _compareItemToMessageId(MessageListItem item, int messageId) {
226237
switch (item) {
227238
case MessageListRecipientHeaderItem(:var message):

lib/model/store.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,9 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor
758758
void unregisterMessageList(MessageListView view) =>
759759
_messages.unregisterMessageList(view);
760760
@override
761+
void markReadFromScroll(Iterable<int> messageIds) =>
762+
_messages.markReadFromScroll(messageIds);
763+
@override
761764
Future<void> sendMessage({required MessageDestination destination, required String content}) {
762765
assert(!_disposed);
763766
return _messages.sendMessage(destination: destination, content: content);

lib/widgets/action_sheet.dart

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -896,9 +896,11 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton {
896896
}
897897

898898
@override void onPressed() async {
899-
final narrow = findMessageListPage().narrow;
899+
final messageListPage = findMessageListPage();
900900
unawaited(ZulipAction.markNarrowAsUnreadFromMessage(pageContext,
901-
message, narrow));
901+
message, messageListPage.narrow));
902+
// TODO should we alert the user about this change somehow? A snackbar?
903+
messageListPage.markReadOnScroll = false;
902904
}
903905
}
904906

0 commit comments

Comments
 (0)