Skip to content

Commit 7ba0055

Browse files
committed
avatar: Fixed image blurriness by changing resolution
fixed image blurriness of profile by changing image resolution based on size and device pixel ratio
1 parent 45e4434 commit 7ba0055

File tree

5 files changed

+134
-9
lines changed

5 files changed

+134
-9
lines changed

lib/model/avatar_url.dart

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/// The size threshold above which is "medium" size for an avatar.
2+
///
3+
/// This is DEFAULT_AVATAR_SIZE in zerver/lib/upload.py.
4+
const defaultUploadSizePx = 100;
5+
6+
abstract class AvatarUrl {
7+
factory AvatarUrl.fromUserData({required Uri resolvedUrl}) {
8+
// TODO(#255): handle computing gravatars
9+
// TODO(#254): handle computing fallback avatar
10+
if (resolvedUrl.toString().startsWith(GravatarUrl.origin)) {
11+
return GravatarUrl(resolvedUrl: resolvedUrl);
12+
} else {
13+
return UploadedAvatarUrl(resolvedUrl: resolvedUrl);
14+
}
15+
}
16+
17+
Uri get(int sizePhysicalPx);
18+
}
19+
20+
class GravatarUrl implements AvatarUrl {
21+
GravatarUrl({required Uri resolvedUrl}) : standardUrl = resolvedUrl;
22+
23+
static String origin = 'https://secure.gravatar.com';
24+
25+
Uri standardUrl;
26+
27+
@override
28+
Uri get(int sizePhysicalPx) {
29+
return standardUrl.replace(queryParameters: {
30+
...standardUrl.queryParameters,
31+
's': sizePhysicalPx.toString(),
32+
});
33+
}
34+
}
35+
36+
class UploadedAvatarUrl implements AvatarUrl {
37+
UploadedAvatarUrl({required Uri resolvedUrl}) : standardUrl = resolvedUrl;
38+
39+
Uri standardUrl;
40+
41+
@override
42+
Uri get(int sizePhysicalPx) {
43+
if (sizePhysicalPx > defaultUploadSizePx) {
44+
return standardUrl.replace(
45+
path: standardUrl.path.replaceFirst(RegExp(r'(?:\.png)?$'), "-medium.png"));
46+
}
47+
48+
return standardUrl;
49+
}
50+
}

lib/widgets/content.dart

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
77

88
import '../api/core.dart';
99
import '../api/model/model.dart';
10+
import '../model/avatar_url.dart';
1011
import '../model/binding.dart';
1112
import '../model/content.dart';
1213
import '../model/internal_link.dart';
@@ -1027,7 +1028,7 @@ class Avatar extends StatelessWidget {
10271028
return AvatarShape(
10281029
size: size,
10291030
borderRadius: borderRadius,
1030-
child: AvatarImage(userId: userId));
1031+
child: AvatarImage(userId: userId, size: size));
10311032
}
10321033
}
10331034

@@ -1040,9 +1041,11 @@ class AvatarImage extends StatelessWidget {
10401041
const AvatarImage({
10411042
super.key,
10421043
required this.userId,
1044+
required this.size,
10431045
});
10441046

10451047
final int userId;
1048+
final double size;
10461049

10471050
@override
10481051
Widget build(BuildContext context) {
@@ -1057,9 +1060,19 @@ class AvatarImage extends StatelessWidget {
10571060
null => null, // TODO(#255): handle computing gravatars
10581061
var avatarUrl => store.tryResolveUrl(avatarUrl),
10591062
};
1060-
return (resolvedUrl == null)
1061-
? const SizedBox.shrink()
1062-
: RealmContentNetworkImage(resolvedUrl, filterQuality: FilterQuality.medium, fit: BoxFit.cover);
1063+
1064+
if (resolvedUrl == null) {
1065+
return const SizedBox.shrink();
1066+
}
1067+
1068+
final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: resolvedUrl);
1069+
final physicalSize = (MediaQuery.of(context).devicePixelRatio * size).ceil();
1070+
1071+
return RealmContentNetworkImage(
1072+
avatarUrl.get(physicalSize),
1073+
filterQuality: FilterQuality.medium,
1074+
fit: BoxFit.cover,
1075+
);
10631076
}
10641077
}
10651078

lib/widgets/recent_dm_conversations.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ class RecentDmConversationsItem extends StatelessWidget {
8181
required this.unreadCount,
8282
});
8383

84+
static const double _avatarSize = 32;
85+
8486
final DmNarrow narrow;
8587
final int unreadCount;
8688

@@ -94,14 +96,14 @@ class RecentDmConversationsItem extends StatelessWidget {
9496
switch (narrow.otherRecipientIds) { // TODO dedupe with DM items in [InboxPage]
9597
case []:
9698
title = selfUser.fullName;
97-
avatar = AvatarImage(userId: selfUser.userId);
99+
avatar = AvatarImage(userId: selfUser.userId, size: _avatarSize);
98100
case [var otherUserId]:
99101
// TODO(#296) actually don't show this row if the user is muted?
100102
// (should we offer a "spam folder" style summary screen of recent
101103
// 1:1 DM conversations from muted users?)
102104
final otherUser = store.users[otherUserId];
103105
title = otherUser?.fullName ?? '(unknown user)';
104-
avatar = AvatarImage(userId: otherUserId);
106+
avatar = AvatarImage(userId: otherUserId, size: _avatarSize);
105107
default:
106108
// TODO(i18n): List formatting, like you can do in JavaScript:
107109
// new Intl.ListFormat('ja').format(['Chris', 'Greg', 'Alya'])
@@ -122,7 +124,7 @@ class RecentDmConversationsItem extends StatelessWidget {
122124
child: ConstrainedBox(constraints: const BoxConstraints(minHeight: 48),
123125
child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [
124126
Padding(padding: const EdgeInsetsDirectional.fromSTEB(12, 8, 0, 8),
125-
child: AvatarShape(size: 32, borderRadius: 3, child: avatar)),
127+
child: AvatarShape(size: _avatarSize, borderRadius: 3, child: avatar)),
126128
const SizedBox(width: 8),
127129
Expanded(child: Padding(
128130
padding: const EdgeInsets.symmetric(vertical: 4),

test/model/avatar_url_test.dart

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import 'package:checks/checks.dart';
2+
import 'package:test/scaffolding.dart';
3+
import 'package:zulip/model/avatar_url.dart';
4+
5+
void main() {
6+
const defaultSize = 30;
7+
const largeSize = 120;
8+
9+
group('GravatarUrl', () {
10+
test('gravatar url', () {
11+
final url = '${GravatarUrl.origin}/avatar/1234';
12+
final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: Uri.parse(url));
13+
14+
check(avatarUrl.get(defaultSize).toString()).equals('$url?s=30');
15+
});
16+
});
17+
18+
group('UploadedAvatarUrl', () {
19+
test('png image', () {
20+
const url = 'https://zulip.example/image.png';
21+
final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: Uri.parse(url));
22+
23+
check(avatarUrl.get(defaultSize).toString()).equals(url);
24+
});
25+
26+
test('png image, larger size', () {
27+
const url = 'https://zulip.example/image.png';
28+
final avatarUrl = AvatarUrl.fromUserData(resolvedUrl: Uri.parse(url));
29+
final expectedUrl = url.replaceAll('.png', '-medium.png');
30+
31+
check(avatarUrl.get(largeSize).toString()).equals(expectedUrl);
32+
});
33+
});
34+
}

test/widgets/content_test.dart

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,7 @@ void main() {
538538
group('AvatarImage', () {
539539
late PerAccountStore store;
540540

541-
Future<Uri?> actualUrl(WidgetTester tester, String avatarUrl) async {
541+
Future<Uri?> actualUrl(WidgetTester tester, String avatarUrl, [double? size]) async {
542542
addTearDown(testBinding.reset);
543543
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
544544
store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
@@ -548,7 +548,7 @@ void main() {
548548
prepareBoringImageHttpClient();
549549
await tester.pumpWidget(GlobalStoreWidget(
550550
child: PerAccountStoreWidget(accountId: eg.selfAccount.id,
551-
child: AvatarImage(userId: user.userId))));
551+
child: AvatarImage(userId: user.userId, size: size ?? 30))));
552552
await tester.pump();
553553
await tester.pump();
554554
tester.widget(find.byType(AvatarImage));
@@ -571,6 +571,32 @@ void main() {
571571
debugNetworkImageHttpClientProvider = null;
572572
});
573573

574+
testWidgets('absolute URL, larger size', (tester) async {
575+
// setting the device pixel ratio to 2.5, effective image
576+
// size is (2.5 * 50) 125px that exceeds the defaultUploadSizePx
577+
tester.view.devicePixelRatio = 2.5;
578+
// reset the device pixel ratio after test completion
579+
addTearDown(tester.view.resetDevicePixelRatio);
580+
581+
const avatarUrl = 'https://example/avatar.png';
582+
check(await actualUrl(tester, avatarUrl, 50)).isNotNull()
583+
.asString.equals(avatarUrl.replaceAll('.png', '-medium.png'));
584+
debugNetworkImageHttpClientProvider = null;
585+
});
586+
587+
testWidgets('relative URL, larger size', (tester) async {
588+
// setting the device pixel ratio to 2.5, effective image
589+
// size is (2.5 * 50) 125px that exceeds the defaultUploadSizePx
590+
tester.view.devicePixelRatio = 2.5;
591+
// reset the device pixel ratio after test completion
592+
addTearDown(tester.view.resetDevicePixelRatio);
593+
594+
const avatarUrl = '/avatar.png';
595+
check(await actualUrl(tester, avatarUrl, 50))
596+
.equals(store.tryResolveUrl('/avatar-medium.png')!);
597+
debugNetworkImageHttpClientProvider = null;
598+
});
599+
574600
testWidgets('smoke with invalid URL', (tester) async {
575601
const avatarUrl = '::not a URL::';
576602
check(await actualUrl(tester, avatarUrl)).isNull();

0 commit comments

Comments
 (0)