-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Feature request/guidance: Zoom an image? #264
Comments
Hi @hardcodet, You're right: rendering more than one gesture-aware component at a time can be a straight path to madness :-) What about using Would it answer you need or did I overlook something? |
Thanks @bd-arc You got the madness part right :) I looked into the gallery - it a bit simplified (I wouldn't want to just set a few image URLs, for example), but it might be a good starting point I I have to go with a custom fork. As far as the combination goes: I wouldn't want to use both carousel and the gallery, since they are very similar, so the distinction might be a bit weird and I would just switch from a non-full-screen image to a full screen image, which doesn't make too much sense in my case. Thanks for the quick reply, I appreciate it! |
Do you have a visual example of what you're trying to achieve? I'm curious and always up for a challenge ;-) |
Haha, awesome! Basically, your carousel can do everything your gallery can (and more!). But I basically "just" need a control that is capable of flexibly handling some content (in the end, it's an image) that can be zoomed, with a pager and preferably some nice animations when flipping between pages. I need it to be a bit more flexible than just supplying a list of image URIs. My current control displays a spinner while processing the image, and then shows the image once it's available. So I have just a wrapper control that I'm currently using along with react-native-pager. That worked beautifully, but once I introduced zooming capabilities in the wrapper, it messed everything up :) Was that somehow understandable? |
Hey @hardcodet, I think I understand what you're after. A carousel with custom animations between items, placeholders, loaders, fade-in apparition of images once loaded... This I can provide ;-) But I wouldn't even know where to start in order to add a zooming feature without loading another view and while preserving the swiping ability. Wix had a pretty good project going on, |
Hi folks, found this issue while also trying to implement a zoom style feature with the gallery. My use case is a digital magazine where you can "flip" through pages, but where you should also be able to zoom in to read content, see images, etc. I ended up utilizing A code snippet is worth 10,000 words, so here's a quick overview of my first stab at it:
Hopefully that helps solve an issue for someone else down the road! If this approach is deemed "okay", I'd be more than happy to write up some documentation for the README or wiki to help future devs. Thanks for your hard work on this project, it's super easy to use and so useful! Much thanks! |
@bd-arc @jacobsmith You guys are awesome! I'll be in the mountains for two days but really looking forward to trying this out! I went a similar route wtih react-native pages, but the results where a bit underwhelming - I'll definitely try this out and will be posting back. Thanks! |
Hey @jacobsmith, Thank you very much for sharing your idea! I wasn't aware of the If this isn't too much to ask, would you mind putting together a simple Snack example so I can check on the usability and the performance of your solution? My only concern has to do with the potentially big number of re-renders; have you experienced any performance issue so far? By the way, since your app is a digital magazine you might be interested in knowing that I've recently implemented a feature that allows you to customize the transition between items (or pages in your use case) ;-) |
I tried to apply your pattern, but the problem is that the nested I did try the same with the snippet you posted, with the same result. This is even true if I hardcode Did you customize |
@bd-arc and @hardcodet I've attached a quick snack here. I haven't noticed any issues with performance (tested up to 100 pages, scrolling and zoom still seem to work fine). Of note, you are either in "Scrolling" mode or in "Zoom" mode. And @bd-arc yes, I've looked at the possibility of adding custom swipe animations. If I end up implementing a "page turn" animation, I'll be sure to let you know! Thanks for your work on this, it's a great project!! @hardcodet let me know if this doesn't fix your problem and I'll try and take a look at your implementation! |
Thanks for putting in the work! Unfortunately, I can confirm that I'm seeing the same issue with your snack - I can easily zoom the bear, but wouldn't know if there are other pictures since swiping doesn't work. Question: Are you testing on a real device? Test done on Android (Samsung Galaxy S6) Edit: I just noticed that the Expo online emulator also doesn't swipe if you select an Android device. |
@hardcodet I've been testing on a real iPhone 5c, but I haven't done it on Android. I'll pull out an old android phone I have later and give it a test (because I need to support Android for the project I'm working on, so if this doesn't work, I'll need to re-evaluate the approach and find something that works on both platforms). Thanks for giving it a shot; hopefully I'll find the issue on the Android side within the next few days and have a fix! |
The problem is the competing gesture listeners. In theory, it would be simple:
The problem now is that the number of touches is always 1 in the PanResponder's Start/Capture callbacks. I didn't see a simple way to delegate the gestures between controls after that. I'm really surprised about that shortcoming in RN - but still hoping I overlooked something. |
Yes, I've come to roughly the same conclusion myself. I very rarely do see swiping on android (maybe 5% of the time), so I wonder if it's a race condition between gestureResponders? (I haven't dealt with eventing in React Native much so that could be way off, just learning as I go). From RN docs:
I'm not quite sure what the impact of that statement is yet... |
The problem is that in the I have a somewhat working solution with a |
@hardcodet would you mind sharing that "dirty hack" of a solution? 😄 |
Sure. Keep in mind that it's really hackery and not in a good state (also, I have only been a RN developer for a few weeks ;) Basically, I have a model that declares a
Here's the
Now what I did is just declare my model's
And in the carousel's
Last but not least, I adjusted
So the workflow is this now:
This works, but the problem is that if you zoom very fast the first time, the gesture is terminated, probably due to some race condition. Also, if you "swipe" fast with two fingers, the carousel starts to swipe and the freezes after a few pixels because the Also, keep in mind that if you put on some debugging controls that cause re-rendering of your component while you are performing gestures, the whole gesture handling can change. I learned that the hard way :) |
I just realized an additional touch responder in the hierarchy also makes things much harder :) When I removed that one, I got very nice result using just an adjusted ImageZoom. You can use this one as a drop-in replacement - no more gesture handlers or additional
|
First, I'd like to thank you both for the top-quality discussion :-) @jacobsmith Thanks for putting the example together! I've tried it on miscellaneous devices; while it works pretty well on iOS (not without a few glitches, but that was expected since there was a bit of hack involved), it unfortunately isn't usable on Android: the swipe gesture works once and then it becomes permanently superseded by the zoom one. I regularly have issues with gesture events and callbacks being handled very differently between iOS and Android; incidentally, it's one of the few things that hold this plugin back... @hardcodet Thank you very much for sharing your component! I can't wait to try it out because, in my experience, if it works properly on Android it will be incredible on iOS ;-) |
One thing to note: Also, if I wrap the I hope there's ways around this, but the whole gesture system is rather tedious (if I felt more competent at RN, I would write broken ;). Still, I think we're on a good track here. |
Thank you both for that info! I agree, as I've been working with that hacky solution, I've found more edge cases that don't play nicely together. @hardcodet I'll have to give that updated code a shot; thanks! |
Started a new job three weeks ago, so I didn't really have time, but will be hopefully back to working on my project as of next week. @jacobsmith - did you by any chance make some progress? |
Did you guys had a chance to make some progress? I did not find any decent carousel + zoom RN libraries. |
I have been pulled away on another project for a while, but I'm coming back to this now. I will try to update if I find anything! |
@hardcodet your solution is great, especially the latter one. At least with 10 min testing, it runs smoothly on Android. This whole issue of image lightbox with zoom has been a surprising one, just like when we tried to find a working @mentioning -package.. just could not find any that worked. In any case, thanks again for the solution. For those who want to use my fork of @hardcodet's solution: https://github.com/ankero/react-native-image-zoom Update After testing with iOS I decided to go with a hybrid solution, as this solution works for Android, but causes some scrolling issues on iOS. So my current solution that we are moving to deeper testing is roughly the following:
This solution seems to work well, the only downside is that the PhotoView does not utilise FastImage. However it is much more performant on iOS Ps. I don't know anything about typescript and saw 2 errors with compiling imagePanResponder and lastTouchStartTime, so added the (!) to the declarations :) |
Fork from @ankero is really useful, but it doesn't seem to work on Android for me - only carousel works. While it does work extremely good on ios (IOS run on the simulator, Android on a real device). |
@TeslaAdis Currently the solution we use is the following. It works ok on Android and iOS. "react-native-snap-carousel": 3.7.5, The structure which enables carousel with zoom for iOS and Android:
|
Ok thank you for your feedback, I'll give it a try :) |
I'm currently trying to implement image zoom using |
Same for me, and with react-native-image-pan-zoom carousel stop working |
I used pinchgesture handler-> import { const scale = new Animated.Value(1);
<GestureHandlerRootView style={{ flex: 1 }}>
|
Is this a bug report or a feature request?
Feature request.
I've been struggling (and failing!) quite a bit getting a decent image zoom within a page/swiper. Don't ask me how I missed this control, which looks way more polished than others I've seen so far. I basically just need a gallery, and as such, my users will want to be able to zoom into an image.
The problem I have probably exists here too though: If I have an ImageZoom control (that allows zooming, panning an image), that will probably collide with the "outer carousel's" own gesture handling. Is there a working sample or guidance I could use as a starting point?
Thanks!
The text was updated successfully, but these errors were encountered: