This is an automated email from the ASF dual-hosted git repository.
EnxDev pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new e403d9b0742 chore(lint): convert legacy plugin chart components to
function components (#39452)
e403d9b0742 is described below
commit e403d9b07424daa02ac88925a71e91e2f8b75314
Author: Evan Rusackas <[email protected]>
AuthorDate: Sun Jul 12 15:37:46 2026 -0700
chore(lint): convert legacy plugin chart components to function components
(#39452)
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: Enzo Martellucci <[email protected]>
---
.../src/HorizonChart.tsx | 101 ++--
.../legacy-plugin-chart-horizon/src/HorizonRow.tsx | 268 +++++-----
.../test/HorizonChart.test.tsx | 51 ++
.../test/HorizonRow.test.tsx | 102 ++++
.../src/PairedTTest.tsx | 77 ++-
.../src/TTestTable.tsx | 545 ++++++++++++---------
.../test/TTestTable.test.tsx | 365 ++++++++++++++
.../src/chart/WordCloud.tsx | 273 ++++++-----
.../test/WordCloud.test.tsx | 161 ++++++
9 files changed, 1324 insertions(+), 619 deletions(-)
diff --git
a/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonChart.tsx
b/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonChart.tsx
index a95cae279dd..9a10d4cd8b6 100644
--- a/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonChart.tsx
+++ b/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonChart.tsx
@@ -16,8 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-/* eslint-disable react/jsx-sort-default-props, react/sort-prop-types */
-import { PureComponent } from 'react';
+import { memo, useMemo } from 'react';
import { extent as d3Extent } from 'd3-array';
import { ensureIsArray } from '@superset-ui/core';
import { styled } from '@apache-superset/core/theme';
@@ -45,18 +44,6 @@ interface HorizonChartProps {
offsetX?: number;
}
-const defaultProps: Partial<HorizonChartProps> = {
- className: '',
- width: 800,
- height: 600,
- seriesHeight: 20,
- bands: Math.floor(DEFAULT_COLORS.length / 2),
- colors: DEFAULT_COLORS,
- colorScale: 'series',
- mode: 'offset',
- offsetX: 0,
-};
-
const StyledDiv = styled.div`
${({ theme }) => `
.superset-legacy-chart-horizon {
@@ -80,24 +67,19 @@ const StyledDiv = styled.div`
`}
`;
-class HorizonChart extends PureComponent<HorizonChartProps> {
- static defaultProps = defaultProps;
-
- render() {
- const {
- className,
- width,
- height,
- data,
- seriesHeight,
- bands,
- colors,
- colorScale,
- mode,
- offsetX,
- } = this.props;
-
- let yDomain: [number, number] | undefined;
+function HorizonChart({
+ className = '',
+ width = 800,
+ height = 600,
+ seriesHeight = 20,
+ data,
+ bands = Math.floor(DEFAULT_COLORS.length / 2),
+ colors = DEFAULT_COLORS,
+ colorScale = 'series',
+ mode = 'offset',
+ offsetX = 0,
+}: HorizonChartProps) {
+ const yDomain = useMemo((): [number, number] | undefined => {
if (colorScale === 'overall') {
const allValues = data.reduce<DataValue[]>(
(acc, current) => acc.concat(current.values),
@@ -106,35 +88,36 @@ class HorizonChart extends
PureComponent<HorizonChartProps> {
const rawExtent = d3Extent(allValues, d => d.y);
// Only set yDomain if we have valid min and max values
if (rawExtent[0] != null && rawExtent[1] != null) {
- yDomain = [rawExtent[0], rawExtent[1]];
+ return [rawExtent[0], rawExtent[1]];
}
}
+ return undefined;
+ }, [colorScale, data]);
- return (
- <StyledDiv>
- <div
- className={`superset-legacy-chart-horizon ${className}`}
- style={{ height }}
- >
- {data.map(row => (
- <HorizonRow
- key={row.key.join(',')}
- width={width}
- height={seriesHeight}
- title={ensureIsArray(row.key).join(', ')}
- data={row.values}
- bands={bands}
- colors={colors}
- colorScale={colorScale}
- mode={mode}
- offsetX={offsetX}
- yDomain={yDomain}
- />
- ))}
- </div>
- </StyledDiv>
- );
- }
+ return (
+ <StyledDiv>
+ <div
+ className={`superset-legacy-chart-horizon ${className}`}
+ style={{ height }}
+ >
+ {data.map(row => (
+ <HorizonRow
+ key={row.key.join(',')}
+ width={width}
+ height={seriesHeight}
+ title={ensureIsArray(row.key).join(', ')}
+ data={row.values}
+ bands={bands}
+ colors={colors}
+ colorScale={colorScale}
+ mode={mode}
+ offsetX={offsetX}
+ yDomain={yDomain}
+ />
+ ))}
+ </div>
+ </StyledDiv>
+ );
}
-export default HorizonChart;
+export default memo(HorizonChart);
diff --git
a/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonRow.tsx
b/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonRow.tsx
index f399d850d7c..5431674b7e9 100644
--- a/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonRow.tsx
+++ b/superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonRow.tsx
@@ -17,9 +17,7 @@
* under the License.
*/
/* eslint-disable no-continue, no-bitwise */
-/* eslint-disable react/jsx-sort-default-props */
-/* eslint-disable react/sort-prop-types */
-import { PureComponent } from 'react';
+import { useRef, useEffect, useCallback, memo } from 'react';
import { extent as d3Extent } from 'd3-array';
import { scaleLinear } from 'd3-scale';
@@ -52,162 +50,140 @@ interface HorizonRowProps {
yDomain?: [number, number];
}
-const defaultProps: Partial<HorizonRowProps> = {
- className: '',
- width: 800,
- height: 20,
- bands: DEFAULT_COLORS.length >> 1,
- colors: DEFAULT_COLORS,
- colorScale: 'series',
- mode: 'offset',
- offsetX: 0,
- title: '',
- yDomain: undefined,
-};
-
-class HorizonRow extends PureComponent<HorizonRowProps> {
- static defaultProps = defaultProps;
-
- private canvas: HTMLCanvasElement | null = null;
-
- componentDidMount() {
- this.drawChart();
- }
-
- componentDidUpdate() {
- this.drawChart();
- }
-
- componentWillUnmount() {
- this.canvas = null;
- }
-
- drawChart() {
- if (this.canvas) {
- const {
- data: rawData,
- yDomain,
- width = 800,
- height = 20,
- bands = DEFAULT_COLORS.length >> 1,
- colors = DEFAULT_COLORS,
- colorScale,
- offsetX = 0,
- mode,
- } = this.props;
-
- const data =
- colorScale === 'change'
- ? rawData.map(d => ({ ...d, y: d.y - rawData[0].y }))
- : rawData;
-
- const context = this.canvas.getContext('2d');
- if (!context) return;
- context.imageSmoothingEnabled = false;
- context.clearRect(0, 0, width, height);
- // Reset transform
- context.setTransform(1, 0, 0, 1, 0, 0);
- context.translate(0.5, 0.5);
-
- const step = width / data.length;
- // the data frame currently being shown:
- const startIndex = Math.floor(Math.max(0, -(offsetX / step)));
- const endIndex = Math.floor(
- Math.min(data.length, startIndex + width / step),
- );
-
- // skip drawing if there's no data to be drawn
- if (startIndex > data.length) {
- return;
+function HorizonRow({
+ className = '',
+ width = 800,
+ height = 20,
+ data: rawData,
+ bands = DEFAULT_COLORS.length >> 1,
+ colors = DEFAULT_COLORS,
+ colorScale = 'series',
+ mode = 'offset',
+ offsetX = 0,
+ title = '',
+ yDomain,
+}: HorizonRowProps) {
+ const canvasRef = useRef<HTMLCanvasElement | null>(null);
+
+ const drawChart = useCallback(() => {
+ const canvas = canvasRef.current;
+ if (!canvas) return;
+
+ const data =
+ colorScale === 'change' && rawData.length > 0
+ ? rawData.map(d => ({ ...d, y: d.y - rawData[0].y }))
+ : rawData;
+
+ const context = canvas.getContext('2d');
+ if (!context) return;
+ context.imageSmoothingEnabled = false;
+ context.clearRect(0, 0, width, height);
+ // Reset transform
+ context.setTransform(1, 0, 0, 1, 0, 0);
+ context.translate(0.5, 0.5);
+
+ const step = width / data.length;
+ // the data frame currently being shown:
+ const startIndex = Math.floor(Math.max(0, -(offsetX / step)));
+ const endIndex = Math.floor(
+ Math.min(data.length, startIndex + width / step),
+ );
+
+ // skip drawing if there's no data to be drawn
+ if (startIndex > data.length) {
+ return;
+ }
+
+ // Create y-scale
+ const [min, max] =
+ yDomain || (d3Extent(data, d => d.y) as [number, number]);
+ const y = scaleLinear()
+ .domain([0, Math.max(-min, max)])
+ .range([0, height]);
+
+ // we are drawing positive & negative bands separately to avoid mutating
canvas state
+ // http://www.html5rocks.com/en/tutorials/canvas/performance/
+ let hasNegative = false;
+ // draw positive bands
+ let value: number;
+ let bExtents: number;
+ for (let b = 0; b < bands; b += 1) {
+ context.fillStyle = colors[bands + b];
+
+ // Adjust the range based on the current band index.
+ bExtents = (b + 1 - bands) * height;
+ y.range([bands * height + bExtents, bExtents]);
+
+ // only the current data frame is being drawn i.e. what's visible:
+ for (let i = startIndex; i < endIndex; i += 1) {
+ value = data[i].y;
+ if (value <= 0) {
+ hasNegative = true;
+ continue;
+ }
+ if (value !== undefined) {
+ context.fillRect(
+ offsetX + i * step,
+ y(value)!,
+ step + 1,
+ y(0)! - y(value)!,
+ );
+ }
+ }
+ }
+
+ // draw negative bands
+ if (hasNegative) {
+ // mirror the negative bands, by flipping the canvas
+ if (mode === 'offset') {
+ context.translate(0, height);
+ context.scale(1, -1);
}
- // Create y-scale
- const [min, max] =
- yDomain || (d3Extent(data, d => d.y) as [number, number]);
- const y = scaleLinear()
- .domain([0, Math.max(-min, max)])
- .range([0, height]);
-
- // we are drawing positive & negative bands separately to avoid mutating
canvas state
- // http://www.html5rocks.com/en/tutorials/canvas/performance/
- let hasNegative = false;
- // draw positive bands
- let value: number;
- let bExtents: number;
for (let b = 0; b < bands; b += 1) {
- context.fillStyle = colors[bands + b];
+ context.fillStyle = colors[bands - b - 1];
// Adjust the range based on the current band index.
bExtents = (b + 1 - bands) * height;
y.range([bands * height + bExtents, bExtents]);
// only the current data frame is being drawn i.e. what's visible:
- for (let i = startIndex; i < endIndex; i += 1) {
- value = data[i].y;
- if (value <= 0) {
- hasNegative = true;
+ for (let ii = startIndex; ii < endIndex; ii += 1) {
+ value = data[ii].y;
+ if (value >= 0) {
continue;
}
- if (value !== undefined) {
- context.fillRect(
- offsetX + i * step,
- y(value)!,
- step + 1,
- y(0)! - y(value)!,
- );
- }
- }
- }
-
- // draw negative bands
- if (hasNegative) {
- // mirror the negative bands, by flipping the canvas
- if (mode === 'offset') {
- context.translate(0, height);
- context.scale(1, -1);
- }
-
- for (let b = 0; b < bands; b += 1) {
- context.fillStyle = colors[bands - b - 1];
-
- // Adjust the range based on the current band index.
- bExtents = (b + 1 - bands) * height;
- y.range([bands * height + bExtents, bExtents]);
-
- // only the current data frame is being drawn i.e. what's visible:
- for (let ii = startIndex; ii < endIndex; ii += 1) {
- value = data[ii].y;
- if (value >= 0) {
- continue;
- }
- context.fillRect(
- offsetX + ii * step,
- y(-value)!,
- step + 1,
- y(0)! - y(-value)!,
- );
- }
+ context.fillRect(
+ offsetX + ii * step,
+ y(-value)!,
+ step + 1,
+ y(0)! - y(-value)!,
+ );
}
}
}
- }
-
- render() {
- const { className, title, width, height } = this.props;
-
- return (
- <div className={`horizon-row ${className}`}>
- <span className="title">{title}</span>
- <canvas
- ref={c => {
- this.canvas = c;
- }}
- width={width}
- height={height}
- />
- </div>
- );
- }
+ }, [
+ rawData,
+ yDomain,
+ width,
+ height,
+ bands,
+ colors,
+ colorScale,
+ offsetX,
+ mode,
+ ]);
+
+ useEffect(() => {
+ drawChart();
+ }, [drawChart]);
+
+ return (
+ <div className={`horizon-row ${className}`}>
+ <span className="title">{title}</span>
+ <canvas ref={canvasRef} width={width} height={height} />
+ </div>
+ );
}
-export default HorizonRow;
+export default memo(HorizonRow);
diff --git
a/superset-frontend/plugins/legacy-plugin-chart-horizon/test/HorizonChart.test.tsx
b/superset-frontend/plugins/legacy-plugin-chart-horizon/test/HorizonChart.test.tsx
new file mode 100644
index 00000000000..cd27c5e973a
--- /dev/null
+++
b/superset-frontend/plugins/legacy-plugin-chart-horizon/test/HorizonChart.test.tsx
@@ -0,0 +1,51 @@
+/**
+ * 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 { render, screen } from 'spec/helpers/testing-library';
+import HorizonChart from '../src/HorizonChart';
+
+beforeAll(() => {
+ // jsdom has no 2d context; HorizonRow guards on a null context
+ jest.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue(null);
+});
+
+const data = [
+ { key: ['series', 'a'], values: [{ y: 1 }, { y: 2 }] },
+ { key: ['series', 'b'], values: [{ y: 3 }, { y: 4 }] },
+];
+
+test('renders one row per data series with joined titles', () => {
+ const { container } = render(<HorizonChart data={data} />);
+
+ expect(container.querySelectorAll('.horizon-row')).toHaveLength(2);
+ expect(screen.getByText('series, a')).toBeInTheDocument();
+ expect(screen.getByText('series, b')).toBeInTheDocument();
+});
+
+test('applies the chart height to the container', () => {
+ const { container } = render(<HorizonChart data={data} height={300} />);
+
+ const chart = container.querySelector('.superset-legacy-chart-horizon');
+ expect(chart).toHaveStyle({ height: '300px' });
+});
+
+test('renders nothing but the container for empty data', () => {
+ const { container } = render(<HorizonChart data={[]} />);
+
+ expect(container.querySelectorAll('.horizon-row')).toHaveLength(0);
+});
diff --git
a/superset-frontend/plugins/legacy-plugin-chart-horizon/test/HorizonRow.test.tsx
b/superset-frontend/plugins/legacy-plugin-chart-horizon/test/HorizonRow.test.tsx
new file mode 100644
index 00000000000..940e070d069
--- /dev/null
+++
b/superset-frontend/plugins/legacy-plugin-chart-horizon/test/HorizonRow.test.tsx
@@ -0,0 +1,102 @@
+/**
+ * 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 { render, screen } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import HorizonRow from '../src/HorizonRow';
+
+const mockContext = {
+ imageSmoothingEnabled: false,
+ fillStyle: '',
+ clearRect: jest.fn(),
+ setTransform: jest.fn(),
+ translate: jest.fn(),
+ scale: jest.fn(),
+ fillRect: jest.fn(),
+};
+
+beforeAll(() => {
+ jest
+ .spyOn(HTMLCanvasElement.prototype, 'getContext')
+ .mockReturnValue(mockContext as unknown as CanvasRenderingContext2D);
+});
+
+beforeEach(() => {
+ jest.clearAllMocks();
+});
+
+const positiveData = [{ y: 1 }, { y: 2 }, { y: 3 }];
+
+test('renders the title and a canvas of the given size', () => {
+ const { container } = render(
+ <HorizonRow data={positiveData} title="series-a" width={100} height={20}
/>,
+ );
+
+ expect(screen.getByText('series-a')).toBeInTheDocument();
+ const canvas = container.querySelector('canvas');
+ expect(canvas).toHaveAttribute('width', '100');
+ expect(canvas).toHaveAttribute('height', '20');
+});
+
+test('draws positive bands to the canvas on mount', () => {
+ render(<HorizonRow data={positiveData} width={100} height={20} />);
+
+ expect(mockContext.clearRect).toHaveBeenCalledWith(0, 0, 100, 20);
+ expect(mockContext.fillRect).toHaveBeenCalled();
+ // No negative values, so the canvas is never flipped
+ expect(mockContext.scale).not.toHaveBeenCalled();
+});
+
+test('flips the canvas to mirror negative bands in offset mode', () => {
+ render(
+ <HorizonRow
+ data={[{ y: -1 }, { y: 2 }]}
+ mode="offset"
+ width={100}
+ height={20}
+ />,
+ );
+
+ expect(mockContext.translate).toHaveBeenCalledWith(0, 20);
+ expect(mockContext.scale).toHaveBeenCalledWith(1, -1);
+ expect(mockContext.fillRect).toHaveBeenCalled();
+});
+
+test('redraws when the data prop changes', () => {
+ const { rerender } = render(
+ <HorizonRow data={positiveData} width={100} height={20} />,
+ );
+ const drawsAfterMount = mockContext.clearRect.mock.calls.length;
+
+ rerender(<HorizonRow data={[{ y: 5 }, { y: 6 }]} width={100} height={20} />);
+
+ expect(mockContext.clearRect.mock.calls.length).toBeGreaterThan(
+ drawsAfterMount,
+ );
+});
+
+test('memo bailout skips redraw when props are unchanged', () => {
+ const { rerender } = render(
+ <HorizonRow data={positiveData} width={100} height={20} />,
+ );
+ const drawsAfterMount = mockContext.clearRect.mock.calls.length;
+
+ rerender(<HorizonRow data={positiveData} width={100} height={20} />);
+
+ expect(mockContext.clearRect.mock.calls.length).toBe(drawsAfterMount);
+});
diff --git
a/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/PairedTTest.tsx
b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/PairedTTest.tsx
index c7a1edfa897..fd339dfb164 100644
---
a/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/PairedTTest.tsx
+++
b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/PairedTTest.tsx
@@ -17,30 +17,23 @@
* under the License.
*/
/* eslint-disable react/no-array-index-key */
-import { PureComponent } from 'react';
+import { memo } from 'react';
import { styled } from '@apache-superset/core/theme';
import TTestTable, { DataEntry } from './TTestTable';
interface PairedTTestProps {
- alpha: number;
- className: string;
+ alpha?: number;
+ className?: string;
data: Record<string, DataEntry[]>;
groups: string[];
- liftValPrec: number;
+ liftValPrec?: number;
metrics: string[];
- pValPrec: number;
+ pValPrec?: number;
}
-const defaultProps = {
- alpha: 0.05,
- className: '',
- liftValPrec: 4,
- pValPrec: 6,
-};
-
const StyledDiv = styled.div`
${({ theme }) => `
- .superset-legacy-chart-paired_ttest .scrollbar-container {
+ .superset-legacy-chart-paired-t-test .scrollbar-container {
overflow: auto;
}
@@ -114,35 +107,37 @@ const StyledDiv = styled.div`
`}
`;
-class PairedTTest extends PureComponent<PairedTTestProps> {
- static defaultProps = defaultProps;
-
- render() {
- const { className, metrics, groups, data, alpha, pValPrec, liftValPrec } =
- this.props;
-
- return (
- <StyledDiv>
- <div className={`superset-legacy-chart-paired-t-test ${className}`}>
- <div className="paired-ttest-table">
- <div className="scrollbar-content">
- {metrics.map((metric, i) => (
- <TTestTable
- key={i}
- metric={metric}
- groups={groups}
- data={data[metric]}
- alpha={alpha}
- pValPrec={Math.min(pValPrec, 32)}
- liftValPrec={Math.min(liftValPrec, 32)}
- />
- ))}
- </div>
+function PairedTTest({
+ alpha = 0.05,
+ className = '',
+ data,
+ groups,
+ liftValPrec = 4,
+ metrics,
+ pValPrec = 6,
+}: PairedTTestProps) {
+ return (
+ <StyledDiv>
+ <div className={`superset-legacy-chart-paired-t-test ${className}`}>
+ <div className="paired-ttest-table">
+ <div className="scrollbar-content">
+ {metrics.map((metric, i) => (
+ <TTestTable
+ key={i}
+ metric={metric}
+ groups={groups}
+ data={data[metric]}
+ alpha={alpha}
+ pValPrec={Math.min(pValPrec, 32)}
+ liftValPrec={Math.min(liftValPrec, 32)}
+ />
+ ))}
</div>
</div>
- </StyledDiv>
- );
- }
+ </div>
+ </StyledDiv>
+ );
}
-export default PairedTTest;
+// memo preserves the shallow-prop render bailout of the PureComponent original
+export default memo(PairedTTest);
diff --git
a/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx
b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx
index 95922fd32de..ca0881558e2 100644
---
a/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx
+++
b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/src/TTestTable.tsx
@@ -18,7 +18,7 @@
*/
/* eslint-disable react/no-array-index-key, react/jsx-no-bind */
import dist from 'distributions';
-import { Component } from 'react';
+import { useState, useEffect, useCallback, useMemo, useRef } from 'react';
import { Table, Tr, Td, Thead, Th } from 'reactable';
interface DataPointValue {
@@ -32,279 +32,338 @@ export interface DataEntry {
}
interface TTestTableProps {
- alpha: number;
+ alpha?: number;
data: DataEntry[];
groups: string[];
- liftValPrec: number;
+ liftValPrec?: number;
metric: string;
- pValPrec: number;
+ pValPrec?: number;
}
-interface TTestTableState {
- control: number;
- liftValues: (string | number)[];
- pValues: (string | number)[];
-}
-
-const defaultProps = {
- alpha: 0.05,
- liftValPrec: 4,
- pValPrec: 6,
-};
+function TTestTable({
+ alpha = 0.05,
+ data,
+ groups,
+ liftValPrec = 4,
+ metric,
+ pValPrec = 6,
+}: TTestTableProps) {
+ const [control, setControl] = useState(0);
+ // Mirrors `control` so the data-change effect can read the latest value
+ // without re-running (and recomputing) after every row click
+ const controlRef = useRef(0);
+ const [liftValues, setLiftValues] = useState<(string | number)[]>([]);
+ const [pValues, setPValues] = useState<(string | number)[]>([]);
-class TTestTable extends Component<TTestTableProps, TTestTableState> {
- static defaultProps = defaultProps;
-
- constructor(props: TTestTableProps) {
- super(props);
- this.state = {
- control: 0,
- liftValues: [],
- pValues: [],
- };
- }
+ const computeLift = useCallback(
+ (values: DataPointValue[], controlValues: DataPointValue[]): string => {
+ // Compute the lift value between two time series
+ let sumValues = 0;
+ let sumControl = 0;
+ values.forEach((value, i) => {
+ sumValues += value.y;
+ sumControl += controlValues[i].y;
+ });
- componentDidMount() {
- const { control } = this.state;
- this.computeTTest(control); // initially populate table
- }
-
- getLiftStatus(row: number): string {
- const { control, liftValues } = this.state;
- // Get a css class name for coloring
- if (row === control) {
- return 'control';
- }
- const liftVal = liftValues[row];
- if (Number.isNaN(liftVal) || !Number.isFinite(liftVal)) {
- return 'invalid'; // infinite or NaN values
- }
-
- return Number(liftVal) >= 0 ? 'true' : 'false'; // green on true, red on
false
- }
+ // A zero control sum yields "Infinity" (or "NaN" for 0/0) via toFixed,
+ // matching the original class component's output
+ return (((sumValues - sumControl) / sumControl) * 100).toFixed(
+ liftValPrec,
+ );
+ },
+ [liftValPrec],
+ );
- getPValueStatus(row: number): string {
- const { control, pValues } = this.state;
- if (row === control) {
- return 'control';
- }
- const pVal = pValues[row];
- if (Number.isNaN(pVal) || !Number.isFinite(pVal)) {
- return 'invalid';
- }
+ const computePValue = useCallback(
+ (
+ values: DataPointValue[],
+ controlValues: DataPointValue[],
+ ): string | number => {
+ // Compute the p-value from Student's t-test
+ // between two time series
+ let diffSum = 0;
+ let diffSqSum = 0;
+ let finiteCount = 0;
+ values.forEach((value, i) => {
+ const diff = controlValues[i].y - value.y;
+ /* eslint-disable-next-line */
+ if (isFinite(diff)) {
+ finiteCount += 1;
+ diffSum += diff;
+ diffSqSum += diff * diff;
+ }
+ });
+ const tvalue = -Math.abs(
+ diffSum *
+ Math.sqrt(
+ (finiteCount - 1) / (finiteCount * diffSqSum - diffSum * diffSum),
+ ),
+ );
+ try {
+ return (2 * new dist.Studentt(finiteCount - 1).cdf(tvalue)).toFixed(
+ pValPrec,
+ ); // two-sided test
+ } catch (error) {
+ return NaN;
+ }
+ },
+ [pValPrec],
+ );
- return ''; // p-values won't normally be colored
- }
+ const computeTTest = useCallback(
+ (controlIndex: number) => {
+ // Compute lift and p-values for each row
+ // against the selected control
+ const newPValues: (string | number)[] = [];
+ const newLiftValues: (string | number)[] = [];
+ if (!data) {
+ return;
+ }
+ for (let i = 0; i < data.length; i += 1) {
+ if (i === controlIndex) {
+ newPValues.push('control');
+ newLiftValues.push('control');
+ } else {
+ newPValues.push(
+ computePValue(data[i].values, data[controlIndex].values),
+ );
+ newLiftValues.push(
+ computeLift(data[i].values, data[controlIndex].values),
+ );
+ }
+ }
+ setControl(controlIndex);
+ controlRef.current = controlIndex;
+ setLiftValues(newLiftValues);
+ setPValues(newPValues);
+ },
+ [data, computeLift, computePValue],
+ );
- getSignificance(row: number): string | boolean {
- const { control, pValues } = this.state;
- const { alpha } = this.props;
- // Color significant as green, else red
- if (row === control) {
- return 'control';
+ // Recompute table when data changes, keeping control index in range.
+ // Row clicks call computeTTest directly, so `control` is read via a ref
+ // here to avoid a duplicate recompute after each click.
+ useEffect(() => {
+ if (!data || data.length === 0) {
+ setControl(0);
+ controlRef.current = 0;
+ setLiftValues([]);
+ setPValues([]);
+ return;
}
- // p-values significant below set threshold
- return Number(pValues[row]) <= alpha;
- }
+ computeTTest(Math.min(controlRef.current, data.length - 1));
+ }, [computeTTest, data]);
- computeLift(values: DataPointValue[], control: DataPointValue[]): string {
- const { liftValPrec } = this.props;
- // Compute the lift value between two time series
- let sumValues = 0;
- let sumControl = 0;
- values.forEach((value, i) => {
- sumValues += value.y;
- sumControl += control[i].y;
- });
+ const getLiftStatus = useCallback(
+ (row: number): string => {
+ // Get a css class name for coloring
+ if (row === control) {
+ return 'control';
+ }
+ const liftVal = liftValues[row];
+ const numericLiftVal = Number(liftVal);
+ if (Number.isNaN(numericLiftVal) || !Number.isFinite(numericLiftVal)) {
+ return 'invalid'; // infinite or NaN values
+ }
- return (((sumValues - sumControl) / sumControl) *
100).toFixed(liftValPrec);
- }
+ return numericLiftVal >= 0 ? 'true' : 'false'; // green on true, red on
false
+ },
+ [control, liftValues],
+ );
- computePValue(
- values: DataPointValue[],
- control: DataPointValue[],
- ): string | number {
- const { pValPrec } = this.props;
- // Compute the p-value from Student's t-test
- // between two time series
- let diffSum = 0;
- let diffSqSum = 0;
- let finiteCount = 0;
- values.forEach((value, i) => {
- const diff = control[i].y - value.y;
- /* eslint-disable-next-line */
- if (isFinite(diff)) {
- finiteCount += 1;
- diffSum += diff;
- diffSqSum += diff * diff;
+ const getPValueStatus = useCallback(
+ (row: number): string => {
+ if (row === control) {
+ return 'control';
+ }
+ const pVal = pValues[row];
+ const numericPVal = Number(pVal);
+ if (Number.isNaN(numericPVal) || !Number.isFinite(numericPVal)) {
+ return 'invalid';
}
- });
- const tvalue = -Math.abs(
- diffSum *
- Math.sqrt(
- (finiteCount - 1) / (finiteCount * diffSqSum - diffSum * diffSum),
- ),
- );
- try {
- return (2 * new dist.Studentt(finiteCount - 1).cdf(tvalue)).toFixed(
- pValPrec,
- ); // two-sided test
- } catch (error) {
- return NaN;
- }
- }
- computeTTest(control: number) {
- // Compute lift and p-values for each row
- // against the selected control
- const { data } = this.props;
- const pValues: (string | number)[] = [];
- const liftValues: (string | number)[] = [];
- if (!data) {
- return;
- }
- for (let i = 0; i < data.length; i += 1) {
- if (i === control) {
- pValues.push('control');
- liftValues.push('control');
- } else {
- pValues.push(this.computePValue(data[i].values, data[control].values));
- liftValues.push(this.computeLift(data[i].values,
data[control].values));
+ return ''; // p-values won't normally be colored
+ },
+ [control, pValues],
+ );
+
+ const getSignificance = useCallback(
+ (row: number): string | boolean => {
+ // Color significant as green, else red
+ if (row === control) {
+ return 'control';
}
- }
- this.setState({ control, liftValues, pValues });
- }
- render() {
- const { data, metric, groups } = this.props;
- const { control, liftValues, pValues } = this.state;
+ // p-values significant below set threshold
+ return Number(pValues[row]) <= alpha;
+ },
+ [control, pValues, alpha],
+ );
- if (!Array.isArray(groups) || groups.length === 0) {
- throw new Error('Group by param is required');
- }
+ const handleRowClick = useCallback(
+ (rowIndex: number) => {
+ computeTTest(rowIndex);
+ },
+ [computeTTest],
+ );
- // Render column header for each group
- const columns = groups.map((group, i) => (
- <Th key={i} column={group}>
- {group}
- </Th>
- ));
- const numGroups = groups.length;
- // Columns for p-value, lift-value, and significance (true/false)
- columns.push(
- <Th key={numGroups + 1} column="pValue">
- p-value
- </Th>,
- );
- columns.push(
- <Th key={numGroups + 2} column="liftValue">
- Lift %
- </Th>,
- );
- columns.push(
- <Th key={numGroups + 3} column="significant">
- Significant
- </Th>,
- );
- const rows = data.map((entry, i) => {
- const values = groups.map(
- (
- group,
- j, // group names
- ) => <Td key={j} column={group} data={entry.group[j]} />,
- );
- values.push(
- <Td
- key={numGroups + 1}
- className={this.getPValueStatus(i)}
- column="pValue"
- data={pValues[i]}
- />,
- );
- values.push(
- <Td
- key={numGroups + 2}
- className={this.getLiftStatus(i)}
- column="liftValue"
- data={liftValues[i]}
- />,
- );
- values.push(
- <Td
- key={numGroups + 3}
- className={this.getSignificance(i).toString()}
- column="significant"
- data={this.getSignificance(i)}
- />,
- );
+ // When sorted ascending, 'control' will always be at top
+ type SortConfigItem =
+ string | { column: string; sortFunction: (a: string, b: string) => number
};
- return (
- <Tr
- key={i}
- className={i === control ? 'control' : ''}
- onClick={this.computeTTest.bind(this, i)}
- >
- {values}
- </Tr>
- );
- });
- // When sorted ascending, 'control' will always be at top
- type SortConfigItem =
- | string
- | { column: string; sortFunction: (a: string, b: string) => number };
- const sortConfig: SortConfigItem[] = (groups as SortConfigItem[]).concat([
- {
- column: 'pValue',
- sortFunction: (a: string, b: string) => {
- if (a === 'control') {
- return -1;
- }
- if (b === 'control') {
- return 1;
- }
+ const sortConfig: SortConfigItem[] = useMemo(
+ () =>
+ (groups as SortConfigItem[]).concat([
+ {
+ column: 'pValue',
+ sortFunction: (a: string, b: string) => {
+ if (a === 'control') {
+ return -1;
+ }
+ if (b === 'control') {
+ return 1;
+ }
+ if (a === b) {
+ return 0;
+ }
- return a > b ? 1 : -1; // p-values ascending
+ return a > b ? 1 : -1; // p-values ascending
+ },
},
- },
- {
- column: 'liftValue',
- sortFunction: (a: string, b: string) => {
- if (a === 'control') {
- return -1;
- }
- if (b === 'control') {
- return 1;
- }
+ {
+ column: 'liftValue',
+ sortFunction: (a: string, b: string) => {
+ if (a === 'control') {
+ return -1;
+ }
+ if (b === 'control') {
+ return 1;
+ }
- return parseFloat(a) > parseFloat(b) ? -1 : 1; // lift values
descending
+ const liftA = parseFloat(a);
+ const liftB = parseFloat(b);
+ const aFinite = Number.isFinite(liftA);
+ const bFinite = Number.isFinite(liftB);
+ // Non-finite (Infinity/NaN) lift values sort ahead of finite ones,
+ // consistently regardless of comparison order, to avoid an
+ // antisymmetric comparator
+ if (!aFinite && !bFinite) {
+ return 0;
+ }
+ if (!aFinite) {
+ return -1;
+ }
+ if (!bFinite) {
+ return 1;
+ }
+ if (liftA === liftB) {
+ return 0;
+ }
+
+ return liftA > liftB ? -1 : 1; // lift values descending
+ },
},
- },
- {
- column: 'significant',
- sortFunction: (a: string, b: string) => {
- if (a === 'control') {
- return -1;
- }
- if (b === 'control') {
- return 1;
- }
+ {
+ column: 'significant',
+ sortFunction: (a: string, b: string) => {
+ if (a === 'control') {
+ return -1;
+ }
+ if (b === 'control') {
+ return 1;
+ }
+ if (a === b) {
+ return 0;
+ }
- return a > b ? -1 : 1; // significant values first
+ return a > b ? -1 : 1; // significant values first
+ },
},
- },
- ]);
+ ]),
+ [groups],
+ );
+
+ if (!Array.isArray(groups) || groups.length === 0) {
+ throw new Error('Group by param is required');
+ }
+
+ // Render column header for each group
+ const columns = groups.map((group, i) => (
+ <Th key={i} column={group}>
+ {group}
+ </Th>
+ ));
+ const numGroups = groups.length;
+ // Columns for p-value, lift-value, and significance (true/false)
+ columns.push(
+ <Th key={numGroups + 1} column="pValue">
+ p-value
+ </Th>,
+ );
+ columns.push(
+ <Th key={numGroups + 2} column="liftValue">
+ Lift %
+ </Th>,
+ );
+ columns.push(
+ <Th key={numGroups + 3} column="significant">
+ Significant
+ </Th>,
+ );
+
+ const rows = (data ?? []).map((entry, i) => {
+ const values = groups.map(
+ (
+ group,
+ j, // group names
+ ) => <Td key={j} column={group} data={entry.group[j]} />,
+ );
+ values.push(
+ <Td
+ key={numGroups + 1}
+ className={getPValueStatus(i)}
+ column="pValue"
+ data={pValues[i]}
+ />,
+ );
+ values.push(
+ <Td
+ key={numGroups + 2}
+ className={getLiftStatus(i)}
+ column="liftValue"
+ data={liftValues[i]}
+ />,
+ );
+ values.push(
+ <Td
+ key={numGroups + 3}
+ className={getSignificance(i).toString()}
+ column="significant"
+ data={getSignificance(i)}
+ />,
+ );
return (
- <div>
- <h3>{metric}</h3>
- <Table className="table" id={`table_${metric}`} sortable={sortConfig}>
- <Thead>{columns}</Thead>
- {rows}
- </Table>
- </div>
+ <Tr
+ key={i}
+ className={i === control ? 'control' : ''}
+ onClick={() => handleRowClick(i)}
+ >
+ {values}
+ </Tr>
);
- }
+ });
+
+ return (
+ <div>
+ <h3>{metric}</h3>
+ <Table className="table" id={`table_${metric}`} sortable={sortConfig}>
+ <Thead>{columns}</Thead>
+ {rows}
+ </Table>
+ </div>
+ );
}
export default TTestTable;
diff --git
a/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/test/TTestTable.test.tsx
b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/test/TTestTable.test.tsx
new file mode 100644
index 00000000000..b8e6dd725e1
--- /dev/null
+++
b/superset-frontend/plugins/legacy-plugin-chart-paired-t-test/test/TTestTable.test.tsx
@@ -0,0 +1,365 @@
+/**
+ * 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 { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import TTestTable from '../src/TTestTable';
+import type { DataEntry } from '../src/TTestTable';
+
+// Mock the distributions module to return a predictable cdf value.
+// cdf returns 0.01 so that p-value = 2 * 0.01 = 0.02
+jest.mock('distributions', () => {
+ class MockStudentt {
+ cdf(_x: number): number {
+ return 0.01;
+ }
+ }
+ return {
+ __esModule: true,
+ default: { Studentt: MockStudentt },
+ };
+});
+
+const mockData: DataEntry[] = [
+ {
+ group: ['group-A'],
+ values: [
+ { x: 1, y: 10 },
+ { x: 2, y: 20 },
+ ],
+ },
+ {
+ group: ['group-B'],
+ values: [
+ { x: 1, y: 15 },
+ { x: 2, y: 25 },
+ ],
+ },
+];
+
+const defaultProps = {
+ alpha: 0.05,
+ data: mockData,
+ groups: ['category'],
+ liftValPrec: 4,
+ metric: 'revenue',
+ pValPrec: 6,
+};
+
+test('renders the metric name as an h3 heading', async () => {
+ render(<TTestTable {...defaultProps} />);
+
+ await waitFor(() => {
+ expect(screen.getByText('revenue')).toBeInTheDocument();
+ });
+
+ const heading = screen.getByText('revenue');
+ expect(heading.tagName).toBe('H3');
+});
+
+test('renders a table with the correct column headers', async () => {
+ render(<TTestTable {...defaultProps} />);
+
+ await waitFor(() => {
+ expect(screen.getByRole('table')).toBeInTheDocument();
+ });
+
+ expect(screen.getByText('category')).toBeInTheDocument();
+ expect(screen.getByText('p-value')).toBeInTheDocument();
+ expect(screen.getByText('Lift %')).toBeInTheDocument();
+ expect(screen.getByText('Significant')).toBeInTheDocument();
+});
+
+test('renders group columns matching the groups prop', async () => {
+ const multiGroupData: DataEntry[] = [
+ {
+ group: ['group-A', 'sub-1'],
+ values: [{ x: 1, y: 10 }],
+ },
+ {
+ group: ['group-B', 'sub-2'],
+ values: [{ x: 1, y: 15 }],
+ },
+ ];
+
+ render(
+ <TTestTable
+ {...defaultProps}
+ groups={['category', 'subcategory']}
+ data={multiGroupData}
+ />,
+ );
+
+ await waitFor(() => {
+ expect(screen.getByRole('table')).toBeInTheDocument();
+ });
+
+ expect(screen.getByText('category')).toBeInTheDocument();
+ expect(screen.getByText('subcategory')).toBeInTheDocument();
+});
+
+test('first row is treated as control by default and shows "control" for
p-value and lift columns', async () => {
+ render(<TTestTable {...defaultProps} />);
+
+ // After componentDidMount, the first row should be control
+ await waitFor(() => {
+ const controlTexts = screen.getAllByText('control');
+ // The control row has "control" in pValue and liftValue columns
+ expect(controlTexts.length).toBeGreaterThanOrEqual(2);
+ });
+});
+
+test('computes lift values correctly for non-control rows', async () => {
+ // Control (group-A): sum of y = 10 + 20 = 30
+ // group-B: sum of y = 15 + 25 = 40
+ // Lift = ((40 - 30) / 30) * 100 = 33.3333%
+ // With liftValPrec=4 => "33.3333"
+ render(<TTestTable {...defaultProps} />);
+
+ await waitFor(() => {
+ expect(screen.getByText('33.3333')).toBeInTheDocument();
+ });
+});
+
+test('computes p-value using the mocked distributions module', async () => {
+ // Mock cdf returns 0.01, so p-value = 2 * 0.01 = 0.02
+ // With pValPrec=6 => "0.020000"
+ render(<TTestTable {...defaultProps} />);
+
+ await waitFor(() => {
+ expect(screen.getByText('0.020000')).toBeInTheDocument();
+ });
+});
+
+test('marks non-control row as significant when p-value is below alpha', async
() => {
+ // p-value = 0.02 < alpha = 0.05, so significance is true
+ render(<TTestTable {...defaultProps} />);
+
+ await waitFor(() => {
+ expect(screen.getByText('true')).toBeInTheDocument();
+ });
+});
+
+test('marks non-control row as not significant when p-value is above alpha',
async () => {
+ // p-value = 0.02 > alpha = 0.01, so significance is false
+ render(<TTestTable {...defaultProps} alpha={0.01} />);
+
+ await waitFor(() => {
+ expect(screen.getByText('false')).toBeInTheDocument();
+ });
+});
+
+test('renders Infinity lift when control values sum to zero', async () => {
+ const zeroControlData: DataEntry[] = [
+ {
+ group: ['zero-group'],
+ values: [
+ { x: 1, y: 0 },
+ { x: 2, y: 0 },
+ ],
+ },
+ {
+ group: ['other-group'],
+ values: [
+ { x: 1, y: 10 },
+ { x: 2, y: 20 },
+ ],
+ },
+ ];
+
+ render(<TTestTable {...defaultProps} data={zeroControlData} />);
+
+ // The lift computation: ((sumValues - sumControl) / sumControl) * 100
+ // = ((30 - 0) / 0) * 100 = Infinity
+ // Infinity.toFixed(4) returns "Infinity", and the component renders it.
+ // getLiftStatus classifies this as "invalid" (non-finite).
+ await waitFor(() => {
+ expect(screen.getByText('Infinity')).toBeInTheDocument();
+ });
+ expect(screen.getByText('Infinity')).toHaveClass('invalid');
+});
+
+test('colors finite lift cells with true/false status classes', async () => {
+ render(<TTestTable {...defaultProps} />);
+
+ // group-B lift vs control group-A: ((40 - 30) / 30) * 100 = 33.3333 (>= 0)
+ await waitFor(() => {
+ expect(screen.getByText('33.3333')).toBeInTheDocument();
+ });
+ expect(screen.getByText('33.3333')).toHaveClass('true');
+});
+
+test('recomputes and clamps the control index when data shrinks', async () => {
+ const threeRowData: DataEntry[] = [
+ { group: ['row-0'], values: [{ x: 1, y: 10 }] },
+ { group: ['row-1'], values: [{ x: 1, y: 20 }] },
+ { group: ['row-2'], values: [{ x: 1, y: 40 }] },
+ ];
+ const { rerender } = render(
+ <TTestTable {...defaultProps} data={threeRowData} />,
+ );
+
+ // Make the last row (index 2) the control
+ await waitFor(() => {
+ expect(screen.getByText('row-2')).toBeInTheDocument();
+ });
+ fireEvent.click(screen.getByText('row-2').closest('tr')!);
+
+ // row-0 lift vs row-2 control: ((10 - 40) / 40) * 100 = -75.0000
+ await waitFor(() => {
+ expect(screen.getByText('-75.0000')).toBeInTheDocument();
+ });
+
+ // Shrink the data so the control index (2) is out of range
+ rerender(<TTestTable {...defaultProps} data={threeRowData.slice(0, 2)} />);
+
+ // Control clamps to the last remaining row (index 1) and values recompute:
+ // row-0 lift vs row-1 control: ((10 - 20) / 20) * 100 = -50.0000
+ await waitFor(() => {
+ expect(screen.getByText('-50.0000')).toBeInTheDocument();
+ });
+ expect(screen.queryByText('row-2')).not.toBeInTheDocument();
+});
+
+test('sorts a non-finite lift value deterministically regardless of comparison
order', async () => {
+ // Control sum is non-zero (finite denominator), so row-normal gets a
+ // finite lift while row-huge overflows to a non-finite ("Infinity") lift,
+ // exercising a mixed finite/non-finite comparison.
+ const mixedLiftData: DataEntry[] = [
+ { group: ['control'], values: [{ x: 1, y: 100 }] },
+ { group: ['row-normal'], values: [{ x: 1, y: 200 }] },
+ { group: ['row-huge'], values: [{ x: 1, y: 1e309 }] },
+ ];
+
+ render(
+ <TTestTable {...defaultProps} data={mixedLiftData} groups={['group']} />,
+ );
+
+ await waitFor(() => {
+ expect(screen.getByText('Infinity')).toBeInTheDocument();
+ expect(screen.getByText('100.0000')).toBeInTheDocument();
+ });
+
+ fireEvent.click(screen.getByText('Lift %'));
+
+ await waitFor(() => {
+ const rowLabels = screen
+ .getAllByRole('row')
+ .slice(1) // drop the header row
+ .map(row => row.textContent);
+ // Sorting is antisymmetric and stable: the same relative order results
+ // whichever pair of rows is compared first, with the non-finite value
+ // consistently ordered ahead of the finite one.
+ expect(rowLabels).toEqual([
+ expect.stringContaining('control'),
+ expect.stringContaining('row-huge'),
+ expect.stringContaining('row-normal'),
+ ]);
+ });
+});
+
+test('throws an error when groups array is empty', () => {
+ // Suppress React error boundary console output for this test
+ const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+
+ expect(() => {
+ render(<TTestTable {...defaultProps} groups={[]} />);
+ }).toThrow('Group by param is required');
+
+ consoleSpy.mockRestore();
+});
+
+test('clicking a non-control row changes it to the new control', async () => {
+ render(<TTestTable {...defaultProps} />);
+
+ // Wait for initial render with group-A as control
+ await waitFor(() => {
+ expect(screen.getByText('group-A')).toBeInTheDocument();
+ expect(screen.getByText('group-B')).toBeInTheDocument();
+ });
+
+ // Initially group-A is control, so its row shows "control" in p-value and
lift columns.
+ // The non-control row (group-B) shows computed values.
+ await waitFor(() => {
+ expect(screen.getByText('33.3333')).toBeInTheDocument();
+ });
+
+ // Click the group-B row to make it the new control.
+ // The row containing "group-B" text is what we need to click.
+ const groupBCell = screen.getByText('group-B');
+ const groupBRow = groupBCell.closest('tr');
+ expect(groupBRow).not.toBeNull();
+ fireEvent.click(groupBRow!);
+
+ // After clicking, group-B becomes control.
+ // group-A lift: ((30 - 40) / 40) * 100 = -25.0000
+ await waitFor(() => {
+ expect(screen.getByText('-25.0000')).toBeInTheDocument();
+ });
+});
+
+test('renders group name data in the table cells', async () => {
+ render(<TTestTable {...defaultProps} />);
+
+ await waitFor(() => {
+ expect(screen.getByText('group-A')).toBeInTheDocument();
+ expect(screen.getByText('group-B')).toBeInTheDocument();
+ });
+});
+
+test('renders with three data rows and computes values for each non-control
row', async () => {
+ const threeRowData: DataEntry[] = [
+ {
+ group: ['control-group'],
+ values: [
+ { x: 1, y: 10 },
+ { x: 2, y: 10 },
+ ],
+ },
+ {
+ group: ['test-group-1'],
+ values: [
+ { x: 1, y: 15 },
+ { x: 2, y: 15 },
+ ],
+ },
+ {
+ group: ['test-group-2'],
+ values: [
+ { x: 1, y: 20 },
+ { x: 2, y: 20 },
+ ],
+ },
+ ];
+
+ render(<TTestTable {...defaultProps} data={threeRowData} />);
+
+ await waitFor(() => {
+ expect(screen.getByText('control-group')).toBeInTheDocument();
+ expect(screen.getByText('test-group-1')).toBeInTheDocument();
+ expect(screen.getByText('test-group-2')).toBeInTheDocument();
+ });
+
+ // control-group: sum = 20
+ // test-group-1: sum = 30, lift = ((30-20)/20)*100 = 50.0000
+ // test-group-2: sum = 40, lift = ((40-20)/20)*100 = 100.0000
+ await waitFor(() => {
+ expect(screen.getByText('50.0000')).toBeInTheDocument();
+ expect(screen.getByText('100.0000')).toBeInTheDocument();
+ });
+});
diff --git
a/superset-frontend/plugins/plugin-chart-word-cloud/src/chart/WordCloud.tsx
b/superset-frontend/plugins/plugin-chart-word-cloud/src/chart/WordCloud.tsx
index 58f831fca4a..c4f77ffb643 100644
--- a/superset-frontend/plugins/plugin-chart-word-cloud/src/chart/WordCloud.tsx
+++ b/superset-frontend/plugins/plugin-chart-word-cloud/src/chart/WordCloud.tsx
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { PureComponent } from 'react';
+import { memo, useCallback, useEffect, useMemo, useRef, useState } from
'react';
import cloudLayout from 'd3-cloud';
import { scaleLinear } from 'd3-scale';
import { seed, CategoricalColorNamespace } from '@superset-ui/core';
@@ -81,18 +81,7 @@ export interface WordCloudProps extends WordCloudVisualProps
{
colorScheme: string;
}
-export interface WordCloudState {
- words: Word[];
- scaleFactor: number;
-}
-
-const defaultProps: Required<WordCloudVisualProps> = {
- encoding: {},
- rotation: 'flat',
-};
-
-type FullWordCloudProps = WordCloudProps &
- typeof defaultProps & { theme: SupersetTheme };
+type FullWordCloudProps = WordCloudProps & { theme: SupersetTheme };
const SCALE_FACTOR_STEP = 0.5;
const MAX_SCALE_FACTOR = 3;
@@ -196,61 +185,86 @@ class SimpleEncoder {
}
}
-class WordCloud extends PureComponent<FullWordCloudProps, WordCloudState> {
- static defaultProps = defaultProps;
-
- isComponentMounted = false;
-
- createEncoder = (encoding?: Partial<WordCloudEncoding>): SimpleEncoder =>
- new SimpleEncoder(encoding ?? {}, {
- color: this.props.theme.colorTextLabel,
- fontFamily: this.props.theme.fontFamily,
- fontSize: 20,
- fontWeight: 'bold',
- text: '',
- });
-
- constructor(props: FullWordCloudProps) {
- super(props);
- this.state = {
- words: [],
- scaleFactor: 1,
- };
- this.setWords = this.setWords.bind(this);
- }
-
- componentDidMount() {
- this.isComponentMounted = true;
- this.update();
- }
-
- componentDidUpdate(prevProps: WordCloudProps) {
- const { data, encoding, width, height, rotation } = this.props;
- if (
- !isEqual(prevProps.data, data) ||
- !isEqual(prevProps.encoding, encoding) ||
- prevProps.width !== width ||
- prevProps.height !== height ||
- prevProps.rotation !== rotation
- ) {
- this.update();
- }
- }
-
- componentWillUnmount() {
- this.isComponentMounted = false;
- }
-
- setWords(words: Word[]) {
- if (this.isComponentMounted) {
- this.setState({ words });
- }
- }
-
- update() {
- const { data, encoding } = this.props;
+function WordCloud({
+ data,
+ encoding = {},
+ width,
+ height,
+ rotation = 'flat',
+ sliceId,
+ colorScheme,
+ theme,
+}: FullWordCloudProps) {
+ const [words, setWords] = useState<Word[]>([]);
+ const [scaleFactor, setScaleFactor] = useState(1);
+ const isMountedRef = useRef(true);
+
+ // Store previous props for comparison
+ const prevPropsRef = useRef<{
+ data: PlainObject[];
+ encoding: Partial<WordCloudEncoding>;
+ width: number;
+ height: number;
+ rotation: RotationType;
+ } | null>(null);
+
+ const createEncoder = useCallback(
+ (enc?: Partial<WordCloudEncoding>): SimpleEncoder =>
+ new SimpleEncoder(enc ?? {}, {
+ color: theme.colorTextLabel,
+ fontFamily: theme.fontFamily,
+ fontSize: 20,
+ fontWeight: 'bold',
+ text: '',
+ }),
+ [theme.colorTextLabel, theme.fontFamily],
+ );
+
+ const commitLayoutIfMounted = useCallback(
+ (newWords: Word[], newScaleFactor: number) => {
+ if (isMountedRef.current) {
+ setWords(newWords);
+ // Persist the accepted scale factor so the SVG viewBox matches the
+ // canvas the layout was computed for (otherwise enlarged layouts clip)
+ setScaleFactor(newScaleFactor);
+ }
+ },
+ [],
+ );
+
+ const generateCloud = useCallback(
+ (
+ encoder: SimpleEncoder,
+ currentScaleFactor: number,
+ isValid: (word: Word[]) => boolean,
+ ) => {
+ cloudLayout()
+ .size([width * currentScaleFactor, height * currentScaleFactor])
+ .words(data.map((d: Word) => ({ ...d })))
+ .padding(5)
+ .rotate(ROTATION[rotation] || ROTATION.flat)
+ .text((d: PlainObject) => encoder.getText(d))
+ .font((d: PlainObject) => encoder.getFontFamily(d))
+ .fontWeight((d: PlainObject) => encoder.getFontWeight(d))
+ .fontSize((d: PlainObject) => encoder.getFontSize(d))
+ .on('end', (cloudWords: Word[]) => {
+ if (isValid(cloudWords) || currentScaleFactor > MAX_SCALE_FACTOR) {
+ commitLayoutIfMounted(cloudWords, currentScaleFactor);
+ } else {
+ generateCloud(
+ encoder,
+ currentScaleFactor + SCALE_FACTOR_STEP,
+ isValid,
+ );
+ }
+ })
+ .start();
+ },
+ [data, width, height, rotation, commitLayoutIfMounted],
+ );
- const encoder = this.createEncoder(encoding);
+ const update = useCallback(() => {
+ const encoder = createEncoder(encoding);
encoder.setDomainFromDataset(data);
const sortedData = [...data].sort(
@@ -262,73 +276,72 @@ class WordCloud extends PureComponent<FullWordCloudProps,
WordCloudState> {
);
const topResults = sortedData.slice(0, topResultsCount);
- this.generateCloud(encoder, 1, (words: Word[]) =>
+ generateCloud(encoder, 1, (cloudWords: Word[]) =>
topResults.every((d: PlainObject) =>
- words.find(({ text }) => encoder.getText(d) === text),
+ cloudWords.find(({ text }) => encoder.getText(d) === text),
),
);
- }
+ }, [data, encoding, createEncoder, generateCloud]);
- generateCloud(
- encoder: SimpleEncoder,
- scaleFactor: number,
- isValid: (word: Word[]) => boolean,
- ) {
- const { data, width, height, rotation } = this.props;
-
- cloudLayout()
- .size([width * scaleFactor, height * scaleFactor])
- .words(data.map((d: Word) => ({ ...d })))
- .padding(5)
- .rotate(ROTATION[rotation] || ROTATION.flat)
- .text((d: PlainObject) => encoder.getText(d))
- .font((d: PlainObject) => encoder.getFontFamily(d))
- .fontWeight((d: PlainObject) => encoder.getFontWeight(d))
- .fontSize((d: PlainObject) => encoder.getFontSize(d))
- .on('end', (words: Word[]) => {
- if (isValid(words) || scaleFactor > MAX_SCALE_FACTOR) {
- this.setWords(words);
- } else {
- this.generateCloud(encoder, scaleFactor + SCALE_FACTOR_STEP,
isValid);
- }
- })
- .start();
- }
+ // Component mount/unmount tracking
+ useEffect(() => {
+ isMountedRef.current = true;
+ return () => {
+ isMountedRef.current = false;
+ };
+ }, []);
- render() {
- const { scaleFactor, words } = this.state;
- const { width, height, encoding, sliceId, colorScheme } = this.props;
-
- const encoder = this.createEncoder(encoding);
-
- const colorFn = CategoricalColorNamespace.getScale(colorScheme);
- const viewBoxWidth = width * scaleFactor;
- const viewBoxHeight = height * scaleFactor;
-
- return (
- <svg
- width={width}
- height={height}
- viewBox={`-${viewBoxWidth / 2} -${viewBoxHeight / 2} ${viewBoxWidth}
${viewBoxHeight}`}
- >
- <g>
- {words.map(w => (
- <text
- key={w.text}
- fontSize={`${w.size}px`}
- fontWeight={w.weight}
- fontFamily={w.font}
- fill={colorFn(encoder.getColor(w as PlainObject), sliceId)}
- textAnchor="middle"
- transform={`translate(${w.x}, ${w.y}) rotate(${w.rotate})`}
- >
- {w.text}
- </text>
- ))}
- </g>
- </svg>
- );
- }
+ // Initial update on mount and when dependencies change
+ useEffect(() => {
+ const prevProps = prevPropsRef.current;
+ const shouldUpdate =
+ !prevProps ||
+ !isEqual(prevProps.data, data) ||
+ !isEqual(prevProps.encoding, encoding) ||
+ prevProps.width !== width ||
+ prevProps.height !== height ||
+ prevProps.rotation !== rotation;
+
+ if (shouldUpdate) {
+ update();
+ }
+
+ prevPropsRef.current = { data, encoding, width, height, rotation };
+ }, [data, encoding, width, height, rotation, update]);
+
+ const encoder = useMemo(
+ () => createEncoder(encoding),
+ [createEncoder, encoding],
+ );
+
+ const colorFn = CategoricalColorNamespace.getScale(colorScheme);
+ const viewBoxWidth = width * scaleFactor;
+ const viewBoxHeight = height * scaleFactor;
+
+ return (
+ <svg
+ width={width}
+ height={height}
+ viewBox={`-${viewBoxWidth / 2} -${viewBoxHeight / 2} ${viewBoxWidth}
${viewBoxHeight}`}
+ >
+ <g>
+ {words.map(w => (
+ <text
+ key={w.text}
+ fontSize={`${w.size}px`}
+ fontWeight={w.weight}
+ fontFamily={w.font}
+ fill={colorFn(encoder.getColor(w as PlainObject), sliceId)}
+ textAnchor="middle"
+ transform={`translate(${w.x}, ${w.y}) rotate(${w.rotate})`}
+ >
+ {w.text}
+ </text>
+ ))}
+ </g>
+ </svg>
+ );
}
-export default withTheme(WordCloud);
+// memo preserves the shallow-prop render bailout of the PureComponent original
+export default withTheme(memo(WordCloud));
diff --git
a/superset-frontend/plugins/plugin-chart-word-cloud/test/WordCloud.test.tsx
b/superset-frontend/plugins/plugin-chart-word-cloud/test/WordCloud.test.tsx
new file mode 100644
index 00000000000..4217c57813e
--- /dev/null
+++ b/superset-frontend/plugins/plugin-chart-word-cloud/test/WordCloud.test.tsx
@@ -0,0 +1,161 @@
+/**
+ * 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 { render, screen } from 'spec/helpers/testing-library';
+import WordCloud from '../src/chart/WordCloud';
+import type { PlainObject, Word } from '../src/chart/WordCloud';
+
+// Shared state for the d3-cloud mock; `mock` prefix allows use inside
+// the hoisted jest.mock factory
+const mockCloudState = {
+ sizes: [] as Array<[number, number]>,
+ // When true, the first layout run returns an incomplete word set,
+ // forcing the component to retry at a larger scale factor
+ dropFirstLayout: false,
+};
+
+jest.mock('d3-cloud', () =>
+ jest.fn(() => {
+ const config: {
+ size?: [number, number];
+ words?: Word[];
+ text?: (d: PlainObject) => string;
+ end?: (words: Word[]) => void;
+ } = {};
+ const layout = {
+ size(s: [number, number]) {
+ config.size = s;
+ return layout;
+ },
+ words(w: Word[]) {
+ config.words = w;
+ return layout;
+ },
+ padding() {
+ return layout;
+ },
+ rotate() {
+ return layout;
+ },
+ text(fn: (d: PlainObject) => string) {
+ config.text = fn;
+ return layout;
+ },
+ font() {
+ return layout;
+ },
+ fontWeight() {
+ return layout;
+ },
+ fontSize() {
+ return layout;
+ },
+ on(_event: string, cb: (words: Word[]) => void) {
+ config.end = cb;
+ return layout;
+ },
+ start() {
+ mockCloudState.sizes.push(config.size ?? [0, 0]);
+ const out = (config.words ?? []).map((w, i) => ({
+ ...w,
+ text: config.text?.(w as PlainObject) ?? '',
+ size: 20,
+ x: i * 10,
+ y: i * 10,
+ rotate: 0,
+ font: 'Arial',
+ weight: 'bold',
+ }));
+ const isFirstRun = mockCloudState.sizes.length === 1;
+ config.end?.(
+ mockCloudState.dropFirstLayout && isFirstRun ? out.slice(0, 1) : out,
+ );
+ return layout;
+ },
+ };
+ return layout;
+ }),
+);
+
+beforeEach(() => {
+ mockCloudState.sizes = [];
+ mockCloudState.dropFirstLayout = false;
+});
+
+const defaultProps = {
+ data: [{ text: 'alpha' }, { text: 'beta' }, { text: 'gamma' }],
+ encoding: { text: { field: 'text' } },
+ width: 200,
+ height: 100,
+ sliceId: 1,
+ colorScheme: 'supersetColors',
+};
+
+test('renders a text element for each word', () => {
+ render(<WordCloud {...defaultProps} />);
+
+ expect(screen.getByText('alpha')).toBeInTheDocument();
+ expect(screen.getByText('beta')).toBeInTheDocument();
+ expect(screen.getByText('gamma')).toBeInTheDocument();
+});
+
+test('uses a 1x viewBox when the first layout fits all top words', () => {
+ const { container } = render(<WordCloud {...defaultProps} />);
+
+ expect(mockCloudState.sizes).toEqual([[200, 100]]);
+ expect(container.querySelector('svg')).toHaveAttribute(
+ 'viewBox',
+ '-100 -50 200 100',
+ );
+});
+
+test('retries at a larger scale factor and matches the viewBox to it', () => {
+ mockCloudState.dropFirstLayout = true;
+
+ const { container } = render(<WordCloud {...defaultProps} />);
+
+ // First run at 1x drops words, so the layout escalates to 1.5x
+ expect(mockCloudState.sizes).toEqual([
+ [200, 100],
+ [300, 150],
+ ]);
+ expect(container.querySelector('svg')).toHaveAttribute(
+ 'viewBox',
+ '-150 -75 300 150',
+ );
+});
+
+test('recomputes the layout when data changes', () => {
+ const { rerender } = render(<WordCloud {...defaultProps} />);
+ expect(mockCloudState.sizes).toHaveLength(1);
+
+ rerender(<WordCloud {...defaultProps} data={[{ text: 'delta' }]} />);
+
+ expect(mockCloudState.sizes).toHaveLength(2);
+ expect(screen.getByText('delta')).toBeInTheDocument();
+ expect(screen.queryByText('alpha')).not.toBeInTheDocument();
+});
+
+test('does not recompute the layout for an unchanged rerender', () => {
+ const { rerender } = render(<WordCloud {...defaultProps} />);
+ expect(mockCloudState.sizes).toHaveLength(1);
+
+ rerender(<WordCloud {...defaultProps} />);
+
+ expect(mockCloudState.sizes).toHaveLength(1);
+});
