From d68bbd45800d40de6fcd19abca61f5ddda0101f6 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 | 45 ++++++++++++++++++++++------- test/widgets/message_list_test.dart | 18 +++++++++++- 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/lib/widgets/icons.dart b/lib/widgets/icons.dart index ef7b8496ae..c5a93eb31f 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 e996104e6d..c00303e98d 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -313,10 +313,8 @@ 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; + final icon = (stream == null) ? ZulipIcons.hash_sign : iconDataForStream(stream); return Row( mainAxisSize: MainAxisSize.min, // TODO(design): The vertical alignment of the stream privacy icon is a bit ad hoc. @@ -324,9 +322,31 @@ class MessageListAppBarTitle extends StatelessWidget { // 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)))), + ]); + } + + 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 +367,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 1bd7f798cf..bda13c941a 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); }); });