Fixes for id attar + add click attr for callback. #8
Conversation
|
Smth wrong with Travis, but definitely, that is not my fault. |
| @@ -20,7 +25,18 @@ angular.module('ui.chart', []) | |||
| } | |||
| } | |||
|
|
|||
| elem.jqplot(data, opts); | |||
| $(elem).unbind("jqplotDataClick"); | |||
There was a problem hiding this comment.
You should be able to just do elem.unbind - elem is normally a jqLite wrapped element, but with jQuery present, it will be a jQuery wrapped element.
|
I will fix these Travis errors tonight - other than these minor issues I've commented on, I like this PR, and am willing to merge this and tag a new release when this is tidied up. |
| @@ -20,7 +25,18 @@ angular.module('ui.chart', []) | |||
| } | |||
| } | |||
|
|
|||
| elem.jqplot(data, opts); | |||
| $(elem).unbind("jqplotDataClick"); | |||
| $.jqplot(id, data, opts).destroy(); | |||
There was a problem hiding this comment.
It would be a good idea to keep a reference to $.jqplot(id, data, opts) cached outside the render function.
|
There should be a test added for the binding of the callback as well, to test that it properly sets the correct binding and unsets the old binding. |
| $(elem).html(''); | ||
| $.jqplot(id, data, opts); | ||
|
|
||
| var click_callback = scope.$eval(attrs.chartClick); |
There was a problem hiding this comment.
click_callback should be clickCallback, to maintain style consistency.
|
Any time this fix will be finalized and committed ? |
First of all - with the latest version of jqplot this directive is not working for me, because it call$(element).jqplot, but according to official jqplot's documentation there is only one appropriate way - $ .jqplot('elemente's id' ....);
Second - I have added chart-click attribute to pass a function that will be called when event 'jqplotDataClick' is fired.