Skip to content

Commit cac6ffc

Browse files
EnxDevgeido
andauthored
fix: Extra controls width for Area Chart on dashboards (#36133)
Co-authored-by: Diego Pucci <[email protected]>
1 parent 08c1d03 commit cac6ffc

File tree

2 files changed

+337
-2
lines changed

2 files changed

+337
-2
lines changed
Lines changed: 311 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,311 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
import {
20+
render,
21+
waitFor,
22+
cleanup,
23+
} from '../../../../spec/helpers/testing-library';
24+
import { AxisType } from '@superset-ui/core';
25+
import type { EChartsCoreOption } from 'echarts/core';
26+
import type { ReactNode } from 'react';
27+
import {
28+
LegendOrientation,
29+
LegendType,
30+
type EchartsHandler,
31+
type EchartsProps,
32+
} from '../types';
33+
import EchartsTimeseries from './EchartsTimeseries';
34+
import {
35+
EchartsTimeseriesSeriesType,
36+
OrientationType,
37+
type EchartsTimeseriesFormData,
38+
type TimeseriesChartTransformedProps,
39+
} from './types';
40+
41+
const mockEchart = jest.fn();
42+
43+
jest.mock('../components/Echart', () => {
44+
const { forwardRef } = jest.requireActual<typeof import('react')>('react');
45+
const MockEchart = forwardRef<EchartsHandler | null, EchartsProps>(
46+
(props, ref) => {
47+
mockEchart(props);
48+
void ref;
49+
return null;
50+
},
51+
);
52+
MockEchart.displayName = 'MockEchart';
53+
return {
54+
__esModule: true,
55+
default: MockEchart,
56+
};
57+
});
58+
59+
jest.mock('../components/ExtraControls', () => ({
60+
ExtraControls: ({ children }: { children?: ReactNode }) => (
61+
<div data-testid="extra-controls">{children}</div>
62+
),
63+
}));
64+
65+
const originalResizeObserver = globalThis.ResizeObserver;
66+
const offsetHeightDescriptor = Object.getOwnPropertyDescriptor(
67+
HTMLElement.prototype,
68+
'offsetHeight',
69+
);
70+
71+
let mockOffsetHeight = 0;
72+
73+
beforeAll(() => {
74+
Object.defineProperty(HTMLElement.prototype, 'offsetHeight', {
75+
configurable: true,
76+
get() {
77+
return mockOffsetHeight;
78+
},
79+
});
80+
});
81+
82+
afterAll(() => {
83+
if (offsetHeightDescriptor) {
84+
Object.defineProperty(
85+
HTMLElement.prototype,
86+
'offsetHeight',
87+
offsetHeightDescriptor,
88+
);
89+
} else {
90+
delete (HTMLElement.prototype as { offsetHeight?: number }).offsetHeight;
91+
}
92+
});
93+
94+
afterEach(() => {
95+
cleanup();
96+
mockEchart.mockReset();
97+
(globalThis as { ResizeObserver?: typeof ResizeObserver }).ResizeObserver =
98+
originalResizeObserver;
99+
});
100+
101+
const defaultFormData: EchartsTimeseriesFormData & {
102+
vizType: string;
103+
dateFormat: string;
104+
numberFormat: string;
105+
granularitySqla?: string;
106+
} = {
107+
annotationLayers: [],
108+
area: false,
109+
colorScheme: undefined,
110+
timeShiftColor: false,
111+
contributionMode: undefined,
112+
forecastEnabled: false,
113+
forecastPeriods: 0,
114+
forecastInterval: 0,
115+
forecastSeasonalityDaily: null,
116+
forecastSeasonalityWeekly: null,
117+
forecastSeasonalityYearly: null,
118+
logAxis: false,
119+
markerEnabled: false,
120+
markerSize: 1,
121+
metrics: [],
122+
minorSplitLine: false,
123+
minorTicks: false,
124+
opacity: 1,
125+
orderDesc: false,
126+
rowLimit: 0,
127+
seriesType: EchartsTimeseriesSeriesType.Line,
128+
stack: null,
129+
stackDimension: '',
130+
timeCompare: [],
131+
tooltipTimeFormat: undefined,
132+
showTooltipTotal: false,
133+
showTooltipPercentage: false,
134+
truncateXAxis: false,
135+
truncateYAxis: false,
136+
yAxisFormat: undefined,
137+
xAxisForceCategorical: false,
138+
xAxisTimeFormat: undefined,
139+
timeGrainSqla: undefined,
140+
forceMaxInterval: false,
141+
xAxisBounds: [null, null],
142+
yAxisBounds: [null, null],
143+
zoomable: false,
144+
richTooltip: false,
145+
xAxisLabelRotation: 0,
146+
xAxisLabelInterval: 0,
147+
showValue: false,
148+
onlyTotal: false,
149+
showExtraControls: true,
150+
percentageThreshold: 0,
151+
orientation: OrientationType.Vertical,
152+
datasource: '1__table',
153+
viz_type: 'echarts_timeseries',
154+
legendMargin: 0,
155+
legendOrientation: LegendOrientation.Top,
156+
legendType: LegendType.Plain,
157+
showLegend: false,
158+
legendSort: null,
159+
xAxisTitle: '',
160+
xAxisTitleMargin: 0,
161+
yAxisTitle: '',
162+
yAxisTitleMargin: 0,
163+
yAxisTitlePosition: '',
164+
time_range: 'No filter',
165+
granularity: undefined,
166+
granularity_sqla: undefined,
167+
sql: '',
168+
url_params: {},
169+
custom_params: {},
170+
extra_form_data: {},
171+
adhoc_filters: [],
172+
order_desc: false,
173+
row_limit: 0,
174+
row_offset: 0,
175+
time_grain_sqla: undefined,
176+
vizType: 'echarts_timeseries',
177+
dateFormat: 'smart_date',
178+
numberFormat: 'SMART_NUMBER',
179+
};
180+
181+
const defaultProps: TimeseriesChartTransformedProps = {
182+
echartOptions: {} as EChartsCoreOption,
183+
formData: defaultFormData,
184+
height: 400,
185+
width: 800,
186+
onContextMenu: jest.fn(),
187+
setDataMask: jest.fn(),
188+
onLegendStateChanged: jest.fn(),
189+
refs: {},
190+
emitCrossFilters: false,
191+
coltypeMapping: {},
192+
onLegendScroll: jest.fn(),
193+
groupby: [],
194+
labelMap: {},
195+
setControlValue: jest.fn(),
196+
selectedValues: {},
197+
legendData: [],
198+
xValueFormatter: String,
199+
xAxis: {
200+
label: 'x',
201+
type: AxisType.Time,
202+
},
203+
onFocusedSeries: jest.fn(),
204+
};
205+
206+
function getLatestHeight() {
207+
const lastCall = mockEchart.mock.calls.at(-1);
208+
expect(lastCall).toBeDefined();
209+
const [props] = lastCall as [EchartsProps];
210+
return props.height;
211+
}
212+
213+
test('observes extra control height changes when ResizeObserver is available', async () => {
214+
const disconnectSpy = jest.fn();
215+
const observeSpy = jest.fn();
216+
217+
class MockResizeObserver implements ResizeObserver {
218+
private static latestInstance: MockResizeObserver | null = null;
219+
private readonly callback: ResizeObserverCallback;
220+
221+
constructor(callback: ResizeObserverCallback) {
222+
this.callback = callback;
223+
MockResizeObserver.latestInstance = this;
224+
}
225+
226+
observe = (target: Element) => {
227+
observeSpy(target);
228+
};
229+
230+
unobserve(_target: Element): void {
231+
void _target;
232+
}
233+
234+
disconnect = () => {
235+
disconnectSpy();
236+
};
237+
238+
trigger(entries: ResizeObserverEntry[] = []) {
239+
this.callback(entries, this);
240+
}
241+
242+
static getLatestInstance() {
243+
return this.latestInstance;
244+
}
245+
}
246+
247+
(globalThis as { ResizeObserver?: typeof ResizeObserver }).ResizeObserver =
248+
MockResizeObserver as unknown as typeof ResizeObserver;
249+
250+
mockOffsetHeight = 42;
251+
const { unmount } = render(<EchartsTimeseries {...defaultProps} />);
252+
253+
await waitFor(() => {
254+
expect(getLatestHeight()).toBe(defaultProps.height - mockOffsetHeight);
255+
});
256+
257+
expect(observeSpy).toHaveBeenCalledWith(expect.any(HTMLElement));
258+
259+
mockOffsetHeight = 24;
260+
MockResizeObserver.getLatestInstance()?.trigger();
261+
262+
await waitFor(() => {
263+
expect(getLatestHeight()).toBe(defaultProps.height - mockOffsetHeight);
264+
});
265+
266+
expect(disconnectSpy).not.toHaveBeenCalled();
267+
268+
expect(MockResizeObserver.getLatestInstance()).not.toBeNull();
269+
270+
unmount();
271+
272+
expect(disconnectSpy).toHaveBeenCalled();
273+
});
274+
275+
test('falls back to window resize listener when ResizeObserver is unavailable', async () => {
276+
(globalThis as { ResizeObserver?: typeof ResizeObserver }).ResizeObserver =
277+
undefined;
278+
279+
const addEventListenerSpy = jest.spyOn(window, 'addEventListener');
280+
const removeEventListenerSpy = jest.spyOn(window, 'removeEventListener');
281+
282+
mockOffsetHeight = 30;
283+
284+
const { unmount } = render(<EchartsTimeseries {...defaultProps} />);
285+
286+
await waitFor(() => {
287+
expect(getLatestHeight()).toBe(defaultProps.height - mockOffsetHeight);
288+
});
289+
290+
expect(addEventListenerSpy).toHaveBeenCalledWith(
291+
'resize',
292+
expect.any(Function),
293+
);
294+
295+
mockOffsetHeight = 10;
296+
window.dispatchEvent(new Event('resize'));
297+
298+
await waitFor(() => {
299+
expect(getLatestHeight()).toBe(defaultProps.height - mockOffsetHeight);
300+
});
301+
302+
unmount();
303+
304+
expect(removeEventListenerSpy).toHaveBeenCalledWith(
305+
'resize',
306+
expect.any(Function),
307+
);
308+
309+
addEventListenerSpy.mockRestore();
310+
removeEventListenerSpy.mockRestore();
311+
});

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,32 @@ export default function EchartsTimeseries({
6767
const extraControlRef = useRef<HTMLDivElement>(null);
6868
const [extraControlHeight, setExtraControlHeight] = useState(0);
6969
useEffect(() => {
70-
const updatedHeight = extraControlRef.current?.offsetHeight || 0;
71-
setExtraControlHeight(updatedHeight);
70+
const element = extraControlRef.current;
71+
if (!element) {
72+
setExtraControlHeight(0);
73+
return;
74+
}
75+
76+
const updateHeight = () => {
77+
setExtraControlHeight(element.offsetHeight || 0);
78+
};
79+
80+
updateHeight();
81+
82+
if (typeof ResizeObserver === 'function') {
83+
const resizeObserver = new ResizeObserver(() => {
84+
updateHeight();
85+
});
86+
resizeObserver.observe(element);
87+
return () => {
88+
resizeObserver.disconnect();
89+
};
90+
}
91+
92+
window.addEventListener('resize', updateHeight);
93+
return () => {
94+
window.removeEventListener('resize', updateHeight);
95+
};
7296
}, [formData.showExtraControls]);
7397

7498
const hasDimensions = ensureIsArray(groupby).length > 0;

0 commit comments

Comments
 (0)