This is an automated email from the ASF dual-hosted git repository.
jli 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 4140261797 fix: subheader should show as subtitle (#33172)
4140261797 is described below
commit 41402617970368f0b14ff4e7acc1d271d5bcdd89
Author: Elizabeth Thompson <[email protected]>
AuthorDate: Thu Apr 17 22:03:20 2025 -0700
fix: subheader should show as subtitle (#33172)
---
.../e2e/explore/visualizations/big_number.test.js | 4 +-
.../BigNumber/BigNumberTotal/controlPanel.test.ts | 97 +++++++++
.../BigNumberTotal/transformProps.test.ts | 220 +++++++++++++++++++++
.../src/BigNumber/BigNumberTotal/transformProps.ts | 11 +-
.../src/BigNumber/BigNumberViz.tsx | 48 -----
.../src/BigNumber/sharedControls.ts | 3 +
.../plugin-chart-echarts/src/BigNumber/types.ts | 2 -
7 files changed, 327 insertions(+), 58 deletions(-)
diff --git
a/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/big_number.test.js
b/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/big_number.test.js
index 2882f6ab4a..72d9e92710 100644
---
a/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/big_number.test.js
+++
b/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');
});
});
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.test.ts
b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/controlPanel.test.ts
new file mode 100644
index 0000000000..4be15b9e4b
--- /dev/null
+++
b/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();
+ });
+});
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.test.ts
b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.test.ts
new file mode 100644
index 0000000000..f41008eb60
--- /dev/null
+++
b/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);
+ });
+});
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts
b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts
index c673ebd9f5..e892ca33f7 100644
---
a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberTotal/transformProps.ts
+++
b/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,
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx
b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx
index fddebc93a5..540efa4908 100644
---
a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx
+++
b/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>
);
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/sharedControls.ts
b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/sharedControls.ts
index 09766ed4bf..9de4065b67 100644
---
a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/sharedControls.ts
+++
b/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,
+ }),
},
};
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/types.ts
b/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/types.ts
index d843ee1fd4..8b5a5abddb 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/types.ts
+++ b/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;
