msglist: Let recipient header overlap date, just like a message

lib/model/message_list.dart

@@ -191,16 +191,19 @@ mixin _MessageSequence {
     if (index == 0 || !haveSameRecipient(messages[index - 1], message)) {
       items.add(MessageListRecipientHeaderItem(message));
       canShareSender = false;
-    } else if (!messagesSameDay(messages[index - 1], message)) {
-      items.add(MessageListDateSeparatorItem(message));
-      canShareSender = false;
     } else {
       assert(items.last is MessageListMessageItem);
       final prevMessageItem = items.last as MessageListMessageItem;
       assert(identical(prevMessageItem.message, messages[index - 1]));
       assert(prevMessageItem.isLastInBlock);
       prevMessageItem.isLastInBlock = false;
-      canShareSender = (prevMessageItem.message.senderId == message.senderId);
+
+      if (!messagesSameDay(prevMessageItem.message, message)) {
+        items.add(MessageListDateSeparatorItem(message));
+        canShareSender = false;
+      } else {
+        canShareSender = (prevMessageItem.message.senderId == message.senderId);
+      }
     }
     items.add(MessageListMessageItem(message, content,
       showSender: !canShareSender, isLastInBlock: true));

test/model/message_list_test.dart

@@ -689,16 +689,17 @@ void main() async {
   });
 
   test('recipient headers are maintained consistently', () async {
+    // TODO test date separators are maintained consistently too
     // This tests the code that maintains the invariant that recipient headers
-    // are present just where [canShareRecipientHeader] requires them.
+    // are present just where they're required.
     // In [checkInvariants] we check the current state against that invariant,
     // so here we just need to exercise that code through all the relevant cases.
     // Each [checkNotifiedOnce] call ensures there's been a [checkInvariants] call
     // (in the listener that increments [notifiedCount]).
     //
-    // A separate unit test covers [canShareRecipientHeader] itself.
-    // So this test just needs messages that can share, and messages that can't,
-    // and doesn't need to exercise the different reasons that messages can't.
+    // A separate unit test covers [haveSameRecipient] itself.  So this test
+    // just needs messages that have the same recipient, and that don't, and
+    // doesn't need to exercise the different reasons that messages don't.
 
     const timestamp = 1693602618;
     final stream = eg.stream();
@@ -780,21 +781,23 @@ void main() async {
     // whether the sender should be shown, but the difference between
     // fetchInitial and maybeAddMessage etc. doesn't matter.
 
-    const timestamp = 1693602618;
+    const t1 = 1693602618;
+    const t2 = t1 + 86400;
     final stream = eg.stream();
-    Message streamMessage(int id, User sender) =>
+    Message streamMessage(int id, int timestamp, User sender) =>
       eg.streamMessage(id: id, sender: sender,
         stream: stream, topic: 'foo', timestamp: timestamp);
-    Message dmMessage(int id, User sender) =>
+    Message dmMessage(int id, int timestamp, User sender) =>
       eg.dmMessage(id: id, from: sender, timestamp: timestamp,
         to: [sender.userId == eg.selfUser.userId ? eg.otherUser : eg.selfUser]);
 
     prepare();
     await prepareMessages(foundOldest: true, messages: [
-      streamMessage(1, eg.selfUser),  // first message, so show sender
-      streamMessage(2, eg.selfUser),  // hide sender
-      streamMessage(3, eg.otherUser), // no recipient header, but new sender
-      dmMessage(4,     eg.otherUser), // same sender, but recipient header
+      streamMessage(1, t1, eg.selfUser),  // first message, so show sender
+      streamMessage(2, t1, eg.selfUser),  // hide sender
+      streamMessage(3, t1, eg.otherUser), // no recipient header, but new sender
+      dmMessage(4,     t1, eg.otherUser), // same sender, but new recipient
+      dmMessage(5,     t2, eg.otherUser), // same sender/recipient, but new day
     ]);
 
     // We check showSender has the right values in [checkInvariants],
@@ -807,6 +810,8 @@ void main() async {
       it()..isA<MessageListMessageItem>().showSender.isTrue(),
       it()..isA<MessageListRecipientHeaderItem>(),
       it()..isA<MessageListMessageItem>().showSender.isTrue(),
+      it()..isA<MessageListDateSeparatorItem>(),
+      it()..isA<MessageListMessageItem>().showSender.isTrue(),
     ]);
   });
 
@@ -946,23 +951,30 @@ void checkInvariants(MessageListView model) {
     check(model.items[i++]).isA<MessageListLoadingItem>();
   }
   for (int j = 0; j < model.messages.length; j++) {
-    bool isFirstInBlock = false;
+    bool forcedShowSender = false;
     if (j == 0
         || !haveSameRecipient(model.messages[j-1], model.messages[j])) {
       check(model.items[i++]).isA<MessageListRecipientHeaderItem>()
         .message.identicalTo(model.messages[j]);
-      isFirstInBlock = true;
+      forcedShowSender = true;
     } else if (!messagesSameDay(model.messages[j-1], model.messages[j])) {
       check(model.items[i++]).isA<MessageListDateSeparatorItem>()
         .message.identicalTo(model.messages[j]);
+      forcedShowSender = true;
     }
     check(model.items[i++]).isA<MessageListMessageItem>()
       ..message.identicalTo(model.messages[j])
       ..content.identicalTo(model.contents[j])
       ..showSender.equals(
-        isFirstInBlock || model.messages[j].senderId != model.messages[j-1].senderId)
+        forcedShowSender || model.messages[j].senderId != model.messages[j-1].senderId)
       ..isLastInBlock.equals(
-        i == model.items.length || model.items[i] is! MessageListMessageItem);
+        i == model.items.length || switch (model.items[i]) {
+          MessageListMessageItem()
+          || MessageListDateSeparatorItem() => false,
+          MessageListRecipientHeaderItem()
+          || MessageListHistoryStartItem()
+          || MessageListLoadingItem()       => true,
+        });
   }
   check(model.items).length.equals(i);
 }