Skip to content

Commit

Permalink
msglist: Show channel name and topic name on two rows
Browse files Browse the repository at this point in the history
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 zulip#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: zulip#348

Signed-off-by: Zixuan James Li <zixuan@zulip.com>
  • Loading branch information
PIG208 committed Nov 25, 2024
1 parent a775793 commit d68bbd4
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 12 deletions.
18 changes: 18 additions & 0 deletions lib/widgets/icons.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
45 changes: 34 additions & 11 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -313,20 +313,40 @@ 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.
// 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)))),
]);
}

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))),
]);
}

Expand All @@ -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);
Expand Down
18 changes: 17 additions & 1 deletion test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,22 @@ void main() {
.page.isA<MessageListPage>().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', () {
Expand Down Expand Up @@ -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);
});
});
Expand Down

0 comments on commit d68bbd4

Please sign in to comment.