diff --git a/lib/model/message_list.dart b/lib/model/message_list.dart index 613b1a712e..947690c426 100644 --- a/lib/model/message_list.dart +++ b/lib/model/message_list.dart @@ -691,6 +691,9 @@ class MessageListView with ChangeNotifier, _MessageSequence { return true; case MentionsNarrow(): + // If changing this, consider whether [Unreads.countInMentionsNarrow] + // should be changed correspondingly, so the message-list view matches + // the unread-count badge. case StarredMessagesNarrow(): case KeywordSearchNarrow(): if (message.conversation case DmConversation(:final allRecipientIds)) { diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index 543cde28b6..b975cb2293 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -160,9 +160,7 @@ class Unreads extends PerAccountStoreBase with ChangeNotifier { // TODO(#370): maintain this count incrementally, rather than recomputing from scratch int countInCombinedFeedNarrow() { int c = 0; - for (final messageIds in dms.values) { - c = c + messageIds.length; - } + c += countInDms(); for (final MapEntry(key: streamId, value: topics) in streams.entries) { for (final MapEntry(key: topic, value: messageIds) in topics.entries) { if (channelStore.isTopicVisible(streamId, topic)) { @@ -222,7 +220,30 @@ class Unreads extends PerAccountStoreBase with ChangeNotifier { int countInDmNarrow(DmNarrow narrow) => dms[narrow]?.length ?? 0; - int countInMentionsNarrow() => mentions.length; + /// The unread count for the mentions narrow. + /// + /// This excludes DM messages in conversations that are considered muted, + /// by [UserStore.shouldMuteDmConversation]. + // If changing which messages to exclude, consider whether the @-mentions + // view should change its "is-message-visible" code correspondingly, + // so the unread-count badge matches what you see in that view. + // TODO: deduplicate "is-message-visible" code + // between [Unreads] and [MessageListView]? + // TODO(#370): maintain this count incrementally, rather than recomputing from scratch + int countInMentionsNarrow() { + int c = 0; + for (final messageId in mentions) { + final narrow = locatorMap[messageId]; + if (narrow == null) continue; // TODO(log) + switch (narrow) { + case DmNarrow(): + if (channelStore.shouldMuteDmConversation(narrow)) continue; + case TopicNarrow(): + } + c++; + } + return c; + } // TODO: Implement unreads handling. int countInStarredMessagesNarrow() => 0; @@ -230,6 +251,19 @@ class Unreads extends PerAccountStoreBase with ChangeNotifier { // TODO: Implement unreads handling? int countInKeywordSearchNarrow() => 0; + /// The aggregated unread count for DM conversations. + /// + /// This excludes conversations that are considered muted, + /// by [UserStore.shouldMuteDmConversation]. + int countInDms() { + int c = 0; + for (final MapEntry(key: narrow, value: messageIds) in dms.entries) { + if (channelStore.shouldMuteDmConversation(narrow)) continue; + c += messageIds.length; + } + return c; + } + int countInNarrow(Narrow narrow) { switch (narrow) { case CombinedFeedNarrow(): diff --git a/lib/widgets/home.dart b/lib/widgets/home.dart index f4a7650f6f..3c0a58d6ee 100644 --- a/lib/widgets/home.dart +++ b/lib/widgets/home.dart @@ -24,6 +24,7 @@ import 'store.dart'; import 'subscription_list.dart'; import 'text.dart'; import 'theme.dart'; +import 'unread_count_badge.dart'; import 'user.dart'; enum _HomePageTab { @@ -506,6 +507,8 @@ abstract class _MenuButton extends StatelessWidget { color: selected ? designVariables.iconSelected : designVariables.icon); } + Widget? buildTrailing(BuildContext context) => null; + void onPressed(BuildContext context); void _handlePress(BuildContext context) { @@ -546,6 +549,8 @@ abstract class _MenuButton extends StatelessWidget { ~WidgetState.pressed: selected ? borderSideSelected : null, })); + final trailing = buildTrailing(context); + return AnimatedScaleOnTap( duration: const Duration(milliseconds: 100), scaleEnd: 0.95, @@ -562,6 +567,7 @@ abstract class _MenuButton extends StatelessWidget { overflow: TextOverflow.ellipsis, style: const TextStyle(fontSize: 19, height: 26 / 19) .merge(weightVariableTextStyle(context, wght: selected ? 600 : 400)))), + ?trailing, ])))); } } @@ -612,6 +618,18 @@ class _InboxButton extends _NavigationBarMenuButton { return zulipLocalizations.inboxPageTitle; } + @override + Widget? buildTrailing(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + final unreadCount = store.unreads.countInCombinedFeedNarrow(); + if (unreadCount == 0) return null; + return UnreadCountBadge( + style: UnreadCountBadgeStyle.mainMenu, + count: unreadCount, + channelIdForBackground: null, + ); + } + @override _HomePageTab get navigationTarget => _HomePageTab.inbox; } @@ -627,6 +645,18 @@ class _MentionsButton extends _MenuButton { return zulipLocalizations.mentionsPageTitle; } + @override + Widget? buildTrailing(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + final unreadCount = store.unreads.countInMentionsNarrow(); + if (unreadCount == 0) return null; + return UnreadCountBadge( + style: UnreadCountBadgeStyle.mainMenu, + count: unreadCount, + channelIdForBackground: null, + ); + } + @override void onPressed(BuildContext context) { Navigator.of(context).push(MessageListPage.buildRoute( @@ -696,6 +726,18 @@ class _DirectMessagesButton extends _NavigationBarMenuButton { return zulipLocalizations.recentDmConversationsPageTitle; } + @override + Widget? buildTrailing(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + final unreadCount = store.unreads.countInDms(); + if (unreadCount == 0) return null; + return UnreadCountBadge( + style: UnreadCountBadgeStyle.mainMenu, + count: unreadCount, + channelIdForBackground: null, + ); + } + @override _HomePageTab get navigationTarget => _HomePageTab.directMessages; } diff --git a/lib/widgets/unread_count_badge.dart b/lib/widgets/unread_count_badge.dart index 828d724dc9..3208a28fb7 100644 --- a/lib/widgets/unread_count_badge.dart +++ b/lib/widgets/unread_count_badge.dart @@ -10,23 +10,20 @@ import 'theme.dart'; /// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=2037-186671&m=dev /// It looks like that component was created for the main menu, /// then adapted for various other contexts, like the Inbox page. +/// See [UnreadCountBadgeStyle]. /// -/// Currently this widget supports only those other contexts (not the main menu) -/// and only the component's "kind=unread" variant (not "kind=quantity"). -/// For example, the "Channels" page and the topic-list page: -/// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6205-26001&m=dev -/// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6823-37113&m=dev -/// (We use this for the topic-list page even though the Figma makes it a bit -/// more compact there…the inconsistency seems worse and might be accidental.) -// TODO support the main-menu context, update dartdoc +/// Currently this widget supports only the component's "kind=unread" variant, +/// not "kind=quantity". // TODO support the "kind=quantity" variant, update dartdoc class UnreadCountBadge extends StatelessWidget { const UnreadCountBadge({ super.key, + this.style = UnreadCountBadgeStyle.other, required this.count, required this.channelIdForBackground, }); + final UnreadCountBadgeStyle style; final int count; /// An optional [Subscription.streamId], for a channel-colorized background. @@ -55,23 +52,52 @@ class UnreadCountBadge extends StatelessWidget { backgroundColor = designVariables.bgCounterUnread; } + final padding = switch (style) { + UnreadCountBadgeStyle.mainMenu => + const EdgeInsets.symmetric(horizontal: 5, vertical: 4), + UnreadCountBadgeStyle.other => + const EdgeInsets.symmetric(horizontal: 5, vertical: 3), + }; + + final double wght = switch (style) { + UnreadCountBadgeStyle.mainMenu => 600, + UnreadCountBadgeStyle.other => 500, + }; + return DecoratedBox( decoration: BoxDecoration( borderRadius: BorderRadius.circular(5), color: backgroundColor, ), child: Padding( - padding: const EdgeInsets.symmetric(horizontal: 5, vertical: 3), + padding: padding, child: Text( style: TextStyle( fontSize: 16, height: (16 / 16), color: textColor, - ).merge(weightVariableTextStyle(context, wght: 500)), + ).merge(weightVariableTextStyle(context, wght: wght)), count.toString()))); } } +enum UnreadCountBadgeStyle { + /// The style to use in the main menu. + /// + /// Figma: + /// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=2037-185126&m=dev + mainMenu, + + /// The style to use in other contexts besides the main menu. + /// + /// Other contexts include the "Channels" page and the topic-list page: + /// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6205-26001&m=dev + /// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6823-37113&m=dev + /// (We use this for the topic-list page even though the Figma makes it a bit + /// more compact there…the inconsistency seems worse and might be accidental.) + other, +} + class MutedUnreadBadge extends StatelessWidget { const MutedUnreadBadge({super.key}); diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index 089ef315cb..91740475d5 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -191,6 +191,7 @@ void main() { await store.addSubscription(eg.subscription(stream2)); await store.addSubscription(eg.subscription(stream3, isMuted: true)); await store.setUserTopic(stream1, 'a', UserTopicVisibilityPolicy.muted); + await store.setMutedUsers([eg.thirdUser.userId]); fillWithMessages([ eg.streamMessage(stream: stream1, topic: 'a', flags: []), eg.streamMessage(stream: stream1, topic: 'b', flags: []), @@ -200,7 +201,7 @@ void main() { eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: []), eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser], flags: []), ]); - check(model.countInCombinedFeedNarrow()).equals(5); + check(model.countInCombinedFeedNarrow()).equals(4); }); test('countInChannel/Narrow', () async { @@ -249,16 +250,71 @@ void main() { check(model.countInDmNarrow(narrow)).equals(5); }); - test('countInMentionsNarrow', () async { - final stream = eg.stream(); - prepare(); - await store.addStream(stream); - fillWithMessages([ - eg.streamMessage(stream: stream, flags: []), - eg.streamMessage(stream: stream, flags: [MessageFlag.mentioned]), - eg.streamMessage(stream: stream, flags: [MessageFlag.wildcardMentioned]), - ]); - check(model.countInMentionsNarrow()).equals(2); + group('countInMentionsNarrow', () { + test('smoke', () async { + final stream = eg.stream(); + prepare(); + await store.addUser(eg.otherUser); + await store.addStream(stream); + fillWithMessages([ + eg.streamMessage(stream: stream, flags: []), + eg.streamMessage(stream: stream, flags: [MessageFlag.mentioned]), + eg.streamMessage(stream: stream, flags: [MessageFlag.wildcardMentioned]), + eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: [MessageFlag.mentioned]), + eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: [MessageFlag.wildcardMentioned]), + ]); + check(model.countInMentionsNarrow()).equals(4); + }); + + test('excludes unreads in muted DM conversations', () async { + // Regression test for https://github.com/zulip/zulip-flutter/issues/2026 + prepare(); + await store.addUser(eg.otherUser); + await store.setMutedUsers([eg.otherUser.userId]); + + fillWithMessages([ + eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: [MessageFlag.mentioned]), + ]); + check(model.countInMentionsNarrow()).equals(0); + }); + + test('not affected by DM sender being unknown', () async { + prepare(); + final unknownUser = eg.user(); + check(store.getUser(unknownUser.userId)).isNull(); + + fillWithMessages([ + eg.dmMessage(from: unknownUser, to: [eg.selfUser], flags: [MessageFlag.mentioned]), + ]); + check(model.countInMentionsNarrow()).equals(1); + }); + + test('not affected by channel being unknown/unsubscribed/muted, nor topic being muted', () async { + prepare(); + + final channel1 = eg.stream(); // unknown + + final channel2 = eg.stream(); // unsubscribed + await store.addStream(channel2); + check(store.subscriptions[channel2.streamId]).isNull(); + + final channel3 = eg.stream(); // muted + await store.addStream(channel3); + await store.addSubscription(eg.subscription(channel3, isMuted: true)); + + final channel4 = eg.stream(); // unmuted, containing muted topic + await store.addStream(channel4); + await store.addSubscription(eg.subscription(channel4, isMuted: false)); + await store.setUserTopic(channel4, 'a', UserTopicVisibilityPolicy.muted); + + fillWithMessages([ + eg.streamMessage(stream: channel1, flags: [MessageFlag.mentioned]), + eg.streamMessage(stream: channel2, flags: [MessageFlag.mentioned]), + eg.streamMessage(stream: channel3, flags: [MessageFlag.mentioned]), + eg.streamMessage(stream: channel4, topic: 'a', flags: [MessageFlag.mentioned]), + ]); + check(model.countInMentionsNarrow()).equals(4); + }); }); test('countInStarredMessagesNarrow', () async { @@ -271,6 +327,20 @@ void main() { ]); check(model.countInStarredMessagesNarrow()).equals(0); }); + + test('countInDms', () async { + prepare(); + await store.setMutedUsers([eg.thirdUser.userId]); + fillWithMessages([ + // No one is muted: don't exclude + eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: []), + // Everyone is muted: exclude + eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser], flags: []), + // One is muted, one isn't: don't exclude + eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser, eg.otherUser], flags: []), + ]); + check(model.countInDms()).equals(2); + }); }); group('isUnread', () {