codesuki / react-d3-components

D3 Components for React
http://codesuki.github.io/react-d3-components/example.html
MIT License
1.63k stars 205 forks source link

Rendering #105

Open solomein opened 8 years ago

solomein commented 8 years ago

Chart has a problematic rendering. Any action, such as cursor movement, raise a full re-rendering. You can see it in profile. This behavior combined with complex data will be critical.

before

I tried to fix it by adding components composition and shouldComponentUpdate implementation. Got some results.

after

Do you have any plans to change the architecture? I think it would be nice to separate the svg layers. Have you thought give up mixins (as deprecated feature) and use composition?

codesuki commented 8 years ago

Hi! Thanks for finding and benchmarking the issue. I will have a closer look at the results tomorrow. (Midnight here already)

I am totally open to change the architecture. What exactly do you mean by separating the svg layers?

In the animation branch I already made most of the charts layerable. Like

Is that related somehow?

Also yes. I want to get rid of the mixins. I didn't think about how to do that yet though.

If you have any more ideas let me here them. If you need to patch something feel free to send a pullreq:)

solomein commented 8 years ago

When I was working on the problem, I thought that the tooltip can be taken out from the component as a plugin (layer above the chart). Now I think that it makes no sense :no_mouth:

I'll look into the code more closely and try to propose the correct solution.

codesuki commented 8 years ago

Yeah! I remember I had a similar issue. I was stumbling at the tooltip. Let me know when you have a solution :) I will try to find some time to think about it too! Thanks!

You might want to look here too: https://github.com/codesuki/react-d3-components/blob/feature/transitions/src/Chart.jsx This is the branch where I implemented the chart layering. I am still not sure if it's the best way to do it. Some props should be shared with the children. Some maybe not.