From 0dcbc8968545f04edb10652c23694f95e2d33970 Mon Sep 17 00:00:00 2001 From: Abhisheksainii Date: Sun, 16 Feb 2025 13:53:59 +0530 Subject: [PATCH 1/2] message-list : add a prompt to notify guest(s) when DM Fixes #798 showDMWarningBanner could be initialized in the initState to shouldShowGuestUserWarningPrompt(store) and Visibility wrapper could have been gotten rid off , but due to unavailability of store beforehand it was not possible. To create the guests list, I got rid off all the users in the store which are not in the recepients list and role-checked them. --- assets/l10n/app_en.arb | 8 +++ lib/api/model/initial_snapshot.dart | 3 + lib/api/model/initial_snapshot.g.dart | 4 ++ lib/generated/l10n/zulip_localizations.dart | 6 ++ .../l10n/zulip_localizations_ar.dart | 11 +++ .../l10n/zulip_localizations_en.dart | 11 +++ .../l10n/zulip_localizations_ja.dart | 11 +++ .../l10n/zulip_localizations_nb.dart | 11 +++ .../l10n/zulip_localizations_pl.dart | 11 +++ .../l10n/zulip_localizations_ru.dart | 11 +++ .../l10n/zulip_localizations_sk.dart | 11 +++ lib/model/store.dart | 4 ++ lib/widgets/message_list.dart | 69 +++++++++++++++++++ lib/widgets/theme.dart | 14 ++++ pubspec.lock | 20 +++--- test/example_data.dart | 2 + 16 files changed, 197 insertions(+), 10 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index 118ab83c70..043062a426 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -831,5 +831,13 @@ "zulipAppTitle": "Zulip", "@zulipAppTitle": { "description": "The name of Zulip. This should be either 'Zulip' or a transliteration." + }, + "bannerText": "{guestCount, plural, =1{{guestNamesList} is a guest in this organization.} other{{guestNamesList} are guests in this organization.}}", + "@bannerText": { + "description": "Message displaying guest names in the organization.", + "placeholders": { + "guestCount": {"type": "int", "example": "3"}, + "guestNamesList": {"type": "String", "example": "Alice, Bob, Charlie"} + } } } diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart index 1adc44196f..7401f0a2cd 100644 --- a/lib/api/model/initial_snapshot.dart +++ b/lib/api/model/initial_snapshot.dart @@ -79,6 +79,8 @@ class InitialSnapshot { /// https://zulip.com/api/roles-and-permissions#determining-if-a-user-is-a-full-member final int realmWaitingPeriodThreshold; + final bool? realmEnableGuestUserDmWarning; + final Map realmDefaultExternalAccounts; final int maxFileUploadSizeMib; @@ -135,6 +137,7 @@ class InitialSnapshot { required this.realmWildcardMentionPolicy, required this.realmMandatoryTopics, required this.realmWaitingPeriodThreshold, + required this.realmEnableGuestUserDmWarning, required this.realmDefaultExternalAccounts, required this.maxFileUploadSizeMib, required this.serverEmojiDataUrl, diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index a69b6ebafe..796f74c94f 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -64,6 +64,8 @@ InitialSnapshot _$InitialSnapshotFromJson(Map json) => realmMandatoryTopics: json['realm_mandatory_topics'] as bool, realmWaitingPeriodThreshold: (json['realm_waiting_period_threshold'] as num).toInt(), + realmEnableGuestUserDmWarning: + json['realm_enable_guest_user_dm_warning'] as bool?, realmDefaultExternalAccounts: (json['realm_default_external_accounts'] as Map).map( (k, e) => MapEntry( @@ -115,6 +117,8 @@ Map _$InitialSnapshotToJson(InitialSnapshot instance) => 'realm_wildcard_mention_policy': instance.realmWildcardMentionPolicy, 'realm_mandatory_topics': instance.realmMandatoryTopics, 'realm_waiting_period_threshold': instance.realmWaitingPeriodThreshold, + 'realm_enable_guest_user_dm_warning': + instance.realmEnableGuestUserDmWarning, 'realm_default_external_accounts': instance.realmDefaultExternalAccounts, 'max_file_upload_size_mib': instance.maxFileUploadSizeMib, 'server_emoji_data_url': instance.serverEmojiDataUrl?.toString(), diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index 9579683908..58ccefa1a5 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -1214,6 +1214,12 @@ abstract class ZulipLocalizations { /// In en, this message translates to: /// **'Zulip'** String get zulipAppTitle; + + /// Message displaying guest names in the organization. + /// + /// In en, this message translates to: + /// **'{guestCount, plural, =1{{guestNamesList} is a guest in this organization.} other{{guestNamesList} are guests in this organization.}}'** + String bannerText(int guestCount, String guestNamesList); } class _ZulipLocalizationsDelegate extends LocalizationsDelegate { diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index 71bf06d8ce..9f013dfcaf 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -648,4 +648,15 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get zulipAppTitle => 'Zulip'; + + @override + String bannerText(int guestCount, String guestNamesList) { + String _temp0 = intl.Intl.pluralLogic( + guestCount, + locale: localeName, + other: '$guestNamesList are guests in this organization.', + one: '$guestNamesList is a guest in this organization.', + ); + return '$_temp0'; + } } diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index 7a33e33567..6bc91dd96b 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -648,4 +648,15 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get zulipAppTitle => 'Zulip'; + + @override + String bannerText(int guestCount, String guestNamesList) { + String _temp0 = intl.Intl.pluralLogic( + guestCount, + locale: localeName, + other: '$guestNamesList are guests in this organization.', + one: '$guestNamesList is a guest in this organization.', + ); + return '$_temp0'; + } } diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index 137883e5e9..382a0ebce6 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -648,4 +648,15 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get zulipAppTitle => 'Zulip'; + + @override + String bannerText(int guestCount, String guestNamesList) { + String _temp0 = intl.Intl.pluralLogic( + guestCount, + locale: localeName, + other: '$guestNamesList are guests in this organization.', + one: '$guestNamesList is a guest in this organization.', + ); + return '$_temp0'; + } } diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index 3dec7d9b5a..68a2cd1612 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -648,4 +648,15 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get zulipAppTitle => 'Zulip'; + + @override + String bannerText(int guestCount, String guestNamesList) { + String _temp0 = intl.Intl.pluralLogic( + guestCount, + locale: localeName, + other: '$guestNamesList are guests in this organization.', + one: '$guestNamesList is a guest in this organization.', + ); + return '$_temp0'; + } } diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 83a777bfc4..e2a45935bf 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -648,4 +648,15 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get zulipAppTitle => 'Zulip'; + + @override + String bannerText(int guestCount, String guestNamesList) { + String _temp0 = intl.Intl.pluralLogic( + guestCount, + locale: localeName, + other: '$guestNamesList are guests in this organization.', + one: '$guestNamesList is a guest in this organization.', + ); + return '$_temp0'; + } } diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 827aaf0155..c0c17d1ef5 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -648,4 +648,15 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get zulipAppTitle => 'Zulip'; + + @override + String bannerText(int guestCount, String guestNamesList) { + String _temp0 = intl.Intl.pluralLogic( + guestCount, + locale: localeName, + other: '$guestNamesList are guests in this organization.', + one: '$guestNamesList is a guest in this organization.', + ); + return '$_temp0'; + } } diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index 38a3f8a240..30fbb20a9d 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -648,4 +648,15 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get zulipAppTitle => 'Zulip'; + + @override + String bannerText(int guestCount, String guestNamesList) { + String _temp0 = intl.Intl.pluralLogic( + guestCount, + locale: localeName, + other: '$guestNamesList are guests in this organization.', + one: '$guestNamesList is a guest in this organization.', + ); + return '$_temp0'; + } } diff --git a/lib/model/store.dart b/lib/model/store.dart index 611494cc8f..f112311948 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -335,6 +335,7 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess recentDmConversationsView: RecentDmConversationsView( initial: initialSnapshot.recentPrivateConversations, selfUserId: account.userId), recentSenders: RecentSenders(), + realmEnableGuestUserDmWarning: initialSnapshot.realmEnableGuestUserDmWarning, ); } @@ -361,6 +362,7 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess required this.unreads, required this.recentDmConversationsView, required this.recentSenders, + required this.realmEnableGuestUserDmWarning, }) : assert(selfUserId == globalStore.getAccount(accountId)!.userId), assert(realmUrl == globalStore.getAccount(accountId)!.realmUrl), assert(realmUrl == connection.realmUrl), @@ -567,6 +569,8 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess final AutocompleteViewManager autocompleteViewManager = AutocompleteViewManager(); + final bool? realmEnableGuestUserDmWarning; + // End of data. //////////////////////////////////////////////////////////////// diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 06ecff110f..77985a8085 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -183,6 +183,8 @@ abstract class MessageListPageState { /// /// This is null if [MessageList] has not mounted yet. MessageListView? get model; + + bool showDMWarningBanner = true; } class MessageListPage extends StatefulWidget { @@ -224,10 +226,14 @@ class _MessageListPageState extends State implements MessageLis MessageListView? get model => _messageListKey.currentState?.model; final GlobalKey<_MessageListState> _messageListKey = GlobalKey(); + @override + late bool showDMWarningBanner; + @override void initState() { super.initState(); narrow = widget.initNarrow; + showDMWarningBanner = narrow is DmNarrow; } void _narrowChanged(Narrow newNarrow) { @@ -241,6 +247,7 @@ class _MessageListPageState extends State implements MessageLis final store = PerAccountStoreWidget.of(context); final messageListTheme = MessageListTheme.of(context); final zulipLocalizations = ZulipLocalizations.of(context); + final designVariables= DesignVariables.of(context); final Color? appBarBackgroundColor; bool removeAppBarBottomBorder = false; @@ -318,10 +325,72 @@ class _MessageListPageState extends State implements MessageLis narrow: narrow, onNarrowChanged: _narrowChanged, ))), + if(shouldShowGuestUserWarningPrompt(store)) + showGuestUserWarningPrompt(narrow as DmNarrow, store, designVariables, zulipLocalizations), if (ComposeBox.hasComposeBox(narrow)) ComposeBox(key: _composeBoxKey, narrow: narrow) ])))); } + + bool shouldShowGuestUserWarningPrompt(PerAccountStore store) => + store.connection.zulipFeatureLevel! >= 348 && + narrow is DmNarrow && (store.realmEnableGuestUserDmWarning ?? false); + + Widget showGuestUserWarningPrompt( + DmNarrow narrow, + PerAccountStore store, + DesignVariables designVariables, + ZulipLocalizations zulipLocalizations, + ) { + final recipients = narrow.otherRecipientIds; + final allUsersInStore = Map.from(store.users); + allUsersInStore.removeWhere((userId, user) => !recipients.contains(userId)); + final guestNames = + allUsersInStore.values + .where((user) => user.role == UserRole.guest) + .map((e) => e.fullName) + .toList(); + + if(guestNames.isEmpty){ + return SizedBox(); + } + + return Visibility( + visible: showDMWarningBanner, + child: Align( + alignment: Alignment.centerLeft, + child: Container( + width: MediaQuery.of(context).size.width, + decoration: BoxDecoration( + borderRadius: BorderRadius.circular(4), + border: Border.all(color: designVariables.dmUserWarningBannerBorder,), + color: designVariables.dmUserWarningBanner, + ), + margin: EdgeInsets.all(4), + padding: EdgeInsets.symmetric(horizontal: 10, vertical: 8), + child: Row( + mainAxisAlignment: MainAxisAlignment.spaceBetween, + children: [ + Flexible( + child: Text( + zulipLocalizations.bannerText(guestNames.length, guestNames.join(', ')), + style: TextStyle(color: Colors.white), + ), + ), + GestureDetector( + onTap: () { + setState(() { + showDMWarningBanner= false; + }); + }, + child: Icon(Icons.close, color: Colors.white), + ), + ], + ), + ), + ), + ); + } } class MessageListAppBarTitle extends StatelessWidget { diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index ec8ad8aecc..fce0785a28 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -167,6 +167,8 @@ class DesignVariables extends ThemeExtension { subscriptionListHeaderLine: const HSLColor.fromAHSL(0.2, 240, 0.1, 0.5).toColor(), subscriptionListHeaderText: const HSLColor.fromAHSL(1.0, 240, 0.1, 0.5).toColor(), unreadCountBadgeTextForChannel: Colors.black.withValues(alpha: 0.9), + dmUserWarningBanner: Color.fromARGB(255, 133, 118, 71), + dmUserWarningBannerBorder: Color.fromARGB(255, 127, 111, 60), ); static final dark = DesignVariables._( @@ -224,6 +226,8 @@ class DesignVariables extends ThemeExtension { // TODO(design-dark) need proper dark-theme color (this is ad hoc) subscriptionListHeaderText: const HSLColor.fromAHSL(1.0, 240, 0.1, 0.75).toColor(), unreadCountBadgeTextForChannel: Colors.white.withValues(alpha: 0.9), + dmUserWarningBanner: Color.fromARGB(255, 133, 118, 71), + dmUserWarningBannerBorder: Color.fromARGB(255, 127, 111, 60), ); DesignVariables._({ @@ -273,6 +277,8 @@ class DesignVariables extends ThemeExtension { required this.subscriptionListHeaderLine, required this.subscriptionListHeaderText, required this.unreadCountBadgeTextForChannel, + required this.dmUserWarningBanner, + required this.dmUserWarningBannerBorder, }); /// The [DesignVariables] from the context's active theme. @@ -335,6 +341,8 @@ class DesignVariables extends ThemeExtension { final Color subscriptionListHeaderLine; final Color subscriptionListHeaderText; final Color unreadCountBadgeTextForChannel; + final Color dmUserWarningBanner; + final Color dmUserWarningBannerBorder; @override DesignVariables copyWith({ @@ -384,6 +392,8 @@ class DesignVariables extends ThemeExtension { Color? subscriptionListHeaderLine, Color? subscriptionListHeaderText, Color? unreadCountBadgeTextForChannel, + Color? dmUserWarningBanner, + Color? dmUserWarningBannerBorder, }) { return DesignVariables._( background: background ?? this.background, @@ -432,6 +442,8 @@ class DesignVariables extends ThemeExtension { subscriptionListHeaderLine: subscriptionListHeaderLine ?? this.subscriptionListHeaderLine, subscriptionListHeaderText: subscriptionListHeaderText ?? this.subscriptionListHeaderText, unreadCountBadgeTextForChannel: unreadCountBadgeTextForChannel ?? this.unreadCountBadgeTextForChannel, + dmUserWarningBanner: dmUserWarningBanner ?? this.dmUserWarningBanner, + dmUserWarningBannerBorder: dmUserWarningBannerBorder ?? this.dmUserWarningBannerBorder, ); } @@ -487,6 +499,8 @@ class DesignVariables extends ThemeExtension { subscriptionListHeaderLine: Color.lerp(subscriptionListHeaderLine, other.subscriptionListHeaderLine, t)!, subscriptionListHeaderText: Color.lerp(subscriptionListHeaderText, other.subscriptionListHeaderText, t)!, unreadCountBadgeTextForChannel: Color.lerp(unreadCountBadgeTextForChannel, other.unreadCountBadgeTextForChannel, t)!, + dmUserWarningBanner: Color.lerp(dmUserWarningBanner, other.dmUserWarningBanner, t)!, + dmUserWarningBannerBorder: Color.lerp(dmUserWarningBannerBorder, other.dmUserWarningBannerBorder, t)!, ); } } diff --git a/pubspec.lock b/pubspec.lock index 4ee3317f8d..d7c9cb6fa3 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -291,10 +291,10 @@ packages: dependency: "direct dev" description: name: fake_async - sha256: "6a95e56b2449df2273fd8c45a662d6947ce1ebb7aafe80e550a3f68297f3cacc" + sha256: "5368f224a74523e8d2e7399ea1638b37aecfca824a3cc4dfdf77bf1fa905ac44" url: "https://pub.dev" source: hosted - version: "1.3.2" + version: "1.3.3" ffi: dependency: transitive description: @@ -607,10 +607,10 @@ packages: dependency: "direct main" description: name: intl - sha256: d6f56758b7d3014a48af9701c085700aac781a92a87a62b1333b46d8879661cf + sha256: "3df61194eb431efc39c4ceba583b95633a403f46c9fd341e550ce0bfa50e9aa5" url: "https://pub.dev" source: hosted - version: "0.19.0" + version: "0.20.2" io: dependency: transitive description: @@ -647,10 +647,10 @@ packages: dependency: transitive description: name: leak_tracker - sha256: c35baad643ba394b40aac41080300150a4f08fd0fd6a10378f8f7c6bc161acec + sha256: "6bb818ecbdffe216e81182c2f0714a2e62b593f4a4f13098713ff1685dfb6ab0" url: "https://pub.dev" source: hosted - version: "10.0.8" + version: "10.0.9" leak_tracker_flutter_testing: dependency: transitive description: @@ -1244,10 +1244,10 @@ packages: dependency: transitive description: name: vm_service - sha256: "0968250880a6c5fe7edc067ed0a13d4bae1577fe2771dcf3010d52c4a9d3ca14" + sha256: ddfa8d30d89985b96407efce8acbdd124701f96741f2d981ca860662f1c0dc02 url: "https://pub.dev" source: hosted - version: "14.3.1" + version: "15.0.0" wakelock_plus: dependency: "direct main" description: @@ -1300,10 +1300,10 @@ packages: dependency: transitive description: name: webdriver - sha256: "3d773670966f02a646319410766d3b5e1037efb7f07cc68f844d5e06cd4d61c8" + sha256: "2f3a14ca026957870cfd9c635b83507e0e51d8091568e90129fbf805aba7cade" url: "https://pub.dev" source: hosted - version: "3.0.4" + version: "3.1.0" webkit_inspection_protocol: dependency: transitive description: diff --git a/test/example_data.dart b/test/example_data.dart index a758481f52..0ee1658d19 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -909,6 +909,7 @@ InitialSnapshot initialSnapshot({ List? realmUsers, List? realmNonActiveUsers, List? crossRealmBots, + bool? realmEnableGuestUserDmWarning, }) { return InitialSnapshot( queueId: queueId ?? '1:2345', @@ -946,6 +947,7 @@ InitialSnapshot initialSnapshot({ realmUsers: realmUsers ?? [], realmNonActiveUsers: realmNonActiveUsers ?? [], crossRealmBots: crossRealmBots ?? [], + realmEnableGuestUserDmWarning: realmEnableGuestUserDmWarning ?? false ); } const _initialSnapshot = initialSnapshot; From bcbb237a25091827bac368761f539a777ddd02a5 Mon Sep 17 00:00:00 2001 From: Abhisheksainii Date: Sun, 23 Feb 2025 15:46:31 +0530 Subject: [PATCH 2/2] messafe-list: add test for dm guest user warning prompt --- test/widgets/message_list_test.dart | 48 ++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 1aa1af81c5..fb4040bc8d 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -1100,7 +1100,7 @@ void main() { .initNarrow.equals(DmNarrow.withUser(eg.otherUser.userId, selfUserId: eg.selfUser.userId)); await tester.pumpAndSettle(); }); - + testWidgets('does not navigate on tapping recipient header in DmNarrow', (tester) async { final pushedRoutes = >[]; final navObserver = TestNavigatorObserver() @@ -1382,4 +1382,50 @@ void main() { ..status.equals(AnimationStatus.dismissed); }); }); + + group('Guest warning banner', () { + Finder guestUserDmWarningBannerFinder(String label) => + find.textContaining(label); + + void checkGuestUserWarningBanner(WidgetTester tester, String label) { + final state = MessageListPage.ancestorOf( + tester.element(find.text("a message")), + ); + bool shouldShow = + store.connection.zulipFeatureLevel! >= 348 && + state.narrow is DmNarrow && + (store.realmEnableGuestUserDmWarning ?? false); + + check( + guestUserDmWarningBannerFinder(label).evaluate().length, + ).equals(shouldShow ? 1 : 0); + } + + void checkDmGuestUserWarning(WidgetTester tester, String label) => + checkGuestUserWarningBanner(tester, label); + + testWidgets('guest user DM warning banner text', (tester) async { + // check if DM warning banner shows text + // if the recipient is a guest user + final zulipLocalizations = GlobalLocalizations.zulipLocalizations; + await setupMessageListPage( + tester, + users: [ + eg.user(userId: 1, fullName: 'User 1') + ], + narrow: DmNarrow( + allRecipientIds: [eg.selfUser.userId], + selfUserId: eg.selfUser.userId, + ), + messages: [eg.streamMessage(content: "

a message

")], + ); + checkDmGuestUserWarning( + tester, + zulipLocalizations.bannerText( + 1, + 'User 1', + ), + ); + }); + }); }