fix(legend): Fix legend template update for dynamic loading (#622)

- Remove .updateLegendWithDefaults() - Renamed .updateLegend() to .updateLegendElement() - Make .updateLegend() to handle template & non-template legend update Fix #621 Close #622
naver · Oct 24, 2018 · 1e465ae · 1e465ae
1 parent 7a62207
commit 1e465ae
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 30 deletions.
diff --git a/spec/internals/legend-spec.js b/spec/internals/legend-spec.js
@@ -323,6 +323,24 @@ describe("LEGEND", () => {
 			expect(items.size()).to.be.equal(2);
 		});
 
+		it("check for template update on dynamic loading", d => {
+			setTimeout(function() {
+				chart.load({
+					columns: [
+						["data3", 200, 100, 300, 130, 20]
+					],
+					unload: true,
+					done: () => {
+						const legend = d3.select("#legend");
+
+						expect(legend.selectAll("li").size()).to.be.equal(1);
+						expect(legend.text()).to.be.equal("data3");
+						d();
+					}
+				});
+			}, 500);
+		});
+
 		it("set options legend.content.template as function", () => {
 			args.legend.contents.template = function(title, color, data) {
 				if (title !== "data1") {

diff --git a/src/internals/ChartInternal.js b/src/internals/ChartInternal.js
@@ -614,7 +614,7 @@ export default class ChartInternal {
 			$$.inputType === "touch" && $$.hideTooltip();
 
 		// update legend and transform each g
-		if (wth.Legend && config.legend_show && !config.legend_contents_bindto) {
+		if (wth.Legend && config.legend_show) {
 			$$.updateLegend($$.mapToIds($$.data.targets), options, transitions);
 		} else if (wth.Dimension) {
 			// need to update dimension (e.g. axis.y.tick.values) because y tick values should change

diff --git a/src/internals/legend.js b/src/internals/legend.js
@@ -25,21 +25,42 @@ extend(ChartInternal.prototype, {
 		$$.legend = $$.svg.append("g");
 
 		if (config.legend_show) {
-			if (config.legend_contents_bindto && config.legend_contents_template) {
-				$$.updateLegendTemplate();
-			} else {
-				$$.legend.attr("transform", $$.getTranslate("legend"));
+			$$.legend.attr("transform", $$.getTranslate("legend"));
 
-				// MEMO: call here to update legend box and translate for all
-				// MEMO: translate will be updated by this, so transform not needed in updateLegend()
-				$$.updateLegendWithDefaults();
-			}
+			// MEMO: call here to update legend box and translate for all
+			// MEMO: translate will be updated by this, so transform not needed in updateLegend()
+			$$.updateLegend();
 		} else {
 			$$.legend.style("visibility", "hidden");
 			$$.hiddenLegendIds = $$.mapToIds($$.data.targets);
 		}
 	},
 
+	/**
+	 * Update legend element
+	 * @param targetIds
+	 * @param options
+	 * @param transitions
+	 * @private
+	 */
+	updateLegend(targetIds, options, transitions) {
+		const $$ = this;
+		const config = $$.config;
+
+		if (config.legend_contents_bindto && config.legend_contents_template) {
+			$$.updateLegendTemplate();
+		} else {
+			$$.updateLegendElement(
+				targetIds || $$.mapToIds($$.data.targets),
+				options || {
+					withTransform: false,
+					withTransitionForTransform: false,
+					withTransition: false
+				}, transitions
+			);
+		}
+	},
+
 	/**
 	 * Update legend using template option
 	 * @private
@@ -76,20 +97,6 @@ extend(ChartInternal.prototype, {
 		}
 	},
 
-	/**
-	 * Update the legend to its default value.
-	 * @private
-	 */
-	updateLegendWithDefaults() {
-		const $$ = this;
-
-		$$.updateLegend($$.mapToIds($$.data.targets), {
-			withTransform: false,
-			withTransitionForTransform: false,
-			withTransition: false
-		});
-	},
-
 	/**
 	 * Update the size of the legend.
 	 * @private
@@ -261,9 +268,7 @@ extend(ChartInternal.prototype, {
 			config.legend_show = true;
 			$$.legend.style("visibility", "visible");
 
-			if (!$$.legendHasRendered) {
-				$$.updateLegendWithDefaults();
-			}
+			!$$.legendHasRendered && $$.updateLegend();
 		}
 		$$.removeHiddenLegendIds(targetIds);
 
@@ -359,11 +364,11 @@ extend(ChartInternal.prototype, {
 	/**
 	 * Update the legend
 	 * @private
-	 * @param {Array} ID's of target
-	 * @param {Object} withTransform : Whether to use the transform property / withTransitionForTransform: Whether transition is used when using the transform property / withTransition : whether or not to transition.
-	 * @param {Object} the return value of the generateTransitions
+	 * @param {Array} targetIds ID's of target
+	 * @param {Object} options withTransform : Whether to use the transform property / withTransitionForTransform: Whether transition is used when using the transform property / withTransition : whether or not to transition.
+	 * @param {Object} transitions Return value of the generateTransitions
 	 */
-	updateLegend(targetIds, options, transitions) {
+	updateLegendElement(targetIds, options, transitions) {
 		const $$ = this;
 		const config = $$.config;
 		const paddingTop = 4;