Wrong height calculated in nested components about react-collapse HOT 22 CLOSED

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

from react-collapse.

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)

from react-collapse.

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.

from react-collapse.

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
.

from react-collapse.

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

from react-collapse.

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.

from react-collapse.

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

from react-collapse.

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 :).

from react-collapse.

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

from react-collapse.

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

from react-collapse.

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

from react-collapse.

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

from react-collapse.

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)

from react-collapse.

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 ?

from react-collapse.

Any news here? :)

from react-collapse.

Most likely to be fixed by #72

from react-collapse.

@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

from react-collapse.

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

from react-collapse.

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.

from react-collapse.

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.

from react-collapse.

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

from react-collapse.

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

from react-collapse.