From 7766d1d90a20254e6cbf1ed5d97752d934fd8122 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 25 Jun 2024 15:38:33 +0430 Subject: [PATCH 1/3] profile: Distinguish `isBot: true` users with a bot marker --- lib/widgets/profile.dart | 22 ++++++++++++++++++---- test/widgets/profile_test.dart | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/lib/widgets/profile.dart b/lib/widgets/profile.dart index afadc289b0..3414c16852 100644 --- a/lib/widgets/profile.dart +++ b/lib/widgets/profile.dart @@ -7,6 +7,7 @@ import '../api/model/model.dart'; import '../model/content.dart'; import '../model/narrow.dart'; import 'content.dart'; +import 'icons.dart'; import 'message_list.dart'; import 'page.dart'; import 'store.dart'; @@ -46,10 +47,23 @@ class ProfilePage extends StatelessWidget { Center( child: Avatar(userId: userId, size: 200, borderRadius: 200 / 8)), const SizedBox(height: 16), - Text(user.fullName, - textAlign: TextAlign.center, - style: _TextStyles.primaryFieldText - .merge(weightVariableTextStyle(context, wght: 700))), + Row( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + if (user.isBot) ...[ + const Icon( + ZulipIcons.bot, + size: 22, + color: Color.fromARGB(255, 159, 173, 173), + ), + const SizedBox(width: 10), + ], + Flexible(child: Text(user.fullName, + textAlign: TextAlign.center, + style: _TextStyles.primaryFieldText + .merge(weightVariableTextStyle(context, wght: 700)))), + ], + ), // TODO(#291) render email field Text(roleToLabel(user.role, zulipLocalizations), textAlign: TextAlign.center, diff --git a/test/widgets/profile_test.dart b/test/widgets/profile_test.dart index f3243b47f8..827330a2ce 100644 --- a/test/widgets/profile_test.dart +++ b/test/widgets/profile_test.dart @@ -7,6 +7,7 @@ import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/widgets/content.dart'; +import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/profile.dart'; @@ -323,5 +324,37 @@ void main() { check(find.textContaining(longString).evaluate()).length.equals(7); }); + + group('bot vs non-bot users', () { + void checkUser(String fullName, {required bool isBot}) { + final nameFinder = find.text(fullName); + final botFinder = find.byIcon(ZulipIcons.bot); + + check(nameFinder.evaluate()).isNotEmpty(); + if (isBot) { + check(botFinder.evaluate().singleOrNull).isNotNull(); + final botAndNameRowFinder = find.ancestor( + of: botFinder, + matching: find.ancestor(of: nameFinder, matching: find.byType(Row))); + check(botAndNameRowFinder.evaluate().singleOrNull).isNotNull(); + } else { + check(botFinder.evaluate().singleOrNull).isNull(); + } + } + + testWidgets('page builds; bot icon is shown with bot user\'s fullName', (tester) async { + final user = eg.user(isBot: true); + await setupPage(tester, pageUserId: user.userId, users: [user]); + + checkUser(user.fullName, isBot: true); + }); + + testWidgets('page builds; bot icon is not shown with non-bot user\'s fullName', (tester) async { + final user = eg.user(isBot: false); + await setupPage(tester, pageUserId: user.userId, users: [user]); + + checkUser(user.fullName, isBot: false); + }); + }); }); } From 930e269dbd82933f1cea142f8a702dfc3ffa77f7 Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Wed, 3 Jul 2024 14:23:34 +0430 Subject: [PATCH 2/3] recent-dms test [nfc]: Change the `checkTitle` function to match rich text In the next commit, the title of recent-dms item is changed from "Text" to "Text.rich", so this commit will make things ready for that change. --- test/widgets/recent_dm_conversations_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/widgets/recent_dm_conversations_test.dart b/test/widgets/recent_dm_conversations_test.dart index a5a214827d..ec47767efe 100644 --- a/test/widgets/recent_dm_conversations_test.dart +++ b/test/widgets/recent_dm_conversations_test.dart @@ -139,7 +139,8 @@ void main() { // TODO(#232): syntax like `check(find(…), findsOneWidget)` final widget = tester.widget(find.descendant( of: find.byType(RecentDmConversationsItem), - matching: find.text(expectedText), + matching: find.byWidgetPredicate((widget) => widget is Text + && (widget.textSpan?.toPlainText(includePlaceholders: false) ?? '') == expectedText), )); if (expectedLines != null) { final renderObject = tester.renderObject(find.byWidget(widget)); From 5ef3c62d9f632c3a4ba2cf5fb750f6f7d5f2e36c Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Wed, 3 Jul 2024 14:27:41 +0430 Subject: [PATCH 3/3] recent-dms: Distinguish isBot: true users with a bot marker Fixes: #636 --- lib/widgets/recent_dm_conversations.dart | 29 +++- .../widgets/recent_dm_conversations_test.dart | 151 +++++++++++++----- 2 files changed, 136 insertions(+), 44 deletions(-) diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index c5c63b6f01..e2fd61a8e5 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -91,24 +91,29 @@ class RecentDmConversationsItem extends StatelessWidget { final store = PerAccountStoreWidget.of(context); final selfUser = store.users[store.selfUserId]!; - final String title; + final List<({String name, bool isBot})> users; final Widget avatar; switch (narrow.otherRecipientIds) { // TODO dedupe with DM items in [InboxPage] case []: - title = selfUser.fullName; + users = [(name: selfUser.fullName, isBot: selfUser.isBot)]; avatar = AvatarImage(userId: selfUser.userId, size: _avatarSize); 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)'; + users = [(name: otherUser?.fullName ?? '(unknown user)', + isBot: otherUser?.isBot ?? false)]; avatar = AvatarImage(userId: otherUserId, size: _avatarSize); default: // TODO(i18n): List formatting, like you can do in JavaScript: // new Intl.ListFormat('ja').format(['Chris', 'Greg', 'Alya']) // // 'Chris、Greg、Alya' - title = narrow.otherRecipientIds.map((id) => store.users[id]?.fullName ?? '(unknown user)').join(', '); + users = narrow.otherRecipientIds.map((id) { + final user = store.users[id]; + return (name: user?.fullName ?? '(unknown user)', + isBot: user?.isBot ?? false); + }).toList(); // TODO(#95) need dark-theme color avatar = ColoredBox(color: const Color(0x33808080), child: Center( @@ -131,7 +136,7 @@ class RecentDmConversationsItem extends StatelessWidget { const SizedBox(width: 8), Expanded(child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), - child: Text( + child: Text.rich( style: const TextStyle( fontSize: 17, height: (20 / 17), @@ -140,7 +145,19 @@ class RecentDmConversationsItem extends StatelessWidget { ), maxLines: 2, overflow: TextOverflow.ellipsis, - title))), + TextSpan(children: List.generate(users.length, (index) { + final user = users[index]; + return TextSpan(text: user.name, children: [ + if (user.isBot) ...[ + const WidgetSpan(child: SizedBox(width: 5)), + const WidgetSpan( + child: Icon(ZulipIcons.bot, size: 15, + color: Color.fromARGB(255, 159, 173, 173)), + alignment: PlaceholderAlignment.middle), + ], + if (index < users.length - 1) const TextSpan(text: ', '), + ]); + }).toList())))), const SizedBox(width: 12), unreadCount > 0 ? Padding(padding: const EdgeInsetsDirectional.only(end: 16), diff --git a/test/widgets/recent_dm_conversations_test.dart b/test/widgets/recent_dm_conversations_test.dart index ec47767efe..2449b458bf 100644 --- a/test/widgets/recent_dm_conversations_test.dart +++ b/test/widgets/recent_dm_conversations_test.dart @@ -150,6 +150,14 @@ void main() { } } + void checkBotIcon({required int count}) { + final botFinder = find.descendant( + of: find.byType(RecentDmConversationsItem), + matching: find.byIcon(ZulipIcons.bot)).hitTestable(); + + check(botFinder.evaluate().length).equals(count); + } + Future markMessageAsRead(WidgetTester tester, Message message) async { final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); await store.handleEvent(UpdateMessageFlagsAddEvent( @@ -177,6 +185,7 @@ void main() { checkAvatar(tester, DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId)); checkTitle(tester, eg.selfUser.fullName); + checkBotIcon(count: 0); }); testWidgets('short name takes one line', (WidgetTester tester) async { @@ -185,6 +194,7 @@ void main() { await setupPage(tester, users: [], dmMessages: [message], newNameForSelfUser: name); checkTitle(tester, name, 1); + checkBotIcon(count: 0); }); testWidgets('very long name takes two lines (must be ellipsized)', (WidgetTester tester) async { @@ -193,6 +203,7 @@ void main() { await setupPage(tester, users: [], dmMessages: [message], newNameForSelfUser: name); checkTitle(tester, name, 2); + checkBotIcon(count: 0); }); testWidgets('unread counts', (WidgetTester tester) async { @@ -206,39 +217,95 @@ void main() { }); group('1:1', () { - 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]); + group('has right avatar/title', () { + Future checkRecipient(WidgetTester tester, { + required bool isBot, + required bool looksBot + }) async { + final user = eg.user(userId: 1, isBot: isBot); + final message = eg.dmMessage(from: eg.selfUser, to: [user]); + await setupPage(tester, users: [user], dmMessages: [message]); + + checkAvatar(tester, DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId)); + checkTitle(tester, user.fullName); + checkBotIcon(count: looksBot ? 1 : 0); + } - checkAvatar(tester, DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId)); - checkTitle(tester, user.fullName); + testWidgets('bot recipient -> shows bot icon', (tester) async { + await checkRecipient(tester, isBot: true, looksBot: true); + }); + + testWidgets('non-bot recipient -> shows no bot icon', (tester) async { + await checkRecipient(tester, isBot: false, looksBot: false); + }); }); - testWidgets('no error when user somehow missing from store.users', (WidgetTester tester) async { - final user = eg.user(userId: 1); - final message = eg.dmMessage(from: eg.selfUser, to: [user]); - await setupPage(tester, - users: [], // exclude user - dmMessages: [message], - ); + group('no error when user somehow missing from store.users', () { + Future checkRecipient(WidgetTester tester, { + required bool isBot, + required bool looksBot + }) async { + final user = eg.user(userId: 1, isBot: isBot); + final message = eg.dmMessage(from: eg.selfUser, to: [user]); + await setupPage(tester, + users: [], // exclude user + dmMessages: [message], + ); + + checkAvatar(tester, DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId)); + checkTitle(tester, '(unknown user)'); + checkBotIcon(count: looksBot ? 1 : 0); + } - checkAvatar(tester, DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId)); - checkTitle(tester, '(unknown user)'); + testWidgets('bot recipient -> shows no bot icon', (tester) async { + await checkRecipient(tester, isBot: true, looksBot: false); + }); + + testWidgets('non-bot recipient -> shows no bot icon', (tester) async { + await checkRecipient(tester, isBot: false, looksBot: false); + }); }); - testWidgets('short name takes one line', (WidgetTester tester) async { - final user = eg.user(userId: 1, fullName: 'Short name'); - final message = eg.dmMessage(from: eg.selfUser, to: [user]); - await setupPage(tester, users: [user], dmMessages: [message]); - checkTitle(tester, user.fullName, 1); + group('short name takes one line', () { + Future checkRecipient(WidgetTester tester, { + required bool isBot, + required bool looksBot + }) async { + final user = eg.user(userId: 1, fullName: 'Short name', isBot: isBot); + final message = eg.dmMessage(from: eg.selfUser, to: [user]); + await setupPage(tester, users: [user], dmMessages: [message]); + checkTitle(tester, user.fullName, 1); + checkBotIcon(count: looksBot ? 1 : 0); + } + + testWidgets('bot recipient -> shows bot icon', (tester) async { + await checkRecipient(tester, isBot: true, looksBot: true); + }); + + testWidgets('non-bot recipient -> shows no bot icon', (tester) async { + await checkRecipient(tester, isBot: false, looksBot: false); + }); }); - testWidgets('very long name takes two lines (must be ellipsized)', (WidgetTester tester) async { - final user = eg.user(userId: 1, fullName: 'Long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name'); - final message = eg.dmMessage(from: eg.selfUser, to: [user]); - await setupPage(tester, users: [user], dmMessages: [message]); - checkTitle(tester, user.fullName, 2); + group('very long name takes two lines (must be ellipsized)', () { + Future checkRecipient(WidgetTester tester, { + required bool isBot, + required bool looksBot + }) async { + final user = eg.user(userId: 1, isBot: isBot, fullName: 'Long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name long name'); + final message = eg.dmMessage(from: eg.selfUser, to: [user]); + await setupPage(tester, users: [user], dmMessages: [message]); + checkTitle(tester, user.fullName, 2); + checkBotIcon(count: looksBot ? 1 : 0); + } + + testWidgets('bot recipient -> shows no bot icon', (WidgetTester tester) async { + await checkRecipient(tester, isBot: true, looksBot: false); + }); + + testWidgets('non-bot recipient -> shows no bot icon', (WidgetTester tester) async { + await checkRecipient(tester, isBot: false, looksBot: false); + }); }); testWidgets('unread counts', (WidgetTester tester) async { @@ -252,37 +319,43 @@ void main() { }); group('group', () { - List usersList(int count) { + List usersList(int count, {List? botUsers}) { + assert(() { + if (botUsers == null) return true; + for (int userIndex in botUsers) { + if (userIndex >= count) return false; + } + return true; + }()); final result = []; for (int i = 0; i < count; i++) { - result.add(eg.user(userId: i, fullName: 'User ${i.toString()}')); + result.add(eg.user(userId: i, fullName: 'User ${i.toString()}', + isBot: botUsers?.contains(i) ?? false)); } return result; } testWidgets('has right title/avatar', (WidgetTester tester) async { - final users = usersList(2); - final user0 = users[0]; - final user1 = users[1]; - final message = eg.dmMessage(from: eg.selfUser, to: [user0, user1]); + final users = usersList(3, botUsers: [0, 2]); + final message = eg.dmMessage(from: eg.selfUser, to: users); await setupPage(tester, users: users, dmMessages: [message]); checkAvatar(tester, DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId)); - checkTitle(tester, '${user0.fullName}, ${user1.fullName}'); + checkTitle(tester, users.map((u) => u.fullName).join(', ')); + checkBotIcon(count: 2); }); testWidgets('no error when one user somehow missing from store.users', (WidgetTester tester) async { - final users = usersList(2); - final user0 = users[0]; - final user1 = users[1]; - final message = eg.dmMessage(from: eg.selfUser, to: [user0, user1]); + final users = usersList(2, botUsers: [1]); + final message = eg.dmMessage(from: eg.selfUser, to: users); await setupPage(tester, - users: [user0], // exclude user1 + users: [users[0]], // exclude user[1], which is bot dmMessages: [message], ); checkAvatar(tester, DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId)); - checkTitle(tester, '${user0.fullName}, (unknown user)'); + checkTitle(tester, '${users[0].fullName}, (unknown user)'); + checkBotIcon(count: 0); }); testWidgets('few names takes one line', (WidgetTester tester) async { @@ -290,6 +363,7 @@ void main() { final message = eg.dmMessage(from: eg.selfUser, to: users); await setupPage(tester, users: users, dmMessages: [message]); checkTitle(tester, users.map((u) => u.fullName).join(', '), 1); + checkBotIcon(count: 0); }); testWidgets('very many names takes two lines (must be ellipsized)', (WidgetTester tester) async { @@ -297,6 +371,7 @@ void main() { final message = eg.dmMessage(from: eg.selfUser, to: users); await setupPage(tester, users: users, dmMessages: [message]); checkTitle(tester, users.map((u) => u.fullName).join(', '), 2); + checkBotIcon(count: 0); }); testWidgets('unread counts', (WidgetTester tester) async {