fix: make lift-column sort comparator deterministic for non-finite values

Guard the liftValue sortFunction against Infinity/NaN so a non-finite
lift always sorts consistently ahead of a finite one, regardless of
comparison order, instead of both (x, nonFinite) and (nonFinite, x)
returning the same result.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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);

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,
-};
+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);
 
-class HorizonRow extends PureComponent<HorizonRowProps> {
-  static defaultProps = defaultProps;
+  const drawChart = useCallback(() => {
+    const canvas = canvasRef.current;
+    if (!canvas) return;
 
-  private canvas: HTMLCanvasElement | null = null;
+    const data =
+      colorScale === 'change' && rawData.length > 0
+        ? rawData.map(d => ({ ...d, y: d.y - rawData[0].y }))
+        : rawData;
 
-  componentDidMount() {
-    this.drawChart();
-  }
+    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);
 
-  componentDidUpdate() {
-    this.drawChart();
-  }
+    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),
+    );
 
-  componentWillUnmount() {
-    this.canvas = null;
-  }
+    // skip drawing if there's no data to be drawn
+    if (startIndex > data.length) {
+      return;
+    }
 
-  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;
+    // 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]);
 
-      const data =
-        colorScale === 'change'
-          ? rawData.map(d => ({ ...d, y: d.y - rawData[0].y }))
-          : rawData;
+    // 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];
 
-      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);
+      // Adjust the range based on the current band index.
+      bExtents = (b + 1 - bands) * height;
+      y.range([bands * height + bExtents, bExtents]);
 
-      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),
-      );
+      // 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)!,
+          );
+        }
+      }
+    }
 
-      // skip drawing if there's no data to be drawn
-      if (startIndex > data.length) {
-        return;
+    // 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)!,
+          );
         }
       }
     }
-  }
+  }, [
+    rawData,
+    yDomain,
+    width,
+    height,
+    bands,
+    colors,
+    colorScale,
+    offsetX,
+    mode,
+  ]);
 
-  render() {
-    const { className, title, width, height } = this.props;
+  useEffect(() => {
+    drawChart();
+  }, [drawChart]);
 
-    return (
-      <div className={`horizon-row ${className}`}>
-        <span className="title">{title}</span>
-        <canvas
-          ref={c => {
-            this.canvas = c;
-          }}
-          width={width}
-          height={height}
-        />
-      </div>
-    );
-  }
+  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);

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);
+});

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);
+});

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);

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)[];
-}
+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)[]>([]);
 
-const defaultProps = {
-  alpha: 0.05,
-  liftValPrec: 4,
-  pValPrec: 6,
-};
+  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;
+      });
 
-class TTestTable extends Component<TTestTableProps, TTestTableState> {
-  static defaultProps = defaultProps;
+      // 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],
+  );
 
-  constructor(props: TTestTableProps) {
-    super(props);
-    this.state = {
-      control: 0,
-      liftValues: [],
-      pValues: [],
-    };
-  }
-
-  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
-  }
-
-  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';
-    }
-
-    return ''; // p-values won't normally be colored
-  }
-
-  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';
-    }
-
-    // p-values significant below set threshold
-    return Number(pValues[row]) <= alpha;
-  }
-
-  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;
-    });
-
-    return (((sumValues - sumControl) / sumControl) * 100).toFixed(liftValPrec);
-  }
-
-  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 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;
       }
-    });
-    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],
+  );
 
-  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) {
+  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],
+  );
+
+  // 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;
     }
-    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));
+
+    computeTTest(Math.min(controlRef.current, data.length - 1));
+  }, [computeTTest, data]);
+
+  const getLiftStatus = useCallback(
+    (row: number): string => {
+      // Get a css class name for coloring
+      if (row === control) {
+        return 'control';
       }
-    }
-    this.setState({ control, liftValues, pValues });
+      const liftVal = liftValues[row];
+      const numericLiftVal = Number(liftVal);
+      if (Number.isNaN(numericLiftVal) || !Number.isFinite(numericLiftVal)) {
+        return 'invalid'; // infinite or NaN values
+      }
+
+      return numericLiftVal >= 0 ? 'true' : 'false'; // green on true, red on false
+    },
+    [control, liftValues],
+  );
+
+  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';
+      }
+
+      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';
+      }
+
+      // p-values significant below set threshold
+      return Number(pValues[row]) <= alpha;
+    },
+    [control, pValues, alpha],
+  );
+
+  const handleRowClick = useCallback(
+    (rowIndex: number) => {
+      computeTTest(rowIndex);
+    },
+    [computeTTest],
+  );
+
+  // When sorted ascending, 'control' will always be at top
+  type SortConfigItem =
+    string | { column: string; sortFunction: (a: string, b: string) => number };
+
+  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
+          },
+        },
+        {
+          column: 'liftValue',
+          sortFunction: (a: string, b: string) => {
+            if (a === 'control') {
+              return -1;
+            }
+            if (b === 'control') {
+              return 1;
+            }
+
+            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;
+            }
+            if (a === b) {
+              return 0;
+            }
+
+            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() {
-    const { data, metric, groups } = this.props;
-    const { control, liftValues, pValues } = this.state;
+  // 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>,
+  );
 
-    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>,
+  const rows = (data ?? []).map((entry, i) => {
+    const values = groups.map(
+      (
+        group,
+        j, // group names
+      ) => <Td key={j} column={group} data={entry.group[j]} />,
     );
-    columns.push(
-      <Th key={numGroups + 2} column="liftValue">
-        Lift %
-      </Th>,
+    values.push(
+      <Td
+        key={numGroups + 1}
+        className={getPValueStatus(i)}
+        column="pValue"
+        data={pValues[i]}
+      />,
     );
-    columns.push(
-      <Th key={numGroups + 3} column="significant">
-        Significant
-      </Th>,
+    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)}
+      />,
     );
-    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)}
-        />,
-      );
-
-      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;
-          }
-
-          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;
-          }
-
-          return parseFloat(a) > parseFloat(b) ? -1 : 1; // lift values descending
-        },
-      },
-      {
-        column: 'significant',
-        sortFunction: (a: string, b: string) => {
-          if (a === 'control') {
-            return -1;
-          }
-          if (b === 'control') {
-            return 1;
-          }
-
-          return a > b ? -1 : 1; // significant values first
-        },
-      },
-    ]);
 
     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;

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();
+  });
+});

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;
+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);
 
-  isComponentMounted = false;
+  // Store previous props for comparison
+  const prevPropsRef = useRef<{
+    data: PlainObject[];
+    encoding: Partial<WordCloudEncoding>;
+    width: number;
+    height: number;
+    rotation: RotationType;
+  } | null>(null);
 
-  createEncoder = (encoding?: Partial<WordCloudEncoding>): SimpleEncoder =>
-    new SimpleEncoder(encoding ?? {}, {
-      color: this.props.theme.colorTextLabel,
-      fontFamily: this.props.theme.fontFamily,
-      fontSize: 20,
-      fontWeight: 'bold',
-      text: '',
-    });
+  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],
+  );
 
-  constructor(props: FullWordCloudProps) {
-    super(props);
-    this.state = {
-      words: [],
-      scaleFactor: 1,
-    };
-    this.setWords = this.setWords.bind(this);
-  }
+  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);
+      }
+    },
+    [],
+  );
 
-  componentDidMount() {
-    this.isComponentMounted = true;
-    this.update();
-  }
+  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],
+  );
 
-  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;
-
-    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;
+  // Component mount/unmount tracking
+  useEffect(() => {
+    isMountedRef.current = true;
+    return () => {
+      isMountedRef.current = false;
+    };
+  }, []);
 
-    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();
-  }
+  // 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;
 
-  render() {
-    const { scaleFactor, words } = this.state;
-    const { width, height, encoding, sliceId, colorScheme } = this.props;
+    if (shouldUpdate) {
+      update();
+    }
 
-    const encoder = this.createEncoder(encoding);
+    prevPropsRef.current = { data, encoding, width, height, rotation };
+  }, [data, encoding, width, height, rotation, update]);
 
-    const colorFn = CategoricalColorNamespace.getScale(colorScheme);
-    const viewBoxWidth = width * scaleFactor;
-    const viewBoxHeight = height * scaleFactor;
+  const encoder = useMemo(
+    () => createEncoder(encoding),
+    [createEncoder, encoding],
+  );
 
-    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>
-    );
-  }
+  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));

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);
+});