Compare commits

...

8 Commits

Author SHA1 Message Date
sadpandajoe
bd0a217d84 fix(calendar): harden tooltip lifecycle cleanup 2026-07-17 19:15:25 +00:00
sadpandajoe
9c6c8e231a test(calendar): cover tooltip lifecycle cleanup 2026-07-15 23:51:33 +00:00
Joe Li
9ffca6df48 Merge branch 'master' into fix-calendar-heatmap-tooltip-cleanup-41583 2026-07-15 15:59:28 -07:00
Joe Li
5c80d38e68 Merge branch 'master' into fix-calendar-heatmap-tooltip-cleanup-41583 2026-07-14 11:50:39 -07:00
sadpandajoe
e827b0566b Merge master into fix-calendar-heatmap-tooltip-cleanup-41583 2026-07-13 22:11:49 +00:00
sadpandajoe
8d4dfba973 chore(calendar): format cal-heatmap tooltip classes 2026-07-07 14:11:06 +00:00
sadpandajoe
251d3f8895 fix(calendar): scope d3-tip cleanup per chart 2026-07-07 14:11:05 +00:00
sadpandajoe
059c01fc28 fix(calendar): clean up d3-tip tooltips 2026-07-07 14:11:05 +00:00
6 changed files with 618 additions and 3 deletions

View File

@@ -23,11 +23,23 @@ import { SupersetTheme } from '@apache-superset/core/theme';
import { t } from '@apache-superset/core/translation';
import CalHeatMapImport from './vendor/cal-heatmap';
import { convertUTCTimestampToLocal } from './utils';
import {
getCalendarTooltipClassName,
removeDisconnectedCalendarTooltips,
} from './tooltip';
// The vendor file is @ts-nocheck, so its export lacks type info.
// Define a minimal constructor interface for use in this file.
interface CalHeatMapInstance {
init(config: Record<string, unknown>): void;
destroy(): null;
}
const calendarInstances = new WeakMap<HTMLElement, CalHeatMapInstance[]>();
function destroyCalendarInstances(element: HTMLElement) {
calendarInstances.get(element)?.forEach(calendar => calendar.destroy());
calendarInstances.delete(element);
}
const CalHeatMap = CalHeatMapImport as unknown as new () => CalHeatMapInstance;
@@ -78,6 +90,12 @@ function Calendar(element: HTMLElement, props: CalendarProps) {
theme,
} = props;
destroyCalendarInstances(element);
removeDisconnectedCalendarTooltips();
const tooltipClassName = getCalendarTooltipClassName(element);
const instances: CalHeatMapInstance[] = [];
const container = d3Select(element)
.classed('superset-legacy-chart-calendar', true)
.style('height', height);
@@ -112,7 +130,7 @@ function Calendar(element: HTMLElement, props: CalendarProps) {
const colorScheme = getSequentialSchemeRegistry().get(linearColorScheme);
const colorScale = colorScheme
? colorScheme.createLinearScale(extents)
: (_v: number) => '#ccc'; // fallback if scheme not found
: () => '#ccc'; // fallback if scheme not found
const legend = d3Range(steps).map(i => extents[0] + step * i);
const legendColors = legend.map(x => colorScale(x));
@@ -130,6 +148,7 @@ function Calendar(element: HTMLElement, props: CalendarProps) {
legendCellPadding: 2,
legendCellRadius: cellRadius,
tooltip: true,
tooltipClassName,
domain: domainGranularity,
subDomain: subdomainGranularity,
range: data.range,
@@ -147,7 +166,12 @@ function Calendar(element: HTMLElement, props: CalendarProps) {
timeFormatter,
subDomainTextFormat,
});
instances.push(cal);
});
if (instances.length > 0) {
calendarInstances.set(element, instances);
}
}
Calendar.displayName = 'Calendar';

View File

@@ -20,6 +20,7 @@ import { reactify } from '@superset-ui/core';
import { styled, css, useTheme } from '@apache-superset/core/theme';
import { Global } from '@emotion/react';
import Component from './Calendar';
import { scheduleCalendarTooltipCleanup } from './tooltip';
// Type-erase the render function to allow flexible prop spreading in the wrapper.
// The Calendar render function has typed props, but the wrapper passes props via spread
@@ -29,6 +30,7 @@ const ReactComponent = reactify(
container: HTMLDivElement,
props: Record<string, unknown>,
) => void,
{ componentWillUnmount: scheduleCalendarTooltipCleanup },
);
interface CalendarWrapperProps {

View File

@@ -0,0 +1,58 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
export const CALENDAR_TOOLTIP_CLASS = 'superset-legacy-chart-calendar-tooltip';
const tooltipOwners = new Map<string, HTMLElement>();
const tooltipClassNames = new WeakMap<HTMLElement, string>();
let tooltipId = 0;
function removeTooltipsByClassName(className: string) {
document
.querySelectorAll(`.${className}`)
.forEach(tooltip => tooltip.remove());
}
export function getCalendarTooltipClassName(element: HTMLElement) {
const existingClassName = tooltipClassNames.get(element);
if (existingClassName) {
// A disconnect sweep may have removed this owner while the WeakMap entry survived.
tooltipOwners.set(existingClassName, element);
return existingClassName;
}
tooltipId += 1;
const className = `${CALENDAR_TOOLTIP_CLASS}-${tooltipId}`;
tooltipClassNames.set(element, className);
tooltipOwners.set(className, element);
return className;
}
export function removeDisconnectedCalendarTooltips() {
tooltipOwners.forEach((element, className) => {
if (!element.isConnected) {
removeTooltipsByClassName(className);
tooltipOwners.delete(className);
}
});
}
export function scheduleCalendarTooltipCleanup() {
window.setTimeout(removeDisconnectedCalendarTooltips, 0);
}

View File

@@ -12,6 +12,7 @@
import d3tip from 'd3-tip';
import { t } from '@apache-superset/core/translation';
import { getContrastingColor } from '@superset-ui/core';
import { CALENDAR_TOOLTIP_CLASS } from '../tooltip';
var d3 = typeof require === 'function' ? require('d3') : window.d3;
@@ -22,7 +23,7 @@ var CalHeatMap = function () {
var self = this;
self.tip = d3tip()
.attr('class', 'd3-tip')
.attr('class', `d3-tip ${CALENDAR_TOOLTIP_CLASS}`)
.direction('n')
.offset([-5, 0])
.html(
@@ -33,7 +34,7 @@ var CalHeatMap = function () {
`,
);
self.legendTip = d3tip()
.attr('class', 'd3-tip')
.attr('class', `d3-tip ${CALENDAR_TOOLTIP_CLASS}`)
.direction('n')
.offset([-5, 0])
.html(d => self.options.valueFormatter(d));
@@ -274,6 +275,8 @@ var CalHeatMap = function () {
tooltip: false,
tooltipClassName: null,
// ================================================
// EVENTS CALLBACK
// ================================================
@@ -780,6 +783,16 @@ var CalHeatMap = function () {
if (self.options.paintOnLoad) {
_initCalendar();
}
var tooltipClassName = [
'd3-tip',
CALENDAR_TOOLTIP_CLASS,
self.options.tooltipClassName,
]
.filter(Boolean)
.join(' ');
self.tip.attr('class', tooltipClassName);
self.legendTip.attr('class', tooltipClassName);
self.root.call(self.tip);
self.root.call(self.legendTip);
@@ -3444,6 +3457,9 @@ CalHeatMap.prototype = {
destroy: function (callback) {
'use strict';
this.tip.destroy();
this.legendTip.destroy();
this.root
.transition()
.duration(this.options.animationDuration)

View File

@@ -0,0 +1,342 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { memo } from 'react';
import { act, cleanup, render, screen } from 'spec/helpers/testing-library';
import ReactCalendar from '../src/ReactCalendar';
const CALENDAR_TOOLTIP_CLASS = 'superset-legacy-chart-calendar-tooltip';
interface MockCalHeatMapConfig {
itemSelector: Element;
tooltipClassName: string;
}
let mockNextInstanceId = 0;
let mockNextOwnerId = 0;
jest.mock('../src/vendor/cal-heatmap', () => ({
__esModule: true,
default: class MockCalHeatMap {
private tooltips: HTMLElement[] = [];
init(config: MockCalHeatMapConfig) {
const mockTooltipClass = 'superset-legacy-chart-calendar-tooltip';
mockNextInstanceId += 1;
const owner = config.itemSelector.closest(
'.superset-legacy-chart-calendar',
) as HTMLElement | null;
if (!owner) {
throw new Error('Expected tooltip owner calendar container');
}
if (!owner.dataset.tooltipOwner) {
mockNextOwnerId += 1;
owner.dataset.tooltipOwner = `calendar-owner-${mockNextOwnerId}`;
}
const { tooltipOwner } = owner.dataset;
if (!tooltipOwner) {
throw new Error('Expected owner-specific tooltip marker');
}
this.tooltips = [0, 1].map(index => {
const tooltip = global.document.createElement('div');
tooltip.className = [
'd3-tip',
mockTooltipClass,
config.tooltipClassName,
].join(' ');
tooltip.dataset.tooltipOwner = tooltipOwner;
tooltip.dataset.tooltipInstance = String(mockNextInstanceId);
tooltip.dataset.tooltipIndex = String(index);
tooltip.textContent = `${tooltipOwner}-tooltip-${mockNextInstanceId}-${index}`;
global.document.body.appendChild(tooltip);
return tooltip;
});
}
destroy() {
this.tooltips.forEach(tooltip => tooltip.remove());
this.tooltips = [];
return null;
}
},
}));
interface CalendarHarnessProps {
firstMetricName: string;
secondMetricName?: string;
showFirstCalendar?: boolean;
}
const CALENDAR_START = 1704067200000;
function createCalendarProps(metricName: string) {
return {
data: {
data: {
[metricName]: {
[String(CALENDAR_START)]: 1,
[String(CALENDAR_START + 86400000)]: 2,
},
},
domain: 'month',
range: 1,
start: CALENDAR_START,
subdomain: 'day',
},
height: 160,
domainGranularity: 'month',
linearColorScheme: 'schemeRdYlBu',
showLegend: false,
showMetricName: true,
showValues: false,
steps: 3,
subdomainGranularity: 'day',
timeFormatter: (value: number | string) => String(value),
valueFormatter: (value: number) => String(value),
verboseMap: {
[metricName]: metricName,
},
};
}
const StableSecondCalendar = memo(function StableSecondCalendar({
metricName,
}: {
metricName: string;
}) {
return (
<div data-test="calendar-second">
<ReactCalendar {...createCalendarProps(metricName)} />
</div>
);
});
function CalendarHarness({
firstMetricName,
secondMetricName = 'second-metric',
showFirstCalendar = true,
}: CalendarHarnessProps) {
return (
<>
{showFirstCalendar ? (
<div data-test="calendar-first">
<ReactCalendar {...createCalendarProps(firstMetricName)} />
</div>
) : null}
<StableSecondCalendar metricName={secondMetricName} />
</>
);
}
function getCalendarOwner(testId: string) {
const owner = screen
.getByTestId(testId)
.querySelector('.superset-legacy-chart-calendar');
if (!(owner instanceof HTMLElement)) {
throw new Error(`Expected mounted calendar owner for ${testId}`);
}
return owner;
}
function getTooltipOwnerId(owner: HTMLElement) {
const tooltipOwnerId = owner.dataset.tooltipOwner;
if (!tooltipOwnerId) {
throw new Error('Expected mounted calendar to have a tooltip owner id');
}
return tooltipOwnerId;
}
function getOwnerTooltips(tooltipOwnerId: string) {
return Array.from(
document.querySelectorAll<HTMLElement>(
`.d3-tip[data-tooltip-owner="${tooltipOwnerId}"]`,
),
);
}
function getSingleTooltipInstanceId(tooltips: HTMLElement[]) {
const tooltipInstanceIds = new Set(
tooltips.map(tooltip => tooltip.dataset.tooltipInstance),
);
expect(tooltipInstanceIds.size).toBe(1);
const [tooltipInstanceId] = Array.from(tooltipInstanceIds);
if (!tooltipInstanceId) {
throw new Error('Expected tooltip instance id');
}
return tooltipInstanceId;
}
afterEach(() => {
cleanup();
act(() => {
jest.runOnlyPendingTimers();
});
jest.useRealTimers();
mockNextInstanceId = 0;
mockNextOwnerId = 0;
document.body.innerHTML = '';
});
test('rerender and unmount clean up only the affected calendar tooltips', () => {
jest.useFakeTimers();
const { rerender, unmount } = render(
<CalendarHarness firstMetricName="first-metric-initial" />,
);
const firstCalendarOwner = getCalendarOwner('calendar-first');
const secondCalendarOwner = getCalendarOwner('calendar-second');
const firstTooltipOwnerId = getTooltipOwnerId(firstCalendarOwner);
const secondTooltipOwnerId = getTooltipOwnerId(secondCalendarOwner);
const firstInitialTooltips = getOwnerTooltips(firstTooltipOwnerId);
const secondInitialTooltips = getOwnerTooltips(secondTooltipOwnerId);
expect(firstInitialTooltips).toHaveLength(2);
expect(secondInitialTooltips).toHaveLength(2);
expect(document.querySelectorAll(`.${CALENDAR_TOOLTIP_CLASS}`)).toHaveLength(
4,
);
const firstInitialInstanceId =
getSingleTooltipInstanceId(firstInitialTooltips);
const secondInitialInstanceId = getSingleTooltipInstanceId(
secondInitialTooltips,
);
rerender(<CalendarHarness firstMetricName="first-metric-rerendered" />);
const firstRerenderedTooltips = getOwnerTooltips(firstTooltipOwnerId);
const secondPreservedTooltips = getOwnerTooltips(secondTooltipOwnerId);
expect(firstRerenderedTooltips).toHaveLength(2);
expect(secondPreservedTooltips).toHaveLength(2);
expect(document.querySelectorAll(`.${CALENDAR_TOOLTIP_CLASS}`)).toHaveLength(
4,
);
expect(firstInitialTooltips.every(tooltip => !tooltip.isConnected)).toBe(
true,
);
expect(secondInitialTooltips.every(tooltip => tooltip.isConnected)).toBe(
true,
);
expect(getSingleTooltipInstanceId(firstRerenderedTooltips)).not.toEqual(
firstInitialInstanceId,
);
expect(getSingleTooltipInstanceId(secondPreservedTooltips)).toEqual(
secondInitialInstanceId,
);
rerender(
<CalendarHarness
firstMetricName="first-metric-rerendered"
showFirstCalendar={false}
/>,
);
expect(getOwnerTooltips(firstTooltipOwnerId)).toHaveLength(2);
expect(getOwnerTooltips(secondTooltipOwnerId)).toHaveLength(2);
expect(document.querySelectorAll(`.${CALENDAR_TOOLTIP_CLASS}`)).toHaveLength(
4,
);
act(() => {
jest.runOnlyPendingTimers();
});
expect(getOwnerTooltips(firstTooltipOwnerId)).toHaveLength(0);
expect(getOwnerTooltips(secondTooltipOwnerId)).toHaveLength(2);
expect(document.querySelectorAll(`.${CALENDAR_TOOLTIP_CLASS}`)).toHaveLength(
2,
);
expect(
getSingleTooltipInstanceId(getOwnerTooltips(secondTooltipOwnerId)),
).toEqual(secondInitialInstanceId);
unmount();
});
test('surviving calendar render synchronously sweeps disconnected sibling tooltips', () => {
jest.useFakeTimers();
const { rerender, unmount } = render(
<CalendarHarness
firstMetricName="first-metric-initial"
secondMetricName="second-metric-initial"
/>,
);
const firstCalendarOwner = getCalendarOwner('calendar-first');
const secondCalendarOwner = getCalendarOwner('calendar-second');
const firstTooltipOwnerId = getTooltipOwnerId(firstCalendarOwner);
const secondTooltipOwnerId = getTooltipOwnerId(secondCalendarOwner);
const secondInitialInstanceId = getSingleTooltipInstanceId(
getOwnerTooltips(secondTooltipOwnerId),
);
rerender(
<CalendarHarness
firstMetricName="first-metric-initial"
secondMetricName="second-metric-initial"
showFirstCalendar={false}
/>,
);
expect(getOwnerTooltips(firstTooltipOwnerId)).toHaveLength(2);
expect(getOwnerTooltips(secondTooltipOwnerId)).toHaveLength(2);
expect(document.querySelectorAll(`.${CALENDAR_TOOLTIP_CLASS}`)).toHaveLength(
4,
);
rerender(
<CalendarHarness
firstMetricName="first-metric-initial"
secondMetricName="second-metric-rerendered"
showFirstCalendar={false}
/>,
);
const secondRerenderedTooltips = getOwnerTooltips(secondTooltipOwnerId);
expect(getOwnerTooltips(firstTooltipOwnerId)).toHaveLength(0);
expect(secondRerenderedTooltips).toHaveLength(2);
expect(document.querySelectorAll(`.${CALENDAR_TOOLTIP_CLASS}`)).toHaveLength(
2,
);
expect(getSingleTooltipInstanceId(secondRerenderedTooltips)).not.toEqual(
secondInitialInstanceId,
);
unmount();
});

View File

@@ -0,0 +1,173 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import {
CALENDAR_TOOLTIP_CLASS,
getCalendarTooltipClassName,
removeDisconnectedCalendarTooltips,
} from '../src/tooltip';
function createCalendarTooltip(className: string) {
const tooltip = document.createElement('div');
tooltip.className = `d3-tip ${CALENDAR_TOOLTIP_CLASS} ${className}`;
document.body.appendChild(tooltip);
return tooltip;
}
afterEach(() => {
document.body.innerHTML = '';
});
test('getCalendarTooltipClassName creates stable owner-specific class names', () => {
const firstCalendar = document.createElement('div');
const secondCalendar = document.createElement('div');
const firstClassName = getCalendarTooltipClassName(firstCalendar);
const secondClassName = getCalendarTooltipClassName(secondCalendar);
expect(firstClassName).toContain(CALENDAR_TOOLTIP_CLASS);
expect(secondClassName).toContain(CALENDAR_TOOLTIP_CLASS);
expect(firstClassName).not.toEqual(secondClassName);
expect(getCalendarTooltipClassName(firstCalendar)).toEqual(firstClassName);
});
test('removeDisconnectedCalendarTooltips preserves mounted calendar tooltips', () => {
const firstCalendar = document.createElement('div');
const secondCalendar = document.createElement('div');
document.body.append(firstCalendar, secondCalendar);
const firstClassName = getCalendarTooltipClassName(firstCalendar);
const secondClassName = getCalendarTooltipClassName(secondCalendar);
const firstTooltip = document.createElement('div');
firstTooltip.className = `d3-tip ${CALENDAR_TOOLTIP_CLASS} ${firstClassName}`;
document.body.appendChild(firstTooltip);
const secondTooltip = document.createElement('div');
secondTooltip.className = `d3-tip ${CALENDAR_TOOLTIP_CLASS} ${secondClassName}`;
document.body.appendChild(secondTooltip);
removeDisconnectedCalendarTooltips();
expect(document.querySelector(`.${firstClassName}`)).toBe(firstTooltip);
expect(document.querySelector(`.${secondClassName}`)).toBe(secondTooltip);
});
test('removeDisconnectedCalendarTooltips removes only disconnected calendar tooltips', () => {
const firstCalendar = document.createElement('div');
const secondCalendar = document.createElement('div');
document.body.append(firstCalendar, secondCalendar);
const firstClassName = getCalendarTooltipClassName(firstCalendar);
const secondClassName = getCalendarTooltipClassName(secondCalendar);
firstCalendar.remove();
const firstTooltip = document.createElement('div');
firstTooltip.className = `d3-tip ${CALENDAR_TOOLTIP_CLASS} ${firstClassName}`;
document.body.appendChild(firstTooltip);
const secondTooltip = document.createElement('div');
secondTooltip.className = `d3-tip ${CALENDAR_TOOLTIP_CLASS} ${secondClassName}`;
document.body.appendChild(secondTooltip);
const otherTooltip = document.createElement('div');
otherTooltip.className = 'd3-tip tooltip-other-chart';
document.body.appendChild(otherTooltip);
removeDisconnectedCalendarTooltips();
expect(document.querySelector(`.${firstClassName}`)).toBeNull();
expect(document.querySelector(`.${secondClassName}`)).toBe(secondTooltip);
expect(document.querySelector('.tooltip-other-chart')).toBe(otherTooltip);
});
test('reused calendar owners are re-armed for disconnected tooltip cleanup', () => {
const calendar = document.createElement('div');
document.body.appendChild(calendar);
const className = getCalendarTooltipClassName(calendar);
const firstTooltip = createCalendarTooltip(className);
calendar.remove();
removeDisconnectedCalendarTooltips();
expect(firstTooltip.isConnected).toBe(false);
document.body.appendChild(calendar);
expect(getCalendarTooltipClassName(calendar)).toBe(className);
const secondTooltip = createCalendarTooltip(className);
calendar.remove();
removeDisconnectedCalendarTooltips();
expect(secondTooltip.isConnected).toBe(false);
expect(document.querySelector(`.${className}`)).toBeNull();
});
test('CalHeatMap tags tips per instance without removing other mounted calendars', () => {
const firstCalendar = document.createElement('div');
const secondCalendar = document.createElement('div');
document.body.append(firstCalendar, secondCalendar);
const firstClassName = getCalendarTooltipClassName(firstCalendar);
const secondClassName = getCalendarTooltipClassName(secondCalendar);
Object.defineProperty(window.SVGSVGElement.prototype, 'createSVGPoint', {
configurable: true,
value: () => ({
matrixTransform: () => ({ x: 0, y: 0 }),
}),
});
// eslint-disable-next-line global-require
const CalHeatMap = require('../src/vendor/cal-heatmap').default;
const firstHeatmap = new CalHeatMap();
firstHeatmap.init({
itemSelector: firstCalendar,
paintOnLoad: false,
tooltip: true,
tooltipClassName: firstClassName,
valueFormatter: String,
timeFormatter: String,
});
const secondHeatmap = new CalHeatMap();
secondHeatmap.init({
itemSelector: secondCalendar,
paintOnLoad: false,
tooltip: true,
tooltipClassName: secondClassName,
valueFormatter: String,
timeFormatter: String,
});
expect(document.querySelectorAll(`.${CALENDAR_TOOLTIP_CLASS}`)).toHaveLength(
4,
);
expect(document.querySelectorAll(`.${firstClassName}`)).toHaveLength(2);
expect(document.querySelectorAll(`.${secondClassName}`)).toHaveLength(2);
firstHeatmap.destroy();
expect(document.querySelectorAll(`.${firstClassName}`)).toHaveLength(0);
expect(document.querySelectorAll(`.${secondClassName}`)).toHaveLength(2);
});