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

Find an appropriate solution for displaying images #4410

Open
4 tasks
nicodh opened this issue Dec 14, 2024 · 9 comments
Open
4 tasks

Find an appropriate solution for displaying images #4410

nicodh opened this issue Dec 14, 2024 · 9 comments
Labels
enhancement New feature or request polish ui/ux UI/UX related issues

Comments

@nicodh
Copy link
Contributor

nicodh commented Dec 14, 2024

We need a solution to meet following requirements:

  • the height of an image must be set even before the image is loaded (core provides dimensions in the message object)
  • image-only messages should not show (extreme) padding except for very narrow images
  • define a range to avoid too big and too small images
  • narrow image-only messages should display a footer wide enough for timestamp and reactions

Related to:

@nicodh nicodh added enhancement New feature or request ui/ux UI/UX related issues polish labels Dec 14, 2024
@Simon-Laux
Copy link
Member

Simon-Laux commented Dec 14, 2024

Basically the main technical issue is that we need to know the size of the message on first render to avoid a message list that jumps around as images are loaded in.

Static image sizes or cropping is a reasonable approach for now, but does not look as good for users as it could be.


my messages on this topic from the internal group/discussion (about a possible approach to use the dimensions that are provided by core):

maybe we could provide a placeholder empty image in the correct dimensions

so we would let the browser make the layout calculations

This might work (seems like you don't even need base64 encoding):

<img src='data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" x="0px" y="0px" viewBox="0 0 100 100" height="100px" width="200px"></svg>' alt="" />

though I'm not sure whats the best way to load the actual image and replace it, maybe an xhr request do that it is cached, then replace the url or sth. or have two elements and remove the placeholder on load

@nicodh
Copy link
Contributor Author

nicodh commented Dec 14, 2024

But we have the dimensions except for images "the core could not understand" and videos.

@nicodh
Copy link
Contributor Author

nicodh commented Dec 14, 2024

I would suggest: go with the dimensions coming from core and do the needed adaptions in the template (like setting limits to not have too big images or paddings)

@Simon-Laux
Copy link
Member

Simon-Laux commented Dec 14, 2024

But we have the dimensions except for images "the core could not understand" and videos.

yes, but that's an edge case for which we could have a fallback.

And maybe we could tell core to save the dimensions from the ui in these cases? But I think only fallback is just fine.

@nicodh
Copy link
Contributor Author

nicodh commented Dec 14, 2024

Let's continue this discussion once we have a concrete proposal

@Simon-Laux
Copy link
Member

Simon-Laux commented Dec 14, 2024

Let's continue this discussion once we have a concrete proposal

I'm not interested in a discussion here at the time, my goal was just to provide more information in the issue thread.

@nicodh
Copy link
Contributor Author

nicodh commented Dec 14, 2024

Ok sure - thanks for that!

@nicodh
Copy link
Contributor Author

nicodh commented Dec 18, 2024

Possible solution UI wise:

Landscape and portrait ratio images have a fixed width, portrait ratio additionally a max height (cropped if too high)

image

Extreme portrait images are cropped to a fix minimal width and maximal height:
Result:
image

Original:
image

The minimal widht should consider possible reactions and the timestamp in the footer

@iequidoo
Copy link

iequidoo commented Dec 19, 2024

Possible solution UI wise: [...]

Looks like a good solution for me as a Desktop user. But reading it, it seems to me that you even don't need to classify images as landscape and portrait ones implementation-wise, basically you just have min_width, max_width and max_height. But i'd say that min_height makes sense too to avoid very narrow horizontal panoramas (better to crop them too apparently). I'd say that min_height should be max_height / max_width * min_width (to have the same ratio limit), but min_height may be a bit less because human eye perceives vertical sizes as slightly larger.

EDIT: Also, for small pictogram-sized images it makes sense to increase them up to min_width and min_height, but no more. Still they need to be increased because of nowadays' very high resolutions screens with small pixels. EDIT: Though not sure, currently DC Desktop doesn't do this and i don't remember complaints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request polish ui/ux UI/UX related issues
Projects
None yet
Development

No branches or pull requests

3 participants