Skip to content
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

Animate image height on open #8

Open
JonathanWi opened this issue Nov 3, 2015 · 16 comments
Open

Animate image height on open #8

JonathanWi opened this issue Nov 3, 2015 · 16 comments

Comments

@JonathanWi
Copy link

Hey, it's me again!

I'm tweaking this component quite a bit to fit one of my need, but I can't find a way to animate the image height on open/close:
Using onOpen & onClose cause a "glitch" (which is - in fact - the expected behavior) : resizing height with this.state.height onOpen and onClose does not animate the element but instead cause an abrubt resizing (which, in the case of onClose happen after the element closed, and not while closing).
Is there a clean way to achieve this? If you need more info, I can put up a gist you can look at!

(Also, there is a couple a feature that could be easily added and would be pretty cool, like a position property to define if the element should be centered or at the top/bottom of the overlay. Should I open other issues to display them?)

Cheers!

@fatonramadani
Copy link

Hi,
I'm having the exact same problem ! I did change the height with onOpen and onClose, but the change is abrupt. I manage a way to avoid this problem by surrounding the LightBox with a view in order to keep its initial size the way I want, and my image is set to the height when it's zoomed. But the problem now, is that the selection zone is the size if the image, and one can see the dark zone when clicking the image.

@oblador
Copy link
Owner

oblador commented Nov 5, 2015

I do have a solution to this and have already implemented features like aligning to top/bottom etc, but for reasons I cannot release it just yet. Try to be patient :-)

@fatonramadani
Copy link

Thanks a lot, I'm looking forward to this !

@oblador
Copy link
Owner

oblador commented Nov 29, 2015

Allright, found some time today to work on my more or less rewrite of this module. I've leaked some stuff to @JonathanWi earlier, but now I've kinked out some issues and also added an example of it in motion.

Some changes

  • Non-Navigator/Modal support is dropped.
  • Lightbox transitions is now configurable as Navigator.SceneConfig (although technically it's not a SceneConfig).
  • The new Navigator approach enables other custom transitions in a pluggable fashion. I've written more transitions but not sure they fit in this module.
  • Origin and target of transition is configurable.
  • Background/header/footer all very configurable.
  • New Image component has built in gallery/pinch to zoom support (but the don't work very good together just yet).
  • Much more performant transitions by optionally using scale transforms (this suits most but not every use case).
  • Support for delaying heavy rendering to avoid animation stuttering.

Todo

  • Improve android support
  • Fix pinch-to-zoom/gallery combo.
  • Better pull to close support (not just relying on ScrollView events).
  • Release an RC.

Check out the navigator-refactor branch and this demo (FPS in gif is not great, actual performance on device is very good IMHO):

lightbox

@adbl
Copy link

adbl commented Dec 16, 2015

Interesting!

@oblador would you use Lightbox.Navigator instead of a regular Navigator if you also have other non-lightbox routes in your app?

@oblador
Copy link
Owner

oblador commented Dec 16, 2015

@adbl: yes exactly. Lightbox.Navigator is just a composition of Navigator and you should be able to do everything else just as before. If you can't then that should be regarded as a bug! I have some known bugs (or rather waiting to be implemented) which are the jump and replace functions.

@adbl
Copy link

adbl commented Dec 16, 2015

I would be a bit anxious replaced the main Navigator in an app. but looking at the code, it seems as it will play along with another parent navigator, if the lightbox needs are limited to certain "primary" routes which could have their own Lightbox.Navigator?

@oblador
Copy link
Owner

oblador commented Dec 16, 2015

@adbl: Basically it enables you to define completely custom transitions via a React component (I'm using this for more stuff than just Lightbox animations). It will only touch routes having the transitionComponent set, the rest will be passed down to Navigator like normal.

@adbl
Copy link

adbl commented Dec 16, 2015

I see, but would the idea I described work? (given as the regular Navigator seems to support such use with a parent navigator prop, though I haven't tried it myself)

@oblador
Copy link
Owner

oblador commented Dec 16, 2015

Aha, yes it would work to nest navigators a bit depending on how your navigation bar is configured.

@adbl
Copy link

adbl commented Dec 16, 2015

I realize the child Lightbox.Navigator needs the full screen to render a lightbox properly... =)

@cooperka
Copy link

It's been about a year with no updates on this / 6 months since any branch activity for the refactor. I'm curious if this is still in progress, or in the meantime if we can start merging some of the longstanding PRs that have been waiting?

I'd be happy to try to help out if there are specific things blocking the release! I'm really excited to have these new features.

@oblador
Copy link
Owner

oblador commented Dec 20, 2016

@cooperka: Yeah I know, there's been very little public progress. I'm myself using a private fork that I don't want to release publicly as it only solves my needs and I don't want to maintain it long term. Overall the basic problem is the bad state of navigation in react native. There's no use in me putting so much effort into this if I have to scrap it and rewrite it every few months. I'm awaiting the upcoming release of the new official navigator, but AFAIK it's already behind schedule. If it looks good and is accepted by the community I'll try to make yet another refactor for that solution.

@cooperka
Copy link

Out of curiosity, how hard would this be to do without relying on a navigator? I'm using this repo in its current state with null passed as the navigator prop and it seems to work just fine. It's not clear to me what the benefits are, though maybe it's more for the up-and-coming features like the gallery.

@oblador
Copy link
Owner

oblador commented Dec 20, 2016

Depends on what parts you want I guess, if you cut the scope to only be an image viewer then it's doable without navigation. Although navigation integration is desirable IMHO as it gives other benefits such as (potential) deep linking, automatic back button handling etc.

@luco
Copy link

luco commented Feb 5, 2017

@oblador I'm getting some error on refactor. It throws Check the render method of 'Bubble'. I can see that @waleedarshad-vf get some related to that on #56 .

lbickston pushed a commit to lbickston/react-native-lightbox that referenced this issue May 13, 2018
Add `React` to lightbox overlay
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants