Skip to content

Commit

Permalink
fix(data): Remove selection on data.onmin/max (#143)
Browse files Browse the repository at this point in the history
Chart elements are rendered async, but callback is executed synchronously.
Element selection is buggy and can cause problem.

Fix #8 
Close #143
  • Loading branch information
netil committed Sep 19, 2017
1 parent e05c10f commit 7709c32
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 39 deletions.
7 changes: 0 additions & 7 deletions spec/data-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1467,11 +1467,6 @@ describe("DATA", () => {
expect(minData[0].value).to.be.equal(minData[1].value);
expect(minData[0].id).to.not.be.equal(minData[1].id);

expect(minData[0].element.tagName).to.be.equal("circle");
expect(minData[1].element.tagName).to.be.equal("circle");
expect(minData[0].element.getAttribute("cy")).to.be.equal(minData[1].element.getAttribute("cy"));
expect(+minData[0].element.getAttribute("cy")).to.be.closeTo(390, 5);

done();
}, 100);
});
Expand All @@ -1483,8 +1478,6 @@ describe("DATA", () => {
expect(maxData[0].value).to.be.equal(400);
expect(maxData[0].id).to.be.equal("data1");

expect(maxData[0].element.tagName).to.be.equal("circle");
expect(+maxData[0].element.getAttribute("cy")).to.be.closeTo(36, 5);
done();
}, 100);
});
Expand Down
4 changes: 2 additions & 2 deletions src/config/Options.js
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ export default class Options {
* @default undefined
* @example
* onmin: function(data) {
* // data - ex) [{x: 3, value: 400, id: "data1", index: 3, element: circle}, ... ]
* // data - ex) [{x: 3, value: 400, id: "data1", index: 3}, ... ]
* ...
* }
*/
Expand All @@ -807,7 +807,7 @@ export default class Options {
* @default undefined
* @example
* onmax: function(data) {
* // data - ex) [{x: 3, value: 400, id: "data1", index: 3, element: circle}, ... ]
* // data - ex) [{x: 3, value: 400, id: "data1", index: 3}, ... ]
* ...
* }
*/
Expand Down
32 changes: 2 additions & 30 deletions src/data/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import {
set as d3Set,
min as d3Min,
max as d3Max,
merge as d3Merge,
select as d3Select
merge as d3Merge
} from "d3";
import CLASS from "../config/classes";
import ChartInternal from "../internals/ChartInternal";
Expand Down Expand Up @@ -228,34 +227,7 @@ extend(ChartInternal.prototype, {
* @return {Array} filtered array data
*/
getFilteredDataByValue(data, value) {
const $$ = this;

return data.filter(t => {
if (t.value === value) {
// select element. It needs schedule by setTimeout to avoid collision with d3's transitions tween
setTimeout(() => (t.element = $$.getElementByDataIndex(t)), 0);

return true;
}

return false;
});
},

/**
* Get element by data index
* @private
* @param {Object} data
* @return {SVGElement|null}
*/
getElementByDataIndex(data) {
const isArcType = this.isTypeOf(data.id, ["pie", "gauge", "donut"]);

// ex. selector for Arc types: .bb-target-data1 .bb-shape-1 / others: .bb-target-data1 .bb-arc-data1
const selector = `.${CLASS.target}-${data.id} .${isArcType ? CLASS.arc : CLASS.shape}-${isArcType ? data.id : data.index}`;
const element = d3Select(selector);

return element.empty() ? null : element.node();
return data.filter(t => t.value === value);
},

getMaxDataCount() {
Expand Down

0 comments on commit 7709c32

Please sign in to comment.