search

txbm / angular-chartjs

Directive set for the ChartJS library. Supports data bindings and attribute-level specification for chart specific options. The only Angular ChartJS module that supports ALL chart options as HTML5 attributes :)
MIT License
146 stars 37 forks source link

Chart not updating #20

Closed julian384 closed 10 years ago

julian384 commented 10 years ago

See http://plnkr.co/edit/pVkf78hh7pAA0u0BembR?p=preview for an example.

I can't get the data on a bar chart to update when the $scope.variable has updated. Unlike in the plunker in my Chrome app the labels update but the data doesn't, in the Plunker neither update even though the chart.update() function is called (see the console logging).

txbm commented 10 years ago

@julian384 this will be fixed shortly, as in like a few hours.

txbm commented 10 years ago

@ziscloud Your solution is sub-optimal and you clearly do not intend to merge your work back into the main branch. Please refrain from attempting to fragment open source codebases as a result of a single feature bug fix or feature request.

That said, contributions are certainly welcome to benefit ALL users of a library. To that end, design your work in the form of a pull request that can be merged back into this main codebase that all users can rely on.

ziscloud commented 10 years ago

@petermelias I hope you understand I did not malicious. Your project is an amazing job, but when I look at the issues page, most of the open issues are not responsed. I cannot wait for so long time to get a fix, so I did that.

txbm commented 10 years ago

@ziscloud The point I am trying to make is that when you go ahead and "fix" something. You should do it in such a way that it is designed to be MERGED back into master so that all of the people who rely on the main repository as a "source of truth" can benefit from your changes.

I do not believe you were being malicious, you were just not being helpful.

Instead of opening a pull request, you started referring people to your fork. This is not helpful because when the main library is updated (with new features), your fork becomes useless.

Furthermore, your solution to the problem was not in line with the development of the Chart.js library, which now supports native functionality for updating chart values and automatic resizing.

If you had opened a pull request, it would have given me (and other developers) the opportunity to review your code and guide it according to the development roadmap for both projects before merging it in.

Keep in mind that when you open a pull request, you do NOT have wait for it to be merged to start using your branch. You simply use your master branch until the pull request has been merged into the main master.

This is the spirit and the purpose of open source collaboration.

ziscloud commented 10 years ago

@petermelias Yes, I am agree with you about the open source, I have removed my helpless comment posted before. Here I want to explain is, we are using bower to manage our javascript lib and before the merge request is approved, we can not get the fix via bower, right? By the way, please add tags/releases, then people can install/update this lib by a bower recognized version number.

txbm commented 10 years ago

Version 0.0.4 was tagged about 13 minutes ago, you can now install the latest code from bower or npm. See updated README for details. Thank you for your contributions.

txbm commented 10 years ago

You can modify your app's bower.json file to install dependencies from any repository URL.

Any branch, any tag, any repo, any user.