msglist: Share recipient headers across messages

lib/model/message_list.dart

@@ -19,22 +19,30 @@ sealed class MessageListItem {
   const MessageListItem();
 }
 
+class MessageListRecipientHeaderItem extends MessageListItem {
+  final Message message;
+
+  MessageListRecipientHeaderItem(this.message);
+}
+
 /// A message to show in the message list.
 class MessageListMessageItem extends MessageListItem {
   final Message message;
-  final ZulipContent content;
+  ZulipContent content;
+  bool isLastInBlock;
 
-  MessageListMessageItem(this.message, this.content);
+  MessageListMessageItem(this.message, this.content, {required this.isLastInBlock});
 }
 
-/// Indicates the app is loading more messages at the top or bottom.
+/// 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 {
   final MessageListDirection direction;
 
   const MessageListLoadingItem(this.direction);
 }
 
-enum MessageListDirection { older, newer }
+enum MessageListDirection { older }
 
 /// Indicates we've reached the oldest message in the narrow.
 class MessageListHistoryStartItem extends MessageListItem {
@@ -102,8 +110,9 @@ mixin _MessageSequence {
       case MessageListLoadingItem():
         switch (item.direction) {
           case MessageListDirection.older:       return -1;
-          case MessageListDirection.newer:       return 1;
         }
+      case MessageListRecipientHeaderItem(:var message):
+        return (message.id <= messageId) ? -1 : 1;
       case MessageListMessageItem(:var message): return message.id.compareTo(messageId);
     }
   }
@@ -118,7 +127,7 @@ mixin _MessageSequence {
     assert(itemIndex > -1
       && items[itemIndex] is MessageListMessageItem
       && identical((items[itemIndex] as MessageListMessageItem).message, message));
-    items[itemIndex] = MessageListMessageItem(message, content);
+    (items[itemIndex] as MessageListMessageItem).content = content;
   }
 
   /// Append [message] to [messages], and update derived data accordingly.
@@ -161,9 +170,20 @@ mixin _MessageSequence {
   /// This message must already have been parsed and reflected in [contents].
   void _processMessage(int index) {
     // This will get more complicated to handle the ways that messages interact
-    // with the display of neighboring messages: sender headings #175,
-    // recipient headings #174, and date separators #173.
-    items.add(MessageListMessageItem(messages[index], contents[index]));
+    // with the display of neighboring messages: sender headings #175
+    // and date separators #173.
+    final message = messages[index];
+    final content = contents[index];
+    if (index > 0 && canShareRecipientHeader(messages[index - 1], message)) {
+      assert(items.last is MessageListMessageItem);
+      final prevMessageItem = items.last as MessageListMessageItem;
+      assert(identical(prevMessageItem.message, messages[index - 1]));
+      assert(prevMessageItem.isLastInBlock);
+      prevMessageItem.isLastInBlock = false;
+    } else {
+      items.add(MessageListRecipientHeaderItem(message));
+    }
+    items.add(MessageListMessageItem(message, content, isLastInBlock: true));
   }
 
   /// Update [items] to include markers at start and end as appropriate.
@@ -197,6 +217,53 @@ mixin _MessageSequence {
   }
 }
 
+@visibleForTesting
+bool canShareRecipientHeader(Message prevMessage, Message message) {
+  if (prevMessage is StreamMessage && message is StreamMessage) {
+    if (prevMessage.streamId != message.streamId) return false;
+    if (prevMessage.subject != message.subject) return false;
+  } else if (prevMessage is DmMessage && message is DmMessage) {
+    if (!_equalIdSequences(prevMessage.allRecipientIds, message.allRecipientIds)) {
+      return false;
+    }
+  } else {
+    return false;
+  }
+
+  // switch ((prevMessage, message)) {
+  //   case (StreamMessage(), StreamMessage()):
+  //     // TODO(dart-3): this doesn't type-narrow prevMessage and message
+  //   case (DmMessage(), DmMessage()):
+  //     // …
+  //   default:
+  //     return false;
+  // }
+
+  // TODO memoize [DateTime]s... also use memoized for showing date/time in msglist
+  final prevTime = DateTime.fromMillisecondsSinceEpoch(prevMessage.timestamp * 1000);
+  final time = DateTime.fromMillisecondsSinceEpoch(message.timestamp * 1000);
+  if (!_sameDay(prevTime, time)) return false;
+
+  return true;
+}
+
+// Intended for [Message.allRecipientIds].  Assumes efficient `length`.
+bool _equalIdSequences(Iterable<int> xs, Iterable<int> ys) {
+  if (xs.length != ys.length) return false;
+  final xs_ = xs.iterator; final ys_ = ys.iterator;
+  while (xs_.moveNext() && ys_.moveNext()) {
+    if (xs_.current != ys_.current) return false;
+  }
+  return true;
+}
+
+bool _sameDay(DateTime date1, DateTime date2) {
+  if (date1.year != date2.year) return false;
+  if (date1.month != date2.month) return false;
+  if (date1.day != date2.day) return false;
+  return true;
+}
+
 /// A view-model for a message list.
 ///
 /// The owner of one of these objects must call [dispose] when the object
@@ -330,6 +397,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
   /// were changed, and ignores any changes to its stream or topic.
   ///
   /// TODO(#150): Handle message moves.
+  // NB that when handling message moves (#150), recipient headers may need updating.
   void maybeUpdateMessage(UpdateMessageEvent event) {
     final idx = _findMessageWithId(event.messageId);
     if (idx == -1)  {

lib/widgets/message_list.dart

@@ -270,22 +270,26 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
       reverse: true,
       itemBuilder: (context, i) {
         final data = model!.items[length - 1 - i];
-        return switch (data) {
-          MessageListHistoryStartItem() =>
-            const Center(
+        switch (data) {
+          case MessageListHistoryStartItem():
+            return const Center(
               child: Padding(
                 padding: EdgeInsets.symmetric(vertical: 16.0),
-                child: Text("No earlier messages."))), // TODO use an icon
-          MessageListLoadingItem() =>
-            const Center(
+                child: Text("No earlier messages."))); // TODO use an icon
+          case MessageListLoadingItem():
+            return const Center(
               child: Padding(
                 padding: EdgeInsets.symmetric(vertical: 16.0),
-                child: CircularProgressIndicator())), // TODO perhaps a different indicator
-          MessageListMessageItem() =>
-            MessageItem(
+                child: CircularProgressIndicator())); // TODO perhaps a different indicator
+          case MessageListRecipientHeaderItem():
+            final header = RecipientHeader(message: data.message);
+            return StickyHeaderItem(allowOverflow: true,
+              header: header, child: header);
+          case MessageListMessageItem():
+            return MessageItem(
               trailing: i == 0 ? const SizedBox(height: 8) : const SizedBox(height: 11),
-              item: data),
-        };
+              item: data);
+        }
       });
   }
 }
@@ -332,15 +336,10 @@ class RecipientHeader extends StatelessWidget {
   Widget build(BuildContext context) {
     // TODO recipient headings depend on narrow
     final message = this.message;
-    switch (message) {
-      case StreamMessage():
-        final store = PerAccountStoreWidget.of(context);
-        final subscription = store.subscriptions[message.streamId];
-        return StreamTopicRecipientHeader(
-          message: message, streamColor: colorForStream(subscription));
-      case DmMessage():
-        return DmRecipientHeader(message: message);
-    }
+    return switch (message) {
+      StreamMessage() => StreamTopicRecipientHeader(message: message),
+      DmMessage() => DmRecipientHeader(message: message),
+    };
   }
 }
 
@@ -381,17 +380,19 @@ class MessageItem extends StatelessWidget {
       // right than at bottom and in the recipient header: black 10% alpha,
       // vs. 88% lightness.  Assume that's an accident.
       shape: Border(
-        left: recipientBorder, bottom: restBorder, right: restBorder));
+        left: recipientBorder,
+        right: restBorder,
+        bottom: item.isLastInBlock ? restBorder : BorderSide.none,
+      ));
 
-    final recipientHeader = RecipientHeader(message: message);
     return StickyHeaderItem(
-      header: recipientHeader,
+      allowOverflow: !item.isLastInBlock,
+      header: RecipientHeader(message: message),
       child: Column(children: [
-        recipientHeader,
         DecoratedBox(
           decoration: borderDecoration,
           child: MessageWithSender(message: message, content: item.content)),
-        if (trailing != null) trailing!,
+        if (trailing != null && item.isLastInBlock) trailing!,
       ]));
 
     // Web handles the left-side recipient marker in a funky way:
@@ -421,20 +422,25 @@ Color colorForStream(Subscription? subscription) {
 }
 
 class StreamTopicRecipientHeader extends StatelessWidget {
-  const StreamTopicRecipientHeader(
-    {super.key, required this.message, required this.streamColor});
+  const StreamTopicRecipientHeader({super.key, required this.message});
 
   final StreamMessage message;
-  final Color streamColor;
 
   @override
   Widget build(BuildContext context) {
-    final streamName = message.displayRecipient; // TODO get from stream data
+    final store = PerAccountStoreWidget.of(context);
+
+    final stream = store.streams[message.streamId];
+    final streamName = stream?.name ?? message.displayRecipient; // TODO(log) if missing
     final topic = message.subject;
+
+    final subscription = store.subscriptions[message.streamId];
+    final streamColor = colorForStream(subscription);
     final contrastingColor =
       ThemeData.estimateBrightnessForColor(streamColor) == Brightness.dark
         ? Colors.white
         : Colors.black;
+
     return GestureDetector(
       onTap: () => Navigator.push(context,
         MessageListPage.buildRoute(context: context,
@@ -491,12 +497,9 @@ class DmRecipientHeader extends StatelessWidget {
 
     return GestureDetector(
       behavior: HitTestBehavior.translucent,
-      onTap: () {
-        final store = PerAccountStoreWidget.of(context);
-        final narrow = DmNarrow.ofMessage(message, selfUserId: store.account.userId);
-        Navigator.push(context,
-          MessageListPage.buildRoute(context: context, narrow: narrow));
-      },
+      onTap: () => Navigator.push(context,
+        MessageListPage.buildRoute(context: context,
+          narrow: DmNarrow.ofMessage(message, selfUserId: store.account.userId))),
       child: DecoratedBox(
         decoration: BoxDecoration(
           color: Colors.white,