Make panel component set the foldIndex prop on its direct Fold children

[VeraZab]

These folds are going to cause issues and at some point we're going to decide where to draw the line on what we're supporting and not.

React dom tree inspection is not that straight forward. When I try to inspect the children of Panel, for example, I'm not able to see the folds as children. It seems like they're not rendered yet and so don't show up in props.children

Because there doesn't seem to be a consistent way of checking if a panel is going to render Folds, I'm making the panel component check if folds are present with document.getElementsByClassName('fold'), this is how I'm currently able to somehow reliable tell if a Panel has Folds, build a state for Folds, and then make each Fold, with an index, check the state it should have through context.

I'm setting foldIndex for each trace in Accordions directly. But it does seem a little cumbersome to have to set foldIndex by hand in situations where Folds are direct children of the Panel.

This pr fixes that by setting a foldIndex on first level children of the Panel component. I'm not venturing into recursively trying to inspect first level children as it didn't consistently work for me when I was trying to do so with Accordions.

<Panel>
   <Fold/>
</Panel>

would this work for you @bpostlethwaite ?
I don't currently have a better solution to propose.

[nicolaskruchten]

is key required here still? I was under the impression that it was required only in list settings?

[nicolaskruchten]

or rather, could Panel set the key just like it sets foldIndex ?

[VeraZab]

key is a React api thing right, and usually when you render more than one component of the same type, you have to set a key on it so that react doesn't get mixed up. Usually nothing breaks when you don't, but from time to time it gives nasty bugs.

I don't want to add anything to the key prop. And the key prop could be anything, not necessarily an index.

[nicolaskruchten]

I don't think the key is need when you just enumerate components like this, just when you generate an array of them with map() or something. If they are required in cases like this, then this PR solves only half the problem :(

[VeraZab]

yeah, ok, rereading docs:
https://reactjs.org/docs/lists-and-keys.html
Key is not a needed prop, I'll remove it from the Default Editor

[nicolaskruchten]

Hmm, despite what the docs say, I think key is required after all, I'm getting key warnings now.

[nicolaskruchten]

Probably because within Panel we are treating children as an array ... so maybe Panel needs to set key after all? It's not common practice to put key in a context like the DefaultEditor, as per the docs.

[nicolaskruchten]

I think that this PR sets us up to avoid future problems with nested Panel > Fold > Panel > Fold like in the Color Bars in the Workspace :)

[bpostlethwaite]

Thinking outside of the ol react box! Great idea

[VeraZab]

That nested UI may probably have to be changed, to look more like the axes panel for ex.

so instead of this

something more like this

So a big button like control at the very top, where you click on the index of the color bar you'd like to change,

and then accordion menus appearing below. I think @aulneau could help us in here : ) once we get to that point

[VeraZab]

ok, merging this for now

[nicolaskruchten]

(the reason it's nested here is that there can be many traces -- i.e. not just all/x/y -- each with their own color bar, no?)

[bpostlethwaite]

We don't have nested Sections Folds currrently though right? Did sections become folds?

[bpostlethwaite]

[VeraZab]

we don't currently have those nested folds, we were just looking at the workspace with @nicolaskruchten , anticipating @bpostlethwaite : )

[nicolaskruchten]

I hate to pile on here, but... now that we're setting props on Folds from Panel ... we can stop the context thing right? and just have Panel tell the Fold if it's folded or not directly?

[nicolaskruchten]

(for a later PR!)

[VeraZab]

hmm, I'd have to think more about that, but for now I'll just merge this in.