From d38bc57eac5df2dea1f4b99366a3e990abf8e78f Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Mon, 25 Nov 2024 14:00:09 -0500 Subject: [PATCH] msglist: Show channel name and topic name on two rows This mostly follows the legacy mobile app. Notably, this changes the title text to use normal font weight (wght=400), and breaks the text into two rows for topic narrow. This will eventually be superseded by #1039, so we should keep the implementation as simple as possible for now. There are some differences from the old design: The legacy mobile uses different colors for the title text, depending on the color of the channel, to make the text more visible. We currently don't have that, so the text just uses the ambient color. The original design also displays the 'mute' icon when the channel is muted: https://github.com/zulip/zulip-mobile/blob/a115df1f71c9dc31e9b41060a8d57b51c017d786/src/streams/StreamIcon.js#L20-L29 In the Flutter app, however, only the privacy level related icons are displayed (e.g.: web public, invite only). We continue to leave out the 'mute' icon in this implementation. This can change after we have a concrete redesign plan. This implementation also shows the corresponding icons for 'muted' and 'unmuted' topics; previously, only the icon for 'follow' was displayed. And we continue using the existing icons in the Flutter app, without trying to match with the exact ones in the old design. References: https://github.com/zulip/zulip-mobile/blob/a115df1f71c9dc31e9b41060a8d57b51c017d786/src/title/TitleStream.js#L113-L141 https://github.com/zulip/zulip-mobile/blob/a115df1f71c9dc31e9b41060a8d57b51c017d786/src/styles/navStyles.js#L5-L18 Fixes: #348 Signed-off-by: Zixuan James Li --- lib/widgets/icons.dart | 18 +++++++++++ lib/widgets/message_list.dart | 49 ++++++++++++++++++++++------- test/widgets/message_list_test.dart | 18 ++++++++++- 3 files changed, 72 insertions(+), 13 deletions(-) diff --git a/lib/widgets/icons.dart b/lib/widgets/icons.dart index ef7b8496aed..c5a93eb31f5 100644 --- a/lib/widgets/icons.dart +++ b/lib/widgets/icons.dart @@ -127,3 +127,21 @@ IconData iconDataForStream(ZulipStream stream) { ZulipStream() => ZulipIcons.hash_sign, }; } + +IconData? iconDataForTopic(UserTopicVisibilityPolicy policy) { + switch (policy) { + case UserTopicVisibilityPolicy.muted: + return ZulipIcons.mute; + case UserTopicVisibilityPolicy.unmuted: + return ZulipIcons.unmute; + case UserTopicVisibilityPolicy.followed: + return ZulipIcons.follow; + case UserTopicVisibilityPolicy.none: + return null; + case UserTopicVisibilityPolicy.unknown: + // This case is unreachable (or should be) because we keep `unknown` out + // of our data structures. We plan to remove the `unknown` case in #1074. + assert(false); + return null; + } +} diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index e996104e6d9..0e9a2d612df 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -313,20 +313,42 @@ class MessageListAppBarTitle extends StatelessWidget { Widget _buildStreamRow(BuildContext context, { ZulipStream? stream, - required String text, }) { - // A null [Icon.icon] makes a blank space. - final icon = (stream != null) ? iconDataForStream(stream) : null; - return Row( + final icon = (stream == null) ? ZulipIcons.hash_sign : iconDataForStream(stream); + Widget result = 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: [ - Icon(size: 16, icon), - const SizedBox(width: 4), - Flexible(child: Text(text)), + Padding(padding: const EdgeInsetsDirectional.only(end: 8.0), + child: Icon(size: 20, icon)), + Flexible(child: Text(stream?.name ?? '(unknown stream)', + style: const TextStyle( + fontSize: 20, + ).merge(weightVariableTextStyle(context)))), + ]); + + return result; + } + + Widget _buildTopicRow(BuildContext context, { + required ZulipStream? stream, + required String topic, + }) { + final store = PerAccountStoreWidget.of(context); + final icon = (stream == null) ? null + : iconDataForTopic(store.topicVisibilityPolicy(stream.streamId, topic)); + return Row( + children: [ + Flexible(child: Text(topic, style: const TextStyle( + fontSize: 13, + ).merge(weightVariableTextStyle(context)))), + if (icon != null) + Padding( + padding: const EdgeInsetsDirectional.only(start: 4), + child: Opacity(opacity: 0.4, child: Icon(icon, size: 14))), ]); } @@ -347,21 +369,24 @@ class MessageListAppBarTitle extends StatelessWidget { case ChannelNarrow(:var streamId): final store = PerAccountStoreWidget.of(context); final stream = store.streams[streamId]; - final streamName = stream?.name ?? '(unknown channel)'; - return _buildStreamRow(context, stream: stream, text: streamName); + return _buildStreamRow(context, stream: stream); case TopicNarrow(:var streamId, :var topic): final store = PerAccountStoreWidget.of(context); final stream = store.streams[streamId]; - final streamName = stream?.name ?? '(unknown channel)'; + return SizedBox( width: double.infinity, child: GestureDetector( behavior: HitTestBehavior.translucent, onLongPress: () => showTopicActionSheet(context, channelId: streamId, topic: topic), - child: _buildStreamRow( - context, stream: stream, text: "$streamName > $topic"))); + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + _buildStreamRow(context, stream: stream), + _buildTopicRow(context, stream: stream, topic: topic), + ]))); case DmNarrow(:var otherRecipientIds): final store = PerAccountStoreWidget.of(context); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 1bd7f798cf0..bda13c941af 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -152,6 +152,22 @@ void main() { .page.isA().initNarrow .equals(ChannelNarrow(channel.streamId)); }); + + testWidgets('show topic visibility policy for topic narrows', (tester) async { + final channel = eg.stream(); + const topic = 'topic'; + await setupMessageListPage(tester, + narrow: TopicNarrow(channel.streamId, topic), + streams: [channel], subscriptions: [eg.subscription(channel)], + messageCount: 1); + await store.handleEvent(eg.userTopicEvent( + channel.streamId, topic, UserTopicVisibilityPolicy.muted)); + await tester.pump(); + + check(find.descendant( + of: find.byType(MessageListAppBarTitle), + matching: find.byIcon(ZulipIcons.mute))).findsOne(); + }); }); group('presents message content appropriately', () { @@ -725,7 +741,7 @@ void main() { ).length.equals(1); check(find.descendant( of: find.byType(MessageListAppBarTitle), - matching: find.text('${channel.name} > new topic')).evaluate() + matching: find.text('new topic')).evaluate() ).length.equals(1); }); });