Skip to content

Fix avatar blurriness in profile screen #534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions lib/model/avatar_url.dart
Original file line number Diff line number Diff line change
@@ -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;
}
}
21 changes: 17 additions & 4 deletions lib/widgets/content.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -1027,7 +1028,7 @@ class Avatar extends StatelessWidget {
return AvatarShape(
size: size,
borderRadius: borderRadius,
child: AvatarImage(userId: userId));
child: AvatarImage(userId: userId, size: size));
}
}

Expand All @@ -1040,9 +1041,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) {
Expand All @@ -1057,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,
);
}
}

Expand Down
8 changes: 5 additions & 3 deletions lib/widgets/recent_dm_conversations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ class RecentDmConversationsItem extends StatelessWidget {
final DmNarrow narrow;
final int unreadCount;

static const double _avatarSize = 32;

@override
Widget build(BuildContext context) {
final store = PerAccountStoreWidget.of(context);
Expand All @@ -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'])
Expand All @@ -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),
Expand Down
34 changes: 34 additions & 0 deletions test/model/avatar_url_test.dart
Original file line number Diff line number Diff line change
@@ -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);
});
});
}
24 changes: 22 additions & 2 deletions test/widgets/content_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ void main() {
group('AvatarImage', () {
late PerAccountStore store;

Future<Uri?> actualUrl(WidgetTester tester, String avatarUrl) async {
Future<Uri?> 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);
Expand All @@ -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));
Expand All @@ -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();
Expand Down