Skip to content

Commit e677f8d

Browse files
gnpricegnprice
committed
Merge remote-tracking branch 'pr/995'
2 parents e1177e8 + 87d149c commit e677f8d

File tree

5 files changed

+149
-57
lines changed

5 files changed

+149
-57
lines changed

‎lib/model/store.dart

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,33 @@ class PerAccountStore extends ChangeNotifier with EmojiStore, ChannelStore, Mess
456456
return byDate.difference(dateJoined).inDays >= realmWaitingPeriodThreshold;
457457
}
458458

459+
/// The given user's real email address, if known, for displaying in the UI.
460+
///
461+
/// Returns null if self-user isn't able to see [user]'s real email address.
462+
String? userDisplayEmail(User user, {required PerAccountStore store}) {
463+
if (store.account.zulipFeatureLevel >= 163) { // TODO(server-7)
464+
// A non-null value means self-user has access to [user]'s real email,
465+
// while a null value means it doesn't have access to the email.
466+
// Search for "delivery_email" in https://zulip.com/api/register-queue.
467+
return user.deliveryEmail;
468+
} else {
469+
if (user.deliveryEmail != null) {
470+
// A non-null value means self-user has access to [user]'s real email,
471+
// while a null value doesn't necessarily mean it doesn't have access
472+
// to the email, ....
473+
return user.deliveryEmail;
474+
} else if (store.emailAddressVisibility == EmailAddressVisibility.everyone) {
475+
// ... we have to also check for [PerAccountStore.emailAddressVisibility].
476+
// See:
477+
// * https://github.com/zulip/zulip-mobile/pull/5515#discussion_r997731727
478+
// * https://chat.zulip.org/#narrow/stream/378-api-design/topic/email.20address.20visibility/near/1296133
479+
return user.email;
480+
} else {
481+
return null;
482+
}
483+
}
484+
}
485+
459486
////////////////////////////////
460487
// Streams, topics, and stuff about them.
461488

‎lib/widgets/autocomplete.dart

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import '../model/autocomplete.dart';
88
import '../model/compose.dart';
99
import '../model/narrow.dart';
1010
import 'compose_box.dart';
11+
import 'text.dart';
12+
import 'theme.dart';
1113

1214
abstract class AutocompleteField<QueryT extends AutocompleteQuery, ResultT extends AutocompleteResult> extends StatefulWidget {
1315
const AutocompleteField({
@@ -210,6 +212,7 @@ class ComposeAutocomplete extends AutocompleteField<ComposeAutocompleteQuery, Co
210212

211213
@override
212214
Widget buildItem(BuildContext context, int index, ComposeAutocompleteResult option) {
215+
final designVariables = DesignVariables.of(context);
213216
final child = switch (option) {
214217
MentionAutocompleteResult() => _MentionAutocompleteItem(option: option),
215218
EmojiAutocompleteResult() => _EmojiAutocompleteItem(option: option),
@@ -218,6 +221,9 @@ class ComposeAutocomplete extends AutocompleteField<ComposeAutocompleteQuery, Co
218221
onTap: () {
219222
_onTapOption(context, option);
220223
},
224+
highlightColor: designVariables.editorButtonPressedBg,
225+
splashFactory: NoSplash.splashFactory,
226+
borderRadius: BorderRadius.circular(5),
221227
child: child);
222228
}
223229
}
@@ -229,20 +235,49 @@ class _MentionAutocompleteItem extends StatelessWidget {
229235

230236
@override
231237
Widget build(BuildContext context) {
238+
final designVariables = DesignVariables.of(context);
239+
232240
Widget avatar;
233241
String label;
242+
String? subLabel;
234243
switch (option) {
235244
case UserMentionAutocompleteResult(:var userId):
236-
avatar = Avatar(userId: userId, size: 32, borderRadius: 3);
237-
label = PerAccountStoreWidget.of(context).users[userId]!.fullName;
245+
final store = PerAccountStoreWidget.of(context);
246+
final user = store.users[userId]!;
247+
avatar = Avatar(userId: userId, size: 36, borderRadius: 4);
248+
label = user.fullName;
249+
subLabel = store.userDisplayEmail(user, store: store);
238250
}
239251

240252
return Padding(
241-
padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
253+
padding: const EdgeInsetsDirectional.fromSTEB(4, 4, 8, 4),
242254
child: Row(children: [
243255
avatar,
244-
const SizedBox(width: 8),
245-
Text(label),
256+
const SizedBox(width: 6),
257+
Expanded(child: Column(
258+
mainAxisSize: MainAxisSize.min,
259+
crossAxisAlignment: CrossAxisAlignment.start,
260+
children: [
261+
Text(
262+
style: TextStyle(
263+
fontSize: 18,
264+
height: 20 / 18,
265+
color: designVariables.contextMenuItemLabel,
266+
)
267+
.merge(weightVariableTextStyle(context, wght: 600)),
268+
overflow: TextOverflow.ellipsis,
269+
maxLines: 1,
270+
label),
271+
if (subLabel != null) Text(
272+
style: TextStyle(
273+
fontSize: 14,
274+
height: 16 / 14,
275+
color: designVariables.contextMenuItemMeta,
276+
),
277+
overflow: TextOverflow.ellipsis,
278+
maxLines: 1,
279+
subLabel),
280+
])),
246281
]));
247282
}
248283
}
@@ -252,12 +287,13 @@ class _EmojiAutocompleteItem extends StatelessWidget {
252287

253288
final EmojiAutocompleteResult option;
254289

255-
static const _size = 32.0;
256-
static const _notoColorEmojiTextSize = 25.7;
290+
static const _size = 24.0;
291+
static const _notoColorEmojiTextSize = 19.3;
257292

258293
@override
259294
Widget build(BuildContext context) {
260295
final store = PerAccountStoreWidget.of(context);
296+
final designVariables = DesignVariables.of(context);
261297
final candidate = option.candidate;
262298

263299
final emojiDisplay = candidate.emojiDisplay.resolve(store.userSettings);
@@ -275,15 +311,31 @@ class _EmojiAutocompleteItem extends StatelessWidget {
275311
? candidate.emojiName
276312
: [candidate.emojiName, ...candidate.aliases].join(", "); // TODO(#1080)
277313

314+
// TODO(design): emoji autocomplete results
315+
// There's no design in Figma for emoji autocomplete results.
316+
// Instead we adapt the design for the emoji picker to the
317+
// context of autocomplete results as exemplified by _MentionAutocompleteItem.
318+
// That means: emoji size, text size, text line-height from emoji picker;
319+
// text color (for contrast with background) and outer padding
320+
// from _MentionAutocompleteItem; padding around emoji glyph
321+
// to bring it to same size as avatar in _MentionAutocompleteItem.
278322
return Padding(
279-
padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
323+
padding: const EdgeInsetsDirectional.fromSTEB(4, 4, 8, 4),
280324
child: Row(children: [
281325
if (glyph != null) ...[
282-
glyph,
283-
const SizedBox(width: 8),
326+
Padding(padding: const EdgeInsets.all(6),
327+
child: glyph),
328+
const SizedBox(width: 6),
284329
],
285330
Expanded(
286331
child: Text(
332+
// The Figma design for the emoji picker actually calls for the
333+
// line-height to be 24px when the label fits on one line,
334+
// and 18px when it wraps. But whether it'll wrap is something we
335+
// don't know at build time, so make it always 18px. Discussion:
336+
// https://github.com/zulip/zulip-flutter/pull/995#discussion_r1868352275
337+
style: TextStyle(fontSize: 17, height: 18 / 17,
338+
color: designVariables.contextMenuItemLabel),
287339
maxLines: 2,
288340
overflow: TextOverflow.ellipsis,
289341
label)),

‎lib/widgets/profile.dart

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,10 @@ import 'dart:convert';
22

33
import 'package:flutter/material.dart';
44

5-
import '../api/model/initial_snapshot.dart';
65
import '../api/model/model.dart';
76
import '../generated/l10n/zulip_localizations.dart';
87
import '../model/content.dart';
98
import '../model/narrow.dart';
10-
import '../model/store.dart';
119
import 'app_bar.dart';
1210
import 'content.dart';
1311
import 'message_list.dart';
@@ -36,32 +34,6 @@ class ProfilePage extends StatelessWidget {
3634
page: ProfilePage(userId: userId));
3735
}
3836

39-
/// The given user's real email address, if known, for displaying in the UI.
40-
///
41-
/// Returns null if self-user isn't able to see [user]'s real email address.
42-
String? _getDisplayEmailFor(User user, {required PerAccountStore store}) {
43-
if (store.account.zulipFeatureLevel >= 163) { // TODO(server-7)
44-
// A non-null value means self-user has access to [user]'s real email,
45-
// while a null value means it doesn't have access to the email.
46-
// Search for "delivery_email" in https://zulip.com/api/register-queue.
47-
return user.deliveryEmail;
48-
} else {
49-
if (user.deliveryEmail != null) {
50-
// A non-null value means self-user has access to [user]'s real email,
51-
// while a null value doesn't necessarily mean it doesn't have access
52-
// to the email, ....
53-
return user.deliveryEmail;
54-
} else if (store.emailAddressVisibility == EmailAddressVisibility.everyone) {
55-
// ... we have to also check for [PerAccountStore.emailAddressVisibility].
56-
// See:
57-
// * https://github.com/zulip/zulip-mobile/pull/5515#discussion_r997731727
58-
// * https://chat.zulip.org/#narrow/stream/378-api-design/topic/email.20address.20visibility/near/1296133
59-
return user.email;
60-
} else {
61-
return null;
62-
}
63-
}
64-
}
6537

6638
@override
6739
Widget build(BuildContext context) {
@@ -72,7 +44,7 @@ class ProfilePage extends StatelessWidget {
7244
return const _ProfileErrorPage();
7345
}
7446

75-
final displayEmail = _getDisplayEmailFor(user, store: store);
47+
final displayEmail = store.userDisplayEmail(user, store: store);
7648
final items = [
7749
Center(
7850
child: Avatar(userId: userId, size: 200, borderRadius: 200 / 8)),

‎lib/widgets/theme.dart

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
132132
composeBoxBg: const Color(0xffffffff),
133133
contextMenuCancelText: const Color(0xff222222),
134134
contextMenuItemBg: const Color(0xff6159e1),
135+
contextMenuItemLabel: const Color(0xff242631),
136+
contextMenuItemMeta: const Color(0xff626573),
135137
contextMenuItemText: const Color(0xff381da7),
136138
editorButtonPressedBg: Colors.black.withValues(alpha: 0.06),
137139
foreground: const Color(0xff000000),
@@ -181,6 +183,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
181183
composeBoxBg: const Color(0xff0f0f0f),
182184
contextMenuCancelText: const Color(0xffffffff).withValues(alpha: 0.75),
183185
contextMenuItemBg: const Color(0xff7977fe),
186+
contextMenuItemLabel: const Color(0xffdfe1e8),
187+
contextMenuItemMeta: const Color(0xff9194a3),
184188
contextMenuItemText: const Color(0xff9398fd),
185189
editorButtonPressedBg: Colors.white.withValues(alpha: 0.06),
186190
foreground: const Color(0xffffffff),
@@ -237,6 +241,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
237241
required this.composeBoxBg,
238242
required this.contextMenuCancelText,
239243
required this.contextMenuItemBg,
244+
required this.contextMenuItemLabel,
245+
required this.contextMenuItemMeta,
240246
required this.contextMenuItemText,
241247
required this.editorButtonPressedBg,
242248
required this.foreground,
@@ -294,6 +300,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
294300
final Color composeBoxBg;
295301
final Color contextMenuCancelText;
296302
final Color contextMenuItemBg;
303+
final Color contextMenuItemLabel;
304+
final Color contextMenuItemMeta;
297305
final Color contextMenuItemText;
298306
final Color editorButtonPressedBg;
299307
final Color foreground;
@@ -346,6 +354,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
346354
Color? composeBoxBg,
347355
Color? contextMenuCancelText,
348356
Color? contextMenuItemBg,
357+
Color? contextMenuItemLabel,
358+
Color? contextMenuItemMeta,
349359
Color? contextMenuItemText,
350360
Color? editorButtonPressedBg,
351361
Color? foreground,
@@ -393,7 +403,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
393403
composeBoxBg: composeBoxBg ?? this.composeBoxBg,
394404
contextMenuCancelText: contextMenuCancelText ?? this.contextMenuCancelText,
395405
contextMenuItemBg: contextMenuItemBg ?? this.contextMenuItemBg,
396-
contextMenuItemText: contextMenuItemText ?? this.contextMenuItemBg,
406+
contextMenuItemLabel: contextMenuItemLabel ?? this.contextMenuItemLabel,
407+
contextMenuItemMeta: contextMenuItemMeta ?? this.contextMenuItemMeta,
408+
contextMenuItemText: contextMenuItemText ?? this.contextMenuItemText,
397409
editorButtonPressedBg: editorButtonPressedBg ?? this.editorButtonPressedBg,
398410
foreground: foreground ?? this.foreground,
399411
icon: icon ?? this.icon,
@@ -447,7 +459,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
447459
composeBoxBg: Color.lerp(composeBoxBg, other.composeBoxBg, t)!,
448460
contextMenuCancelText: Color.lerp(contextMenuCancelText, other.contextMenuCancelText, t)!,
449461
contextMenuItemBg: Color.lerp(contextMenuItemBg, other.contextMenuItemBg, t)!,
450-
contextMenuItemText: Color.lerp(contextMenuItemText, other.contextMenuItemBg, t)!,
462+
contextMenuItemLabel: Color.lerp(contextMenuItemLabel, other.contextMenuItemLabel, t)!,
463+
contextMenuItemMeta: Color.lerp(contextMenuItemMeta, other.contextMenuItemMeta, t)!,
464+
contextMenuItemText: Color.lerp(contextMenuItemText, other.contextMenuItemText, t)!,
451465
editorButtonPressedBg: Color.lerp(editorButtonPressedBg, other.editorButtonPressedBg, t)!,
452466
foreground: Color.lerp(foreground, other.foreground, t)!,
453467
icon: Color.lerp(icon, other.icon, t)!,

‎test/widgets/autocomplete_test.dart

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'package:zulip/model/localizations.dart';
1313
import 'package:zulip/model/narrow.dart';
1414
import 'package:zulip/model/store.dart';
1515
import 'package:zulip/model/typing_status.dart';
16+
import 'package:zulip/widgets/content.dart';
1617
import 'package:zulip/widgets/message_list.dart';
1718

1819
import '../api/fake_api.dart';
@@ -126,17 +127,22 @@ void main() {
126127
TestZulipBinding.ensureInitialized();
127128

128129
group('@-mentions', () {
129-
void checkUserShown(User user, PerAccountStore store, {required bool expected}) {
130+
131+
Finder findAvatarImage(int userId) =>
132+
find.byWidgetPredicate((widget) => widget is AvatarImage && widget.userId == userId);
133+
134+
void checkUserShown(User user, {required bool expected, bool? deliveryEmailExpected}) {
135+
deliveryEmailExpected ??= expected;
130136
check(find.text(user.fullName).evaluate().length).equals(expected ? 1 : 0);
131-
final avatarFinder =
132-
findNetworkImage(store.tryResolveUrl(user.avatarUrl!).toString());
137+
check(find.text(user.deliveryEmail?? "").evaluate().length).equals(deliveryEmailExpected ? 1 : 0);
138+
final avatarFinder = findAvatarImage(user.userId);
133139
check(avatarFinder.evaluate().length).equals(expected ? 1 : 0);
134140
}
135141

136142
testWidgets('options appear, disappear, and change correctly', (tester) async {
137-
final user1 = eg.user(userId: 1, fullName: 'User One', avatarUrl: 'user1.png');
138-
final user2 = eg.user(userId: 2, fullName: 'User Two', avatarUrl: 'user2.png');
139-
final user3 = eg.user(userId: 3, fullName: 'User Three', avatarUrl: 'user3.png');
143+
final user1 = eg.user(userId: 1, fullName: 'User One', avatarUrl: 'user1.png',deliveryEmail: '[email protected]');
144+
final user2 = eg.user(userId: 2, fullName: 'User Two', avatarUrl: 'user2.png', deliveryEmail: '[email protected]');
145+
final user3 = eg.user(userId: 3, fullName: 'User Three', avatarUrl: 'user3.png', deliveryEmail: '[email protected]');
140146
final composeInputFinder = await setupToComposeInput(tester, users: [user1, user2, user3]);
141147
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
142148

@@ -147,33 +153,54 @@ void main() {
147153
await tester.pumpAndSettle(); // async computation; options appear
148154

149155
// "User Two" and "User Three" appear, but not "User One"
150-
checkUserShown(user1, store, expected: false);
151-
checkUserShown(user2, store, expected: true);
152-
checkUserShown(user3, store, expected: true);
156+
checkUserShown(user1, expected: false);
157+
checkUserShown(user2, expected: true);
158+
checkUserShown(user3, expected: true);
153159

154160
// Finishing autocomplete updates compose box; causes options to disappear
155161
await tester.tap(find.text('User Three'));
156162
await tester.pump();
157163
check(tester.widget<TextField>(composeInputFinder).controller!.text)
158164
.contains(mention(user3, users: store.users));
159-
checkUserShown(user1, store, expected: false);
160-
checkUserShown(user2, store, expected: false);
161-
checkUserShown(user3, store, expected: false);
165+
checkUserShown(user1, expected: false);
166+
checkUserShown(user2, expected: false);
167+
checkUserShown(user3, expected: false);
162168

163169
// Then a new autocomplete intent brings up options again
164170
// TODO(#226): Remove this extra edit when this bug is fixed.
165171
await tester.enterText(composeInputFinder, 'hello @user tw');
166172
await tester.enterText(composeInputFinder, 'hello @user two');
167173
await tester.pumpAndSettle(); // async computation; options appear
168-
checkUserShown(user2, store, expected: true);
174+
checkUserShown(user2, expected: true);
169175

170176
// Removing autocomplete intent causes options to disappear
171177
// TODO(#226): Remove one of these edits when this bug is fixed.
172178
await tester.enterText(composeInputFinder, '');
173179
await tester.enterText(composeInputFinder, ' ');
174-
checkUserShown(user1, store, expected: false);
175-
checkUserShown(user2, store, expected: false);
176-
checkUserShown(user3, store, expected: false);
180+
checkUserShown(user1, expected: false);
181+
checkUserShown(user2, expected: false);
182+
checkUserShown(user3, expected: false);
183+
184+
debugNetworkImageHttpClientProvider = null;
185+
});
186+
187+
testWidgets('test delivery email visibility', (tester) async {
188+
final user2 = eg.user(userId: 2, fullName: 'User Two', avatarUrl: 'user2.png',);
189+
final user3 = eg.user(userId: 3, fullName: 'User Three', avatarUrl: 'user3.png', deliveryEmail: '[email protected]');
190+
final composeInputFinder = await setupToComposeInput(tester, users: [user2, user3]);
191+
192+
TypingNotifier.debugEnable = false;
193+
addTearDown(TypingNotifier.debugReset);
194+
195+
// Options are filtered correctly for query
196+
// TODO(#226): Remove this extra edit when this bug is fixed.
197+
await tester.enterText(composeInputFinder, 'hello @user ');
198+
await tester.enterText(composeInputFinder, 'hello @user t');
199+
await tester.pumpAndSettle(); // async computation; options appear
200+
201+
// "User Two"'s delivery email is not visible and "User Three"'s delivery email is visible
202+
checkUserShown(user2, expected: true, deliveryEmailExpected: false);
203+
checkUserShown(user3, expected: true, deliveryEmailExpected: true);
177204

178205
debugNetworkImageHttpClientProvider = null;
179206
});

0 commit comments

Comments
 (0)