From 30e332e26058df85697669bd241c6c6425cfa107 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 24 Oct 2023 15:46:45 -0400 Subject: [PATCH 1/7] RecentDmConversationsPage: Add missing `dispose` override to state --- lib/widgets/recent_dm_conversations.dart | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index ccf6909c45..4f43876c24 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -31,6 +31,12 @@ class _RecentDmConversationsPageState extends State w ..addListener(_modelChanged); } + @override + void dispose() { + model?.removeListener(_modelChanged); + super.dispose(); + } + void _modelChanged() { setState(() { // The actual state lives in [model]. From 45487c9ab920dc9088233e7c6184385f5060c7a4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 24 Oct 2023 16:23:03 -0400 Subject: [PATCH 2/7] test [nfc]: Make a helper more compact --- test/widgets/recent_dm_conversations_test.dart | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/widgets/recent_dm_conversations_test.dart b/test/widgets/recent_dm_conversations_test.dart index 02d6d127dc..7532915fbf 100644 --- a/test/widgets/recent_dm_conversations_test.dart +++ b/test/widgets/recent_dm_conversations_test.dart @@ -61,12 +61,9 @@ void main() { TestZulipBinding.ensureInitialized(); group('RecentDmConversationsPage', () { - Finder findConversationItem(Narrow narrow) { - return find.byWidgetPredicate( - (widget) => - widget is RecentDmConversationsItem && widget.narrow == narrow, - ); - } + Finder findConversationItem(Narrow narrow) => find.byWidgetPredicate( + (widget) => widget is RecentDmConversationsItem && widget.narrow == narrow, + ); testWidgets('page builds; conversations appear in order', (WidgetTester tester) async { final user1 = eg.user(userId: 1); From 12c6a0c4fc8de4ec139eded2af54566239e4d9f1 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 24 Oct 2023 17:41:51 -0400 Subject: [PATCH 3/7] recent_dm_conversations test: Retitle some tests to be more specific There's about to be another kind of content to test for: a DM conversation's unread count. --- test/widgets/recent_dm_conversations_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/widgets/recent_dm_conversations_test.dart b/test/widgets/recent_dm_conversations_test.dart index 7532915fbf..43df5dd955 100644 --- a/test/widgets/recent_dm_conversations_test.dart +++ b/test/widgets/recent_dm_conversations_test.dart @@ -146,7 +146,7 @@ void main() { } group('self-1:1', () { - testWidgets('has right content', (WidgetTester tester) async { + testWidgets('has right title/avatar', (WidgetTester tester) async { final message = eg.dmMessage(from: eg.selfUser, to: []); await setupPage(tester, users: [], dmMessages: [message]); @@ -172,7 +172,7 @@ void main() { }); group('1:1', () { - testWidgets('has right content', (WidgetTester tester) async { + testWidgets('has right title/avatar', (WidgetTester tester) async { final user = eg.user(userId: 1); final message = eg.dmMessage(from: eg.selfUser, to: [user]); await setupPage(tester, users: [user], dmMessages: [message]); @@ -217,7 +217,7 @@ void main() { return result; } - testWidgets('has right content', (WidgetTester tester) async { + testWidgets('has right title/avatar', (WidgetTester tester) async { final users = usersList(2); final user0 = users[0]; final user1 = users[1]; From 386cfbd0a5b068d304743b507161e748f6926b3a Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 27 Oct 2023 18:31:48 -0400 Subject: [PATCH 4/7] unreads: Add countInDmNarrow method As Greg proposes: https://github.com/zulip/zulip-flutter/pull/334#discussion_r1372455137 --- lib/model/unreads.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/model/unreads.dart b/lib/model/unreads.dart index 477680b745..b10a7587bd 100644 --- a/lib/model/unreads.dart +++ b/lib/model/unreads.dart @@ -123,6 +123,8 @@ class Unreads extends ChangeNotifier { final int selfUserId; + int countInDmNarrow(DmNarrow narrow) => dms[narrow]?.length ?? 0; + void handleMessageEvent(MessageEvent event) { final message = event.message; if (message.flags.contains(MessageFlag.read)) { From 65641a7a9275baeab1709d9872a855b6b28d70e5 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 31 Oct 2023 10:47:19 -0400 Subject: [PATCH 5/7] RecentDmConversationsPage: Make start/end margins symmetric, at 12px As specified by Vlad: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20DM-conversation.20list/near/1672137 --- lib/widgets/recent_dm_conversations.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index 4f43876c24..f157135c38 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -107,7 +107,7 @@ class RecentDmConversationsItem extends StatelessWidget { maxLines: 2, overflow: TextOverflow.ellipsis, title))), - const SizedBox(width: 8), + const SizedBox(width: 12), // TODO(#253): Unread count ]))); } From 36dc4daa69a6338b2cfbba79a88534c0fd5a8f69 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 23 Oct 2023 18:07:10 -0400 Subject: [PATCH 6/7] RecentDmConversationsPage: Show unread counts This follows Vlad's suggestion to adjust the gap between the right edge of the text and the counter, from the Figma's 8px to 12px: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20DM-conversation.20list/near/1672155 Fixes: #328 --- lib/widgets/recent_dm_conversations.dart | 49 ++++++++++++++++-- test/flutter_checks.dart | 4 ++ .../widgets/recent_dm_conversations_test.dart | 50 ++++++++++++++++++- 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index f157135c38..8dccc39ecf 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -1,7 +1,10 @@ +import 'dart:ui'; + import 'package:flutter/material.dart'; import '../model/narrow.dart'; import '../model/recent_dm_conversations.dart'; +import '../model/unreads.dart'; import 'content.dart'; import 'icons.dart'; import 'message_list.dart'; @@ -23,24 +26,30 @@ class RecentDmConversationsPage extends StatefulWidget { class _RecentDmConversationsPageState extends State with PerAccountStoreAwareStateMixin { RecentDmConversationsView? model; + Unreads? unreadsModel; @override void onNewStore() { model?.removeListener(_modelChanged); model = PerAccountStoreWidget.of(context).recentDmConversationsView ..addListener(_modelChanged); + + unreadsModel?.removeListener(_modelChanged); + unreadsModel = PerAccountStoreWidget.of(context).unreads + ..addListener(_modelChanged); } @override void dispose() { model?.removeListener(_modelChanged); + unreadsModel?.removeListener(_modelChanged); super.dispose(); } void _modelChanged() { setState(() { - // The actual state lives in [model]. - // This method was called because that just changed. + // The actual state lives in [model] and [unreadsModel]. + // This method was called because one of those just changed. }); } @@ -51,14 +60,25 @@ class _RecentDmConversationsPageState extends State w appBar: AppBar(title: const Text('Direct messages')), body: ListView.builder( itemCount: sorted.length, - itemBuilder: (context, index) => RecentDmConversationsItem(narrow: sorted[index]))); + itemBuilder: (context, index) { + final narrow = sorted[index]; + return RecentDmConversationsItem( + narrow: narrow, + unreadCount: unreadsModel!.countInDmNarrow(narrow), + ); + })); } } class RecentDmConversationsItem extends StatelessWidget { - const RecentDmConversationsItem({super.key, required this.narrow}); + const RecentDmConversationsItem({ + super.key, + required this.narrow, + required this.unreadCount, + }); final DmNarrow narrow; + final int unreadCount; @override Widget build(BuildContext context) { @@ -108,7 +128,26 @@ class RecentDmConversationsItem extends StatelessWidget { overflow: TextOverflow.ellipsis, title))), const SizedBox(width: 12), - // TODO(#253): Unread count + unreadCount > 0 + ? Padding( + padding: const EdgeInsetsDirectional.only(end: 16), + child: DecoratedBox( + decoration: BoxDecoration( + borderRadius: BorderRadius.circular(3), + color: const Color.fromRGBO(102, 102, 153, 0.15), + ), + child: Padding( + padding: const EdgeInsetsDirectional.fromSTEB(4, 0, 4, 1), + child: Text( + style: const TextStyle( + fontFamily: 'Source Sans 3', + fontSize: 16, + height: (18 / 16), + fontFeatures: [FontFeature.enable('smcp')], // small caps + color: Color(0xFF222222), + ).merge(weightVariableTextStyle(context)), + unreadCount.toString())))) + : const SizedBox(), ]))); } } diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index d6ee6d9b88..6d3dfab777 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -35,6 +35,10 @@ extension ValueNotifierChecks on Subject> { Subject get value => has((c) => c.value, 'value'); } +extension TextChecks on Subject { + Subject get data => has((t) => t.data, 'data'); +} + extension TextStyleChecks on Subject { Subject get inherit => has((t) => t.inherit, 'inherit'); Subject?> get fontVariations => has((t) => t.fontVariations, 'fontVariations'); diff --git a/test/widgets/recent_dm_conversations_test.dart b/test/widgets/recent_dm_conversations_test.dart index 43df5dd955..aa1dedd3ac 100644 --- a/test/widgets/recent_dm_conversations_test.dart +++ b/test/widgets/recent_dm_conversations_test.dart @@ -13,6 +13,7 @@ import 'package:zulip/widgets/recent_dm_conversations.dart'; import 'package:zulip/widgets/store.dart'; import '../example_data.dart' as eg; +import '../flutter_checks.dart'; import '../model/binding.dart'; import '../model/test_store.dart'; import '../test_navigation.dart'; @@ -106,7 +107,7 @@ void main() { }); group('RecentDmConversationsItem', () { - group('appearance', () { + group('content/appearance', () { void checkAvatar(WidgetTester tester, DmNarrow narrow) { final shape = tester.widget( find.descendant( @@ -145,6 +146,26 @@ void main() { } } + Future markMessageAsRead(WidgetTester tester, Message message) async { + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + store.handleEvent(UpdateMessageFlagsAddEvent( + id: 1, flag: MessageFlag.read, all: false, messages: [message.id])); + await tester.pump(); + } + + void checkUnreadCount(WidgetTester tester, int expectedCount) { + final Text? textWidget = tester.widgetList(find.descendant( + of: find.byType(RecentDmConversationsItem), + matching: find.textContaining(RegExp(r'^\d+$'), + ))).singleOrNull; + + if (expectedCount == 0) { + check(textWidget).isNull(); + } else { + check(textWidget).isNotNull().data.equals(expectedCount.toString()); + } + } + group('self-1:1', () { testWidgets('has right title/avatar', (WidgetTester tester) async { final message = eg.dmMessage(from: eg.selfUser, to: []); @@ -169,6 +190,15 @@ void main() { newNameForSelfUser: name); checkTitle(tester, name, 2); }); + + testWidgets('unread counts', (WidgetTester tester) async { + final message = eg.dmMessage(from: eg.selfUser, to: []); + await setupPage(tester, users: [], dmMessages: [message]); + + checkUnreadCount(tester, 1); + await markMessageAsRead(tester, message); + checkUnreadCount(tester, 0); + }); }); group('1:1', () { @@ -206,6 +236,15 @@ void main() { await setupPage(tester, users: [user], dmMessages: [message]); checkTitle(tester, user.fullName, 2); }); + + testWidgets('unread counts', (WidgetTester tester) async { + final message = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser]); + await setupPage(tester, users: [], dmMessages: [message]); + + checkUnreadCount(tester, 1); + await markMessageAsRead(tester, message); + checkUnreadCount(tester, 0); + }); }); group('group', () { @@ -255,6 +294,15 @@ void main() { await setupPage(tester, users: users, dmMessages: [message]); checkTitle(tester, users.map((u) => u.fullName).join(', '), 2); }); + + testWidgets('unread counts', (WidgetTester tester) async { + final message = eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser, eg.otherUser]); + await setupPage(tester, users: [], dmMessages: [message]); + + checkUnreadCount(tester, 1); + await markMessageAsRead(tester, message); + checkUnreadCount(tester, 0); + }); }); }); From 44e6642dddf6bad34a66ef45e1da08acff7d90ef Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 24 Oct 2023 16:02:40 -0400 Subject: [PATCH 7/7] RecentDmConversationsPage [nfc]: Add TODO for muting users --- lib/widgets/recent_dm_conversations.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index 8dccc39ecf..5c437f6f9a 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -92,6 +92,9 @@ class RecentDmConversationsItem extends StatelessWidget { title = selfUser.fullName; avatar = AvatarImage(userId: selfUser.userId); case [var otherUserId]: + // TODO(#296) actually don't show this row if the user is muted? + // (should we offer a "spam folder" style summary screen of recent + // 1:1 DM conversations from muted users?) final otherUser = store.users[otherUserId]; title = otherUser?.fullName ?? '(unknown user)'; avatar = AvatarImage(userId: otherUserId);