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

Wrong image size and positioning on "Retina"-screen devices + portrait orientation #1021

Closed
hatsumatsu opened this issue Nov 16, 2015 · 6 comments

Comments

@hatsumatsu
Copy link

It seems that there is something wrong with the calculation of the image size on devices with a high pixel density AKA "Retina"-Screens. When I open photoswipe with a portrait image on both an Android Tablet (Chrome on Samsung Galaxy Tab S) and an iPad (Safari iPad Retina iOS 8.1), the image is taller than the screen, overlapping the UI and caption.

Try this demo (open first image):

http://s.codepen.io/superstructure-net/debug/GpzWjP

However I cannot reproduce this in desktop browsers...

@mjau-mjau
Copy link

Not quite sure about codepen's involvment, but you need to add meta viewport:

<meta name="viewport" content="width = device-width, initial-scale = 1.0">

Works here:
http://s.codepen.io/mjau-mjau/debug/ZbwJLp

@hatsumatsu
Copy link
Author

To be more specific, the image is not taller but scaled to 100% of the screen height, not respecting the barSizesetting, thus being overlapped by the toolbar and caption (and my custom UI). It seems that photoSwipe ignores the barSize option whenever _fitControlsInViewport() return true ( https://github.com/dimsemenov/PhotoSwipe/blob/master/src/js/ui/photoswipe-ui-default.js#L149 ), which is the case on screen width < 1200px.

This value seems a little arbitrary, is there a way to adjust these settings?

@dimsemenov
Copy link
Owner

barsSize is indeed ignored on devices with hardware touch support, without mouse, and which screen size is smaller then 1200px. As when bars are enabled on small devices, there is too few space for the image.

For now there is no option to disable this, except modifying the default UI JS file. But, I think adding a setting that would allow adjust this 1200 offset is a good idea:

return !pswp.likelyTouchDevice || _options.mouseUsed || screen.width > _options.fitControlsWidth;

so developer will be able to set it from options fitControlsWidth:1200. Do you agree with this solution, or you have a better one?

@hatsumatsu
Copy link
Author

Sounds perfect! Thx for the quick feedback.

@davidhariri
Copy link

davidhariri commented Jan 4, 2018

@dimsemenov Is this an option now? I'm having the same issue on iPhone X with a long image for a client project...

simulator screen shot - iphone x - 2018-01-04 at 12 00 53

@davidhariri
Copy link

I've patched my ui js file by overriding _fitControlsInViewport to always return true. Is there a risk in doing that that I may not be seeing? Thank you 😌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants