From ef9d18469a99d623e19af6d84270e85b78f2a806 Mon Sep 17 00:00:00 2001 From: Shu Chen Date: Mon, 20 Nov 2023 14:06:01 +0000 Subject: [PATCH 1/2] ui [nfc]: Pull out iconDataForStream helper --- lib/widgets/icons.dart | 13 +++++++++++++ lib/widgets/inbox.dart | 8 +------- lib/widgets/message_list.dart | 8 ++------ 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/widgets/icons.dart b/lib/widgets/icons.dart index 84cb8a7fc1..8f3113e5fb 100644 --- a/lib/widgets/icons.dart +++ b/lib/widgets/icons.dart @@ -1,6 +1,8 @@ import 'package:flutter/widgets.dart'; +import '../api/model/model.dart'; + // ignore_for_file: constant_identifier_names /// Identifiers for Zulip's custom icons. @@ -63,3 +65,14 @@ abstract final class ZulipIcons { // END GENERATED ICON DATA } + +IconData iconDataForStream(ZulipStream stream) { + // TODO: these icons aren't quite right yet; + // see this message and the one after it: + // https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20.23F117.20.22Inbox.22.20screen/near/1680637 + return switch(stream) { + ZulipStream(isWebPublic: true) => ZulipIcons.globe, + ZulipStream(inviteOnly: true) => ZulipIcons.lock, + ZulipStream() => ZulipIcons.hash_sign, + }; +} diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index b5f5dad94b..8ca6f8c642 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -385,13 +385,7 @@ class _StreamHeaderItem extends _HeaderItem { }); @override get title => subscription.name; - @override get icon => switch (subscription) { - // TODO these icons aren't quite right yet; see this message and the following: - // https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20.23F117.20.22Inbox.22.20screen/near/1680637 - Subscription(isWebPublic: true) => ZulipIcons.globe, - Subscription(inviteOnly: true) => ZulipIcons.lock, - Subscription() => ZulipIcons.hash_sign, - }; + @override get icon => iconDataForStream(subscription); @override get collapsedIconColor => subscription.colorSwatch().iconOnPlainBackground; @override get uncollapsedIconColor => subscription.colorSwatch().iconOnBarBackground; @override get uncollapsedBackgroundColor => diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 3050dac91c..4cfc642cce 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -88,12 +88,8 @@ class MessageListAppBarTitle extends StatelessWidget { final Narrow narrow; Widget _buildStreamRow(ZulipStream? stream, String text) { - final icon = switch (stream) { - ZulipStream(isWebPublic: true) => ZulipIcons.globe, - ZulipStream(inviteOnly: true) => ZulipIcons.lock, - ZulipStream() => ZulipIcons.hash_sign, - null => null, // A null [Icon.icon] makes a blank space. - }; + // A null [Icon.icon] makes a blank space. + final icon = (stream != null) ? iconDataForStream(stream) : null; return Row( mainAxisSize: MainAxisSize.min, // TODO(design): The vertical alignment of the stream privacy icon is a bit ad hoc. From e4e2e8832f959b4cbba333b489c54faaaab07d58 Mon Sep 17 00:00:00 2001 From: Shu Chen Date: Fri, 10 Nov 2023 12:54:33 +0000 Subject: [PATCH 2/2] subscription_list: Add new SubscriptionListPage Fixes: #187 --- lib/widgets/app.dart | 6 + lib/widgets/subscription_list.dart | 238 +++++++++++++++++++++++ test/widgets/subscription_list_test.dart | 139 +++++++++++++ 3 files changed, 383 insertions(+) create mode 100644 lib/widgets/subscription_list.dart create mode 100644 test/widgets/subscription_list_test.dart diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index fab04677ee..d421823742 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -14,6 +14,7 @@ import 'message_list.dart'; import 'page.dart'; import 'recent_dm_conversations.dart'; import 'store.dart'; +import 'subscription_list.dart'; class ZulipApp extends StatelessWidget { const ZulipApp({super.key, this.navigatorObservers}); @@ -247,6 +248,11 @@ class HomePage extends StatelessWidget { InboxPage.buildRoute(context: context)), child: const Text("Inbox")), // TODO(i18n) const SizedBox(height: 16), + ElevatedButton( + onPressed: () => Navigator.push(context, + SubscriptionListPage.buildRoute(context: context)), + child: const Text("Subscribed streams")), + const SizedBox(height: 16), ElevatedButton( onPressed: () => Navigator.push(context, RecentDmConversationsPage.buildRoute(context: context)), diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart new file mode 100644 index 0000000000..796758fbc8 --- /dev/null +++ b/lib/widgets/subscription_list.dart @@ -0,0 +1,238 @@ +import 'package:collection/collection.dart'; +import 'package:flutter/material.dart'; + +import '../api/model/model.dart'; +import '../model/narrow.dart'; +import '../model/unreads.dart'; +import 'icons.dart'; +import 'message_list.dart'; +import 'page.dart'; +import 'store.dart'; +import 'text.dart'; +import 'unread_count_badge.dart'; + +/// Scrollable listing of subscribed streams. +class SubscriptionListPage extends StatefulWidget { + const SubscriptionListPage({super.key}); + + static Route buildRoute({required BuildContext context}) { + return MaterialAccountWidgetRoute(context: context, + page: const SubscriptionListPage()); + } + + @override + State createState() => _SubscriptionListPageState(); +} + +class _SubscriptionListPageState extends State with PerAccountStoreAwareStateMixin { + Unreads? unreadsModel; + + @override + void onNewStore() { + unreadsModel?.removeListener(_modelChanged); + unreadsModel = PerAccountStoreWidget.of(context).unreads + ..addListener(_modelChanged); + } + + @override + void dispose() { + unreadsModel?.removeListener(_modelChanged); + super.dispose(); + } + + void _modelChanged() { + setState(() { + // The actual state lives in [unreadsModel]. + // This method was called because that just changed. + }); + } + + @override + Widget build(BuildContext context) { + // Design referenced from: + // https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=171-12359&mode=design&t=4d0vykoYQ0KGpFuu-0 + + // This is an initial version with "Pinned" and "Unpinned" + // sections following behavior in mobile. Recalculating + // groups and sorting on every `build` here: it performs well + // enough and not worth optimizing as it will be replaced + // with a different behavior: + // TODO: Implement new grouping behavior and design, see discussion at: + // https://chat.zulip.org/#narrow/stream/101-design/topic/UI.20redesign.3A.20left.20sidebar/near/1540147 + + // TODO: Implement collapsible topics + + // TODO(i18n): localize strings on page + // Strings here left unlocalized as they likely will not + // exist in the settled design. + final store = PerAccountStoreWidget.of(context); + + final List pinned = []; + final List unpinned = []; + for (final subscription in store.subscriptions.values) { + if (subscription.pinToTop) { + pinned.add(subscription); + } else { + unpinned.add(subscription); + } + } + // TODO(i18n): add locale-aware sorting + pinned.sortBy((subscription) => subscription.name); + unpinned.sortBy((subscription) => subscription.name); + + return Scaffold( + appBar: AppBar(title: const Text("Streams")), + body: Center( + child: CustomScrollView( + slivers: [ + if (pinned.isEmpty && unpinned.isEmpty) + const _NoSubscriptionsItem(), + if (pinned.isNotEmpty) ...[ + const _SubscriptionListHeader(label: "Pinned"), + _SubscriptionList(unreadsModel: unreadsModel, subscriptions: pinned), + ], + if (unpinned.isNotEmpty) ...[ + const _SubscriptionListHeader(label: "Unpinned"), + _SubscriptionList(unreadsModel: unreadsModel, subscriptions: unpinned), + ], + + // TODO(#188): add button leading to "All Streams" page with ability to subscribe + + // This ensures last item in scrollable can settle in an unobstructed area. + const SliverSafeArea(sliver: SliverToBoxAdapter(child: SizedBox.shrink())), + ]))); + } +} + +class _NoSubscriptionsItem extends StatelessWidget { + const _NoSubscriptionsItem(); + + @override + Widget build(BuildContext context) { + return SliverToBoxAdapter( + child: Padding( + padding: const EdgeInsets.all(10), + child: Text("No streams found", + textAlign: TextAlign.center, + style: TextStyle( + color: const HSLColor.fromAHSL(1.0, 240, 0.1, 0.5).toColor(), + fontFamily: 'Source Sans 3', + fontSize: 18, + height: (20 / 18), + ).merge(weightVariableTextStyle(context))))); + } +} + +class _SubscriptionListHeader extends StatelessWidget { + const _SubscriptionListHeader({required this.label}); + + final String label; + + @override + Widget build(BuildContext context) { + return SliverToBoxAdapter( + child: ColoredBox( + color: Colors.white, + child: Row(crossAxisAlignment: CrossAxisAlignment.center, + children: [ + const SizedBox(width: 16), + Expanded(child: Divider( + color: const HSLColor.fromAHSL(0.2, 240, 0.1, 0.5).toColor())), + const SizedBox(width: 8), + Padding( + padding: const EdgeInsets.symmetric(vertical: 7), + child: Text(label, + textAlign: TextAlign.center, + style: TextStyle( + color: const HSLColor.fromAHSL(1.0, 240, 0.1, 0.5).toColor(), + fontFamily: 'Source Sans 3', + fontSize: 14, + letterSpacing: 0.04 * 14, + height: (16 / 14), + ).merge(weightVariableTextStyle(context)))), + const SizedBox(width: 8), + Expanded(child: Divider( + color: const HSLColor.fromAHSL(0.2, 240, 0.1, 0.5).toColor())), + const SizedBox(width: 16), + ]))); + } +} + +class _SubscriptionList extends StatelessWidget { + const _SubscriptionList({ + required this.unreadsModel, + required this.subscriptions, + }); + + final Unreads? unreadsModel; + final List subscriptions; + + @override + Widget build(BuildContext context) { + return SliverList.builder( + itemCount: subscriptions.length, + itemBuilder: (BuildContext context, int index) { + final subscription = subscriptions[index]; + final unreadCount = unreadsModel!.countInStreamNarrow(subscription.streamId); + return SubscriptionItem(subscription: subscription, unreadCount: unreadCount); + }); + } +} + +@visibleForTesting +class SubscriptionItem extends StatelessWidget { + const SubscriptionItem({ + super.key, + required this.subscription, + required this.unreadCount, + }); + + final Subscription subscription; + final int unreadCount; + + @override + Widget build(BuildContext context) { + final swatch = subscription.colorSwatch(); + final hasUnreads = (unreadCount > 0); + return Material( + color: Colors.white, + child: InkWell( + onTap: () { + Navigator.push(context, + MessageListPage.buildRoute(context: context, + narrow: StreamNarrow(subscription.streamId))); + }, + child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [ + const SizedBox(width: 16), + Padding( + padding: const EdgeInsets.symmetric(vertical: 11), + 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( + fontFamily: 'Source Sans 3', + fontSize: 18, + height: (20 / 18), + color: Color(0xFF262626), + ).merge(hasUnreads + ? weightVariableTextStyle(context, wght: 600, wghtIfPlatformRequestsBold: 900) + : weightVariableTextStyle(context)), + maxLines: 1, + overflow: TextOverflow.ellipsis, + subscription.name))), + if (unreadCount > 0) ...[ + const SizedBox(width: 12), + // TODO(#384) show @-mention indicator when it applies + 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 new file mode 100644 index 0000000000..1d596a0497 --- /dev/null +++ b/test/widgets/subscription_list_test.dart @@ -0,0 +1,139 @@ +import 'package:checks/checks.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/api/model/initial_snapshot.dart'; +import 'package:zulip/api/model/model.dart'; +import 'package:zulip/widgets/store.dart'; +import 'package:zulip/widgets/subscription_list.dart'; +import 'package:zulip/widgets/unread_count_badge.dart'; + +import '../model/binding.dart'; +import '../example_data.dart' as eg; + +void main() { + TestZulipBinding.ensureInitialized(); + + Future setupStreamListPage(WidgetTester tester, { + required List subscriptions, + UnreadMessagesSnapshot? unreadMsgs, + }) async { + addTearDown(testBinding.reset); + final initialSnapshot = eg.initialSnapshot( + subscriptions: subscriptions, + streams: subscriptions.toList(), + unreadMsgs: unreadMsgs, + ); + await testBinding.globalStore.add(eg.selfAccount, initialSnapshot); + + await tester.pumpWidget( + MaterialApp( + home: GlobalStoreWidget( + child: PerAccountStoreWidget( + accountId: eg.selfAccount.id, + child: const SubscriptionListPage())))); + + // global store, per-account store + await tester.pumpAndSettle(); + } + + bool isPinnedHeaderInTree() { + return find.text('Pinned').evaluate().isNotEmpty; + } + + bool isUnpinnedHeaderInTree() { + return find.text('Unpinned').evaluate().isNotEmpty; + } + + int getItemCount() { + return find.byType(SubscriptionItem).evaluate().length; + } + + testWidgets('smoke', (tester) async { + await setupStreamListPage(tester, subscriptions: []); + check(getItemCount()).equals(0); + check(isPinnedHeaderInTree()).isFalse(); + check(isUnpinnedHeaderInTree()).isFalse(); + }); + + testWidgets('basic subscriptions', (tester) async { + await setupStreamListPage(tester, subscriptions: [ + eg.subscription(eg.stream(streamId: 1), pinToTop: true), + eg.subscription(eg.stream(streamId: 2), pinToTop: true), + eg.subscription(eg.stream(streamId: 3), pinToTop: false), + ]); + check(getItemCount()).equals(3); + check(isPinnedHeaderInTree()).isTrue(); + check(isUnpinnedHeaderInTree()).isTrue(); + }); + + testWidgets('only pinned subscriptions', (tester) async { + await setupStreamListPage(tester, subscriptions: [ + eg.subscription(eg.stream(streamId: 1), pinToTop: true), + eg.subscription(eg.stream(streamId: 2), pinToTop: true), + ]); + check(getItemCount()).equals(2); + check(isPinnedHeaderInTree()).isTrue(); + check(isUnpinnedHeaderInTree()).isFalse(); + }); + + testWidgets('only unpinned subscriptions', (tester) async { + await setupStreamListPage(tester, subscriptions: [ + eg.subscription(eg.stream(streamId: 1), pinToTop: false), + eg.subscription(eg.stream(streamId: 2), pinToTop: false), + ]); + check(getItemCount()).equals(2); + check(isPinnedHeaderInTree()).isFalse(); + check(isUnpinnedHeaderInTree()).isTrue(); + }); + + testWidgets('subscription sort', (tester) async { + await setupStreamListPage(tester, subscriptions: [ + eg.subscription(eg.stream(streamId: 1, name: 'd'), pinToTop: true), + eg.subscription(eg.stream(streamId: 2, name: 'c'), pinToTop: false), + eg.subscription(eg.stream(streamId: 3, name: 'b'), pinToTop: true), + eg.subscription(eg.stream(streamId: 4, name: 'a'), pinToTop: false), + ]); + check(isPinnedHeaderInTree()).isTrue(); + check(isUnpinnedHeaderInTree()).isTrue(); + + final streamListItems = tester.widgetList(find.byType(SubscriptionItem)).toList(); + check(streamListItems.map((e) => e.subscription.streamId)).deepEquals([3, 1, 4, 2]); + }); + + testWidgets('unread badge shows with unreads', (tester) async { + final stream = eg.stream(); + final unreadMsgs = eg.unreadMsgs(streams: [ + UnreadStreamSnapshot(streamId: stream.streamId, topic: 'a', unreadMessageIds: [1, 2]), + ]); + await setupStreamListPage(tester, subscriptions: [ + eg.subscription(stream), + ], unreadMsgs: unreadMsgs); + check(find.byType(UnreadCountBadge).evaluate()).length.equals(1); + }); + + testWidgets('unread badge does not show with no unreads', (tester) async { + final stream = eg.stream(); + final unreadMsgs = eg.unreadMsgs(streams: []); + await setupStreamListPage(tester, subscriptions: [ + eg.subscription(stream), + ], unreadMsgs: unreadMsgs); + check(find.byType(UnreadCountBadge).evaluate()).length.equals(0); + }); + + testWidgets('color propagates to icon and badge', (tester) async { + final stream = eg.stream(); + final unreadMsgs = eg.unreadMsgs(streams: [ + UnreadStreamSnapshot(streamId: stream.streamId, topic: 'a', unreadMessageIds: [1, 2]), + ]); + final subscription = eg.subscription(stream, color: Colors.red.value); + final swatch = subscription.colorSwatch(); + await setupStreamListPage(tester, subscriptions: [ + subscription, + ], unreadMsgs: unreadMsgs); + check(getItemCount()).equals(1); + check(tester.widget(find.byType(Icon)).color) + .equals(swatch.iconOnPlainBackground); + check(tester.widget(find.byType(UnreadCountBadge)).backgroundColor) + .equals(swatch); + }); +}