From 8f80637b7c83c333e45017ac3153aab3055be9fa Mon Sep 17 00:00:00 2001 From: Jae Sung Park Date: Mon, 15 Jan 2024 16:59:11 +0900 Subject: [PATCH] fix(tooltip): Fix wrong tooltip selection on browser scroll change (#3593) Reflect current scroll position(body and container scroll also) for correct data selection on tooltip show Fix #3592 --- src/ChartInternal/data/data.ts | 8 +- src/ChartInternal/interactions/eventrect.ts | 15 ++- src/ChartInternal/internals/type.ts | 2 +- src/module/util.ts | 14 +++ test/assets/module/util.ts | 1 + test/internals/tooltip-spec.ts | 106 +++++++++++++++++++- test/internals/util-spec.ts | 2 +- 7 files changed, 139 insertions(+), 9 deletions(-) diff --git a/src/ChartInternal/data/data.ts b/src/ChartInternal/data/data.ts index 960ba900c..3d832a8d1 100644 --- a/src/ChartInternal/data/data.ts +++ b/src/ChartInternal/data/data.ts @@ -9,6 +9,7 @@ import type {IData, IDataPoint, IDataRow} from "./IData"; import { findIndex, getUnique, + getScrollPosition, hasValue, isArray, isboolean, @@ -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) { @@ -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 diff --git a/src/ChartInternal/interactions/eventrect.ts b/src/ChartInternal/interactions/eventrect.ts index d1cfd24d5..53361ff8d 100644 --- a/src/ChartInternal/interactions/eventrect.ts +++ b/src/ChartInternal/interactions/eventrect.ts @@ -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 { /** @@ -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) { diff --git a/src/ChartInternal/internals/type.ts b/src/ChartInternal/internals/type.ts index 84911d133..100fbc3d4 100644 --- a/src/ChartInternal/internals/type.ts +++ b/src/ChartInternal/internals/type.ts @@ -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; diff --git a/src/module/util.ts b/src/module/util.ts index 95a8f93c8..2c4c2b21c 100644 --- a/src/module/util.ts +++ b/src/module/util.ts @@ -33,6 +33,7 @@ export { getRandom, getRange, getRectSegList, + getScrollPosition, getTranslation, getUnique, hasValue, @@ -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 diff --git a/test/assets/module/util.ts b/test/assets/module/util.ts index 451348086..0aa83a24d 100644 --- a/test/assets/module/util.ts +++ b/test/assets/module/util.ts @@ -33,6 +33,7 @@ export const { getRandom, getRange, getRectSegList, + getScrollPosition, getTranslation, getUnique, hasValue, diff --git a/test/internals/tooltip-spec.ts b/test/internals/tooltip-spec.ts index 607dc525c..0aafc37ed 100644 --- a/test/internals/tooltip-spec.ts +++ b/test/internals/tooltip-spec.ts @@ -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 = { @@ -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", () => { diff --git a/test/internals/util-spec.ts b/test/internals/util-spec.ts index db5d237f7..db90f0449 100644 --- a/test/internals/util-spec.ts +++ b/test/internals/util-spec.ts @@ -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;