fix: subheader should show as subtitle (#33172)

superset-frontend/cypress-base/cypress/e2e/explore/visualizations/big_number.test.js

@@ -56,7 +56,6 @@ describe('Visualization > Big Number with Trendline', () => {
   it('should work', () => {
     verify(BIG_NUMBER_FORM_DATA);
     cy.get('.chart-container .header-line');
-    cy.get('.chart-container .subheader-line');
     cy.get('.chart-container canvas');
   });
 
@@ -66,7 +65,7 @@ describe('Visualization > Big Number with Trendline', () => {
       compare_lag: null,
     });
     cy.get('.chart-container .header-line');
-    cy.get('.chart-container .subheader-line').should('not.exist');
+    cy.get('.chart-container .subtitle-line').should('not.exist');
     cy.get('.chart-container canvas');
   });
 
@@ -76,7 +75,6 @@ describe('Visualization > Big Number with Trendline', () => {
       show_trend_line: false,
     });
     cy.get('[data-test="chart-container"] .header-line');
-    cy.get('[data-test="chart-container"] .subheader-line');
     cy.get('[data-test="chart-container"] canvas').should('not.exist');
   });
 });

superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.test.ts

@@ -0,0 +1,97 @@
+/**
+ * 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 { SqlaFormData } from '@superset-ui/core';
+import * as ChartControls from '@superset-ui/chart-controls';
+import controlPanel from './controlPanel';
+
+const { __mockShiftMetric } = ChartControls as any;
+
+jest.mock('@superset-ui/core', () => ({
+  GenericDataType: { Numeric: 'numeric' },
+  SMART_DATE_ID: 'SMART_DATE_ID',
+  t: (str: string) => str,
+}));
+
+jest.mock('@superset-ui/chart-controls', () => {
+  // Define the mock function inside the factory
+  const mockShiftMetric = jest.fn(() => 'shiftedMetric');
+  return {
+    ControlPanelConfig: {},
+    D3_FORMAT_DOCS: 'Format docs',
+    D3_TIME_FORMAT_OPTIONS: [['', 'default']],
+    getStandardizedControls: () => ({
+      shiftMetric: mockShiftMetric,
+    }),
+    // Optional export to let tests access the mock
+    __mockShiftMetric: mockShiftMetric,
+  };
+});
+
+describe('BigNumber Total Control Panel Config', () => {
+  it('should have the required control panel sections', () => {
+    expect(controlPanel).toHaveProperty('controlPanelSections');
+    const sections = controlPanel.controlPanelSections;
+    expect(Array.isArray(sections)).toBe(true);
+    expect(sections.length).toBe(2);
+
+    // First section should have label 'Query' and contain rows with metric and adhoc_filters
+    expect(sections[0]!.label).toBe('Query');
+    expect(Array.isArray(sections[0]!.controlSetRows)).toBe(true);
+    expect(sections[0]!.controlSetRows[0]).toEqual(['metric']);
+    expect(sections[0]!.controlSetRows[1]).toEqual(['adhoc_filters']);
+
+    // Second section should contain a control named subtitle
+    const secondSectionRow = sections[1]!.controlSetRows[1];
+    expect(secondSectionRow[0]).toHaveProperty('name', 'subtitle');
+
+    // Second section should include controls for time_format and conditional_formatting
+    const thirdSection = sections[1]!.controlSetRows;
+    // Check time_format control exists in one of the rows
+    const timeFormatRow = thirdSection.find(row =>
+      row.some((control: any) => control.name === 'time_format'),
+    );
+    expect(timeFormatRow).toBeTruthy();
+    // Check conditional_formatting control exists in one of the rows
+    const conditionalFormattingRow = thirdSection.find(row =>
+      row.some((control: any) => control.name === 'conditional_formatting'),
+    );
+    expect(conditionalFormattingRow).toBeTruthy();
+  });
+
+  it('should have y_axis_format override with correct label', () => {
+    expect(controlPanel).toHaveProperty('controlOverrides');
+    expect(controlPanel.controlOverrides).toHaveProperty('y_axis_format');
+    expect(controlPanel.controlOverrides!.y_axis_format!.label).toBe(
+      'Number format',
+    );
+  });
+
+  it('should override formData metric using getStandardizedControls', () => {
+    const dummyFormData = { someProp: 'test' } as unknown as SqlaFormData;
+    const newFormData = controlPanel.formDataOverrides!(dummyFormData);
+
+    // The original properties are spread correctly.
+    expect(newFormData.someProp).toBe('test');
+    // The metric property should be replaced by the output of shiftMetric.
+    expect(newFormData.metric).toBe('shiftedMetric');
+
+    // Ensure that the mockShiftMetric function was called.
+    expect(__mockShiftMetric).toHaveBeenCalled();
+  });
+});

superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.test.ts

@@ -0,0 +1,220 @@
+/**
+ * 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 { GenericDataType } from '@superset-ui/core';
+import { getColorFormatters } from '@superset-ui/chart-controls';
+import { BigNumberTotalChartProps } from '../types';
+import transformProps from './transformProps';
+
+jest.mock('@superset-ui/chart-controls', () => ({
+  getColorFormatters: jest.fn(),
+}));
+
+jest.mock('@superset-ui/core', () => ({
+  GenericDataType: { Temporal: 2, String: 1 },
+  getMetricLabel: jest.fn(metric => metric),
+  extractTimegrain: jest.fn(() => 'P1D'),
+  getValueFormatter: jest.fn(() => (v: any) => `$${v}`),
+}));
+
+jest.mock('../utils', () => ({
+  getDateFormatter: jest.fn(() => (v: any) => `${v}pm`),
+  parseMetricValue: jest.fn(val => Number(val)),
+}));
+
+describe('BigNumberTotal transformProps', () => {
+  const onContextMenu = jest.fn();
+  const baseFormData = {
+    headerFontSize: 20,
+    metric: 'value',
+    subheader: 'sub header text',
+    subheaderFontSize: 14,
+    forceTimestampFormatting: false,
+    timeFormat: 'YYYY-MM-DD',
+    yAxisFormat: 'SMART_NUMBER',
+    conditionalFormatting: [{ color: 'red', op: '>', value: 0 }],
+    currencyFormat: { symbol: '$', symbolPosition: 'prefix' },
+  };
+
+  const baseDatasource = {
+    currencyFormats: { value: '$0,0.00' },
+    columnFormats: { value: '$0,0.00' },
+    metrics: [{ metric_name: 'value', d3format: '.2f' }],
+  };
+
+  const baseHooks = { onContextMenu };
+
+  const baseRawFormData = { dummy: 'raw' };
+
+  it('should return null bigNumber when no data is provided', () => {
+    const chartProps = {
+      width: 400,
+      height: 300,
+      queriesData: [{ data: [], coltypes: [] }],
+      formData: baseFormData,
+      rawFormData: baseRawFormData,
+      hooks: baseHooks,
+      datasource: baseDatasource,
+    };
+
+    const result = transformProps(
+      chartProps as unknown as BigNumberTotalChartProps,
+    );
+    expect(result.bigNumber).toBeNull();
+    expect(result.width).toBe(400);
+    expect(result.height).toBe(300);
+    expect(result.subtitle).toBe(baseFormData.subheader);
+    expect(result.onContextMenu).toBe(onContextMenu);
+    expect(result.refs).toEqual({});
+    // headerFormatter should be set even if there's no data
+    expect(typeof result.headerFormatter).toBe('function');
+    // colorThresholdFormatters fallback to empty array when getColorFormatters returns falsy
+    expect(result.colorThresholdFormatters).toEqual([]);
+  });
+  it('should convert subheader to subtitle', () => {
+    const chartProps = {
+      width: 400,
+      height: 300,
+      queriesData: [{ data: [], coltypes: [] }],
+      formData: { ...baseFormData, subheader: 'test' },
+      rawFormData: baseRawFormData,
+      hooks: baseHooks,
+      datasource: baseDatasource,
+    };
+    const result = transformProps(
+      chartProps as unknown as BigNumberTotalChartProps,
+    );
+    expect(result.subtitle).toBe('test');
+  });
+
+  it('should compute bigNumber using parseMetricValue when data exists', () => {
+    const chartProps = {
+      width: 500,
+      height: 400,
+      queriesData: [
+        { data: [{ value: '456' }], coltypes: [GenericDataType.String] },
+      ],
+      formData: { ...baseFormData, forceTimestampFormatting: false },
+      rawFormData: baseRawFormData,
+      hooks: baseHooks,
+      datasource: baseDatasource,
+      sortBy: 'value',
+    };
+
+    const result = transformProps(
+      chartProps as unknown as BigNumberTotalChartProps,
+    );
+    // parseMetricValue converts '456' to number 456 by our mock
+    expect(result.bigNumber).toEqual(456);
+  });
+
+  it('should use formatTime as headerFormatter for Temporal or String types or forced formatting', () => {
+    // Case 1: Temporal type
+    const chartPropsTemporal = {
+      width: 600,
+      height: 450,
+      queriesData: [
+        { data: [{ value: '789' }], coltypes: [GenericDataType.Temporal] },
+      ],
+      formData: { ...baseFormData, forceTimestampFormatting: false },
+      rawFormData: baseRawFormData,
+      hooks: baseHooks,
+      datasource: baseDatasource,
+    };
+
+    const resultTemporal = transformProps(
+      chartPropsTemporal as unknown as BigNumberTotalChartProps,
+    );
+    expect(resultTemporal.headerFormatter(5)).toBe('5pm');
+
+    // Case 2: String type regardless of forcing formatting
+    const chartPropsString = {
+      width: 600,
+      height: 450,
+      queriesData: [
+        { data: [{ value: '789' }], coltypes: [GenericDataType.String] },
+      ],
+      formData: { ...baseFormData, forceTimestampFormatting: false },
+      rawFormData: baseRawFormData,
+      hooks: baseHooks,
+      datasource: baseDatasource,
+    };
+
+    const resultString = transformProps(
+      chartPropsString as unknown as BigNumberTotalChartProps,
+    );
+    expect(resultString.headerFormatter(5)).toBe('5pm');
+
+    // Case 3: Forced timestamp formatting
+    const chartPropsForced = {
+      width: 600,
+      height: 450,
+      queriesData: [{ data: [{ value: '789' }], coltypes: [0] }], // non-temporal/non-string
+      formData: { ...baseFormData, forceTimestampFormatting: true },
+      rawFormData: baseRawFormData,
+      hooks: baseHooks,
+      datasource: baseDatasource,
+    };
+
+    const resultForced = transformProps(
+      chartPropsForced as unknown as BigNumberTotalChartProps,
+    );
+    expect(resultForced.headerFormatter(5)).toBe('5pm');
+  });
+
+  it('should use numberFormatter as headerFormatter when not Temporal/String and no forced formatting', () => {
+    const chartProps = {
+      width: 700,
+      height: 500,
+      queriesData: [{ data: [{ value: '321' }], coltypes: [0] }], // non-temporal/non-string
+      formData: { ...baseFormData, forceTimestampFormatting: false },
+      rawFormData: baseRawFormData,
+      hooks: baseHooks,
+      datasource: baseDatasource,
+    };
+
+    const result = transformProps(
+      chartProps as unknown as BigNumberTotalChartProps,
+    );
+    expect(result.headerFormatter(500)).toBe('$500');
+  });
+
+  it('should propagate colorThresholdFormatters from getColorFormatters', () => {
+    // Override the getColorFormatters mock to return specific value
+    const mockFormatters = [{ formatter: 'red' }];
+    (getColorFormatters as jest.Mock).mockReturnValueOnce(mockFormatters);
+
+    const chartProps = {
+      width: 800,
+      height: 600,
+      queriesData: [
+        { data: [{ value: '100' }], coltypes: [GenericDataType.Temporal] },
+      ],
+      formData: baseFormData,
+      rawFormData: baseRawFormData,
+      hooks: baseHooks,
+      datasource: baseDatasource,
+    };
+
+    const result = transformProps(
+      chartProps as unknown as BigNumberTotalChartProps,
+    );
+    expect(result.colorThresholdFormatters).toEqual(mockFormatters);
+  });
+});

superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts

@@ -47,19 +47,22 @@ export default function transformProps(
   const {
     headerFontSize,
     metric = 'value',
-    subtitle = '',
+    subtitle,
     subtitleFontSize,
     forceTimestampFormatting,
     timeFormat,
     yAxisFormat,
     conditionalFormatting,
     currencyFormat,
+    subheader,
+    subheaderFontSize,
   } = formData;
   const refs: Refs = {};
   const { data = [], coltypes = [] } = queriesData[0];
   const granularity = extractTimegrain(rawFormData as QueryFormData);
   const metricName = getMetricLabel(metric);
-  const formattedSubtitle = subtitle;
+  const formattedSubtitle = subtitle || subheader || '';
+  const formattedSubtitleFontSize = subheaderFontSize || subtitleFontSize;
   const bigNumber =
     data.length === 0 ? null : parseMetricValue(data[0][metricName]);
 
@@ -105,10 +108,8 @@ export default function transformProps(
     bigNumber,
     headerFormatter,
     headerFontSize,
-    subtitleFontSize,
+    subtitleFontSize: formattedSubtitleFontSize,
     subtitle: formattedSubtitle,
-    subheader: '',
-    subheaderFontSize: subtitleFontSize,
     onContextMenu,
     refs,
     colorThresholdFormatters,

superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx

@@ -188,47 +188,6 @@ class BigNumberVis extends PureComponent<BigNumberVizProps> {
     );
   }
 
-  renderSubheader(maxHeight: number) {
-    const { bigNumber, subheader, width, bigNumberFallback } = this.props;
-    let fontSize = 0;
-
-    const NO_DATA_OR_HASNT_LANDED = t(
-      'No data after filtering or data is NULL for the latest time record',
-    );
-    const NO_DATA = t(
-      'Try applying different filters or ensuring your datasource has data',
-    );
-    let text = subheader;
-    if (bigNumber === null) {
-      text = bigNumberFallback ? NO_DATA : NO_DATA_OR_HASNT_LANDED;
-    }
-    if (text) {
-      const container = this.createTemporaryContainer();
-      document.body.append(container);
-      fontSize = computeMaxFontSize({
-        text,
-        maxWidth: width * 0.9, // max width reduced
-        maxHeight,
-        className: 'subheader-line',
-        container,
-      });
-      container.remove();
-
-      return (
-        <div
-          className="subheader-line"
-          style={{
-            fontSize,
-            height: maxHeight,
-          }}
-        >
-          {text}
-        </div>
-      );
-    }
-    return null;
-  }
-
   renderSubtitle(maxHeight: number) {
     const { subtitle, width } = this.props;
     let fontSize = 0;
@@ -315,7 +274,6 @@ class BigNumberVis extends PureComponent<BigNumberVizProps> {
       height,
       kickerFontSize,
       headerFontSize,
-      subheaderFontSize,
       subtitleFontSize,
     } = this.props;
     const className = this.getClassName();
@@ -336,11 +294,6 @@ class BigNumberVis extends PureComponent<BigNumberVizProps> {
             {this.renderHeader(
               Math.ceil(headerFontSize * (1 - PROPORTION.TRENDLINE) * height),
             )}
-            {this.renderSubheader(
-              Math.ceil(
-                subheaderFontSize * (1 - PROPORTION.TRENDLINE) * height,
-              ),
-            )}
             {this.renderSubtitle(
               Math.ceil(subtitleFontSize * (1 - PROPORTION.TRENDLINE) * height),
             )}
@@ -355,7 +308,6 @@ class BigNumberVis extends PureComponent<BigNumberVizProps> {
         {this.renderFallbackWarning()}
         {this.renderKicker((kickerFontSize || 0) * height)}
         {this.renderHeader(Math.ceil(headerFontSize * height))}
-        {this.renderSubheader(Math.ceil(subheaderFontSize * height))}
         {this.renderSubtitle(Math.ceil(subtitleFontSize * height))}
       </div>
     );

superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/sharedControls.ts

@@ -129,5 +129,8 @@ export const subtitleControl: CustomControlItem = {
     label: t('Subtitle'),
     renderTrigger: true,
     description: t('Description text that shows up below your Big Number'),
+    mapStateToProps: state => ({
+      value: state.form_data.subheader || state.form_data.subtitle,
+    }),
   },
 };

superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/types.ts

@@ -77,9 +77,7 @@ export type BigNumberVizProps = {
   formatTime?: TimeFormatter;
   headerFontSize: number;
   kickerFontSize?: number;
-  subheader: string;
   subtitle: string;
-  subheaderFontSize: number;
   subtitleFontSize: number;
   showTimestamp?: boolean;
   showTrendLine?: boolean;