diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 90769e815b..4c12403b85 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -614,7 +614,10 @@ extension type const TopicName(String _value) { /// Different from [MessageDestination], this information comes from /// [getMessages] or [getEvents], identifying the conversation that contains a /// message. -sealed class Conversation {} +sealed class Conversation { + /// Whether [this] and [other] refer to the same Zulip conversation. + bool isSameAs(Conversation other); +} /// The conversation a stream message is in. @JsonSerializable(fieldRename: FieldRename.snake, createToJson: false) @@ -640,6 +643,13 @@ class StreamConversation extends Conversation { factory StreamConversation.fromJson(Map json) => _$StreamConversationFromJson(json); + + @override + bool isSameAs(Conversation other) { + return other is StreamConversation + && streamId == other.streamId + && topic.isSameAs(other.topic); + } } /// The conversation a DM message is in. @@ -653,6 +663,21 @@ class DmConversation extends Conversation { DmConversation({required this.allRecipientIds}) : assert(isSortedWithoutDuplicates(allRecipientIds.toList())); + + bool _equalIdSequences(Iterable xs, Iterable ys) { + if (xs.length != ys.length) return false; + final xs_ = xs.iterator; final ys_ = ys.iterator; + while (xs_.moveNext() && ys_.moveNext()) { + if (xs_.current != ys_.current) return false; + } + return true; + } + + @override + bool isSameAs(Conversation other) { + if (other is! DmConversation) return false; + return _equalIdSequences(allRecipientIds, other.allRecipientIds); + } } /// A message or message-like object, for showing in a message list. diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 58a0e1bb95..2e4c4fb600 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -35,18 +35,31 @@ class MessageListDateSeparatorItem extends MessageListItem { MessageListDateSeparatorItem(this.message); } -/// A message to show in the message list. -class MessageListMessageItem extends MessageListItem { - final Message message; - ZulipMessageContent content; +/// A [MessageBase] to show in the message list. +sealed class MessageListMessageBaseItem extends MessageListItem { + MessageBase get message; + ZulipMessageContent get content; bool showSender; bool isLastInBlock; + MessageListMessageBaseItem({ + required this.showSender, + required this.isLastInBlock, + }); +} + +/// A [Message] to show in the message list. +class MessageListMessageItem extends MessageListMessageBaseItem { + @override + final Message message; + @override + ZulipMessageContent content; + MessageListMessageItem( this.message, this.content, { - required this.showSender, - required this.isLastInBlock, + required super.showSender, + required super.isLastInBlock, }); } @@ -350,31 +363,12 @@ mixin _MessageSequence { } @visibleForTesting -bool haveSameRecipient(Message prevMessage, Message message) { - if (prevMessage is StreamMessage && message is StreamMessage) { - if (prevMessage.streamId != message.streamId) return false; - if (prevMessage.topic.canonicalize() != message.topic.canonicalize()) return false; - } else if (prevMessage is DmMessage && message is DmMessage) { - if (!_equalIdSequences(prevMessage.allRecipientIds, message.allRecipientIds)) { - return false; - } - } else { - return false; - } - return true; - - // switch ((prevMessage, message)) { - // case (StreamMessage(), StreamMessage()): - // // TODO(dart-3): this doesn't type-narrow prevMessage and message - // case (DmMessage(), DmMessage()): - // // … - // default: - // return false; - // } +bool haveSameRecipient(MessageBase prevMessage, MessageBase message) { + return prevMessage.conversation.isSameAs(message.conversation); } @visibleForTesting -bool messagesSameDay(Message prevMessage, Message message) { +bool messagesSameDay(MessageBase prevMessage, MessageBase message) { // TODO memoize [DateTime]s... also use memoized for showing date/time in msglist final prevTime = DateTime.fromMillisecondsSinceEpoch(prevMessage.timestamp * 1000); final time = DateTime.fromMillisecondsSinceEpoch(message.timestamp * 1000); @@ -382,16 +376,6 @@ bool messagesSameDay(Message prevMessage, Message message) { return true; } -// Intended for [Message.allRecipientIds]. Assumes efficient `length`. -bool _equalIdSequences(Iterable xs, Iterable ys) { - if (xs.length != ys.length) return false; - final xs_ = xs.iterator; final ys_ = ys.iterator; - while (xs_.moveNext() && ys_.moveNext()) { - if (xs_.current != ys_.current) return false; - } - return true; -} - bool _sameDay(DateTime date1, DateTime date2) { if (date1.year != date2.year) return false; if (date1.month != date2.month) return false; @@ -439,19 +423,20 @@ class MessageListView with ChangeNotifier, _MessageSequence { /// one way or another. /// /// See also [_allMessagesVisible]. - bool _messageVisible(Message message) { + bool _messageVisible(MessageBase message) { switch (narrow) { case CombinedFeedNarrow(): - return switch (message) { - StreamMessage() => - store.isTopicVisible(message.streamId, message.topic), - DmMessage() => true, + return switch (message.conversation) { + StreamConversation(:final streamId, :final topic) => + store.isTopicVisible(streamId, topic), + DmConversation() => true, }; case ChannelNarrow(:final streamId): - assert(message is StreamMessage && message.streamId == streamId); - if (message is! StreamMessage) return false; - return store.isTopicVisibleInStream(streamId, message.topic); + assert(message is MessageBase + && message.conversation.streamId == streamId); + if (message is! MessageBase) return false; + return store.isTopicVisibleInStream(streamId, message.conversation.topic); case TopicNarrow(): case DmNarrow(): diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index c9e20ce87a..ae7c51b67c 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -992,25 +992,32 @@ class MessageItem extends StatelessWidget { this.trailingWhitespace, }); - final MessageListMessageItem item; + final MessageListMessageBaseItem item; final Widget header; final double? trailingWhitespace; @override Widget build(BuildContext context) { - final message = item.message; final messageListTheme = MessageListTheme.of(context); + + final item = this.item; + Widget child = ColoredBox( + color: messageListTheme.bgMessageRegular, + child: Column(children: [ + switch (item) { + MessageListMessageItem() => MessageWithPossibleSender(item: item), + }, + if (trailingWhitespace != null && item.isLastInBlock) SizedBox(height: trailingWhitespace!), + ])); + if (item case MessageListMessageItem(:final message)) { + child = _UnreadMarker( + isRead: message.flags.contains(MessageFlag.read), + child: child); + } return StickyHeaderItem( allowOverflow: !item.isLastInBlock, header: header, - child: _UnreadMarker( - isRead: message.flags.contains(MessageFlag.read), - child: ColoredBox( - color: messageListTheme.bgMessageRegular, - child: Column(children: [ - MessageWithPossibleSender(item: item), - if (trailingWhitespace != null && item.isLastInBlock) SizedBox(height: trailingWhitespace!), - ])))); + child: child); } } @@ -1338,14 +1345,14 @@ String formatHeaderDate( } } -/// A Zulip message, showing the sender's name and avatar if specified. -// Design referenced from: -// - https://github.com/zulip/zulip-mobile/issues/5511 -// - https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=538%3A20849&mode=dev -class MessageWithPossibleSender extends StatelessWidget { - const MessageWithPossibleSender({super.key, required this.item}); +// TODO(i18n): web seems to ignore locale in formatting time, but we could do better +final _kMessageTimestampFormat = DateFormat('h:mm aa', 'en_US'); - final MessageListMessageItem item; +class _SenderRow extends StatelessWidget { + const _SenderRow({required this.message, required this.showTimestamp}); + + final Message message; + final bool showTimestamp; @override Widget build(BuildContext context) { @@ -1353,14 +1360,12 @@ class MessageWithPossibleSender extends StatelessWidget { final messageListTheme = MessageListTheme.of(context); final designVariables = DesignVariables.of(context); - final message = item.message; final sender = store.getUser(message.senderId); - - Widget? senderRow; - if (item.showSender) { - final time = _kMessageTimestampFormat - .format(DateTime.fromMillisecondsSinceEpoch(1000 * message.timestamp)); - senderRow = Row( + final time = _kMessageTimestampFormat + .format(DateTime.fromMillisecondsSinceEpoch(1000 * message.timestamp)); + return Padding( + padding: const EdgeInsets.fromLTRB(16, 2, 16, 0), + child: Row( mainAxisAlignment: MainAxisAlignment.spaceBetween, crossAxisAlignment: CrossAxisAlignment.baseline, textBaseline: localizedTextBaseline(context), @@ -1392,16 +1397,33 @@ class MessageWithPossibleSender extends StatelessWidget { ), ], ]))), - const SizedBox(width: 4), - Text(time, - style: TextStyle( - color: messageListTheme.labelTime, - fontSize: 16, - height: (18 / 16), - fontFeatures: const [FontFeature.enable('c2sc'), FontFeature.enable('smcp')], - ).merge(weightVariableTextStyle(context))), - ]); - } + if (showTimestamp) ...[ + const SizedBox(width: 4), + Text(time, + style: TextStyle( + color: messageListTheme.labelTime, + fontSize: 16, + height: (18 / 16), + fontFeatures: const [FontFeature.enable('c2sc'), FontFeature.enable('smcp')], + ).merge(weightVariableTextStyle(context))), + ], + ])); + } +} + +/// A Zulip message, showing the sender's name and avatar if specified. +// Design referenced from: +// - https://github.com/zulip/zulip-mobile/issues/5511 +// - https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=538%3A20849&mode=dev +class MessageWithPossibleSender extends StatelessWidget { + const MessageWithPossibleSender({super.key, required this.item}); + + final MessageListMessageItem item; + + @override + Widget build(BuildContext context) { + final designVariables = DesignVariables.of(context); + final message = item.message; final localizations = ZulipLocalizations.of(context); String? editStateText; @@ -1430,9 +1452,8 @@ class MessageWithPossibleSender extends StatelessWidget { child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), child: Column(children: [ - if (senderRow != null) - Padding(padding: const EdgeInsets.fromLTRB(16, 2, 16, 0), - child: senderRow), + if (item.showSender) + _SenderRow(message: message, showTimestamp: true), Row( crossAxisAlignment: CrossAxisAlignment.baseline, textBaseline: localizedTextBaseline(context), @@ -1460,6 +1481,3 @@ class MessageWithPossibleSender extends StatelessWidget { ]))); } } - -// TODO(i18n): web seems to ignore locale in formatting time, but we could do better -final _kMessageTimestampFormat = DateFormat('h:mm aa', 'en_US'); diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index f92bdfc096..4aedea1d03 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -360,37 +360,39 @@ void main() { }); }); - test('MessageEvent', () async { - final stream = eg.stream(); - await prepare(narrow: ChannelNarrow(stream.streamId)); - await prepareMessages(foundOldest: true, messages: - List.generate(30, (i) => eg.streamMessage(stream: stream))); + group('MessageEvent', () { + test('in narrow', () async { + final stream = eg.stream(); + await prepare(narrow: ChannelNarrow(stream.streamId)); + await prepareMessages(foundOldest: true, messages: + List.generate(30, (i) => eg.streamMessage(stream: stream))); - check(model).messages.length.equals(30); - await store.addMessage(eg.streamMessage(stream: stream)); - checkNotifiedOnce(); - check(model).messages.length.equals(31); - }); + check(model).messages.length.equals(30); + await store.addMessage(eg.streamMessage(stream: stream)); + checkNotifiedOnce(); + check(model).messages.length.equals(31); + }); - test('MessageEvent, not in narrow', () async { - final stream = eg.stream(); - await prepare(narrow: ChannelNarrow(stream.streamId)); - await prepareMessages(foundOldest: true, messages: - List.generate(30, (i) => eg.streamMessage(stream: stream))); + test('not in narrow', () async { + final stream = eg.stream(); + await prepare(narrow: ChannelNarrow(stream.streamId)); + await prepareMessages(foundOldest: true, messages: + List.generate(30, (i) => eg.streamMessage(stream: stream))); - check(model).messages.length.equals(30); - final otherStream = eg.stream(); - await store.addMessage(eg.streamMessage(stream: otherStream)); - checkNotNotified(); - check(model).messages.length.equals(30); - }); + check(model).messages.length.equals(30); + final otherStream = eg.stream(); + await store.addMessage(eg.streamMessage(stream: otherStream)); + checkNotNotified(); + check(model).messages.length.equals(30); + }); - test('MessageEvent, before fetch', () async { - final stream = eg.stream(); - await prepare(narrow: ChannelNarrow(stream.streamId)); - await store.addMessage(eg.streamMessage(stream: stream)); - checkNotNotified(); - check(model).fetched.isFalse(); + test('before fetch', () async { + final stream = eg.stream(); + await prepare(narrow: ChannelNarrow(stream.streamId)); + await store.addMessage(eg.streamMessage(stream: stream)); + checkNotNotified(); + check(model).fetched.isFalse(); + }); }); group('UserTopicEvent', () { @@ -2001,13 +2003,17 @@ extension MessageListDateSeparatorItemChecks on Subject get message => has((x) => x.message, 'message'); } -extension MessageListMessageItemChecks on Subject { - Subject get message => has((x) => x.message, 'message'); +extension MessageListMessageBaseItemChecks on Subject { + Subject get message => has((x) => x.message, 'message'); Subject get content => has((x) => x.content, 'content'); Subject get showSender => has((x) => x.showSender, 'showSender'); Subject get isLastInBlock => has((x) => x.isLastInBlock, 'isLastInBlock'); } +extension MessageListMessageItemChecks on Subject { + Subject get message => has((x) => x.message, 'message'); +} + extension MessageListViewChecks on Subject { Subject get store => has((x) => x.store, 'store'); Subject get narrow => has((x) => x.narrow, 'narrow');