francoischalifour / medium-zoom

🔎🖼 A JavaScript library for zooming images like Medium
https://medium-zoom.francoischalifour.com
MIT License
3.59k stars 164 forks source link

Wrong dimensions: take picture elements into account? #33

Open mimamuh opened 6 years ago

mimamuh commented 6 years ago

Hi,

I like the simplicity of this lib. I recognized, that the lib doesn't handle <picture> elements properly in the sense, when I use the <picture> element for art directed images – this means, I have different resolutions and crop sizes of the image based on the device size – it could lead to distorted image (dimensions).

For example: My mobile image is a crop of the desktop image showing only a detail of it with the dimension 10:8 and the desktop image is 16:9.

When I use the 10:8 as my default <img /> within the <picture /> element, but I am on the desktop view and click on the image, then medium-zoom uses the default image from <img />. But as they have different dimensions (10:8 vs 16:9) the image is distorted (it has mapped the 10:8 to the 16:9 dimension as the default image is 10:8 but the dimensions medium-zoom is using are based on the responsive image on desktop with the dimensions of 16:9).

So it even doesn't work when I use the data-zoom-target attribute. If the data-zoom-target image has a different dimension to the current visible one, it is distorted too.

Example of a picture element:

<picture >
    <source
        media="(min-width: 880px)"
        srcset="./../assets/img/home/example-guy_d.jpg"> <!-- may has a dimension of 16:9 -->
    <img
        class="o-section-stock__photo-img"
        src="./../assets/img/home/example-guy_m.jpg" <!-- may has a dimension of 10:8 -->
        alt="…"
        data-zoom="medium-zoom"
    >
</picture>

I know, it is complicated. I'm not sure if there is a simple solution for that. My first idea was that medium-zoom could read the srcset and decide which image it should use/is the current visible one on the screen and use it. But I think it will be a longer project with all the browser polyfills needed etc.

So maybe the easiest would be that medium-zoom takes the dimensions of the data-zoom-target into account when it uses this as the image source. So it would be a simple workaround for picture elements to use the data-zoom-target. But of course, it still needs a way to scale from dimension A to B ... but maybe easier than hacking around with srcset and all it's -maybe- possibilities ...

francoischalifour commented 6 years ago

Hi @MiMaMuh,

Dynamic image sources is something @ismay and I have been researching (see #27). We've come to the conclusion that accepting srcset as dimensions for the zoomed image would be a nice feature. It would solve most dimension issues without requiring extra work from the user (the user won't have to specify all the image versions to medium-zoom). This is something that I'll implement if we don't come up with a better alternative.

To be honest, I think that handling different sources with the picture element is out of scope for medium-zoom. The main reason being that we'd need to start selecting sibling nodes as opposed to the unique node that the srcset solution provides. It'd add a lot of complexity to the project for not enough value (there's not a lot of demand for supporting picture sources yet).

Would bringing srcset support solve your problem?

mimamuh commented 6 years ago

Hi @francoischalifour, thanks for your answer. I already saw the post. I think, srcset alone wouldn't solve the issue. Because the issue is more, that medium-zoom takes the dimension from the current visible image. When I use picture then the dimensions may be different because of art direction decisions.

For me it would be fine when medium-zoom always use the default image of the pictrue element which is basically the <img> element within <picture>. But then it has to figure out dynamically the dimensions from the <img> element when it zooms it, because they may differe ... Maybe I just post a repro next week to demonstrate you the issue. :)

gfellerph commented 6 years ago

I could imagine a similar fix for this problem as just PR'd for srcset. I never had the pleasure to work with art directed picture elements so I'd actually like to see a more or less complicated example to test with. Can you post something, @MiMaMuh ? Maybe there is a way to just let the browser decide which image to load. Just taking the fallback <img> would be a feasible option too, I believe.

mimamuh commented 6 years ago

@tuelsch I made a simple example on codesandbox.io for you. I coloured the three images. Each image has a different dimension as it is common when using art directed images. And to see the problem arise, I added a text, a square and a circle and of course different colors to show what the issue would be in such an case.

knynkwl commented 5 years ago

I'm running into a similar problem. I have 1:1 square thumbnails, but want the HD image to respect the dimensions of the actual image. Instead when clicking the thumbnail, I get a 1:1 HD image.

m1guelpf commented 4 years ago

Has anyone figured a workaround for this yet?

ShivamJoker commented 1 year ago

@mimamuh has this feature been abandoned?

TheAnachronism commented 1 year ago

has this feature been abandoned?

Looks like it