fix(api): Correct toggle interaction (#456)

Split show & hide tasks for grouping and ran separately. For hide tasks schedule tasks using setTimeout. Fix #454 Close #456
naver · Jun 21, 2018 · 3682a83 · 3682a83
1 parent 353f809
commit 3682a83
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 19 deletions.
diff --git a/spec/api/api.show-spec.js b/spec/api/api.show-spec.js
@@ -29,8 +29,10 @@ describe("API show", () => {
 			chart.hide("data1");
 
 			setTimeout(() => {
-				expect(+main.select(`.${CLASS.chartLine}.${CLASS.target}-data1`).style("opacity")).to.be.equal(0);
-				expect(+main.select(`.${CLASS.chartLine}.${CLASS.target}-data2`).style("opacity")).to.be.equal(1);
+				const selector = `.${CLASS.chartLine}.${CLASS.target}`;
+
+				expect(+main.select(`${selector}-data1`).style("opacity")).to.be.equal(0);
+				expect(+main.select(`${selector}-data2`).style("opacity")).to.be.equal(1);
 
 				expect(+internal.svg.selectAll(`.${CLASS.legendItemHidden}`).size()).to.be.equal(1);
 
@@ -75,8 +77,10 @@ describe("API show", () => {
 			chart.show("data1");
 
 			setTimeout(() => {
-				expect(+main.select(`.${CLASS.chartLine}.${CLASS.target}-data1`).style("opacity")).to.be.equal(1);
-				expect(+main.select(`.${CLASS.chartLine}.${CLASS.target}-data2`).style("opacity")).to.be.equal(0);
+				const selector = `.${CLASS.chartLine}.${CLASS.target}`;
+
+				expect(+main.select(`${selector}-data1`).style("opacity")).to.be.equal(1);
+				expect(+main.select(`${selector}-data2`).style("opacity")).to.be.equal(0);
 
 				expect(+internal.svg.selectAll(`.${CLASS.legendItemHidden}`).size()).to.be.equal(1);
 
@@ -152,20 +156,22 @@ describe("API show", () => {
 			// hide data
 			chart.toggle();
 
-			main.selectAll(`.${CLASS.chartLine}`).each(function() {
-				expect(+this.style.opacity).to.be.below(1);
-			});
+			setTimeout(() => {
+				main.selectAll(`.${CLASS.chartLine}`).each(function() {
+					expect(+this.style.opacity).to.be.below(1);
+				});
 
-			legend = internal.svg.selectAll(`.${CLASS.legendItemHidden}`);
+				legend = internal.svg.selectAll(`.${CLASS.legendItemHidden}`);
 
-			expect(+legend.size()).to.be.equal(chart.data().length);
+				expect(+legend.size()).to.be.equal(chart.data().length);
 
-			legend.each(function() {
-				expect(+d3.select(this).style("opacity")).to.be.equal(0.15);
-			});
+				legend.each(function() {
+					expect(+d3.select(this).style("opacity")).to.be.equal(0.15);
+				});
 
-			// show data
-			chart.toggle();
+				// show data
+				chart.toggle();
+			}, 100);
 
 			setTimeout(() => {
 				main.selectAll(`.${CLASS.chartLine}`).each(function() {
@@ -184,4 +190,51 @@ describe("API show", () => {
 			}, 500);
 		});
 	});
+
+	describe("check toggle interaction", () => {
+		const ids = ["FirstPercentage", "SecondPercentage", "ThirdPercentage"];
+
+		it("set options", () => {
+			args = {
+				data: {
+					json: [{
+						Name: "Some Data",
+						FirstPercentage: 0.20,
+						SecondPercentage: 0.10,
+						ThirdPercentage: 0.15
+					}],
+					keys: {
+						x: "Name",
+						value: ids
+					},
+					type: "bar",
+					hide: [ids[2]]
+				},
+				axis: {
+					x: {
+						type: "category"
+					}
+				},
+				transition: {
+					duration: 10
+				}
+			};
+		});
+
+		it("should correctly rendered having same width", done => {
+			const main = chart.internal.main;
+			const barWidth = main.select(`.${CLASS.bars}-${ids[0]}`).node().getBBox().width;
+
+			chart.toggle(ids.concat().splice(1));
+
+			setTimeout(() => {
+				main.selectAll(`.${CLASS.bars}-${ids[0]}, .${CLASS.bars}-${ids[2]}`)
+					.each(function() {
+						expect(this.getBBox().width).to.be.equal(barWidth);
+					});
+
+				done();
+			}, 500);
+		});
+	});
 });
diff --git a/src/api/api.show.js b/src/api/api.show.js
@@ -108,13 +108,17 @@ extend(Chart.prototype, {
 	 * chart.toggle(["data1", "data3"]);
 	 */
 	toggle(targetIds, options = {}) {
-		const that = this;
 		const $$ = this.internal;
+		const targets = {show: [], hide: []};
 
-		$$.mapToTargetIds(targetIds).forEach(targetId => {
-			$$.isTargetToShow(targetId) ?
-				that.hide(targetId, options) : that.show(targetId, options);
-		});
+		// sort show & hide target ids
+		$$.mapToTargetIds(targetIds)
+			.forEach(id => targets[$$.isTargetToShow(id) ? "hide" : "show"].push(id));
+
+		// perform show & hide task separately
+		// https://github.com/naver/billboard.js/issues/454
+		targets.show.length && this.show(targets.show, options);
+		targets.hide.length && setTimeout(() => this.hide(targets.hide, options), 0);
 	}
 });