EuiAccordion initialIsOpen doesn't work correctly

[ppisljar]

Seems that when the element is not visible initially and initialIsOpen is set to true, height is incorrectly calculated and set to 0px? This prevents using an EuiAccordion in any form of tabs right now.

this is my PR where its going wrong: elastic/kibana#18579

[snide]

Jumping on this.

[ppisljar]

this is still happening with latest EUI version

[ppisljar]

the problem is probably the fact that the accordion is inside a div with display:none ...

why don't we leave height to auto if accordion is not collapsed, and only set it when user clicks the collapse button (in order to animate it).

[snide]

Why are you putting it inside a div with display none? Use visibility: hidden instead?

[ppisljar]

as div with visibility hidden will still occupy its space ? also not sure it will behave correctly with flex display ? (which is used in this part ... i tried just replacing display: none with visibility: hidden and everything is broken ... )

its angular legacy code (ng-show, ng-if)

[timroes]

@snide visibility: hidden does actually reserve the space that element needs and just make it invisible (similar to opacity: 0). That's not a behavior that would work in the most of places where you use display: none (and want the element to be "gone" invisible).

I think that using the accordion in that kind of situation should still work, since a component inside a UI framework should assume as less as possible around it's context it's used in.

I think we could write that component in a way, that it would still work in those scenarios by not calculating the size initially on componentDidMount, but instead only start calculating the size at that moment it's needed, i.e. right before we change the height. This is a) when clicking to collapse the item (in which case we would need it stored) b) before an update occurs (in case that children height changes), i.e. componentWillUpdate in the old React lifecycles and pretty much that example I think in the new React lifecycle: Reading DOM properties before an update

[snide]

@timroes That sounds fine if you want to set up a PR, we can help review.

[cchaos]

So I looked deeply into this issue and I don't think there is anything we can do on our side to fix this. The problem in your situation is that when the component mounts (and initially it mounts, then updates twice, not sure why), it is initially hidden from the DOM leaving it with no height to calculate.

Ok, fine, no big deal, we'll just wait for it to get un-hidden. The problem with that, is when you change tabs, there is no event fired back at the component to update. You will need to somehow force it do so.

Updating components in react will only happen if you change the props or state, and you're doing neither in this case.

This may not be anything new to any of you, but it's good to jot down what is exactly going on and why it's dependent on implementation and not the component itself.

[snide]

I'm reclosing this. Right now I consider this an implementation problem in Kibana rather than a problem with the component itself.

If an engineer has a better way to handle this that makes it more flexible please submit a PR and we can help review.

[timroes]

Hey @cchaos, thanks a lot for digging so deep into this!

We'll look for a way to properly work around this for now. In the new editor that shouldn't be an issue at all, since we won't have any Angular frame around it and can use an aligned state management in the hole editor!