Skip to content

Commit 9b9265b

Browse files
committed
msglist [nfc]: Migrate most MessageListItem subclasses to use DisplayableMessage
except MessageListMessageItem. This keeps changes minimal, leaving most of the helpers in lib/model/message_list.dart untouched, to avoid unnecessary generalization. Regarding potential performance impact: StreamMessage.destination and DmMessage.destination both construct MessageDestination from scratch, to avoid keeping duplicate info in memory. On the flip side, there might be some slight cost of constructing DmMessage.destination for builds when we convert DmMessage.allRecipientIds to lists. Since we are already doing O(N) traversals on the list anyway (see DmRecipientHeader.build), this cost should be negligible.
1 parent 1e32458 commit 9b9265b

File tree

3 files changed

+35
-26
lines changed

3 files changed

+35
-26
lines changed

lib/model/message_list.dart

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@ sealed class MessageListItem {
2424
}
2525

2626
class MessageListRecipientHeaderItem extends MessageListItem {
27-
final Message message;
27+
final DisplayableMessage message;
2828

2929
MessageListRecipientHeaderItem(this.message);
3030
}
3131

3232
class MessageListDateSeparatorItem extends MessageListItem {
33-
final Message message;
33+
final DisplayableMessage message;
3434

3535
MessageListDateSeparatorItem(this.message);
3636
}
@@ -155,7 +155,7 @@ mixin _MessageSequence {
155155
}
156156
case MessageListRecipientHeaderItem(:var message):
157157
case MessageListDateSeparatorItem(:var message):
158-
return (message.id <= messageId) ? -1 : 1;
158+
return message.id != null && message.id! <= messageId ? -1 : 1;
159159
case MessageListMessageItem(:var message): return message.id.compareTo(messageId);
160160
}
161161
}

lib/widgets/message_list.dart

+30-21
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:flutter_color_models/flutter_color_models.dart';
77
import 'package:intl/intl.dart' hide TextDirection;
88

99
import '../api/model/model.dart';
10+
import '../api/route/messages.dart';
1011
import '../generated/l10n/zulip_localizations.dart';
1112
import '../model/message_list.dart';
1213
import '../model/narrow.dart';
@@ -901,23 +902,28 @@ class _MarkAsReadAnimationState extends State<MarkAsReadAnimation> {
901902
class RecipientHeader extends StatelessWidget {
902903
const RecipientHeader({super.key, required this.message, required this.narrow});
903904

904-
final Message message;
905+
final DisplayableMessage message;
905906
final Narrow narrow;
906907

907908
@override
908909
Widget build(BuildContext context) {
909910
final message = this.message;
910-
return switch (message) {
911-
StreamMessage() => StreamMessageRecipientHeader(message: message, narrow: narrow),
912-
DmMessage() => DmRecipientHeader(message: message, narrow: narrow),
913-
};
911+
switch (message) {
912+
case DisplayableMessage<StreamDestination>():
913+
return StreamMessageRecipientHeader(message: message, narrow: narrow);
914+
case DisplayableMessage<DmDestination>():
915+
return DmRecipientHeader(message: message, narrow: narrow);
916+
case DisplayableMessage<MessageDestination>():
917+
assert(false, 'Unexpected concrete subclass of DisplayableMessage<MessageDestination>');
918+
return SizedBox.shrink();
919+
}
914920
}
915921
}
916922

917923
class DateSeparator extends StatelessWidget {
918924
const DateSeparator({super.key, required this.message});
919925

920-
final Message message;
926+
final DisplayableMessage message;
921927

922928
@override
923929
Widget build(BuildContext context) {
@@ -1027,7 +1033,7 @@ class StreamMessageRecipientHeader extends StatelessWidget {
10271033
required this.narrow,
10281034
});
10291035

1030-
final StreamMessage message;
1036+
final DisplayableMessage<StreamDestination> message;
10311037
final Narrow narrow;
10321038

10331039
static bool _containsDifferentChannels(Narrow narrow) {
@@ -1053,11 +1059,11 @@ class StreamMessageRecipientHeader extends StatelessWidget {
10531059
final designVariables = DesignVariables.of(context);
10541060
final zulipLocalizations = ZulipLocalizations.of(context);
10551061

1056-
final topic = message.topic;
1062+
final StreamDestination(:streamId, :topic) = message.destination;
10571063

10581064
final messageListTheme = MessageListTheme.of(context);
10591065

1060-
final subscription = store.subscriptions[message.streamId];
1066+
final subscription = store.subscriptions[streamId];
10611067
final Color backgroundColor;
10621068
final Color iconColor;
10631069
if (subscription != null) {
@@ -1073,16 +1079,17 @@ class StreamMessageRecipientHeader extends StatelessWidget {
10731079
if (!_containsDifferentChannels(narrow)) {
10741080
streamWidget = const SizedBox(width: 16);
10751081
} else {
1076-
final stream = store.streams[message.streamId];
1082+
final stream = store.streams[streamId];
1083+
final message = this.message;
10771084
final streamName = stream?.name
1078-
?? message.displayRecipient
1085+
?? (message is StreamMessage ? message.displayRecipient : null)
10791086
?? zulipLocalizations.unknownChannelName; // TODO(log)
10801087

10811088
streamWidget = GestureDetector(
10821089
onTap: () => Navigator.push(context,
10831090
MessageListPage.buildRoute(context: context,
1084-
narrow: ChannelNarrow(message.streamId))),
1085-
onLongPress: () => showChannelActionSheet(context, channelId: message.streamId),
1091+
narrow: ChannelNarrow(streamId))),
1092+
onLongPress: () => showChannelActionSheet(context, channelId: streamId),
10861093
child: Row(
10871094
crossAxisAlignment: CrossAxisAlignment.center,
10881095
children: [
@@ -1130,7 +1137,7 @@ class StreamMessageRecipientHeader extends StatelessWidget {
11301137
Icon(size: 14, color: designVariables.title.withFadedAlpha(0.5),
11311138
// A null [Icon.icon] makes a blank space.
11321139
iconDataForTopicVisibilityPolicy(
1133-
store.topicVisibilityPolicy(message.streamId, topic))),
1140+
store.topicVisibilityPolicy(streamId, topic))),
11341141
]));
11351142

11361143
return GestureDetector(
@@ -1141,9 +1148,9 @@ class StreamMessageRecipientHeader extends StatelessWidget {
11411148
onTap: narrow is TopicNarrow ? null
11421149
: () => Navigator.push(context,
11431150
MessageListPage.buildRoute(context: context,
1144-
narrow: TopicNarrow.ofMessage(message))),
1151+
narrow: TopicNarrow(streamId, topic))),
11451152
onLongPress: () => showTopicActionSheet(context,
1146-
channelId: message.streamId,
1153+
channelId: streamId,
11471154
topic: topic,
11481155
someMessageIdInTopic: message.id),
11491156
child: ColoredBox(
@@ -1168,16 +1175,17 @@ class DmRecipientHeader extends StatelessWidget {
11681175
required this.narrow,
11691176
});
11701177

1171-
final DmMessage message;
1178+
final DisplayableMessage<DmDestination> message;
11721179
final Narrow narrow;
11731180

11741181
@override
11751182
Widget build(BuildContext context) {
11761183
final zulipLocalizations = ZulipLocalizations.of(context);
11771184
final store = PerAccountStoreWidget.of(context);
11781185
final String title;
1179-
if (message.allRecipientIds.length > 1) {
1180-
title = zulipLocalizations.messageListGroupYouAndOthers(message.allRecipientIds
1186+
final allRecipientIds = message.destination.userIds;
1187+
if (allRecipientIds.length > 1) {
1188+
title = zulipLocalizations.messageListGroupYouAndOthers(allRecipientIds
11811189
.where((id) => id != store.selfUserId)
11821190
.map(store.userDisplayName)
11831191
.sorted()
@@ -1197,7 +1205,8 @@ class DmRecipientHeader extends StatelessWidget {
11971205
onTap: narrow is DmNarrow ? null
11981206
: () => Navigator.push(context,
11991207
MessageListPage.buildRoute(context: context,
1200-
narrow: DmNarrow.ofMessage(message, selfUserId: store.selfUserId))),
1208+
narrow: DmNarrow.withUsers(
1209+
allRecipientIds, selfUserId: store.selfUserId))),
12011210
child: ColoredBox(
12021211
color: messageListTheme.dmRecipientHeaderBg,
12031212
child: Padding(
@@ -1233,7 +1242,7 @@ TextStyle recipientHeaderTextStyle(BuildContext context, {FontStyle? fontStyle})
12331242
class RecipientHeaderDate extends StatelessWidget {
12341243
const RecipientHeaderDate({super.key, required this.message});
12351244

1236-
final Message message;
1245+
final DisplayableMessage message;
12371246

12381247
@override
12391248
Widget build(BuildContext context) {

test/model/message_list_test.dart

+2-2
Original file line numberDiff line numberDiff line change
@@ -1999,11 +1999,11 @@ void checkInvariants(MessageListView model) {
19991999
}
20002000

20012001
extension MessageListRecipientHeaderItemChecks on Subject<MessageListRecipientHeaderItem> {
2002-
Subject<Message> get message => has((x) => x.message, 'message');
2002+
Subject<DisplayableMessage> get message => has((x) => x.message, 'message');
20032003
}
20042004

20052005
extension MessageListDateSeparatorItemChecks on Subject<MessageListDateSeparatorItem> {
2006-
Subject<Message> get message => has((x) => x.message, 'message');
2006+
Subject<DisplayableMessage> get message => has((x) => x.message, 'message');
20072007
}
20082008

20092009
extension MessageListMessageItemChecks on Subject<MessageListMessageItem> {

0 commit comments

Comments
 (0)