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

Cropped thumbnails open distorted #854

Closed
csborgman opened this issue Jun 2, 2015 · 11 comments
Closed

Cropped thumbnails open distorted #854

csborgman opened this issue Jun 2, 2015 · 11 comments

Comments

@csborgman
Copy link

Works perfect when only width is set (example: W260px X H999px), but if I set a fixed thumbnail size there is a problem when opening the images. They open stretched then quickly "snap" back to normal.

Short video: http://screencast-o-matic.com/watch/co1niIf2VV

Test website: http://master.lightboxlive.com/portfolios/
Please check soon as I makes changes when testing.

Any ideas?

@dimsemenov
Copy link
Owner

Thumbnail width/height ratio must match large image ratio if you want zoom in/out animation to work properly. Otherwise – disable the opening/closing transition entirely and do not pass small thumbnail (do not define msrc).

@csborgman
Copy link
Author

I don't know how to do this, can you give me some suggestions? Thanks.

@dimsemenov
Copy link
Owner

Please elaborate what exactly part you don't understand.

@csborgman
Copy link
Author

I'm not a programmer so I need some hints to what I'm looking for and where to find it to do as you've suggested:

• disable the opening/closing transition entirely
• do not pass small thumbnail (do not define msrc).
• what is "msrc?"

Sorry for my ignorance. If it's complicated I won't be able to do it so I'll hire someone. But I'd like to be able to better point him to what needs to be done. No one knows better than the author.

Thanks.

@dimsemenov
Copy link
Owner

msrc is a property of slide object that defined path to the small thumbnail http://photoswipe.com/documentation/getting-started.html#creating-slide-objects-array , if you don't define it – zoom animation won't run.


To disable closing/opening transitions add options:

hideAnimationDuration:0,
showAnimationDuration: 0

http://photoswipe.com/documentation/options.html


@louy
Copy link
Contributor

louy commented Jun 7, 2015

I think we can solve this issue easily by centering .pswp__img--placeholder.

For example, for a placeholder of width 500px and height 400px:
position: absolute; top: 50%; left: 50%; margin: -200px 0 0 -250px;

@louy
Copy link
Contributor

louy commented Jun 12, 2015

What do you think @dimsemenov?

@dimsemenov
Copy link
Owner

I think that if you can't get a thumbnail image that matches an aspect ratio of a large image (which is quite weird, as it can be cropped via CSS if you need "square" initial grid items) just don't serve it, PhotoSwipe will just display dark rectangle behind it. It looks not very good when image with different aspect ratio loads on top even if it's centered.

@louy
Copy link
Contributor

louy commented Jun 12, 2015

I don't think cropping with css is a solution as we shouldn't mess with the page layout. You might be right about the animation. But in my opinion it's better to make it centred than to leave it as is. As far as I know many other plugins implement it this way too and it looks great.
I can submit a PR if you want. It shouldn't be more than a few lines.

@dimsemenov
Copy link
Owner

As far as I know many other plugins implement it this way too and it looks great.

Can you please link?

I can submit a PR if you want. It shouldn't be more than a few lines.

Sure, you may send PR, if it'll be compact – I'll merge it. https://github.com/dimsemenov/PhotoSwipe/blob/master/src/js/items-controller.js#L431

@louy
Copy link
Contributor

louy commented Jun 12, 2015

I'm not sure where I've seen that. I did an experiment with FancyApps and apparently they preload the image and then stretch/squeeze it to match the thumbnail, which is not that nice.
http://output.jsbin.com/cazacimomi/1/

I think centring it would be better than this. Anyway I'll implement it and then we'll see. Thanks btw!

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

3 participants