diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index 341d75bd0e..24832480c0 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -249,7 +249,9 @@ abstract class _HeaderItem extends StatelessWidget { Color collapsedIconColor(BuildContext context); Color uncollapsedIconColor(BuildContext context); Color uncollapsedBackgroundColor(BuildContext context); - Color? unreadCountBadgeBackgroundColor(BuildContext context); + + /// A channel ID, if this represents a channel, else null. + int? get channelId; Future onCollapseButtonTap() async { if (!collapsed) { @@ -307,7 +309,7 @@ abstract class _HeaderItem extends StatelessWidget { if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge( - backgroundColor: unreadCountBadgeBackgroundColor(context), + channelIdForBackground: channelId, bold: true, count: count)), ]))); @@ -332,7 +334,7 @@ class _AllDmsHeaderItem extends _HeaderItem { @override Color uncollapsedIconColor(context) => DesignVariables.of(context).labelMenuButton; @override Color uncollapsedBackgroundColor(context) => DesignVariables.of(context).dmHeaderBg; - @override Color? unreadCountBadgeBackgroundColor(context) => null; + @override int? get channelId => null; @override Future onCollapseButtonTap() async { await super.onCollapseButtonTap(); @@ -429,7 +431,7 @@ class _DmItem extends StatelessWidget { const SizedBox(width: 12), if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign), Padding(padding: const EdgeInsetsDirectional.only(end: 16), - child: UnreadCountBadge(backgroundColor: null, + child: UnreadCountBadge(channelIdForBackground: null, count: count)), ])))); } @@ -462,8 +464,7 @@ class _StreamHeaderItem extends _HeaderItem with _LongPressable { colorSwatchFor(context, subscription).iconOnBarBackground; @override Color uncollapsedBackgroundColor(context) => colorSwatchFor(context, subscription).barBackground; - @override Color? unreadCountBadgeBackgroundColor(context) => - colorSwatchFor(context, subscription).unreadCountBadgeBackground; + @override int? get channelId => subscription.streamId; @override Future onCollapseButtonTap() async { await super.onCollapseButtonTap(); @@ -526,7 +527,6 @@ class _TopicItem extends StatelessWidget { :topic, :count, :hasMention, :lastUnreadId) = data; final store = PerAccountStoreWidget.of(context); - final subscription = store.subscriptions[streamId]!; final designVariables = DesignVariables.of(context); final visibilityIcon = iconDataForTopicVisibilityPolicy( @@ -566,7 +566,7 @@ class _TopicItem extends StatelessWidget { if (visibilityIcon != null) _IconMarker(icon: visibilityIcon), Padding(padding: const EdgeInsetsDirectional.only(end: 16), child: UnreadCountBadge( - backgroundColor: colorSwatchFor(context, subscription), + channelIdForBackground: streamId, count: count)), ])))); } diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index f9eb688a10..719568e73d 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -233,7 +233,7 @@ class RecentDmConversationsItem extends StatelessWidget { const SizedBox(width: 12), unreadCount > 0 ? Padding(padding: const EdgeInsetsDirectional.only(end: 16), - child: UnreadCountBadge(backgroundColor: null, + child: UnreadCountBadge(channelIdForBackground: null, count: unreadCount)) : const SizedBox(), ])))); diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index ad286cf71f..8337bda7a4 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -337,7 +337,7 @@ class SubscriptionItem extends StatelessWidget { opacity: opacity, child: UnreadCountBadge( count: unreadCount, - backgroundColor: swatch, + channelIdForBackground: subscription.streamId, bold: true)), ] else if (showMutedUnreadBadge) ...[ const SizedBox(width: 12), diff --git a/lib/widgets/unread_count_badge.dart b/lib/widgets/unread_count_badge.dart index 7d2cdc1f27..b14bd6972d 100644 --- a/lib/widgets/unread_count_badge.dart +++ b/lib/widgets/unread_count_badge.dart @@ -1,6 +1,6 @@ import 'package:flutter/widgets.dart'; -import 'channel_colors.dart'; +import 'store.dart'; import 'text.dart'; import 'theme.dart'; @@ -12,35 +12,42 @@ class UnreadCountBadge extends StatelessWidget { const UnreadCountBadge({ super.key, required this.count, - required this.backgroundColor, + required this.channelIdForBackground, this.bold = false, }); final int count; final bool bold; - /// The badge's background color. + /// An optional [Subscription.streamId], for a channel-colorized background. /// - /// Pass a [ChannelColorSwatch] if this badge represents messages in one - /// specific stream. The appropriate color from the swatch will be used. + /// Useful when this badge represents messages in one specific channel. /// /// If null, the default neutral background will be used. - final Color? backgroundColor; + final int? channelIdForBackground; @override Widget build(BuildContext context) { + final store = PerAccountStoreWidget.of(context); final designVariables = DesignVariables.of(context); - final effectiveBackgroundColor = switch (backgroundColor) { - ChannelColorSwatch(unreadCountBadgeBackground: var color) => color, - Color() => backgroundColor, - null => designVariables.bgCounterUnread, - }; + final Color textColor; + final Color backgroundColor; + if (channelIdForBackground != null) { + textColor = designVariables.unreadCountBadgeTextForChannel; + + final subscription = store.subscriptions[channelIdForBackground!]; + final swatch = colorSwatchFor(context, subscription); + backgroundColor = swatch.unreadCountBadgeBackground; + } else { + textColor = designVariables.labelCounterUnread; + backgroundColor = designVariables.bgCounterUnread; + } return DecoratedBox( decoration: BoxDecoration( borderRadius: BorderRadius.circular(3), - color: effectiveBackgroundColor, + color: backgroundColor, ), child: Padding( padding: const EdgeInsets.fromLTRB(4, 0, 4, 1), @@ -49,9 +56,7 @@ class UnreadCountBadge extends StatelessWidget { fontSize: 16, height: (18 / 16), fontFeatures: const [FontFeature.enable('smcp')], // small caps - color: backgroundColor is ChannelColorSwatch - ? designVariables.unreadCountBadgeTextForChannel - : designVariables.labelCounterUnread, + color: textColor, ).merge(weightVariableTextStyle(context, wght: bold ? 600 : null)), count.toString()))); diff --git a/test/widgets/checks.dart b/test/widgets/checks.dart index 856b3ebebe..90b640bd8e 100644 --- a/test/widgets/checks.dart +++ b/test/widgets/checks.dart @@ -95,7 +95,7 @@ extension PerAccountStoreWidgetChecks on Subject { extension UnreadCountBadgeChecks on Subject { Subject get count => has((b) => b.count, 'count'); Subject get bold => has((b) => b.bold, 'bold'); - Subject get backgroundColor => has((b) => b.backgroundColor, 'backgroundColor'); + Subject get channelIdForBackground => has((b) => b.channelIdForBackground, 'channelIdForBackground'); } extension UnicodeEmojiWidgetChecks on Subject { diff --git a/test/widgets/inbox_test.dart b/test/widgets/inbox_test.dart index fe96b427ab..63aad6292c 100644 --- a/test/widgets/inbox_test.dart +++ b/test/widgets/inbox_test.dart @@ -9,6 +9,7 @@ import 'package:zulip/widgets/color.dart'; import 'package:zulip/widgets/home.dart'; import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/channel_colors.dart'; +import 'package:zulip/widgets/theme.dart'; import 'package:zulip/widgets/unread_count_badge.dart'; import '../example_data.dart' as eg; @@ -206,6 +207,28 @@ void main() { await setupVarious(tester); }); + testWidgets('UnreadCountBadge text color for a channel', (tester) async { + // Regression test for a bug where + // DesignVariables.labelCounterUnread was used for the text instead of + // DesignVariables.unreadCountBadgeTextForChannel. + final channel = eg.stream(); + final subscription = eg.subscription(channel); + await setupPage(tester, + streams: [channel], + subscriptions: [subscription], + unreadMessages: generateStreamMessages(stream: channel, count: 1, flags: [])); + + final text = tester.widget( + find.descendant( + of: find.byWidget(findRowByLabel(tester, channel.name)!), + matching: find.descendant( + of: find.byType(UnreadCountBadge), + matching: find.text('1')))); + + final expectedTextColor = DesignVariables.light.unreadCountBadgeTextForChannel; + check(text).style.isNotNull().color.isNotNull().isSameColorAs(expectedTextColor); + }); + // TODO test that tapping a conversation row opens the message list // for the conversation diff --git a/test/widgets/subscription_list_test.dart b/test/widgets/subscription_list_test.dart index d0eb50624d..49034f2166 100644 --- a/test/widgets/subscription_list_test.dart +++ b/test/widgets/subscription_list_test.dart @@ -2,6 +2,7 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter_checks/flutter_checks.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:legacy_checks/legacy_checks.dart'; import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/widgets/color.dart'; @@ -273,8 +274,12 @@ void main() { check(getItemCount()).equals(1); check(tester.widget(find.byIcon(iconDataForStream(stream))).color) .isNotNull().isSameColorAs(swatch.iconOnPlainBackground); - check(tester.widget(find.byType(UnreadCountBadge)).backgroundColor) - .isNotNull().isSameColorAs(swatch); + + final unreadCountBadgeRenderBox = tester.renderObject(find.byType(UnreadCountBadge)); + check(unreadCountBadgeRenderBox).legacyMatcher( + // `paints` isn't a [Matcher] so we wrap it with `equals`; + // awkward but it works + equals(paints..rrect(color: swatch.unreadCountBadgeBackground))); }); testWidgets('muted streams are displayed as faded', (tester) async { diff --git a/test/widgets/unread_count_badge_test.dart b/test/widgets/unread_count_badge_test.dart index 5c07f9524a..739146050a 100644 --- a/test/widgets/unread_count_badge_test.dart +++ b/test/widgets/unread_count_badge_test.dart @@ -3,32 +3,44 @@ import 'package:flutter_checks/flutter_checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/model/model.dart'; import 'package:zulip/widgets/channel_colors.dart'; import 'package:zulip/widgets/unread_count_badge.dart'; +import '../example_data.dart' as eg; import '../model/binding.dart'; +import '../model/test_store.dart'; import 'test_app.dart'; void main() { TestZulipBinding.ensureInitialized(); group('UnreadCountBadge', () { - testWidgets('smoke test; no crash', (tester) async { + Future prepare(WidgetTester tester, { + required Widget child, + Subscription? subscription, + }) async { addTearDown(testBinding.reset); - await tester.pumpWidget(const TestZulipApp( - child: UnreadCountBadge(count: 1, backgroundColor: null))); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + if (subscription != null) { + await store.addStream(ZulipStream.fromSubscription(subscription)); + await store.addSubscription(subscription); + } + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + child: child)); + await tester.pump(); await tester.pump(); + } + + testWidgets('smoke test; no crash', (tester) async { + await prepare(tester, + child: const UnreadCountBadge(count: 1, channelIdForBackground: null)); tester.widget(find.text("1")); }); group('background', () { - Future prepare(WidgetTester tester, Color? backgroundColor) async { - addTearDown(testBinding.reset); - await tester.pumpWidget(TestZulipApp( - child: UnreadCountBadge(count: 1, backgroundColor: backgroundColor))); - await tester.pump(); - } - Color? findBackgroundColor(WidgetTester tester) { final widget = tester.widget(find.byType(DecoratedBox)); final decoration = widget.decoration as BoxDecoration; @@ -36,19 +48,20 @@ void main() { } testWidgets('default color', (tester) async { - await prepare(tester, null); + await prepare(tester, + child: UnreadCountBadge(count: 1, channelIdForBackground: null)); check(findBackgroundColor(tester)).isNotNull().isSameColorAs(const Color(0x26666699)); }); - testWidgets('specified color', (tester) async { - await prepare(tester, Colors.pink); - check(findBackgroundColor(tester)).isNotNull().isSameColorAs(Colors.pink); - }); - testWidgets('stream color', (tester) async { - final swatch = ChannelColorSwatch.light(0xff76ce90); - await prepare(tester, swatch); - check(findBackgroundColor(tester)).isNotNull().isSameColorAs(swatch.unreadCountBadgeBackground); + final subscription = eg.subscription(eg.stream(), color: 0xff76ce90); + await prepare(tester, + subscription: subscription, + child: UnreadCountBadge( + count: 1, + channelIdForBackground: subscription.streamId)); + check(findBackgroundColor(tester)).isNotNull() + .isSameColorAs(ChannelColorSwatch.light(0xff76ce90).unreadCountBadgeBackground); }); }); });