From 36f0cb71823c602675ae54008e76bc60e95a6a7e Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 29 Apr 2025 21:17:34 -0400 Subject: [PATCH 1/3] theme [nfc]: Move bgMessageRegular to DesignVariables --- lib/widgets/content.dart | 3 ++- lib/widgets/message_list.dart | 14 +++----------- lib/widgets/theme.dart | 7 +++++++ test/widgets/message_list_test.dart | 7 ++++--- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 40b510305d..62801ab867 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -26,6 +26,7 @@ import 'poll.dart'; import 'scrolling.dart'; import 'store.dart'; import 'text.dart'; +import 'theme.dart'; /// A central place for styles for Zulip content (rendered Zulip Markdown). /// @@ -988,7 +989,7 @@ class WebsitePreview extends StatelessWidget { // TODO(#488) use different color for non-message contexts // TODO(#647) use different color for highlighted messages // TODO(#681) use different color for DM messages - color: MessageListTheme.of(context).bgMessageRegular, + color: DesignVariables.of(context).bgMessageRegular, child: ClipRect( child: ConstrainedBox( constraints: BoxConstraints(maxHeight: 80), diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index dcd063a62a..44ae56fb1e 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -28,7 +28,6 @@ import 'theme.dart'; /// Message-list styles that differ between light and dark themes. class MessageListTheme extends ThemeExtension { static final light = MessageListTheme._( - bgMessageRegular: const HSLColor.fromAHSL(1, 0, 0, 1).toColor(), dmRecipientHeaderBg: const HSLColor.fromAHSL(1, 46, 0.35, 0.93).toColor(), labelTime: const HSLColor.fromAHSL(0.49, 0, 0, 0).toColor(), senderBotIcon: const HSLColor.fromAHSL(1, 180, 0.08, 0.65).toColor(), @@ -46,7 +45,6 @@ class MessageListTheme extends ThemeExtension { ); static final dark = MessageListTheme._( - bgMessageRegular: const HSLColor.fromAHSL(1, 0, 0, 0.11).toColor(), dmRecipientHeaderBg: const HSLColor.fromAHSL(1, 46, 0.15, 0.2).toColor(), labelTime: const HSLColor.fromAHSL(0.5, 0, 0, 1).toColor(), senderBotIcon: const HSLColor.fromAHSL(1, 180, 0.05, 0.5).toColor(), @@ -63,7 +61,6 @@ class MessageListTheme extends ThemeExtension { ); MessageListTheme._({ - required this.bgMessageRegular, required this.dmRecipientHeaderBg, required this.labelTime, required this.senderBotIcon, @@ -82,7 +79,6 @@ class MessageListTheme extends ThemeExtension { return extension!; } - final Color bgMessageRegular; final Color dmRecipientHeaderBg; final Color labelTime; final Color senderBotIcon; @@ -92,7 +88,6 @@ class MessageListTheme extends ThemeExtension { @override MessageListTheme copyWith({ - Color? bgMessageRegular, Color? dmRecipientHeaderBg, Color? labelTime, Color? senderBotIcon, @@ -101,7 +96,6 @@ class MessageListTheme extends ThemeExtension { Color? unreadMarkerGap, }) { return MessageListTheme._( - bgMessageRegular: bgMessageRegular ?? this.bgMessageRegular, dmRecipientHeaderBg: dmRecipientHeaderBg ?? this.dmRecipientHeaderBg, labelTime: labelTime ?? this.labelTime, senderBotIcon: senderBotIcon ?? this.senderBotIcon, @@ -117,7 +111,6 @@ class MessageListTheme extends ThemeExtension { return this; } return MessageListTheme._( - bgMessageRegular: Color.lerp(bgMessageRegular, other.bgMessageRegular, t)!, dmRecipientHeaderBg: Color.lerp(dmRecipientHeaderBg, other.dmRecipientHeaderBg, t)!, labelTime: Color.lerp(labelTime, other.labelTime, t)!, senderBotIcon: Color.lerp(senderBotIcon, other.senderBotIcon, t)!, @@ -981,13 +974,12 @@ class DateSeparator extends StatelessWidget { // to align with the vertically centered divider lines. const textBottomPadding = 2.0; - final messageListTheme = MessageListTheme.of(context); final designVariables = DesignVariables.of(context); final line = BorderSide(width: 0, color: designVariables.foreground); // TODO(#681) use different color for DM messages - return ColoredBox(color: messageListTheme.bgMessageRegular, + return ColoredBox(color: designVariables.bgMessageRegular, child: Padding( padding: const EdgeInsets.symmetric(vertical: 8, horizontal: 2), child: Row(children: [ @@ -1026,11 +1018,11 @@ class MessageItem extends StatelessWidget { @override Widget build(BuildContext context) { - final messageListTheme = MessageListTheme.of(context); + final designVariables = DesignVariables.of(context); final item = this.item; Widget child = ColoredBox( - color: messageListTheme.bgMessageRegular, + color: designVariables.bgMessageRegular, child: Column(children: [ switch (item) { MessageListMessageItem() => MessageWithPossibleSender(item: item), diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index 276e308b2b..492f82c88a 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -137,6 +137,7 @@ class DesignVariables extends ThemeExtension { bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.15), bgMenuButtonActive: Colors.black.withValues(alpha: 0.05), bgMenuButtonSelected: Colors.white, + bgMessageRegular: const HSLColor.fromAHSL(1, 0, 0, 1).toColor(), bgTopBar: const Color(0xfff5f5f5), borderBar: Colors.black.withValues(alpha: 0.2), borderMenuButtonSelected: Colors.black.withValues(alpha: 0.2), @@ -197,6 +198,7 @@ class DesignVariables extends ThemeExtension { bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.37), bgMenuButtonActive: Colors.black.withValues(alpha: 0.2), bgMenuButtonSelected: Colors.black.withValues(alpha: 0.25), + bgMessageRegular: const HSLColor.fromAHSL(1, 0, 0, 0.11).toColor(), bgTopBar: const Color(0xff242424), borderBar: const Color(0xffffffff).withValues(alpha: 0.1), borderMenuButtonSelected: Colors.white.withValues(alpha: 0.1), @@ -265,6 +267,7 @@ class DesignVariables extends ThemeExtension { required this.bgCounterUnread, required this.bgMenuButtonActive, required this.bgMenuButtonSelected, + required this.bgMessageRegular, required this.bgTopBar, required this.borderBar, required this.borderMenuButtonSelected, @@ -334,6 +337,7 @@ class DesignVariables extends ThemeExtension { final Color bgCounterUnread; final Color bgMenuButtonActive; final Color bgMenuButtonSelected; + final Color bgMessageRegular; final Color bgTopBar; final Color borderBar; final Color borderMenuButtonSelected; @@ -398,6 +402,7 @@ class DesignVariables extends ThemeExtension { Color? bgCounterUnread, Color? bgMenuButtonActive, Color? bgMenuButtonSelected, + Color? bgMessageRegular, Color? bgTopBar, Color? borderBar, Color? borderMenuButtonSelected, @@ -457,6 +462,7 @@ class DesignVariables extends ThemeExtension { bgCounterUnread: bgCounterUnread ?? this.bgCounterUnread, bgMenuButtonActive: bgMenuButtonActive ?? this.bgMenuButtonActive, bgMenuButtonSelected: bgMenuButtonSelected ?? this.bgMenuButtonSelected, + bgMessageRegular: bgMessageRegular ?? this.bgMessageRegular, bgTopBar: bgTopBar ?? this.bgTopBar, borderBar: borderBar ?? this.borderBar, borderMenuButtonSelected: borderMenuButtonSelected ?? this.borderMenuButtonSelected, @@ -523,6 +529,7 @@ class DesignVariables extends ThemeExtension { bgCounterUnread: Color.lerp(bgCounterUnread, other.bgCounterUnread, t)!, bgMenuButtonActive: Color.lerp(bgMenuButtonActive, other.bgMenuButtonActive, t)!, bgMenuButtonSelected: Color.lerp(bgMenuButtonSelected, other.bgMenuButtonSelected, t)!, + bgMessageRegular: Color.lerp(bgMessageRegular, other.bgMessageRegular, t)!, bgTopBar: Color.lerp(bgTopBar, other.bgTopBar, t)!, borderBar: Color.lerp(borderBar, other.borderBar, t)!, borderMenuButtonSelected: Color.lerp(borderMenuButtonSelected, other.borderMenuButtonSelected, t)!, diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 88c4cdb64b..9b614b735b 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -27,6 +27,7 @@ import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/store.dart'; import 'package:zulip/widgets/channel_colors.dart'; +import 'package:zulip/widgets/theme.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -282,17 +283,17 @@ void main() { return widget.color; } - check(backgroundColor()).isSameColorAs(MessageListTheme.light.bgMessageRegular); + check(backgroundColor()).isSameColorAs(DesignVariables.light.bgMessageRegular); tester.platformDispatcher.platformBrightnessTestValue = Brightness.dark; await tester.pump(); await tester.pump(kThemeAnimationDuration * 0.4); - final expectedLerped = MessageListTheme.light.lerp(MessageListTheme.dark, 0.4); + final expectedLerped = DesignVariables.light.lerp(DesignVariables.dark, 0.4); check(backgroundColor()).isSameColorAs(expectedLerped.bgMessageRegular); await tester.pump(kThemeAnimationDuration * 0.6); - check(backgroundColor()).isSameColorAs(MessageListTheme.dark.bgMessageRegular); + check(backgroundColor()).isSameColorAs(DesignVariables.dark.bgMessageRegular); }); group('fetch initial batch of messages', () { From d68d5a7e2cd87d89588389cfda566846c84310a9 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 29 Apr 2025 21:16:09 -0400 Subject: [PATCH 2/3] topics: Add topic list page For the topic-list page app bar, we leave out the icon "chevron_down.svg" since it's related to a new design (#1039) we haven't implemented yet. This also why "TOPICS" is not aligned to the middle part of the app bar on the message-list page. We also leave out the new topic button and topic filtering, which are out-of-scope for #1158. The topic-list implementation is quite similar to parts of inbox page and message-list page. Therefore, we structure the code to make it easy to maintain in the future. Especially, this helps us (a) when we're changing one, apply the same change to the other, where appropriate, and (b) later reconcile the differences they do have and then refactor to unify them. Figma design: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6819-35869&m=dev The "TOPICS" icon on message-list page in a topic narrow is a UX change from the design. See CZO discussion: https://chat.zulip.org/#narrow/channel/48-mobile/topic/Flutter.20beta.3A.20missing.20topic.20list/near/2177505 --- assets/l10n/app_en.arb | 4 + lib/generated/l10n/zulip_localizations.dart | 6 + .../l10n/zulip_localizations_ar.dart | 3 + .../l10n/zulip_localizations_de.dart | 3 + .../l10n/zulip_localizations_en.dart | 3 + .../l10n/zulip_localizations_ja.dart | 3 + .../l10n/zulip_localizations_nb.dart | 3 + .../l10n/zulip_localizations_pl.dart | 3 + .../l10n/zulip_localizations_ru.dart | 3 + .../l10n/zulip_localizations_sk.dart | 3 + .../l10n/zulip_localizations_uk.dart | 3 + lib/widgets/message_list.dart | 55 ++- lib/widgets/topic_list.dart | 347 ++++++++++++++++++ test/widgets/message_list_test.dart | 40 ++ test/widgets/topic_list_test.dart | 330 +++++++++++++++++ 15 files changed, 801 insertions(+), 8 deletions(-) create mode 100644 lib/widgets/topic_list.dart create mode 100644 test/widgets/topic_list_test.dart diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index d11bf43eda..1c0ec3d7e8 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -769,6 +769,10 @@ "@mainMenuMyProfile": { "description": "Label for main-menu button leading to the user's own profile." }, + "topicsButtonLabel": "TOPICS", + "@topicsButtonLabel": { + "description": "Label for message list button leading to topic-list page. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.)" + }, "channelFeedButtonTooltip": "Channel feed", "@channelFeedButtonTooltip": { "description": "Tooltip for button to navigate to a given channel's feed" diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index ecb0eee16a..566ff617ad 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -1151,6 +1151,12 @@ abstract class ZulipLocalizations { /// **'My profile'** String get mainMenuMyProfile; + /// Label for message list button leading to topic-list page. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.) + /// + /// In en, this message translates to: + /// **'TOPICS'** + String get topicsButtonLabel; + /// Tooltip for button to navigate to a given channel's feed /// /// In en, this message translates to: diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index 98dd9a7af6..d1025b7151 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -629,6 +629,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get mainMenuMyProfile => 'My profile'; + @override + String get topicsButtonLabel => 'TOPICS'; + @override String get channelFeedButtonTooltip => 'Channel feed'; diff --git a/lib/generated/l10n/zulip_localizations_de.dart b/lib/generated/l10n/zulip_localizations_de.dart index 08d09bb3c4..eee1309e1b 100644 --- a/lib/generated/l10n/zulip_localizations_de.dart +++ b/lib/generated/l10n/zulip_localizations_de.dart @@ -629,6 +629,9 @@ class ZulipLocalizationsDe extends ZulipLocalizations { @override String get mainMenuMyProfile => 'My profile'; + @override + String get topicsButtonLabel => 'TOPICS'; + @override String get channelFeedButtonTooltip => 'Channel feed'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index 105162429b..fbdcea3935 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -629,6 +629,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get mainMenuMyProfile => 'My profile'; + @override + String get topicsButtonLabel => 'TOPICS'; + @override String get channelFeedButtonTooltip => 'Channel feed'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index 74a2d4bedb..a9a2e40dce 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -629,6 +629,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get mainMenuMyProfile => 'My profile'; + @override + String get topicsButtonLabel => 'TOPICS'; + @override String get channelFeedButtonTooltip => 'Channel feed'; diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index 02913278b8..afe3c9794a 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -629,6 +629,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get mainMenuMyProfile => 'My profile'; + @override + String get topicsButtonLabel => 'TOPICS'; + @override String get channelFeedButtonTooltip => 'Channel feed'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 26b4b7e306..7653f8d31d 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -638,6 +638,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get mainMenuMyProfile => 'Mój profil'; + @override + String get topicsButtonLabel => 'TOPICS'; + @override String get channelFeedButtonTooltip => 'Strumień kanału'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 5d8899290d..6ea916c359 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -642,6 +642,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get mainMenuMyProfile => 'Мой профиль'; + @override + String get topicsButtonLabel => 'TOPICS'; + @override String get channelFeedButtonTooltip => 'Лента канала'; diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index 3ff534eca5..5430a885c4 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -631,6 +631,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get mainMenuMyProfile => 'Môj profil'; + @override + String get topicsButtonLabel => 'TOPICS'; + @override String get channelFeedButtonTooltip => 'Channel feed'; diff --git a/lib/generated/l10n/zulip_localizations_uk.dart b/lib/generated/l10n/zulip_localizations_uk.dart index 94fee8825a..db213893db 100644 --- a/lib/generated/l10n/zulip_localizations_uk.dart +++ b/lib/generated/l10n/zulip_localizations_uk.dart @@ -641,6 +641,9 @@ class ZulipLocalizationsUk extends ZulipLocalizations { @override String get mainMenuMyProfile => 'Мій профіль'; + @override + String get topicsButtonLabel => 'TOPICS'; + @override String get channelFeedButtonTooltip => 'Стрічка каналу'; diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 44ae56fb1e..0003ae5ed7 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -24,6 +24,7 @@ import 'sticky_header.dart'; import 'store.dart'; import 'text.dart'; import 'theme.dart'; +import 'topic_list.dart'; /// Message-list styles that differ between light and dark themes. class MessageListTheme extends ThemeExtension { @@ -220,14 +221,23 @@ class _MessageListPageState extends State implements MessageLis removeAppBarBottomBorder = true; } - List? actions; - if (narrow case TopicNarrow(:final streamId)) { - (actions ??= []).add(IconButton( - icon: const Icon(ZulipIcons.message_feed), - tooltip: zulipLocalizations.channelFeedButtonTooltip, - onPressed: () => Navigator.push(context, - MessageListPage.buildRoute(context: context, - narrow: ChannelNarrow(streamId))))); + List actions = []; + switch (narrow) { + case CombinedFeedNarrow(): + case MentionsNarrow(): + case StarredMessagesNarrow(): + case DmNarrow(): + break; + case ChannelNarrow(:final streamId): + actions.add(_TopicListButton(streamId: streamId)); + case TopicNarrow(:final streamId): + actions.add(IconButton( + icon: const Icon(ZulipIcons.message_feed), + tooltip: zulipLocalizations.channelFeedButtonTooltip, + onPressed: () => Navigator.push(context, + MessageListPage.buildRoute(context: context, + narrow: ChannelNarrow(streamId))))); + actions.add(_TopicListButton(streamId: streamId)); } // Insert a PageRoot here, to provide a context that can be used for @@ -277,6 +287,35 @@ class _MessageListPageState extends State implements MessageLis } } +class _TopicListButton extends StatelessWidget { + const _TopicListButton({required this.streamId}); + + final int streamId; + + @override + Widget build(BuildContext context) { + final zulipLocalizations = ZulipLocalizations.of(context); + final designVariables = DesignVariables.of(context); + return GestureDetector( + onTap: () { + Navigator.of(context).push(TopicListPage.buildRoute( + context: context, streamId: streamId)); + }, + behavior: HitTestBehavior.opaque, + child: Padding( + padding: EdgeInsetsDirectional.fromSTEB(12, 8, 12, 8), + child: Center(child: Text(zulipLocalizations.topicsButtonLabel, + style: TextStyle( + color: designVariables.icon, + fontSize: 18, + height: 19 / 18, + // This is equivalent to css `all-small-caps`, see: + // https://developer.mozilla.org/en-US/docs/Web/CSS/font-variant-caps#all-small-caps + fontFeatures: const [FontFeature.enable('c2sc'), FontFeature.enable('smcp')], + ).merge(weightVariableTextStyle(context, wght: 600)))))); + } +} + class MessageListAppBarTitle extends StatelessWidget { const MessageListAppBarTitle({ super.key, diff --git a/lib/widgets/topic_list.dart b/lib/widgets/topic_list.dart new file mode 100644 index 0000000000..61e16df6ec --- /dev/null +++ b/lib/widgets/topic_list.dart @@ -0,0 +1,347 @@ +import 'package:flutter/material.dart'; + +import '../api/model/model.dart'; +import '../api/route/channels.dart'; +import '../generated/l10n/zulip_localizations.dart'; +import '../model/narrow.dart'; +import '../model/unreads.dart'; +import 'action_sheet.dart'; +import 'app_bar.dart'; +import 'color.dart'; +import 'icons.dart'; +import 'message_list.dart'; +import 'page.dart'; +import 'store.dart'; +import 'text.dart'; +import 'theme.dart'; + +class TopicListPage extends StatelessWidget { + const TopicListPage({super.key, required this.streamId}); + + final int streamId; + + static AccountRoute buildRoute({ + required BuildContext context, + required int streamId, + }) { + return MaterialAccountWidgetRoute( + context: context, + page: TopicListPage(streamId: streamId)); + } + + @override + Widget build(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); + final appBarBackgroundColor = colorSwatchFor( + context, store.subscriptions[streamId]).barBackground; + + return PageRoot(child: Scaffold( + appBar: ZulipAppBar( + backgroundColor: appBarBackgroundColor, + buildTitle: (willCenterTitle) => + _TopicListAppBarTitle(streamId: streamId, willCenterTitle: willCenterTitle), + actions: [ + IconButton( + icon: const Icon(ZulipIcons.message_feed), + tooltip: zulipLocalizations.channelFeedButtonTooltip, + onPressed: () => Navigator.push(context, + MessageListPage.buildRoute(context: context, + narrow: ChannelNarrow(streamId)))), + ]), + body: _TopicList(streamId: streamId))); + } +} + +// This is adapted from [MessageListAppBarTitle]. +class _TopicListAppBarTitle extends StatelessWidget { + const _TopicListAppBarTitle({ + required this.streamId, + required this.willCenterTitle, + }); + + final int streamId; + final bool willCenterTitle; + + Widget _buildStreamRow(BuildContext context) { + // TODO(#1039) implement a consistent app bar design here + final zulipLocalizations = ZulipLocalizations.of(context); + final designVariables = DesignVariables.of(context); + final store = PerAccountStoreWidget.of(context); + final stream = store.streams[streamId]; + final channelIconColor = colorSwatchFor(context, + store.subscriptions[streamId]).iconOnBarBackground; + + // A null [Icon.icon] makes a blank space. + final icon = stream != null ? iconDataForStream(stream) : null; + return Row( + mainAxisSize: MainAxisSize.min, + // TODO(design): The vertical alignment of the stream privacy icon is a bit ad hoc. + // For screenshots of some experiments, see: + // https://github.com/zulip/zulip-flutter/pull/219#discussion_r1281024746 + crossAxisAlignment: CrossAxisAlignment.center, + children: [ + Padding(padding: const EdgeInsets.symmetric(horizontal: 5, vertical: 6), + child: Icon(size: 18, icon, color: channelIconColor)), + Flexible(child: Text( + stream?.name ?? zulipLocalizations.unknownChannelName, + style: TextStyle( + fontSize: 20, + height: 30 / 20, + color: designVariables.title, + ).merge(weightVariableTextStyle(context, wght: 600)))), + ]); + } + + @override + Widget build(BuildContext context) { + final alignment = willCenterTitle + ? Alignment.center + : AlignmentDirectional.centerStart; + return SizedBox( + width: double.infinity, + child: GestureDetector( + behavior: HitTestBehavior.translucent, + onLongPress: () { + showChannelActionSheet(context, channelId: streamId); + }, + child: Align(alignment: alignment, + child: _buildStreamRow(context)))); + } +} + +class _TopicList extends StatefulWidget { + const _TopicList({required this.streamId}); + + final int streamId; + + @override + State<_TopicList> createState() => _TopicListState(); +} + +class _TopicListState extends State<_TopicList> with PerAccountStoreAwareStateMixin { + Unreads? unreadsModel; + // TODO(#1499): store the results on [ChannelStore], and keep them + // up-to-date by handling events + List? lastFetchedTopics; + + @override + void onNewStore() { + unreadsModel?.removeListener(_modelChanged); + final store = PerAccountStoreWidget.of(context); + unreadsModel = store.unreads..addListener(_modelChanged); + _fetchTopics(); + } + + @override + void dispose() { + unreadsModel?.removeListener(_modelChanged); + super.dispose(); + } + + void _modelChanged() { + setState(() { + // The actual state lives in `unreadsModel`. + }); + } + + void _fetchTopics() async { + // Do nothing when the fetch fails; the topic-list will stay on + // the loading screen, until the user navigates away and back. + // TODO(design) show a nice error message on screen when this fails + final store = PerAccountStoreWidget.of(context); + final result = await getStreamTopics(store.connection, + streamId: widget.streamId, + allowEmptyTopicName: true); + if (!mounted) return; + setState(() { + lastFetchedTopics = result.topics; + }); + } + + @override + Widget build(BuildContext context) { + if (lastFetchedTopics == null) { + return const Center(child: CircularProgressIndicator()); + } + + // TODO(design) handle the rare case when `lastFetchedTopics` is empty + + // This is adapted from parts of the build method on [_InboxPageState]. + final topicItems = <_TopicItemData>[]; + for (final GetStreamTopicsEntry(:maxId, name: topic) in lastFetchedTopics!) { + final unreadMessageIds = + unreadsModel!.streams[widget.streamId]?[topic] ?? []; + final countInTopic = unreadMessageIds.length; + final hasMention = unreadMessageIds.any((messageId) => + unreadsModel!.mentions.contains(messageId)); + topicItems.add(_TopicItemData( + topic: topic, + unreadCount: countInTopic, + hasMention: hasMention, + // `lastFetchedTopics.maxId` can become outdated when a new message + // arrives or when there are message moves, until we re-fetch. + // TODO(#1499): track changes to this + maxId: maxId, + )); + } + topicItems.sort((a, b) { + final aMaxId = a.maxId; + final bMaxId = b.maxId; + return bMaxId.compareTo(aMaxId); + }); + + return SafeArea( + // Don't pad the bottom here; we want the list content to do that. + bottom: false, + child: ListView.builder( + itemCount: topicItems.length, + itemBuilder: (context, index) => + _TopicItem(streamId: widget.streamId, data: topicItems[index])), + ); + } +} + +class _TopicItemData { + final TopicName topic; + final int unreadCount; + final bool hasMention; + final int maxId; + + const _TopicItemData({ + required this.topic, + required this.unreadCount, + required this.hasMention, + required this.maxId, + }); +} + +// This is adapted from `_TopicItem` in lib/widgets/inbox.dart. +// TODO(#1527) see if we can reuse this in redesign +class _TopicItem extends StatelessWidget { + const _TopicItem({required this.streamId, required this.data}); + + final int streamId; + final _TopicItemData data; + + @override + Widget build(BuildContext context) { + final _TopicItemData( + :topic, :unreadCount, :hasMention, :maxId) = data; + + final store = PerAccountStoreWidget.of(context); + final designVariables = DesignVariables.of(context); + + final visibilityPolicy = store.topicVisibilityPolicy(streamId, topic); + final double opacity; + switch (visibilityPolicy) { + case UserTopicVisibilityPolicy.muted: + opacity = 0.5; + case UserTopicVisibilityPolicy.none: + case UserTopicVisibilityPolicy.unmuted: + case UserTopicVisibilityPolicy.followed: + opacity = 1; + case UserTopicVisibilityPolicy.unknown: + assert(false); + opacity = 1; + } + + final visibilityIcon = iconDataForTopicVisibilityPolicy(visibilityPolicy); + + return Material( + color: designVariables.bgMessageRegular, + child: InkWell( + onTap: () { + final narrow = TopicNarrow(streamId, topic); + Navigator.push(context, + MessageListPage.buildRoute(context: context, narrow: narrow)); + }, + onLongPress: () => showTopicActionSheet(context, + channelId: streamId, + topic: topic, + someMessageIdInTopic: maxId), + splashFactory: NoSplash.splashFactory, + child: Padding(padding: EdgeInsetsDirectional.fromSTEB(6, 8, 12, 8), + child: Row( + spacing: 8, + // In the Figma design, the text and icons on the topic item row + // are aligned to the start on the cross axis + // (i.e., `align-items: flex-start`). The icons are padded down + // 2px relative to the start, to visibly sit on the baseline. + // To account for scaled text, we align everything on the row + // to [CrossAxisAlignment.center] instead ([Row]'s default), + // like we do for the topic items on the inbox page. + // TODO(#1528): align to baseline (and therefore to first line of + // topic name), but with adjustment for icons + // CZO discussion: + // https://chat.zulip.org/#narrow/channel/243-mobile-team/topic/topic.20list.20item.20alignment/near/2173252 + children: [ + // A null [Icon.icon] makes a blank space. + _IconMarker(icon: topic.isResolved ? ZulipIcons.check : null), + Expanded(child: Opacity( + opacity: opacity, + child: Text( + style: TextStyle( + fontSize: 17, + height: 20 / 17, + fontStyle: topic.displayName == null ? FontStyle.italic : null, + color: designVariables.textMessage, + ), + maxLines: 3, + overflow: TextOverflow.ellipsis, + topic.unresolve().displayName ?? store.realmEmptyTopicDisplayName))), + Opacity(opacity: opacity, child: Row( + spacing: 4, + children: [ + if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign), + if (visibilityIcon != null) _IconMarker(icon: visibilityIcon), + if (unreadCount > 0) _UnreadCountBadge(count: unreadCount), + ])), + ])))); + } +} + +class _IconMarker extends StatelessWidget { + const _IconMarker({required this.icon}); + + final IconData? icon; + + @override + Widget build(BuildContext context) { + final designVariables = DesignVariables.of(context); + final textScaler = MediaQuery.textScalerOf(context); + // Since we align the icons to [CrossAxisAlignment.center], the top padding + // from the Figma design is omitted. + return Icon(icon, + size: textScaler.clamp(maxScaleFactor: 1.5).scale(16), + color: designVariables.textMessage.withFadedAlpha(0.4)); + } +} + +// This is adapted from [UnreadCountBadge]. +// TODO(#1406) see if we can reuse this in redesign +// TODO(#1527) see if we can reuse this in redesign +class _UnreadCountBadge extends StatelessWidget { + const _UnreadCountBadge({required this.count}); + + final int count; + + @override + Widget build(BuildContext context) { + final designVariables = DesignVariables.of(context); + + return DecoratedBox( + decoration: BoxDecoration( + borderRadius: BorderRadius.circular(5), + color: designVariables.bgCounterUnread, + ), + child: Padding( + padding: const EdgeInsets.symmetric(horizontal: 4, vertical: 2), + child: Text(count.toString(), + style: TextStyle( + fontSize: 15, + height: 16 / 15, + color: designVariables.labelCounterUnread, + ).merge(weightVariableTextStyle(context, wght: 500))))); + } +} diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 9b614b735b..606a01f420 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -11,6 +11,7 @@ import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/model/narrow.dart'; +import 'package:zulip/api/route/channels.dart'; import 'package:zulip/api/route/messages.dart'; import 'package:zulip/model/actions.dart'; import 'package:zulip/model/localizations.dart'; @@ -28,6 +29,7 @@ import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/store.dart'; import 'package:zulip/widgets/channel_colors.dart'; import 'package:zulip/widgets/theme.dart'; +import 'package:zulip/widgets/topic_list.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -229,6 +231,25 @@ void main() { .equals(ChannelNarrow(channel.streamId)); }); + testWidgets('has topic-list action for topic narrows', (tester) async { + final channel = eg.stream(name: 'channel foo'); + await setupMessageListPage(tester, + narrow: eg.topicNarrow(channel.streamId, 'topic foo'), + streams: [channel], + messages: [eg.streamMessage(stream: channel, topic: 'topic foo')]); + + connection.prepare(json: GetStreamTopicsResult(topics: [ + eg.getStreamTopicsEntry(name: 'topic foo'), + ]).toJson()); + await tester.tap(find.text('TOPICS')); + await tester.pump(); // tap the button + await tester.pump(Duration.zero); // wait for request + check(find.descendant( + of: find.byType(TopicListPage), + matching: find.text('channel foo')), + ).findsOne(); + }); + testWidgets('show topic visibility policy for topic narrows', (tester) async { final channel = eg.stream(); const topic = 'topic'; @@ -244,6 +265,25 @@ void main() { of: find.byType(MessageListAppBarTitle), matching: find.byIcon(ZulipIcons.mute))).findsOne(); }); + + testWidgets('has topic-list action for channel narrows', (tester) async { + final channel = eg.stream(name: 'channel foo'); + await setupMessageListPage(tester, + narrow: ChannelNarrow(channel.streamId), + streams: [channel], + messages: [eg.streamMessage(stream: channel, topic: 'topic foo')]); + + connection.prepare(json: GetStreamTopicsResult(topics: [ + eg.getStreamTopicsEntry(name: 'topic foo'), + ]).toJson()); + await tester.tap(find.text('TOPICS')); + await tester.pump(); // tap the button + await tester.pump(Duration.zero); // wait for request + check(find.descendant( + of: find.byType(TopicListPage), + matching: find.text('channel foo')), + ).findsOne(); + }); }); group('presents message content appropriately', () { diff --git a/test/widgets/topic_list_test.dart b/test/widgets/topic_list_test.dart new file mode 100644 index 0000000000..cf76ff3917 --- /dev/null +++ b/test/widgets/topic_list_test.dart @@ -0,0 +1,330 @@ +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:http/http.dart' as http; +import 'package:zulip/api/model/initial_snapshot.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/route/channels.dart'; +import 'package:zulip/model/narrow.dart'; +import 'package:zulip/model/store.dart'; +import 'package:zulip/widgets/app_bar.dart'; +import 'package:zulip/widgets/icons.dart'; +import 'package:zulip/widgets/message_list.dart'; +import 'package:zulip/widgets/topic_list.dart'; + +import '../api/fake_api.dart'; +import '../example_data.dart' as eg; +import '../model/binding.dart'; +import '../model/test_store.dart'; +import '../stdlib_checks.dart'; +import 'test_app.dart'; + +void main() { + TestZulipBinding.ensureInitialized(); + + late PerAccountStore store; + late FakeApiConnection connection; + + Future prepare(WidgetTester tester, { + ZulipStream? channel, + List? topics, + List userTopics = const [], + List? messages, + }) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + connection = store.connection as FakeApiConnection; + + await store.addUser(eg.selfUser); + channel ??= eg.stream(); + await store.addStream(channel); + await store.addSubscription(eg.subscription(channel)); + for (final userTopic in userTopics) { + await store.addUserTopic( + channel, userTopic.topicName.apiName, userTopic.visibilityPolicy); + } + topics ??= [eg.getStreamTopicsEntry()]; + messages ??= [eg.streamMessage(stream: channel, topic: topics.first.name.apiName)]; + await store.addMessages(messages); + + connection.prepare(json: GetStreamTopicsResult(topics: topics).toJson()); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + child: TopicListPage(streamId: channel.streamId))); + await tester.pump(); + await tester.pump(Duration.zero); + check(connection.takeRequests()).single.isA() + ..method.equals('GET') + ..url.path.equals('/api/v1/users/me/${channel.streamId}/topics') + ..url.queryParameters.deepEquals({'allow_empty_topic_name': 'true'}); + } + + group('app bar', () { + testWidgets('unknown channel name', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + final channel = eg.stream(); + + (store.connection as FakeApiConnection).prepare( + json: GetStreamTopicsResult(topics: []).toJson()); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + child: TopicListPage(streamId: channel.streamId))); + await tester.pump(); + await tester.pump(Duration.zero); + check(find.widgetWithText(ZulipAppBar, '(unknown channel)')).findsOne(); + }); + + testWidgets('navigate to channel feed', (tester) async { + final channel = eg.stream(name: 'channel foo'); + await prepare(tester, channel: channel); + + connection.prepare(json: eg.newestGetMessagesResult( + foundOldest: true, messages: [eg.streamMessage(stream: channel)]).toJson()); + await tester.tap(find.byIcon(ZulipIcons.message_feed)); + await tester.pump(); + await tester.pump(Duration.zero); + check(find.descendant( + of: find.byType(MessageListPage), + matching: find.text('channel foo')), + ).findsOne(); + }); + + testWidgets('show channel action sheet', (tester) async { + final channel = eg.stream(name: 'channel foo'); + await prepare(tester, channel: channel, + messages: [eg.streamMessage(stream: channel)]); + + await tester.longPress(find.text('channel foo')); + await tester.pump(Duration(milliseconds: 100)); // bottom-sheet animation + check(find.text('Mark channel as read')).findsOne(); + }); + }); + + testWidgets('show loading indicator', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + final channel = eg.stream(); + + (store.connection as FakeApiConnection).prepare( + json: GetStreamTopicsResult(topics: []).toJson(), + delay: Duration(seconds: 1), + ); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + child: TopicListPage(streamId: channel.streamId))); + await tester.pump(); + check(find.byType(CircularProgressIndicator)).findsOne(); + + await tester.pump(Duration(seconds: 1)); + check(find.byType(CircularProgressIndicator)).findsNothing(); + }); + + testWidgets('fetch again when navigating away and back', (tester) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + final connection = store.connection as FakeApiConnection; + final channel = eg.stream(); + + // Start from a message list page in a channel narrow. + connection.prepare(json: eg.newestGetMessagesResult( + foundOldest: true, messages: []).toJson()); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + child: MessageListPage(initNarrow: ChannelNarrow(channel.streamId)))); + await tester.pump(); + + // Tap "TOPICS" button navigating to the topic-list page… + connection.prepare(json: GetStreamTopicsResult( + topics: [eg.getStreamTopicsEntry(name: 'topic A')]).toJson()); + await tester.tap(find.text('TOPICS')); + await tester.pump(); + await tester.pump(Duration.zero); + check(find.text('topic A')).findsOne(); + + // … go back to the message list page… + await tester.pageBack(); + await tester.pump(); + + // … then back to the topic-list page, expecting to fetch again. + connection.prepare(json: GetStreamTopicsResult( + topics: [eg.getStreamTopicsEntry(name: 'topic B')]).toJson()); + await tester.tap(find.text('TOPICS')); + await tester.pump(); + await tester.pump(Duration.zero); + check(find.text('topic A')).findsNothing(); + check(find.text('topic B')).findsOne(); + }); + + Finder topicItemFinder = find.descendant( + of: find.byType(ListView), + matching: find.byType(Material)); + + Finder findInTopicItemAt(int index, Finder finder) => find.descendant( + of: topicItemFinder.at(index), + matching: finder); + + testWidgets('show topic action sheet', (tester) async { + final channel = eg.stream(); + await prepare(tester, channel: channel, + topics: [eg.getStreamTopicsEntry(name: 'topic foo')]); + await tester.longPress(topicItemFinder); + await tester.pump(Duration(milliseconds: 150)); // bottom-sheet animation + + connection.prepare(json: {}); + await tester.tap(find.text('Mute topic')); + await tester.pump(); + await tester.pump(Duration.zero); + check(connection.takeRequests()).single.isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/user_topics') + ..bodyFields.deepEquals({ + 'stream_id': channel.streamId.toString(), + 'topic': 'topic foo', + 'visibility_policy': UserTopicVisibilityPolicy.muted.apiValue.toString(), + }); + }); + + testWidgets('sort topics by maxId', (tester) async { + await prepare(tester, topics: [ + eg.getStreamTopicsEntry(name: 'A', maxId: 3), + eg.getStreamTopicsEntry(name: 'B', maxId: 2), + eg.getStreamTopicsEntry(name: 'C', maxId: 4), + ]); + + check(findInTopicItemAt(0, find.text('C'))).findsOne(); + check(findInTopicItemAt(1, find.text('A'))).findsOne(); + check(findInTopicItemAt(2, find.text('B'))).findsOne(); + }); + + testWidgets('resolved and unresolved topics', (tester) async { + final resolvedTopic = TopicName('resolved').resolve(); + final unresolvedTopic = TopicName('unresolved'); + await prepare(tester, topics: [ + eg.getStreamTopicsEntry(maxId: 2, name: resolvedTopic.apiName), + eg.getStreamTopicsEntry(maxId: 1, name: unresolvedTopic.apiName), + ]); + + assert(resolvedTopic.displayName == '✔ resolved', resolvedTopic.displayName); + check(findInTopicItemAt(0, find.text('✔ resolved'))).findsNothing(); + + check(findInTopicItemAt(0, find.text('resolved'))).findsOne(); + check(findInTopicItemAt(0, find.byIcon(ZulipIcons.check).hitTestable())) + .findsOne(); + + check(findInTopicItemAt(1, find.text('unresolved'))).findsOne(); + check(findInTopicItemAt(1, find.byType(Icon)).hitTestable()) + .findsNothing(); + }); + + testWidgets('handle empty topics', (tester) async { + await prepare(tester, topics: [ + eg.getStreamTopicsEntry(name: ''), + ]); + check(findInTopicItemAt(0, + find.text(eg.defaultRealmEmptyTopicDisplayName))).findsOne(); + }); + + group('unreads', () { + testWidgets('muted and non-muted topics', (tester) async { + final channel = eg.stream(); + await prepare(tester, channel: channel, + topics: [ + eg.getStreamTopicsEntry(maxId: 2, name: 'muted'), + eg.getStreamTopicsEntry(maxId: 1, name: 'non-muted'), + ], + userTopics: [ + eg.userTopicItem(channel, 'muted', UserTopicVisibilityPolicy.muted), + ], + messages: [ + eg.streamMessage(stream: channel, topic: 'muted'), + eg.streamMessage(stream: channel, topic: 'non-muted'), + eg.streamMessage(stream: channel, topic: 'non-muted'), + ]); + + check(findInTopicItemAt(0, find.text('1'))).findsOne(); + check(findInTopicItemAt(0, find.text('muted'))).findsOne(); + check(findInTopicItemAt(0, find.byIcon(ZulipIcons.mute).hitTestable())) + .findsOne(); + + check(findInTopicItemAt(1, find.text('2'))).findsOne(); + check(findInTopicItemAt(1, find.text('non-muted'))).findsOne(); + check(findInTopicItemAt(1, find.byType(Icon).hitTestable())) + .findsNothing(); + }); + + testWidgets('with and without unread mentions', (tester) async { + final channel = eg.stream(); + await prepare(tester, channel: channel, + topics: [ + eg.getStreamTopicsEntry(maxId: 2, name: 'not mentioned'), + eg.getStreamTopicsEntry(maxId: 1, name: 'mentioned'), + ], + messages: [ + eg.streamMessage(stream: channel, topic: 'not mentioned'), + eg.streamMessage(stream: channel, topic: 'not mentioned'), + eg.streamMessage(stream: channel, topic: 'not mentioned', + flags: [MessageFlag.mentioned, MessageFlag.read]), + eg.streamMessage(stream: channel, topic: 'mentioned', + flags: [MessageFlag.mentioned]), + ]); + + check(findInTopicItemAt(0, find.text('2'))).findsOne(); + check(findInTopicItemAt(0, find.text('not mentioned'))).findsOne(); + check(findInTopicItemAt(0, find.byType(Icons))).findsNothing(); + + check(findInTopicItemAt(1, find.text('1'))).findsOne(); + check(findInTopicItemAt(1, find.text('mentioned'))).findsOne(); + check(findInTopicItemAt(1, find.byIcon(ZulipIcons.at_sign))).findsOne(); + }); + }); + + group('topic visibility', () { + testWidgets('default', (tester) async { + final channel = eg.stream(); + await prepare(tester, channel: channel, + topics: [eg.getStreamTopicsEntry(name: 'topic')]); + + check(find.descendant(of: topicItemFinder, + matching: find.byType(Icons))).findsNothing(); + }); + + testWidgets('muted', (tester) async { + final channel = eg.stream(); + await prepare(tester, channel: channel, + topics: [eg.getStreamTopicsEntry(name: 'topic')], + userTopics: [ + eg.userTopicItem(channel, 'topic', UserTopicVisibilityPolicy.muted), + ]); + check(find.descendant(of: topicItemFinder, + matching: find.byIcon(ZulipIcons.mute))).findsOne(); + }); + + testWidgets('unmuted', (tester) async { + final channel = eg.stream(); + await prepare(tester, channel: channel, + topics: [eg.getStreamTopicsEntry(name: 'topic')], + userTopics: [ + eg.userTopicItem(channel, 'topic', UserTopicVisibilityPolicy.unmuted), + ]); + check(find.descendant(of: topicItemFinder, + matching: find.byIcon(ZulipIcons.unmute))).findsOne(); + }); + + testWidgets('followed', (tester) async { + final channel = eg.stream(); + await prepare(tester, channel: channel, + topics: [eg.getStreamTopicsEntry(name: 'topic')], + userTopics: [ + eg.userTopicItem(channel, 'topic', UserTopicVisibilityPolicy.followed), + ]); + check(find.descendant(of: topicItemFinder, + matching: find.byIcon(ZulipIcons.follow))).findsOne(); + }); + }); +} From d81b812af4aef99a9e5789c30ffe986ffe991cd1 Mon Sep 17 00:00:00 2001 From: lakshya1goel Date: Thu, 1 May 2025 20:38:51 +0530 Subject: [PATCH 3/3] topics: Add TopicListButton to channel action sheet The icon was taken from CZO discussion: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/Topic.20list.20in.20channel/near/2140324 Fixes: #1158 Co-authored-by: Zixuan James Li --- assets/icons/ZulipIcons.ttf | Bin 14108 -> 14384 bytes assets/icons/topics.svg | 3 ++ assets/l10n/app_en.arb | 4 ++ lib/generated/l10n/zulip_localizations.dart | 6 +++ .../l10n/zulip_localizations_ar.dart | 3 ++ .../l10n/zulip_localizations_de.dart | 3 ++ .../l10n/zulip_localizations_en.dart | 3 ++ .../l10n/zulip_localizations_ja.dart | 3 ++ .../l10n/zulip_localizations_nb.dart | 3 ++ .../l10n/zulip_localizations_pl.dart | 3 ++ .../l10n/zulip_localizations_ru.dart | 3 ++ .../l10n/zulip_localizations_sk.dart | 3 ++ .../l10n/zulip_localizations_uk.dart | 3 ++ lib/widgets/action_sheet.dart | 40 +++++++++++++----- lib/widgets/icons.dart | 7 ++- test/widgets/action_sheet_test.dart | 16 ++++++- 16 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 assets/icons/topics.svg diff --git a/assets/icons/ZulipIcons.ttf b/assets/icons/ZulipIcons.ttf index 84e19a9cfaed21884cc933dd7b92621425d79f1d..0ef0c3f461d17e792be725867eb5298a77c384c8 100644 GIT binary patch delta 1981 zcmb7FOH5-`82(Nlmq!a@p-kHuMW8r?(C6)KOG^u-z!>9XOolKaCJL3|r9A3@2%30V zx|q%6ZdtgPxY31*3!`XA#<(^yAu(|=8xj{26UgX7Blw-m;o;I=zWe{@fB*kE_pr6} zdDVmf;K3%GNW3&URvvwH=Lh#nKaK7I?jWrurxq*K!1vQn0qs6z`s~8S zO!()wmH_Wf;QIL7bmi{a{tsW$_A!x}qo7u}M#%3FopXz;>z}@S{11KCnZv)ZJXNfG z_4jwwd3O1_zgStXiq}PkN|~^2sj@iT@^wiEQV#(~wz|Bs`bTo&2uT0R_3Z~c;f4{r z19tIk)5z@yPm*H9Y8A8Q8XD1oPw*pt!=Iv4Oo}bBD-InCj;BsI$Ki$-G2Fl#ScL=6 zAj&6(I1)&~KpGixQDo7J9C;^4bZ&7jcT=9>6G0#C67^T89fYcHq1n zSZC!fJdXw@j38+zG#JRBwnV!;3)2up2({GfEMyJ6J%35{iT;IqJ>sPQ&oDI3mpZz4I@A&og<2=zM7a`4t?o56<%^G&zMc-R^4dx80tWNGugh@_!M(%AAVLIn* zx$HN~gn6{H7=zXsuH~rnoQ(U-s#Eqvjr4D08wPEXc!AtWe3?4J(=?75+{1^cH@T~F zBm;JbI@6pVnmT*ja;N)kxxvseS{jtslGE*=eSGIRT8)8=)I~{a?UGJBJ!)1mN&^d# z8#8F12aU&-`=6n(9^D-LEj-JeHyAF@Ao2hum|T`8o#jNHW9t9v(_BlaC+uY^&!1B5WM2~gTy=#5!c3ED{ zj33fb{+_UU$wf%(c@Z2k@GjT&?B2m=0`u57=&`=h!pQ`!3XGd%;xrq}af|wyx5&ce zJj$#h&yt5Y-8cs^Xf@W1xqBE@vwT|RD*X$6Nh95xqhG41$4cdBJ~K2BA0D(0YSyhb z?ULzbZa7Qw&EGIFp^zZ0C`3r#R49;6DfE%vRY;OfD;T6R3Te_=1>U=uQ^=9doAO0u ziMJK{Nf#7Sq>BngQeMV3cxPf+Ax2tND3QLSP$s>n5G7qv$dj%r6nPorU4h^`m zZ#D&kO-Gx)P`B8dQ}RH-u_GieJbn-VgfAHMsh5;;=|odVggS`dPnk#dTD}~@X1)H) oe(y2P#K&jcwl}fw;(xP4<0FT-vHgB}QfRBo)%mFv>z7RVUp~waG5`Po delta 1698 zcmb7^%WoT16vn@CY{yBQq|g?Uwv^C>5@I{{*zqfN65EM@N{9j>2%(79qAPB(%iO27bZ(3O5tIy}oJ@4<_nd^;f z>rFY5y7ao*lzMb_uJ&|gjsvG4we6jqO{SmSo86N_5f0BUtkoN(Z=dOx!rPn;EwAn@ zo$b|EQgm24-B?+yU)Z|!`G?qk4RR|SM4aFZ`x{_rWo>i&{WE|6iSG(I4y|r1Ow>Pk z>K%f2UBlpwwfc6$nRjw1`*{5A9_p6(PF{S*383JX{PGMK1u(0~nuLydVQfkON`eXPdu$=HC^zf9zC+ph} z&`ea#1nJ9|n?9;ICfH@vOTYvbru~9U_yIkHc>(JTD@o!E`yzT*!?2X#BuAE=2@N%wMA}A=(g36-&1Rr1GJIBCZz}2wGx=rac<9ZB)+K2aMSHGGU+$yD%oUn zaCtg>g3V+nNN`Lu`SZbNmOgznX-az8Vgk8p!RJqF7G>nRg^S#q8LapWb7B68Cri+mMY`4b#UTNabZ*DOYneDr-7hIT9}yB~M_;dq~Q&F0@e iT=@S>+I>8FYjC&Uy&2@w{a5zN?v>nm$NM>7b@UJAG}@*B diff --git a/assets/icons/topics.svg b/assets/icons/topics.svg new file mode 100644 index 0000000000..c07afa80b3 --- /dev/null +++ b/assets/icons/topics.svg @@ -0,0 +1,3 @@ + + + \ No newline at end of file diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index 1c0ec3d7e8..91206fabc6 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -84,6 +84,10 @@ "@actionSheetOptionMarkChannelAsRead": { "description": "Label for marking a channel as read." }, + "actionSheetOptionListOfTopics": "List of topics", + "@actionSheetOptionListOfTopics": { + "description": "Label for navigating to a channel's topic-list page." + }, "actionSheetOptionMuteTopic": "Mute topic", "@actionSheetOptionMuteTopic": { "description": "Label for muting a topic on action sheet." diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index 566ff617ad..675407f68d 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -239,6 +239,12 @@ abstract class ZulipLocalizations { /// **'Mark channel as read'** String get actionSheetOptionMarkChannelAsRead; + /// Label for navigating to a channel's topic-list page. + /// + /// In en, this message translates to: + /// **'List of topics'** + String get actionSheetOptionListOfTopics; + /// Label for muting a topic on action sheet. /// /// In en, this message translates to: diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index d1025b7151..febbb2c585 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -76,6 +76,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override + String get actionSheetOptionListOfTopics => 'List of topics'; + @override String get actionSheetOptionMuteTopic => 'Mute topic'; diff --git a/lib/generated/l10n/zulip_localizations_de.dart b/lib/generated/l10n/zulip_localizations_de.dart index eee1309e1b..a9188773c0 100644 --- a/lib/generated/l10n/zulip_localizations_de.dart +++ b/lib/generated/l10n/zulip_localizations_de.dart @@ -76,6 +76,9 @@ class ZulipLocalizationsDe extends ZulipLocalizations { @override String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override + String get actionSheetOptionListOfTopics => 'List of topics'; + @override String get actionSheetOptionMuteTopic => 'Mute topic'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index fbdcea3935..73a4212ee3 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -76,6 +76,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override + String get actionSheetOptionListOfTopics => 'List of topics'; + @override String get actionSheetOptionMuteTopic => 'Mute topic'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index a9a2e40dce..b614f71cbb 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -76,6 +76,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override + String get actionSheetOptionListOfTopics => 'List of topics'; + @override String get actionSheetOptionMuteTopic => 'Mute topic'; diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index afe3c9794a..4929eae453 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -76,6 +76,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override + String get actionSheetOptionListOfTopics => 'List of topics'; + @override String get actionSheetOptionMuteTopic => 'Mute topic'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 7653f8d31d..a5ee696797 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -78,6 +78,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations { String get actionSheetOptionMarkChannelAsRead => 'Oznacz kanał jako przeczytany'; + @override + String get actionSheetOptionListOfTopics => 'List of topics'; + @override String get actionSheetOptionMuteTopic => 'Wycisz wątek'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 6ea916c359..1fcde0d7a2 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -78,6 +78,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations { String get actionSheetOptionMarkChannelAsRead => 'Отметить канал как прочитанный'; + @override + String get actionSheetOptionListOfTopics => 'List of topics'; + @override String get actionSheetOptionMuteTopic => 'Отключить тему'; diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index 5430a885c4..30cd4c89f8 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -76,6 +76,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override + String get actionSheetOptionListOfTopics => 'List of topics'; + @override String get actionSheetOptionMuteTopic => 'Stlmiť tému'; diff --git a/lib/generated/l10n/zulip_localizations_uk.dart b/lib/generated/l10n/zulip_localizations_uk.dart index db213893db..a77d28aeff 100644 --- a/lib/generated/l10n/zulip_localizations_uk.dart +++ b/lib/generated/l10n/zulip_localizations_uk.dart @@ -79,6 +79,9 @@ class ZulipLocalizationsUk extends ZulipLocalizations { String get actionSheetOptionMarkChannelAsRead => 'Позначити канал як прочитаний'; + @override + String get actionSheetOptionListOfTopics => 'List of topics'; + @override String get actionSheetOptionMuteTopic => 'Заглушити тему'; diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 04e535e65a..6bd4e1024a 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -29,6 +29,7 @@ import 'page.dart'; import 'store.dart'; import 'text.dart'; import 'theme.dart'; +import 'topic_list.dart'; void _showActionSheet( BuildContext context, { @@ -175,24 +176,43 @@ void showChannelActionSheet(BuildContext context, { final pageContext = PageRoot.contextOf(context); final store = PerAccountStoreWidget.of(pageContext); - final optionButtons = []; + final optionButtons = [ + TopicListButton(pageContext: pageContext, channelId: channelId), + ]; + final unreadCount = store.unreads.countInChannelNarrow(channelId); if (unreadCount > 0) { optionButtons.add( MarkChannelAsReadButton(pageContext: pageContext, channelId: channelId)); } - if (optionButtons.isEmpty) { - // TODO(a11y): This case makes a no-op gesture handler; as a consequence, - // we're presenting some UI (to people who use screen-reader software) as - // though it offers a gesture interaction that it doesn't meaningfully - // offer, which is confusing. The solution here is probably to remove this - // is-empty case by having at least one button that's always present, - // such as "copy link to channel". - return; - } + _showActionSheet(pageContext, optionButtons: optionButtons); } +class TopicListButton extends ActionSheetMenuItemButton { + const TopicListButton({ + super.key, + required this.channelId, + required super.pageContext, + }); + + final int channelId; + + @override + IconData get icon => ZulipIcons.topics; + + @override + String label(ZulipLocalizations zulipLocalizations) { + return zulipLocalizations.actionSheetOptionListOfTopics; + } + + @override + void onPressed() { + Navigator.push(pageContext, + TopicListPage.buildRoute(context: pageContext, streamId: channelId)); + } +} + class MarkChannelAsReadButton extends ActionSheetMenuItemButton { const MarkChannelAsReadButton({ super.key, diff --git a/lib/widgets/icons.dart b/lib/widgets/icons.dart index bab7b152de..be088afd48 100644 --- a/lib/widgets/icons.dart +++ b/lib/widgets/icons.dart @@ -144,11 +144,14 @@ abstract final class ZulipIcons { /// The Zulip custom icon "topic". static const IconData topic = IconData(0xf128, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "topics". + static const IconData topics = IconData(0xf129, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "unmute". - static const IconData unmute = IconData(0xf129, fontFamily: "Zulip Icons"); + static const IconData unmute = IconData(0xf12a, fontFamily: "Zulip Icons"); /// The Zulip custom icon "user". - static const IconData user = IconData(0xf12a, fontFamily: "Zulip Icons"); + static const IconData user = IconData(0xf12b, fontFamily: "Zulip Icons"); // END GENERATED ICON DATA } diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 6b8011510a..16cc36b096 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -226,6 +226,7 @@ void main() { group('showChannelActionSheet', () { void checkButtons() { check(actionSheetFinder).findsOne(); + checkButton('List of topics'); checkButton('Mark channel as read'); } @@ -244,7 +245,7 @@ void main() { testWidgets('show with no unread messages', (tester) async { await prepare(hasUnreadMessages: false); await showFromSubscriptionList(tester); - check(actionSheetFinder).findsNothing(); + check(findButtonForLabel('Mark channel as read')).findsNothing(); }); testWidgets('show from app bar in channel narrow', (tester) async { @@ -268,6 +269,19 @@ void main() { }); }); + testWidgets('TopicListButton', (tester) async { + await prepare(); + await showFromAppBar(tester, + narrow: ChannelNarrow(someChannel.streamId)); + + connection.prepare(json: GetStreamTopicsResult(topics: [ + eg.getStreamTopicsEntry(name: 'some topic foo'), + ]).toJson()); + await tester.tap(findButtonForLabel('List of topics')); + await tester.pumpAndSettle(); + check(find.text('some topic foo')).findsOne(); + }); + group('MarkChannelAsReadButton', () { void checkRequest(int channelId) { check(connection.takeRequests()).single.isA()