Merge remote-tracking branch 'pr/995'

gnprice · gnprice · commit 80257795b830 · 2024-12-11T17:24:41.000-08:00
diff --git a/lib/model/store.dart b/lib/model/store.dart
@@ -456,6 +456,33 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess
     return byDate.difference(dateJoined).inDays >= realmWaitingPeriodThreshold;
   }
 
+  /// The given user's real email address, if known, for displaying in the UI.
+  ///
+  /// Returns null if self-user isn't able to see [user]'s real email address.
+  String? userDisplayEmail(User user, {required PerAccountStore store}) {
+    if (store.account.zulipFeatureLevel >= 163) { // TODO(server-7)
+      // A non-null value means self-user has access to [user]'s real email,
+      // while a null value means it doesn't have access to the email.
+      // Search for "delivery_email" in https://zulip.com/api/register-queue.
+      return user.deliveryEmail;
+    } else {
+      if (user.deliveryEmail != null) {
+        // A non-null value means self-user has access to [user]'s real email,
+        // while a null value doesn't necessarily mean it doesn't have access
+        // to the email, ....
+        return user.deliveryEmail;
+      } else if (store.emailAddressVisibility == EmailAddressVisibility.everyone) {
+        // ... we have to also check for [PerAccountStore.emailAddressVisibility].
+        // See:
+        //   * https://github.com/zulip/zulip-mobile/pull/5515#discussion_r997731727
+        //   * https://chat.zulip.org/#narrow/stream/378-api-design/topic/email.20address.20visibility/near/1296133
+        return user.email;
+      } else {
+        return null;
+      }
+    }
+  }
+
   ////////////////////////////////
   // Streams, topics, and stuff about them.
 
diff --git a/lib/widgets/autocomplete.dart b/lib/widgets/autocomplete.dart
@@ -8,6 +8,8 @@ import '../model/autocomplete.dart';
 import '../model/compose.dart';
 import '../model/narrow.dart';
 import 'compose_box.dart';
+import 'text.dart';
+import 'theme.dart';
 
 abstract class AutocompleteField<QueryT extends AutocompleteQuery, ResultT extends AutocompleteResult> extends StatefulWidget {
   const AutocompleteField({
@@ -210,6 +212,7 @@ class ComposeAutocomplete extends AutocompleteField<ComposeAutocompleteQuery, Co
 
   @override
   Widget buildItem(BuildContext context, int index, ComposeAutocompleteResult option) {
+    final designVariables = DesignVariables.of(context);
     final child = switch (option) {
       MentionAutocompleteResult() => _MentionAutocompleteItem(option: option),
       EmojiAutocompleteResult() => _EmojiAutocompleteItem(option: option),
@@ -218,6 +221,9 @@ class ComposeAutocomplete extends AutocompleteField<ComposeAutocompleteQuery, Co
       onTap: () {
         _onTapOption(context, option);
       },
+      highlightColor: designVariables.editorButtonPressedBg,
+      splashFactory: NoSplash.splashFactory,
+      borderRadius: BorderRadius.circular(5),
       child: child);
   }
 }
@@ -229,20 +235,49 @@ class _MentionAutocompleteItem extends StatelessWidget {
 
   @override
   Widget build(BuildContext context) {
+    final designVariables = DesignVariables.of(context);
+
     Widget avatar;
     String label;
+    String? subLabel;
     switch (option) {
       case UserMentionAutocompleteResult(:var userId):
-        avatar = Avatar(userId: userId, size: 32, borderRadius: 3);
-        label = PerAccountStoreWidget.of(context).users[userId]!.fullName;
+        final store = PerAccountStoreWidget.of(context);
+        final user = store.users[userId]!;
+        avatar = Avatar(userId: userId, size: 36, borderRadius: 4);
+        label = user.fullName;
+        subLabel = store.userDisplayEmail(user, store: store);
     }
 
     return Padding(
-      padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
+      padding: const EdgeInsetsDirectional.fromSTEB(4, 4, 8, 4),
       child: Row(children: [
         avatar,
-        const SizedBox(width: 8),
-        Text(label),
+        const SizedBox(width: 6),
+        Expanded(child: Column(
+          mainAxisSize: MainAxisSize.min,
+          crossAxisAlignment: CrossAxisAlignment.start,
+          children: [
+            Text(
+              style: TextStyle(
+                fontSize: 18,
+                height: 20 / 18,
+                color: designVariables.contextMenuItemLabel,
+              )
+                .merge(weightVariableTextStyle(context, wght: 600)),
+              overflow: TextOverflow.ellipsis,
+              maxLines: 1,
+              label),
+            if (subLabel != null) Text(
+              style: TextStyle(
+                fontSize: 14,
+                height: 16 / 14,
+                color: designVariables.contextMenuItemMeta,
+              ),
+              overflow: TextOverflow.ellipsis,
+              maxLines: 1,
+              subLabel),
+          ])),
       ]));
   }
 }
@@ -252,12 +287,13 @@ class _EmojiAutocompleteItem extends StatelessWidget {
 
   final EmojiAutocompleteResult option;
 
-  static const _size = 32.0;
-  static const _notoColorEmojiTextSize = 25.7;
+  static const _size = 24.0;
+  static const _notoColorEmojiTextSize = 19.3;
 
   @override
   Widget build(BuildContext context) {
     final store = PerAccountStoreWidget.of(context);
+    final designVariables = DesignVariables.of(context);
     final candidate = option.candidate;
 
     // TODO deduplicate this logic with [EmojiPickerListEntry]
@@ -276,15 +312,31 @@ class _EmojiAutocompleteItem extends StatelessWidget {
       ? candidate.emojiName
       : [candidate.emojiName, ...candidate.aliases].join(", "); // TODO(#1080)
 
+    // TODO(design): emoji autocomplete results
+    // There's no design in Figma for emoji autocomplete results.
+    // Instead we adapt the design for the emoji picker to the
+    // context of autocomplete results as exemplified by _MentionAutocompleteItem.
+    // That means: emoji size, text size, text line-height from emoji picker;
+    // text color (for contrast with background) and outer padding
+    // from _MentionAutocompleteItem; padding around emoji glyph
+    // to bring it to same size as avatar in _MentionAutocompleteItem.
     return Padding(
-      padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
+      padding: const EdgeInsetsDirectional.fromSTEB(4, 4, 8, 4),
       child: Row(children: [
         if (glyph != null) ...[
-          glyph,
-          const SizedBox(width: 8),
+          Padding(padding: const EdgeInsets.all(6),
+            child: glyph),
+          const SizedBox(width: 6),
         ],
         Expanded(
           child: Text(
+            // The Figma design for the emoji picker actually calls for the
+            // line-height to be 24px when the label fits on one line,
+            // and 18px when it wraps.  But whether it'll wrap is something we
+            // don't know at build time, so make it always 18px.  Discussion:
+            //   https://github.com/zulip/zulip-flutter/pull/995#discussion_r1868352275
+            style: TextStyle(fontSize: 17, height: 18 / 17,
+              color: designVariables.contextMenuItemLabel),
             maxLines: 2,
             overflow: TextOverflow.ellipsis,
             label)),
diff --git a/lib/widgets/profile.dart b/lib/widgets/profile.dart
@@ -2,12 +2,10 @@ import 'dart:convert';
 
 import 'package:flutter/material.dart';
 
-import '../api/model/initial_snapshot.dart';
 import '../api/model/model.dart';
 import '../generated/l10n/zulip_localizations.dart';
 import '../model/content.dart';
 import '../model/narrow.dart';
-import '../model/store.dart';
 import 'app_bar.dart';
 import 'content.dart';
 import 'message_list.dart';
@@ -36,32 +34,6 @@ class ProfilePage extends StatelessWidget {
       page: ProfilePage(userId: userId));
   }
 
-  /// The given user's real email address, if known, for displaying in the UI.
-  ///
-  /// Returns null if self-user isn't able to see [user]'s real email address.
-  String? _getDisplayEmailFor(User user, {required PerAccountStore store}) {
-    if (store.account.zulipFeatureLevel >= 163) { // TODO(server-7)
-      // A non-null value means self-user has access to [user]'s real email,
-      // while a null value means it doesn't have access to the email.
-      // Search for "delivery_email" in https://zulip.com/api/register-queue.
-      return user.deliveryEmail;
-    } else {
-      if (user.deliveryEmail != null) {
-        // A non-null value means self-user has access to [user]'s real email,
-        // while a null value doesn't necessarily mean it doesn't have access
-        // to the email, ....
-        return user.deliveryEmail;
-      } else if (store.emailAddressVisibility == EmailAddressVisibility.everyone) {
-        // ... we have to also check for [PerAccountStore.emailAddressVisibility].
-        // See:
-        //   * https://github.com/zulip/zulip-mobile/pull/5515#discussion_r997731727
-        //   * https://chat.zulip.org/#narrow/stream/378-api-design/topic/email.20address.20visibility/near/1296133
-        return user.email;
-      } else {
-        return null;
-      }
-    }
-  }
 
   @override
   Widget build(BuildContext context) {
@@ -72,7 +44,7 @@ class ProfilePage extends StatelessWidget {
       return const _ProfileErrorPage();
     }
 
-    final displayEmail = _getDisplayEmailFor(user, store: store);
+    final displayEmail = store.userDisplayEmail(user, store: store);
     final items = [
       Center(
         child: Avatar(userId: userId, size: 200, borderRadius: 200 / 8)),
diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart
@@ -134,6 +134,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
       composeBoxBg: const Color(0xffffffff),
       contextMenuCancelText: const Color(0xff222222),
       contextMenuItemBg: const Color(0xff6159e1),
+      contextMenuItemLabel: const Color(0xff242631),
+      contextMenuItemMeta: const Color(0xff626573),
       contextMenuItemText: const Color(0xff381da7),
       editorButtonPressedBg: Colors.black.withValues(alpha: 0.06),
       foreground: const Color(0xff000000),
@@ -184,6 +186,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
       composeBoxBg: const Color(0xff0f0f0f),
       contextMenuCancelText: const Color(0xffffffff).withValues(alpha: 0.75),
       contextMenuItemBg: const Color(0xff7977fe),
+      contextMenuItemLabel: const Color(0xffdfe1e8),
+      contextMenuItemMeta: const Color(0xff9194a3),
       contextMenuItemText: const Color(0xff9398fd),
       editorButtonPressedBg: Colors.white.withValues(alpha: 0.06),
       foreground: const Color(0xffffffff),
@@ -241,6 +245,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
     required this.composeBoxBg,
     required this.contextMenuCancelText,
     required this.contextMenuItemBg,
+    required this.contextMenuItemLabel,
+    required this.contextMenuItemMeta,
     required this.contextMenuItemText,
     required this.editorButtonPressedBg,
     required this.foreground,
@@ -299,6 +305,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
   final Color composeBoxBg;
   final Color contextMenuCancelText;
   final Color contextMenuItemBg;
+  final Color contextMenuItemLabel;
+  final Color contextMenuItemMeta;
   final Color contextMenuItemText;
   final Color editorButtonPressedBg;
   final Color foreground;
@@ -352,6 +360,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
     Color? composeBoxBg,
     Color? contextMenuCancelText,
     Color? contextMenuItemBg,
+    Color? contextMenuItemLabel,
+    Color? contextMenuItemMeta,
     Color? contextMenuItemText,
     Color? editorButtonPressedBg,
     Color? foreground,
@@ -400,7 +410,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
       composeBoxBg: composeBoxBg ?? this.composeBoxBg,
       contextMenuCancelText: contextMenuCancelText ?? this.contextMenuCancelText,
       contextMenuItemBg: contextMenuItemBg ?? this.contextMenuItemBg,
-      contextMenuItemText: contextMenuItemText ?? this.contextMenuItemBg,
+      contextMenuItemLabel: contextMenuItemLabel ?? this.contextMenuItemLabel,
+      contextMenuItemMeta: contextMenuItemMeta ?? this.contextMenuItemMeta,
+      contextMenuItemText: contextMenuItemText ?? this.contextMenuItemText,
       editorButtonPressedBg: editorButtonPressedBg ?? this.editorButtonPressedBg,
       foreground: foreground ?? this.foreground,
       icon: icon ?? this.icon,
@@ -455,7 +467,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
       composeBoxBg: Color.lerp(composeBoxBg, other.composeBoxBg, t)!,
       contextMenuCancelText: Color.lerp(contextMenuCancelText, other.contextMenuCancelText, t)!,
       contextMenuItemBg: Color.lerp(contextMenuItemBg, other.contextMenuItemBg, t)!,
-      contextMenuItemText: Color.lerp(contextMenuItemText, other.contextMenuItemBg, t)!,
+      contextMenuItemLabel: Color.lerp(contextMenuItemLabel, other.contextMenuItemLabel, t)!,
+      contextMenuItemMeta: Color.lerp(contextMenuItemMeta, other.contextMenuItemMeta, t)!,
+      contextMenuItemText: Color.lerp(contextMenuItemText, other.contextMenuItemText, t)!,
       editorButtonPressedBg: Color.lerp(editorButtonPressedBg, other.editorButtonPressedBg, t)!,
       foreground: Color.lerp(foreground, other.foreground, t)!,
       icon: Color.lerp(icon, other.icon, t)!,
diff --git a/test/widgets/autocomplete_test.dart b/test/widgets/autocomplete_test.dart
@@ -13,6 +13,7 @@ import 'package:zulip/model/localizations.dart';
 import 'package:zulip/model/narrow.dart';
 import 'package:zulip/model/store.dart';
 import 'package:zulip/model/typing_status.dart';
+import 'package:zulip/widgets/content.dart';
 import 'package:zulip/widgets/message_list.dart';
 
 import '../api/fake_api.dart';
@@ -126,17 +127,22 @@ void main() {
   TestZulipBinding.ensureInitialized();
 
   group('@-mentions', () {
-    void checkUserShown(User user, PerAccountStore store, {required bool expected}) {
+
+    Finder findAvatarImage(int userId) =>
+      find.byWidgetPredicate((widget) => widget is AvatarImage && widget.userId == userId);
+
+    void checkUserShown(User user, {required bool expected, bool? deliveryEmailExpected}) {
+      deliveryEmailExpected ??= expected;
       check(find.text(user.fullName).evaluate().length).equals(expected ? 1 : 0);
-      final avatarFinder =
-        findNetworkImage(store.tryResolveUrl(user.avatarUrl!).toString());
+      check(find.text(user.deliveryEmail?? "").evaluate().length).equals(deliveryEmailExpected ? 1 : 0);
+      final avatarFinder = findAvatarImage(user.userId);
       check(avatarFinder.evaluate().length).equals(expected ? 1 : 0);
     }
 
     testWidgets('options appear, disappear, and change correctly', (tester) async {
-      final user1 = eg.user(userId: 1, fullName: 'User One', avatarUrl: 'user1.png');
-      final user2 = eg.user(userId: 2, fullName: 'User Two', avatarUrl: 'user2.png');
-      final user3 = eg.user(userId: 3, fullName: 'User Three', avatarUrl: 'user3.png');
+      final user1 = eg.user(userId: 1, fullName: 'User One', avatarUrl: 'user1.png',deliveryEmail: 'email1@email.com');
+      final user2 = eg.user(userId: 2, fullName: 'User Two', avatarUrl: 'user2.png', deliveryEmail: 'email2@email.com');
+      final user3 = eg.user(userId: 3, fullName: 'User Three', avatarUrl: 'user3.png', deliveryEmail: 'email3@email.com');
       final composeInputFinder = await setupToComposeInput(tester, users: [user1, user2, user3]);
       final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
 
@@ -147,33 +153,54 @@ void main() {
       await tester.pumpAndSettle(); // async computation; options appear
 
       // "User Two" and "User Three" appear, but not "User One"
-      checkUserShown(user1, store, expected: false);
-      checkUserShown(user2, store, expected: true);
-      checkUserShown(user3, store, expected: true);
+      checkUserShown(user1, expected: false);
+      checkUserShown(user2, expected: true);
+      checkUserShown(user3, expected: true);
 
       // Finishing autocomplete updates compose box; causes options to disappear
       await tester.tap(find.text('User Three'));
       await tester.pump();
       check(tester.widget<TextField>(composeInputFinder).controller!.text)
         .contains(mention(user3, users: store.users));
-      checkUserShown(user1, store, expected: false);
-      checkUserShown(user2, store, expected: false);
-      checkUserShown(user3, store, expected: false);
+      checkUserShown(user1, expected: false);
+      checkUserShown(user2, expected: false);
+      checkUserShown(user3, expected: false);
 
       // Then a new autocomplete intent brings up options again
       // TODO(#226): Remove this extra edit when this bug is fixed.
       await tester.enterText(composeInputFinder, 'hello @user tw');
       await tester.enterText(composeInputFinder, 'hello @user two');
       await tester.pumpAndSettle(); // async computation; options appear
-      checkUserShown(user2, store, expected: true);
+      checkUserShown(user2, expected: true);
 
       // Removing autocomplete intent causes options to disappear
       // TODO(#226): Remove one of these edits when this bug is fixed.
       await tester.enterText(composeInputFinder, '');
       await tester.enterText(composeInputFinder, ' ');
-      checkUserShown(user1, store, expected: false);
-      checkUserShown(user2, store, expected: false);
-      checkUserShown(user3, store, expected: false);
+      checkUserShown(user1, expected: false);
+      checkUserShown(user2, expected: false);
+      checkUserShown(user3, expected: false);
+
+      debugNetworkImageHttpClientProvider = null;
+    });
+
+    testWidgets('test delivery email visibility', (tester) async {
+      final user2 = eg.user(userId: 2, fullName: 'User Two', avatarUrl: 'user2.png',);
+      final user3 = eg.user(userId: 3, fullName: 'User Three', avatarUrl: 'user3.png', deliveryEmail: 'email3@email.com');
+      final composeInputFinder = await setupToComposeInput(tester, users: [user2, user3]);
+
+      TypingNotifier.debugEnable = false;
+      addTearDown(TypingNotifier.debugReset);
+
+      // Options are filtered correctly for query
+      // TODO(#226): Remove this extra edit when this bug is fixed.
+      await tester.enterText(composeInputFinder, 'hello @user ');
+      await tester.enterText(composeInputFinder, 'hello @user t');
+      await tester.pumpAndSettle(); // async computation; options appear
+
+      // "User Two"'s delivery email is not visible and "User Three"'s delivery email is visible
+      checkUserShown(user2, expected: true, deliveryEmailExpected: false);
+      checkUserShown(user3, expected: true, deliveryEmailExpected: true);
 
       debugNetworkImageHttpClientProvider = null;
     });
