Skip to content
This repository was archived by the owner on Jun 6, 2024. It is now read-only.

Conversation

@benzitohhh
Copy link
Contributor

Hi @topicus, hope all good with you.

Unfortunately this.props.renderEnd is only triggered the first time the chart is rendered.

For the project I'm working on, we need the callback to trigger on every chart update or window resize.

Specifically, our callback renders a grey "predicted values" rectangle on top of the chart, showing the user which values are predictions. So if the data changes or the window is resized, we really need to redraw the grey rectangle.

This patch fixes the problem.

I understand that perhaps the problem should be fixed within nvd3 itself. Still, it would be really great to have this feature as a workaround for now.

Thanks,
Ben

screen shot 2016-02-22 at 14 15 25

@topicus
Copy link
Contributor

topicus commented Feb 22, 2016

I will rename the current renderEnd as onReady and bind the renderEnd properly. It seems like renderEnd in nvd3 differs across charts. I looked at that meanwhile I was solving another issue.

@benzitohhh
Copy link
Contributor Author

ok thanks - really appreciated.

The callback we pass into nvd3's addGraph() is only called once, regardless of chart type:
https://github.com/novus/nvd3/blob/master/src/core.js#L98

@topicus
Copy link
Contributor

topicus commented Feb 23, 2016

@benzitohhh I've added this those events https://github.com/NuCivic/react-nvd3#events. Let me know if that works for you.

@benzitohhh
Copy link
Contributor Author

Works great - thanks for the super speedy response.

@topicus
Copy link
Contributor

topicus commented Feb 23, 2016

Awesome. Closing this.

@topicus topicus closed this Feb 23, 2016
@benzitohhh
Copy link
Contributor Author

Sorry @topicus, turns out I did not check the commit properly.

The renderEnd does not work for LineCharts. It is never called. You can see this on the /examples/lineChart:

ReactDOM.render(
    React.createElement(NVD3Chart, {
      ...
      type:'lineChart',
      datum: datum,
      ...
      renderEnd: function() { console.log('renderEnd'); } // this is never called
    }),
    document.getElementById('lineChart')
  );

Seems like the dispatch stuff is really inconsistent in nvd3.

The only way I can get this stuff to work is as in the patch I submitted before.

One workaround would be to expose a postUpdate property, and call that on update/windowResize. This would be as in my previous patch, except calling postUpdate rather than renderEnd). I added a pull request for it here: #29

What do you think?

@topicus
Copy link
Contributor

topicus commented Feb 24, 2016

@benzitohhh it's working for me in the lineChart at least. I've updated the example to demo how it works. Let me know if you still have issues with this.

@benzitohhh
Copy link
Contributor Author

Hmmm that's really weird.

For me.... on the lineChart demo, renderEnd callback is never fired (both on change data, and on resizeWindow).

I just grabbed a fresh clone from github. Am I missing something?

@topicus
Copy link
Contributor

topicus commented Feb 25, 2016

Which browser are you using?

screen shot 2016-02-25 at 11 09 32 am

@benzitohhh
Copy link
Contributor Author

Hmmm that's interesting. I've tried it on Chrome, Safari and Firefox; and also on two different machines (both OSX); but the renderEnd never fires for me.

I did an npm update to make sure I have the latest dependencies, and also updated gulp, but still no joy:

d3 v3.5.16
nvd3 v1.8.2
react v0.14.7
react-dom v0.14.7
gulp v3.9.1

I'm running the example by using gulp serve, and then naviagting to http://localhost:3000/examples/lineChart/

Any ideas?

screen shot 2016-02-25 at 20 53 44

screen shot 2016-02-25 at 20 54 29

@topicus
Copy link
Contributor

topicus commented Feb 26, 2016

I was able to reproduce it by clone the repo again. It seems like something change in the nvd3 library which causes lineChart not being animated by default.

I've pushed a workaround since it's an issue in the base library (nvd3).

In order to trigger the renderEnd event you need to set the duration property 1 at least. If duration is 0 then renderEnd is never triggered

I've updated the lineChart example.

@benzitohhh
Copy link
Contributor Author

ok cool - thanks for looking into this

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants