reactochart icon indicating copy to clipboard operation
reactochart copied to clipboard

Reactochart Breaks with Fragments as Children

Open ekh64 opened this issue 5 years ago • 5 comments

It'd be amazing if Reactochart could handle Fragments in some way. Currently if you pass in a component with Fragments, it doesn't render properly and the entire visualization breaks.

ekh64 avatar Aug 22 '19 18:08 ekh64

Hey @ekh64 thanks for filing! Would you be able to dive deeper into your use case and what you're using fragments for? Are you hoping to do something like this?

<XYPlot />
  <Barchart />
  <React.Fragment>
    <Component A/>
    <Component B/>
  <React.Fragment/>
<XYPlot/>

krissalvador27 avatar Aug 22 '19 19:08 krissalvador27

Exactly, or more specifically:

<XYPlot>
  <MyComponent />
<XYPlot>

where MyComponent is

  function MyComponent(...props) {
   return (<Fragment>
      <ReactochartComponentA />
      <ReactochartComponentB />
    </Fragment>)
  }

I've since found that if I pass through ...props to each of those children, it should work, but it'd be even nicer to have that pass down automatically in some Fragment type component.

ekh64 avatar Aug 27 '19 18:08 ekh64

+1 to this, this is a feature we used to support a long time ago; it got dropped in one of the rewrites and I've always felt that it would be worth returning to and getting working again. This is not just an issue with Fragments - the XYPlot currently requires all of its plots to be direct children, not ancestors.

The reason for this is because XYPlot uses resolveXYScales to determine all the necessary information about the shared X & Y chart scales - this process involves looking "down the tree" at XYPlot.children to gather information about the child charts, aggregating that information to generate the scales etc., and then passing the scales to them as props.

In order to support having other layers of wrappers/fragments in between XYPlot and its charts, XYPlot would need to recursively look down its tree of ancestors, not just at its direct children, during the resolveXYScales phase. I think this would necessitate having some way of identifying which components are valid XYPlot charts, so that this recursive process would know when to "stop" on a chart vs. continuing to recurse down to that chart's children. This could be as simple as putting a static isXYChart = true flag on all of our chart components.

Additionally, after the resolveXYScales phase, there is more work needed to make sure the generated props (scales etc.) are passed down to all of the charts... Currently this is accomplished by cloning XYPlot's direct children with the additional props, but this gets more complicated when the charts are not direct children... We could just say that any intermediate wrapper components are responsible for passing through any props from XYPlot along to their children, but not sure how this would work with Fragments.

Hope this makes sense... Just wanted to get my thought process written down as this is something I've thought about before. :)

dandelany avatar Aug 27 '19 18:08 dandelany

In a recent meet up internally I proposed a halfway solution using some kind of reactochart fragment that would do some of what resolveXYScales is doing but then expose its own children's results up to resolveXYScales. It'd allow folks to use nesting like fragments etc (as folks are wont to do), while also making it an explicit move (avoiding needless tree pathing)

scottsheffield avatar Aug 27 '19 19:08 scottsheffield

^ yeah, I think this is a reasonable path too @scottsheffield - ie. Reactochart provides a generalized wrapper component or HOC, which implements getDomain etc. (the methods used by resolveXYScales) by calling getDomain on all of its children and aggregating the results.

dandelany avatar Aug 27 '19 20:08 dandelany