search

hckrs / hckrs.io

Social community for hackers
http://hckrs.io
Other
29 stars 2 forks source link

Reduce size of profile images #99

Closed Dandandan closed 9 years ago

Dandandan commented 9 years ago

The new front page (it's wonderful BTW! ) uses a lot of profile images, mostly from Github. At the moment, the full versions of the images are used, but it may be wise to reduce the resolution of the images (&s=64). Maybe we could also use the bengott:meteor-avatar package (also used by Telescope)? That may require some more time, I guess.

ramshorst commented 9 years ago

I am very happy with the prontpage two :D The bengott:meteor-avatar package looks good. I do not fully understand what is does. What are the parts that you think are interesting @Dandandan ? On thing there is to say for how we handle it today is that it's kinda 'retina'/hight resolution. 140px /2 = ~64px. But i guess that that is kind of a silly way to do it^^ What are the interesting parts

Dandandan commented 9 years ago

Yes, 128x128 or 140x140 might be ok too, but ATM they are not resized.

The package may simplify our own code a bit, as this does exactly the same. So it may not necessarily improve things, but at least we can remove some code.

Dandandan commented 9 years ago

What I said was not completely true: it seems Facebook & Github pics are OK, but Twitter profile pictures are in original resolution. We should add a 200x200 resolution like: http://pbs.twimg.com/profile_images/477746684919701505/PFQK8zQY_200x200.jpeg

@JarnoLeConte what would be the best way to change the pictures to the 200x200 versions?

JarnoLeConte commented 9 years ago

O really, is this possible, because they don't mentioned it here and developers were complaining about this too. But it sounds great that we can choose a dimension manually.

The best way to implement: In one of our packages, I guess "accounts", the twitter data is retrieved through the Twitter API. There is some function in there that post-process the retrieved data. There you can modify the Twitter URL to make use of the right dimension.

Notes: A Twitter picture uses a static URL. Only once per hour or once per day the server checks all twitter accounts to see if the URL is updated. You can call this manually to test your development. An example of such call can be found in /web/server/{startup|server}.js

Op 9 mrt. 2015 om 00:29 heeft Dandandan notifications@github.com het volgende geschreven:

What I said was not completely true: it seems Facebook & Github pics are OK, but Twitter profile pictures are in original resolution. We should add a 200x200 resolution like: http://pbs.twimg.com/profile_images/477746684919701505/PFQK8zQY_200x200.jpeg

@JarnoLeConte what would be the best way to change the pictures to the 200x200 versions?

— Reply to this email directly or view it on GitHub.

Dandandan commented 9 years ago

Thanks for the help, yes they didn't document it, but at quite a few places this is used (also in the meteor-avatar package), so I think using this is pretty safe.

I think we can close this issue as pictures from Facebook / Twitter / Github now use lower resolutions, maybe in the future we can try the meteor-avatar package :).

Dandandan commented 9 years ago

BTW, this has probably a bigger impact on the loading speed than I thought it did, when viewing for example the Lyon city, many pictures from Twitter are in the range of 0.5 - 1.0 MB, which makes loading the hackers / map pages much slower.

JarnoLeConte commented 9 years ago

Do you mean that it now runs faster? Or do you mean that requesting resized images takes longer?

Op 9 mrt. 2015 om 11:04 heeft Dandandan notifications@github.com het volgende geschreven:

BTW, this has probably a bigger impact on the loading than I thought it did, when viewing for example the Lyon city, many pictures from Twitter are in the range of 0.5 - 1.0 MB, which makes loading the hackers / map page much slower.

— Reply to this email directly or view it on GitHub.

Dandandan commented 9 years ago

It now runs (sometime much) faster.

ramshorst commented 9 years ago

Excellent jobs guys!

Dandandan commented 9 years ago

Seems it did only convert some pictures to the 200x200 size.

JarnoLeConte commented 9 years ago

I already see. The pictures are not working. So what we can do is test if 200x200 exists, otherwise we must take the original one OR maybe we must choose some smaller one to improve performance.

Op 10 mrt. 2015 om 15:04 heeft Dandandan notifications@github.com het volgende geschreven:

Seems it did only convert some pictures to the 200x200 size.

— Reply to this email directly or view it on GitHub.

Dandandan commented 9 years ago

Which pictures are not working? Here they seem they show op normal. I meant they didn't convert to the _200x200 in the filename .

JarnoLeConte commented 9 years ago

What about the others? Are they still working or just not resized? And what are alternatives to resize Twitter pictures, there are none, right?

Op 10 mrt. 2015 om 15:04 heeft Dandandan notifications@github.com het volgende geschreven:

Seems it did only convert some pictures to the 200x200 size.

— Reply to this email directly or view it on GitHub.

Dandandan commented 9 years ago

I think the _200x200 solution always works, but the code doesn't update all the pictures at the moment.

Dandandan commented 9 years ago

Could it be that most twitter pictures don't end with _normal.ext? Then it could be a simple one line change to convert those too.

https://github.com/hckrs/hckrs.io/blob/cb6d419af6039f36ed2268bb1fa16235ff734ce4/web/packages/account/account/servicedata.jsdoc#L134

JarnoLeConte commented 9 years ago

It looks like there are pictures NOT containing the _normal.ext suffix. What is your suggestion for modifying the regex?

Dandandan commented 9 years ago

See #111

Dandandan commented 9 years ago

Thanks for checking!

Dandandan commented 9 years ago

Would be cool if this is going to work :)

Dandandan commented 9 years ago

Closing for now, hope it's working now!