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

Lightbox doesn't handle hi-dpi images (and shows retina images at 2x physical size rather than 1x physical size) #18496

Open
ara4n opened this issue Aug 11, 2021 · 7 comments
Labels
A-Media O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Low/no impact on users T-Enhancement Z-Platform-Specific

Comments

@ara4n
Copy link
Member

ara4n commented Aug 11, 2021

No description provided.

@SimonBrandner
Copy link
Contributor

The current behaviour was a conscious decision, see #17049

@SimonBrandner SimonBrandner added A-Media T-Enhancement X-Needs-Design O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Tolerable Low/no impact on users labels Aug 11, 2021
@SimonBrandner
Copy link
Contributor

Related: #17114

@ara4n
Copy link
Member Author

ara4n commented Aug 11, 2021

The current behaviour was a conscious decision, see #17049

...but #17049 was asking for the same behaviour that i'm asking for here: if the image is small, it starts at 100%, and you would click to zoom. If it's big, it's shrunk to fit on the page, and you click to zoom.

So the current behaviour may be a conscious decision, but i'm not sure it's the right one?

matrix-org/matrix-react-sdk#5916 sounds promising in terms of specifying dynamic zoom settings - can we change it so that we display at 100% by default (unless the image needs to be shrunk to fit on the page?)

@robintown
Copy link
Member

I'm confused - isn't it already the current behavior to show images at 100% zoom by default, shrinking to fit the screen if necessary? For example, for an image that is smaller than the lightbox at 100% zoom, this is what I see: (namely, 100% zoom)

lightbox

@SimonBrandner
Copy link
Contributor

I'm confused - isn't it already the current behavior to show images at 100% zoom by default, shrinking to fit the screen if necessary?

Yes, it should be

@SimonBrandner SimonBrandner added the X-Needs-Info This issue is blocked awaiting information from the reporter label Aug 11, 2021
@ara4n
Copy link
Member Author

ara4n commented Aug 14, 2021

Ah - the actual bug may be that the lightbox doesn't respect hi-dpi images (but the timeline does). So if you take a screenshot on macOS (e.g. the PNG below), the timeline renders it correctly at 2x dpi, but both the upload preview and then the lightbox view it at 1x dpi (i.e. twice as physically large as it should be).

Screenshot 2021-08-13 at 14 35 39

given 99% of the images i post on EW are screenshots, i assumed it was doing it for everything.

(n.b. github correctly handles hi-dpi images, and shows the image here at the right physical size).

@ara4n ara4n changed the title Should lightbox show images at 100% zoom by default, rather than oversampling them by zooming them to fill the screen? Lightbox doesn't handle hi-dpi images (and shows retina images at 2x physical size rather than 1x physical size) Aug 14, 2021
@ara4n ara4n removed X-Needs-Info This issue is blocked awaiting information from the reporter X-Needs-Design labels Aug 14, 2021
@ara4n
Copy link
Member Author

ara4n commented Aug 14, 2021

see also #9530

@SimonBrandner SimonBrandner added O-Occasional Affects or can be seen by some users regularly or most users rarely Z-Platform-Specific and removed O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Aug 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Media O-Occasional Affects or can be seen by some users regularly or most users rarely S-Tolerable Low/no impact on users T-Enhancement Z-Platform-Specific
Projects
None yet
Development

No branches or pull requests

3 participants