avatar: Show placeholder on image load error.

Fixes #1558

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,18 +1,17 @@
 import 'package:checks/checks.dart';
-import 'package:flutter/cupertino.dart';
 import 'package:flutter/material.dart';
-import 'package:flutter/rendering.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();
@@ -28,9 +27,12 @@ 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 +80,72 @@ 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(
+        find.descendant(
+          of: find.byType(AvatarImage),
+          matching: find.byIcon(ZulipIcons.person),
+        ).evaluate().length
+      ).equals(1);
+      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(
+          find.descendant(
+            of: find.byType(AvatarImage),
+            matching: find.byIcon(ZulipIcons.person),
+          ).evaluate().length
+      ).equals(1);
+    });
+
+    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(
+          find.descendant(
+            of: find.byType(AvatarImage),
+            matching: find.byIcon(ZulipIcons.person),
+          ).evaluate().length
+      ).equals(1);
+    });
   });
 }