Skip to content

Commit 354f0d4

Browse files
committed
wip; Support outbox messages in message list
TODOs - Perhaps make senderFullName available on OutboxMessage - Add tests.
1 parent 2899cd9 commit 354f0d4

File tree

3 files changed

+195
-42
lines changed

3 files changed

+195
-42
lines changed

lib/model/message_list.dart

Lines changed: 133 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import '../api/route/messages.dart';
1010
import 'algorithms.dart';
1111
import 'channel.dart';
1212
import 'content.dart';
13+
import 'message.dart';
1314
import 'narrow.dart';
1415
import 'store.dart';
1516

@@ -36,20 +37,47 @@ class MessageListDateSeparatorItem extends MessageListItem {
3637
}
3738

3839
/// A message to show in the message list.
39-
class MessageListMessageItem extends MessageListItem {
40-
final Message message;
41-
ZulipMessageContent content;
40+
sealed class MessageListDisplayableMessageItem extends MessageListItem {
41+
DisplayableMessage get message;
42+
ZulipMessageContent get content;
4243
bool showSender;
4344
bool isLastInBlock;
4445

46+
MessageListDisplayableMessageItem({
47+
required this.showSender,
48+
required this.isLastInBlock,
49+
});
50+
}
51+
52+
class MessageListMessageItem extends MessageListDisplayableMessageItem {
53+
@override
54+
final Message message;
55+
@override
56+
ZulipMessageContent content;
57+
4558
MessageListMessageItem(
4659
this.message,
4760
this.content, {
48-
required this.showSender,
49-
required this.isLastInBlock,
61+
required super.showSender,
62+
required super.isLastInBlock,
5063
});
5164
}
5265

66+
class MessageListOutboxMessageItem extends MessageListDisplayableMessageItem {
67+
@override
68+
final OutboxMessage message;
69+
@override
70+
final ZulipContent content;
71+
72+
MessageListOutboxMessageItem(
73+
this.message, {
74+
required super.showSender,
75+
required super.isLastInBlock,
76+
}) : content = ZulipContent(nodes: [
77+
ParagraphNode(links: [], nodes: [TextNode(message.content)]),
78+
]);
79+
}
80+
5381
/// Indicates the app is loading more messages at the top.
5482
// TODO(#80): or loading at the bottom, by adding a [MessageListDirection.newer]
5583
class MessageListLoadingItem extends MessageListItem {
@@ -77,6 +105,13 @@ mixin _MessageSequence {
77105
/// See also [contents] and [items].
78106
final List<Message> messages = [];
79107

108+
/// The messages sent by the self-user.
109+
///
110+
/// See also [items].
111+
// Usually this should not have that many items, so we do not anticipate
112+
// performance issues with unoptimized O(N) iterations through this list.
113+
final List<OutboxMessage> outboxMessages = [];
114+
80115
/// Whether [messages] and [items] represent the results of a fetch.
81116
///
82117
/// This allows the UI to distinguish "still working on fetching messages"
@@ -129,11 +164,12 @@ mixin _MessageSequence {
129164
/// The messages and their siblings in the UI, in order.
130165
///
131166
/// This has a [MessageListMessageItem] corresponding to each element
132-
/// of [messages], in order. It may have additional items interspersed
133-
/// before, between, or after the messages.
167+
/// of [messages], followed by each element in [outboxMessages] in order.
168+
/// It may have additional items interspersed before, between, or after the
169+
/// messages.
134170
///
135-
/// This information is completely derived from [messages] and
136-
/// the flags [haveOldest], [fetchingOlder] and [fetchOlderCoolingDown].
171+
/// This information is completely derived from [messages], [outboxMessages]
172+
/// and the flags [haveOldest], [fetchingOlder] and [fetchOlderCoolingDown].
137173
/// It exists as an optimization, to memoize that computation.
138174
final QueueList<MessageListItem> items = QueueList();
139175

@@ -157,6 +193,7 @@ mixin _MessageSequence {
157193
case MessageListDateSeparatorItem(:var message):
158194
return message.id != null && message.id! <= messageId ? -1 : 1;
159195
case MessageListMessageItem(:var message): return message.id.compareTo(messageId);
196+
case MessageListOutboxMessageItem(): return 1;
160197
}
161198
}
162199

@@ -264,6 +301,7 @@ mixin _MessageSequence {
264301
void _reset() {
265302
generation += 1;
266303
messages.clear();
304+
outboxMessages.clear();
267305
_fetched = false;
268306
_haveOldest = false;
269307
_fetchingOlder = false;
@@ -282,17 +320,18 @@ mixin _MessageSequence {
282320
_reprocessAll();
283321
}
284322

285-
/// Prepare [items] before a [MessageListMessageItem] can be appended.
323+
/// Prepare [items] before a [MessageListDisplayableMessageItem] can be appended.
286324
///
287325
/// Returns whether the sender can be shared between the items for both
288-
/// messages, as in the negation of [MessageListMessageItem.showSender].
289-
bool _prepareTailForMessage(Message message, {required Message? prevMessage}) {
326+
/// messages, as in the negation of [MessageListDisplayableMessageItem.showSender].
327+
bool _prepareTailForMessage(DisplayableMessage message, {
328+
required DisplayableMessage? prevMessage,
329+
}) {
290330
if (prevMessage == null || !haveSameRecipient(prevMessage, message)) {
291331
items.add(MessageListRecipientHeaderItem(message));
292332
return false;
293333
} else {
294-
assert(items.last is MessageListMessageItem);
295-
final prevMessageItem = items.last as MessageListMessageItem;
334+
final prevMessageItem = items.last as MessageListDisplayableMessageItem;
296335
assert(identical(prevMessageItem.message, prevMessage));
297336
assert(prevMessageItem.isLastInBlock);
298337
prevMessageItem.isLastInBlock = false;
@@ -320,6 +359,28 @@ mixin _MessageSequence {
320359
showSender: !canShareSender, isLastInBlock: true));
321360
}
322361

362+
void _processOutboxMessage(int index) {
363+
final prevMessage = index == 0 ? messages.last : outboxMessages[index - 1];
364+
final message = outboxMessages[index];
365+
366+
final canShareSender = _prepareTailForMessage(message, prevMessage: prevMessage);
367+
items.add(MessageListOutboxMessageItem(message,
368+
showSender: !canShareSender, isLastInBlock: true));
369+
}
370+
371+
/// Remove from [items] the ones associated with [outboxMessages].
372+
void _removeOutboxMessageItems() {
373+
while (items.isNotEmpty && items.last is! MessageListMessageItem) {
374+
items.removeLast();
375+
}
376+
final lastMessageItem = items.lastOrNull;
377+
assert(items.none((e) => e is MessageListOutboxMessageItem));
378+
if (lastMessageItem is MessageListDisplayableMessageItem) {
379+
assert(lastMessageItem is MessageListMessageItem);
380+
lastMessageItem.isLastInBlock = true;
381+
}
382+
}
383+
323384
/// Update [items] to include markers at start and end as appropriate.
324385
void _updateEndMarkers() {
325386
assert(fetched);
@@ -344,23 +405,27 @@ mixin _MessageSequence {
344405
}
345406
}
346407

347-
/// Recompute [items] from scratch, based on [messages], [contents], and flags.
408+
/// Recompute [items] from scratch, based on [messages], [contents],
409+
/// [outboxMessages] and flags.
348410
void _reprocessAll() {
349411
items.clear();
350412
for (var i = 0; i < messages.length; i++) {
351413
_processMessage(i);
352414
}
415+
for (var i = 0; i < outboxMessages.length; i++) {
416+
_processOutboxMessage(i);
417+
}
353418
_updateEndMarkers();
354419
}
355420
}
356421

357422
@visibleForTesting
358-
bool haveSameRecipient(Message prevMessage, Message message) {
359-
if (prevMessage is StreamMessage && message is StreamMessage) {
360-
if (prevMessage.streamId != message.streamId) return false;
361-
if (prevMessage.topic.canonicalize() != message.topic.canonicalize()) return false;
362-
} else if (prevMessage is DmMessage && message is DmMessage) {
363-
if (!_equalIdSequences(prevMessage.allRecipientIds, message.allRecipientIds)) {
423+
bool haveSameRecipient(DisplayableMessage prevMessage, DisplayableMessage message) {
424+
if (prevMessage is DisplayableMessage<StreamDestination> && message is DisplayableMessage<StreamDestination>) {
425+
if (prevMessage.destination.streamId != message.destination.streamId) return false;
426+
if (prevMessage.destination.topic.canonicalize() != message.destination.topic.canonicalize()) return false;
427+
} else if (prevMessage is DisplayableMessage<DmDestination> && message is DisplayableMessage<DmDestination>) {
428+
if (!_equalIdSequences(prevMessage.destination.userIds, message.destination.userIds)) {
364429
return false;
365430
}
366431
} else {
@@ -379,7 +444,7 @@ bool haveSameRecipient(Message prevMessage, Message message) {
379444
}
380445

381446
@visibleForTesting
382-
bool messagesSameDay(Message prevMessage, Message message) {
447+
bool messagesSameDay(DisplayableMessage prevMessage, DisplayableMessage message) {
383448
// TODO memoize [DateTime]s... also use memoized for showing date/time in msglist
384449
final prevTime = DateTime.fromMillisecondsSinceEpoch(prevMessage.timestamp * 1000);
385450
final time = DateTime.fromMillisecondsSinceEpoch(message.timestamp * 1000);
@@ -444,19 +509,20 @@ class MessageListView with ChangeNotifier, _MessageSequence {
444509
/// one way or another.
445510
///
446511
/// See also [_allMessagesVisible].
447-
bool _messageVisible(Message message) {
512+
bool _messageVisible(DisplayableMessage message) {
448513
switch (narrow) {
449514
case CombinedFeedNarrow():
450-
return switch (message) {
451-
StreamMessage() =>
452-
store.isTopicVisible(message.streamId, message.topic),
453-
DmMessage() => true,
515+
return switch (message.destination) {
516+
StreamDestination(:final streamId, :final topic) =>
517+
store.isTopicVisible(streamId, topic),
518+
DmDestination() => true,
454519
};
455520

456521
case ChannelNarrow(:final streamId):
457-
assert(message is StreamMessage && message.streamId == streamId);
458-
if (message is! StreamMessage) return false;
459-
return store.isTopicVisibleInStream(streamId, message.topic);
522+
assert(message is DisplayableMessage<StreamDestination>
523+
&& message.destination.streamId == streamId);
524+
if (message is! DisplayableMessage<StreamDestination>) return false;
525+
return store.isTopicVisibleInStream(streamId, message.destination.topic);
460526

461527
case TopicNarrow():
462528
case DmNarrow():
@@ -525,6 +591,9 @@ class MessageListView with ChangeNotifier, _MessageSequence {
525591
_addMessage(message);
526592
}
527593
}
594+
for (final outboxMessage in store.outboxMessages.values) {
595+
handleOutboxMessage(outboxMessage);
596+
}
528597
_fetched = true;
529598
_haveOldest = result.foundOldest;
530599
_updateEndMarkers();
@@ -631,6 +700,19 @@ class MessageListView with ChangeNotifier, _MessageSequence {
631700
}
632701
}
633702

703+
void handleOutboxMessage(OutboxMessage outboxMessage) {
704+
if (outboxMessage.hidden) return;
705+
if (narrow.containsMessage(outboxMessage) && _messageVisible(outboxMessage)) {
706+
outboxMessages.add(outboxMessage);
707+
_processOutboxMessage(outboxMessages.length - 1);
708+
notifyListeners();
709+
}
710+
}
711+
712+
void handleUpdateOutboxMessage(int localMessageId) {
713+
notifyListenersIfOutboxMessagePresent(localMessageId);
714+
}
715+
634716
void handleUserTopicEvent(UserTopicEvent event) {
635717
switch (_canAffectVisibility(event)) {
636718
case VisibilityEffect.none:
@@ -666,14 +748,26 @@ class MessageListView with ChangeNotifier, _MessageSequence {
666748
void handleMessageEvent(MessageEvent event) {
667749
final message = event.message;
668750
if (!narrow.containsMessage(message) || !_messageVisible(message)) {
751+
assert(event.localMessageId == null
752+
|| outboxMessages.every((message) =>
753+
message.localMessageId != event.localMessageId!));
669754
return;
670755
}
671756
if (!_fetched) {
672757
// TODO mitigate this fetch/event race: save message to add to list later
673758
return;
674759
}
760+
_removeOutboxMessageItems();
675761
// TODO insert in middle instead, when appropriate
676762
_addMessage(message);
763+
final localMessageId = event.localMessageId;
764+
if (localMessageId != null) {
765+
outboxMessages.removeWhere(
766+
(message) => message.localMessageId == localMessageId);
767+
for (int i = 0; i < outboxMessages.length; i++) {
768+
_processOutboxMessage(i);
769+
}
770+
}
677771
notifyListeners();
678772
}
679773

@@ -792,6 +886,15 @@ class MessageListView with ChangeNotifier, _MessageSequence {
792886
}
793887
}
794888

889+
/// Notify listeners if the given outbox message is present in this view.
890+
void notifyListenersIfOutboxMessagePresent(int localMessageId) {
891+
final isAnyPresent =
892+
outboxMessages.any((message) => message.localMessageId == localMessageId);
893+
if (isAnyPresent) {
894+
notifyListeners();
895+
}
896+
}
897+
795898
/// Called when the app is reassembled during debugging, e.g. for hot reload.
796899
///
797900
/// This will redo from scratch any computations we can, such as parsing

0 commit comments

Comments
 (0)