Skip to content

Commit 0408831

Browse files
vinodsingh-dev-gsocvinodsingh-dev-gsoc
committed
avatar: Show placeholder on image load error.
1 parent 1699aa5 commit 0408831

File tree

4 files changed

+129
-20
lines changed

4 files changed

+129
-20
lines changed

‎lib/widgets/user.dart‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,8 @@ class AvatarImage extends StatelessWidget {
6666
final user = store.getUser(userId);
6767

6868
if (user == null) { // TODO(log)
69-
return const SizedBox.shrink();
69+
return _AvatarPlaceholder(size: size);
7070
}
71-
7271
if (replaceIfMuted && store.isUserMuted(userId)) {
7372
return _AvatarPlaceholder(size: size);
7473
}
@@ -79,7 +78,7 @@ class AvatarImage extends StatelessWidget {
7978
};
8079

8180
if (resolvedUrl == null) {
82-
return const SizedBox.shrink();
81+
return _AvatarPlaceholder(size: size);
8382
}
8483

8584
final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: resolvedUrl);
@@ -89,6 +88,7 @@ class AvatarImage extends StatelessWidget {
8988
avatarUrl.get(physicalSize),
9089
filterQuality: FilterQuality.medium,
9190
fit: BoxFit.cover,
91+
errorBuilder: (_, _, _) => _AvatarPlaceholder(size: size),
9292
);
9393
}
9494
}

‎test/test_images.dart‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import 'dart:async';
22
import 'dart:io';
3-
3+
import 'dart:typed_data';
44
import 'package:flutter/widgets.dart';
55
import 'package:flutter_test/flutter_test.dart';
66

@@ -12,12 +12,18 @@ import 'package:flutter_test/flutter_test.dart';
1212
/// before the end of the test.
1313
// TODO(upstream) simplify callers by using addTearDown: https://github.com/flutter/flutter/issues/123189
1414
// See also: https://github.com/flutter/flutter/issues/121917
15-
FakeImageHttpClient prepareBoringImageHttpClient() {
15+
FakeImageHttpClient prepareBoringImageHttpClient({bool success = true}) {
1616
final httpClient = FakeImageHttpClient();
1717
debugNetworkImageHttpClientProvider = () => httpClient;
18-
httpClient.request.response
19-
..statusCode = HttpStatus.ok
20-
..content = kSolidBlueAvatar;
18+
if (success) {
19+
httpClient.request.response
20+
..statusCode = HttpStatus.ok
21+
..content = kSolidBlueAvatar;
22+
} else {
23+
httpClient.request.response
24+
..statusCode = HttpStatus.notFound
25+
..content = Uint8List(0);
26+
}
2127
return httpClient;
2228
}
2329

‎test/widgets/new_dm_sheet_test.dart‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,17 +254,31 @@ void main() {
254254
});
255255

256256
group('user selection', () {
257-
void checkUserSelected(WidgetTester tester, User user, bool expected) {
258-
final icon = tester.widget<Icon>(find.descendant(
259-
of: findUserTile(user),
260-
matching: find.byType(Icon)));
261257

258+
void checkUserSelected(WidgetTester tester, User user, bool expected) {
259+
final userTileFinder = findUserTile(user);
262260
if (expected) {
263261
check(findUserChip(user)).findsOne();
264-
check(icon).icon.equals(ZulipIcons.check_circle_checked);
262+
check(find.descendant(
263+
of: userTileFinder,
264+
matching: find.byIcon(ZulipIcons.check_circle_checked),
265+
)).findsOne();
266+
check(find.descendant(
267+
of: userTileFinder,
268+
matching: find.byIcon(ZulipIcons.check_circle_unchecked),
269+
)).findsNothing();
270+
265271
} else {
266272
check(findUserChip(user)).findsNothing();
267-
check(icon).icon.equals(ZulipIcons.check_circle_unchecked);
273+
check(find.descendant(
274+
of: userTileFinder,
275+
matching: find.byIcon(ZulipIcons.check_circle_unchecked),
276+
)).findsOne();
277+
278+
check(find.descendant(
279+
of: userTileFinder,
280+
matching: find.byIcon(ZulipIcons.check_circle_checked),
281+
)).findsNothing();
268282
}
269283
}
270284

‎test/widgets/user_test.dart‎

Lines changed: 95 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
import 'package:checks/checks.dart';
2-
import 'package:flutter/cupertino.dart';
32
import 'package:flutter/material.dart';
4-
import 'package:flutter/rendering.dart';
53
import 'package:flutter_test/flutter_test.dart';
64
import 'package:zulip/model/store.dart';
75
import 'package:zulip/widgets/image.dart';
6+
import 'package:zulip/widgets/icons.dart';
87
import 'package:zulip/widgets/store.dart';
8+
import 'package:zulip/widgets/theme.dart';
99
import 'package:zulip/widgets/user.dart';
10-
1110
import '../example_data.dart' as eg;
1211
import '../model/binding.dart';
1312
import '../model/test_store.dart';
1413
import '../stdlib_checks.dart';
1514
import '../test_images.dart';
15+
import 'test_app.dart';
1616

1717
void main() {
1818
TestZulipBinding.ensureInitialized();
@@ -28,9 +28,17 @@ void main() {
2828
await store.addUser(user);
2929

3030
prepareBoringImageHttpClient();
31-
await tester.pumpWidget(GlobalStoreWidget(
32-
child: PerAccountStoreWidget(accountId: eg.selfAccount.id,
33-
child: AvatarImage(userId: user.userId, size: size ?? 30))));
31+
await tester.pumpWidget(
32+
MaterialApp(
33+
theme: ThemeData(extensions: [
34+
DesignVariables.light,
35+
]),
36+
home: GlobalStoreWidget(
37+
child: PerAccountStoreWidget(accountId: eg.selfAccount.id,
38+
child: AvatarImage(userId: user.userId, size: size ?? 30)),
39+
),
40+
),
41+
);
3442
await tester.pump();
3543
await tester.pump();
3644
tester.widget(find.byType(AvatarImage));
@@ -78,5 +86,86 @@ void main() {
7886
check(await actualUrl(tester, avatarUrl)).isNull();
7987
debugNetworkImageHttpClientProvider = null;
8088
});
89+
90+
testWidgets('shows placeholder when image URL gives error', (WidgetTester tester) async {
91+
addTearDown(testBinding.reset);
92+
prepareBoringImageHttpClient(success: false);
93+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
94+
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
95+
final badUser = eg.user(avatarUrl: 'https://zulip.com/avatarinvalid.png');
96+
await store.addUser(badUser);
97+
await tester.pumpWidget(
98+
MaterialApp(
99+
theme: ThemeData(extensions: [
100+
DesignVariables.light,
101+
]),
102+
home: TestZulipApp(
103+
accountId: eg.selfAccount.id,
104+
child: AvatarImage(userId: badUser.userId, size: 30))));
105+
await tester.pumpAndSettle();
106+
check(
107+
find.descendant(
108+
of: find.byType(AvatarImage),
109+
matching: find.byIcon(ZulipIcons.person),
110+
).evaluate().length
111+
).equals(1);
112+
debugNetworkImageHttpClientProvider = null;
113+
});
114+
115+
testWidgets('shows placeholder when user avatarUrl is null', (WidgetTester tester) async {
116+
addTearDown(testBinding.reset);
117+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
118+
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
119+
120+
final userWithNoUrl = eg.user(avatarUrl: null);
121+
await store.addUser(userWithNoUrl);
122+
123+
await tester.pumpWidget(
124+
MaterialApp(
125+
theme: ThemeData(extensions: [
126+
DesignVariables.light,
127+
]),
128+
home: TestZulipApp(
129+
accountId: eg.selfAccount.id,
130+
child: AvatarImage(userId: userWithNoUrl.userId, size: 30),
131+
),
132+
),
133+
);
134+
await tester.pumpAndSettle();
135+
136+
check(
137+
find.descendant(
138+
of: find.byType(AvatarImage),
139+
matching: find.byIcon(ZulipIcons.person),
140+
).evaluate().length
141+
).equals(1);
142+
});
143+
144+
testWidgets('shows placeholder when user is not found', (WidgetTester tester) async {
145+
addTearDown(testBinding.reset);
146+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
147+
148+
const nonExistentUserId = 9999999;
149+
150+
await tester.pumpWidget(
151+
MaterialApp(
152+
theme: ThemeData(extensions: [
153+
DesignVariables.light,
154+
]),
155+
home: TestZulipApp(
156+
accountId: eg.selfAccount.id,
157+
child: AvatarImage(userId: nonExistentUserId, size: 30),
158+
),
159+
),
160+
);
161+
await tester.pumpAndSettle();
162+
163+
check(
164+
find.descendant(
165+
of: find.byType(AvatarImage),
166+
matching: find.byIcon(ZulipIcons.person),
167+
).evaluate().length
168+
).equals(1);
169+
});
81170
});
82171
}

0 commit comments

Comments
 (0)