fix(flow): Fix data points removal

- Update removal nodes selection range on .flow() - refactor point.js - Update useless return from forEach on .generateWait() - Update getting x tick values differ for flow and non-flow Fix #1006
naver · Aug 5, 2019 · 5463150 · 5463150
1 parent 6083b9b
commit 5463150
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 107 deletions.
diff --git a/src/api/api.flow.js b/src/api/api.flow.js
@@ -339,31 +339,25 @@ extend(ChartInternal.prototype, {
 			]);
 
 			gt.call(wait, () => {
-				const shapes = [];
-				const texts = [];
-				const eventRects = [];
-
 				// remove flowed elements
 				if (flowLength) {
-					for (let i = 0; i < flowLength; i++) {
-						const index = flowIndex + i;
+					const target = {
+						shapes: [],
+						texts: [],
+						eventRects: []
+					};
 
-						shapes.push(`.${CLASS.shape}-${index}`);
-						texts.push(`.${CLASS.text}-${index}`);
-						eventRects.push(`.${CLASS.eventRect}-${index}`);
+					for (let i = 0; i < flowLength; i++) {
+						target.shapes.push(`.${CLASS.shape}-${i}`);
+						target.texts.push(`.${CLASS.text}-${i}`);
+						target.eventRects.push(`.${CLASS.eventRect}-${i}`);
 					}
 
-					$$.svg.selectAll(`.${CLASS.shapes}`)
-						.selectAll(shapes)
-						.remove();
-
-					$$.svg.selectAll(`.${CLASS.texts}`)
-						.selectAll(texts)
-						.remove();
-
-					$$.svg.selectAll(`.${CLASS.eventRects}`)
-						.selectAll(eventRects)
-						.remove();
+					["shapes", "texts", "eventRects"].forEach(v => {
+						$$.svg.selectAll(`.${CLASS[v]}`)
+							.selectAll(target[v])
+							.remove();
+					});
 
 					$$.svg.select(`.${CLASS.xgrid}`)
 						.remove();

diff --git a/src/data/data.js b/src/data/data.js
@@ -840,7 +840,9 @@ extend(ChartInternal.prototype, {
 	updateDataIndexByX() {
 		const $$ = this;
 		const isTimeSeries = $$.isTimeSeries();
-		const tickValues = $$.axis.getTickValues("x") || [];
+		const tickValues = $$.flowing ?
+			$$.getMaxDataCountTarget($$.data.targets).values.map(v => v.x) :
+			($$.axis.getTickValues("x") || []);
 
 		$$.data.targets.forEach(t => {
 			t.values.forEach((v, i) => {

diff --git a/src/interactions/interaction.js b/src/interactions/interaction.js
@@ -60,17 +60,12 @@ extend(ChartInternal.prototype, {
 			eventRectUpdate = $$.generateEventRectsForMultipleXs(eventRectUpdate.enter())
 				.merge(eventRectUpdate);
 		} else {
-			let xAxisTickValues = $$.axis.getTickValues("x") || $$.getMaxDataCountTarget($$.data.targets);
-
-			if (isObject(xAxisTickValues)) {
-				xAxisTickValues = xAxisTickValues.values;
-			}
-
 			// Set data and update $$.eventRect
-			const xAxisTarget = (xAxisTickValues || [])
-				.map((x, index) => ({x, index}));
+			const xAxisTickValues = $$.flowing ?
+				$$.getMaxDataCountTarget($$.data.targets).values :
+				($$.axis.getTickValues("x") || []).map((x, index) => ({x, index}));
 
-			eventRects.datum(xAxisTarget);
+			eventRects.datum(xAxisTickValues);
 
 			$$.eventRect = eventRects.selectAll(`.${CLASS.eventRect}`);
 			eventRectUpdate = $$.eventRect.data(d => d);

diff --git a/src/internals/ChartInternal.js b/src/internals/ChartInternal.js
@@ -660,13 +660,13 @@ export default class ChartInternal {
 
 	/**
 	 * Generate redraw list
-	 * @param {Object} targetsToShow targets data to be shown
+	 * @param {Object} targets targets data to be shown
 	 * @param {Object} flow
 	 * @param {Object} duration
 	 * @param {Boolean} withSubchart whether or not to show subchart
 	 * @private
 	 */
-	generateRedrawList(targetsToShow, flow, duration, withSubchart) {
+	generateRedrawList(targets, flow, duration, withSubchart) {
 		const $$ = this;
 		const config = $$.config;
 		const shape = $$.getDrawShape();
@@ -676,8 +676,8 @@ export default class ChartInternal {
 
 		// generate flow
 		const flowFn = flow && $$.generateFlow({
-			targets: targetsToShow,
-			flow: flow,
+			targets,
+			flow,
 			duration: flow.duration,
 			shape,
 			xv: $$.xv.bind($$)
@@ -1135,18 +1135,18 @@ export default class ChartInternal {
 			function loop() {
 				let done = 0;
 
-				transitionsToWait.forEach(t => {
+				for (let i = 0, t; (t = transitionsToWait[i]); i++) {
 					if (t.empty()) {
 						done++;
-						return;
+						continue;
 					}
 
 					try {
 						t.transition();
 					} catch (e) {
 						done++;
 					}
-				});
+				}
 
 				timer && clearTimeout(timer);
 

diff --git a/src/shape/point.js b/src/shape/point.js
@@ -72,12 +72,10 @@ extend(ChartInternal.prototype, {
 			return function(d) {
 				const id = d.id || (d.data && d.data.id) || d;
 				const element = d3Select(this);
-				let point;
 
-				if (ids.indexOf(id) < 0) {
-					ids.push(id);
-				}
-				point = pattern[ids.indexOf(id) % pattern.length];
+				ids.indexOf(id) < 0 && ids.push(id);
+
+				let point = pattern[ids.indexOf(id) % pattern.length];
 
 				if ($$.hasValidPointType(point)) {
 					point = $$[point];
@@ -89,10 +87,10 @@ extend(ChartInternal.prototype, {
 						$$.insertPointInfoDefs(point, pointId);
 					}
 
-					if (method === "create") {
-						return $$.custom.create.bind(context)(element, pointId, ...args);
-					} else if (method === "update") {
-						return $$.custom.update.bind(context)(element, ...args);
+					if (/^(create|update)$/.test(method)) {
+						method === "create" && args.unshift(pointId);
+
+						return $$.custom[method].bind(context)(element, ...args);
 					}
 				}
 
@@ -126,25 +124,15 @@ extend(ChartInternal.prototype, {
 			if (withTransition) {
 				const transitionName = $$.getTransitionName();
 
-				if (flow) {
-					mainCircles = element
-						.attr("x", xPosFn2);
-				}
-
-				mainCircles = element
-					.transition(transitionName)
-					.attr("x", xPosFn2)
-					.attr("y", yPosFn2)
-					.transition(transitionName);
+				flow && mainCircles.attr("x", xPosFn2);
 
+				mainCircles = mainCircles.transition(transitionName);
 				selectedCircles.transition($$.getTransitionName());
-			} else {
-				mainCircles = element
-					.attr("x", xPosFn2)
-					.attr("y", yPosFn2);
 			}
 
 			return mainCircles
+				.attr("x", xPosFn2)
+				.attr("y", yPosFn2)
 				.style("opacity", opacityStyleFn)
 				.style("fill", fillStyleFn);
 		}
@@ -167,34 +155,24 @@ extend(ChartInternal.prototype, {
 
 			// when '.load()' called, bubble size should be updated
 			if ($$.hasType("bubble")) {
-				mainCircles = mainCircles
-					.attr("r", $$.pointR.bind($$));
+				mainCircles.attr("r", $$.pointR.bind($$));
 			}
 
 			if (withTransition) {
 				const transitionName = $$.getTransitionName();
 
-				if (flow) {
-					mainCircles = mainCircles
-						.attr("cx", xPosFn);
-				}
+				flow && mainCircles.attr("cx", xPosFn);
 
-				mainCircles = element.attr("cx") ?
-					mainCircles.transition(transitionName)
-						.attr("cx", xPosFn)
-						.attr("cy", yPosFn)
-						.transition(transitionName) :
-					mainCircles.attr("cx", xPosFn)
-						.attr("cy", yPosFn);
+				if (mainCircles.attr("cx")) {
+					mainCircles = mainCircles.transition(transitionName);
+				}
 
 				selectedCircles.transition($$.getTransitionName());
-			} else {
-				mainCircles = mainCircles
-					.attr("cx", xPosFn)
-					.attr("cy", yPosFn);
 			}
 
 			return mainCircles
+				.attr("cx", xPosFn)
+				.attr("cy", yPosFn)
 				.style("opacity", opacityStyleFn)
 				.style("fill", fillStyleFn);
 		}
@@ -225,25 +203,15 @@ extend(ChartInternal.prototype, {
 			if (withTransition) {
 				const transitionName = $$.getTransitionName();
 
-				if (flow) {
-					mainCircles = mainCircles
-						.attr("x", rectXPosFn);
-				}
-
-				mainCircles = mainCircles
-					.transition(transitionName)
-					.attr("x", rectXPosFn)
-					.attr("y", rectYPosFn)
-					.transition(transitionName);
+				flow && mainCircles.attr("x", rectXPosFn);
 
+				mainCircles = mainCircles.transition(transitionName);
 				selectedCircles.transition($$.getTransitionName());
-			} else {
-				mainCircles = mainCircles
-					.attr("x", rectXPosFn)
-					.attr("y", rectYPosFn);
 			}
 
 			return mainCircles
+				.attr("x", rectXPosFn)
+				.attr("y", rectYPosFn)
 				.style("opacity", opacityStyleFn)
 				.style("fill", fillStyleFn);
 		}

diff --git a/src/shape/shape.js b/src/shape/shape.js
@@ -34,26 +34,26 @@ extend(ChartInternal.prototype, {
 		const indices = {};
 		let i = 0;
 
-		$$.filterTargetsToShow($$.data.targets
-			.filter(typeFilter, $$))
-			.forEach(d => {
-				for (let j = 0, groups; (groups = config.data_groups[j]); j++) {
-					if (groups.indexOf(d.id) < 0) {
-						continue;
-					}
+		$$.filterTargetsToShow(
+			$$.data.targets.filter(typeFilter, $$)
+		).forEach(d => {
+			for (let j = 0, groups; (groups = config.data_groups[j]); j++) {
+				if (groups.indexOf(d.id) < 0) {
+					continue;
+				}
 
-					for (let k = 0, row; (row = groups[k]); k++) {
-						if (row in indices) {
-							indices[d.id] = indices[row];
-							break;
-						}
+				for (let k = 0, row; (row = groups[k]); k++) {
+					if (row in indices) {
+						indices[d.id] = indices[row];
+						break;
 					}
 				}
+			}
 
-				if (isUndefined(indices[d.id])) {
-					indices[d.id] = i++;
-				}
-			});
+			if (isUndefined(indices[d.id])) {
+				indices[d.id] = i++;
+			}
+		});
 
 		indices.__max__ = i - 1;