Skip to content

Commit

Permalink
fix(tooltip): Correct tooltip on dynamic loading
Browse files Browse the repository at this point in the history
Make to update eventRect elements

Fix #963
  • Loading branch information
netil committed Jul 8, 2019
1 parent f6f18b6 commit c24bddb
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 27 deletions.
121 changes: 111 additions & 10 deletions spec/internals/tooltip-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
`<table class="bb-tooltip"><tbody><tr><th colspan="2">1</th></tr>
<tr class="bb-tooltip-name-data2">
<td class="name"><span style="background-color:#fa7171"></span>data2</td>
<td class="value">20</td>
</tr>
</tbody></table>`.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);
});
});

Expand All @@ -715,14 +711,14 @@ describe("TOOLTIP", function() {
});

it("load data to be adding more columns", done => {
setTimeout(() => {
chart.load({
columns: [
["data2", 44, 134, 98, 170]
],
done: () => {
try {
chart.tooltip.show({index: 3});
expect(+chart.$.tooltip.select(".value").html()).to.be.equal(170);
} catch(e) {
expect(false).to.be.true;
}
Expand All @@ -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();
}
});
});
});

Expand Down
3 changes: 2 additions & 1 deletion src/axis/Axis.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
7 changes: 5 additions & 2 deletions src/data/data.convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]) || [])
Expand All @@ -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}".`);
}
});
Expand Down
39 changes: 31 additions & 8 deletions src/data/data.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
});
});
}
});
14 changes: 11 additions & 3 deletions src/interactions/interaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions src/internals/ChartInternal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
7 changes: 5 additions & 2 deletions src/internals/tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/shape/line.js
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ extend(ChartInternal.prototype, {
const result = fn(d);

mainCircles.push(result);
});
}).attr("class", $$.classCircle.bind($$));

const posAttr = $$.isCirclePoint() ? "c" : "";

Expand Down

0 comments on commit c24bddb

Please sign in to comment.