fix(selection): Correct multiple callback calls (#120)

Check last toggle circle element to avoid multiple calls Close #117 Fixed #120
naver · Aug 23, 2017 · f7cae57 · f7cae57
1 parent 62273bf
commit f7cae57
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 4 deletions.
diff --git a/spec/selection-spec.js b/spec/selection-spec.js
@@ -0,0 +1,93 @@
+/**
+ * Copyright (c) 2017 NAVER Corp.
+ * billboard.js project is licensed under the MIT license
+ */
+/* eslint-disable */
+/* global describe, beforeEach, it, expect */
+import util from "./assets/util";
+import CLASS from "../src/config/classes";
+
+describe("SELECTION", () => {
+	let chart;
+	let args;
+	const spySelected = sinon.spy();
+	const spyUnSelected = sinon.spy();
+
+	beforeEach(() => {
+		spySelected.reset();
+		spyUnSelected.reset();
+
+		chart = util.generate(args);
+	});
+
+	describe("check for callbacks", () => {
+		describe("data.selction.enabled", () => {
+			before(() => {
+				args = {
+					data: {
+						columns: [
+							["data1", 30, 200, 100, 400, 150, 250],
+							["data2", 10, 190, 95, 40, 15, 25]
+						],
+						selection: {
+							enabled: true
+						},
+						onselected: spySelected,
+						onunselected: spyUnSelected
+					}
+				};
+			});
+
+			it("multiple selection & onselected callback", () => {
+				let circle = d3.select(`.${CLASS.shape}-3`).node().getBBox();
+				let rect = d3.select(`.${CLASS.eventRect}-3`).node();
+
+				util.setMouseEvent(chart, "click", circle.x, circle.y, rect);
+				expect(spySelected.calledOnce).to.be.true;
+
+				circle = d3.select(`.${CLASS.shape}-4`).node().getBBox();
+				rect = d3.select(`.${CLASS.eventRect}-4`).node();
+
+				util.setMouseEvent(chart, "click", circle.x, circle.y, rect);
+				expect(spySelected.calledTwice).to.be.true;
+
+				// should be selected multiple data points
+				expect(d3.selectAll(`.${CLASS.SELECTED}`).size() > 1).to.be.true;
+			});
+
+			it("set options data.selection.multiple=false", () => {
+				args.data.selection.multiple = false;
+			});
+
+			it("one selection & onunselected callback", () => {
+				let circle = d3.select(`.${CLASS.shape}-3`).node().getBBox();
+				let rect = d3.select(`.${CLASS.eventRect}-3`).node();
+
+				util.setMouseEvent(chart, "click", circle.x, circle.y, rect);
+				expect(spySelected.calledOnce).to.be.true;
+
+				circle = d3.select(`.${CLASS.shape}-4`).node().getBBox();
+				rect = d3.select(`.${CLASS.eventRect}-4`).node();
+
+				util.setMouseEvent(chart, "click", circle.x, circle.y, rect);
+				expect(spySelected.calledTwice).to.be.true;
+				expect(spyUnSelected.calledOnce).to.be.true;
+
+				// should be selected one data point only
+				expect(d3.selectAll(`.${CLASS.SELECTED}`).size() === 1).to.be.true;
+			});
+
+			it("onselected & onunselected callback should be called once", () => {
+				const circle = d3.select(`.${CLASS.shape}-3`).node().getBBox();
+				const rect = d3.select(`.${CLASS.eventRect}-3`).node();
+
+				util.setMouseEvent(chart, "click", circle.x, circle.y, rect);
+				util.setMouseEvent(chart, "click", circle.x, circle.y, rect);
+
+				expect(spySelected.calledOnce).to.be.true;
+				expect(spyUnSelected.calledOnce).to.be.true;
+			});
+		});
+
+	});
+});
diff --git a/src/internals/selection.js b/src/internals/selection.js
@@ -139,6 +139,7 @@ extend(ChartInternal.prototype, {
 		} else if (that.nodeName === "path") {
 			toggle = $$.togglePath;
 		}
+
 		return toggle;
 	},
 
@@ -155,6 +156,7 @@ extend(ChartInternal.prototype, {
 		const shape = d3Select(that);
 		const isSelected = shape.classed(CLASS.SELECTED);
 		const toggle = $$.getToggle(that, d).bind($$);
+		let toggledShape;
 
 		if (config.data_selection_enabled && config.data_selection_isselectable(d)) {
 			if (!config.data_selection_multiple) {
@@ -164,18 +166,22 @@ extend(ChartInternal.prototype, {
 					selecter = `.${selecter}${$$.getTargetSelectorSuffix(d.id)}`;
 				}
 
-				$$.main.selectAll(selecter)
-					.selectAll(`.${CLASS.shape}`)
+				$$.main.selectAll(`${selecter}`)
+					.selectAll(`circle.${CLASS.shape}`)
 					.each(function(d, i) {
 						const shape = d3Select(this);
 
 						if (shape.classed(CLASS.SELECTED)) {
+							toggledShape = shape;
 							toggle(false, shape.classed(CLASS.SELECTED, false), d, i);
 						}
 					});
 			}
-			shape.classed(CLASS.SELECTED, !isSelected);
-			toggle(!isSelected, shape, d, i);
+
+			if (!toggledShape || toggledShape.node() !== shape.node()) {
+				shape.classed(CLASS.SELECTED, !isSelected);
+				toggle(!isSelected, shape, d, i);
+			}
 		}
 	},
 });