Skip to content

Commit

Permalink
fix(tooltip): Fix wrong tooltip selection on browser scroll change (n…
Browse files Browse the repository at this point in the history
…aver#3593)

Reflect current scroll position(body and container scroll also)
for correct data selection on tooltip show

Fix naver#3592
  • Loading branch information
netil authored Jan 15, 2024
1 parent d5b5d09 commit 8f80637
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 9 deletions.
8 changes: 6 additions & 2 deletions src/ChartInternal/data/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {IData, IDataPoint, IDataRow} from "./IData";
import {
findIndex,
getUnique,
getScrollPosition,
hasValue,
isArray,
isboolean,
Expand Down Expand Up @@ -671,7 +672,7 @@ export default {
*/
getDataIndexFromEvent(event): number {
const $$ = this;
const {config, state: {hasRadar, inputType, eventReceiver: {coords, rect}}} = $$;
const {$el, config, state: {hasRadar, inputType, eventReceiver: {coords, rect}}} = $$;
let index;

if (hasRadar) {
Expand All @@ -687,13 +688,16 @@ export default {
index = d && Object.keys(d).length === 1 ? d.index : undefined;
} else {
const isRotated = config.axis_rotated;
const scrollPos = getScrollPosition($el.chart.node());

// get data based on the mouse coords
const e = inputType === "touch" && event.changedTouches ? event.changedTouches[0] : event;

index = findIndex(
coords,
isRotated ? e.clientY - rect.top : e.clientX - rect.left,
isRotated ?
e.clientY + scrollPos.y - rect.top :
e.clientX + scrollPos.x - rect.left,
0,
coords.length - 1,
isRotated
Expand Down
15 changes: 11 additions & 4 deletions src/ChartInternal/interactions/eventrect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* billboard.js project is licensed under the MIT license
*/
import {$COMMON, $EVENT, $SHAPE} from "../../config/classes";
import {isboolean, getPointer, isFunction} from "../../module/util";
import {isboolean, getPointer, getScrollPosition, isFunction} from "../../module/util";

export default {
/**
Expand Down Expand Up @@ -194,9 +194,16 @@ export default {
const rectElement = eventRect || $el.eventRect;

const updateClientRect = (): void => {
eventReceiver && (
eventReceiver.rect = rectElement.node().getBoundingClientRect()
);
if (eventReceiver) {
const scrollPos = getScrollPosition($el.chart.node());

eventReceiver.rect = rectElement.node()
.getBoundingClientRect()
.toJSON();

eventReceiver.rect.top += scrollPos.y;
eventReceiver.rect.left += scrollPos.x;
}
};

if (!rendered || resizing || force) {
Expand Down
2 changes: 1 addition & 1 deletion src/ChartInternal/internals/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export default {
*/
isTypeOf(d, type): boolean {
const id = isString(d) ? d : d.id;
const dataType = this.config?.data_types?.[id] || this.config.data_type;
const dataType = this.config && (this.config.data_types?.[id] || this.config.data_type);

return isArray(type) ?
type.indexOf(dataType) >= 0 : dataType === type;
Expand Down
14 changes: 14 additions & 0 deletions src/module/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export {
getRandom,
getRange,
getRectSegList,
getScrollPosition,
getTranslation,
getUnique,
hasValue,
Expand Down Expand Up @@ -511,6 +512,19 @@ function getCssRules(styleSheets: any[]) {
return rules;
}

/**
* Get current window and container scroll position
* @param {HTMLElement} node Target element
* @returns {object} window scroll position
* @private
*/
function getScrollPosition(node: HTMLElement) {
return {
x: (window.pageXOffset ?? window.scrollX ?? 0) + node.scrollLeft ?? 0,
y: (window.pageYOffset ?? window.scrollY ?? 0) + node.scrollTop ?? 0
};
}

/**
* Gets the SVGMatrix of an SVGGElement
* @param {SVGElement} node Node element
Expand Down
1 change: 1 addition & 0 deletions test/assets/module/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const {
getRandom,
getRange,
getRectSegList,
getScrollPosition,
getTranslation,
getUnique,
hasValue,
Expand Down
106 changes: 105 additions & 1 deletion test/internals/tooltip-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,110 @@ describe("TOOLTIP", function() {
});
});

describe("when document or container element is scrolled", () => {
before(() => {
args = {
size: {
width: 640,
height: 480
},
data: {
columns: [
["data1", 30, 200, 100, 400, 150, 250],
["data2", 10, 190, 95, 40, 15, 25]
]
},
axis: {
rotated: false
}
};
});

it("tooltip correctly showed?", function(done) {
chart.$.chart.attr("style", "position: relative; top: 0px; left: 0px; width:300px;height:400px;overflow:scroll;");

const top = chart.$.chart.node().getBoundingClientRect().top;
const {tooltip} = chart.$;
const index = [];

new Promise(resolve => {
util.hoverChart(chart, "mousemove", {
clientX: 50,
clientY: top + 50
});

index.push(+tooltip.select("th").text());

setTimeout(() => {
chart.$.chart.node().scrollTo(500, 0);
resolve();
}, 300);
}).then(() => {
new Promise(resolve => {
util.hoverChart(chart, "mousemove", {
clientX: 200,
clientY: top + 50
});

setTimeout(resolve, 300);
});
}).then(() => {
index.push(+tooltip.select("th").text());

expect(index).to.be.deep.equal([0, 5]);

chart.$.chart.node().scrollTo(0, 0);
done();
});
});

it("set option: axis.rotated=true", () => {
args.size = {
width: 480,
height: 640
};

args.axis.rotated = true;
});

it("tooltip correctly showed on rotated axis?", function(done) {
chart.$.chart.attr("style", "position: relative; top: 0px; left: 0px; width:480px;height:640px;overflow:scroll;");
const {tooltip} = chart.$;
const index = [];

new Promise(resolve => {
util.hoverChart(chart, "mousemove", {
clientX: 100,
clientY: 50
});

index.push(+tooltip.select("th").text());

setTimeout(() => {
chart.$.chart.node().scrollTo(0, 500);
resolve();
}, 300);
}).then(() => {
new Promise(resolve => {
util.hoverChart(chart, "mousemove", {
clientX: 100,
clientY: 550
});

setTimeout(resolve, 300);
});
}).then(() => {
index.push(+tooltip.select("th").text());

expect(index).to.be.deep.equal([0, 5]);

chart.$.chart.attr("style", "position: relative; top: 0px; left: 0px; width:640px;height:480px;");

done();
});
});
});

describe("check interference", () => {
before(() => {
args = {
Expand Down Expand Up @@ -978,7 +1082,7 @@ describe("TOOLTIP", function() {

// check for tooltip position to not overflow the chart
expect(tooltipLeft).to.be.lessThanOrEqual(state.width);
});
});
});

describe("Narrow width container's tooltip position", () => {
Expand Down
2 changes: 1 addition & 1 deletion test/internals/util-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ describe("UTIL", function() {

describe("getPathBox", () => {
it("should return element's path box value", () => {
const pathBox = getPathBox(document.body.querySelector("svg"));
const pathBox = getPathBox(document.body.querySelector("svg") as SVGGraphicsElement);

for (let x in pathBox) {
expect(isNumber(pathBox[x])).to.be.true;
Expand Down

0 comments on commit 8f80637

Please sign in to comment.