Skip to content
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

Should use smaller version of user icons in main view #1711

Closed
wlach opened this issue Apr 5, 2019 · 2 comments · Fixed by #1933
Closed

Should use smaller version of user icons in main view #1711

wlach opened this issue Apr 5, 2019 · 2 comments · Fixed by #1933

Comments

@wlach
Copy link
Contributor

wlach commented Apr 5, 2019

On the front page, we currently get github and gravatar images using their native sizes, which are rather large (80x80 in the case of gravatar, up to 500x500 in the case of github)

We get the URL here:

notebooks = (

(the part of the query where we fetch the avatar URL is owner__avatar)

And render it in react here:

const Avatar = ({ src }) => <img src={src} alt={src} />;

This results in a large amount of unnecessary bandwidth being used and the pages loading more slowly.

For gravatar, we should be able to add s=30 to the URL to get them at the right size: https://en.gravatar.com/site/implement/images/

Github seems to support the same, e.g. https://avatars3.githubusercontent.com/u/20569?v=4&s=30

To do this properly, I'd recommend a small helper function on the client that takes a URL and adds either a ?s=30 or a &s=30 to it. I'm a bit torn on whether to do this on the client or server side, but am leaning towards doing it on the client. @robhudson @rafrombrc this would be another good bug to tackle.

@teonbrooks
Copy link
Contributor

@wlach what about adding it to the url in the middleware here? is there a disadvantage adding to the server side?

@wlach
Copy link
Contributor Author

wlach commented Jun 12, 2019

@wlach what about adding it to the url in the middleware here? is there a disadvantage adding to the server side?

I would lean against adding it to that piece of middleware, as some API endpoints might wind up using whatever is calculated there -- and whatever uses that may not always want 30x30 dimensions.

In general I think it's best to keep all the UI decisions in src/ (i.e. the js/css code we compile with webpack). This makes it easier for someone to know where a change should be made in cases like this -- obviously every rule has exceptions, the recent favicon work you did would be an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants