PowerlineApp / powerline-mobile

Other
4 stars 16 forks source link

Standard Image Size for Attachments in Newsfeed #480

Closed jterps08 closed 7 years ago

jterps08 commented 7 years ago

Powerline currently has a single parameter by which all images are processed for imgix - the width of the screen. This is insufficient and creates wildly different image sizes in the newsfeed. See the example below:

image

In the example below, note how there is a consistent image size for all images that appear in the newsfeed.

image

We need to apply the same concept for Powerline - consistent image size for newsfeed attachments (pictures and video-preview thumbnails). I believe this is a matter of making a few imgix query parameter adjustments here (but not sure) - https://github.com/PowerlineApp/powerline-mobile/blob/366126e4569970a6c450d63c19e814e507b73425/www/js/controllers/home.js#L332

If you have not worked with imgix image processing API before, please reference https://www.imgix.com/imgix-js

If this is not a change we make on the frontend and instead a change we make on the backend, please assign to Igor.

https://docs.imgix.com/apis/url/size/fit (clip) https://docs.imgix.com/apis/url/size/max-h https://docs.imgix.com/apis/url/auto (format and compress)

jterps08 commented 7 years ago

@devotebest I think this parameter query needs to be applied to the following places:

Newsfeed

The newsfeed slows down substantially when multiple images are in the feed and that's because we don't have this imgix query parameter on the call.

devotebest commented 7 years ago

@jterps08 could you attach screenshots to get more clear idea?

jterps08 commented 7 years ago

image

@devotebest The newsfeed are the top row of items. All this should be is appending the correct imgix query to wherever the image URL is loaded. You'll want to reference the comments above and the imgix documentation.

Anytime we load an image anywhere in the app we want to make sure that it is being processed via imgix with the appropriate parameters in place to reduce file size for faster performance...

devotebest commented 7 years ago

@jterps08 It means all images on the feed should be cropped as same height?

jterps08 commented 7 years ago

I will touch base with you in slack @devotebest

jterps08 commented 7 years ago

I believe this is closed. I will inspect the commit once available.

jterps08 commented 7 years ago

@devotebest when the user enters a web address in a post, the system correctly previews the content based on the address provided. This usually works well for web sites, but if the user enters a youtube link (user zxc did in zxcgroup1), then the system will provide a preview of that youtube link in the newsfeed. That's okay... however the size of the youtube container is huge. We need to make this the same size constraints as above work...

jterps08 commented 7 years ago

image

jterps08 commented 7 years ago

For additional clarity:

image

jterps08 commented 7 years ago

It seems that this issue was resolved but never delivered in build. Marking as closed.

devotebest commented 7 years ago

I was sure. that was done with this commit https://github.com/PowerlineApp/powerline-mobile/commit/3f7f9e9659b1a173d8c9ab228c2a50943cb78e22

Sent from my iPhone