fix(chart): Correct the order to set '$' node values

- Change the order to get primary node elements to be accessible during onrendered callbacks - Update jsdoc description to be exposed on navbar - rename insternal .getChartElements() to .setChartElements() Fix #994
naver · Jul 25, 2019 · b97558c · b97558c
1 parent 61cf047
commit b97558c
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 86 deletions.
diff --git a/spec/internals/bb-spec.js b/spec/internals/bb-spec.js
@@ -10,94 +10,96 @@ import CLASS from "../../src/config/classes";7
 describe("Interface & initialization", () => {
 	let chart;
 
-	it("Check for billboard.js object", () => {
-		expect(bb).not.to.be.null;
-		expect(typeof bb).to.be.equal("object");
-		expect(typeof bb.generate).to.be.equal("function");
-	});
+	describe("Initialization", () => {
+		const checkElements = $ => {
+			const isD3Node = v => v && "node" in v || false;
+
+			Object.values($).forEach(v1 => {
+				const isNode = isD3Node(v1);
+
+				if (isNode) {
+					expect(isNode).to.be.true;
+				} else {
+					Object.values(v1).forEach(v2 => {
+						expect(isD3Node(v2)).to.be.true;
+					});
+				}
+			});
+		};
 
-	it("Check for initialization", () => {
-		chart = util.generate({
-			title: {
-				text: "test"
-			},
-			data: {
-				columns: [
-					["data1", 30]
-				],
-				labels: {
-					show: true
-				},
-				type: "bar"
-			}
+		it("Check for billboard.js object", () => {
+			expect(bb).not.to.be.null;
+			expect(typeof bb).to.be.equal("object");
+			expect(typeof bb.generate).to.be.equal("function");
 		});
-		const internal = chart.internal;
-
-		expect(chart).not.to.be.null;
-		expect(d3.select(chart.element).classed("bb")).to.be.true;
-		expect(internal.svg.node().tagName).to.be.equal("svg");
-		expect(internal.convertInputType()).to.be.equal(internal.inputType);
-
-		expect(chart).to.be.equal(bb.instance[0]);
 
-		// onrendered value should be undefined for default
-		expect(chart.internal.config.onrendered).to.be.undefined;
-	});
-
-	it("should return version string", () => {
-		expect(bb.version.length > 0).to.be.ok;
-	});
-
-	it("should be accessing node elements", () => {
-		const isD3Node = v => v && "node" in v || false;
+		it("Check for initialization", () => {
+			chart = util.generate({
+				title: {
+					text: "test"
+				},
+				data: {
+					columns: [
+						["data1", 30]
+					],
+					labels: {
+						show: true
+					},
+					type: "bar"
+				},
+				onrendered: ctx => checkElements(ctx.$)
+			});
+			const internal = chart.internal;
 
-		Object.values(chart.$).forEach(v1 => {
-			const isNode = isD3Node(v1);
+			expect(chart).not.to.be.null;
+			expect(d3.select(chart.element).classed("bb")).to.be.true;
+			expect(internal.svg.node().tagName).to.be.equal("svg");
+			expect(internal.convertInputType()).to.be.equal(internal.inputType);
+			expect(chart).to.be.equal(bb.instance[0]);
+		});
 
-			if (isNode) {
-				expect(isNode).to.be.true;
-			} else {
-				Object.values(v1).forEach(v2 => {
-					expect(isD3Node(v2)).to.be.true;
-				});
-			}
+		it("should return version string", () => {
+			expect(bb.version.length > 0).to.be.ok;
 		});
-	});
 
-	it("instantiate with non-existing element", () => {
-		chart = util.generate({
-			bindto: "#no-exist-element",
-			data: {
-				columns: [
-					["data1", 30]
-				]
-			}
+		it("should be accessing node elements", () => {
+			checkElements(chart.$);
 		});
 
-		expect(chart.element.classList.contains("bb")).to.be.true;
-	});
+		it("instantiate with non-existing element", () => {
+			chart = util.generate({
+				bindto: "#no-exist-element",
+				data: {
+					columns: [
+						["data1", 30]
+					]
+				}
+			});
 
-	it("instantiate with different classname on wrapper element", () => {
-		const bindtoClassName = "billboard-js";
-		chart = bb.generate({
-			bindto: {
-				element: "#chart",
-				classname: bindtoClassName
-			},
-			data: {
-				columns: [
-					["data1", 30, 200, 100, 400],
-					["data2", 500, 800, 500, 2000]
-				]
-			}
+			expect(chart.element.classList.contains("bb")).to.be.true;
 		});
 
-		expect(d3.select(chart.element).classed(bindtoClassName)).to.be.true;
+		it("instantiate with different classname on wrapper element", () => {
+			const bindtoClassName = "billboard-js";
+			chart = bb.generate({
+				bindto: {
+					element: "#chart",
+					classname: bindtoClassName
+				},
+				data: {
+					columns: [
+						["data1", 30, 200, 100, 400],
+						["data2", 500, 800, 500, 2000]
+					]
+				}
+			});
+
+			expect(d3.select(chart.element).classed(bindtoClassName)).to.be.true;
+		});
 	});
 
 	describe("auto resize", () => {
 		let container;
-		let innerHTML = "";
 
 		beforeEach(() => {
 			container = document.getElementById("container");

diff --git a/src/internals/Chart.js b/src/internals/Chart.js
@@ -20,9 +20,8 @@ import ChartInternal from "./ChartInternal";
  * @see {@link bb.generate} for the initialization.
 */
 /**
- * Access primary node elements
- * @name Chart#$
- * @type {Object}
+ * Access instance's primary node elements
+ * @member {Object} $
  * @property {Object} $
  * @property {d3.selection} $.chart Wrapper element
  * @property {d3.selection} $.svg Main svg element
@@ -41,7 +40,6 @@ import ChartInternal from "./ChartInternal";
  * @property {d3.selection} $.line.circles Data point circle elements
  * @property {Object} $.text
  * @property {d3.selection} $.text.texts Data label text elements
- * @instance
  * @memberof Chart
  * @example
  * var chart = bb.generate({ ... });
@@ -55,11 +53,10 @@ export default class Chart {
 
 		/**
 		 * Plugin instance array
-		 * @name Chart#plugins
-		 * @type {Array}
-		 * @instance
+		 * @member {Array} plugins
 		 * @memberof Chart
-		 * @example
+		 * @instance
+ 	 	 * @example
 		 *  var chart = bb.generate({
 		 *     ...
 		 *     plugins: [
@@ -71,15 +68,11 @@ export default class Chart {
 		 *  chart.plugins; // [Stanford, PluginA] - instance array
 		 */
 		this.plugins = [];
-
 		this.internal = $$;
 
 		$$.loadConfig(config);
 		$$.beforeInit(config);
 		$$.init();
-
-		this.$ = $$.getChartElements();
-
 		$$.afterInit(config);
 
 		// bind "this" to nested API

diff --git a/src/internals/ChartInternal.js b/src/internals/ChartInternal.js
@@ -348,10 +348,10 @@ export default class ChartInternal {
 		notEmpty($$.config.data_labels) && $$.initText();
 	}
 
-	getChartElements() {
+	setChartElements() {
 		const $$ = this;
 
-		return {
+		$$.api.$ = {
 			chart: $$.selectChart,
 			svg: $$.svg,
 			defs: $$.defs,
@@ -652,6 +652,8 @@ export default class ChartInternal {
 			$$.bindZoomEvent();
 		}
 
+		initializing && $$.setChartElements();
+
 		$$.generateRedrawList(targetsToShow, flow, duration, wth.Subchart);
 		$$.callPluginHook("$redraw", options, duration);
 	}