From 436255973dba2a22b57b75b5c8d5b4fb76bad2a5 Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Wed, 14 Aug 2024 13:31:59 +0300 Subject: [PATCH 1/6] msglist: Add message to mentions if mentioned Fixes: #649 --- lib/model/store.dart | 2 +- lib/model/unreads.dart | 14 +++++++++++--- test/model/unreads_test.dart | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/model/store.dart b/lib/model/store.dart index 8cc25f2759..fa6c6f9304 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -407,7 +407,7 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess @override void reconcileMessages(List messages) { _messages.reconcileMessages(messages); - // TODO(#649) notify [unreads] of the just-fetched messages + unreads.reconcileMessages(messages); // TODO(#650) notify [recentDmConversationsView] of the just-fetched messages } diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index 76429fb20a..522cc88e42 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -32,8 +32,6 @@ import 'channel.dart'; /// unsubscribed streams and messages sent by muted users. // TODO When [oldUnreadsMissing], if you load a message list with very old unreads, // sync to those unreads, because the user has shown an interest in them. -// TODO When loading a message list with stream messages, check all the stream -// messages and refresh [mentions] (see [mentions] dartdoc). class Unreads extends ChangeNotifier { factory Unreads({ required UnreadMessagesSnapshot initial, @@ -103,7 +101,7 @@ class Unreads extends ChangeNotifier { /// a) the message is edited at all ([UpdateMessageEvent]), /// assuming it still has a direct or wildcard mention after the edit, or /// b) the message gains a direct @-mention ([UpdateMessageFlagsEvent]), or - /// c) TODO unimplemented: the user loads the message in the message list + /// c) the user loads the message in the message list /// But otherwise, assume its unread state remains unknown to [mentions]. /// /// [1] This item applies verbatim at Server 8.0+. For older servers, the @@ -214,6 +212,16 @@ class Unreads extends ChangeNotifier { } } + void reconcileMessages(List messages) { + for (final message in messages) { + if (message.flags.contains(MessageFlag.read)) continue; + if (message.flags.contains(MessageFlag.mentioned) + || message.flags.contains(MessageFlag.wildcardMentioned)) { + mentions.add(message.id); + } + } + } + void handleMessageEvent(MessageEvent event) { final message = event.message; if (message.flags.contains(MessageFlag.read)) { diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index 47f257dc89..d46aa8138b 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -240,6 +240,23 @@ void main() { }); }); + group('onMessagesFetched', () { + test('messages are added to mentions when they are not read and include mention', () async { + final stream = eg.stream(); + prepare(); + await channelStore.addStream(stream); + check(model.mentions).isEmpty(); + fillWithMessages([ + eg.streamMessage(stream: stream, flags: []), + eg.streamMessage(stream: stream, flags: [MessageFlag.mentioned, MessageFlag.read]), + eg.streamMessage(stream: stream, flags: [MessageFlag.mentioned]), + eg.streamMessage(stream: stream, flags: [MessageFlag.wildcardMentioned]), + ]); + + check(model.mentions.length).equals(2); + }); + }); + group('handleMessageEvent', () { for (final (isUnread, isStream, isDirectMentioned, isWildcardMentioned) in [ (true, true, true, true ), From b3b9f530013120d00e84b92f73662ad969e125ae Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Fri, 9 Aug 2024 21:12:47 +0300 Subject: [PATCH 2/6] unreads [nfc]: Introduce _streamIdsAndTopicsByMessageId data structure This will be helpful for efficiently checking for @-mention in subscription_list. --- lib/model/unreads.dart | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index 522cc88e42..3f3ad7f9a7 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -39,6 +39,7 @@ class Unreads extends ChangeNotifier { required ChannelStore channelStore, }) { final streams = >>{}; + final streamIdsAndTopicsByMessageId = {}; final dms = >{}; final mentions = Set.of(initial.mentions); @@ -46,6 +47,9 @@ class Unreads extends ChangeNotifier { final streamId = unreadChannelSnapshot.streamId; final topic = unreadChannelSnapshot.topic; (streams[streamId] ??= {})[topic] = QueueList.from(unreadChannelSnapshot.unreadMessageIds); + final messageInfo = (streamId: streamId, topic: topic); + streamIdsAndTopicsByMessageId.addEntries( + unreadChannelSnapshot.unreadMessageIds.map((messageId) => MapEntry(messageId, messageInfo))); } for (final unreadDmSnapshot in initial.dms) { @@ -62,6 +66,7 @@ class Unreads extends ChangeNotifier { return Unreads._( channelStore: channelStore, streams: streams, + streamIdsAndTopicsByMessageId: streamIdsAndTopicsByMessageId, dms: dms, mentions: mentions, oldUnreadsMissing: initial.oldUnreadsMissing, @@ -72,11 +77,12 @@ class Unreads extends ChangeNotifier { Unreads._({ required this.channelStore, required this.streams, + required Map streamIdsAndTopicsByMessageId, required this.dms, required this.mentions, required this.oldUnreadsMissing, required this.selfUserId, - }); + }) : _streamIdsAndTopicsByMessageId = streamIdsAndTopicsByMessageId; final ChannelStore channelStore; @@ -86,6 +92,11 @@ class Unreads extends ChangeNotifier { /// Unread stream messages, as: stream ID → topic → message IDs (sorted). final Map>> streams; + /// Stream IDs and topics for unread stream messages, as: message ID → (stream ID, topic). + /// + /// This is the same data as [streams], but in this message ID-keyed form. + final Map _streamIdsAndTopicsByMessageId; + /// Unread DM messages, as: DM narrow → message IDs (sorted). final Map> dms; @@ -350,6 +361,7 @@ class Unreads extends ChangeNotifier { case UpdateMessageFlagsAddEvent(): if (event.all) { streams.clear(); + _streamIdsAndTopicsByMessageId.clear(); dms.clear(); mentions.clear(); oldUnreadsMissing = false; @@ -446,6 +458,7 @@ class Unreads extends ChangeNotifier { void _addLastInStreamTopic(int messageId, int streamId, String topic) { ((streams[streamId] ??= {})[topic] ??= QueueList()).addLast(messageId); + _streamIdsAndTopicsByMessageId[messageId] = (streamId: streamId, topic: topic); } // [messageIds] must be sorted ascending and without duplicates. @@ -458,6 +471,9 @@ class Unreads extends ChangeNotifier { // TODO(server-6) remove 6.0 comment (existing) => setUnion(existing, messageIds), ); + final messageInfo = (streamId: streamId, topic: topic); + _streamIdsAndTopicsByMessageId.addEntries( + messageIds.map((messageId) => MapEntry(messageId, messageInfo))); } // TODO use efficient model lookups @@ -481,6 +497,9 @@ class Unreads extends ChangeNotifier { for (final streamId in newlyEmptyStreams) { streams.remove(streamId); } + for (final messageId in idsToRemove) { + _streamIdsAndTopicsByMessageId.remove(messageId); + } } void _removeAllInStreamTopic(Set incomingMessageIds, int streamId, String topic) { @@ -497,6 +516,9 @@ class Unreads extends ChangeNotifier { streams.remove(streamId); } } + for (final messageId in incomingMessageIds) { + _streamIdsAndTopicsByMessageId.remove(messageId); + } } // TODO use efficient model lookups From 2a00ea1c6c0b72be54e4b1712780d343410572ac Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Sat, 10 Aug 2024 14:52:38 +0300 Subject: [PATCH 3/6] inbox [nfc]: Move AtMentionMarker to unread_count_badge AtMentionMarker is going to be used in subscription_list as well as inbox, so it would be better to reuse it by having it declared as a public class. --- lib/widgets/inbox.dart | 20 +++----------------- lib/widgets/unread_count_badge.dart | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index 9259c81364..2c95d904af 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -282,7 +282,7 @@ abstract class _HeaderItem extends StatelessWidget { overflow: TextOverflow.ellipsis, title))), const SizedBox(width: 12), - if (hasMention) const _AtMentionMarker(), + if (hasMention) const AtMentionMarker(), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge( backgroundColor: unreadCountBadgeBackgroundColor(context), @@ -405,7 +405,7 @@ class _DmItem extends StatelessWidget { overflow: TextOverflow.ellipsis, title))), const SizedBox(width: 12), - if (hasMention) const _AtMentionMarker(), + if (hasMention) const AtMentionMarker(), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge(backgroundColor: null, count: count)), @@ -530,7 +530,7 @@ class _TopicItem extends StatelessWidget { overflow: TextOverflow.ellipsis, topic))), const SizedBox(width: 12), - if (hasMention) const _AtMentionMarker(), + if (hasMention) const AtMentionMarker(), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge( backgroundColor: colorSwatchFor(context, subscription), @@ -538,17 +538,3 @@ class _TopicItem extends StatelessWidget { ])))); } } - -class _AtMentionMarker extends StatelessWidget { - const _AtMentionMarker(); - - @override - Widget build(BuildContext context) { - final designVariables = DesignVariables.of(context); - // Design for at-mention marker based on Figma screen: - // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=224-16386&mode=design&t=JsNndFQ8fKFH0SjS-0 - return Padding( - padding: const EdgeInsetsDirectional.only(end: 4), - child: Icon(ZulipIcons.at_sign, size: 14, color: designVariables.atMentionMarker)); - } -} diff --git a/lib/widgets/unread_count_badge.dart b/lib/widgets/unread_count_badge.dart index 5c09e156cd..505b39c653 100644 --- a/lib/widgets/unread_count_badge.dart +++ b/lib/widgets/unread_count_badge.dart @@ -1,6 +1,7 @@ import 'package:flutter/widgets.dart'; import 'channel_colors.dart'; +import 'icons.dart'; import 'text.dart'; import 'theme.dart'; @@ -72,3 +73,17 @@ class MutedUnreadBadge extends StatelessWidget { shape: BoxShape.circle)); } } + +class AtMentionMarker extends StatelessWidget { + const AtMentionMarker({super.key}); + + @override + Widget build(BuildContext context) { + final designVariables = DesignVariables.of(context); + // Design for at-mention marker based on Figma screen: + // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=224-16386&mode=design&t=JsNndFQ8fKFH0SjS-0 + return Padding( + padding: const EdgeInsetsDirectional.only(end: 4), + child: Icon(ZulipIcons.at_sign, size: 14, color: designVariables.atMentionMarker)); + } +} From ef6d4c3333288e276fed0bb7d670e15848cf659f Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Wed, 14 Aug 2024 15:19:18 +0300 Subject: [PATCH 4/6] inbox [nfc]: Introduce muted property to AtMentionMarker --- lib/widgets/inbox.dart | 6 +++--- lib/widgets/theme.dart | 11 +++++++++-- lib/widgets/unread_count_badge.dart | 9 +++++++-- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index 2c95d904af..03fdee5cb5 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -282,7 +282,7 @@ abstract class _HeaderItem extends StatelessWidget { overflow: TextOverflow.ellipsis, title))), const SizedBox(width: 12), - if (hasMention) const AtMentionMarker(), + if (hasMention) const AtMentionMarker(muted: false), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge( backgroundColor: unreadCountBadgeBackgroundColor(context), @@ -405,7 +405,7 @@ class _DmItem extends StatelessWidget { overflow: TextOverflow.ellipsis, title))), const SizedBox(width: 12), - if (hasMention) const AtMentionMarker(), + if (hasMention) const AtMentionMarker(muted: false), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge(backgroundColor: null, count: count)), @@ -530,7 +530,7 @@ class _TopicItem extends StatelessWidget { overflow: TextOverflow.ellipsis, topic))), const SizedBox(width: 12), - if (hasMention) const AtMentionMarker(), + if (hasMention) const AtMentionMarker(muted: false), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge( backgroundColor: colorSwatchFor(context, subscription), diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index d7a6ddabf6..066178f371 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -117,7 +117,8 @@ class DesignVariables extends ThemeExtension { mainBackground: const Color(0xfff0f0f0), title: const Color(0xff1a1a1a), channelColorSwatches: ChannelColorSwatches.light, - atMentionMarker: const HSLColor.fromAHSL(0.5, 0, 0, 0.2).toColor(), + atMentionMarker: const HSLColor.fromAHSL(0.7, 0, 0, 0.2).toColor(), + mutedAtMentionMarker: const HSLColor.fromAHSL(0.35, 0, 0, 0.2).toColor(), dmHeaderBg: const HSLColor.fromAHSL(1, 46, 0.35, 0.93).toColor(), errorBannerBackground: const HSLColor.fromAHSL(1, 4, 0.33, 0.90).toColor(), errorBannerBorder: const HSLColor.fromAHSL(0.4, 3, 0.57, 0.33).toColor(), @@ -147,7 +148,8 @@ class DesignVariables extends ThemeExtension { title: const Color(0xffffffff), channelColorSwatches: ChannelColorSwatches.dark, // TODO(design-dark) need proper dark-theme color (this is ad hoc) - atMentionMarker: const HSLColor.fromAHSL(0.4, 0, 0, 1).toColor(), + atMentionMarker: const HSLColor.fromAHSL(0.7, 0, 0, 1).toColor(), + mutedAtMentionMarker: const HSLColor.fromAHSL(0.4, 0, 0, 1).toColor(), dmHeaderBg: const HSLColor.fromAHSL(1, 46, 0.15, 0.2).toColor(), errorBannerBackground: const HSLColor.fromAHSL(1, 0, 0.61, 0.19).toColor(), errorBannerBorder: const HSLColor.fromAHSL(0.4, 3, 0.73, 0.74).toColor(), @@ -182,6 +184,7 @@ class DesignVariables extends ThemeExtension { required this.title, required this.channelColorSwatches, required this.atMentionMarker, + required this.mutedAtMentionMarker, required this.dmHeaderBg, required this.errorBannerBackground, required this.errorBannerBorder, @@ -223,6 +226,7 @@ class DesignVariables extends ThemeExtension { // Not named variables in Figma; taken from older Figma drafts, or elsewhere. final Color atMentionMarker; + final Color mutedAtMentionMarker; final Color dmHeaderBg; final Color errorBannerBackground; final Color errorBannerBorder; @@ -251,6 +255,7 @@ class DesignVariables extends ThemeExtension { Color? title, ChannelColorSwatches? channelColorSwatches, Color? atMentionMarker, + Color? mutedAtMentionMarker, Color? dmHeaderBg, Color? errorBannerBackground, Color? errorBannerBorder, @@ -278,6 +283,7 @@ class DesignVariables extends ThemeExtension { title: title ?? this.title, channelColorSwatches: channelColorSwatches ?? this.channelColorSwatches, atMentionMarker: atMentionMarker ?? this.atMentionMarker, + mutedAtMentionMarker: mutedAtMentionMarker ?? this.mutedAtMentionMarker, dmHeaderBg: dmHeaderBg ?? this.dmHeaderBg, errorBannerBackground: errorBannerBackground ?? this.errorBannerBackground, errorBannerBorder: errorBannerBorder ?? this.errorBannerBorder, @@ -312,6 +318,7 @@ class DesignVariables extends ThemeExtension { title: Color.lerp(title, other.title, t)!, channelColorSwatches: ChannelColorSwatches.lerp(channelColorSwatches, other.channelColorSwatches, t), atMentionMarker: Color.lerp(atMentionMarker, other.atMentionMarker, t)!, + mutedAtMentionMarker: Color.lerp(mutedAtMentionMarker, other.mutedAtMentionMarker, t)!, dmHeaderBg: Color.lerp(dmHeaderBg, other.dmHeaderBg, t)!, errorBannerBackground: Color.lerp(errorBannerBackground, other.errorBannerBackground, t)!, errorBannerBorder: Color.lerp(errorBannerBorder, other.errorBannerBorder, t)!, diff --git a/lib/widgets/unread_count_badge.dart b/lib/widgets/unread_count_badge.dart index 505b39c653..e46a59061b 100644 --- a/lib/widgets/unread_count_badge.dart +++ b/lib/widgets/unread_count_badge.dart @@ -75,7 +75,9 @@ class MutedUnreadBadge extends StatelessWidget { } class AtMentionMarker extends StatelessWidget { - const AtMentionMarker({super.key}); + const AtMentionMarker({super.key, required this.muted}); + + final bool muted; @override Widget build(BuildContext context) { @@ -84,6 +86,9 @@ class AtMentionMarker extends StatelessWidget { // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=224-16386&mode=design&t=JsNndFQ8fKFH0SjS-0 return Padding( padding: const EdgeInsetsDirectional.only(end: 4), - child: Icon(ZulipIcons.at_sign, size: 14, color: designVariables.atMentionMarker)); + child: Icon( + ZulipIcons.at_sign, + size: 14, + color: muted ? designVariables.mutedAtMentionMarker : designVariables.atMentionMarker)); } } From b1e8fb76f34f694ddc4991190bee044b241d5c00 Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Wed, 14 Aug 2024 16:18:36 +0300 Subject: [PATCH 5/6] subscription_list: Show @-mention marker when that applies Fixes: #747 --- lib/model/unreads.dart | 22 ++++++++++ lib/widgets/subscription_list.dart | 17 ++++++-- test/model/unreads_test.dart | 46 ++++++++++++++++++++ test/widgets/subscription_list_test.dart | 54 ++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 3 deletions(-) diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index 3f3ad7f9a7..e7c52ba6db 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -223,6 +223,28 @@ class Unreads extends ChangeNotifier { } } + Set get channelsWithUnreadMentions { + final channels = {}; + for (var messageId in mentions) { + final streamId = _streamIdsAndTopicsByMessageId[messageId]?.streamId; + if (streamId != null) { + channels.add(streamId); + } + } + return channels; + } + + Set get channelsWithUnmutedMentions { + final channels = {}; + for (var messageId in mentions) { + final (:streamId, :topic) = _streamIdsAndTopicsByMessageId[messageId]!; + if (channelStore.isTopicVisible(streamId, topic)) { + channels.add(streamId); + } + } + return channels; + } + void reconcileMessages(List messages) { for (final message in messages) { if (message.flags.contains(MessageFlag.read)) continue; diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index 86a9caf4e4..23d94390b5 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -185,6 +185,8 @@ class _SubscriptionList extends StatelessWidget { @override Widget build(BuildContext context) { + final channelsWithMentions = unreadsModel!.channelsWithUnreadMentions; + final channelsWithUnmutedMentions = unreadsModel!.channelsWithUnmutedMentions; return SliverList.builder( itemCount: subscriptions.length, itemBuilder: (BuildContext context, int index) { @@ -192,9 +194,14 @@ class _SubscriptionList extends StatelessWidget { final unreadCount = unreadsModel!.countInChannel(subscription.streamId); final showMutedUnreadBadge = unreadCount == 0 && unreadsModel!.countInChannelNarrow(subscription.streamId) > 0; + final hasMentions = channelsWithMentions.contains(subscription.streamId); + final hasOnlyMutedMentions = !subscription.isMuted && hasMentions + && !channelsWithUnmutedMentions.contains(subscription.streamId); return SubscriptionItem(subscription: subscription, unreadCount: unreadCount, - showMutedUnreadBadge: showMutedUnreadBadge); + showMutedUnreadBadge: showMutedUnreadBadge, + hasMentions: hasMentions, + hasOnlyMutedMentions: hasOnlyMutedMentions); }); } } @@ -206,11 +213,15 @@ class SubscriptionItem extends StatelessWidget { required this.subscription, required this.unreadCount, required this.showMutedUnreadBadge, + required this.hasMentions, + required this.hasOnlyMutedMentions, }); final Subscription subscription; final int unreadCount; final bool showMutedUnreadBadge; + final bool hasMentions; + final bool hasOnlyMutedMentions; @override Widget build(BuildContext context) { @@ -258,7 +269,7 @@ class SubscriptionItem extends StatelessWidget { subscription.name)))), if (hasUnreads) ...[ const SizedBox(width: 12), - // TODO(#747) show @-mention indicator when it applies + if (hasMentions) AtMentionMarker(muted: !subscription.isMuted && hasOnlyMutedMentions), Opacity( opacity: opacity, child: UnreadCountBadge( @@ -267,7 +278,7 @@ class SubscriptionItem extends StatelessWidget { bold: true)), ] else if (showMutedUnreadBadge) ...[ const SizedBox(width: 12), - // TODO(#747) show @-mention indicator when it applies + if (hasMentions) const AtMentionMarker(muted: true), const MutedUnreadBadge(), ], const SizedBox(width: 16), diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index d46aa8138b..eb48c9feb9 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -326,6 +326,52 @@ void main() { }); } }); + + test('channelsWithUnreadMentions', () { + final stream1 = eg.stream(); + final stream2 = eg.stream(); + + prepare(); + fillWithMessages([ + eg.streamMessage(stream: stream1, flags: [MessageFlag.mentioned]), + eg.streamMessage(stream: stream1, flags: []), + eg.streamMessage(stream: stream2, flags: []), + ]); + + check(model.channelsWithUnreadMentions).single.equals(stream1.streamId); + }); + + test('channelsWithUnmutedMentions', () async { + final stream1 = eg.stream(); + final stream2 = eg.stream(); + final stream3 = eg.stream(); + final stream4 = eg.stream(); + final streams = [stream1, stream2, stream3, stream4]; + + prepare(); + + await channelStore.addStreams(streams); + await channelStore.addSubscriptions([ + eg.subscription(stream1), + eg.subscription(stream2), + eg.subscription(stream3, isMuted: true), + eg.subscription(stream4), + ]); + + await channelStore.addUserTopic(stream1, 'a normal', UserTopicVisibilityPolicy.none); + await channelStore.addUserTopic(stream2, 'b muted', UserTopicVisibilityPolicy.muted); + await channelStore.addUserTopic(stream3, 'c normal', UserTopicVisibilityPolicy.none); + await channelStore.addUserTopic(stream4, 'd normal no mentions', UserTopicVisibilityPolicy.none); + + fillWithMessages([ + eg.streamMessage(stream: stream1, flags: [MessageFlag.mentioned], topic: 'a normal'), + eg.streamMessage(stream: stream2, flags: [MessageFlag.mentioned], topic: 'b muted'), + eg.streamMessage(stream: stream3, flags: [MessageFlag.mentioned], topic: 'c normal'), + eg.streamMessage(stream: stream4, flags: [], topic: 'd normal no mentions'), + ]); + + check(model.channelsWithUnmutedMentions.single).equals(stream1.streamId); + }); }); group('DM messages', () { diff --git a/test/widgets/subscription_list_test.dart b/test/widgets/subscription_list_test.dart index 130b6c802a..27d0d81823 100644 --- a/test/widgets/subscription_list_test.dart +++ b/test/widgets/subscription_list_test.dart @@ -317,4 +317,58 @@ void main() { checkStreamNameWght(mutedStreamWithUnmutedUnreads.name, 400); checkStreamNameWght(mutedStreamWithNoUnmutedUnreads.name, 400); }); + + group('@-mention marker', () { + Iterable getAtMentionMarkers(WidgetTester tester) { + return tester.widgetList(find.byType(AtMentionMarker)); + } + + testWidgets('is shown when subscription has unread mentions', (tester) async { + final streamWithMentions = eg.stream(); + final streamWithNoMentions = eg.stream(); + + await setupStreamListPage(tester, + subscriptions: [ + eg.subscription(streamWithMentions), + eg.subscription(streamWithNoMentions), + ], + userTopics: [ + eg.userTopicItem(streamWithMentions, 'a', UserTopicVisibilityPolicy.none), + eg.userTopicItem(streamWithNoMentions, 'b', UserTopicVisibilityPolicy.none), + ], + unreadMsgs: eg.unreadMsgs( + mentions: [1], + channels: [ + UnreadChannelSnapshot(streamId: streamWithMentions.streamId, topic: 'a', unreadMessageIds: [1]), + UnreadChannelSnapshot(streamId: streamWithNoMentions.streamId, topic: 'b', unreadMessageIds: [2]), + ]), + ); + + check(getAtMentionMarkers(tester)).single; + }); + + testWidgets('is muted when subscription has only muted mentions', (tester) async { + final streamWithMentions = eg.stream(); + final streamWithOnlyMutedMentions = eg.stream(); + + await setupStreamListPage(tester, + subscriptions: [ + eg.subscription(streamWithMentions), + eg.subscription(streamWithOnlyMutedMentions, isMuted: true), + ], + userTopics: [ + eg.userTopicItem(streamWithMentions, 'a', UserTopicVisibilityPolicy.none), + eg.userTopicItem(streamWithOnlyMutedMentions, 'b', UserTopicVisibilityPolicy.none), + ], + unreadMsgs: eg.unreadMsgs( + mentions: [1, 2], + channels: [ + UnreadChannelSnapshot(streamId: streamWithMentions.streamId, topic: 'a', unreadMessageIds: [1]), + UnreadChannelSnapshot(streamId: streamWithOnlyMutedMentions.streamId, topic: 'b', unreadMessageIds: [2]), + ]), + ); + + check(getAtMentionMarkers(tester).map((e) => e.muted)).deepEquals([false, true]); + }); + }); } From a635d4fa588867467f33b8b8211f997aa8043233 Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Wed, 14 Aug 2024 16:47:05 +0300 Subject: [PATCH 6/6] subscription_list: Show unread count when hasOnlyMutedMentions --- lib/model/unreads.dart | 19 +++++++++++++++++ lib/widgets/subscription_list.dart | 17 +++++++++++++++- test/widgets/subscription_list_test.dart | 26 ++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index e7c52ba6db..c33aebf1e3 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -194,6 +194,25 @@ class Unreads extends ChangeNotifier { return c; } + /// The "broadest" unread count for this channel, + /// without doing any checking on visibility policy. + /// + /// This includes all topics that have regardless visibility policy, + /// even if the channel is muted. + /// + /// This is needed for one specific case, which is when the channel has + /// only muted unreads including a mention or more, in that case we show + /// total unread count including muted unreads. + int countAll(int streamId) { + final topics = streams[streamId]; + if (topics == null) return 0; + int c = 0; + for (final entry in topics.entries) { + c = c + entry.value.length; + } + return c; + } + int countInTopicNarrow(int streamId, String topic) { final topics = streams[streamId]; return topics?[topic]?.length ?? 0; diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index 23d94390b5..7ea90686bb 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -197,8 +197,11 @@ class _SubscriptionList extends StatelessWidget { final hasMentions = channelsWithMentions.contains(subscription.streamId); final hasOnlyMutedMentions = !subscription.isMuted && hasMentions && !channelsWithUnmutedMentions.contains(subscription.streamId); + final mutedUnreadCount = hasOnlyMutedMentions && unreadCount == 0 ? + unreadsModel!.countAll(subscription.streamId) : 0; return SubscriptionItem(subscription: subscription, unreadCount: unreadCount, + mutedUnreadCount: mutedUnreadCount, showMutedUnreadBadge: showMutedUnreadBadge, hasMentions: hasMentions, hasOnlyMutedMentions: hasOnlyMutedMentions); @@ -212,6 +215,7 @@ class SubscriptionItem extends StatelessWidget { super.key, required this.subscription, required this.unreadCount, + required this.mutedUnreadCount, required this.showMutedUnreadBadge, required this.hasMentions, required this.hasOnlyMutedMentions, @@ -219,6 +223,7 @@ class SubscriptionItem extends StatelessWidget { final Subscription subscription; final int unreadCount; + final int mutedUnreadCount; final bool showMutedUnreadBadge; final bool hasMentions; final bool hasOnlyMutedMentions; @@ -229,7 +234,8 @@ class SubscriptionItem extends StatelessWidget { final swatch = colorSwatchFor(context, subscription); final hasUnreads = (unreadCount > 0); - final opacity = subscription.isMuted ? 0.55 : 1.0; + const mutedOpacity = 0.55; + final opacity = subscription.isMuted ? mutedOpacity : 1.0; return Material( // TODO(design) check if this is the right variable color: designVariables.background, @@ -276,6 +282,15 @@ class SubscriptionItem extends StatelessWidget { count: unreadCount, backgroundColor: swatch, bold: true)), + ] else if (hasOnlyMutedMentions && !subscription.isMuted) ...[ + const SizedBox(width: 12), + const AtMentionMarker(muted: true), + Opacity( + opacity: mutedOpacity, + child: UnreadCountBadge( + count: mutedUnreadCount, + backgroundColor: swatch, + bold: true)), ] else if (showMutedUnreadBadge) ...[ const SizedBox(width: 12), if (hasMentions) const AtMentionMarker(muted: true), diff --git a/test/widgets/subscription_list_test.dart b/test/widgets/subscription_list_test.dart index 27d0d81823..fca9212ad8 100644 --- a/test/widgets/subscription_list_test.dart +++ b/test/widgets/subscription_list_test.dart @@ -190,6 +190,32 @@ void main() { check(find.byType(MutedUnreadBadge).evaluate().length).equals(0); }); + testWidgets('unread badge shows as faded when non-muted subscription has only muted mentions', (tester) async { + final stream = eg.stream(); + + await setupStreamListPage(tester, + subscriptions: [ + eg.subscription(stream), + ], + userTopics: [ + eg.userTopicItem(stream, 'a', UserTopicVisibilityPolicy.muted), + ], + unreadMsgs: eg.unreadMsgs( + mentions: [1, 2], + channels: [ + UnreadChannelSnapshot(streamId: stream.streamId, topic: 'a', unreadMessageIds: [1, 2]), + ]), + ); + + check(find.byType(AtMentionMarker).evaluate()).single; + check(tester.widget(find.descendant( + of: find.byType(UnreadCountBadge), + matching: find.byType(Text)))).data.equals('2'); + check(tester.widget(find.ancestor( + of: find.byType(UnreadCountBadge), + matching: find.byType(Opacity))).opacity).equals(0.55); + }); + testWidgets('muted unread badge shows when unreads are visible in channel but not inbox', (tester) async { final stream = eg.stream(); final unreadMsgs = eg.unreadMsgs(channels: [