Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reactochart Breaks with Fragments as Children #174

Open
ekh64 opened this issue Aug 22, 2019 · 5 comments
Open

Reactochart Breaks with Fragments as Children #174

ekh64 opened this issue Aug 22, 2019 · 5 comments

Comments

@ekh64
Copy link
Collaborator

ekh64 commented Aug 22, 2019

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.

@krissalvador27
Copy link
Contributor

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/>

@ekh64
Copy link
Collaborator Author

ekh64 commented Aug 27, 2019

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.

@dandelany
Copy link
Contributor

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

@scottsheffield
Copy link
Contributor

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)

@dandelany
Copy link
Contributor

^ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants