fix(subchart, zoom): Fix returning domain value

- Adjust .subchart()/.zoom() domain return value - Update subchart.onbrush callback to be called only by dragging or API call Fix #3347
naver · Aug 18, 2023 · 90338ec · 90338ec
1 parent 976f04a
commit 90338ec
Show file tree

Hide file tree

Showing 11 changed files with 180 additions and 91 deletions.
diff --git a/src/Chart/api/subchart.ts b/src/Chart/api/subchart.ts
@@ -8,6 +8,8 @@ import type {TDomain} from "../../ChartInternal/data/IData";
 
 /**
  * Select subchart by giving x domain range.
+ * - **ℹ️ NOTE:**
+ *  - Due to the limitations of floating point precision, domain value may not be exact returning approximately values.
  * @function subchart
  * @instance
  * @memberof Chart
@@ -18,31 +20,40 @@ import type {TDomain} from "../../ChartInternal/data/IData";
  *  chart.subchart([1, 2]);
  *
  *  // Get the current subchart selection domain range
+ *  // Domain value may not be exact returning approximately values.
  *  chart.subchart();
  */
 // NOTE: declared funciton assigning to variable to prevent duplicated method generation in JSDoc.
 const subchart = function<T = TDomain[]>(domainValue?: T): T | undefined {
 	const $$ = this.internal;
-	const {axis, brush, config, scale: {x, subX}} = $$;
-	let domain: any = domainValue;
+	const {axis, brush, config, scale: {x, subX}, state} = $$;
+	let domain;
 
-	if (config.subchart_show && Array.isArray(domain)) {
-		if (axis.isTimeSeries()) {
-			domain = domain.map(x => parseDate.bind($$)(x));
-		}
+	if (config.subchart_show) {
+		domain = domainValue;
+
+		if (Array.isArray(domain)) {
+			if (axis.isTimeSeries()) {
+				domain = domain.map(x => parseDate.bind($$)(x));
+			}
+
+			const isWithinRange = $$.withinRange(
+				domain,
+				$$.getZoomDomain("subX", true),
+				$$.getZoomDomain("subX")
+			);
 
-		const isWithinRange = $$.withinRange(
-			domain,
-			$$.getZoomDomain("subX", true),
-			$$.getZoomDomain("subX")
-		);
-
-		isWithinRange && brush.move(
-			brush.getSelection(),
-			domain.map(subX)
-		);
-	} else {
-		domain = x.orgDomain();
+			if (isWithinRange) {
+				state.domain = domain;
+
+				brush.move(
+					brush.getSelection(),
+					domain.map(subX)
+				);
+			}
+		} else {
+			domain = state.domain ?? x.orgDomain();
+		}
 	}
 
 	return domain as T;

diff --git a/src/Chart/api/zoom.ts b/src/Chart/api/zoom.ts
@@ -8,9 +8,11 @@ import type {TDomainRange} from "../../ChartInternal/data/IData";
 
 /**
  * Zoom by giving x domain range.
- * - **NOTE:**
+ * - **ℹ️ NOTE:**
  *  - For `wheel` type zoom, the minimum zoom range will be set as the given domain range. To get the initial state, [.unzoom()](#unzoom) should be called.
  *  - To be used [zoom.enabled](Options.html#.zoom) option should be set as `truthy`.
+ *  - When x axis type is `category`, domain range should be specified as index numbers.
+ *  - Due to the limitations of floating point precision, domain value may not be exact returning approximately values.
  * @function zoom
  * @instance
  * @memberof Chart
@@ -20,70 +22,80 @@ import type {TDomainRange} from "../../ChartInternal/data/IData";
  *  // Zoom to specified domain range
  *  chart.zoom([10, 20]);
  *
- *  // For timeseries, the domain value can be string, but the format should match with the 'data.xFormat' option.
+ *  // For timeseries x axis, the domain value can be string, but the format should match with the 'data.xFormat' option.
  *  chart.zoom(["2021-02-03", "2021-02-08"]);
  *
+ *  // For category x axis, the domain value should be index number.
+ *  chart.zoom([0, 3]);
+ *
  *  // Get the current zoomed domain range
+ *  // Domain value may not be exact returning approximately values.
  *  chart.zoom();
  */
 // NOTE: declared funciton assigning to variable to prevent duplicated method generation in JSDoc.
 const zoom = function<T = TDomainRange>(domainValue?: T): T | undefined {
 	const $$ = this.internal;
-	const {$el, axis, config, org, scale} = $$;
+	const {$el, axis, config, org, scale, state} = $$;
 	const isRotated = config.axis_rotated;
 	const isCategorized = axis.isCategorized();
-	let domain: any = domainValue;
-
-	if (config.zoom_enabled && Array.isArray(domain)) {
-		if (axis.isTimeSeries()) {
-			domain = domain.map(x => parseDate.bind($$)(x));
-		}
+	let domain;
 
-		const isWithinRange = $$.withinRange(
-			domain,
-			$$.getZoomDomain("zoom", true),
-			$$.getZoomDomain("zoom")
-		);
+	if (config.zoom_enabled) {
+		domain = domainValue;
 
-		if (isWithinRange) {
-			if (isCategorized) {
-				domain = domain.map((v, i) => Number(v) + (i === 0 ? 0 : 1)) as T;
+		if (Array.isArray(domain)) {
+			if (axis.isTimeSeries()) {
+				domain = domain.map(x => parseDate.bind($$)(x));
 			}
 
-			// hide any possible tooltip show before the zoom
-			$$.api.tooltip.hide();
-
-			if (config.subchart_show) {
-				const x = scale.zoom || scale.x;
-
-				$$.brush.getSelection().call($$.brush.move, domain.map(x));
-				// resultDomain = domain;
-			} else {
-				// in case of 'config.zoom_rescale=true', use org.xScale
-				const x = isCategorized ? scale.x.orgScale() : (org.xScale || scale.x);
-
-				// Get transform from given domain value
-				// https://github.com/d3/d3-zoom/issues/57#issuecomment-246434951
-				const translate = [-x(domain[0]), 0];
-				const transform = d3ZoomIdentity
-					.scale(x.range()[1] / (
-						x(domain[1]) - x(domain[0])
-					))
-					.translate(
-						...(isRotated ? translate.reverse() : translate) as [number, number]
-					);
-
-				$el.eventRect
-					.call($$.zoom.transform, transform);
+			const isWithinRange = $$.withinRange(
+				domain,
+				$$.getZoomDomain("zoom", true),
+				$$.getZoomDomain("zoom")
+			);
+
+			if (isWithinRange) {
+				state.domain = domain;
+
+				if (isCategorized) {
+					domain = domain.map((v, i) => Number(v) + (i === 0 ? 0 : 1));
+				}
+
+				// hide any possible tooltip show before the zoom
+				$$.api.tooltip.hide();
+
+				if (config.subchart_show) {
+					const x = scale.zoom || scale.x;
+
+					$$.brush.getSelection().call($$.brush.move, domain.map(x));
+					// resultDomain = domain;
+				} else {
+					// in case of 'config.zoom_rescale=true', use org.xScale
+					const x = isCategorized ? scale.x.orgScale() : (org.xScale || scale.x);
+
+					// Get transform from given domain value
+					// https://github.com/d3/d3-zoom/issues/57#issuecomment-246434951
+					const translate = [-x(domain[0]), 0];
+					const transform = d3ZoomIdentity
+						.scale(x.range()[1] / (
+							x(domain[1]) - x(domain[0])
+						))
+						.translate(
+							...(isRotated ? translate.reverse() : translate) as [number, number]
+						);
+
+					$el.eventRect
+						.call($$.zoom.transform, transform);
+				}
+
+				$$.setZoomResetButton();
 			}
-
-			$$.setZoomResetButton();
+		} else {
+			domain = $$.zoom.getDomain();
 		}
-	} else {
-		domain = scale.zoom?.domain() ?? scale.x.orgDomain();
 	}
 
-	return domain;
+	return state.domain ?? domain;
 };
 
 extend(zoom, {
@@ -217,7 +229,7 @@ export default {
 	 */
 	unzoom(): void {
 		const $$ = this.internal;
-		const {config, $el: {eventRect, zoomResetBtn}} = $$;
+		const {config, $el: {eventRect, zoomResetBtn}, state} = $$;
 
 		if ($$.scale.zoom) {
 			config.subchart_show ?
@@ -231,6 +243,8 @@ export default {
 			if (d3ZoomTransform(eventRect.node()) !== d3ZoomIdentity) {
 				$$.zoom.transform(eventRect, d3ZoomIdentity);
 			}
+
+			state.domain = undefined;
 		}
 	}
 };
diff --git a/src/ChartInternal/interactions/subchart.ts b/src/ChartInternal/interactions/subchart.ts
@@ -18,9 +18,10 @@ export default {
 	 */
 	initBrush(): void {
 		const $$ = this;
-		const {config, scale, $el: {subchart}} = $$;
+		const {config, scale, $el: {subchart}, state} = $$;
 		const isRotated = config.axis_rotated;
 		let lastDomain;
+		let lastSelection;
 		let timeout;
 
 		// set the brush
@@ -42,14 +43,25 @@ export default {
 
 		// bind brush event
 		$$.brush.on("start brush end", event => {
-			const {selection, target, type} = event;
+			const {selection, sourceEvent, target, type} = event;
 
 			if (type === "start") {
 				$$.state.inputType === "touch" && $$.hideTooltip();
+				lastSelection = sourceEvent ? selection : null;
+				// sourceEvent && (state.domain = null);
 			}
 
+			// if (type === "brush") {
 			if (/(start|brush)/.test(type)) {
-				$$.redrawForBrush();
+				// when brush selection updates happens on one edge, update only chainging edge and
+				// is only for adjustment of given domain range to be used to return current domain range.
+				type === "brush" && sourceEvent && state.domain && lastSelection?.forEach((v, i) => {
+					if (v !== selection[i]) {
+						state.domain[i] = scale.x.orgDomain()[i];
+					}
+				});
+
+				$$.redrawForBrush(type !== "start");
 			}
 
 			if (type === "end") {
@@ -320,23 +332,28 @@ export default {
 					$$.redrawCircle(cx, cy, withTransition, undefined, true);
 				}
 
-				!state.rendered && initRange && $$.brush.move(
-					$$.brush.getSelection(),
-					initRange.map($$.scale.x)
-				);
+				if (!state.rendered && initRange) {
+					state.domain = initRange;
+
+					$$.brush.move(
+						$$.brush.getSelection(),
+						initRange.map($$.scale.x)
+					);
+				}
 			}
 		}
 	},
 
 	/**
 	 * Redraw the brush.
+	 * @param {boolean} [callCallbck=true] Call 'onbrush' callback or not.
 	 * @private
 	 */
-	redrawForBrush() {
+	redrawForBrush(callCallbck = true): void {
 		const $$ = this;
 		const {config: {
 			subchart_onbrush: onBrush, zoom_rescale: withY
-		}, scale} = $$;
+		}, scale, state} = $$;
 
 		$$.redraw({
 			withTransition: false,
@@ -346,7 +363,8 @@ export default {
 			withDimension: false
 		});
 
-		onBrush.bind($$.api)(scale.x.orgDomain());
+		callCallbck && state.rendered &&
+			onBrush.bind($$.api)(state.domain ?? scale.x.orgDomain());
 	},
 
 	/**

diff --git a/src/ChartInternal/interactions/zoom.ts b/src/ChartInternal/interactions/zoom.ts
@@ -125,7 +125,7 @@ export default {
 		 * @private
 		 */
 		// @ts-ignore
-		zoom.getDomain = (): number|Date[] => {
+		zoom.getDomain = (): (number | Date)[] => {
 			const domain = scale[scale.zoom ? "zoom" : "subX"].domain();
 			const isCategorized = $$.axis.isCategorized();
 
@@ -135,6 +135,7 @@ export default {
 
 			return domain;
 		};
+
 		$$.zoom = zoom;
 	},
 
@@ -175,6 +176,7 @@ export default {
 
 		if (event.sourceEvent) {
 			state.zooming = true;
+			state.domain = undefined;
 		}
 
 		const isMousemove = sourceEvent?.type === "mousemove";
@@ -206,7 +208,11 @@ export default {
 		$$.state.cancelClick = isMousemove;
 
 		// do not call event cb when is .unzoom() is called
-		!isUnZoom && callFn(config.zoom_onzoom, $$.api, $$.zoom.getDomain());
+		!isUnZoom && callFn(
+			config.zoom_onzoom,
+			$$.api,
+			$$.state.domain ?? $$.zoom.getDomain()
+		);
 	},
 
 	/**
@@ -239,7 +245,11 @@ export default {
 		state.zooming = false;
 
 		// do not call event cb when is .unzoom() is called
-		!isUnZoom && (e || state.dragging) && callFn(config.zoom_onzoomend, $$.api, $$.zoom.getDomain());
+		!isUnZoom && (e || state.dragging) && callFn(
+			config.zoom_onzoomend,
+			$$.api,
+			$$.state.domain ?? $$.zoom.getDomain()
+		);
 	},
 
 	/**

diff --git a/src/config/Store/State.ts b/src/config/Store/State.ts
@@ -46,6 +46,9 @@ export default class State {
 			cssRule: {},
 
 			current: {
+				// current domain value. Assigned when is zoom is called
+				domain: undefined,
+
 				// chart whole dimension
 				width: 0,
 				height: 0,