From 473f89ff9f0cb692ff014ea5c0b19ed9a541c888 Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Mon, 15 Jul 2024 10:59:29 +0300 Subject: [PATCH 1/4] test: Generate random increasing streamId for example data by default This allows `eg.stream` to be used in contexts where it's important that the stream IDs don't collide or even that they're increasing, without the test having to explicitly give specific stream IDs when no other fact about them is relevant. Some tests are dependent on the stream IDs being static. Those are updated to use `eg.defaultStreamMessageStreamId` explicitly. --- test/example_data.dart | 20 ++++++++++++++++++-- test/model/message_list_test.dart | 14 +++++++------- test/model/message_test.dart | 2 +- test/widgets/message_list_test.dart | 2 +- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index 481c208533..6d3ae0fdf5 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -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, @@ -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 ?? '

A description

', // TODO generate random @@ -281,6 +292,8 @@ Map _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 @@ -289,6 +302,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({ @@ -303,7 +319,7 @@ StreamMessage streamMessage({ int? timestamp, List? 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 diff --git a/test/model/message_list_test.dart b/test/model/message_list_test.dart index b7c1086f96..812a8bcdda 100644 --- a/test/model/message_list_test.dart +++ b/test/model/message_list_test.dart @@ -41,7 +41,7 @@ void main() { /// Initialize [model] and the rest of the test state. Future 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); @@ -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(); @@ -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) => @@ -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); @@ -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'); diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 7d65975f84..482d77b2c1 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -34,7 +34,7 @@ void main() { /// Initialize [store] and the rest of the test state. Future 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); diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 2ddcbda415..b583446d78 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -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); From b517593b29cd9c089a1e86f238f94934a059ee95 Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Mon, 15 Jul 2024 11:01:13 +0300 Subject: [PATCH 2/4] subscription_list: Show muted streams below unmuted ones --- lib/widgets/subscription_list.dart | 15 +++++++++++---- test/widgets/subscription_list_test.dart | 12 ++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index a6622dc71e..bbcf7068a5 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -1,4 +1,3 @@ -import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; import '../api/model/model.dart'; @@ -48,6 +47,15 @@ class _SubscriptionListPageState extends State with PerAcc }); } + void _sortSubs(List 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: @@ -77,9 +85,8 @@ class _SubscriptionListPageState extends State 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")), diff --git a/test/widgets/subscription_list_test.dart b/test/widgets/subscription_list_test.dart index a13ae5cb31..638457b9eb 100644 --- a/test/widgets/subscription_list_test.dart +++ b/test/widgets/subscription_list_test.dart @@ -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 { From a9ad78ec379fcf77ef01bc55500785bb40f8880b Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Mon, 15 Jul 2024 11:01:45 +0300 Subject: [PATCH 3/4] test [nfc]: Factor out userTopicItem helper --- test/example_data.dart | 10 ++++++++++ test/model/stream_test.dart | 34 ++++++++++++---------------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/test/example_data.dart b/test/example_data.dart index 6d3ae0fdf5..67f0cd7b04 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -233,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. // diff --git a/test/model/stream_test.dart b/test/model/stream_test.dart index adc6a68505..d6a2c5df10 100644 --- a/test/model/stream_test.dart +++ b/test/model/stream_test.dart @@ -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 expected) { final expectedStore = eg.store(initialSnapshot: eg.initialSnapshot( userTopics: expected, @@ -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: { @@ -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), ]); }); @@ -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), ]); }); @@ -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), ]); }); @@ -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), ]); }); @@ -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); From b0b8a50703d2633fbc629a680f4f58ac4a6e8251 Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Mon, 15 Jul 2024 11:05:13 +0300 Subject: [PATCH 4/4] subscription_list: Show muted streams as faded in streams list Fixes: #424 --- lib/widgets/subscription_list.dart | 38 +++++++++++++++--------- test/widgets/subscription_list_test.dart | 33 ++++++++++++++++++++ 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index bbcf7068a5..fb876a82dc 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -209,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, @@ -222,8 +223,10 @@ 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( @@ -231,21 +234,28 @@ class SubscriptionItem extends StatelessWidget { // 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), ]))); diff --git a/test/widgets/subscription_list_test.dart b/test/widgets/subscription_list_test.dart index 638457b9eb..1d504ceb66 100644 --- a/test/widgets/subscription_list_test.dart +++ b/test/widgets/subscription_list_test.dart @@ -202,4 +202,37 @@ void main() { check(tester.widget(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( + find.ancestor(of: streamFinder, matching: find.byType(Opacity))); + final badgeFinder = find.text('$unreadCount'); + final badgeOpacity = tester.widget( + 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); + }); }