From 3697cdc1e8d6c0062e7fdccfdfe5c01ad0082757 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 19 Nov 2025 14:47:24 -0800 Subject: [PATCH 1/4] unreads [nfc]: Factor out countInAllDms, for public use and as a helper --- lib/model/unreads.dart | 12 +++++++++--- test/model/unreads_test.dart | 10 ++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index 543cde28b6..e5183b12d3 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 += countInAllDms(); 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)) { @@ -230,6 +228,14 @@ class Unreads extends PerAccountStoreBase with ChangeNotifier { // TODO: Implement unreads handling? int countInKeywordSearchNarrow() => 0; + int countInAllDms() { + int c = 0; + for (final messageIds in dms.values) { + c += messageIds.length; + } + return c; + } + int countInNarrow(Narrow narrow) { switch (narrow) { case CombinedFeedNarrow(): diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index 089ef315cb..f0f18ae97d 100644 --- a/test/model/unreads_test.dart +++ b/test/model/unreads_test.dart @@ -271,6 +271,16 @@ void main() { ]); check(model.countInStarredMessagesNarrow()).equals(0); }); + + test('countInAllDms', () async { + prepare(); + fillWithMessages([ + eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: []), + eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser], flags: []), + eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser, eg.otherUser], flags: []), + ]); + check(model.countInCombinedFeedNarrow()).equals(3); + }); }); group('isUnread', () { From d78ce57887d409b38bc602507d6bd17c99663c6b Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 19 Nov 2025 14:58:04 -0800 Subject: [PATCH 2/4] unreads: Exclude muted DM conversations in combined-feed and all-dm counts --- lib/model/unreads.dart | 3 ++- test/model/unreads_test.dart | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index e5183b12d3..a145bf8a4c 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -230,7 +230,8 @@ class Unreads extends PerAccountStoreBase with ChangeNotifier { int countInAllDms() { int c = 0; - for (final messageIds in dms.values) { + for (final MapEntry(key: narrow, value: messageIds) in dms.entries) { + if (channelStore.shouldMuteDmConversation(narrow)) continue; c += messageIds.length; } return c; diff --git a/test/model/unreads_test.dart b/test/model/unreads_test.dart index f0f18ae97d..9ab5feef84 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: []), @@ -198,9 +199,10 @@ void main() { eg.streamMessage(stream: stream2, topic: 'c', flags: []), eg.streamMessage(stream: stream3, topic: 'd', flags: []), eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: []), + // Exclude because user is muted eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser], flags: []), ]); - check(model.countInCombinedFeedNarrow()).equals(5); + check(model.countInCombinedFeedNarrow()).equals(4); }); test('countInChannel/Narrow', () async { @@ -274,12 +276,16 @@ void main() { test('countInAllDms', () 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.countInCombinedFeedNarrow()).equals(3); + check(model.countInCombinedFeedNarrow()).equals(2); }); }); From c744d3651998e69950085014cc598326fcb48f36 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 19 Nov 2025 12:18:21 -0800 Subject: [PATCH 3/4] home: Show unread counts in main menu Fixes-partly: #1088 --- lib/widgets/home.dart | 42 ++++++++++++++++++++++++++ lib/widgets/unread_count_badge.dart | 46 ++++++++++++++++++++++------- 2 files changed, 78 insertions(+), 10 deletions(-) diff --git a/lib/widgets/home.dart b/lib/widgets/home.dart index 62c09c0857..7c3fedbf9e 100644 --- a/lib/widgets/home.dart +++ b/lib/widgets/home.dart @@ -22,6 +22,7 @@ import 'store.dart'; import 'subscription_list.dart'; import 'text.dart'; import 'theme.dart'; +import 'unread_count_badge.dart'; import 'user.dart'; enum _HomePageTab { @@ -351,6 +352,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) { @@ -391,6 +394,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, @@ -407,6 +412,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, ])))); } } @@ -457,6 +463,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; } @@ -472,6 +490,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( @@ -541,6 +571,18 @@ class _DirectMessagesButton extends _NavigationBarMenuButton { return zulipLocalizations.recentDmConversationsPageTitle; } + @override + Widget? buildTrailing(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + final unreadCount = store.unreads.countInAllDms(); + 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}); From 1909c88cb566febff12e4a4024c2fa42feb36dbc Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 19 Nov 2025 12:19:34 -0800 Subject: [PATCH 4/4] home: Tweak main-menu buttons to follow Figma The buttons were 48px instead of 44px tall, and the label's line height was 26px instead of 23px. --- lib/widgets/home.dart | 68 ++++++++++++++++++++++--------------- test/widgets/home_test.dart | 16 +++++++++ 2 files changed, 56 insertions(+), 28 deletions(-) diff --git a/lib/widgets/home.dart b/lib/widgets/home.dart index 7c3fedbf9e..d09df5c7f9 100644 --- a/lib/widgets/home.dart +++ b/lib/widgets/home.dart @@ -331,8 +331,13 @@ void _showMainMenu(BuildContext context, { }); } -abstract class _MenuButton extends StatelessWidget { - const _MenuButton(); +/// A button in the main menu. +/// +/// See Figma: +/// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=2037-243759&m=dev +@visibleForTesting +abstract class MenuButton extends StatelessWidget { + const MenuButton({super.key}); String label(ZulipLocalizations zulipLocalizations); @@ -369,11 +374,20 @@ abstract class _MenuButton extends StatelessWidget { final designVariables = DesignVariables.of(context); final zulipLocalizations = ZulipLocalizations.of(context); + // Make [TextButton] set 44 instead of 48 for the height. + final visualDensity = VisualDensity(vertical: -1); + // A value that [TextButton] adds to some of its layout parameters; + // we can cancel out those adjustments by subtracting it. + final densityVerticalAdjustment = visualDensity.baseSizeAdjustment.dy; + final borderSideSelected = BorderSide(width: 1, strokeAlign: BorderSide.strokeAlignOutside, color: designVariables.borderMenuButtonSelected); final buttonStyle = TextButton.styleFrom( - padding: const EdgeInsets.symmetric(vertical: 9, horizontal: 8), + // Make the button 44px instead of 48px tall, to match the Figma. + visualDensity: visualDensity, + padding: EdgeInsets.symmetric( + vertical: 10 - densityVerticalAdjustment, horizontal: 8), foregroundColor: designVariables.labelMenuButton, // This has a default behavior of affecting the background color of the // button for states including "hovered", "focused" and "pressed". @@ -399,26 +413,24 @@ abstract class _MenuButton extends StatelessWidget { return AnimatedScaleOnTap( duration: const Duration(milliseconds: 100), scaleEnd: 0.95, - child: ConstrainedBox( - constraints: const BoxConstraints(minHeight: 44), - child: TextButton( - onPressed: () => _handlePress(context), - style: buttonStyle, - child: Row(spacing: 8, children: [ - SizedBox.square(dimension: _iconSize, - child: buildLeading(context)), - Expanded(child: Text(label(zulipLocalizations), - // TODO(design): determine if we prefer to wrap - overflow: TextOverflow.ellipsis, - style: const TextStyle(fontSize: 19, height: 26 / 19) - .merge(weightVariableTextStyle(context, wght: selected ? 600 : 400)))), - ?trailing, - ])))); + child: TextButton( + onPressed: () => _handlePress(context), + style: buttonStyle, + child: Row(spacing: 8, children: [ + SizedBox.square(dimension: _iconSize, + child: buildLeading(context)), + Expanded(child: Text(label(zulipLocalizations), + // TODO(design): determine if we prefer to wrap + overflow: TextOverflow.ellipsis, + style: const TextStyle(fontSize: 19, height: 23 / 19) + .merge(weightVariableTextStyle(context, wght: selected ? 600 : 400)))), + ?trailing, + ]))); } } /// A menu button controlling the selected [_HomePageTab] on the bottom nav bar. -abstract class _NavigationBarMenuButton extends _MenuButton { +abstract class _NavigationBarMenuButton extends MenuButton { const _NavigationBarMenuButton({required this.tabNotifier}); final ValueNotifier<_HomePageTab> tabNotifier; @@ -434,7 +446,7 @@ abstract class _NavigationBarMenuButton extends _MenuButton { } } -class _SearchButton extends _MenuButton { +class _SearchButton extends MenuButton { const _SearchButton(); @override @@ -479,7 +491,7 @@ class _InboxButton extends _NavigationBarMenuButton { _HomePageTab get navigationTarget => _HomePageTab.inbox; } -class _MentionsButton extends _MenuButton { +class _MentionsButton extends MenuButton { const _MentionsButton(); @override @@ -509,7 +521,7 @@ class _MentionsButton extends _MenuButton { } } -class _StarredMessagesButton extends _MenuButton { +class _StarredMessagesButton extends MenuButton { const _StarredMessagesButton(); @override @@ -527,7 +539,7 @@ class _StarredMessagesButton extends _MenuButton { } } -class _CombinedFeedButton extends _MenuButton { +class _CombinedFeedButton extends MenuButton { const _CombinedFeedButton(); @override @@ -587,7 +599,7 @@ class _DirectMessagesButton extends _NavigationBarMenuButton { _HomePageTab get navigationTarget => _HomePageTab.directMessages; } -class _MyProfileButton extends _MenuButton { +class _MyProfileButton extends MenuButton { const _MyProfileButton(); @override @@ -598,7 +610,7 @@ class _MyProfileButton extends _MenuButton { final store = PerAccountStoreWidget.of(context); return Avatar( userId: store.selfUserId, - size: _MenuButton._iconSize, + size: MenuButton._iconSize, borderRadius: 4, showPresence: false, ); @@ -617,7 +629,7 @@ class _MyProfileButton extends _MenuButton { } } -class _SwitchAccountButton extends _MenuButton { +class _SwitchAccountButton extends MenuButton { const _SwitchAccountButton(); @override @@ -634,7 +646,7 @@ class _SwitchAccountButton extends _MenuButton { } } -class _SettingsButton extends _MenuButton { +class _SettingsButton extends MenuButton { const _SettingsButton(); @override @@ -651,7 +663,7 @@ class _SettingsButton extends _MenuButton { } } -class _AboutZulipButton extends _MenuButton { +class _AboutZulipButton extends MenuButton { const _AboutZulipButton(); @override diff --git a/test/widgets/home_test.dart b/test/widgets/home_test.dart index 1ee0a0ae8e..1e10fb5bcc 100644 --- a/test/widgets/home_test.dart +++ b/test/widgets/home_test.dart @@ -224,6 +224,22 @@ void main () { .isSameColorAs(designVariables.icon); } + testWidgets('buttons are 44px tall', (tester) async { + await prepare(tester); + + await tapOpenMenuAndAwait(tester); + checkIconSelected(tester, inboxMenuIconFinder); + checkIconNotSelected(tester, channelsMenuIconFinder); + + final inboxElement = tester.element( + find.ancestor(of: inboxMenuIconFinder, matching: find.bySubtype())); + check((inboxElement.renderObject as RenderBox).size).height.equals(44); + + final channelsElement = tester.element( + find.ancestor(of: inboxMenuIconFinder, matching: find.bySubtype())); + check((channelsElement.renderObject as RenderBox).size).height.equals(44); + }); + testWidgets('navigation states reflect on navigation bar menu buttons', (tester) async { await prepare(tester);