element-hq / element-android

A Matrix collaboration client for Android.
https://element.io/
GNU Affero General Public License v3.0
3.39k stars 732 forks source link

URL preview images should not be zoomed in #5755

Open HarHarLinks opened 2 years ago

HarHarLinks commented 2 years ago

Your use case

What would you like to do?

show thumbnail of the whole picture, not some arbitrary zoomed in part

Why would you like to do it?

How would you like to achieve it?

just use the whole picture

Have you considered any alternatives?

No response

Additional context

No response

HarHarLinks commented 2 years ago

E android screenshot: image

E web screenshot: image

Signal screenshot: image

ouchadam commented 2 years ago

this is caused by https://github.com/vector-im/element-android/blob/develop/vector/src/main/java/im/vector/app/features/media/ImageContentRenderer.kt#L105

Playing around with the different scale types

CENTER CROP (current) FIT XY FIT CENTER
Screenshot_20220414_131732 Screenshot_20220414_131538 Screenshot_20220414_131447

however we can't apply these scale types directly as we'll end up stretching square/portrait images in odd ways

we'll probably want a combination of stretching and center cropping depending on the image aspect ratio

looping in design to help decide how to handle different image sizes

HarHarLinks commented 2 years ago

stretching and center cropping

Can you not simply fit the frame to the image instead of trying to fit the image to the frame?

ouchadam commented 2 years ago

then we end up with https://github.com/vector-im/element-android/issues/4741 :sweat:

HarHarLinks commented 2 years ago

Ok so I can agree there are both lower and upper limits to the dimensions to have this work in a reasonable manner. I would

  1. scale image to screen width
  2. if it is then too high, instead scale by the upper height limit and pad the width with some color (generated from the closest real pixel if you're fancy)
  3. if it is below the minimum height (to have a clickable size), center-crop, because this is a really weird edge case image
  4. if 3 but its full size has almost no height (absolute pixels) so it would just blur, then don't display an image preview at all
ofalvai commented 2 years ago

I noticed the same issue today, and while I don't know what is the best solution to this problem, I found a website that reimplements the layout of Facebook, Twitter, Discord, etc. and lets you preview various URLs: https://www.opengraph.xyz/

I'm not sure how we could translate that layout into Android layout rules, but those preview cards look good no matter how I resize the browser window and what URL I preview.

HarHarLinks commented 2 years ago

preview cards look good no matter how I resize the browser window

view image ![image](https://user-images.githubusercontent.com/2803622/163672007-ed73048f-ba92-4e95-a5f1-bbaa87eed0ea.png)

Discord looks good on a mobile-ish narrow browser window, twitter gets lucky because it only shaves off white space, the others aren't perfect in my opinion.

If instead of fixing the height of the card__image <div> and displaying the image via background-image you specify no dimension and include the image with a regular <img> you get the same effect as discord's proper sizing (because that is what discord does, at least according to opengraph.xyz)

view image ![image](https://user-images.githubusercontent.com/2803622/163672437-5cdc750c-dc27-4f3f-8759-c1ad5bd529d5.png)

Now I realize android probably isn't css, but the same principle can apply. Since the image dimensions are known from the event metadata, you can apply above rules beforehand.