From aa123be14b0da07c15c4937a1606ce8225c598d3 Mon Sep 17 00:00:00 2001 From: Vinod Singh Date: Fri, 3 Oct 2025 19:31:19 +0530 Subject: [PATCH 1/4] avatar: Show placeholder on image load error --- lib/widgets/user.dart | 4 +++- test/widgets/user_test.dart | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/widgets/user.dart b/lib/widgets/user.dart index 182a073d30..7c17f60e9d 100644 --- a/lib/widgets/user.dart +++ b/lib/widgets/user.dart @@ -1,5 +1,4 @@ import 'package:flutter/material.dart'; - import '../api/model/model.dart'; import '../model/avatar_url.dart'; import '../model/binding.dart'; @@ -90,6 +89,9 @@ class AvatarImage extends StatelessWidget { avatarUrl.get(physicalSize), filterQuality: FilterQuality.medium, fit: BoxFit.cover, + errorBuilder: (context, error, stackTrace) { + return _AvatarPlaceholder(size: size); + }, ); } } diff --git a/test/widgets/user_test.dart b/test/widgets/user_test.dart index 5078da0497..f3d0a32d7e 100644 --- a/test/widgets/user_test.dart +++ b/test/widgets/user_test.dart @@ -1,18 +1,26 @@ import 'package:checks/checks.dart'; import 'package:flutter/cupertino.dart'; import 'package:flutter/material.dart'; -import 'package:flutter/rendering.dart'; +import 'package:zulip/widgets/icons.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/store.dart'; import 'package:zulip/widgets/user.dart'; - +import 'dart:io'; 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'; + +class MockHttpClient extends Fake implements HttpClient { + @override + Future getUrl(Uri url) async { + throw const SocketException('test error'); + } +} void main() { TestZulipBinding.ensureInitialized(); @@ -78,5 +86,28 @@ void main() { check(await actualUrl(tester, avatarUrl)).isNull(); debugNetworkImageHttpClientProvider = null; }); + + testWidgets('shows placeholder when image URL gives error', (WidgetTester tester) async { + await HttpOverrides.runZoned(() async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + final user = eg.user(avatarUrl: 'https://zulip.com/avatar.png'); + await store.addUser(user); + + await tester.pumpWidget( + TestZulipApp( + accountId: eg.selfAccount.id, + child: AvatarImage(userId: user.userId, size: 32), + ), + ); + await tester.pump(); // Image provider is created + await tester.pump(); // Image fails to load + expect(find.byIcon(ZulipIcons.person), findsOneWidget); + + expect(find.byType(DecoratedBox), findsOneWidget); + + }, createHttpClient: (context) => MockHttpClient()); + }); }); } From 57ce7f1f996c09a7eee6ae560bea9e68da1bda72 Mon Sep 17 00:00:00 2001 From: Vinod Singh Date: Sat, 4 Oct 2025 05:22:30 +0530 Subject: [PATCH 2/4] Apply suggestion from @chrisbobbe Co-authored-by: Chris Bobbe --- lib/widgets/user.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/widgets/user.dart b/lib/widgets/user.dart index 7c17f60e9d..905eb2ee71 100644 --- a/lib/widgets/user.dart +++ b/lib/widgets/user.dart @@ -89,9 +89,7 @@ class AvatarImage extends StatelessWidget { avatarUrl.get(physicalSize), filterQuality: FilterQuality.medium, fit: BoxFit.cover, - errorBuilder: (context, error, stackTrace) { - return _AvatarPlaceholder(size: size); - }, + errorBuilder: (_, _, _) => _AvatarPlaceholder(size: size), ); } } From 97c936da5ac6d895d350167f44916d9ba2be4f5d Mon Sep 17 00:00:00 2001 From: Vinod Singh Date: Sat, 4 Oct 2025 05:23:03 +0530 Subject: [PATCH 3/4] Apply suggestion from @chrisbobbe Co-authored-by: Chris Bobbe --- test/widgets/user_test.dart | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/widgets/user_test.dart b/test/widgets/user_test.dart index f3d0a32d7e..7b0ddacd79 100644 --- a/test/widgets/user_test.dart +++ b/test/widgets/user_test.dart @@ -95,12 +95,9 @@ void main() { final user = eg.user(avatarUrl: 'https://zulip.com/avatar.png'); await store.addUser(user); - await tester.pumpWidget( - TestZulipApp( - accountId: eg.selfAccount.id, - child: AvatarImage(userId: user.userId, size: 32), - ), - ); + await tester.pumpWidget(TestZulipApp( + accountId: eg.selfAccount.id, + child: AvatarImage(userId: user.userId, size: 32))); await tester.pump(); // Image provider is created await tester.pump(); // Image fails to load expect(find.byIcon(ZulipIcons.person), findsOneWidget); From 3ed8e3ce616ed193d533e3aad906fe1ff366ee7f Mon Sep 17 00:00:00 2001 From: Vinod Singh Date: Fri, 3 Oct 2025 19:31:19 +0530 Subject: [PATCH 4/4] avatar: Show placeholder on image load error --- test/test_images.dart | 17 +++++++++----- test/widgets/user_test.dart | 46 ++++++++++++++++--------------------- 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/test/test_images.dart b/test/test_images.dart index c7a04c264f..8a6eb53677 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,15 +12,20 @@ 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; } - class FakeImageHttpClient extends Fake implements HttpClient { final FakeImageHttpClientRequest request = FakeImageHttpClientRequest(); diff --git a/test/widgets/user_test.dart b/test/widgets/user_test.dart index 7b0ddacd79..d5ca4c83e0 100644 --- a/test/widgets/user_test.dart +++ b/test/widgets/user_test.dart @@ -1,13 +1,13 @@ import 'package:checks/checks.dart'; import 'package:flutter/cupertino.dart'; import 'package:flutter/material.dart'; -import 'package:zulip/widgets/icons.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/user.dart'; -import 'dart:io'; import '../example_data.dart' as eg; import '../model/binding.dart'; import '../model/test_store.dart'; @@ -15,13 +15,6 @@ import '../stdlib_checks.dart'; import '../test_images.dart'; import 'test_app.dart'; -class MockHttpClient extends Fake implements HttpClient { - @override - Future getUrl(Uri url) async { - throw const SocketException('test error'); - } -} - void main() { TestZulipBinding.ensureInitialized(); @@ -86,25 +79,26 @@ void main() { check(await actualUrl(tester, avatarUrl)).isNull(); debugNetworkImageHttpClientProvider = null; }); - testWidgets('shows placeholder when image URL gives error', (WidgetTester tester) async { - await HttpOverrides.runZoned(() async { - addTearDown(testBinding.reset); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); - final user = eg.user(avatarUrl: 'https://zulip.com/avatar.png'); - await store.addUser(user); - - await tester.pumpWidget(TestZulipApp( + 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( + child: PerAccountStoreWidget( accountId: eg.selfAccount.id, - child: AvatarImage(userId: user.userId, size: 32))); - await tester.pump(); // Image provider is created - await tester.pump(); // Image fails to load - expect(find.byIcon(ZulipIcons.person), findsOneWidget); - - expect(find.byType(DecoratedBox), findsOneWidget); - - }, createHttpClient: (context) => MockHttpClient()); + child: AvatarImage(userId: badUser.userId, size: 30)))); + await tester.pumpAndSettle(); + expect( + find.descendant( + of: find.byType(AvatarImage), + matching: find.byIcon(ZulipIcons.person), + ), + findsOneWidget, + ); + debugNetworkImageHttpClientProvider = null; }); }); }