re-animation on re-render

[tcurdt]

Now on a few occasions a react re-render causes the animation to re-run (the end bit of it at least).
Unfortunately I cannot reliably reproduce it yet.

It's a bit weird. I have the redux store changing (of course that causes a re-render) but nothing related to react-collapse changes.

[nkbt]

Hi, @tcurdt

it is very hard to tell what is going on with your code without being able to run it =).

The easiest I can advise blindly - create a small component with shouldComponentUpdate that only depends on props changes, and renders your larger components. And then wrap it in ReactCollapse.

The rule is simple - when immediate children are re-rendered - Collapse recalculates height and is re-rendered too. So if children are changed by anything from outside (not Collapse), this will happen. shouldComponentUpdate helps to avoid extra re-renders.

Also put something like console.log(this.props) into wrapped by Collapse component's render() method to see when and why it is rendered.

If you can fork http://codepen.io/nkbt/pen/MarzEg and create your own reproducable example - that would help a lot.

[tikotzky]

I'm seeing this behavior as well.
It only seems to be happening on the first rerender.
I'll see if I can isolate into a reproducible bin.

[tcurdt]

I know - it's hard to help when you can't reproduce it.

Things is though: The children are not changing at all. So if the re-calc happens - it will give the very same result. Yet the animation is running for a very brief moment.

[tikotzky]

@tcurdt yup I am seeing the same thing. The children are not changing yet animation runs for a split second...

[jermspeaks]

I know it's a cop out, and it's better to change the element instead, but passing the isOpened parameter value to the child component and changing it's style prop with display: 'none' is a workaround.

<Collapse isOpened={this.state.opened}>
  <ChildComponent
    style={{
      display:  this.state.opened ? 'inherit' : 'none'
    }}
  />
</Collapse>

If I get time, I'll try to reproduce it since I also have a redux store changing in my app.

[nkbt]

@jermspeaks
When you do this way, your component will just dissapear and will not be collapsing to 0 height =(

[jermspeaks]

@nkbt Yeah, I noticed that. Like I said, I'll try to reproduce the issue. This temporary workaround is not hitting the bigger issue of mounting child components. If I figure it out, I'll send a PR. :)

[nkbt]

Neat, thanks.

Let me know if you need any help. You can find me in http://www.reactiflux.com or https://gitter.im/nkbt if you need real-time comunication

[aarimond]

Hi,

I have this issue when nesting Collapse (version 2.3.1) and just want to give some more info, maybe it helps.
My setting is something like the following, even though I'm not sure if it reproduces everything correctly. It's just kind of extracted from my code.

<Collapse isOpened={selected} keepCollapsedContent={true}>
   <Collapsible1/>
   <Collapsible2/>
</Collapse>

where the inner Collapsible are classes (typescript) like this:

class Collapsible1 extends React.Component<...> {

constructor(props){
    this.state = {collapsed: true};
}

render (){
  return (
     <div>
          <div className={'head'} onClick={() => this.setState({collapsed: !this.state.collapsed})}>Open</div>
          <Collapse isOpened={this.state.collapsed}>Some content</Collapse>
     </div>
  )  
}
}

What I found from debugging is that when uncollapsing the inner Collapsible by clicking on the head, the height of the whole element changes dynamically, but I think the outer Collapse.height attribute is not updated (onHeightReady is not called). After that, there is some other action in my app that triggers the redux reload which triggers a re-rendering, which then recalculates height on the outer Collapse (onHeightReady) which then triggers the animation, but only once on the first redux reload.

After some trial and error on the src code I found the following patch fixing my problem (at least for the moment):

        function onHeightReady(height) {
            const {isOpened, keepCollapsedContent, onHeightReady} = this.props;

            if (this.renderStatic && isOpened) {
                this.height = Collapse.prototype.stringHeight(height);
            }
            if (keepCollapsedContent) {
                // ----------- introduced this line -------------
                this.height = Collapse.prototype.stringHeight(height);
                // -----------------------------------------
                this.setState({height});
            } else {
                this.setState({height: isOpened || !this.renderStatic ? height : 0});
            }

            const reportHeight = isOpened ? height : 0;

            if (this.state.height !== reportHeight) {
                onHeightReady(reportHeight);
            }
        };

I found the additional line ensures skipAnimation = true when Collapse.getMotionHeight() is called.
Furthermore, keepCollapsedContent seems to be relevant here.

I haven't been able to understand the whole lifecycle of Collapse, but maybe this gives some hint.

[nkbt]

I am doing some major rewrite in #72, which should address most issues (except the realtime tracking of nested height, which is "by design" decision, though I might find the way to fix it too).

[nkbt]

Confirmed, that it is fixed by #72

[nkbt]

Just published react-collapse@next (not latest channel yet until better tested) with full rewrite, which fixes this issue. Give it a shot if you are keen.