sghall / react-move

React Move | Beautiful, data-driven animations for React
https://react-move-docs.netlify.app
MIT License
6.59k stars 168 forks source link

Zoomable Sunburst example #31

Closed techniq closed 6 years ago

techniq commented 6 years ago

I am trying to reproduce the Zoomable Sunburst d3 example using vx and react-move. So far I've been able to reproduce the example without transitions (static example) but I'm having difficulty understanding how to incorporate react-move to perform the transitions/interpolations.

Basically the zoom works by changing/transitioning the xScale's domain and yScale's domain and range based on a selection and generating new paths along the way.

Ultimately it's this logic I'm trying try "port" to react-move.

function click(d) {
  svg.transition()
    .duration(750)
    .tween("scale", function() {
      var xd = d3.interpolate(x.domain(), [d.x0, d.x1]);
      var yd = d3.interpolate(y.domain(), [d.y0, 1]);
      var yr = d3.interpolate(y.range(), [d.y0 ? 20 : 0, radius]);

      return function(t) {
          x.domain(xd(t));
          y.domain(yd(t))
            .range(yr(t));
      };
    })
    .selectAll("path")
      .attrTween("d", function(d) { return function() { return arc(d); }; });
}

Any assistance would be much appreciated.

sghall commented 6 years ago

Hey there. Awesome. I did a zoomable sunburst example in the old resonance repo (where react-move came from). It uses redux to store the state, but you can abstract that out for your own purposes.

Online demo

Source code

Hopefully that will get you started. Works pretty well but there's room for improvement. Let me know if you have questions. Like to see your version if you want to pass a link later.

sghall commented 6 years ago

Just looking at my example...might not be super clear. It uses reselect to update the derivative data.

All the logic for updating the data is here. I made some optimizations that are annotated in the code.

techniq commented 6 years ago

Thanks for this, it looks exactly what I'm trying to do.. I've got to run to my boy's soccer game now, but I'll digest this example tonight. I should be able to break it down and figure it out, but if I have any questions, I'll followup in here (and close the issue)

techniq commented 6 years ago

So it looks like this example is using a d3-timer directly to tween the scales...

  componentWillMount() {
    const { xScale, yScale } = this.props;

    x.range(xScale.range()).domain(xScale.domain());
    y.range(yScale.range()).domain(yScale.domain());
  }

  componentWillReceiveProps(next) {
    const { props } = this;
    const { duration } = this.state;

    // ...

    if (
      next.xScale !== props.xScale ||
      next.yScale !== props.yScale
    ) {
      if (this.transition) {
        this.transition.stop();
      }

      const { xd, yd, yr } = getScaleInterpolators(next);

      this.transition = timer((elapsed) => {
        const t = easeQuad(elapsed < duration ? (elapsed / duration) : 1);

        x.domain(xd(t));
        y.domain(yd(t)).range(yr(t));

        if (t >= 1) {
          this.transition.stop();
          this.transition = null;
        }
      });
    }
  }

https://github.com/sghall/resonance/blob/master/docs/src/routes/reduxExamples/webpackSunburst/components/index.js#L55-L67

I guess I was expecting this to be encapsulated by resonance/react-move.

I've been thinking that maybe this is possibly by nesting an Animate around a NodeGroup - Animate to transition the scales, NodeGroup to transitions the paths). I'm still trying to grok all this (and have only been looking on and off today, so I may be way off base. Getting ready to head out again, but plan to keep thinking on this later.

sghall commented 6 years ago

Yeah, great idea. I think wrapping an Animate around the NodeGroup that would stream the scale values would be a much cleaner way to do this with react-move. Be interested to see a working version.

techniq commented 6 years ago

I'm in the process of trying to get this to work now. Making some headway, but I'll share a link once I get it a little further along. Thanks for the input.

On Sun, Oct 15, 2017 at 9:44 PM Steve Hall notifications@github.com wrote:

Yeah, great idea. I think wrapping an Animate around the NodeGroup that would stream the scale values would be a much cleaner way to do this with react-move. Be interested to see a working version.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/react-tools/react-move/issues/31#issuecomment-336759450, or mute the thread https://github.com/notifications/unsubscribe-auth/AAK1RFT_lMFN9yWJoliwNiSArtTMYxFjks5ssrTpgaJpZM4P5wXp .

techniq commented 6 years ago

Well I got it working... https://codesandbox.io/s/v3x0yw11q5

The performance is fairly poor right now so it will need some digging into. I saw in your webpack sunburst example you had made some optimizations (1, 2) so I'll likely have to try to figure out something as well.

I'm also not overly happy with the approach I'm currently using - I'm overloading a lot in state, I feel like update={...} should be returning the arc generator, etc.

One thing that might be helpful from react-move is if you could pass a function to update={} that returned an object instead of just an object. This would allow for some prep work to be done outside of the timing functions (but know an update is getting ready to happen, without using a start event (useful for variables/closure, etc)).

Anyways, something like this seems easier to read than my current approach of using state to store xd, yd, and yr

<Animate
  start={{}}
  update={() => {
    const xd = interpolate(this.xScale.domain(), xDomain);
    const yd = interpolate(this.yScale.domain(), yDomain);
    const yr = interpolate(this.yScale.range(), yRange);

    return {
      unused: t => {
        this.xScale.domain(xd(t));
        this.yScale.domain(yd(t))
          .range(yr(t));
      },
      timing: {
        duration: 800
      }
    }}
  }}
>

and/or allow on a per key basis (a function that returns another function or object

<Animate
  start={{}}
  update={{
    unused : () => {
      const xd = interpolate(this.xScale.domain(), xDomain);
      const yd = interpolate(this.yScale.domain(), yDomain);
      const yr = interpolate(this.yScale.range(), yRange);

      return t => {
        this.xScale.domain(xd(t));
        this.yScale.domain(yd(t))
          .range(yr(t));
      },
      timing: {
        duration: 800
      }
    }}
  }}
>
techniq commented 6 years ago

Perf was caused by the dev version of React. Deployed to now, which builds with prod React, is much smoother - https://csb-v3x0yw11q5-qsarbnjcst.now.sh/

I'd like like your feedback about passing functions in the previous comment

sghall commented 6 years ago

Totally agree on passing functions in Animate. That's the way it was before and got changed in the switch to react-move. You want to do a PR? It's a pretty simple change.

I'll take another look at this later but off the top of my head it seems like you'd get a boost from only calling descendants once per update. That creates a new array each time you call it. Less garbage collection at least I would think...

    return (
      <svg width={width} height={height}>
        <Partition
          top={margin.top}
          left={margin.left}
          root={root}
        >
          {({ data }) => {
            const descendants = data.descendants()  // call it here

            return (
              <Group>
                <Animate
                  start={{}}
                  update={{
                    unused: t => {
                      this.xScale.domain(xd(t));
                      this.yScale.domain(yd(t))
                        .range(yr(t));
                    },
                    timing: {
                      duration: 800
                    }
                  }}
                >
                  {({ d }) => {
                    return (
                      <Group>
                        <Group top={height / 2} left={width / 2}>
                          {descendants.map((node, i) => ( // use it here
                            <path
                              d={this.arc(node)}
                              stroke="#fff"
                              fill={color((node.children ? node.data : node.parent.data).name)}
                              fillRule="evenodd"
                              onClick={() => this.handleClick(node)}
                              key={`node-${i}`}
                            />
                          ))}
                        </Group>
                      </Group>
                    )
                  }}
                </Animate>
              </Group>
            )
          }}
        </Partition>
      </svg>
    );

Also, given that you call it once higher up, it seems like you could just make an array with the fills, click handlers and a unique key (probably not great to use the index) that's all ready to go so you don't do that work in the high speed updates. I think the only thing that actually changes is the d attribute, right? I need to pull it down and try it locally.

Anyway, the performance is great as is and this is a much simpler example than mine. Might just look to make a few tweaks.

Nice work. It's a great example.

techniq commented 6 years ago

Thanks for the feedback :).

I did a quick profile on the before, moving up only data.descendants(), and moving all node props (and yes, only the d prop is needed in the Animate loop)...

Before any changes

image

Moving only data.descendants() above Animate

image

Moving all props except path

image

Surprisingly there wasn't much difference between the 3 (these were all with the dev build of react, so there could be some perf gains only reveled within the prod version)

If you do come across any perf wins yourself, please feel free to share :)

Regarding the PR, which of the 2 options did you like from the above (or both)?

<Animate
  update={() => {
    // do some work before loop
    return {
      someProp: t => { ... }
    }
  }}
/>

or

<Animate
  update={{
    someProps: {() => {
      // do some work before loop
      return t => {...}
    }
  }}
/>

I'll try to find some time to submit a PR if it looks pretty trivial as I think it could help clean up this example and streamline the code some more.

sghall commented 6 years ago

Yeah, I guess that amounts to removing one traversal of the tree per frame and, relative to the amount work being done by react and the browser, it just doesn't matter much. Oh, well.

I think we want to be consistent with the NodeGroup API. So it would be the first one...

<Animate
  update={() => {
    // do some work before loop
    return {
      someProp: t => { ... }
    }
  }}
/>

That would be simple I think, just changing start, enter, update and leave to be functions that return an object.

techniq commented 6 years ago

@sghall - Thanks for the quick turnaround on the PR merge and publish. I updated my example to take advantage of the function to build the scale interpolates in update={() => {...}) and I am much happier with the code structure and readability. I also ported the zoombale icicle example.

Sunburst

Icicle

I'm going to close this issue, but if you have any additional feedback on any performance improvements, I'd love to hear them, as it is still sluggish on mobile compared to the pure-d3 examples.