fix(tiles): Fix background tiles id

Update pattern element to have unique id name to be properly applied on multiple chart instance on same page. Fix #225 Close #226
naver · Dec 26, 2017 · c452b43 · c452b43
1 parent 2d9025c
commit c452b43
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 42 deletions.
diff --git a/spec/color-spec.js b/spec/color-spec.js
@@ -115,17 +115,19 @@ describe("COLOR", () => {
 			});
 
 			it("should create correct pattern names", () => {
+				const internal = chart.internal;
+				const datetimeId = internal.datetimeId;
 				const expectedIds = [
-					`${CLASS.colorizePattern}-red`,
-					`${CLASS.colorizePattern}-gold`,
-					`${CLASS.colorizePattern}-green`
+					`${datetimeId}-pattern-red`,
+					`${datetimeId}-pattern-gold`,
+					`${datetimeId}-pattern-green`
 				];
 
-				const patterns = chart.internal.patterns;
+				internal.patterns.forEach((p, idx) => {
+					const id = `${expectedIds[idx]}-${idx}`;
 
-				patterns.forEach((p, idx) => {
-					expect(p.id).to.be.equal(expectedIds[idx]);
-					expect(p.node.id).to.be.equal(expectedIds[idx]);
+					expect(p.id).to.be.equal(id);
+					expect(p.node.id).to.be.equal(id);
 				});
 			});
 		});

diff --git a/src/config/classes.js b/src/config/classes.js
@@ -78,7 +78,6 @@ export default {
 	legendItemTile: "bb-legend-item-tile",
 	legendItemHidden: "bb-legend-item-hidden",
 	legendItemFocused: "bb-legend-item-focused",
-	colorizePattern: "bb-colorize-pattern",
 	dragarea: "bb-dragarea",
 	EXPANDED: "_expanded_",
 	SELECTED: "_selected_",

diff --git a/src/internals/ChartInternal.js b/src/internals/ChartInternal.js
@@ -81,8 +81,10 @@ export default class ChartInternal {
 		const $$ = this;
 		const config = $$.config;
 
-		// MEMO: clipId needs to be unique because it conflicts when multiple charts exist
+		// datetime to be used for uniqueness
 		$$.datetimeId = `bb-${+new Date()}`;
+
+		// MEMO: clipId needs to be unique because it conflicts when multiple charts exist
 		$$.clipId = `${$$.datetimeId}-clip`;
 		$$.clipIdForXAxis = `${$$.clipId}-xaxis`;
 		$$.clipIdForYAxis = `${$$.clipId}-yaxis`;

diff --git a/src/internals/color.js b/src/internals/color.js
@@ -3,11 +3,37 @@
  * billboard.js project is licensed under the MIT license
  */
 import {
+	select as d3Select,
 	scaleOrdinal as d3ScaleOrdinal,
 	schemeCategory10 as d3SchemeCategory10
 } from "d3";
 import ChartInternal from "./ChartInternal";
-import {notEmpty, extend, isFunction, colorizePattern} from "./util";
+import {notEmpty, extend, isFunction} from "./util";
+
+/**
+ * Set pattern's background color
+ * (it adds a <rect> element to simulate bg-color)
+ * @param {SVGPatternElement} pattern SVG pattern element
+ * @param {String} color Color string
+ * @param {String} id ID to be set
+ * @return {{id: string, node: SVGPatternElement}}
+ * @private
+ */
+const colorizePattern = (pattern, color, id) => {
+	const node = d3Select(pattern.cloneNode(true));
+
+	node
+		.attr("id", id)
+		.insert("rect", ":first-child")
+		.attr("width", node.attr("width"))
+		.attr("height", node.attr("height"))
+		.style("fill", color);
+
+	return {
+		id,
+		node: node.node()
+	};
+};
 
 extend(ChartInternal.prototype, {
 	generateColor() {
@@ -23,18 +49,23 @@ extend(ChartInternal.prototype, {
 			const tiles = config.color_tiles();
 
 			// Add background color to patterns
-			const colorizedPatterns = pattern.map((p, index) =>
-				colorizePattern(tiles[index % tiles.length], p)
-			);
+			const colorizedPatterns = pattern.map((p, index) => {
+				const color = p.replace(/[#\(\)\s,]/g, "");
+				const id = `${$$.datetimeId}-pattern-${color}-${index}`;
+
+				return colorizePattern(tiles[index % tiles.length], p, id);
+			});
 
 			pattern = colorizedPatterns.map(p => `url(#${p.id})`);
 			$$.patterns = colorizedPatterns;
 		}
 
 		return function(d) {
 			const id = d.id || (d.data && d.data.id) || d;
+			const isLine = $$.isTypeOf(id, "line");
 			let color;
 
+
 			// if callback function is provided
 			if (colors[id] instanceof Function) {
 				color = colors[id](d);
@@ -44,7 +75,7 @@ extend(ChartInternal.prototype, {
 				color = colors[id];
 
 			// if not specified, choose from pattern
-			} else {
+			} else if (!isLine) {
 				if (ids.indexOf(id) < 0) { ids.push(id); }
 				color = pattern[ids.indexOf(id) % pattern.length];
 				colors[id] = color;

diff --git a/src/internals/util.js b/src/internals/util.js
@@ -4,7 +4,6 @@
  */
 import {
 	event as d3Event,
-	select as d3Select,
 	brushSelection as d3BrushSelection
 } from "d3";
 import CLASS from "../config/classes";
@@ -237,32 +236,6 @@ const merge = (target, ...objectN) => {
 	return extend(target, ...objectN);
 };
 
-/**
- * Set pattern's background color
- * (it adds a <rect> element to simulate bg-color)
- * @param {SVGPatternElement} pattern
- * @param {String} color
- * @return {{id: string, node: SVGPatternElement}}
- * @private
- */
-const colorizePattern = (pattern, color) => {
-	const suffix = color.replace(/[#\(\)\s,]/g, "");
-	const id = `${CLASS.colorizePattern}-${suffix}`;
-	const node = d3Select(pattern.cloneNode(true));
-
-	node
-		.attr("id", id)
-		.insert("rect", ":first-child")
-		.attr("width", node.attr("width"))
-		.attr("height", node.attr("height"))
-		.style("fill", color);
-
-	return {
-		id,
-		node: node.node()
-	};
-};
-
 /**
  * Copy array like object to array
  * @param {Object} v
@@ -321,7 +294,6 @@ export {
 	getRectSegList,
 	merge,
 	capitalize,
-	colorizePattern,
 	toArray,
 	getCssRules
 };