Skip to content

Commit 6be7be5

Browse files
avatar: Show placeholder on image load error.
Previously, AvatarImage showed a blank space when the user was null or the image URL failed to load. This commit adds a placeholder icon in all such cases and updates the tests to verify this new behavior. Fixes #1558. Co-authored-by: Chris Bobbe <[email protected]> Co-authored-by: Greg Price <[email protected]>
1 parent c82cfa0 commit 6be7be5

File tree

3 files changed

+68
-12
lines changed

3 files changed

+68
-12
lines changed

lib/widgets/user.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ 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
}
7171

7272
if (replaceIfMuted && store.isUserMuted(userId)) {
@@ -79,7 +79,7 @@ class AvatarImage extends StatelessWidget {
7979
};
8080

8181
if (resolvedUrl == null) {
82-
return const SizedBox.shrink();
82+
return _AvatarPlaceholder(size: size);
8383
}
8484

8585
final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: resolvedUrl);
@@ -89,6 +89,7 @@ class AvatarImage extends StatelessWidget {
8989
avatarUrl.get(physicalSize),
9090
filterQuality: FilterQuality.medium,
9191
fit: BoxFit.cover,
92+
errorBuilder: (_, _, _) => _AvatarPlaceholder(size: size),
9293
);
9394
}
9495
}

test/test_images.dart

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import 'dart:async';
22
import 'dart:io';
3+
import 'dart:typed_data';
34

45
import 'package:flutter/widgets.dart';
56
import 'package:flutter_test/flutter_test.dart';
@@ -12,12 +13,18 @@ import 'package:flutter_test/flutter_test.dart';
1213
/// before the end of the test.
1314
// TODO(upstream) simplify callers by using addTearDown: https://github.com/flutter/flutter/issues/123189
1415
// See also: https://github.com/flutter/flutter/issues/121917
15-
FakeImageHttpClient prepareBoringImageHttpClient() {
16+
FakeImageHttpClient prepareBoringImageHttpClient({bool success = true}) {
1617
final httpClient = FakeImageHttpClient();
1718
debugNetworkImageHttpClientProvider = () => httpClient;
18-
httpClient.request.response
19-
..statusCode = HttpStatus.ok
20-
..content = kSolidBlueAvatar;
19+
if (success) {
20+
httpClient.request.response
21+
..statusCode = HttpStatus.ok
22+
..content = kSolidBlueAvatar;
23+
} else {
24+
httpClient.request.response
25+
..statusCode = HttpStatus.notFound
26+
..content = Uint8List(0);
27+
}
2128
return httpClient;
2229
}
2330

test/widgets/user_test.dart

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,30 @@
11
import 'package:checks/checks.dart';
2-
import 'package:flutter/cupertino.dart';
32
import 'package:flutter/material.dart';
4-
import 'package:flutter/rendering.dart';
3+
import 'package:flutter_checks/flutter_checks.dart';
54
import 'package:flutter_test/flutter_test.dart';
65
import 'package:zulip/model/store.dart';
76
import 'package:zulip/widgets/image.dart';
8-
import 'package:zulip/widgets/store.dart';
7+
import 'package:zulip/widgets/icons.dart';
98
import 'package:zulip/widgets/user.dart';
109

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();
1919

2020
group('AvatarImage', () {
2121
late PerAccountStore store;
2222

23+
final findPlaceholder = find.descendant(
24+
of: find.byType(AvatarImage),
25+
matching: find.byIcon(ZulipIcons.person),
26+
);
27+
2328
Future<Uri?> actualUrl(WidgetTester tester, String avatarUrl, [double? size]) async {
2429
addTearDown(testBinding.reset);
2530
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
@@ -28,9 +33,9 @@ void main() {
2833
await store.addUser(user);
2934

3035
prepareBoringImageHttpClient();
31-
await tester.pumpWidget(GlobalStoreWidget(
32-
child: PerAccountStoreWidget(accountId: eg.selfAccount.id,
33-
child: AvatarImage(userId: user.userId, size: size ?? 30))));
36+
await tester.pumpWidget(
37+
TestZulipApp(accountId: eg.selfAccount.id,
38+
child: AvatarImage(userId: user.userId, size: size ?? 30)));
3439
await tester.pump();
3540
await tester.pump();
3641
tester.widget(find.byType(AvatarImage));
@@ -78,5 +83,48 @@ void main() {
7883
check(await actualUrl(tester, avatarUrl)).isNull();
7984
debugNetworkImageHttpClientProvider = null;
8085
});
86+
87+
testWidgets('shows placeholder when image URL gives error', (WidgetTester tester) async {
88+
addTearDown(testBinding.reset);
89+
prepareBoringImageHttpClient(success: false);
90+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
91+
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
92+
final badUser = eg.user(avatarUrl: 'https://zulip.com/avatarinvalid.png');
93+
await store.addUser(badUser);
94+
await tester.pumpWidget(
95+
TestZulipApp(accountId: eg.selfAccount.id,
96+
child: AvatarImage(userId: badUser.userId, size: 30)));
97+
await tester.pumpAndSettle();
98+
check(findPlaceholder).findsOne();
99+
debugNetworkImageHttpClientProvider = null;
100+
});
101+
102+
testWidgets('shows placeholder when user avatarUrl is null', (WidgetTester tester) async {
103+
addTearDown(testBinding.reset);
104+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
105+
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
106+
107+
final userWithNoUrl = eg.user(avatarUrl: null);
108+
await store.addUser(userWithNoUrl);
109+
110+
await tester.pumpWidget(
111+
TestZulipApp(accountId: eg.selfAccount.id,
112+
child: AvatarImage(userId: userWithNoUrl.userId, size: 30)));
113+
await tester.pumpAndSettle();
114+
check(findPlaceholder).findsOne();
115+
});
116+
117+
testWidgets('shows placeholder when user is not found', (WidgetTester tester) async {
118+
addTearDown(testBinding.reset);
119+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
120+
121+
const nonExistentUserId = 9999999;
122+
123+
await tester.pumpWidget(
124+
TestZulipApp(accountId: eg.selfAccount.id,
125+
child: AvatarImage(userId: nonExistentUserId, size: 30)));
126+
await tester.pumpAndSettle();
127+
check(findPlaceholder).findsOne();
128+
});
81129
});
82130
}

0 commit comments

Comments
 (0)