zulip / zulip-mobile

Zulip mobile apps for Android and iOS.
https://zulip.com/apps/
Apache License 2.0
1.3k stars 656 forks source link

Profile Image is low resolution #3358

Closed NidhiKJha closed 4 years ago

NidhiKJha commented 5 years ago

The profile picture of an account when clicked is low resolution and blurry. Steps to reproduce:

timabbott commented 5 years ago

On the web, we use the -medium size avatar image for large profile pictures. (Actually, technically, we render the small avatar first while we load the higher-resolution version, to provide a more instant initial rendering)

NidhiKJha commented 5 years ago

Why is the loading so slow then? I'm on a good Wi-fi connection.

timabbott commented 5 years ago

I suppose the other possibility here is that we are using the medium size image in the app, and the specific original image you're looking at was just a blurry image.

borisyankov commented 5 years ago

I have noticed we were showing lower resolution on mobile when a higher resolution was shown on the web.

NidhiKJha commented 5 years ago

Can I work on this issue?

jainkuniya commented 5 years ago

Thanks @NidhiKJha for tracking this down. And ya! sure you can work on it.

Here are few observation of mine:

or mine test account avatar: https://secure.gravatar.com/avatar/54b6910062c636a4f836bd57c02823e0?d=identicon&s=500&version=1&

So now this is clear that we are passing/handling medium correctly. And the problem is only in avatar which comes from https://secure.gravatar.com.

On the quick look on the code: I found that we have a logic for getting url, basically CZO url or https://secure.gravatar.com url.

getGravatarFromEmail is for constructing gavatar url, which can take size, and default value is 80.

And in Avatar.js we are not calling getGravatarFromEmail with size, thus always a small size image is fetched (with size 80) and is stretched in the placeholder, and gets blurred.

Just passing size to getGravatarFromEmail fixes this, like:

const fullAvatarUrl =
      typeof avatarUrl === 'string'
        ? getFullUrl(avatarUrl, realm)
        : getGravatarFromEmail(email, size);
gnprice commented 4 years ago

It looks like this was fixed as part of #3344, specifically at f29954bbb90fea0a7a5fbf32dd3629095a7deb67. That was a few days after this issue, happily, but we neglected to refer back to it or close it.

Specifically the fix looked like this suggestion:

Just passing size to getGravatarFromEmail fixes this, like:

const fullAvatarUrl =
      typeof avatarUrl === 'string'
        ? getFullUrl(avatarUrl, realm)
        : getGravatarFromEmail(email, size);