Wrong height calculated in nested components

[datoml]

Hello.
I found a strange behavior with react collapse when you dont wrap the content inside a div with padding > 0.

The problem appears inside my sidebar menu.

I took the Nested.js from the examples and modified it. So it should be easy to reproduce it.

This code works:

        <Collapse
          style={style.container}
          isOpened={isOpened}
          keepCollapsedContent={keepContent}>
            <div style={{padding: 1}}>
              <ul>
                <li>Hallo
                  <ul>
                    <li>
                      <VariableHeight />
                    </li>
                  </ul>
                </li>
              </ul>
            </div>
        </Collapse>

This one calculates the height wrong and jumps the the right size after animation is finished.

        <Collapse
          style={style.container}
          isOpened={isOpened}
          keepCollapsedContent={keepContent}>            
              <ul>
                <li>Hallo
                  <ul>
                    <li>
                      <VariableHeight />
                    </li>
                  </ul>
                </li>
              </ul>            
        </Collapse>

Here's how it looks.

Thank you in advance. :)

[nkbt]

Ul/LI have some padding/margins by default (as you can easily check in dev tools). If you don't reset them to 0 or don't use proper wrapper (block element, without padding/margins) - behaviour may not be as expected.

Margins are the most trickiest ones. Better compensate them with paddings of collapsible container

[datoml]

Hmm.
I tried to set margin and padding to 0 and it works now.
But when I nest them multiple times I have the same effect like in the video.

[nkbt]

Could you please create a Codepen to try it out? Thanks!

[datoml]

Hey :).
Here's the codepen example.
https://codepen.io/anon/pen/MKPENE?editors=0110

Steps to reproduce:

• Fast uncheck and check the first one.
• After severel times you fast uncheck and chek the first one it starts working
• Uncheck the second one
• Uncheck the first one
• Check the fist one
• Fast uncheck and check the first one again

I hope you can reproduce it :).

[nkbt]

Need to doublecheck if height is re-measured after height: auto, so nested elements can fix themselves in case of quick open/close actions.

[nkbt]

I really need chenglou/react-motion#273 to be published, that would allow me to fix this issue quite effectively.

[Justkant]

Yeah really waiting for this to be fix.
Nice work on this lib

[benzen]

Heads up:
https://github.com/chenglou/react-motion/blob/master/src/Motion.js#L29
Seams like this was merged to master.
Still not released

[nkbt]

It is actually released as far as I know.
On Tue, 22 Mar 2016 at 03:42, Benjamin Dreux [email protected]
wrote:

Heads up:
https://github.com/chenglou/react-motion/blob/master/src/Motion.js#L29
Seams like this was merged to master.
Still not released

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#34 (comment)

[benzen]

Weird since this was merged on 29 of january an the is only on gh release which was made on the 30.
But the release does not mention it.

Anyway, @nkbt do you have time to work on this ?

[nkbt]

I do and I will when I am back from holidays :)

No worries.
On Tue, 22 Mar 2016 at 23:46, Benjamin Dreux [email protected]
wrote:

Weird since this was merged on 29 of january an the is only on gh release
which was made on the 30.
But the release does not mention it.

Anyway, @nkbt https://github.com/nkbt do you have time to work on this ?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#34 (comment)

[davidspiess]

Any news here? :)

[nkbt]

oh jeez that was hell of a holiday I reckon... Thanks for ping @davidspiess

[nkbt]

Most likely to be 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.

[sassanh]

@nkbt thanks for this great module, I found the nested behavior is not still buggy if you use more than one <Collapse> tag in a single render. It works alright if you use only one <Collapse> per component and nest them but if you use more than one <Collapse> in a single component it'll behave strange.
Look at this: https://codepen.io/sassanh/pen/GqLZVy?editors=0010

[nkbt]

Yeah I found issues with nested collapse too recently. Can be solved by
specifying if collapse has nested collapse elements (and then change
internal state transition logic), since it is tricky to catch that
automatically. It is not implemented yet, I will be able to get back to it
bit later.

PS: we are using collapse quite extensively in our work projects, so this
issue will be eventually solved :)

Thanks a lot for a Codepen, I will add it to examples when I have some
time. Feel free to add this example as PR if you are keen
On Sun, 21 Aug 2016 at 13:47, Sassan Haradji [email protected]
wrote:

@nkbt https://github.com/nkbt thanks for this great module, I found the
nested behavior is not still buggy if you use more than one
tag in a single render. It works alright if you use only one
per component and nest them but if you use more than one in a
single component it'll behave strange.
Look at this: https://codepen.io/sassanh/pen/GqLZVy?editors=0010

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#34 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKsoAlx_QKwHOhIGATVKkPQdjC99t34ks5qh8pogaJpZM4HUNTX
.

[sassanh]

I'm glad to hear it'll be fixed. Meanwhile do you know any simple temporary workaround for this?

[sassanh]

Nvm, I was able to handle it with defining a <DummyTag> (which in contrast to its name is pretty complicated to be able to render child nodes considering requirements of my project.) and now it's working alright.

[nkbt]

the solution for now is to not pass any props through parent collapse if only children are changed (so children should be connected to a flux/redux store or something). Now when props of paren collapse are changed it is transitioned to state WAITING, which locks its height. Since it is own height is not changed after that (because children are expanded with animation, so the next step height is exactly as it was before) - it may forever stay in this state =(.

So it works perfectly when children are expanded instantly (no nested collapse), but not working when collapse elements are nested.

I consider adding a flag hasNestedCollapse to skip this transition to WAITING state. This way everything would work perfectly since it has height: auto.

[nkbt]

@sassanh, I've opened #76 to track progress on this.

[sassanh]

Thanks for info, I'll continue on that issue then.