react-native-lightbox icon indicating copy to clipboard operation
react-native-lightbox copied to clipboard

Road to 1.0 (request for comments)

Open oblador opened this issue 6 years ago • 25 comments

I'm taking up development of this repo again of this package after a long silence in belief that this should be solved at the navigation level. I've been convinced that there still use cases for this package after all and my hopes of navigation libraries properly supporting shared element transitions hasn't really been fulfilled.

Since there has been a lot of developments in the React Native community (such as Android support, native animation driver etc) since the inception of this package, I'll be starting over from a clean slate. API will most likely change, but will keep backwards support when possible/sane. Before starting it'd be nice to hear from the community which features are the most important, so I've done a quick outline of my requirements/priorities.

Please comment with your thoughts and priorities. If you want to help out in in form (like development, documentation, issue triaging, testing etc) please reach out here or to me personally on twitter at @trastknast.

Must have

  • [x] Support for custom Image component (ImageProgress, FastImage etc)
  • [ ] Support for lo-res thumbnail -> hi-res transition
  • [ ] Set up CI to run tests and release
  • [ ] Andorid hardware back button support
  • [ ] Gesture to close by swiping down/up
  • [x] No dependency on navigation lib
  • [x] No dependency on Modal
  • [ ] Improved peformance with native animation driver
  • [ ] Feature parity between Android and iOS

Really nice to have

  • [ ] Support for aspect-ratio change from thumbnail -> hi-res
  • [ ] Pinch to zoom
  • [ ] Support for gallery mode
  • [ ] Configurable Touchable component
  • [ ] Configurable transition properties
  • [ ] Configurable status bar transitioning
  • [ ] Configurable/custom header/footer

Nice to have

  • [ ] Pure JavaScript implementation
  • [ ] Integration library with react-navigation (et al)
  • [ ] Integration library with redux
  • [ ] Support Image->Video transition
  • [ ] Some form of lifecycle support (will/did open/close)

On the fence

  • [ ] Orientaion change support
  • [ ] Programmatic way of opening/closing lightbox

Not doing

  • [ ] Generic shared element transition

Ping @StevenMasini @peteroid @elkinjosetm @bilalsyed001 @ncuillery @ZackLeonardo @Kureev

oblador avatar Apr 08 '18 09:04 oblador

@oblador I don't know what you think about it, but if we are starting a new version of react-native-lightbox from scratch, we could review, add to the list and close the previous issues.

For instance, I think the iPhone X Support issues should also be added to the Must have list.

StevenMasini avatar Apr 09 '18 09:04 StevenMasini

@oblador The list looks good. How should we prioritize it?

IMO, these are first-class must-have:

  • Feature parity (Android, iOS)
  • Performance
    • Supports useNativeDriver
    • Support for lo-res thumbnail -> hi-res transition
  • Gesture to close by swiping down/up
  • Dependency-less
    • at least no navigation

Maybe we can branch a new v1 to contribute.

peteroid avatar Apr 09 '18 12:04 peteroid

@StevenMasini Yup, iPhone X support is a given, but wouldn't consider it a major feature to list here. I have an X so no worries that it'll be implemented :-)

@peteroid Cool, those are all in the must haves right? Prioritization within each section is less important, just wanted to make sure I didn't omit anything important, or placed something in the wrong bucket.

oblador avatar Apr 09 '18 19:04 oblador

pull request #85 let overlay can be zoomed. Pinch to zoom😊

ZackLeonardo avatar Apr 11 '18 02:04 ZackLeonardo

Support for gallery mode, I will try this.

ZackLeonardo avatar Apr 11 '18 02:04 ZackLeonardo

@ZackLeonardo Awesome! Before you start on gallery mode can we discuss the overall API in a ticket first?

oblador avatar Apr 11 '18 06:04 oblador

@oblador Sure. I run into kind of similar scene in my spare time app, I am thinking of its outline after work.

ZackLeonardo avatar Apr 11 '18 07:04 ZackLeonardo

@oblador My opinion: add galleryMode(true/false) and Key props to LightBox component, then those which galleryMode values are true and have the same Key value can be swiped in LightboxOverlay component.

ZackLeonardo avatar Apr 12 '18 05:04 ZackLeonardo

@oblador I just coded the gallery mode and commit it here: pull request #85 . I wanna get your suggestions to make it better.

ZackLeonardo avatar Apr 15 '18 11:04 ZackLeonardo

@oblador Any progression here ? How can we contribute to this ?

StevenMasini avatar May 07 '18 07:05 StevenMasini

@StevenMasini I've gotten to a good start, have a first PR up here: https://github.com/oblador/react-native-lightbox/pull/106

My thought is to get a foundation up in the phoenix branch and then you guys can start contributing by making PRs to that branch. I'll give you repo access soon too.

oblador avatar May 13 '18 17:05 oblador

@oblador thank you for the push access. I will run and review both branch phoenix and phonenix/pinch-to-zoom asap.

StevenMasini avatar May 14 '18 03:05 StevenMasini

phoenix is empty so far, I intend to build it up via pull requests the coming few days for visibility.

oblador avatar May 14 '18 04:05 oblador

Having a hard time breaking this up in well scoped PRs before knowing what I want as the final API, so I've gone ahead to create a proof of concept implementation with a new API. You can try it out here: https://github.com/oblador/react-native-lightbox/tree/phoenix-wip/Example/src

In essence you'll wrap your application with a LightboxPresenterProvider which will contain the overlay component (thus removing need for Modals) and general configuration such as how to render the header, what image component to use etc. I've only implemented one presenter so far which has AirBnB like transitions/interactions, but have plans on doing a neater shared element transition later too.

Galleries will similarly be wrapped with an optional GalleryProvider that contains a list of all images and potentially more meta data like hires source, title, description etc. It would be great to have just a <GalleryProvider> that will somehow aggregate the image sources of all nested children so you wouldn't have to duplicate the data, but can't come up with a clean and simple way so waiting a bit with that.

Lastly we have the LightboxImage that is a clickable image that will trigger the ligthbox. I'm not sure if this is the best API, maybe the current more generic approach with Lightbox is better but also harder to get right. Might also have multiple components as entry points to the lightbox.

LMK what your thoughts are and if I'm totally off somewhere.

oblador avatar May 25 '18 21:05 oblador

It's good that we don't rely on <Modal /> 👍

I am just wondering for the LightboxImage, how can we manage the image cache ? or use custom image framework ?

StevenMasini avatar May 28 '18 02:05 StevenMasini

@StevenMasini You'd send in a ImageComponent prop to PresenterProvider and it will propagate to the LightboxImage and the overlay. Like this:

<ModalPresenterProvider ImageComponent={FastImage}>
  <LightboxImage source={{ uri: 'https://...' }} />
</ModalPresenterProvider>

oblador avatar May 28 '18 07:05 oblador

@StevenMasini Any thoughts on the general approach? Want to help out by working on the shared element transition? I'm going on vacation for two weeks now so won't be working actively on it for that time.

oblador avatar May 31 '18 20:05 oblador

@oblador The general approach sound good to me. We already fit three of our main requirement 👍 What do you mean by shared element transition ? Animation ?

We should sync up more via Twitter, faster to interact and less formal I think.

StevenMasini avatar Jun 06 '18 01:06 StevenMasini

@oblador It looks awesome that we can remove the dependency of the Modal. In addition to the LightboxImage, should we provide an extra component like LightboxView to let people to apply the Lightbox on different views, e.g. videos and other non-image presentation?

peteroid avatar Jun 12 '18 05:06 peteroid

Looking forward to 1.0!

I'd highly recommend having a footer (like the header)

It will be very useful for actions like liking an image or other things like flag/comments.

Cheers and thanks for the hard work.

nica0012 avatar Sep 14 '18 21:09 nica0012

@oblador Is there a timeline for release v1.0? Or has it moved repo as I see there have been no commits for some time...

bnbon avatar Oct 17 '18 09:10 bnbon

This project seems dead. Any alternatives?

HommeSauvage avatar Sep 16 '19 04:09 HommeSauvage

@redgenie None that I know of. If you want to make it with react navigation you can consider using https://github.com/fram-x/FluidTransitions

Otherwise... Build one 😄 This is your chance 😎

RWOverdijk avatar Oct 05 '19 16:10 RWOverdijk

This project by @terrysahaidak looks very promising. It uses reanimated 2. https://github.com/terrysahaidak/react-native-gallery-toolkit

@terrysahaidak is there a way to implement a lightbox with your lib? Also, do you plan on supporting web?

Update I found this lightbox example: https://github.com/software-mansion/react-native-reanimated/pull/842. It's not a standalone package, but it's a start

nandorojo avatar Oct 22 '20 23:10 nandorojo

The lightbox behavior as well as Gallery component is in progress right now, I have opened draft pull request for that.

Pretty much everything I see in the topic is already added/I plan to add to the library.

Also, since Reanimated 2 and gesture handler supports web, the lib can be run on web too. It's not perfect right now, but can be improved in the future.

Also, I'm in contact with Reanimated team. Since my library is kind of "pushing Reanimated to limits" we constantly finding some bugs and fixing them and making sure Reanimated the best as possible. So we target 60fps and "no-lag-on-mount".

Also, documentation website for all available right now components will be published soon.

terrysahaidak avatar Oct 23 '20 05:10 terrysahaidak