Skip to content

Commit 8025779

Browse files
gnpricegnprice
committed
Merge remote-tracking branch 'pr/995'
2 parents 8a1ffa5 + 87d149c commit 8025779

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
// TODO deduplicate this logic with [EmojiPickerListEntry]
@@ -276,15 +312,31 @@ class _EmojiAutocompleteItem extends StatelessWidget {
276312
? candidate.emojiName
277313
: [candidate.emojiName, ...candidate.aliases].join(", "); // TODO(#1080)
278314

315+
// TODO(design): emoji autocomplete results
316+
// There's no design in Figma for emoji autocomplete results.
317+
// Instead we adapt the design for the emoji picker to the
318+
// context of autocomplete results as exemplified by _MentionAutocompleteItem.
319+
// That means: emoji size, text size, text line-height from emoji picker;
320+
// text color (for contrast with background) and outer padding
321+
// from _MentionAutocompleteItem; padding around emoji glyph
322+
// to bring it to same size as avatar in _MentionAutocompleteItem.
279323
return Padding(
280-
padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
324+
padding: const EdgeInsetsDirectional.fromSTEB(4, 4, 8, 4),
281325
child: Row(children: [
282326
if (glyph != null) ...[
283-
glyph,
284-
const SizedBox(width: 8),
327+
Padding(padding: const EdgeInsets.all(6),
328+
child: glyph),
329+
const SizedBox(width: 6),
285330
],
286331
Expanded(
287332
child: Text(
333+
// The Figma design for the emoji picker actually calls for the
334+
// line-height to be 24px when the label fits on one line,
335+
// and 18px when it wraps. But whether it'll wrap is something we
336+
// don't know at build time, so make it always 18px. Discussion:
337+
// https://github.com/zulip/zulip-flutter/pull/995#discussion_r1868352275
338+
style: TextStyle(fontSize: 17, height: 18 / 17,
339+
color: designVariables.contextMenuItemLabel),
288340
maxLines: 2,
289341
overflow: TextOverflow.ellipsis,
290342
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
@@ -134,6 +134,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
134134
composeBoxBg: const Color(0xffffffff),
135135
contextMenuCancelText: const Color(0xff222222),
136136
contextMenuItemBg: const Color(0xff6159e1),
137+
contextMenuItemLabel: const Color(0xff242631),
138+
contextMenuItemMeta: const Color(0xff626573),
137139
contextMenuItemText: const Color(0xff381da7),
138140
editorButtonPressedBg: Colors.black.withValues(alpha: 0.06),
139141
foreground: const Color(0xff000000),
@@ -184,6 +186,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
184186
composeBoxBg: const Color(0xff0f0f0f),
185187
contextMenuCancelText: const Color(0xffffffff).withValues(alpha: 0.75),
186188
contextMenuItemBg: const Color(0xff7977fe),
189+
contextMenuItemLabel: const Color(0xffdfe1e8),
190+
contextMenuItemMeta: const Color(0xff9194a3),
187191
contextMenuItemText: const Color(0xff9398fd),
188192
editorButtonPressedBg: Colors.white.withValues(alpha: 0.06),
189193
foreground: const Color(0xffffffff),
@@ -241,6 +245,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
241245
required this.composeBoxBg,
242246
required this.contextMenuCancelText,
243247
required this.contextMenuItemBg,
248+
required this.contextMenuItemLabel,
249+
required this.contextMenuItemMeta,
244250
required this.contextMenuItemText,
245251
required this.editorButtonPressedBg,
246252
required this.foreground,
@@ -299,6 +305,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
299305
final Color composeBoxBg;
300306
final Color contextMenuCancelText;
301307
final Color contextMenuItemBg;
308+
final Color contextMenuItemLabel;
309+
final Color contextMenuItemMeta;
302310
final Color contextMenuItemText;
303311
final Color editorButtonPressedBg;
304312
final Color foreground;
@@ -352,6 +360,8 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
352360
Color? composeBoxBg,
353361
Color? contextMenuCancelText,
354362
Color? contextMenuItemBg,
363+
Color? contextMenuItemLabel,
364+
Color? contextMenuItemMeta,
355365
Color? contextMenuItemText,
356366
Color? editorButtonPressedBg,
357367
Color? foreground,
@@ -400,7 +410,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
400410
composeBoxBg: composeBoxBg ?? this.composeBoxBg,
401411
contextMenuCancelText: contextMenuCancelText ?? this.contextMenuCancelText,
402412
contextMenuItemBg: contextMenuItemBg ?? this.contextMenuItemBg,
403-
contextMenuItemText: contextMenuItemText ?? this.contextMenuItemBg,
413+
contextMenuItemLabel: contextMenuItemLabel ?? this.contextMenuItemLabel,
414+
contextMenuItemMeta: contextMenuItemMeta ?? this.contextMenuItemMeta,
415+
contextMenuItemText: contextMenuItemText ?? this.contextMenuItemText,
404416
editorButtonPressedBg: editorButtonPressedBg ?? this.editorButtonPressedBg,
405417
foreground: foreground ?? this.foreground,
406418
icon: icon ?? this.icon,
@@ -455,7 +467,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
455467
composeBoxBg: Color.lerp(composeBoxBg, other.composeBoxBg, t)!,
456468
contextMenuCancelText: Color.lerp(contextMenuCancelText, other.contextMenuCancelText, t)!,
457469
contextMenuItemBg: Color.lerp(contextMenuItemBg, other.contextMenuItemBg, t)!,
458-
contextMenuItemText: Color.lerp(contextMenuItemText, other.contextMenuItemBg, t)!,
470+
contextMenuItemLabel: Color.lerp(contextMenuItemLabel, other.contextMenuItemLabel, t)!,
471+
contextMenuItemMeta: Color.lerp(contextMenuItemMeta, other.contextMenuItemMeta, t)!,
472+
contextMenuItemText: Color.lerp(contextMenuItemText, other.contextMenuItemText, t)!,
459473
editorButtonPressedBg: Color.lerp(editorButtonPressedBg, other.editorButtonPressedBg, t)!,
460474
foreground: Color.lerp(foreground, other.foreground, t)!,
461475
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)