diff --git a/lib/widgets/user.dart b/lib/widgets/user.dart index 182a073d30..27c7d03b60 100644 --- a/lib/widgets/user.dart +++ b/lib/widgets/user.dart @@ -67,9 +67,8 @@ 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)) { return _AvatarPlaceholder(size: size); } @@ -80,7 +79,7 @@ class AvatarImage extends StatelessWidget { }; if (resolvedUrl == null) { - return const SizedBox.shrink(); + return _AvatarPlaceholder(size: size); } final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: resolvedUrl); @@ -90,6 +89,7 @@ class AvatarImage extends StatelessWidget { avatarUrl.get(physicalSize), filterQuality: FilterQuality.medium, fit: BoxFit.cover, + errorBuilder: (_, _, _) => _AvatarPlaceholder(size: size), ); } } diff --git a/test/test_images.dart b/test/test_images.dart index c7a04c264f..d8d797fc85 100644 --- a/test/test_images.dart +++ b/test/test_images.dart @@ -1,6 +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 +12,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; } diff --git a/test/widgets/user_test.dart b/test/widgets/user_test.dart index 5078da0497..2932fa27f9 100644 --- a/test/widgets/user_test.dart +++ b/test/widgets/user_test.dart @@ -1,18 +1,18 @@ 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/content.dart'; +import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/store.dart'; +import 'package:zulip/widgets/theme.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 +28,17 @@ 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( + MaterialApp( + theme: ThemeData(extensions: [ + DesignVariables.light, + ]), + home: GlobalStoreWidget( + child: PerAccountStoreWidget(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 +86,86 @@ 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( + MaterialApp( + theme: ThemeData(extensions: [ + DesignVariables.light, + ]), + home: 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( + MaterialApp( + theme: ThemeData(extensions: [ + DesignVariables.light, + ]), + home: 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( + MaterialApp( + theme: ThemeData(extensions: [ + DesignVariables.light, + ]), + home: 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); + }); }); }