Add support of fragments

[aaliakseyenka]

Currently it is not possible to wrap splitter/element into fragment or custom component. This feature I would say highly needed to make composable layout, otherwise we get complex components handling both layout and logic around.

Do you have any plans bringing this feature?

Example

<ReflexContainer orientation="horizontal">
        <ReflexElement>
        </ReflexElement>
        <Bar/>
</ReflexContainer

function Bar() {
  // appearance logic
  return (
    <>
        <ReflexSplitter />
        <ReflexElement>
          <ReflexHandle/>
        </ReflexElement>
    </>
  )
}

[leefsmp]

Unfortunately it is not in the scope at the moment, based on current implementation of the library it is not easily doable or would require fundamental changes in the logic. Based on my personal experience and other developers requests so far I never felt it was a highly needed feature as you say.

If you want to separate layout appearance and core custom logic, why not simply wrapping your own components inside ReflexElements:

// layout logic
<ReflexContainer orientation="horizontal">
  <ReflexElement>
    <Component1/>
  </ReflexElement>
  <ReflexSplitter/>
  <ReflexElement>
    <Component2/>
  </ReflexElement>
</ReflexContainer

// custom logic
 Component1, Component2, ...

[aaliakseyenka]

yet, that possible, although I want to conditionally render the splitter as well and that leads to following

// layout logic
const showCmp = {logic}
<ReflexContainer orientation="horizontal">
  <ReflexElement>
    <Component1/>
  </ReflexElement>
{showCmp &&  <ReflexSplitter/>}
{showCmp &&  <ReflexElement>
    <Component2/>
  </ReflexElement>}
</ReflexContainer

which may look fine if you have jsut 2 components, but if you have rich UI you may end up in lots of such checks in single components. That's why I would move this to inner components. Especially when it relates to ReflexHandle which should be inside Component1

[leefsmp]

To me it looks more like a point of view, showing the splitter and other elements of the layout definitely sounds like layout logic so I would rather have that logic at the layout level even if it can get complicated at first rather than having it hidden at some deeper level over multiple sub-components.

Support for fragments might be doableand will allow to better organize the layout into multiple functions that can return a fragment with splitter and element so I will take a look at it, however custom components is not easily achievable I'm afraid.

[growthwp]

So, the problem here stems from the fact that the library internally uses cloneElement on the children it receives to pass down props, this means that every single time you wrap ReflexElement into something, the props don't get passed down because they're caught by that fragment.

I know exactly what you're trying to achieve. You want to generate (probably a .map) a list of ReflexElement followed by ReflexSplitter, as one would normally expect to be able to, but unfortunately, as @leefsmp mentioned, it's not currently possible. Well, sort-of. Take a look at this update I posted: https://github.com/leefsmp/Re-Flex/issues/127#issuecomment-886937054

If you pass the props to ReflexElement, it works, but I can't figure out exactly what to pass to ReflexSplitter to get it to work.

I feel disheartened. The library clearly took care of so many small details and its backlog of fixes shows commitment to get it to work but the fact that I can't generate my ReflexElement lists on the fly is...weird? It's something you write every day in your React app. What if I don't know how many elements I have upfront? What if I want to generate them based off a list?

I'm sure you know how to get it to work but, of course, it requires work. Hopefully we see it happen as this left me pretty gloomy.

[leefsmp]

I would need to look at the fragment support but I'm rather busy at the moment, also I'm actively using the lib in several projects and the topic has never been a problem so I didn't really focus on providing a better approach. There might be a better way than using cloneElement but it would be disruptive for the implementation of the logic probably.

In the meantime I might have a solution that could help, what I did in one component to handle variable/undetermined number of elements/spitters is the following (pseudo-ish code):

render () {

    return (
      <ReflexContainer>
        { this.state.children }
      </ReflexContainer>
    )
  }

I could use in the state an array of RelflexElement and Splitter which could be dynamically modified by internal logic or determined by props:

const children = []

const e1 = <ReflexElement>...</ReflexElement>

children.push(e1)

if (...) {
 const e2 = <ReflexElement>...</ReflexElement>
 const splitter = <ReflexSplitter/>
 children.push(splitter)
 children.push(e2)
}

this.setState({
     children: children
})

I agree it's not as straightforward as pure react code but it did the job.

I could also use a custom component wrapping ReflexElement assuming you propagate all the props to it:

class MyCustomElement {

  render () {

    return (
      <ReflexElement size={this.state.size} {...this.props}>
        <div className="pane-element" style={style}>
          { this.renderTitle() }
          <div className="content">
             { this.props.children }
          </div>
        </div>
      </ReflexElement>
    )
  }
}

[growthwp]

That works. Here's an implementation of it:

https://codesandbox.io/s/stoic-newton-l1nlw?file=/src/App.js

The limitation is that now you need to have the ReflexContainer and ReflexEelement/Splitter in the same component unless you use Context or something to update {someChildren} from actual children.

[AlaaZorkane]

Up! Any updates on this?
As a workaround you can do something like this:

const items = ["one", "two", "three", "four"]

const children = React.useMemo<ReactNodeArray>(() => {
  const firstItem = items[0];
  const els: ReactNodeArray = [
    <ReflexElement key={firstItem} className="left-pane">
      <div className="pane-content">
        {firstItem}
      </div>
    </ReflexElement>,
  ];

  if (items.length === 1) return els;

  for (let index = 1; index < items.length; index++) {
    const itemName = items[index];

    const { length } = items;
    const isLast = index === length - 1;

    els.push(<ReflexSplitter propagate />);

    els.push(
      <ReflexElement
        key={itemName}
        className={isLast ? "right-pane" : "middle-pane"}
      >
        <div className="pane-content">
          {itemName}
        </div>
      </ReflexElement>,
    );
  }

  return els;
}, [items]);

return (
  <ReflexContainer orientation="vertical">{children}</ReflexContainer>
);

[growthwp]

@AlaaZorkane I think Phillipe will come back to you with more, but, yea, your implementation is practically what we found as a hotfix. Also, don't expect a fix to this from any library, because the fix is to re-write the entire thing.

As I detailed here: #139 (comment) the reason for why this "bug" happens is cloneElement and I can tell you that, unfortunately, none of the libraries currently out there actually work without their children being direct descendants.

I found out that this type of approach also has deep and awful performance implications in some cases. Assume your initial flexes were stored somewhere and you need to retrieve them, but also constantly update them. What ends up happening is that you need to create a selector for all of the flexes, in one place. Because you initialize the flex of an item and you need to generate all of the elements in a loop (all in one place), every single time an item's flex changes, all others re-render to the point where you might end up having thousands of re-renders/s even with only a few dozen components in the tree.

---

As a side note: the reason for why this doesn't work as we want it to is because, really, it's not the "cover 80% of use-cases" approach, however, I digress and, as bad as it sounds, it's just the reality of our world: you implement for the majority, paired with the teachings of an old world. cloneElement used to be a thing in the past, this was a legitimate "before context" strategy for automated prop drilling. However, had the creators thought about the rest of the 20% use-cases, it would immediately be apparent that the cloneElement approach is flawed, but, frankly, I, as well any of the developers I know would've implemented it the same. It's "just good enough".