From c5f6c9c1cee4879b42b66ed419f73dbac71398a7 Mon Sep 17 00:00:00 2001 From: saisumith770 Date: Wed, 6 Mar 2024 22:32:27 +0530 Subject: [PATCH 1/2] avatar [nfc]: Add size property to AvatarImage --- lib/widgets/content.dart | 4 +++- lib/widgets/recent_dm_conversations.dart | 8 +++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index c5e2682ad2..61cbdcb195 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -1027,7 +1027,7 @@ class Avatar extends StatelessWidget { return AvatarShape( size: size, borderRadius: borderRadius, - child: AvatarImage(userId: userId)); + child: AvatarImage(userId: userId, size: size)); } } @@ -1040,9 +1040,11 @@ class AvatarImage extends StatelessWidget { const AvatarImage({ super.key, required this.userId, + required this.size, }); final int userId; + final double size; @override Widget build(BuildContext context) { diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index aafdc8c42f..2d2b0bb247 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -81,6 +81,8 @@ class RecentDmConversationsItem extends StatelessWidget { required this.unreadCount, }); + static const double _avatarSize = 32; + final DmNarrow narrow; final int unreadCount; @@ -94,14 +96,14 @@ class RecentDmConversationsItem extends StatelessWidget { switch (narrow.otherRecipientIds) { // TODO dedupe with DM items in [InboxPage] case []: title = selfUser.fullName; - avatar = AvatarImage(userId: selfUser.userId); + avatar = AvatarImage(userId: selfUser.userId, size: _avatarSize); case [var otherUserId]: // TODO(#296) actually don't show this row if the user is muted? // (should we offer a "spam folder" style summary screen of recent // 1:1 DM conversations from muted users?) final otherUser = store.users[otherUserId]; title = otherUser?.fullName ?? '(unknown user)'; - avatar = AvatarImage(userId: otherUserId); + avatar = AvatarImage(userId: otherUserId, size: _avatarSize); default: // TODO(i18n): List formatting, like you can do in JavaScript: // new Intl.ListFormat('ja').format(['Chris', 'Greg', 'Alya']) @@ -122,7 +124,7 @@ class RecentDmConversationsItem extends StatelessWidget { child: ConstrainedBox(constraints: const BoxConstraints(minHeight: 48), child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [ Padding(padding: const EdgeInsetsDirectional.fromSTEB(12, 8, 0, 8), - child: AvatarShape(size: 32, borderRadius: 3, child: avatar)), + child: AvatarShape(size: _avatarSize, borderRadius: 3, child: avatar)), const SizedBox(width: 8), Expanded(child: Padding( padding: const EdgeInsets.symmetric(vertical: 4), From 58b315515e977b0536520b8e16cf8bc8fb4a7dd9 Mon Sep 17 00:00:00 2001 From: saisumith770 Date: Wed, 6 Mar 2024 22:33:22 +0530 Subject: [PATCH 2/2] avatar: Fix blurriness on profile screen by adjusting physical image size Fixes: #301 --- lib/model/avatar_url.dart | 50 ++++++++++++++++++++++++ lib/widgets/content.dart | 17 ++++++-- lib/widgets/recent_dm_conversations.dart | 4 +- test/model/avatar_url_test.dart | 34 ++++++++++++++++ test/widgets/content_test.dart | 24 +++++++++++- 5 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 lib/model/avatar_url.dart create mode 100644 test/model/avatar_url_test.dart diff --git a/lib/model/avatar_url.dart b/lib/model/avatar_url.dart new file mode 100644 index 0000000000..8177a4e8c0 --- /dev/null +++ b/lib/model/avatar_url.dart @@ -0,0 +1,50 @@ +/// The size threshold above which is "medium" size for an avatar. +/// +/// This is DEFAULT_AVATAR_SIZE in zerver/lib/upload.py. +const defaultUploadSizePx = 100; + +abstract class AvatarUrl { + factory AvatarUrl.fromUserData({required Uri resolvedUrl}) { + // TODO(#255): handle computing gravatars + // TODO(#254): handle computing fallback avatar + if (resolvedUrl.toString().startsWith(GravatarUrl.origin)) { + return GravatarUrl(resolvedUrl: resolvedUrl); + } else { + return UploadedAvatarUrl(resolvedUrl: resolvedUrl); + } + } + + Uri get(int sizePhysicalPx); +} + +class GravatarUrl implements AvatarUrl { + GravatarUrl({required Uri resolvedUrl}) : standardUrl = resolvedUrl; + + static String origin = 'https://secure.gravatar.com'; + + Uri standardUrl; + + @override + Uri get(int sizePhysicalPx) { + return standardUrl.replace(queryParameters: { + ...standardUrl.queryParameters, + 's': sizePhysicalPx.toString(), + }); + } +} + +class UploadedAvatarUrl implements AvatarUrl { + UploadedAvatarUrl({required Uri resolvedUrl}) : standardUrl = resolvedUrl; + + Uri standardUrl; + + @override + Uri get(int sizePhysicalPx) { + if (sizePhysicalPx > defaultUploadSizePx) { + return standardUrl.replace( + path: standardUrl.path.replaceFirst(RegExp(r'(?:\.png)?$'), "-medium.png")); + } + + return standardUrl; + } +} diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 61cbdcb195..24794464b4 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -7,6 +7,7 @@ import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import '../api/core.dart'; import '../api/model/model.dart'; +import '../model/avatar_url.dart'; import '../model/binding.dart'; import '../model/content.dart'; import '../model/internal_link.dart'; @@ -1059,9 +1060,19 @@ class AvatarImage extends StatelessWidget { null => null, // TODO(#255): handle computing gravatars var avatarUrl => store.tryResolveUrl(avatarUrl), }; - return (resolvedUrl == null) - ? const SizedBox.shrink() - : RealmContentNetworkImage(resolvedUrl, filterQuality: FilterQuality.medium, fit: BoxFit.cover); + + if (resolvedUrl == null) { + return const SizedBox.shrink(); + } + + final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: resolvedUrl); + final physicalSize = (MediaQuery.devicePixelRatioOf(context) * size).ceil(); + + return RealmContentNetworkImage( + avatarUrl.get(physicalSize), + filterQuality: FilterQuality.medium, + fit: BoxFit.cover, + ); } } diff --git a/lib/widgets/recent_dm_conversations.dart b/lib/widgets/recent_dm_conversations.dart index 2d2b0bb247..27bf2a7560 100644 --- a/lib/widgets/recent_dm_conversations.dart +++ b/lib/widgets/recent_dm_conversations.dart @@ -81,11 +81,11 @@ class RecentDmConversationsItem extends StatelessWidget { required this.unreadCount, }); - static const double _avatarSize = 32; - final DmNarrow narrow; final int unreadCount; + static const double _avatarSize = 32; + @override Widget build(BuildContext context) { final store = PerAccountStoreWidget.of(context); diff --git a/test/model/avatar_url_test.dart b/test/model/avatar_url_test.dart new file mode 100644 index 0000000000..f12d3a60f9 --- /dev/null +++ b/test/model/avatar_url_test.dart @@ -0,0 +1,34 @@ +import 'package:checks/checks.dart'; +import 'package:test/scaffolding.dart'; +import 'package:zulip/model/avatar_url.dart'; + +void main() { + const defaultSize = 30; + const largeSize = 120; + + group('GravatarUrl', () { + test('gravatar url', () { + final url = '${GravatarUrl.origin}/avatar/1234'; + final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: Uri.parse(url)); + + check(avatarUrl.get(defaultSize).toString()).equals('$url?s=30'); + }); + }); + + group('UploadedAvatarUrl', () { + test('png image', () { + const url = 'https://zulip.example/image.png'; + final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: Uri.parse(url)); + + check(avatarUrl.get(defaultSize).toString()).equals(url); + }); + + test('png image, larger size', () { + const url = 'https://zulip.example/image.png'; + final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: Uri.parse(url)); + final expectedUrl = url.replaceAll('.png', '-medium.png'); + + check(avatarUrl.get(largeSize).toString()).equals(expectedUrl); + }); + }); +} diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index c485f585a7..5976171eec 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -538,7 +538,7 @@ void main() { group('AvatarImage', () { late PerAccountStore store; - Future actualUrl(WidgetTester tester, String avatarUrl) async { + Future actualUrl(WidgetTester tester, String avatarUrl, [double? size]) async { addTearDown(testBinding.reset); await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); store = await testBinding.globalStore.perAccount(eg.selfAccount.id); @@ -548,7 +548,7 @@ void main() { prepareBoringImageHttpClient(); await tester.pumpWidget(GlobalStoreWidget( child: PerAccountStoreWidget(accountId: eg.selfAccount.id, - child: AvatarImage(userId: user.userId)))); + child: AvatarImage(userId: user.userId, size: size ?? 30)))); await tester.pump(); await tester.pump(); tester.widget(find.byType(AvatarImage)); @@ -571,6 +571,26 @@ void main() { debugNetworkImageHttpClientProvider = null; }); + testWidgets('absolute URL, larger size', (tester) async { + tester.view.devicePixelRatio = 2.5; + addTearDown(tester.view.resetDevicePixelRatio); + + const avatarUrl = 'https://example/avatar.png'; + check(await actualUrl(tester, avatarUrl, 50)).isNotNull() + .asString.equals(avatarUrl.replaceAll('.png', '-medium.png')); + debugNetworkImageHttpClientProvider = null; + }); + + testWidgets('relative URL, larger size', (tester) async { + tester.view.devicePixelRatio = 2.5; + addTearDown(tester.view.resetDevicePixelRatio); + + const avatarUrl = '/avatar.png'; + check(await actualUrl(tester, avatarUrl, 50)) + .equals(store.tryResolveUrl('/avatar-medium.png')!); + debugNetworkImageHttpClientProvider = null; + }); + testWidgets('smoke with invalid URL', (tester) async { const avatarUrl = '::not a URL::'; check(await actualUrl(tester, avatarUrl)).isNull();