Skip to content

subscription_list: Show muted streams as faded in streams list #711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 35 additions & 18 deletions lib/widgets/subscription_list.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import 'package:collection/collection.dart';
import 'package:flutter/material.dart';

import '../api/model/model.dart';
Expand Down Expand Up @@ -48,6 +47,15 @@ class _SubscriptionListPageState extends State<SubscriptionListPage> with PerAcc
});
}

void _sortSubs(List<Subscription> list) {
list.sort((a, b) {
if (a.isMuted && !b.isMuted) return 1;
if (!a.isMuted && b.isMuted) return -1;
// TODO(i18n): add locale-aware sorting
return a.name.toLowerCase().compareTo(b.name.toLowerCase());
});
}

@override
Widget build(BuildContext context) {
// Design referenced from:
Expand Down Expand Up @@ -77,9 +85,8 @@ class _SubscriptionListPageState extends State<SubscriptionListPage> with PerAcc
unpinned.add(subscription);
}
}
// TODO(i18n): add locale-aware sorting
pinned.sortBy((subscription) => subscription.name.toLowerCase());
unpinned.sortBy((subscription) => subscription.name.toLowerCase());
_sortSubs(pinned);
_sortSubs(unpinned);

return Scaffold(
appBar: AppBar(title: const Text("Channels")),
Expand Down Expand Up @@ -202,6 +209,7 @@ class SubscriptionItem extends StatelessWidget {
Widget build(BuildContext context) {
final swatch = colorSwatchFor(context, subscription);
final hasUnreads = (unreadCount > 0);
final opacity = subscription.isMuted ? 0.55 : 1.0;
return Material(
// TODO(#95) need dark-theme color
color: Colors.white,
Expand All @@ -215,30 +223,39 @@ class SubscriptionItem extends StatelessWidget {
const SizedBox(width: 16),
Padding(
padding: const EdgeInsets.symmetric(vertical: 11),
child: Icon(size: 18, color: swatch.iconOnPlainBackground,
iconDataForStream(subscription))),
child: Opacity(
opacity: opacity,
child: Icon(size: 18, color: swatch.iconOnPlainBackground,
iconDataForStream(subscription)))),
const SizedBox(width: 5),
Expanded(
child: Padding(
padding: const EdgeInsets.symmetric(vertical: 10),
// TODO(design): unclear whether bold text is applied to all subscriptions
// or only those with unreads:
// https://github.com/zulip/zulip-flutter/pull/397#pullrequestreview-1742524205
child: Text(
style: const TextStyle(
fontSize: 18,
height: (20 / 18),
// TODO(#95) need dark-theme color
color: Color(0xFF262626),
).merge(weightVariableTextStyle(context,
wght: hasUnreads ? 600 : null)),
maxLines: 1,
overflow: TextOverflow.ellipsis,
subscription.name))),
child: Opacity(
opacity: opacity,
child: Text(
style: const TextStyle(
fontSize: 18,
height: (20 / 18),
// TODO(#95) need dark-theme color
color: Color(0xFF262626),
).merge(weightVariableTextStyle(context,
wght: hasUnreads ? 600 : null)),
maxLines: 1,
overflow: TextOverflow.ellipsis,
subscription.name)))),
if (unreadCount > 0) ...[
const SizedBox(width: 12),
// TODO(#747) show @-mention indicator when it applies
UnreadCountBadge(count: unreadCount, backgroundColor: swatch, bold: true),
Opacity(
opacity: opacity,
child: UnreadCountBadge(
count: unreadCount,
backgroundColor: swatch,
bold: true)),
],
const SizedBox(width: 16),
])));
Expand Down
30 changes: 28 additions & 2 deletions test/example_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,17 @@ final User fourthUser = user(fullName: 'Fourth User', email: 'fourth@example');
// Streams and subscriptions.
//

/// A fresh stream ID, from a random but always strictly increasing sequence.
int _nextStreamId() => (_lastStreamId += 1 + Random().nextInt(10));
int _lastStreamId = 200;

/// Construct an example stream.
///
/// If stream ID `streamId` is not given, it will be generated from a random
/// but increasing sequence.
/// Use an explicit `streamId` only if the ID needs to correspond to some
/// other data in the test, or if the IDs need to increase in a different order
/// from the calls to [stream].
ZulipStream stream({
int? streamId,
String? name,
Expand All @@ -165,7 +176,7 @@ ZulipStream stream({
int? streamWeeklyTraffic,
}) {
return ZulipStream(
streamId: streamId ?? 123, // TODO generate example IDs
streamId: streamId ?? _nextStreamId(),
name: name ?? 'A stream', // TODO generate example names
description: description ?? 'A description', // TODO generate example descriptions
renderedDescription: renderedDescription ?? '<p>A description</p>', // TODO generate random
Expand Down Expand Up @@ -222,6 +233,16 @@ Subscription subscription(
);
}

UserTopicItem userTopicItem(
ZulipStream stream, String topic, UserTopicVisibilityPolicy policy) {
return UserTopicItem(
streamId: stream.streamId,
topicName: topic,
lastUpdated: 1234567890,
visibilityPolicy: policy,
);
}

////////////////////////////////////////////////////////////////
// Messages, and pieces of messages.
//
Expand Down Expand Up @@ -281,6 +302,8 @@ Map<String, dynamic> _messagePropertiesFromContent(String? content, String? cont
int _nextMessageId() => (_lastMessageId += 1 + Random().nextInt(100));
int _lastMessageId = 1000;

const defaultStreamMessageStreamId = 123;

/// Construct an example stream message.
///
/// If the message ID `id` is not given, it will be generated from a random
Expand All @@ -289,6 +312,9 @@ int _lastMessageId = 1000;
/// in the test, or if the IDs need to increase in a different order from the
/// calls to [streamMessage] and [dmMessage].
///
/// The message will be in `stream` if given. Otherwise,
/// an example stream with ID `defaultStreamMessageStreamId` will be used.
///
/// See also:
/// * [dmMessage], to construct an example direct message.
StreamMessage streamMessage({
Expand All @@ -303,7 +329,7 @@ StreamMessage streamMessage({
int? timestamp,
List<MessageFlag>? flags,
}) {
final effectiveStream = stream ?? _stream();
final effectiveStream = stream ?? _stream(streamId: defaultStreamMessageStreamId);
// The use of JSON here is convenient in order to delegate parts of the data
// to helper functions. The main downside is that it loses static typing
// of the properties as we're constructing the data. That's probably OK
Expand Down
14 changes: 7 additions & 7 deletions test/model/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ void main() {

/// Initialize [model] and the rest of the test state.
Future<void> prepare({Narrow narrow = const CombinedFeedNarrow()}) async {
final stream = eg.stream();
final stream = eg.stream(streamId: eg.defaultStreamMessageStreamId);
subscription = eg.subscription(stream);
store = eg.store();
await store.addStream(stream);
Expand Down Expand Up @@ -247,13 +247,13 @@ void main() {
});

test('MessageEvent, not in narrow', () async {
final stream = eg.stream(streamId: 123);
final stream = eg.stream();
await prepare(narrow: StreamNarrow(stream.streamId));
await prepareMessages(foundOldest: true, messages:
List.generate(30, (i) => eg.streamMessage(stream: stream)));

check(model).messages.length.equals(30);
final otherStream = eg.stream(streamId: 234);
final otherStream = eg.stream();
await store.handleEvent(MessageEvent(id: 0,
message: eg.streamMessage(stream: otherStream)));
checkNotNotified();
Expand Down Expand Up @@ -709,7 +709,7 @@ void main() {
// doesn't need to exercise the different reasons that messages don't.

const timestamp = 1693602618;
final stream = eg.stream();
final stream = eg.stream(streamId: eg.defaultStreamMessageStreamId);
Message streamMessage(int id) =>
eg.streamMessage(id: id, stream: stream, topic: 'foo', timestamp: timestamp);
Message dmMessage(int id) =>
Expand Down Expand Up @@ -790,7 +790,7 @@ void main() {

const t1 = 1693602618;
const t2 = t1 + 86400;
final stream = eg.stream();
final stream = eg.stream(streamId: eg.defaultStreamMessageStreamId);
Message streamMessage(int id, int timestamp, User sender) =>
eg.streamMessage(id: id, sender: sender,
stream: stream, topic: 'foo', timestamp: timestamp);
Expand Down Expand Up @@ -831,8 +831,8 @@ void main() {
});

test('stream messages match just if same stream/topic', () {
final stream0 = eg.stream(streamId: 123);
final stream1 = eg.stream(streamId: 234);
final stream0 = eg.stream();
final stream1 = eg.stream();
final messageAB = eg.streamMessage(stream: stream0, topic: 'foo');
final messageXB = eg.streamMessage(stream: stream1, topic: 'foo');
final messageAX = eg.streamMessage(stream: stream0, topic: 'bar');
Expand Down
2 changes: 1 addition & 1 deletion test/model/message_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void main() {

/// Initialize [store] and the rest of the test state.
Future<void> prepare({Narrow narrow = const CombinedFeedNarrow()}) async {
final stream = eg.stream();
final stream = eg.stream(streamId: eg.defaultStreamMessageStreamId);
subscription = eg.subscription(stream);
store = eg.store();
await store.addStream(stream);
Expand Down
34 changes: 12 additions & 22 deletions test/model/stream_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -189,16 +189,6 @@ void main() {
});
});

UserTopicItem makeUserTopicItem(
ZulipStream stream, String topic, UserTopicVisibilityPolicy policy) {
return UserTopicItem(
streamId: stream.streamId,
topicName: topic,
lastUpdated: 1234567890,
visibilityPolicy: policy,
);
}

void compareTopicVisibility(PerAccountStore store, List<UserTopicItem> expected) {
final expectedStore = eg.store(initialSnapshot: eg.initialSnapshot(
userTopics: expected,
Expand All @@ -211,10 +201,10 @@ void main() {
final store = eg.store(initialSnapshot: eg.initialSnapshot(
streams: [stream1, stream2],
userTopics: [
makeUserTopicItem(stream1, 'topic 1', UserTopicVisibilityPolicy.muted),
makeUserTopicItem(stream1, 'topic 2', UserTopicVisibilityPolicy.unmuted),
makeUserTopicItem(stream2, 'topic 3', UserTopicVisibilityPolicy.unknown),
makeUserTopicItem(stream2, 'topic 4', UserTopicVisibilityPolicy.followed),
eg.userTopicItem(stream1, 'topic 1', UserTopicVisibilityPolicy.muted),
eg.userTopicItem(stream1, 'topic 2', UserTopicVisibilityPolicy.unmuted),
eg.userTopicItem(stream2, 'topic 3', UserTopicVisibilityPolicy.unknown),
eg.userTopicItem(stream2, 'topic 4', UserTopicVisibilityPolicy.followed),
]));
check(store.debugStreamStore.topicVisibility).deepEquals({
stream1.streamId: {
Expand All @@ -233,7 +223,7 @@ void main() {
final store = eg.store();
await store.addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted);
compareTopicVisibility(store, [
makeUserTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.muted),
eg.userTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.muted),
]);
});

Expand All @@ -242,8 +232,8 @@ void main() {
await store.addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted);
await store.addUserTopic(stream1, 'other topic', UserTopicVisibilityPolicy.unmuted);
compareTopicVisibility(store, [
makeUserTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.muted),
makeUserTopicItem(stream1, 'other topic', UserTopicVisibilityPolicy.unmuted),
eg.userTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.muted),
eg.userTopicItem(stream1, 'other topic', UserTopicVisibilityPolicy.unmuted),
]);
});

Expand All @@ -252,7 +242,7 @@ void main() {
await store.addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.muted);
await store.addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.unmuted);
compareTopicVisibility(store, [
makeUserTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.unmuted),
eg.userTopicItem(stream1, 'topic', UserTopicVisibilityPolicy.unmuted),
]);
});

Expand All @@ -262,7 +252,7 @@ void main() {
await store.addUserTopic(stream1, 'other topic', UserTopicVisibilityPolicy.unmuted);
await store.addUserTopic(stream1, 'topic', UserTopicVisibilityPolicy.none);
compareTopicVisibility(store, [
makeUserTopicItem(stream1, 'other topic', UserTopicVisibilityPolicy.unmuted),
eg.userTopicItem(stream1, 'other topic', UserTopicVisibilityPolicy.unmuted),
]);
});

Expand All @@ -288,9 +278,9 @@ void main() {
final store = eg.store(initialSnapshot: eg.initialSnapshot(
streams: [stream],
userTopics: [
makeUserTopicItem(stream, 'topic 1', UserTopicVisibilityPolicy.muted),
makeUserTopicItem(stream, 'topic 2', UserTopicVisibilityPolicy.unmuted),
makeUserTopicItem(stream, 'topic 3', UserTopicVisibilityPolicy.followed),
eg.userTopicItem(stream, 'topic 1', UserTopicVisibilityPolicy.muted),
eg.userTopicItem(stream, 'topic 2', UserTopicVisibilityPolicy.unmuted),
eg.userTopicItem(stream, 'topic 3', UserTopicVisibilityPolicy.followed),
]));
check(store.topicVisibilityPolicy(stream.streamId, 'topic 1'))
.equals(UserTopicVisibilityPolicy.muted);
Expand Down
2 changes: 1 addition & 1 deletion test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void main() {
UnreadMessagesSnapshot? unreadMsgs,
}) async {
addTearDown(testBinding.reset);
streams ??= subscriptions ??= [eg.subscription(eg.stream())];
streams ??= subscriptions ??= [eg.subscription(eg.stream(streamId: eg.defaultStreamMessageStreamId))];
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot(
streams: streams, subscriptions: subscriptions, unreadMsgs: unreadMsgs));
store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
Expand Down
45 changes: 45 additions & 0 deletions test/widgets/subscription_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,18 @@ void main() {
]);
check(listedStreamIds(tester)).deepEquals([1, 2, 3, 4, 5, 6]);
});

testWidgets('muted subscriptions come last among pinned streams and among unpinned streams', (tester) async {
await setupStreamListPage(tester, subscriptions: [
eg.subscription(eg.stream(streamId: 1, name: 'a'), isMuted: true, pinToTop: true),
eg.subscription(eg.stream(streamId: 2, name: 'b'), isMuted: false, pinToTop: true),
eg.subscription(eg.stream(streamId: 3, name: 'c'), isMuted: true, pinToTop: true),
eg.subscription(eg.stream(streamId: 4, name: 'd'), isMuted: false, pinToTop: false),
eg.subscription(eg.stream(streamId: 5, name: 'e'), isMuted: true, pinToTop: false),
eg.subscription(eg.stream(streamId: 6, name: 'f'), isMuted: false, pinToTop: false),
]);
check(listedStreamIds(tester)).deepEquals([2, 1, 3, 4, 6, 5]);
});
});

testWidgets('unread badge shows with unreads', (tester) async {
Expand Down Expand Up @@ -190,4 +202,37 @@ void main() {
check(tester.widget<UnreadCountBadge>(find.byType(UnreadCountBadge)).backgroundColor)
.equals(swatch);
});

testWidgets('muted streams are displayed as faded', (tester) async {
void checkOpacityForStreamAndBadge(String streamName, int unreadCount, double opacity) {
final streamFinder = find.text(streamName);
final streamOpacity = tester.widget<Opacity>(
find.ancestor(of: streamFinder, matching: find.byType(Opacity)));
final badgeFinder = find.text('$unreadCount');
final badgeOpacity = tester.widget<Opacity>(
find.ancestor(of: badgeFinder, matching: find.byType(Opacity)));
check(streamOpacity.opacity).equals(opacity);
check(badgeOpacity.opacity).equals(opacity);
}

final stream1 = eg.stream(name: 'Stream 1');
final stream2 = eg.stream(name: 'Stream 2');
await setupStreamListPage(tester,
subscriptions: [
eg.subscription(stream1, isMuted: true),
eg.subscription(stream2, isMuted: false),
],
userTopics: [
eg.userTopicItem(stream1, 'a', UserTopicVisibilityPolicy.unmuted),
eg.userTopicItem(stream2, 'b', UserTopicVisibilityPolicy.unmuted),
],
unreadMsgs: eg.unreadMsgs(streams: [
UnreadStreamSnapshot(streamId: stream1.streamId, topic: 'a', unreadMessageIds: [1, 2]),
UnreadStreamSnapshot(streamId: stream2.streamId, topic: 'b', unreadMessageIds: [3]),
]),
);

checkOpacityForStreamAndBadge('Stream 1', 2, 0.55);
checkOpacityForStreamAndBadge('Stream 2', 1, 1.0);
});
}