msglist: Support outbox messages in message list

This adds some overhead in magnitude of O(1) (where the constant is
the number of outbox messages in a view, expected to be small) on
message event handling.

We add outboxMessages as a list independent from messages on
_MessageSequence.  Because outbox messages are not rendered
(the raw content is shown as plain text), we leave the 1-1
relationship between `messages` and `contents` unchanged.

When computing `items`, we now start to look at `outboxMessages` as
well, with the guarantee that the items related to outbox messages
always come after those for other messages.  Look for places that call
`_processOutboxMessage(int index)` for references, and the changes to
`checkInvariants` on how this affects the message list invariants.

`handleOutboxMessage`, similar to `handleMessage`, also look at
`fetched` before adding the outbox message.  However, there is no data
race to prevent — we can totally not clear `outboxMessages` in `_reset`,
and do the initial fetch from `MessageListView.init`, so that outbox
messages do not rely on the fetched state at all.  I decided against it
since it is easy to check `fetched`, and making `fetchInitial` reprocess
`outboxMessages` from a clean state (which is inexpensive) helps
ensuring message list invariants.

Author: PIG208

lib/model/message.dart

@@ -321,10 +321,9 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
     }
     _outboxMessageDebounceTimers.remove(localMessageId)?.cancel();
     outboxMessage.unhide();
-    // TODO: uncomment this once message list support is added:
-    // for (final view in _messageListViews) {
-    //   view.handleOutboxMessage(outboxMessage);
-    // }
+    for (final view in _messageListViews) {
+      view.handleOutboxMessage(outboxMessage);
+    }
   }
 
   /// Update the state of the [OutboxMessage] with the given [localMessageId],
@@ -346,10 +345,9 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
     if (outboxMessage.hidden) {
       return;
     }
-    // TODO: uncomment this once message list support is added:
-    // for (final view in _messageListViews) {
-    //   view.notifyListenersIfOutboxMessagePresent(localMessageId);
-    // }
+    for (final view in _messageListViews) {
+      view.notifyListenersIfOutboxMessagePresent(localMessageId);
+    }
   }
 
 
@@ -362,10 +360,9 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
       assert(false, 'Removing unknown outbox message with localMessageId: $localMessageId');
       return;
     }
-    // TODO: uncomment this once message list support is added:
-    // for (final view in _messageListViews) {
-    //   view.removeOutboxMessageIfExists(removed);
-    // }
+    for (final view in _messageListViews) {
+      view.removeOutboxMessageIfExists(removed);
+    }
   }
 
   @override

lib/model/message_list.dart

@@ -10,6 +10,7 @@ import '../api/route/messages.dart';
 import 'algorithms.dart';
 import 'channel.dart';
 import 'content.dart';
+import 'message.dart';
 import 'narrow.dart';
 import 'store.dart';
 
@@ -62,6 +63,21 @@ class MessageListMessageItem extends MessageListMessageBaseItem {
   });
 }
 
+class MessageListOutboxMessageItem extends MessageListMessageBaseItem {
+  @override
+  final OutboxMessage message;
+  @override
+  final ZulipContent content;
+
+  MessageListOutboxMessageItem(
+    this.message, {
+    required super.showSender,
+    required super.isLastInBlock,
+  }) : content = ZulipContent(nodes: [
+    ParagraphNode(links: [], nodes: [TextNode(message.content)]),
+  ]);
+}
+
 /// Indicates the app is loading more messages at the top.
 // TODO(#80): or loading at the bottom, by adding a [MessageListDirection.newer]
 class MessageListLoadingItem extends MessageListItem {
@@ -89,7 +105,15 @@ mixin _MessageSequence {
   /// See also [contents] and [items].
   final List<Message> messages = [];
 
-  /// Whether [messages] and [items] represent the results of a fetch.
+  /// The messages sent by the self-user.
+  ///
+  /// See also [items].
+  // Usually this should not have that many items, so we do not anticipate
+  // performance issues with unoptimized O(N) iterations through this list.
+  final List<OutboxMessage> outboxMessages = [];
+
+  /// Whether [messages], [outboxMessages], and [items] represent the results
+  /// of a fetch.
   ///
   /// This allows the UI to distinguish "still working on fetching messages"
   /// from "there are in fact no messages here".
@@ -141,11 +165,12 @@ mixin _MessageSequence {
   /// The messages and their siblings in the UI, in order.
   ///
   /// This has a [MessageListMessageItem] corresponding to each element
-  /// of [messages], in order.  It may have additional items interspersed
-  /// before, between, or after the messages.
+  /// of [messages], followed by each element in [outboxMessages] in order.
+  /// It may have additional items interspersed before, between, or after the
+  /// messages.
   ///
-  /// This information is completely derived from [messages] and
-  /// the flags [haveOldest], [fetchingOlder] and [fetchOlderCoolingDown].
+  /// This information is completely derived from [messages], [outboxMessages]
+  /// and the flags [haveOldest], [fetchingOlder] and [fetchOlderCoolingDown].
   /// It exists as an optimization, to memoize that computation.
   final QueueList<MessageListItem> items = QueueList();
 
@@ -170,6 +195,7 @@ mixin _MessageSequence {
         if (message.id == null)                  return 1;
         return message.id! <= messageId ? -1 : 1;
       case MessageListMessageItem(:var message): return message.id.compareTo(messageId);
+      case MessageListOutboxMessageItem():       return 1;
     }
   }
 
@@ -277,6 +303,7 @@ mixin _MessageSequence {
   void _reset() {
     generation += 1;
     messages.clear();
+    outboxMessages.clear();
     _fetched = false;
     _haveOldest = false;
     _fetchingOlder = false;
@@ -300,7 +327,8 @@ mixin _MessageSequence {
   ///
   /// Returns whether an item has been appended or not.
   ///
-  /// The caller must append a [MessageListMessageBaseItem] after this.
+  /// The caller must append a [MessageListMessageBaseItem] for [message]
+  /// after this.
   bool _maybeAppendAuxillaryItem(MessageBase message, {
     required MessageBase? prevMessage,
   }) {
@@ -337,6 +365,40 @@ mixin _MessageSequence {
       isLastInBlock: true));
   }
 
+  /// Append to [items] based on the index-th outbox message.
+  ///
+  /// All [messages] and previous messages in [outboxMessages] must already have
+  /// been processed.
+  void _processOutboxMessage(int index) {
+    final prevMessage = index == 0 ? messages.lastOrNull : outboxMessages[index - 1];
+    final message = outboxMessages[index];
+
+    final appended = _maybeAppendAuxillaryItem(message, prevMessage: prevMessage);
+    items.add(MessageListOutboxMessageItem(message,
+      showSender: appended || prevMessage?.senderId != message.senderId,
+      isLastInBlock: true));
+  }
+
+  /// Remove items associated with [outboxMessages] from [items].
+  ///
+  /// This is efficient due to the expected small size of [outboxMessages].
+  void _removeOutboxMessageItems() {
+    // This loop relies on the assumption that all [MessageListMessageItem]
+    // items comes before those associated with outbox messages.  If there
+    // is no [MessageListMessageItem] at all, this will end up removing
+    // end markers as well.
+    while (items.isNotEmpty && items.last is! MessageListMessageItem) {
+      items.removeLast();
+    }
+    assert(items.none((e) => e is MessageListOutboxMessageItem));
+
+    if (items.isNotEmpty) {
+      final lastItem = items.last as MessageListMessageItem;
+      lastItem.isLastInBlock = true;
+    }
+    _updateEndMarkers();
+  }
+
   /// Update [items] to include markers at start and end as appropriate.
   void _updateEndMarkers() {
     assert(fetched);
@@ -361,12 +423,16 @@ mixin _MessageSequence {
     }
   }
 
-  /// Recompute [items] from scratch, based on [messages], [contents], and flags.
+  /// Recompute [items] from scratch, based on [messages], [contents],
+  /// [outboxMessages] and flags.
   void _reprocessAll() {
     items.clear();
     for (var i = 0; i < messages.length; i++) {
       _processMessage(i);
     }
+    for (var i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
     _updateEndMarkers();
   }
 }
@@ -527,7 +593,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     // TODO(#80): fetch from anchor firstUnread, instead of newest
     // TODO(#82): fetch from a given message ID as anchor
     assert(!fetched && !haveOldest && !fetchingOlder && !fetchOlderCoolingDown);
-    assert(messages.isEmpty && contents.isEmpty);
+    assert(messages.isEmpty && contents.isEmpty && outboxMessages.isEmpty);
     // TODO schedule all this in another isolate
     final generation = this.generation;
     final result = await getMessages(store.connection,
@@ -545,6 +611,9 @@ class MessageListView with ChangeNotifier, _MessageSequence {
         _addMessage(message);
       }
     }
+    for (final outboxMessage in store.outboxMessages.values) {
+      _maybeAddOutboxMessage(outboxMessage);
+    }
     _fetched = true;
     _haveOldest = result.foundOldest;
     _updateEndMarkers();
@@ -651,6 +720,45 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     }
   }
 
+  /// Add [outboxMessage] if it belongs to the view.
+  ///
+  /// Returns true if the message was added, false otherwise.
+  bool _maybeAddOutboxMessage(OutboxMessage outboxMessage) {
+    assert(outboxMessages.none(
+      (message) => message.localMessageId == outboxMessage.localMessageId));
+    if (!outboxMessage.hidden
+        && narrow.containsMessage(outboxMessage)
+        && _messageVisible(outboxMessage)) {
+      outboxMessages.add(outboxMessage);
+      _processOutboxMessage(outboxMessages.length - 1);
+      return true;
+    }
+    return false;
+  }
+
+  void handleOutboxMessage(OutboxMessage outboxMessage) {
+    if (!fetched) return;
+    if (_maybeAddOutboxMessage(outboxMessage)) {
+      notifyListeners();
+    }
+  }
+
+  /// Remove the [outboxMessage] from the view.
+  ///
+  /// This is a no-op if the message is not found.
+  void removeOutboxMessageIfExists(OutboxMessage outboxMessage) {
+    final removed = outboxMessages.remove(outboxMessage);
+    if (!removed) {
+      return;
+    }
+
+    _removeOutboxMessageItems();
+    for (int i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
+    notifyListeners();
+  }
+
   void handleUserTopicEvent(UserTopicEvent event) {
     switch (_canAffectVisibility(event)) {
       case VisibilityEffect.none:
@@ -686,14 +794,29 @@ class MessageListView with ChangeNotifier, _MessageSequence {
   void handleMessageEvent(MessageEvent event) {
     final message = event.message;
     if (!narrow.containsMessage(message) || !_messageVisible(message)) {
+      assert(event.localMessageId == null || outboxMessages.none((message) =>
+        message.localMessageId == int.parse(event.localMessageId!, radix: 10)));
       return;
     }
     if (!_fetched) {
       // TODO mitigate this fetch/event race: save message to add to list later
       return;
     }
+    // We always remove all outbox message items
+    // to ensure that message items come before them.
+    _removeOutboxMessageItems();
     // TODO insert in middle instead, when appropriate
     _addMessage(message);
+    if (event.localMessageId != null) {
+      final localMessageId = int.parse(event.localMessageId!);
+      // [outboxMessages] is epxected to be short, so removing the corresponding
+      // outbox message and reprocessing them all in linear time is efficient.
+      outboxMessages.removeWhere(
+        (message) => message.localMessageId == localMessageId);
+    }
+    for (int i = 0; i < outboxMessages.length; i++) {
+      _processOutboxMessage(i);
+    }
     notifyListeners();
   }
 
@@ -812,6 +935,15 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     }
   }
 
+  /// Notify listeners if the given outbox message is present in this view.
+  void notifyListenersIfOutboxMessagePresent(int localMessageId) {
+    final isAnyPresent =
+      outboxMessages.any((message) => message.localMessageId == localMessageId);
+    if (isAnyPresent) {
+      notifyListeners();
+    }
+  }
+
   /// Called when the app is reassembled during debugging, e.g. for hot reload.
   ///
   /// This will redo from scratch any computations we can, such as parsing

lib/widgets/message_list.dart

@@ -687,6 +687,12 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
           header: header,
           trailingWhitespace: i == 1 ? 8 : 11,
           item: data);
+      case MessageListOutboxMessageItem():
+        final header = RecipientHeader(message: data.message, narrow: widget.narrow);
+        return MessageItem(
+          header: header,
+          trailingWhitespace: 11,
+          item: data);
     }
   }
 }
@@ -982,6 +988,7 @@ class MessageItem extends StatelessWidget {
       child: Column(children: [
         switch (item) {
           MessageListMessageItem() => MessageWithPossibleSender(item: item),
+          MessageListOutboxMessageItem() => OutboxMessageWithPossibleSender(item: item),
         },
         if (trailingWhitespace != null && item.isLastInBlock) SizedBox(height: trailingWhitespace!),
       ]));
@@ -1335,7 +1342,7 @@ final _kMessageTimestampFormat = DateFormat('h:mm aa', 'en_US');
 class _SenderRow extends StatelessWidget {
   const _SenderRow({required this.message});
 
-  final Message message;
+  final MessageBase message;
 
   @override
   Widget build(BuildContext context) {
@@ -1364,7 +1371,9 @@ class _SenderRow extends StatelessWidget {
                     userId: message.senderId),
                   const SizedBox(width: 8),
                   Flexible(
-                    child: Text(message.senderFullName, // TODO(#716): use `store.senderDisplayName`
+                    child: Text(message is Message
+                        ? store.senderDisplayName(message as Message)
+                        : store.userDisplayName(message.senderId),
                       style: TextStyle(
                         fontSize: 18,
                         height: (22 / 18),
@@ -1461,3 +1470,30 @@ class MessageWithPossibleSender extends StatelessWidget {
         ])));
   }
 }
+
+/// A placeholder for Zulip message sent by the self-user.
+///
+/// See also [OutboxMessage].
+class OutboxMessageWithPossibleSender extends StatelessWidget {
+  const OutboxMessageWithPossibleSender({super.key, required this.item});
+
+  final MessageListOutboxMessageItem item;
+
+  @override
+  Widget build(BuildContext context) {
+    final message = item.message;
+    return Padding(
+      padding: const EdgeInsets.symmetric(vertical: 4),
+      child: Column(children: [
+        if (item.showSender) _SenderRow(message: message),
+        Padding(
+          padding: const EdgeInsets.symmetric(horizontal: 16),
+          // This is adapated from [MessageContent].
+          // TODO(#576): Offer InheritedMessage ancestor once we are ready
+          //   to support local echoing images and lightbox.
+          child: DefaultTextStyle(
+            style: ContentTheme.of(context).textStylePlainParagraph,
+            child: BlockContentList(nodes: item.content.nodes))),
+      ]));
+  }
+}