From c24bddb28282fa3f6b7e9717b2287f743bf8343c Mon Sep 17 00:00:00 2001 From: Jae Sung Park Date: Mon, 8 Jul 2019 14:06:00 +0900 Subject: [PATCH] fix(tooltip): Correct tooltip on dynamic loading Make to update eventRect elements Fix #963 --- spec/internals/tooltip-spec.js | 121 +++++++++++++++++++++++++++++--- src/axis/Axis.js | 3 +- src/data/data.convert.js | 7 +- src/data/data.js | 39 +++++++--- src/interactions/interaction.js | 14 +++- src/internals/ChartInternal.js | 3 + src/internals/tooltip.js | 7 +- src/shape/line.js | 2 +- 8 files changed, 169 insertions(+), 27 deletions(-) diff --git a/spec/internals/tooltip-spec.js b/spec/internals/tooltip-spec.js index 3935a7242..1abf97504 100644 --- a/spec/internals/tooltip-spec.js +++ b/spec/internals/tooltip-spec.js @@ -691,14 +691,10 @@ describe("TOOLTIP", function() { it("check when first data is null", () => { chart.tooltip.show({x:1}); - expect(chart.$.tooltip.html()).to.be.equal( - ` - - - - -
1
data220
`.replace(/[\r\n\t]/g, "") - ); + const tooltip = chart.$.tooltip; + + expect(tooltip.select(".name").node().textContent).to.be.equal("data2"); + expect(+tooltip.select(".value").node().textContent).to.be.equal(20); }); }); @@ -715,7 +711,6 @@ describe("TOOLTIP", function() { }); it("load data to be adding more columns", done => { - setTimeout(() => { chart.load({ columns: [ ["data2", 44, 134, 98, 170] @@ -723,6 +718,7 @@ describe("TOOLTIP", function() { done: () => { try { chart.tooltip.show({index: 3}); + expect(+chart.$.tooltip.select(".value").html()).to.be.equal(170); } catch(e) { expect(false).to.be.true; } @@ -731,7 +727,112 @@ describe("TOOLTIP", function() { done(); } }); - }, 500); + }); + + it("set options", () => { + args = { + data: { + x: "x", + columns: [ + ["x", "2013-01-01", "2013-01-02", "2013-01-03", "2013-01-04", "2013-01-05"], + ["data1", 30, 200, 100, 400, 150] + ] + }, + axis: { + x: { + type: "timeseries", + tick: { + format: "%Y-%m-%d" + } + } + } + }; + }); + + it("should correctly showing tooltip for loaded data", done => { + chart.load({ + columns: [ + ["x", "2013-01-02", "2013-01-03", "2013-01-04", "2013-01-05"], + ["data2", 220, 150, 40, 250] + ], + done: () => { + const index = 1; + const expected = [200, 220]; + + chart.tooltip.show({index}); + + const tooltipValue = chart.$.tooltip.selectAll(".value").nodes(); + + chart.data().forEach((v, i) => { + expect(+tooltipValue[i].textContent).to.be.equal(expected[i]); + }); + + done(); + } + }); + }); + + it("set options", () => { + args = { + data: { + x:"x", + columns: [ + ["x", 10, 30, 40, 60, 80], + ["data1", 230, 190, 50, 10, 60] + ] + }, + }; + }); + + it("should correclty show tooltip for new added x Axis ticks", done => { + chart.load({ + columns: [ + // when load different data name than the generated, it will add new axis ticks + ["x", 35, 60, 85], + ["data2", 10, 20, 160] + ], + done: () => { + const value = []; + + [1, 2, 5, 6].forEach(v => { + chart.tooltip.show({index: v}); + value.push(chart.$.tooltip.select(".value").html()); + }); + + [190, 10, 60, 160].forEach((v, i) => { + expect(+value[i]).to.be.equal(v); + }); + + // when + chart.toggle("data1"); + + setTimeout(() => { + chart.tooltip.show({index: 2}); + expect(+chart.$.tooltip.select(".value").html()).to.be.equal(160); + + done(); + }, 500); + } + }); + }); + + it("should correclty show tooltip for overriden x Axis ticks", done => { + chart.load({ + columns: [ + // when load same data name than the generated, it will add override axis ticks + ["x", 35, 60, 85], + ["data1", 10, 20, 160] + ], + done: () => { + chart.data()[0].values.forEach((v, i) => { + chart.tooltip.show({index: i}); + + expect(+chart.$.tooltip.select(".value").html()).to.be.equal(v.value); + }); + + done(); + } + }); }); }); diff --git a/src/axis/Axis.js b/src/axis/Axis.js index ef88e0cd2..4b2c08cea 100644 --- a/src/axis/Axis.js +++ b/src/axis/Axis.js @@ -304,7 +304,8 @@ export default class Axis { const tickValues = $$.config[`axis_${id}_tick_values`]; const axis = $$[`${id}Axis`]; - return tickValues || (axis ? axis.tickValues() : undefined); + return (isFunction(tickValues) ? tickValues() : tickValues) || + (axis ? axis.tickValues() : undefined); } getXAxisTickValues() { diff --git a/src/data/data.convert.js b/src/data/data.convert.js index 7b6eeb55a..dafdda3b1 100644 --- a/src/data/data.convert.js +++ b/src/data/data.convert.js @@ -224,16 +224,19 @@ extend(ChartInternal.prototype, { convertDataToTargets(data, appendXs) { const $$ = this; const config = $$.config; + const isTimeSeries = $$.isTimeSeries(); + const dataKeys = Object.keys(data[0] || {}); const ids = dataKeys.length ? dataKeys.filter($$.isNotX, $$) : []; const xs = dataKeys.length ? dataKeys.filter($$.isX, $$) : []; + let xsData; // save x for update data by load when custom x and bb.x API ids.forEach(id => { const xKey = this.getXKey(id); - if (this.isCustomX() || this.isTimeSeries()) { + if (this.isCustomX() || isTimeSeries) { // if included in input data if (xs.indexOf(xKey) >= 0) { xsData = ((appendXs && $$.data.xs[id]) || []) @@ -259,7 +262,7 @@ extend(ChartInternal.prototype, { // check x is defined ids.forEach(id => { - if (!xsData) { + if (!this.data.xs[id]) { throw new Error(`x is not defined for id = "${id}".`); } }); diff --git a/src/data/data.js b/src/data/data.js index a1155b471..bf3bcdc72 100644 --- a/src/data/data.js +++ b/src/data/data.js @@ -193,15 +193,8 @@ extend(ChartInternal.prototype, { updateXs() { const $$ = this; - const targets = $$.data.targets; - if (targets.length) { - $$.xs = []; - - targets[0].values.forEach(v => { - $$.xs[v.index] = v.x; - }); - } + $$.xs = $$.axis.getXAxisTickValues() || []; }, getPrevX(i) { @@ -833,5 +826,35 @@ extend(ChartInternal.prototype, { } return asPercent && ratio ? ratio * 100 : ratio; + }, + + /** + * Sort data index to be aligned with x axis. + */ + updateDataIndexByX() { + const $$ = this; + const isTimeSeries = $$.isTimeSeries(); + const tickValues = $$.axis.getXAxisTickValues() || []; + + $$.data.targets.forEach(t => { + t.values.forEach((v, i) => { + if (isTimeSeries) { + tickValues.some((d, j) => { + if (+d === +v.x) { + v.index = j; + return true; + } + + return false; + }); + } else { + v.index = tickValues.indexOf(v.x); + } + + if (!isNumber(v.index) || v.index === -1) { + v.index = i; + } + }); + }); } }); diff --git a/src/interactions/interaction.js b/src/interactions/interaction.js index 2969e588f..d6c12f904 100644 --- a/src/interactions/interaction.js +++ b/src/interactions/interaction.js @@ -60,10 +60,18 @@ extend(ChartInternal.prototype, { eventRectUpdate = $$.generateEventRectsForMultipleXs(eventRectUpdate.enter()) .merge(eventRectUpdate); } else { + let xAxisTickValues = $$.axis.getXAxisTickValues() || $$.getMaxDataCountTarget($$.data.targets); + + if (isObject(xAxisTickValues)) { + xAxisTickValues = xAxisTickValues.values; + } + // Set data and update $$.eventRect - const maxDataCountTarget = $$.getMaxDataCountTarget($$.data.targets); + const xAxisTarget = (xAxisTickValues || []) + .map((x, index) => ({x, index})); + + eventRects.datum(xAxisTarget); - eventRects.datum(maxDataCountTarget ? maxDataCountTarget.values : []); $$.eventRect = eventRects.selectAll(`.${CLASS.eventRect}`); eventRectUpdate = $$.eventRect.data(d => d); @@ -239,7 +247,7 @@ extend(ChartInternal.prototype, { rectX = d => { const x = getPrevNextX(d); - const thisX = $$.data.xs[d.id][d.index]; + const thisX = d.x; // if there this is a single data point position the eventRect at 0 if (x.prev === null && x.next === null) { diff --git a/src/internals/ChartInternal.js b/src/internals/ChartInternal.js index b864c3c92..1b5f4ac7b 100644 --- a/src/internals/ChartInternal.js +++ b/src/internals/ChartInternal.js @@ -598,6 +598,9 @@ export default class ChartInternal { // @TODO: Make 'init' state to be accessible everywhere not passing as argument. $$.redrawAxis(targetsToShow, wth, transitions, flow, initializing); + // update data's index value to be alinged with the x Axis + $$.updateDataIndexByX(); + // update circleY based on updated parameters $$.updateCircleY(); diff --git a/src/internals/tooltip.js b/src/internals/tooltip.js index 20408903a..e8f888ac9 100644 --- a/src/internals/tooltip.js +++ b/src/internals/tooltip.js @@ -132,14 +132,17 @@ extend(ChartInternal.prototype, { } const tpl = $$.getTooltipContentTemplate(tplStr); + const len = d.length; let text; let row; let param; let value; let i; - for (i = 0; (row = d[i]); i++) { - if (!(getRowValue(row) || getRowValue(row) === 0)) { + for (i = 0; i < len; i++) { + row = d[i]; + + if (!row || !(getRowValue(row) || getRowValue(row) === 0)) { continue; } diff --git a/src/shape/line.js b/src/shape/line.js index c4514d63d..ab84b363c 100644 --- a/src/shape/line.js +++ b/src/shape/line.js @@ -560,7 +560,7 @@ extend(ChartInternal.prototype, { const result = fn(d); mainCircles.push(result); - }); + }).attr("class", $$.classCircle.bind($$)); const posAttr = $$.isCirclePoint() ? "c" : "";