react-compare-slider icon indicating copy to clipboard operation
react-compare-slider copied to clipboard

Add support for images with different sizes

Open jahirfiquitiva opened this issue 1 year ago • 14 comments

Hey, I'm trying to use this lib, which looks pretty good, although I'm missing a small feature

I was using react-compare-image before, and it has this aspectRatio prop feature, allowing for the slider to look fine when images aspect ratios differ, and therefore being able to view both images in full size.

Shot 2024-01-29 at 12 09 01@2x

Here's their examples: https://react-compare-image-git-master-junkboy0315.vercel.app/?path=/story/images-with-different-aspect-ratios--default-behavior

I created a sandbox showing the current behavior

https://codesandbox.io/p/devbox/infallible-shape-t5p8tl?file=/src/App.tsx:16,10

Would it be possible for you to add such feature? Or is it already possible? And if it is, how to set it up?

Thanks in advance!

jahirfiquitiva avatar Jan 29 '24 17:01 jahirfiquitiva

Hi @jahirfiquitiva, that's an interesting use case!

There's no built-in prop to do this but you could achieve what you need with a bit of extra logic.

The most foolproof way would probably be to attach a resize observer to the slider component and refs to the images to get their natural widths and heights and then calculate their aspect ratios. You'd then opt for the higher or taller one. One thing to note is that it looks like react-compare-image uses fixed widths and heights on the slider where as this lib uses the intrinsic sizing of itemOne so you'd also want to set a fixed width and height on the slider component itself if you want to mimic what react-compare-image does.

I'll take a look at getting a demo set up, if it ends up being complex I think it would be worth adding a prop to this lib similarly to react-compare-image.

nerdyman avatar Jan 29 '24 21:01 nerdyman

Thanks for your reply @nerdyman .. A demo would be incredibly helpful. Thanks in advance!

jahirfiquitiva avatar Jan 29 '24 22:01 jahirfiquitiva

@jahirfiquitiva I've got a basic demo working here https://codesandbox.io/p/sandbox/rcs-aspect-ratio-forked-fngydc?file=%2Fsrc%2FApp.tsx%3A12%2C1-13%2C1

It starts with the default behaviour (shown as undefined) then cycles through undefined|'taller'|'wider'. I ended up using the aspect-ratio CSS prop rather than setting fixed widths and heights on things which is a bit cleaner IMO.

This is still fairly basic since it won't automatically change the aspect ratio if you change the images in itemOne and itemTwo. To do that I'd either use a resize observer to fire handleAspectRatio() on resize or a useEffect to fire handleAspectRatio() on prop changes.

The logic is a bit involved so I'll have a think about how to best add this to the lib but this should hopefully get you going in the meantime!

nerdyman avatar Jan 30 '24 14:01 nerdyman

@nerdyman it says Sandbox not found, maybe it isn't public? 😬

jahirfiquitiva avatar Jan 30 '24 14:01 jahirfiquitiva

@jahirfiquitiva should be ok now 🙏

nerdyman avatar Jan 30 '24 14:01 nerdyman

@nerdyman wow, thank you so much for the help. It seems to work fine in the demo, I'll give it a try in my project later and let you know how it goes. Thanks again! 🎉

jahirfiquitiva avatar Jan 30 '24 14:01 jahirfiquitiva

@jahirfiquitiva Happy to help 😄 let me know how it goes!

nerdyman avatar Jan 30 '24 14:01 nerdyman

hey @nerdyman , it took me a bit to get back to this, sorry ... Anyway, I think this doesn't seem to work quite fine. Probably because the images are bigger than the container? 🤔

This is the result with your demo code:

This is the expected result:

I've made a PR to my website with these changes, in case you could take a look there 🙏

https://github.com/jahirfiquitiva/jahir.dev/pull/74

Thanks in advance!

jahirfiquitiva avatar Feb 03 '24 15:02 jahirfiquitiva

Hi @jahirfiquitiva, Thanks for proving the link to your use case 🙌

I think it's because the slider is still using the bounds of the first image. In the sandbox the slider is filling the width of the container so the images fit better.

I was thinking of adding a separate hook to the lib to handle this so I'll take a look at your repro and figure something out.

nerdyman avatar Feb 03 '24 16:02 nerdyman

@nerdyman I kinda fixed it by using the container width to calculate its aspect ratio and waiting for the images to load

jahirfiquitiva/jahir.dev@f628fab (#74)

Anyway, it causes a layout shift.

https://github.com/nerdyman/react-compare-slider/assets/10360816/863fa369-fd13-4cac-afaa-adb62df21f2b

jahirfiquitiva avatar Feb 03 '24 16:02 jahirfiquitiva