naoufal / react-native-accordion

An Accordion Component for React Native
https://www.npmjs.com/package/react-native-accordion
437 stars 101 forks source link

Refactored animations to work in iOS and Android. Breaking changes #42

Open thireven opened 7 years ago

thireven commented 7 years ago

Replaced react-tween-state with native Animated library so that the animations work in both iOS and Android. For this reason, there is one breaking change: the easing property no longer takes a string, but an Easing function.

atlanteh commented 7 years ago

Works great for me! Thx!

atlanteh commented 7 years ago

You have a small bug that expanded prop (If the accordion is expanded by default when mounted.) doesn't work..to fix this:

  getInitialState() {
    return {
      is_visible: false, // change false to this.props.expanded
      height: null,
      content_height: 0
    };
  },
thireven commented 7 years ago

Fixed. Thanks!

compojoom commented 7 years ago

I'm getting cannot read property 'nativeEvent' of undefined. on line 73

That's when my view get loaded. If I dismiss the error I can interract with the accordion? Any ideas?

I just copied the modified accordion to my installation. Do I need to do anything else?

thireven commented 7 years ago

@compojoom It could be a race condition. From the documentation for onLayout (which is used to get the dimensions):

Invoked on mount and layout changes with:

{nativeEvent: { layout: {x, y, width, height}}}

This event is fired immediately once the layout has been calculated, but the new layout may not yet be reflected on the screen at the time the event is received, especially if a layout animation is in progress.

I'm wondering if it's possible that something else is slowing it down so much that the onLayout is called before the component is rendered. Could you provide more information like what version of RN you're using, platform, os version, etc?

naoufal commented 7 years ago

Thanks for taking the initiative @thireven! Is this in a good state to be reviewed or are you still addressing bugs?

thireven commented 7 years ago

I believe it's in a good state to be reviewed. Haven't personally run into any other bugs.

compojoom commented 7 years ago

I just tried it again and this time it seems to work. Not sure what I've done wrong last time.

compojoom commented 7 years ago

I was too fast. So upon reloading my js(not jut hot loading) I get the same error as before.

I'm trying this with RN 0.36

compojoom commented 7 years ago

I'm trying to debug what's wrong but so far without success. It seems that in my case render get's called twice and this calls _getContentHeight twice. The first time the event is undefined, the second time the event is correct. So adding a check if the event is defined solved that for me.

However I have another issue. In my case the content can be dinamic. If the accordion is open, when I add a new item to the content I need to recalculate the height. I can't figure out how to do that with the current modifications.

In the past I was able use willReceiveProps and call _getContentHeight from there, but with your modifications _getContentHeight now expects an event and I can't pass one. Ideas?

thireven commented 7 years ago

@compojoom I'm not able to reproduce the behavior you're describing with the Accordion example. Could you update the example to replicate your issue and post the code here?

ciceroneves commented 7 years ago

Any ETA for when this is going to be merged? I really would like to remove the dependency on react-tween-state package

atlanteh commented 7 years ago

This project hasn't been updated in almost a year. it seems abandoned. You have to options. You can either fork this project and use your fork or use this PR as a dependency: "react-native-accordion": "https://github.com/naoufal/react-native-accordion#pull/42"