Skip to content

Commit 2649bc6

Browse files
committed
msglist [nfc]: Migrate most MessageListItem subclasses to use MessageBase
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 381e0c0 commit 2649bc6

File tree

4 files changed

+33
-24
lines changed

4 files changed

+33
-24
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 MessageBase message;
2828

2929
MessageListRecipientHeaderItem(this.message);
3030
}
3131

3232
class MessageListDateSeparatorItem extends MessageListItem {
33-
final Message message;
33+
final MessageBase 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/model/narrow.dart

+3-1
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,9 @@ class DmNarrow extends Narrow implements SendableNarrow {
194194
);
195195
}
196196

197-
factory DmNarrow.ofMessage(DmMessage message, {required int selfUserId}) {
197+
factory DmNarrow.ofMessage(MessageBase<DmDestination> message, {
198+
required int selfUserId,
199+
}) {
198200
return DmNarrow(
199201
allRecipientIds: List.unmodifiable(message.destination.userIds),
200202
selfUserId: selfUserId,

lib/widgets/message_list.dart

+25-18
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 MessageBase 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 MessageBase<StreamDestination>():
913+
return StreamMessageRecipientHeader(message: message, narrow: narrow);
914+
case MessageBase<DmDestination>():
915+
return DmRecipientHeader(message: message, narrow: narrow);
916+
case MessageBase<MessageDestination>():
917+
assert(false, 'Unexpected concrete subclass of MessageBase<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 MessageBase 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 MessageBase<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,7 +1175,7 @@ class DmRecipientHeader extends StatelessWidget {
11681175
required this.narrow,
11691176
});
11701177

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

11741181
@override
@@ -1233,7 +1240,7 @@ TextStyle recipientHeaderTextStyle(BuildContext context, {FontStyle? fontStyle})
12331240
class RecipientHeaderDate extends StatelessWidget {
12341241
const RecipientHeaderDate({super.key, required this.message});
12351242

1236-
final Message message;
1243+
final MessageBase message;
12371244

12381245
@override
12391246
Widget build(BuildContext context) {

test/model/message_list_test.dart

+2-2
Original file line numberDiff line numberDiff line change
@@ -1994,11 +1994,11 @@ void checkInvariants(MessageListView model) {
19941994
}
19951995

19961996
extension MessageListRecipientHeaderItemChecks on Subject<MessageListRecipientHeaderItem> {
1997-
Subject<Message> get message => has((x) => x.message, 'message');
1997+
Subject<MessageBase> get message => has((x) => x.message, 'message');
19981998
}
19991999

20002000
extension MessageListDateSeparatorItemChecks on Subject<MessageListDateSeparatorItem> {
2001-
Subject<Message> get message => has((x) => x.message, 'message');
2001+
Subject<MessageBase> get message => has((x) => x.message, 'message');
20022002
}
20032003

20042004
extension MessageListMessageItemChecks on Subject<MessageListMessageItem> {

0 commit comments

Comments
 (0)