Skip to content

Commit 4bd0212

Browse files
committed
Fix tooltip coordinates when scrolled down
The tooltip uses `position: fixed` which interprets `left/top` as viewport-relative, but `ChartCanvas` was storing `event.pageX/pageY` (document-relative, including scroll offsets). In a non-scrolling page these match, but inside a scrollable container like our benchmark page they diverge by `window.scrollY`, dragging the tooltip down. Switched the state field from `pageX/pageY` to `clientX/clientY`.
1 parent f2083ff commit 4bd0212

1 file changed

Lines changed: 26 additions & 23 deletions

File tree

src/components/shared/chart/Canvas.tsx

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,16 @@ type Props<Item> = {
4343
} | null;
4444
};
4545

46-
// The naming of the X and Y coordinates here correspond to the ones
47-
// found on the MouseEvent interface.
46+
// Mouse coordinates passed to the Tooltip component. The Tooltip uses
47+
// `position: fixed`, so these must be VIEWPORT-relative (i.e. clientX/clientY,
48+
// not pageX/pageY). Otherwise the tooltip is mispositioned whenever the page
49+
// is scrolled (the chart's container can be in a scrollable region — e.g. the
50+
// benchmark-comparison page — even though the chart itself doesn't scroll).
4851
type State<Item> = {
4952
hoveredItem: Item | null;
5053
selectedItem: Item | null;
51-
pageX: CssPixels;
52-
pageY: CssPixels;
54+
clientX: CssPixels;
55+
clientY: CssPixels;
5356
};
5457

5558
export type ChartCanvasScale = {
@@ -103,8 +106,8 @@ export class ChartCanvas<Item> extends React.Component<
103106
override state: State<Item> = {
104107
hoveredItem: null,
105108
selectedItem: null,
106-
pageX: 0,
107-
pageY: 0,
109+
clientX: 0,
110+
clientY: 0,
108111
};
109112

110113
_scheduleDraw(
@@ -238,8 +241,8 @@ export class ChartCanvas<Item> extends React.Component<
238241
if (this.props.stickyTooltips) {
239242
this.setState((state) => ({
240243
selectedItem: state.hoveredItem,
241-
pageX: e.pageX,
242-
pageY: e.pageY,
244+
clientX: e.clientX,
245+
clientY: e.clientY,
243246
}));
244247
}
245248

@@ -292,20 +295,18 @@ export class ChartCanvas<Item> extends React.Component<
292295
const maybeHoveredItem = this.props.hitTest(offsetX, offsetY);
293296
if (maybeHoveredItem !== null) {
294297
if (this.state.selectedItem === null) {
295-
// Update both the hovered item and the pageX and pageY values. The
296-
// pageX and pageY values are used to change the position of the tooltip
297-
// and if there is no selected item, it means that we can update this
298-
// position freely.
298+
// Update both the hovered item and the cursor position. The cursor
299+
// position is used to position the tooltip; if there is no selected
300+
// item we can update it freely.
299301
this.setState({
300302
hoveredItem: maybeHoveredItem,
301-
pageX: event.pageX,
302-
pageY: event.pageY,
303+
clientX: event.clientX,
304+
clientY: event.clientY,
303305
});
304306
} else {
305307
// If there is a selected item, only update the hoveredItem and not the
306-
// pageX and pageY values which is used for the position of the tooltip.
307-
// By keeping the x and y values the same, we make sure that the tooltip
308-
// stays in its initial position where it's clicked.
308+
// cursor position used to position the tooltip. By keeping the x and y
309+
// values the same, the tooltip stays at the position where it was clicked.
309310
this.setState({
310311
hoveredItem: maybeHoveredItem,
311312
});
@@ -385,9 +386,11 @@ export class ChartCanvas<Item> extends React.Component<
385386

386387
const { offsetX, offsetY } = selectedItemTooltipOffset;
387388
const canvasRect = this._canvas.getBoundingClientRect();
388-
const pageX = canvasRect.left + window.scrollX + offsetX;
389-
const pageY = canvasRect.top + window.scrollY + offsetY;
390-
this.setState({ selectedItem, pageX, pageY });
389+
// Viewport-relative coordinates (no scroll offsets), since the Tooltip is
390+
// positioned with `position: fixed`. See the State type comment above.
391+
const clientX = canvasRect.left + offsetX;
392+
const clientY = canvasRect.top + offsetY;
393+
this.setState({ selectedItem, clientX, clientY });
391394
};
392395

393396
override UNSAFE_componentWillReceiveProps() {
@@ -495,7 +498,7 @@ export class ChartCanvas<Item> extends React.Component<
495498

496499
override render() {
497500
const { isDragging } = this.props;
498-
const { hoveredItem, pageX, pageY } = this.state;
501+
const { hoveredItem, clientX, clientY } = this.state;
499502

500503
const className = classNames({
501504
chartCanvas: true,
@@ -519,8 +522,8 @@ export class ChartCanvas<Item> extends React.Component<
519522
/>
520523
{!isDragging && tooltipContents ? (
521524
<Tooltip
522-
mouseX={pageX}
523-
mouseY={pageY}
525+
mouseX={clientX}
526+
mouseY={clientY}
524527
className={classNames({
525528
clickable: this.state.selectedItem !== null,
526529
})}

0 commit comments

Comments
 (0)