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 zulip#1558.

Co-authored-by: Chris Bobbe <[email protected]>
Co-authored-by:	 Greg Price <[email protected]>

Author: vinodsingh-dev-gsoc

lib/widgets/user.dart

@@ -66,7 +66,7 @@ class AvatarImage extends StatelessWidget {
     final user = store.getUser(userId);
 
     if (user == null) { // TODO(log)
-      return const SizedBox.shrink();
+      return _AvatarPlaceholder(size: size);
     }
 
     if (replaceIfMuted && store.isUserMuted(userId)) {
@@ -79,7 +79,7 @@ class AvatarImage extends StatelessWidget {
     };
 
     if (resolvedUrl == null) {
-      return const SizedBox.shrink();
+      return _AvatarPlaceholder(size: size);
     }
 
     final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: resolvedUrl);
@@ -89,6 +89,7 @@ class AvatarImage extends StatelessWidget {
       avatarUrl.get(physicalSize),
       filterQuality: FilterQuality.medium,
       fit: BoxFit.cover,
+      errorBuilder: (_, _, _) => _AvatarPlaceholder(size: size),
     );
   }
 }

test/test_images.dart

@@ -1,5 +1,6 @@
 import 'dart:async';
 import 'dart:io';
+import 'dart:typed_data';
 
 import 'package:flutter/widgets.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -12,12 +13,18 @@ import 'package:flutter_test/flutter_test.dart';
 /// before the end of the test.
 // TODO(upstream) simplify callers by using addTearDown: https://github.com/flutter/flutter/issues/123189
 //   See also: https://github.com/flutter/flutter/issues/121917
-FakeImageHttpClient prepareBoringImageHttpClient() {
+FakeImageHttpClient prepareBoringImageHttpClient({bool success = true}) {
   final httpClient = FakeImageHttpClient();
   debugNetworkImageHttpClientProvider = () => httpClient;
-  httpClient.request.response
-    ..statusCode = HttpStatus.ok
-    ..content = kSolidBlueAvatar;
+  if (success) {
+    httpClient.request.response
+      ..statusCode = HttpStatus.ok
+      ..content = kSolidBlueAvatar;
+  } else {
+    httpClient.request.response
+      ..statusCode = HttpStatus.notFound
+      ..content = Uint8List(0);
+  }
   return httpClient;
 }
 

test/widgets/user_test.dart

@@ -1,25 +1,30 @@
 import 'package:checks/checks.dart';
-import 'package:flutter/cupertino.dart';
 import 'package:flutter/material.dart';
-import 'package:flutter/rendering.dart';
+import 'package:flutter_checks/flutter_checks.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:zulip/model/store.dart';
 import 'package:zulip/widgets/image.dart';
-import 'package:zulip/widgets/store.dart';
+import 'package:zulip/widgets/icons.dart';
 import 'package:zulip/widgets/user.dart';
 
 import '../example_data.dart' as eg;
 import '../model/binding.dart';
 import '../model/test_store.dart';
 import '../stdlib_checks.dart';
 import '../test_images.dart';
+import 'test_app.dart';
 
 void main() {
   TestZulipBinding.ensureInitialized();
 
   group('AvatarImage', () {
     late PerAccountStore store;
 
+    final findPlaceholder = find.descendant(
+      of: find.byType(AvatarImage),
+      matching: find.byIcon(ZulipIcons.person),
+    );
+
     Future<Uri?> actualUrl(WidgetTester tester, String avatarUrl, [double? size]) async {
       addTearDown(testBinding.reset);
       await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
@@ -28,9 +33,9 @@ void main() {
       await store.addUser(user);
 
       prepareBoringImageHttpClient();
-      await tester.pumpWidget(GlobalStoreWidget(
-        child: PerAccountStoreWidget(accountId: eg.selfAccount.id,
-          child: AvatarImage(userId: user.userId, size: size ?? 30))));
+      await tester.pumpWidget(
+        TestZulipApp(accountId: eg.selfAccount.id,
+          child: AvatarImage(userId: user.userId, size: size ?? 30)));
       await tester.pump();
       await tester.pump();
       tester.widget(find.byType(AvatarImage));
@@ -78,5 +83,51 @@ void main() {
       check(await actualUrl(tester, avatarUrl)).isNull();
       debugNetworkImageHttpClientProvider = null;
     });
+
+    testWidgets('shows placeholder when image URL gives error', (WidgetTester tester) async {
+      addTearDown(testBinding.reset);
+      prepareBoringImageHttpClient(success: false);
+      await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+      final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
+
+      final badUser = eg.user(avatarUrl: 'https://zulip.com/avatarinvalid.png');
+      await store.addUser(badUser);
+
+      await tester.pumpWidget(
+         TestZulipApp(accountId: eg.selfAccount.id,
+           child: AvatarImage(userId: badUser.userId, size: 30)));
+      await tester.pumpAndSettle();
+      check(findPlaceholder).findsOne();
+
+      debugNetworkImageHttpClientProvider = null;
+    });
+
+    testWidgets('shows placeholder when user avatarUrl is null', (WidgetTester tester) async {
+      addTearDown(testBinding.reset);
+      await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+      final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
+
+      final userWithNoUrl = eg.user(avatarUrl: null);
+      await store.addUser(userWithNoUrl);
+
+      await tester.pumpWidget(
+        TestZulipApp(accountId: eg.selfAccount.id,
+          child: AvatarImage(userId: userWithNoUrl.userId, size: 30)));
+      await tester.pumpAndSettle();
+      check(findPlaceholder).findsOne();
+    });
+
+    testWidgets('shows placeholder when user is not found', (WidgetTester tester) async {
+      addTearDown(testBinding.reset);
+      await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+
+      const nonExistentUserId = 9999999;
+
+      await tester.pumpWidget(
+        TestZulipApp(accountId: eg.selfAccount.id,
+          child: AvatarImage(userId: nonExistentUserId, size: 30)));
+      await tester.pumpAndSettle();
+      check(findPlaceholder).findsOne();
+    });
   });
 }