Skip to content

Commit b7f604c

Browse files
authored
Merge pull request #3744 from ag-grid/ag-14117/fix-tooltip-size-forcing-relayout
AG-14117 - Fix tooltip forced relayout + misc optimisations.
2 parents 7cc3276 + 7a2368e commit b7f604c

File tree

5 files changed

+52
-10
lines changed

5 files changed

+52
-10
lines changed

packages/ag-charts-community/src/chart/data/dataModel.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,9 @@ export class DataModel<
889889
const columnScopes = new Set(def.scopes);
890890
const columnScope = first(def.scopes);
891891
const columnSource = sources.get(columnScope) as unknown[];
892-
const column = columnSource.map((valueDatum, datumIndex) => {
892+
const column = new Array<unknown>();
893+
for (let datumIndex = 0; datumIndex < columnSource.length; datumIndex++) {
894+
const valueDatum = columnSource[datumIndex];
893895
const invalidKey = invalidKeys.get(columnScope)?.[datumIndex];
894896

895897
const result = processValue(def, valueDatum, datumIndex, def.scopes);
@@ -907,8 +909,8 @@ export class DataModel<
907909
value = invalidValue;
908910
}
909911

910-
return value;
911-
});
912+
column[datumIndex] = value;
913+
}
912914

913915
columns.push(column);
914916
allColumnScopes.push(columnScopes);

packages/ag-charts-community/src/chart/tooltip/tooltip.ts

+12-4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { DOMManager } from '../../dom/domManager';
55
import type { LocaleManager } from '../../locale/localeManager';
66
import { type Bounds, type Placement, calculatePlacement } from '../../util/placement';
77
import { BaseProperties } from '../../util/properties';
8+
import { SizeMonitor } from '../../util/sizeMonitor';
89
import {
910
ARRAY_OF,
1011
BOOLEAN,
@@ -372,7 +373,6 @@ export class Tooltip extends BaseProperties {
372373
}
373374

374375
element.innerHTML = html;
375-
this._elementSize = { width: element.clientWidth, height: element.clientHeight };
376376
} else if (element == null || element.innerHTML === '') {
377377
this.toggle(false);
378378
return;
@@ -454,9 +454,17 @@ export class Tooltip extends BaseProperties {
454454
}
455455

456456
if (visible) {
457-
// We can only measure the element when it's actually visible
458-
// This removes a possible jump for the tooltip
459-
this.updateTooltipPosition();
457+
// Avoid reading the tooltip size immediately after a DOM mutation, wait for
458+
// a natural layout before positioning the tooltip.
459+
SizeMonitor.singleShot(this.element, (size) => {
460+
if (!this._visible) return;
461+
462+
this._elementSize = size;
463+
464+
// We can only measure the element when it's actually visible
465+
// This removes a possible jump for the tooltip
466+
this.updateTooltipPosition();
467+
});
460468
}
461469
}
462470

packages/ag-charts-community/src/scene/shape/text.ts

+10-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,16 @@ export class Text<D = any> extends Shape<D> {
9494
const { fill, stroke, strokeWidth } = this;
9595
const { pixelRatio } = this.layerManager.canvas;
9696

97-
ctx.font = TextUtils.toFontString(this);
97+
if (!fill && !(stroke != null && strokeWidth > 0)) {
98+
// Short circuit early if nothing will be rendered.
99+
return super.render(renderCtx);
100+
}
101+
102+
const font = TextUtils.toFontString(this);
103+
// Try to avoid this assignment, which typically always incurs a font switch cost.
104+
if (ctx.font !== font) {
105+
ctx.font = font;
106+
}
98107
ctx.textAlign = this.textAlign;
99108
ctx.textBaseline = this.textBaseline;
100109

packages/ag-charts-community/src/util/sizeMonitor.ts

+23
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,29 @@ export class SizeMonitor {
2020
private documentReady = false;
2121
private queuedObserveRequests: [HTMLElement, OnSizeChange][] = [];
2222

23+
static singleShot(element: HTMLElement, cb: OnSizeChange) {
24+
if (typeof ResizeObserver === 'undefined') {
25+
// Fallback to using clientWidth + clientHeight, which will force a layout.
26+
cb({ width: element.clientWidth, height: element.clientHeight, pixelRatio: 1 }, element);
27+
return;
28+
}
29+
30+
const observer = new ResizeObserver((entries) => {
31+
try {
32+
for (const {
33+
contentRect: { width, height },
34+
target,
35+
} of entries) {
36+
// Round to mimic Element.clientWidth + Element.clientHeight.
37+
cb({ width: Math.round(width), height: Math.round(height), pixelRatio: 1 }, target as HTMLElement);
38+
}
39+
} finally {
40+
observer.disconnect();
41+
}
42+
});
43+
observer.observe(element);
44+
}
45+
2346
constructor() {
2447
if (typeof ResizeObserver !== 'undefined') {
2548
this.resizeObserver = new ResizeObserver((entries) => {

packages/ag-charts-enterprise/src/features/error-bar/__snapshots__/errorBar.test.ts.snap

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ exports[`ErrorBars AG-10525 should render tooltips with no errorbars 1`] = `
44
NodeList [
55
<div
66
aria-hidden="true"
7-
class="ag-charts-tooltip ag-charts-tooltip--no-interaction ag-charts-tooltip--arrow-bottom ag-charts-tooltip--wrap-hyphenate"
7+
class="ag-charts-tooltip ag-charts-tooltip--no-interaction ag-charts-tooltip--wrap-hyphenate ag-charts-tooltip--arrow-bottom"
88
data-presented-as-popover=""
99
popover="manual"
1010
style="pointer-events: none; --top: 0px; --left: 0px;"
@@ -36,7 +36,7 @@ exports[`ErrorBars should render default tooltips 1`] = `
3636
NodeList [
3737
<div
3838
aria-hidden="true"
39-
class="ag-charts-tooltip ag-charts-tooltip--no-interaction ag-charts-tooltip--arrow-bottom ag-charts-tooltip--wrap-hyphenate"
39+
class="ag-charts-tooltip ag-charts-tooltip--no-interaction ag-charts-tooltip--wrap-hyphenate ag-charts-tooltip--arrow-bottom"
4040
data-presented-as-popover=""
4141
popover="manual"
4242
style="pointer-events: none; --top: 0px; --left: 0px;"

0 commit comments

Comments
 (0)