Add lightbox for enlarging chat images #304

cooperka · 2016-12-12T21:22:00Z

Images sent in messages will be enlarged when you tap them.

Also adds 2 new props: imageProps and lightboxProps. These will be passed through to the <Image> and <Lightbox>, respectively. For example, you can now specify an image onLoad callback.

Tested and verified on iOS and Android.

Note: Uses a commit hash instead of version number in order to fix a warning on Android that's currently unpublished.

You can read more about react-native-lightbox here 🍹

kfiroo · 2016-12-13T13:14:21Z

src/MessageImage.js

-          source={{uri: this.props.currentMessage.image}}
-        />
+        <Lightbox
+          navigator={this.props.navigator}


@cooperka I don't think you can assume this.props.navigator exists.
Even if it does, not everyone uses react-natives Navigator which is what this package is assuming.

If navigator is null, the lightbox still renders just fine :) In fact, I don't even use it myself. It's recommended by lightbox, but not necessary by any means.

I just added it to propTypes where it defaults to null. Hopefully this helps.

kfiroo · 2016-12-13T13:17:51Z

@cooperka Hey! Thanks for the PR :)
I'm not sure that this fits the scope of this component, plus, it can be super easily achieved with overriding renderMessage / renderMessageImage.

I would love to hear what others have to say though

cooperka · 2016-12-13T17:22:50Z

Thanks @kfiroo! Honestly to me it seems like an oversight that this feature doesn't already exist, as everyone I've shown the chat interface to has tried to tap my mock image to see it larger and been disappointed when nothing happens. We could definitely let people implement this on a per-app basis instead, but that just seems a little redundant to me. This chat is supposed to be "gifted" 😉. I'm also interested to see what other people have to say.

dgdavid · 2016-12-14T08:55:35Z

Guys, at first I agree with @kfiroo's doubts about the component scope. But honestly we need to admit that @cooperka have good, strong arguments. So, I keep my thumb up for this feature.

kfiroo · 2016-12-14T09:44:37Z

@cooperka @dgdavid Yeah, I hear you. I'm starting to turn too :)

Few questions though,

Is the Lightboxed image zoomable? I feel like this is a must if we are going with this.
We need to be able to use a custom Image component (like ResponsiveImage / CachedImage etc.)
Do we want to be able to customize the Lightbox view (add some info / actions)?

cooperka · 2016-12-14T13:02:19Z

@kfiroo it looks like there are a ton of new features coming as part of a major refactor, and I'm not quite sure but maybe it will even be able to use its own instance of Lightbox.Navigator if you don't pass one in yourself? The GIF on that linked comment shows off the new stuff. Unfortunately progress seems to have halted on the refactor... I added a comment to inquire.

Before the refactor (currently):

Lightbox is not zoomable, but the purpose of Lightbox is to zoom the image that was sent in chat. I feel like Lightbox is a must if we are going to allow images sent in chat, even if it lacks some features in the short-term
Arbitrary custom content should already work, Lightbox seems to be able to zoom any child content, not just images (see the GIF in their README)
That would be great! See below.

After the refactor:

Yes
Same answer
It looks like this is made much easier by the refactor

TL;DR: I think we should go ahead and merge this PR, and if needed we/I can help contribute to Lightbox to bring it up to par with where we want it in the long-term.

kfiroo · 2016-12-14T13:41:33Z

@cooperka take a look at this fork https://github.com/shoutem/react-native-lightbox

cooperka · 2016-12-14T15:15:18Z

@kfiroo Nice! Want me to use that one here instead?

kfiroo · 2016-12-14T15:51:38Z

@cooperka Not sure actually, we should use the one that works better :)
I'm also fiddling with "zoom views" now, I'll let you know if I have anything interesting.
Here's another interesting project https://github.com/ldn0x7dc/react-native-view-transformer
Which has a child projects https://github.com/ldn0x7dc/react-native-transformable-image and https://github.com/ldn0x7dc/react-native-gallery

cooperka · 2016-12-14T15:57:51Z

Interesting, let me know how it turns out! Lightbox does seem by far the most popular of the options, and intends to cover the gallery use-case with the refactor, but if something else works better I'm all for it.

kfiroo · 2016-12-14T16:06:11Z

I wouldn't hold my breath waiting for that refactor to land.. repo hasn't changed in a year or so

cooperka · 2016-12-14T17:20:32Z

Definitely agree. I asked about the status in that thread (oblador/react-native-lightbox#8 (comment)), and at worst I'll just fork the repo in order to merge a few of the nice PRs there. Should work fine either way.

cooperka · 2016-12-21T13:13:12Z

@kfiroo there's an update here: oblador/react-native-lightbox#8. Honestly we don't need all the bells and whistles from the proposed refactor of lightbox, but there are several PRs that would be nice to have merged. Shall we fork it?

kfiroo · 2016-12-21T13:36:10Z

@cooperka I didn't like how it works, but I guess it might be a good enough, out-of-the-box solution for many users, so I we can merge it or try with the navigator-refactor, what ever you think is better.
I'm only missing a way to disable it, but I guess you could override renderMessageImage and render what ever you want.

In my app, I ended up writing something myself, I can try to extract it to its own repo and see if we like it better.

cooperka · 2016-12-21T15:14:18Z

I agree that lightbox is not ideal... if you can extract your logic into something simple and usable I'm all for it! How long do you expect that to take?

kfiroo · 2016-12-21T16:46:29Z

@cooperka I'll try to do it this weekend

coderberry · 2016-12-27T18:19:06Z

+1

Currently the fix is unpublished.

cooperka · 2017-01-10T16:06:35Z

@kfiroo how goes your extraction?

I updated the props a bit on this PR, I think it's better and more generic after the changes. Also allows passing image props like onLoad now.

kfiroo · 2017-01-10T16:36:37Z

@cooperka I couldn't find time to do it, I think we can merge your PR for now and get to it later if I ever get to it :)
Can I merge this? Are we good on both android and ios?

cooperka · 2017-01-10T16:41:30Z

Yes, good to merge! I tested it just now after adding the new prop.

booboothefool · 2017-03-09T19:13:14Z

@cooperka Does this work out of the box, or is there something I have to configure to get the lightbox to pop up? I simply tried updating to the latest version of react-native-gifted-chat and clicked on an image, but nothing happened. Sorry for the stupid question. 😛

UPDATE:

had to restart packager terminal and simulator
I was specifying lightboxProps thinking I had to, but not needed, as it just works great with the default props

Thanks for the pr. So awesome! 👍

cooperka · 2017-03-09T19:48:09Z

@booboothefool yeah it should work out of the box, no setup needed! You can try getting it to work in the example app, and if it's still not working just send us a link to your code and we can take a look.

Might be silly, but also make sure you've run npm install and reloaded your app after updating your version of the library.

Also, if you're passing in a custom component e.g. renderBubble or renderMessageImage then of course you won't get the new changes unless you update the component yourself.

booboothefool · 2017-03-10T18:03:51Z

@cooperka Well I got everything hooked up, but do you notice any performance issues with lightbox? The close is extremely slow on iOS. It's like the laggiest thing I've ever seen. 😆

oblador/react-native-lightbox#2
oblador/react-native-lightbox#6

Doesn't seem to be any solid solutions though, unfortunately.

cooperka · 2017-03-10T21:07:31Z

Haha yeah it's really bad for large images. The issue is actually that it's trying to display an enormous image at full resolution instead of down-sampling it.

I wrote custom logic in my client app to resize the image to 720p at 80% quality before displaying, and then it works very smoothly.

booboothefool · 2017-03-10T22:33:37Z

@cooperka Can you explain how you did it?

I'm using https://github.com/ivpusic/react-native-image-crop-picker for the upload:

    CropPicker.openPicker({
      compressImageMaxWidth: 100,
      compressImageMaxHeight: 100,
      compressImageQuality: 0.1,
    })

which makes the image look like a butthole, like you can see every single big pixel and can't even tell what the picture is anymore, but I still can't get the animation smooth. 😝

UPDATE: Nevermind. I can't get this to work smoothly enough for my needs, so I just ended up using react-native-router-flux to just throw up another modal, and it's super smooth, even with huge full-resolution images. Just without the super slick lightbox animation unfortunately, but I'd rather it not lag. 👍

mengpeilee · 2018-04-05T21:01:03Z

Does image can be zoomed now?
I see the PR(zoom image) still open in react-native-lightbox

mafiusu · 2018-04-05T21:10:54Z

Sadly there is not much engagement for react-native-lightbox. Last change was 7 month ago.

mengpeilee · 2018-04-06T08:00:47Z

Oh no><
@mafiusu
Does there have any way to let zoom image become possible?

kfiroo reviewed Dec 13, 2016

View reviewed changes

cooperka added 3 commits January 2, 2017 17:01

Add react-native-lightbox for enlarging chat images

e84de91

Add navigator to propTypes

370d031

Use a commit hash to fix Lightbox warning [Android]

330b18f

Currently the fix is unpublished.

cooperka force-pushed the lightbox branch from 067bbe8 to 330b18f Compare January 2, 2017 22:04

cooperka added 2 commits January 10, 2017 10:54

Add 'imageProps' prop, passed through to <Image>

8e87bc4

Use 'lightboxProps' instead of just 'navigator'

c73fa48

kfiroo merged commit 9d7ca0b into FaridSafi:master Jan 10, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add lightbox for enlarging chat images #304

Add lightbox for enlarging chat images #304

cooperka commented Dec 12, 2016 •

edited

Loading

kfiroo Dec 13, 2016

cooperka Dec 13, 2016

cooperka Dec 13, 2016

kfiroo commented Dec 13, 2016

cooperka commented Dec 13, 2016

dgdavid commented Dec 14, 2016

kfiroo commented Dec 14, 2016

cooperka commented Dec 14, 2016

kfiroo commented Dec 14, 2016

cooperka commented Dec 14, 2016

kfiroo commented Dec 14, 2016

cooperka commented Dec 14, 2016

kfiroo commented Dec 14, 2016

cooperka commented Dec 14, 2016

cooperka commented Dec 21, 2016

kfiroo commented Dec 21, 2016

cooperka commented Dec 21, 2016

kfiroo commented Dec 21, 2016

coderberry commented Dec 27, 2016

cooperka commented Jan 10, 2017

kfiroo commented Jan 10, 2017

cooperka commented Jan 10, 2017

booboothefool commented Mar 9, 2017 •

edited

Loading

cooperka commented Mar 9, 2017

booboothefool commented Mar 10, 2017

cooperka commented Mar 10, 2017

booboothefool commented Mar 10, 2017 •

edited

Loading

mengpeilee commented Apr 5, 2018

mafiusu commented Apr 5, 2018 •

edited

Loading

mengpeilee commented Apr 6, 2018 •

edited

Loading

Add lightbox for enlarging chat images #304

Add lightbox for enlarging chat images #304

Conversation

cooperka commented Dec 12, 2016 • edited Loading

kfiroo Dec 13, 2016

Choose a reason for hiding this comment

cooperka Dec 13, 2016

Choose a reason for hiding this comment

cooperka Dec 13, 2016

Choose a reason for hiding this comment

kfiroo commented Dec 13, 2016

cooperka commented Dec 13, 2016

dgdavid commented Dec 14, 2016

kfiroo commented Dec 14, 2016

cooperka commented Dec 14, 2016

kfiroo commented Dec 14, 2016

cooperka commented Dec 14, 2016

kfiroo commented Dec 14, 2016

cooperka commented Dec 14, 2016

kfiroo commented Dec 14, 2016

cooperka commented Dec 14, 2016

cooperka commented Dec 21, 2016

kfiroo commented Dec 21, 2016

cooperka commented Dec 21, 2016

kfiroo commented Dec 21, 2016

coderberry commented Dec 27, 2016

cooperka commented Jan 10, 2017

kfiroo commented Jan 10, 2017

cooperka commented Jan 10, 2017

booboothefool commented Mar 9, 2017 • edited Loading

cooperka commented Mar 9, 2017

booboothefool commented Mar 10, 2017

cooperka commented Mar 10, 2017

booboothefool commented Mar 10, 2017 • edited Loading

mengpeilee commented Apr 5, 2018

mafiusu commented Apr 5, 2018 • edited Loading

mengpeilee commented Apr 6, 2018 • edited Loading

cooperka commented Dec 12, 2016 •

edited

Loading

booboothefool commented Mar 9, 2017 •

edited

Loading

booboothefool commented Mar 10, 2017 •

edited

Loading

mafiusu commented Apr 5, 2018 •

edited

Loading

mengpeilee commented Apr 6, 2018 •

edited

Loading