diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index f30d7fac0a..d9ccb96bae 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -44,10 +44,12 @@ sealed class MessageListMessageBaseItem extends MessageListItem { ZulipMessageContent get content; bool showSender; bool isLastInBlock; + bool isLastInFeed; MessageListMessageBaseItem({ required this.showSender, required this.isLastInBlock, + required this.isLastInFeed, }); } @@ -63,6 +65,7 @@ class MessageListMessageItem extends MessageListMessageBaseItem { this.content, { required super.showSender, required super.isLastInBlock, + required super.isLastInFeed, }); } @@ -77,6 +80,7 @@ class MessageListOutboxMessageItem extends MessageListMessageBaseItem { this.message, { required super.showSender, required super.isLastInBlock, + required super.isLastInFeed, }) : content = ZulipContent(nodes: [ ParagraphNode(links: null, nodes: [TextNode(message.contentMarkdown)]), ]); @@ -435,7 +439,7 @@ mixin _MessageSequence { required MessageListMessageBaseItem Function(bool canShareSender) buildItem, }) { final bool canShareSender; - if (prevMessage == null || !haveSameRecipient(prevMessage, message)) { + if (prevMessage == null) { items.add(MessageListRecipientHeaderItem(message)); canShareSender = false; } else { @@ -443,19 +447,29 @@ mixin _MessageSequence { final prevMessageItem = items.last as MessageListMessageBaseItem; assert(identical(prevMessageItem.message, prevMessage)); assert(prevMessageItem.isLastInBlock); - prevMessageItem.isLastInBlock = false; + assert(prevMessageItem.isLastInFeed); - if (!messagesSameDay(prevMessageItem.message, message)) { - items.add(MessageListDateSeparatorItem(message)); + prevMessageItem.isLastInFeed = false; + + if (!haveSameRecipient(prevMessage, message)) { + items.add(MessageListRecipientHeaderItem(message)); canShareSender = false; } else { - canShareSender = prevMessageItem.message.senderId == message.senderId; + prevMessageItem.isLastInBlock = false; + + if (!messagesSameDay(prevMessageItem.message, message)) { + items.add(MessageListDateSeparatorItem(message)); + canShareSender = false; + } else { + canShareSender = prevMessageItem.message.senderId == message.senderId; + } } } final item = buildItem(canShareSender); assert(identical(item.message, message)); assert(item.showSender == !canShareSender); assert(item.isLastInBlock); + assert(item.isLastInFeed); if (shouldSetMiddleItem) { middleItem = items.length; } @@ -476,7 +490,7 @@ mixin _MessageSequence { shouldSetMiddleItem: index == middleMessage, prevMessage: prevMessage, buildItem: (bool canShareSender) => MessageListMessageItem( - message, content, showSender: !canShareSender, isLastInBlock: true)); + message, content, showSender: !canShareSender, isLastInBlock: true, isLastInFeed: true)); } /// Append to [items] based on the index-th message in [outboxMessages]. @@ -494,7 +508,7 @@ mixin _MessageSequence { shouldSetMiddleItem: index == 0 && middleMessage == messages.length, prevMessage: prevMessage, buildItem: (bool canShareSender) => MessageListOutboxMessageItem( - message, showSender: !canShareSender, isLastInBlock: true)); + message, showSender: !canShareSender, isLastInBlock: true, isLastInFeed: true)); } /// Remove items associated with [outboxMessages] from [items]. @@ -513,6 +527,7 @@ mixin _MessageSequence { if (items.isNotEmpty) { final lastItem = items.last as MessageListMessageItem; lastItem.isLastInBlock = true; + lastItem.isLastInFeed = true; } if (middleMessage == messages.length) middleItem = items.length; } diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 39ab1b4f04..84f3012b41 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -963,8 +963,8 @@ class _MessageListState extends State with PerAccountStoreAwareStat // TODO perhaps offer mark-as-read even when not done fetching? MarkAsReadWidget(narrow: widget.narrow), // To reinforce that the end of the feed has been reached: - // https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680603 - const SizedBox(height: 36), + // https://chat.zulip.org/#narrow/channel/48-mobile/topic/space.20at.20end.20of.20thread/near/2203391 + const SizedBox(height: 12), ]); } else if (model.busyFetchingMore) { // See [_buildStartCap] for why this condition shows a loading indicator. @@ -1334,9 +1334,7 @@ class MessageItem extends StatelessWidget { MessageListMessageItem() => MessageWithPossibleSender(item: item), MessageListOutboxMessageItem() => OutboxMessageWithPossibleSender(item: item), }, - // TODO refine this padding; discussion: - // https://github.com/zulip/zulip-flutter/pull/1453#discussion_r2106526985 - if (item.isLastInBlock) const SizedBox(height: 11), + if (item.isLastInBlock && !item.isLastInFeed) const SizedBox(height: 11), ])); if (item case MessageListMessageItem(:final message)) { child = _UnreadMarker( diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index a19229e4a2..bd4f0cf376 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -3037,7 +3037,8 @@ void checkInvariants(MessageListView model) { || MessageListOutboxMessageItem() || MessageListDateSeparatorItem() => false, MessageListRecipientHeaderItem() => true, - }); + }) + ..isLastInFeed.equals(i == model.items.length); } check(model.items).length.equals(i); @@ -3082,6 +3083,7 @@ extension MessageListMessageBaseItemChecks on Subject get content => has((x) => x.content, 'content'); Subject get showSender => has((x) => x.showSender, 'showSender'); Subject get isLastInBlock => has((x) => x.isLastInBlock, 'isLastInBlock'); + Subject get isLastInFeed => has((x) => x.isLastInFeed, 'isLastInFeed'); } extension MessageListMessageItemChecks on Subject {