msglist: Show recipient headers on all messages, in Mentions / Starred

Fixes: zulip#1637

Author: chrisbobbe

lib/model/message_list.dart

@@ -105,6 +105,18 @@ enum FetchingStatus {
 ///
 /// This comprises much of the guts of [MessageListView].
 mixin _MessageSequence {
+  /// Whether each message should have its own recipient header,
+  /// even if it's in the same conversation as the previous message.
+  ///
+  /// In some message-list views, notably "Mentions" and "Starred",
+  /// it would be misleading to give the impression that consecutive messages
+  /// in the same conversation were sent one after the other
+  /// with no other messages in between.
+  /// By giving each message its own recipient header (a `true` value for this),
+  /// we intend to avoid giving that impression.
+  @visibleForTesting
+  bool get oneMessagePerBlock;
+
   /// A sequence number for invalidating stale fetches.
   int generation = 0;
 
@@ -435,7 +447,11 @@ mixin _MessageSequence {
     required MessageListMessageBaseItem Function(bool canShareSender) buildItem,
   }) {
     final bool canShareSender;
-    if (prevMessage == null || !haveSameRecipient(prevMessage, message)) {
+    if (
+      prevMessage == null
+      || oneMessagePerBlock
+      || !haveSameRecipient(prevMessage, message)
+    ) {
       items.add(MessageListRecipientHeaderItem(message));
       canShareSender = false;
     } else {
@@ -623,6 +639,15 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     super.dispose();
   }
 
+  @override bool get oneMessagePerBlock => switch (narrow) {
+    CombinedFeedNarrow()
+      || ChannelNarrow()
+      || TopicNarrow()
+      || DmNarrow() => false,
+    MentionsNarrow()
+      || StarredMessagesNarrow() => true,
+  };
+
   /// Whether [message] should actually appear in this message list,
   /// given that it does belong to the narrow.
   ///

test/model/message_list_test.dart

@@ -2742,6 +2742,53 @@ void main() {
     checkNotifiedOnce();
   }));
 
+  group('one message per block?', () {
+    final channelId = 1;
+    final topic = 'some topic';
+    void doTest({required Narrow narrow, required bool expected}) {
+      test('$narrow: ${expected ? 'yes' : 'no'}', () => awaitFakeAsync((async) async {
+        final sender = eg.user();
+        final channel = eg.stream(streamId: channelId);
+        final message1 = eg.streamMessage(
+          sender: sender,
+          stream: channel,
+          topic: topic,
+          flags: [MessageFlag.starred, MessageFlag.mentioned],
+        );
+        final message2 = eg.streamMessage(
+          sender: sender,
+          stream: channel,
+          topic: topic,
+          flags: [MessageFlag.starred, MessageFlag.mentioned],
+        );
+
+        await prepare(
+          narrow: narrow,
+          stream: channel,
+        );
+        connection.prepare(json: newestResult(
+          foundOldest: false,
+          messages: [message1, message2],
+        ).toJson());
+        await model.fetchInitial();
+        checkNotifiedOnce();
+
+        check(model).items.deepEquals(<Condition<Object?>>[
+          (it) => it.isA<MessageListRecipientHeaderItem>(),
+          (it) => it.isA<MessageListMessageItem>(),
+          if (expected) (it) => it.isA<MessageListRecipientHeaderItem>(),
+          (it) => it.isA<MessageListMessageItem>(),
+        ]);
+      }));
+    }
+
+    doTest(narrow: CombinedFeedNarrow(),                expected: false);
+    doTest(narrow: ChannelNarrow(channelId),            expected: false);
+    doTest(narrow: TopicNarrow(channelId, eg.t(topic)), expected: false);
+    doTest(narrow: StarredMessagesNarrow(),             expected: true);
+    doTest(narrow: MentionsNarrow(),                    expected: true);
+  });
+
   test('showSender is maintained correctly', () => awaitFakeAsync((async) async {
     // TODO(#150): This will get more complicated with message moves.
     // Until then, we always compute this sequentially from oldest to newest.
@@ -3011,6 +3058,7 @@ void checkInvariants(MessageListView model) {
   for (int j = 0; j < allMessages.length; j++) {
     bool forcedShowSender = false;
     if (j == 0
+        || model.oneMessagePerBlock
         || !haveSameRecipient(allMessages[j-1], allMessages[j])) {
       check(model.items[i++]).isA<MessageListRecipientHeaderItem>()
         .message.identicalTo(allMessages[j]);