From 83dfd96f0d6f5337bba8dac1d0b1ea6335b0bb85 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 24 Jun 2025 12:02:22 -0600 Subject: [PATCH 1/4] msglist: Decrease end-of-feed gap from 36px to 12px See discussion linked in the comment. --- lib/widgets/message_list.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 39ab1b4f04..c49a7836d0 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. From f060dc086930ef534828326d36d36db434c4cff0 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 24 Jun 2025 13:35:46 -0600 Subject: [PATCH 2/4] msglist [nfc]: Make a common codepath for prevMessage != null I.e., the top of the `else` that belongs to the `if (prevMessage == null)`. We'll use this to implement MessageListMessageBaseItem.isLastInFeed, coming up. --- lib/model/message_list.dart | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index f30d7fac0a..419d2bd4f4 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -435,21 +435,26 @@ 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 { - assert(items.last is MessageListMessageBaseItem); - final prevMessageItem = items.last as MessageListMessageBaseItem; - assert(identical(prevMessageItem.message, prevMessage)); - assert(prevMessageItem.isLastInBlock); - prevMessageItem.isLastInBlock = false; - - if (!messagesSameDay(prevMessageItem.message, message)) { - items.add(MessageListDateSeparatorItem(message)); + if (!haveSameRecipient(prevMessage, message)) { + items.add(MessageListRecipientHeaderItem(message)); canShareSender = false; } else { - canShareSender = prevMessageItem.message.senderId == message.senderId; + assert(items.last is MessageListMessageBaseItem); + final prevMessageItem = items.last as MessageListMessageBaseItem; + assert(identical(prevMessageItem.message, prevMessage)); + assert(prevMessageItem.isLastInBlock); + 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); From cd7a9ead6239b3fbe3ad2611a1418cbfe7738c50 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 24 Jun 2025 13:40:55 -0600 Subject: [PATCH 3/4] msglist [nfc]: Pull out some asserts that also apply when haveSameRecipient These invariants apply regardless of `haveSameRecipient`. --- lib/model/message_list.dart | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 419d2bd4f4..b63f858910 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -439,14 +439,15 @@ mixin _MessageSequence { items.add(MessageListRecipientHeaderItem(message)); canShareSender = false; } else { + assert(items.last is MessageListMessageBaseItem); + final prevMessageItem = items.last as MessageListMessageBaseItem; + assert(identical(prevMessageItem.message, prevMessage)); + assert(prevMessageItem.isLastInBlock); + if (!haveSameRecipient(prevMessage, message)) { items.add(MessageListRecipientHeaderItem(message)); canShareSender = false; } else { - assert(items.last is MessageListMessageBaseItem); - final prevMessageItem = items.last as MessageListMessageBaseItem; - assert(identical(prevMessageItem.message, prevMessage)); - assert(prevMessageItem.isLastInBlock); prevMessageItem.isLastInBlock = false; if (!messagesSameDay(prevMessageItem.message, message)) { From 15b7af944993f27da39073e9dff06b62cedddf69 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 24 Jun 2025 13:48:09 -0600 Subject: [PATCH 4/4] msglist: Remove 11px bottom padding in MessageItem at end of feed See discussions: https://github.com/zulip/zulip-flutter/pull/1453#discussion_r2106526985 https://chat.zulip.org/#narrow/channel/48-mobile/topic/space.20at.20end.20of.20thread/near/2203417 --- lib/model/message_list.dart | 13 +++++++++++-- lib/widgets/message_list.dart | 4 +--- test/model/message_list_test.dart | 4 +++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index b63f858910..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)]), ]); @@ -443,6 +447,9 @@ mixin _MessageSequence { final prevMessageItem = items.last as MessageListMessageBaseItem; assert(identical(prevMessageItem.message, prevMessage)); assert(prevMessageItem.isLastInBlock); + assert(prevMessageItem.isLastInFeed); + + prevMessageItem.isLastInFeed = false; if (!haveSameRecipient(prevMessage, message)) { items.add(MessageListRecipientHeaderItem(message)); @@ -462,6 +469,7 @@ mixin _MessageSequence { assert(identical(item.message, message)); assert(item.showSender == !canShareSender); assert(item.isLastInBlock); + assert(item.isLastInFeed); if (shouldSetMiddleItem) { middleItem = items.length; } @@ -482,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]. @@ -500,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]. @@ -519,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 c49a7836d0..84f3012b41 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -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 {