msglist: Show date separators

Fixes: zulip#173
sirpengi · Jan 5, 2024 · fe4655f · fe4655f
1 parent 69767fb
commit fe4655f
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 8 deletions.
diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart
@@ -25,6 +25,12 @@ class MessageListRecipientHeaderItem extends MessageListItem {
   MessageListRecipientHeaderItem(this.message);
 }
 
+class MessageListDateSeparatorItem extends MessageListItem {
+  final Message message;
+
+  MessageListDateSeparatorItem(this.message);
+}
+
 /// A message to show in the message list.
 class MessageListMessageItem extends MessageListItem {
   final Message message;
@@ -118,6 +124,7 @@ mixin _MessageSequence {
           case MessageListDirection.older:       return -1;
         }
       case MessageListRecipientHeaderItem(:var message):
+      case MessageListDateSeparatorItem(:var message):
         return (message.id <= messageId) ? -1 : 1;
       case MessageListMessageItem(:var message): return message.id.compareTo(messageId);
     }
@@ -181,18 +188,19 @@ mixin _MessageSequence {
     final message = messages[index];
     final content = contents[index];
     bool canShareSender;
-    if (index > 0
-        && haveSameRecipient(messages[index - 1], message)
-        && messagesSameDay(messages[index - 1], message)) {
+    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);
-    } else {
-      items.add(MessageListRecipientHeaderItem(message));
-      canShareSender = false;
     }
     items.add(MessageListMessageItem(message, content,
       showSender: !canShareSender, isLastInBlock: true));

diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart
@@ -344,6 +344,11 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
             final header = RecipientHeader(message: data.message, narrow: widget.narrow);
             return StickyHeaderItem(allowOverflow: true,
               header: header, child: header);
+          case MessageListDateSeparatorItem():
+            final header = RecipientHeader(message: data.message, narrow: widget.narrow);
+            return StickyHeaderItem(allowOverflow: true,
+              header: header,
+              child: DateSeparator(message: data.message));
           case MessageListMessageItem():
             final header = RecipientHeader(message: data.message, narrow: widget.narrow);
             return MessageItem(
@@ -463,6 +468,50 @@ class RecipientHeader extends StatelessWidget {
   }
 }
 
+class DateSeparator extends StatelessWidget {
+  const DateSeparator({super.key, required this.message});
+
+  final Message message;
+
+  // This color matches recipient headers.  TODO(design) is that what we want?
+  static final _textColor = Colors.black.withOpacity(0.4);
+
+  @override
+  Widget build(BuildContext context) {
+    // This makes the small-caps text vertically centered,
+    // to align with the vertically centered divider lines.
+    const textBottomPadding = 2.0;
+
+    return ColoredBox(color: Colors.white,
+      child: Padding(
+        padding: const EdgeInsets.symmetric(vertical: 8, horizontal: 2),
+        child: Row(children: [
+          const Expanded(
+            child: SizedBox(height: 0,
+              child: DecoratedBox(
+                decoration: BoxDecoration(
+                  border: Border(
+                    bottom: BorderSide(
+                      width: 0,
+                      color: Colors.black)))))),
+          Padding(padding: const EdgeInsets.fromLTRB(2, 0, 2, textBottomPadding),
+            child: DateText(
+              color: _textColor,
+              fontSize: 16,
+              height: (16 / 16),
+              timestamp: message.timestamp)),
+          const SizedBox(height: 0, width: 12,
+            child: DecoratedBox(
+              decoration: BoxDecoration(
+                border: Border(
+                  bottom: BorderSide(
+                    width: 0,
+                    color: Colors.black)))))
+        ])),
+    );
+  }
+}
+
 class MessageItem extends StatelessWidget {
   const MessageItem({
     super.key,

diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart
@@ -948,11 +948,13 @@ void checkInvariants(MessageListView model) {
   for (int j = 0; j < model.messages.length; j++) {
     bool isFirstInBlock = false;
     if (j == 0
-        || !haveSameRecipient(model.messages[j-1], model.messages[j])
-        || !messagesSameDay(model.messages[j-1], model.messages[j])) {
+        || !haveSameRecipient(model.messages[j-1], model.messages[j])) {
       check(model.items[i++]).isA<MessageListRecipientHeaderItem>()
         .message.identicalTo(model.messages[j]);
       isFirstInBlock = true;
+    } else if (!messagesSameDay(model.messages[j-1], model.messages[j])) {
+      check(model.items[i++]).isA<MessageListDateSeparatorItem>()
+        .message.identicalTo(model.messages[j]);
     }
     check(model.items[i++]).isA<MessageListMessageItem>()
       ..message.identicalTo(model.messages[j])
@@ -969,6 +971,10 @@ extension MessageListRecipientHeaderItemChecks on Subject<MessageListRecipientHe
   Subject<Message> get message => has((x) => x.message, 'message');
 }
 
+extension MessageListDateSeparatorItemChecks on Subject<MessageListDateSeparatorItem> {
+  Subject<Message> get message => has((x) => x.message, 'message');
+}
+
 extension MessageListMessageItemChecks on Subject<MessageListMessageItem> {
   Subject<Message> get message => has((x) => x.message, 'message');
   Subject<ZulipContent> get content => has((x) => x.content, 'content');