Skip to content
This repository was archived by the owner on May 25, 2019. It is now read-only.

Fixes for id attar + add click attr for callback. #8

Open
developer88 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixes for id attar + add click attr for callback. #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ Add the chart module as a dependency to your application module:

Apply the directive to your div elements as an element, attribute, class, or comment:

<ui-chart="data1"></ui-chart>
<div ui-chart="data2"></div>
<div class="ui-chart; data3"></div>
<!-- directive: ui-chart data4 -->
<ui-chart="data1" id="some-id"></ui-chart>
<div ui-chart="data2" id="some-id"></div>
<div class="ui-chart; data3" id="some-id"></div>

Your data to pass to `$.jqplot` will be the evaluated value of the `ui-chart` attribute, while the options to pass to `$.jqplot` will be the evaluated value of the `chart-options` attribute - the evaluations are done in scope.
Your data to pass to `$.jqplot` will be the evaluated value of the `ui-chart` attribute, while the options to pass to `$.jqplot` will be the evaluated value of the `chart-options` attribute - the evaluations are done in scope. It is necessary to specify `id` attribute because it will be used by `$.jqplot`. It is possible to pass a function to `chart-click` attribute that will be called after click event on any node in the plot

# Options

Expand Down
20 changes: 18 additions & 2 deletions src/chart.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@ angular.module('ui.chart', [])
link: function (scope, elem, attrs) {
var renderChart = function () {
var data = scope.$eval(attrs.uiChart);
elem.html('');

if (!angular.isArray(data)) {
return;
}

var id = elem.attr('id');
if (angular.isUndefined(id)) {
throw 'Invalid ui.graph id attribute';
Copy link
Contributor

@wesleycho wesleycho Apr 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a little ungraceful - perhaps create an internal counter before the return block in the directive definition, i.e.

.directive('uiChart', function() {
  var id = 0;
  return {
    ...
    link: function (scope, elem, attrs) {
      ...
      var chartId = attrs.id;
      if (angular.isUndefined(chartId)) {
        chartId = 'chart-' + id++;
      }
      ...
  };
});

}

var opts = {};
if (!angular.isUndefined(attrs.chartOptions)) {
opts = scope.$eval(attrs.chartOptions);
Expand All @@ -20,7 +25,18 @@ angular.module('ui.chart', [])
}
}

elem.jqplot(data, opts);
$(elem).unbind("jqplotDataClick");
Copy link
Contributor

@wesleycho wesleycho Apr 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

$.jqplot(id, data, opts).destroy();
Copy link
Contributor

@wesleycho wesleycho Apr 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be a good idea to keep a reference to $.jqplot(id, data, opts) cached outside the render function.

$(elem).html('');
Copy link
Contributor

@wesleycho wesleycho Apr 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above on elem being fine instead of $(elem).

$.jqplot(id, data, opts);
Copy link
Contributor

@wesleycho wesleycho Apr 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see this cached, i.e. chart = $.jqplot(id, [data], opts). I think this breaking change with [data] instead of data is also preferable given the change in jqPlot and the documentation for jqPlot - this would keep it matched.

Copy link
Contributor

@wesleycho wesleycho Apr 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, ignore my comment about [data] instead of data.


var click_callback = scope.$eval(attrs.chartClick);
Copy link
Contributor

@wesleycho wesleycho Apr 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

click_callback should be clickCallback, to maintain style consistency.

if (angular.isFunction(click_callback)) {
return $(elem).bind('jqplotDataClick', function(ev, seriesIndex, pointIndex, data) {
Copy link
Contributor

@wesleycho wesleycho Apr 17, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here as well.

return click_callback(ev, seriesIndex, pointIndex, data);
});
}

};

scope.$watch(attrs.uiChart, function () {
Expand Down