fix(zoom): fix zoom event triggering for drag type

Fix triggering zoom event

Fix #2254

package.json

@@ -103,7 +103,7 @@
 		"@babel/plugin-transform-runtime": "^7.15.0",
 		"@babel/preset-env": "^7.15.0",
 		"@babel/preset-typescript": "^7.15.0",
-		"@babel/runtime": "^7.14.8",
+		"@babel/runtime": "^7.15.3",
 		"@commitlint/cli": "^13.1.0",
 		"@commitlint/config-conventional": "^13.1.0",
 		"@rollup/plugin-babel": "^5.3.0",
@@ -147,11 +147,11 @@
 		"d3-polygon": "^3.0.1",
 		"d3-voronoi": "^1.1.4",
 		"docdash": "^1.2.0",
-		"dtslint": "^4.1.3",
+		"dtslint": "^4.1.4",
 		"eslint": "^7.32.0",
 		"eslint-config-naver": "^2.1.0",
 		"eslint-plugin-import": "^2.24.0",
-		"eslint-plugin-jsdoc": "^36.0.6",
+		"eslint-plugin-jsdoc": "^36.0.7",
 		"eslint-webpack-plugin": "^3.0.1",
 		"exports-loader": "^3.0.0",
 		"hammer-simulator": "0.0.1",
@@ -183,7 +183,7 @@
 		"string-replace-loader": "^3.0.3",
 		"style-loader": "^3.2.1",
 		"terser-webpack-plugin": "^5.1.4",
-		"tslib": "^2.3.0",
+		"tslib": "^2.3.1",
 		"typescript": "^4.3.5",
 		"webpack": "^5.50.0",
 		"webpack-bundle-analyzer": "^4.4.2",

src/Chart/api/zoom.ts

@@ -3,7 +3,7 @@
  * billboard.js project is licensed under the MIT license
  */
 import {zoomIdentity as d3ZoomIdentity, zoomTransform as d3ZoomTransform} from "d3-zoom";
-import {callFn, extend, getMinMax, isDefined, isObject, parseDate} from "../../module/util";
+import {extend, getMinMax, isDefined, isObject, parseDate} from "../../module/util";
 
 /**
  * Check if the given domain is within zoom range
@@ -83,7 +83,6 @@ const zoom = function(domainValue?: (number|Date)[]): (number|Date)[]|undefined
 			}
 
 			$$.setZoomResetButton();
-			callFn(config.zoom_onzoom, $$.api, domain);
 		}
 	} else {
 		domain = scale.zoom ?
@@ -215,6 +214,7 @@ export default {
 
 	/**
 	 * Unzoom zoomed area
+	 * - **NOTE:** Calling .unzoom() will not trigger zoom events.
 	 * @function unzoom
 	 * @instance
 	 * @memberof Chart
@@ -223,7 +223,7 @@ export default {
 	 */
 	unzoom(): void {
 		const $$ = this.internal;
-		const {config, $el: {eventRect, zoomResetBtn}, $T} = $$;
+		const {config, $el: {eventRect, zoomResetBtn}} = $$;
 
 		if ($$.scale.zoom) {
 			config.subchart_show ?
@@ -235,7 +235,7 @@ export default {
 
 			// reset transform
 			if (d3ZoomTransform(eventRect.node()) !== d3ZoomIdentity) {
-				$$.zoom.transform($T(eventRect), d3ZoomIdentity);
+				$$.zoom.transform(eventRect, d3ZoomIdentity);
 			}
 		}
 	}

src/ChartInternal/ChartInternal.ts

@@ -127,6 +127,7 @@ export default class ChartInternal {
 	$T(selection: SVGElement | d3Selection, force?: boolean, name?: string): d3Selection {
 		const {config, state} = this;
 		const duration = config.transition_duration;
+		const subchart = config.subchart_show;
 		let t = selection;
 
 		if (t) {
@@ -137,12 +138,14 @@ export default class ChartInternal {
 
 			// do not transit on:
 			// - wheel zoom (state.zooming = true)
+			// - when has no subchart
 			// - initialization
 			// - resizing
 			const transit = ((force !== false && duration) || force) &&
-				!state.zooming &&
+				(!state.zooming || state.dragging) &&
 				!state.resizing &&
-				state.rendered;
+				state.rendered &&
+				!subchart;
 
 			t = (transit ? t.transition(name).duration(duration) : t) as d3Selection;
 		}
@@ -634,19 +637,19 @@ export default class ChartInternal {
 
 	getWithOption(options) {
 		const withOptions = {
-			Y: true,
+			Dimension: true,
+			EventRect: true,
+			Legend: false,
 			Subchart: true,
+			Transform: false,
 			Transition: true,
-			EventRect: true,
-			Dimension: true,
 			TrimXDomain: true,
-			Transform: false,
+			UpdateXAxis: "UpdateXDomain",
 			UpdateXDomain: false,
 			UpdateOrgXDomain: false,
-			Legend: false,
-			UpdateXAxis: "UpdateXDomain",
 			TransitionForExit: "Transition",
-			TransitionForAxis: "Transition"
+			TransitionForAxis: "Transition",
+			Y: true
 		};
 
 		Object.keys(withOptions).forEach(key => {

src/ChartInternal/interactions/subchart.ts

@@ -72,7 +72,6 @@ export default {
 			}
 		});
 
-
 		$$.brush.updateResize = function() {
 			timeout && clearTimeout(timeout);
 			timeout = setTimeout(() => {

src/ChartInternal/interactions/zoom.ts

@@ -3,7 +3,7 @@
  * billboard.js project is licensed under the MIT license
  */
 import {drag as d3Drag} from "d3-drag";
-import {zoom as d3Zoom} from "d3-zoom";
+import {zoomIdentity as d3ZoomIdentity, zoom as d3Zoom} from "d3-zoom";
 import CLASS from "../../config/classes";
 import {callFn, diffDomain, getPointer, isFunction} from "../../module/util";
 
@@ -149,13 +149,11 @@ export default {
 		const $$ = this;
 		const {sourceEvent} = event;
 
-		if (!sourceEvent) {
-			return;
+		if (sourceEvent) {
+			$$.zoom.startEvent = sourceEvent;
+			$$.state.zooming = true;
+			callFn($$.config.zoom_onzoomstart, $$.api, event);
 		}
-
-		$$.zoom.startEvent = sourceEvent;
-		$$.state.zooming = true;
-		callFn($$.config.zoom_onzoomstart, $$.api, event);
 	},
 
 	/**
@@ -165,8 +163,9 @@ export default {
 	 */
 	onZoom(event): void {
 		const $$ = this;
-		const {config, scale, org} = $$;
+		const {config, scale, state, org} = $$;
 		const {sourceEvent} = event;
+		const isUnZoom = event?.transform === d3ZoomIdentity;
 
 		if (
 			!config.zoom_enabled ||
@@ -186,17 +185,26 @@ export default {
 
 		$$.zoom.updateTransformScale(transform);
 
+		// do zoom transiton when:
+		// - zoom type 'drag'
+		// - when .unzoom() is called (event.transform === d3ZoomIdentity)
+		const doTransition = config.transition_duration > 0 &&
+			!config.subchart_show && (
+			state.dragging || isUnZoom
+		);
 
 		$$.redraw({
-			withTransition: false,
+			withTransition: doTransition,
 			withY: config.zoom_rescale,
 			withSubchart: false,
 			withEventRect: false,
 			withDimension: false
 		});
 
 		$$.state.cancelClick = isMousemove;
-		callFn(config.zoom_onzoom, $$.api, $$.zoom.getDomain());
+
+		// do not call event cb when is .unzoom() is called
+		!isUnZoom && callFn(config.zoom_onzoom, $$.api, $$.zoom.getDomain());
 	},
 
 	/**
@@ -206,9 +214,10 @@ export default {
 	 */
 	onZoomEnd(event): void {
 		const $$ = this;
-		const {config} = $$;
+		const {config, state} = $$;
 		let {startEvent} = $$.zoom;
 		let e = event?.sourceEvent;
+		const isUnZoom = event?.transform === d3ZoomIdentity;
 
 		if ((startEvent && startEvent.type.indexOf("touch") > -1)) {
 			startEvent = startEvent.changedTouches[0];
@@ -225,8 +234,10 @@ export default {
 		$$.redrawEventRect();
 		$$.updateZoom();
 
-		$$.state.zooming = false;
-		callFn(config.zoom_onzoomend, $$.api, $$.zoom.getDomain());
+		state.zooming = false;
+
+		// do not call event cb when is .unzoom() is called
+		!isUnZoom && callFn(config.zoom_onzoomend, $$.api, $$.zoom.getDomain());
 	},
 
 	/**
@@ -326,7 +337,6 @@ export default {
 				const scale = $$.scale.zoom || $$.scale.x;
 
 				state.event = event;
-				$$.setDragStatus(false);
 
 				zoomRect
 					.attr(prop.axis, 0)
@@ -343,8 +353,9 @@ export default {
 
 				if (start !== end) {
 					$$.api.zoom([start, end].map(v => scale.invert(v)));
-					$$.onZoomEnd(event);
 				}
+
+				$$.setDragStatus(false);
 			});
 	},
 

src/ChartInternal/internals/redraw.ts

@@ -36,6 +36,7 @@ export default {
 			// no need to update axis in it because they will be updated in redraw()
 			$$.updateDimension(true);
 		}
+
 		// update circleY based on updated parameters
 		if (!$$.hasArcType() || state.hasRadar) {
 			$$.updateCircleY && ($$.circleY = $$.updateCircleY());

test/interactions/zoom-spec.ts

@@ -72,10 +72,11 @@ describe("ZOOM", function() {
 	});
 
 	describe("zoom event", () => {
-		let zoomDomain;
 		const spyOnZoomStart = sinon.spy();
 		const spyOnZoom = sinon.spy(domain => (zoomDomain = domain));
 		const spyOnZoomEnd = sinon.spy(domain => (zoomDomain = domain));
+		let zoomDomain;
+		let eventOrder = [];
 
 		before(() => {
 			args = {
@@ -231,6 +232,65 @@ describe("ZOOM", function() {
 			chart.unzoom(); // zoom set to initial
 			expect(subX.domain()).to.be.deep.equal(x.orgDomain()); // subX value not updated on zoom in
 		});
+
+		it("set options: zoom.type='drag'", () => {
+			args.zoom = {
+				enabled: true,
+				type: "drag",
+				onzoomstart: () => {
+					eventOrder.push("start");
+				},
+				onzoom: () => {
+					eventOrder.push("zoom");
+				},
+				onzoomend: () => {
+					eventOrder.push("end");
+				}
+			};
+		});
+
+		it("check on zoom event triggering during drag zooming", done => {
+			const {$: {main}, internal: {scale, $el}} = chart;
+			const eventRect = $el.eventRect.node();;
+
+
+			new Promise((resolve, reject) => {
+				util.fireEvent(eventRect, "mousedown", {
+					clientX: 50,
+					clientY: 100
+				}, chart);
+
+				resolve(true);
+			}).then(() => {
+				return new Promise((resolve, reject) => {
+					setTimeout(() => {
+						util.fireEvent(eventRect, "mousemove", {
+							clientX: 150,
+							clientY: 100
+						}, chart);
+
+						resolve(true);
+					}, 500);
+				});
+			}).then(() => {
+				setTimeout(() => {
+					util.fireEvent(eventRect, "mouseup", {
+						clientX: 150,
+						clientY: 100
+					}, chart);
+
+					expect(eventOrder).to.be.deep.equal(["start", "zoom", "end"]);
+
+					// when
+					chart.unzoom();
+
+					// the call of .unzoom() shouldn't be triggering zooming event
+					expect(eventOrder).to.be.deep.equal(["start", "zoom", "end"]);
+
+					done();
+				}, 500);
+			});
+		});
 	});
 
 	describe("zoom wheel", () => {
@@ -794,14 +854,14 @@ describe("ZOOM", function() {
 					}, chart);
 
 					// y axis rescaled?
-					// const tickText = +main.selectAll(`.${CLASS.axisY} .tick tspan`).nodes().pop().textContent;
+					const tickText = +main.selectAll(`.${CLASS.axisY} .tick tspan`).nodes().pop().textContent;
 
-					// expect(tickText).to.be.below(yAxisTickText);
-					// expect(tickText).to.be.equal(400);
+					expect(tickText).to.be.below(yAxisTickText);
+					expect(tickText).to.be.equal(400);
 
-					// scale.x.domain().forEach((v, i) => {
-					// 	expect(v).to.be[i ? "below" : "above"](zoomedDomain[i]);
-					// });
+					scale.x.domain().forEach((v, i) => {
+						expect(v).to.be[i ? "below" : "above"](zoomedDomain[i]);
+					});
 
 					done();
 				}, 500);