davidwilemski / openrunlog

[Currently Unmaintained] An open source web application to record and display your workouts
https://openrunlog.org
Other
10 stars 5 forks source link

Reactify Chart re-rendering #72

Open davidwilemski opened 10 years ago

davidwilemski commented 10 years ago

This could be tricky since you call out to the library instead of editing markup. right now I'm just removing the dom elements and rebuilding them.

A potential hack for doing this could be something like this: http://jsfiddle.net/davidwilemski/6kzM9/

bkendall commented 10 years ago

I’m sure you thought of this, but what’s wrong with keeping all the state in react (the data would come from the ‘template’ right, not the data endpoint?) and then simply updating the data through a POST and updating the react stuff? Help me learn! ha

davidwilemski commented 10 years ago

Yeah you're right that it's possible. The tricky part isn't with where to store the data - there's multiple ways to do that including what you mention.

The part that doesn't quite work with react's flow is that react tries to be efficient in which DOM elements it updates so it has a virtual representation that it updates first and runs a diff against what's currently there and only updates nodes that should change. The problem here is that since the chart itself is inserted by the xChart library and not rendered directly by the react component, react doesn't know about the chart and so it won't update.

The "hack" that I've figured out to get around that is to just force the chart update on every call to render() and let the graphing library figure out what to do:

if (this.myChart) {
    this.myChart.setData(this.getInitialState()["data"]);
}

As a result, the above sort of drops benefits of react but it's the best I've been able to come up with so far.

bkendall commented 10 years ago

Ohhh - I see. Would it be helpful if this was re-written in pure D3? Though, I think, you’d still have the issue with D3 trying to figure out what to do (you’d end up having to call an equivalent ‘render’ on D3 to make your changes appear, but then it’s pure D3 (and not one more layer of abstraction through that charting library?).

davidwilemski commented 10 years ago

Yeah I'm not sure if that's beneficial since it wouldn't alleviate these problems - so I'm not sure it's worth the work. I think the chart library itself pretty smart about not rendering if the data hasn't changed so it might not actually matter.

xCharts is actually pretty nice but I think I've encountered a few bugs with it in the past few days relating to setData() , if anything I might try to track those down.