From ed622e254a49b383541d3c056c7c67efd735a1dc Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 13 Mar 2026 21:51:53 +0300 Subject: [PATCH] feat(matrixify): Revamp control panel (#38519) --- .../src/sections/matrixify.tsx | 42 +++---- .../src/shared-controls/matrixifyControls.tsx | 114 +++++++++--------- .../Matrixify/MatrixifyGridRenderer.test.tsx | 56 +++++---- .../Matrixify/MatrixifyGridRenderer.tsx | 6 +- .../src/chart/types/matrixify.test.ts | 57 +++++---- .../src/chart/types/matrixify.ts | 82 +++++++------ .../ChartContextMenu/ChartContextMenu.tsx | 6 +- .../components/Chart/ChartRenderer.test.tsx | 22 ++-- .../src/components/Chart/ChartRenderer.tsx | 6 +- .../Chart/DrillBy/DrillBySubmenu.test.tsx | 4 +- .../Chart/DrillBy/DrillBySubmenu.tsx | 6 +- .../ControlPanelsContainer.test.tsx | 9 +- .../components/ControlPanelsContainer.tsx | 7 +- .../ExploreChartHeader.test.tsx | 2 +- .../components/ExploreChartPanel/index.tsx | 10 +- .../controls/MatrixifyDimensionControl.tsx | 74 +++++++++--- .../components/controls/SwitchControl.tsx | 65 ++++++++++ .../controls/VerticalRadioControl.tsx | 104 ++++++++++++++++ .../src/explore/components/controls/index.ts | 4 + .../src/explore/controlPanels/sections.tsx | 86 ++++++++----- .../pages/ChartList/ChartList.testHelpers.tsx | 2 +- 21 files changed, 517 insertions(+), 247 deletions(-) create mode 100644 superset-frontend/src/explore/components/controls/SwitchControl.tsx create mode 100644 superset-frontend/src/explore/components/controls/VerticalRadioControl.tsx diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/matrixify.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/matrixify.tsx index dde1f0431db..928ec6331b8 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/matrixify.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/matrixify.tsx @@ -25,30 +25,17 @@ export const matrixifyEnableSection: ControlPanelSectionConfig = { controlSetRows: [ [ { - name: 'matrixify_enable_horizontal_layout', + name: 'matrixify_enable', config: { - type: 'CheckboxControl', - label: t('Enable horizontal layout (columns)'), - description: t( - 'Create matrix columns by placing charts side-by-side', - ), - default: false, - renderTrigger: true, - }, - }, - ], - [ - { - name: 'matrixify_enable_vertical_layout', - config: { - type: 'CheckboxControl', - label: t('Enable vertical layout (rows)'), - description: t('Create matrix rows by stacking charts vertically'), + type: 'SwitchControl', + label: t('Enable matrixify'), default: false, renderTrigger: true, }, }, ], + ['matrixify_mode_columns'], + ['matrixify_mode_rows'], ], tabOverride: 'matrixify', }; @@ -57,8 +44,11 @@ export const matrixifySection: ControlPanelSectionConfig = { label: t('Cell layout & styling'), expanded: false, visibility: ({ controls }) => - controls?.matrixify_enable_vertical_layout?.value === true || - controls?.matrixify_enable_horizontal_layout?.value === true, + controls?.matrixify_enable?.value === true && + (controls?.matrixify_mode_rows?.value === 'metrics' || + controls?.matrixify_mode_rows?.value === 'dimensions' || + controls?.matrixify_mode_columns?.value === 'metrics' || + controls?.matrixify_mode_columns?.value === 'dimensions'), controlSetRows: [ [ { @@ -119,13 +109,13 @@ export const matrixifySection: ControlPanelSectionConfig = { }; export const matrixifyRowSection: ControlPanelSectionConfig = { - label: t('Vertical layout (rows)'), expanded: false, visibility: ({ controls }) => - controls?.matrixify_enable_vertical_layout?.value === true, + controls?.matrixify_enable?.value === true && + (controls?.matrixify_mode_rows?.value === 'metrics' || + controls?.matrixify_mode_rows?.value === 'dimensions'), controlSetRows: [ ['matrixify_show_row_labels'], - ['matrixify_mode_rows'], ['matrixify_rows'], ['matrixify_dimension_rows'], ['matrixify_dimension_selection_mode_rows'], @@ -137,13 +127,13 @@ export const matrixifyRowSection: ControlPanelSectionConfig = { }; export const matrixifyColumnSection: ControlPanelSectionConfig = { - label: t('Horizontal layout (columns)'), expanded: false, visibility: ({ controls }) => - controls?.matrixify_enable_horizontal_layout?.value === true, + controls?.matrixify_enable?.value === true && + (controls?.matrixify_mode_columns?.value === 'metrics' || + controls?.matrixify_mode_columns?.value === 'dimensions'), controlSetRows: [ ['matrixify_show_column_headers'], - ['matrixify_mode_columns'], ['matrixify_columns'], ['matrixify_dimension_columns'], ['matrixify_dimension_selection_mode_columns'], diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/matrixifyControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/matrixifyControls.tsx index be93b6fec1c..48dcaa3dafc 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/matrixifyControls.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/matrixifyControls.tsx @@ -34,19 +34,18 @@ const isMatrixifyVisible = ( controls: any, axis: 'rows' | 'columns', mode?: 'metrics' | 'dimensions', - selectionMode?: 'members' | 'topn', + selectionMode?: 'members' | 'topn' | 'all', ) => { - const layoutControl = `matrixify_enable_${axis === 'rows' ? 'vertical' : 'horizontal'}_layout`; const modeControl = `matrixify_mode_${axis}`; const selectionModeControl = `matrixify_dimension_selection_mode_${axis}`; - const isLayoutEnabled = controls?.[layoutControl]?.value === true; + const modeValue = controls?.[modeControl]?.value; + const isLayoutEnabled = modeValue === 'metrics' || modeValue === 'dimensions'; if (!isLayoutEnabled) return false; if (mode) { - const isModeMatch = controls?.[modeControl]?.value === mode; - if (!isModeMatch) return false; + if (modeValue !== mode) return false; if (selectionMode && mode === 'dimensions') { return controls?.[selectionModeControl]?.value === selectionMode; @@ -66,22 +65,20 @@ const matrixifyControls: Record> = {}; matrixifyControls[`matrixify_mode_${axis}`] = { type: 'RadioButtonControl', - label: t(`Metrics / Dimensions`), - default: axis === 'columns' ? 'metrics' : 'dimensions', + default: 'disabled', renderTrigger: true, tabOverride: 'matrixify', - visibility: ({ controls }) => isMatrixifyVisible(controls, axis), mapStateToProps: ({ controls }) => { const otherAxisControlName = `matrixify_mode_${otherAxis}`; const otherAxisValue = - controls?.[otherAxisControlName]?.value ?? - (otherAxis === 'columns' ? 'metrics' : 'dimensions'); + controls?.[otherAxisControlName]?.value ?? 'disabled'; const isMetricsDisabled = otherAxisValue === 'metrics'; return { options: [ + { value: 'disabled', label: t('Disabled') }, { value: 'metrics', label: t('Metrics'), @@ -92,7 +89,7 @@ const matrixifyControls: Record> = {}; ) : undefined, }, - { value: 'dimensions', label: t('Dimension members') }, + { value: 'dimensions', label: t('Dimensions') }, ], }; }, @@ -125,6 +122,7 @@ const matrixifyControls: Record> = {}; `matrixify_topn_metric_${axis}`, `matrixify_topn_order_${axis}`, `matrixify_dimension_selection_mode_${axis}`, + `matrixify_all_sort_by_${axis}`, ]; return fieldsToCheck.some( @@ -161,7 +159,10 @@ const matrixifyControls: Record> = {}; selectionMode, topNMetric: getValue(`matrixify_topn_metric_${axis}`), topNValue: getValue(`matrixify_topn_value_${axis}`), - topNOrder: getValue(`matrixify_topn_order_${axis}`), + topNOrder: getValue(`matrixify_topn_order_${axis}`, true) + ? 'DESC' + : 'ASC', + allSortBy: getValue(`matrixify_all_sort_by_${axis}`, 'a_to_z'), formData: form_data, validators, }; @@ -187,19 +188,24 @@ const matrixifyControls: Record> = {}; visibility: () => false, }; - // Add selection mode control (Dimension Members vs TopN) + // Add selection mode control (Dimension Members / Top N / All) matrixifyControls[`matrixify_dimension_selection_mode_${axis}`] = { - type: 'RadioButtonControl', + type: 'VerticalRadioControl', label: t(`Selection method`), default: 'members', - options: [ - ['members', t('Dimension members')], - ['topn', t('Top n')], - ], renderTrigger: true, tabOverride: 'matrixify', visibility: ({ controls }) => isMatrixifyVisible(controls, axis, 'dimensions'), + options: [ + { value: 'members', label: t('Dimension members') }, + { value: 'topn', label: t('Top n') }, + { + value: 'all', + label: t('All dimensions'), + tooltip: t('Uses the first 25 values if the dimension has more.'), + }, + ], }; // TopN controls @@ -236,15 +242,15 @@ const matrixifyControls: Record> = {}; description: t(`Metric to use for ordering Top N values`), tabOverride: 'matrixify', visibility: ({ controls }) => - isMatrixifyVisible(controls, axis, 'dimensions', 'topn'), + isMatrixifyVisible(controls, axis, 'dimensions', 'topn') || + (isMatrixifyVisible(controls, axis, 'dimensions', 'all') && + controls?.[`matrixify_all_sort_by_${axis}`]?.value === 'metric'), mapStateToProps: (state, controlState) => { const { controls, datasource } = state; - const isVisible = isMatrixifyVisible( - controls, - axis, - 'dimensions', - 'topn', - ); + const isVisible = + isMatrixifyVisible(controls, axis, 'dimensions', 'topn') || + (isMatrixifyVisible(controls, axis, 'dimensions', 'all') && + controls?.[`matrixify_all_sort_by_${axis}`]?.value === 'metric'); const originalProps = dndAdhocMetricControl.mapStateToProps?.(state, controlState) || {}; @@ -261,17 +267,31 @@ const matrixifyControls: Record> = {}; }; matrixifyControls[`matrixify_topn_order_${axis}`] = { - type: 'RadioButtonControl', - label: t(`Sort order`), - default: 'desc', - options: [ - ['asc', t('Ascending')], - ['desc', t('Descending')], - ], + type: 'CheckboxControl', + label: t('Sort descending'), + default: true, renderTrigger: true, tabOverride: 'matrixify', visibility: ({ controls }) => - isMatrixifyVisible(controls, axis, 'dimensions', 'topn'), + isMatrixifyVisible(controls, axis, 'dimensions', 'topn') || + (isMatrixifyVisible(controls, axis, 'dimensions', 'all') && + controls?.[`matrixify_all_sort_by_${axis}`]?.value === 'metric'), + }; + + matrixifyControls[`matrixify_all_sort_by_${axis}`] = { + type: 'SelectControl', + label: t('Sort by'), + default: 'a_to_z', + clearable: false, + renderTrigger: true, + tabOverride: 'matrixify', + visibility: ({ controls }) => + isMatrixifyVisible(controls, axis, 'dimensions', 'all'), + choices: [ + ['a_to_z', t('A-Z')], + ['z_to_a', t('Z-A')], + ['metric', t('Metric')], + ], }; }); @@ -317,24 +337,6 @@ matrixifyControls.matrixify_charts_per_row = { !controls?.matrixify_fit_columns_dynamically?.value, }; -matrixifyControls.matrixify_enable_vertical_layout = { - type: 'CheckboxControl', - label: t('Enable vertical layout (rows)'), - description: t('Create matrix rows by stacking charts vertically'), - default: false, - renderTrigger: true, - tabOverride: 'matrixify', -}; - -matrixifyControls.matrixify_enable_horizontal_layout = { - type: 'CheckboxControl', - label: t('Enable horizontal layout (columns)'), - description: t('Create matrix columns by placing charts side-by-side'), - default: false, - renderTrigger: true, - tabOverride: 'matrixify', -}; - // Cell title control for Matrixify matrixifyControls.matrixify_cell_title_template = { type: 'TextControl', @@ -345,8 +347,8 @@ matrixifyControls.matrixify_cell_title_template = { default: '', renderTrigger: true, visibility: ({ controls }) => - controls?.matrixify_enable_vertical_layout?.value === true || - controls?.matrixify_enable_horizontal_layout?.value === true, + isMatrixifyVisible(controls, 'rows') || + isMatrixifyVisible(controls, 'columns'), }; // Matrix display controls @@ -357,8 +359,7 @@ matrixifyControls.matrixify_show_row_labels = { default: true, renderTrigger: true, tabOverride: 'matrixify', - visibility: ({ controls }) => - controls?.matrixify_enable_vertical_layout?.value === true, + visibility: ({ controls }) => isMatrixifyVisible(controls, 'rows'), }; matrixifyControls.matrixify_show_column_headers = { @@ -368,8 +369,7 @@ matrixifyControls.matrixify_show_column_headers = { default: true, renderTrigger: true, tabOverride: 'matrixify', - visibility: ({ controls }) => - controls?.matrixify_enable_horizontal_layout?.value === true, + visibility: ({ controls }) => isMatrixifyVisible(controls, 'columns'), }; export { matrixifyControls }; diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx index 7c1fc8d7bc7..588585372da 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx @@ -22,6 +22,7 @@ import '@testing-library/jest-dom'; import { ThemeProvider } from '@apache-superset/core/theme'; import { supersetTheme } from '@apache-superset/core/theme'; import MatrixifyGridRenderer from './MatrixifyGridRenderer'; +import type { MatrixifyMode } from '../../types/matrixify'; import { generateMatrixifyGrid } from './MatrixifyGridGenerator'; // Mock the MatrixifyGridGenerator @@ -74,8 +75,9 @@ test('should create single group when fitting columns dynamically', () => { const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, matrixify_fit_columns_dynamically: true, matrixify_charts_per_row: 3, matrixify_show_row_labels: true, @@ -124,8 +126,9 @@ test('should create multiple groups when not fitting columns dynamically', () => const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, matrixify_fit_columns_dynamically: false, matrixify_charts_per_row: 3, matrixify_show_row_labels: true, @@ -160,8 +163,9 @@ test('should handle exact division of columns', () => { const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, matrixify_fit_columns_dynamically: false, matrixify_charts_per_row: 2, matrixify_show_row_labels: true, @@ -189,8 +193,9 @@ test('should handle case where charts_per_row exceeds total columns', () => { const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, matrixify_fit_columns_dynamically: false, matrixify_charts_per_row: 5, matrixify_show_row_labels: true, @@ -220,8 +225,9 @@ test('should show headers for each group when wrapping occurs', () => { const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, matrixify_fit_columns_dynamically: false, matrixify_charts_per_row: 2, matrixify_show_row_labels: true, @@ -255,8 +261,9 @@ test('should show headers only on first row when not wrapping', () => { const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, matrixify_fit_columns_dynamically: true, // No wrapping matrixify_show_row_labels: true, matrixify_show_column_headers: true, @@ -285,8 +292,9 @@ test('should hide headers when disabled', () => { const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, matrixify_show_row_labels: false, matrixify_show_column_headers: false, }; @@ -313,8 +321,9 @@ test('should place cells correctly in wrapped layout', () => { const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, matrixify_fit_columns_dynamically: false, matrixify_charts_per_row: 2, matrixify_show_row_labels: true, @@ -344,8 +353,9 @@ test('should handle null grid gracefully', () => { const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, }; const { container } = renderWithTheme( @@ -366,8 +376,9 @@ test('should handle empty grid gracefully', () => { const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, }; const { container } = renderWithTheme( @@ -391,8 +402,9 @@ test('should use default values for missing configuration', () => { const formData = { viz_type: 'test_chart', - matrixify_enable_vertical_layout: true, - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics' as MatrixifyMode, + matrixify_mode_columns: 'metrics' as MatrixifyMode, // Missing optional configurations }; diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx index 90402f8e782..fc4b6422459 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx @@ -130,10 +130,12 @@ function MatrixifyGridRenderer({ // Determine layout parameters - only show headers/labels if layout is enabled const showRowLabels = - formData.matrixify_enable_vertical_layout === true && + formData.matrixify_mode_rows !== undefined && + formData.matrixify_mode_rows !== 'disabled' && (formData.matrixify_show_row_labels ?? true); const showColumnHeaders = - formData.matrixify_enable_horizontal_layout === true && + formData.matrixify_mode_columns !== undefined && + formData.matrixify_mode_columns !== 'disabled' && (formData.matrixify_show_column_headers ?? true); const rowHeight = formData.matrixify_row_height || DEFAULT_ROW_HEIGHT; const fitColumnsDynamically = diff --git a/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts b/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts index 6fe052a5a74..b0eaf837fcd 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts +++ b/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.test.ts @@ -37,12 +37,11 @@ test('isMatrixifyEnabled should return false when no matrixify configuration exi expect(isMatrixifyEnabled(formData)).toBe(false); }); -test('isMatrixifyEnabled should return false when layout controls are false', () => { +test('isMatrixifyEnabled should return false when layout controls are disabled', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: false, - matrixify_enable_horizontal_layout: false, - matrixify_mode_rows: 'metrics', + matrixify_mode_rows: 'disabled', + matrixify_mode_columns: 'disabled', matrixify_rows: [createMetric('Revenue')], } as MatrixifyFormData; @@ -52,7 +51,7 @@ test('isMatrixifyEnabled should return false when layout controls are false', () test('isMatrixifyEnabled should return true for valid metrics mode configuration', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, matrixify_mode_rows: 'metrics', matrixify_mode_columns: 'metrics', matrixify_rows: [createMetric('Revenue')], @@ -65,7 +64,7 @@ test('isMatrixifyEnabled should return true for valid metrics mode configuration test('isMatrixifyEnabled should return true for valid dimensions mode configuration', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, matrixify_mode_rows: 'dimensions', matrixify_mode_columns: 'dimensions', matrixify_dimension_rows: { dimension: 'country', values: ['USA'] }, @@ -78,7 +77,7 @@ test('isMatrixifyEnabled should return true for valid dimensions mode configurat test('isMatrixifyEnabled should return true for mixed mode configuration', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, matrixify_mode_rows: 'metrics', matrixify_mode_columns: 'dimensions', matrixify_rows: [createMetric('Revenue')], @@ -91,7 +90,7 @@ test('isMatrixifyEnabled should return true for mixed mode configuration', () => test('isMatrixifyEnabled should return true for topn dimension selection mode', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, matrixify_mode_rows: 'dimensions', matrixify_mode_columns: 'dimensions', matrixify_dimension_rows: { @@ -110,7 +109,7 @@ test('isMatrixifyEnabled should return true for topn dimension selection mode', test('isMatrixifyEnabled should return false when both axes have empty metrics arrays', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, matrixify_mode_rows: 'metrics', matrixify_mode_columns: 'metrics', matrixify_rows: [], @@ -123,7 +122,7 @@ test('isMatrixifyEnabled should return false when both axes have empty metrics a test('isMatrixifyEnabled should return false when both dimensions have empty values and no topn mode', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, matrixify_mode_rows: 'dimensions', matrixify_mode_columns: 'dimensions', matrixify_dimension_rows: { dimension: 'country', values: [] }, @@ -141,7 +140,6 @@ test('getMatrixifyConfig should return null when no matrixify configuration exis test('getMatrixifyConfig should return valid config for metrics mode', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, matrixify_mode_rows: 'metrics', matrixify_mode_columns: 'metrics', matrixify_rows: [createMetric('Revenue')], @@ -159,7 +157,6 @@ test('getMatrixifyConfig should return valid config for metrics mode', () => { test('getMatrixifyConfig should return valid config for dimensions mode', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, matrixify_mode_rows: 'dimensions', matrixify_mode_columns: 'dimensions', matrixify_dimension_rows: { dimension: 'country', values: ['USA'] }, @@ -183,7 +180,6 @@ test('getMatrixifyConfig should return valid config for dimensions mode', () => test('getMatrixifyConfig should handle topn selection mode', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, matrixify_mode_rows: 'dimensions', matrixify_mode_columns: 'dimensions', matrixify_dimension_rows: { @@ -204,8 +200,8 @@ test('getMatrixifyConfig should handle topn selection mode', () => { test('getMatrixifyValidationErrors should return empty array when matrixify is not enabled', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: false, - matrixify_enable_horizontal_layout: false, + matrixify_mode_rows: 'disabled', + matrixify_mode_columns: 'disabled', } as MatrixifyFormData; expect(getMatrixifyValidationErrors(formData)).toEqual([]); @@ -214,7 +210,6 @@ test('getMatrixifyValidationErrors should return empty array when matrixify is n test('getMatrixifyValidationErrors should return empty array when properly configured', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, matrixify_mode_rows: 'metrics', matrixify_mode_columns: 'metrics', matrixify_rows: [createMetric('Revenue')], @@ -227,17 +222,16 @@ test('getMatrixifyValidationErrors should return empty array when properly confi test('getMatrixifyValidationErrors should return error when enabled but no configuration exists', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, + matrixify_mode_rows: 'metrics', } as MatrixifyFormData; const errors = getMatrixifyValidationErrors(formData); - expect(errors).toContain('Please configure at least one row or column axis'); + expect(errors.length).toBeGreaterThan(0); }); test('getMatrixifyValidationErrors should return error when metrics mode has no metrics', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, matrixify_mode_rows: 'metrics', matrixify_rows: [], matrixify_columns: [], @@ -260,19 +254,28 @@ test('should handle empty form data object', () => { expect(isMatrixifyEnabled(formData)).toBe(false); }); -test('isMatrixifyEnabled should return false when layout enabled but no axis modes configured', () => { +test('isMatrixifyEnabled should return false when no axis modes configured', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, // No matrixify_mode_rows or matrixify_mode_columns set } as MatrixifyFormData; expect(isMatrixifyEnabled(formData)).toBe(false); }); +test('isMatrixifyEnabled should return false when switch is off even with valid axis config', () => { + const formData = { + viz_type: 'table', + matrixify_enable: false, + matrixify_mode_rows: 'metrics', + matrixify_rows: [createMetric('Revenue')], + } as MatrixifyFormData; + expect(isMatrixifyEnabled(formData)).toBe(false); +}); + test('getMatrixifyValidationErrors should return dimension error for rows when dimension has no data', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, matrixify_mode_rows: 'dimensions', // No matrixify_dimension_rows set matrixify_mode_columns: 'metrics', @@ -286,7 +289,6 @@ test('getMatrixifyValidationErrors should return dimension error for rows when d test('getMatrixifyValidationErrors should return metric error for columns when metrics array is empty', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, matrixify_mode_rows: 'metrics', matrixify_rows: [createMetric('Revenue')], matrixify_mode_columns: 'metrics', @@ -300,7 +302,6 @@ test('getMatrixifyValidationErrors should return metric error for columns when m test('getMatrixifyValidationErrors should return dimension error for columns when no dimension data', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, matrixify_mode_rows: 'metrics', matrixify_rows: [createMetric('Revenue')], matrixify_mode_columns: 'dimensions', @@ -311,10 +312,9 @@ test('getMatrixifyValidationErrors should return dimension error for columns whe expect(errors).toContain('Please select a dimension and values for columns'); }); -test('getMatrixifyValidationErrors skips row check when matrixify_mode_rows is not set (line 240 false, line 279 || false)', () => { +test('getMatrixifyValidationErrors skips row check when matrixify_mode_rows is not set', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, // No matrixify_mode_rows — hasRowMode = false matrixify_mode_columns: 'metrics', matrixify_columns: [createMetric('Q1')], @@ -324,10 +324,9 @@ test('getMatrixifyValidationErrors skips row check when matrixify_mode_rows is n expect(errors).toEqual([]); }); -test('getMatrixifyValidationErrors evaluates full && expression when dimension is set but values are empty (lines 244, 264, 283, 291 true branches)', () => { +test('getMatrixifyValidationErrors evaluates full && expression when dimension is set but values are empty', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, matrixify_mode_rows: 'dimensions', matrixify_dimension_rows: { dimension: 'country', values: [] }, matrixify_mode_columns: 'dimensions', @@ -345,7 +344,7 @@ test('getMatrixifyValidationErrors evaluates full && expression when dimension i test('should handle partial configuration with one axis only', () => { const formData = { viz_type: 'table', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, matrixify_mode_rows: 'metrics', matrixify_rows: [createMetric('Revenue')], // No columns configuration diff --git a/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.ts b/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.ts index 8c01651ca60..e43dbe31406 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.ts +++ b/superset-frontend/packages/superset-ui-core/src/chart/types/matrixify.ts @@ -23,6 +23,7 @@ import { AdhocMetric } from '../../query'; * Constants for Matrixify filter generation * These match the literal types used in Filter.ts */ + export const MatrixifyFilterConstants = { // Filter expression types ExpressionType: { @@ -46,12 +47,12 @@ export const MatrixifyFilterConstants = { /** * Mode for selecting matrix axis values */ -export type MatrixifyMode = 'metrics' | 'dimensions'; +export type MatrixifyMode = 'disabled' | 'metrics' | 'dimensions'; /** * Selection method for dimension values */ -export type MatrixifySelectionMode = 'members' | 'topn'; +export type MatrixifySelectionMode = 'members' | 'topn' | 'all'; /** * Sort order for top N selection @@ -96,18 +97,18 @@ export interface MatrixifyAxisConfig { * Complete Matrixify configuration in form data */ export interface MatrixifyFormData { - // Layout enable controls - matrixify_enable_vertical_layout?: boolean; - matrixify_enable_horizontal_layout?: boolean; + // Global enable switch + matrixify_enable?: boolean; - // Row axis configuration + // Row axis configuration (mode 'disabled' means axis is off) matrixify_mode_rows?: MatrixifyMode; matrixify_rows?: AdhocMetric[]; matrixify_dimension_selection_mode_rows?: MatrixifySelectionMode; matrixify_dimension_rows?: MatrixifyDimensionValue; matrixify_topn_value_rows?: number; matrixify_topn_metric_rows?: AdhocMetric; - matrixify_topn_order_rows?: MatrixifySortOrder; + matrixify_topn_order_rows?: boolean; + matrixify_all_sort_by_rows?: 'a_to_z' | 'z_to_a' | 'metric'; // Column axis configuration matrixify_mode_columns?: MatrixifyMode; @@ -116,7 +117,8 @@ export interface MatrixifyFormData { matrixify_dimension_columns?: MatrixifyDimensionValue; matrixify_topn_value_columns?: number; matrixify_topn_metric_columns?: AdhocMetric; - matrixify_topn_order_columns?: MatrixifySortOrder; + matrixify_topn_order_columns?: boolean; + matrixify_all_sort_by_columns?: 'a_to_z' | 'z_to_a' | 'metric'; // Grid layout configuration matrixify_row_height?: number; @@ -139,75 +141,85 @@ export interface MatrixifyConfig { columns: MatrixifyAxisConfig; } +/** + * Check if a given axis mode is active (not disabled) + */ +function isAxisEnabled(mode?: MatrixifyMode): boolean { + return mode === 'metrics' || mode === 'dimensions'; +} + /** * Helper function to extract Matrixify configuration from form data */ export function getMatrixifyConfig( formData: MatrixifyFormData & any, ): MatrixifyConfig | null { - const hasRowConfig = formData.matrixify_mode_rows; - const hasColumnConfig = formData.matrixify_mode_columns; + const rowEnabled = isAxisEnabled(formData.matrixify_mode_rows); + const colEnabled = isAxisEnabled(formData.matrixify_mode_columns); - if (!hasRowConfig && !hasColumnConfig) { + if (!rowEnabled && !colEnabled) { return null; } return { rows: { - mode: formData.matrixify_mode_rows || 'metrics', + mode: formData.matrixify_mode_rows || 'disabled', metrics: formData.matrixify_rows, selectionMode: formData.matrixify_dimension_selection_mode_rows, dimension: formData.matrixify_dimension_rows, topnValue: formData.matrixify_topn_value_rows, topnMetric: formData.matrixify_topn_metric_rows, - topnOrder: formData.matrixify_topn_order_rows, + topnOrder: formData.matrixify_topn_order_rows === false ? 'asc' : 'desc', }, columns: { - mode: formData.matrixify_mode_columns || 'metrics', + mode: formData.matrixify_mode_columns || 'disabled', metrics: formData.matrixify_columns, selectionMode: formData.matrixify_dimension_selection_mode_columns, dimension: formData.matrixify_dimension_columns, topnValue: formData.matrixify_topn_value_columns, topnMetric: formData.matrixify_topn_metric_columns, - topnOrder: formData.matrixify_topn_order_columns, + topnOrder: + formData.matrixify_topn_order_columns === false ? 'asc' : 'desc', }, }; } -/** - * Check if Matrixify is enabled and properly configured - */ export function isMatrixifyEnabled(formData: MatrixifyFormData): boolean { - // Check if either vertical or horizontal layout is enabled - const hasVerticalLayout = formData.matrixify_enable_vertical_layout === true; - const hasHorizontalLayout = - formData.matrixify_enable_horizontal_layout === true; - - if (!hasVerticalLayout && !hasHorizontalLayout) { + if (formData.matrixify_enable !== true) { + return false; + } + + const rowEnabled = isAxisEnabled(formData.matrixify_mode_rows); + const colEnabled = isAxisEnabled(formData.matrixify_mode_columns); + + if (!rowEnabled && !colEnabled) { return false; } - // Then validate that we have proper configuration const config = getMatrixifyConfig(formData); if (!config) { return false; } const hasRowData = - config.rows.mode === 'metrics' + rowEnabled && + (config.rows.mode === 'metrics' ? config.rows.metrics && config.rows.metrics.length > 0 : config.rows.dimension?.dimension && (config.rows.selectionMode === 'topn' || + config.rows.selectionMode === 'all' || (config.rows.dimension.values && - config.rows.dimension.values.length > 0)); + config.rows.dimension.values.length > 0))); const hasColumnData = - config.columns.mode === 'metrics' + colEnabled && + (config.columns.mode === 'metrics' ? config.columns.metrics && config.columns.metrics.length > 0 : config.columns.dimension?.dimension && (config.columns.selectionMode === 'topn' || + config.columns.selectionMode === 'all' || (config.columns.dimension.values && - config.columns.dimension.values.length > 0)); + config.columns.dimension.values.length > 0))); return Boolean(hasRowData || hasColumnData); } @@ -220,12 +232,10 @@ export function getMatrixifyValidationErrors( ): string[] { const errors: string[] = []; - // Only validate if matrixify is enabled - const hasVerticalLayout = formData.matrixify_enable_vertical_layout === true; - const hasHorizontalLayout = - formData.matrixify_enable_horizontal_layout === true; + const rowEnabled = isAxisEnabled(formData.matrixify_mode_rows); + const colEnabled = isAxisEnabled(formData.matrixify_mode_columns); - if (!hasVerticalLayout && !hasHorizontalLayout) { + if (!rowEnabled && !colEnabled) { return errors; } @@ -243,6 +253,7 @@ export function getMatrixifyValidationErrors( ? config.rows.metrics && config.rows.metrics.length > 0 : config.rows.dimension?.dimension && (config.rows.selectionMode === 'topn' || + config.rows.selectionMode === 'all' || (config.rows.dimension.values && config.rows.dimension.values.length > 0)); @@ -263,6 +274,7 @@ export function getMatrixifyValidationErrors( ? config.columns.metrics && config.columns.metrics.length > 0 : config.columns.dimension?.dimension && (config.columns.selectionMode === 'topn' || + config.columns.selectionMode === 'all' || (config.columns.dimension.values && config.columns.dimension.values.length > 0)); @@ -281,6 +293,7 @@ export function getMatrixifyValidationErrors( ? config.rows.metrics && config.rows.metrics.length > 0 : config.rows.dimension?.dimension && (config.rows.selectionMode === 'topn' || + config.rows.selectionMode === 'all' || (config.rows.dimension.values && config.rows.dimension.values.length > 0)); @@ -289,6 +302,7 @@ export function getMatrixifyValidationErrors( ? config.columns.metrics && config.columns.metrics.length > 0 : config.columns.dimension?.dimension && (config.columns.selectionMode === 'topn' || + config.columns.selectionMode === 'all' || (config.columns.dimension.values && config.columns.dimension.values.length > 0)); diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx index ecb52a7d87d..cdab2f54af5 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx +++ b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx @@ -187,8 +187,10 @@ const ChartContextMenu = ( canDrillBy && isDisplayed(ContextMenuItem.DrillBy) && !( - formData.matrixify_enable_vertical_layout === true || - formData.matrixify_enable_horizontal_layout === true + (formData.matrixify_mode_rows !== undefined && + formData.matrixify_mode_rows !== 'disabled') || + (formData.matrixify_mode_columns !== undefined && + formData.matrixify_mode_columns !== 'disabled') ); // Disable drill by when matrixify is enabled const datasetResource = useDatasetDrillInfo( diff --git a/superset-frontend/src/components/Chart/ChartRenderer.test.tsx b/superset-frontend/src/components/Chart/ChartRenderer.test.tsx index 1c35af4efc8..9fd400fd562 100644 --- a/superset-frontend/src/components/Chart/ChartRenderer.test.tsx +++ b/superset-frontend/src/components/Chart/ChartRenderer.test.tsx @@ -163,7 +163,7 @@ test('should detect changes in matrixify properties', () => { ...requiredProps.formData, datasource: '', viz_type: VizType.Table, - matrixify_enable_vertical_layout: true, + matrixify_mode_rows: 'metrics', matrixify_dimension_x: { dimension: 'country', values: ['USA'] }, matrixify_dimension_y: { dimension: 'category', values: ['Tech'] }, matrixify_charts_per_row: 3, @@ -177,9 +177,9 @@ test('should detect changes in matrixify properties', () => { // Since we can't directly test shouldComponentUpdate, we verify the component // correctly identifies matrixify-related properties by checking the implementation - expect( - (initialProps.formData as JsonObject).matrixify_enable_vertical_layout, - ).toBe(true); + expect((initialProps.formData as JsonObject).matrixify_mode_rows).toBe( + 'metrics', + ); expect((initialProps.formData as JsonObject).matrixify_dimension_x).toEqual({ dimension: 'country', values: ['USA'], @@ -212,7 +212,7 @@ test('should identify matrixify property changes correctly', () => { formData: { datasource: '', viz_type: VizType.Table, - matrixify_enable_vertical_layout: true, + matrixify_mode_rows: 'metrics', matrixify_dimension_x: { dimension: 'country', values: ['USA'] }, matrixify_charts_per_row: 3, }, @@ -234,7 +234,7 @@ test('should identify matrixify property changes correctly', () => { formData: { datasource: '', viz_type: VizType.Table, - matrixify_enable_vertical_layout: true, + matrixify_mode_rows: 'metrics', matrixify_dimension_x: { dimension: 'country', values: ['USA', 'Canada'], // Changed @@ -277,7 +277,7 @@ test('should handle matrixify-related form data changes', () => { formData: { datasource: '', viz_type: VizType.Table, - matrixify_enable_vertical_layout: true, // This is a significant change + matrixify_mode_rows: 'metrics', // This is a significant change regular_control: 'value1', }, }; @@ -296,7 +296,7 @@ test('should detect matrixify property addition', () => { formData: { datasource: '', viz_type: VizType.Table, - matrixify_enable_vertical_layout: true, + matrixify_mode_rows: 'metrics', // No matrixify_dimension_x initially }, queriesResponse: [{ data: 'current' } as unknown as JsonObject], @@ -317,7 +317,7 @@ test('should detect matrixify property addition', () => { formData: { datasource: '', viz_type: VizType.Table, - matrixify_enable_vertical_layout: true, + matrixify_mode_rows: 'metrics', matrixify_dimension_x: { dimension: 'country', values: ['USA'] }, // Added }, }; @@ -336,7 +336,7 @@ test('should detect nested matrixify property changes', () => { formData: { datasource: '', viz_type: VizType.Table, - matrixify_enable_vertical_layout: true, + matrixify_mode_rows: 'metrics', matrixify_dimension_x: { dimension: 'country', values: ['USA'], @@ -361,7 +361,7 @@ test('should detect nested matrixify property changes', () => { formData: { datasource: '', viz_type: VizType.Table, - matrixify_enable_vertical_layout: true, + matrixify_mode_rows: 'metrics', matrixify_dimension_x: { dimension: 'country', values: ['USA'], diff --git a/superset-frontend/src/components/Chart/ChartRenderer.tsx b/superset-frontend/src/components/Chart/ChartRenderer.tsx index 33528f78a38..b58b3966b36 100644 --- a/superset-frontend/src/components/Chart/ChartRenderer.tsx +++ b/superset-frontend/src/components/Chart/ChartRenderer.tsx @@ -275,8 +275,10 @@ class ChartRenderer extends Component { const nextFormData = nextProps.formData as JsonObject; const currentFormData = this.props.formData as JsonObject; const isMatrixifyEnabled = - nextFormData.matrixify_enable_vertical_layout === true || - nextFormData.matrixify_enable_horizontal_layout === true; + (nextFormData.matrixify_mode_rows !== undefined && + nextFormData.matrixify_mode_rows !== 'disabled') || + (nextFormData.matrixify_mode_columns !== undefined && + nextFormData.matrixify_mode_columns !== 'disabled'); if (!isMatrixifyEnabled) return false; // Check all matrixify-related properties diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx index 3b60e756911..1a7f23a9e4a 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx @@ -272,9 +272,9 @@ test('When menu item is clicked, call onSelection with clicked column and drill ); }); -test('matrixify_enable_vertical_layout should not render component', () => { +test('matrixify_mode_rows enabled should not render component', () => { const { container } = renderSubmenu({ - formData: { ...defaultFormData, matrixify_enable_vertical_layout: true }, + formData: { ...defaultFormData, matrixify_mode_rows: 'metrics' }, }); expect(container).toBeEmptyDOMElement(); }); diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx index 8bb347e7dfa..40e18a264be 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.tsx @@ -179,8 +179,10 @@ export const DrillBySubmenu = ({ } if ( - formData.matrixify_enable_vertical_layout === true || - formData.matrixify_enable_horizontal_layout === true + (formData.matrixify_mode_rows !== undefined && + formData.matrixify_mode_rows !== 'disabled') || + (formData.matrixify_mode_columns !== undefined && + formData.matrixify_mode_columns !== 'disabled') ) { return null; } diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx index fe67e52eb6f..466c56ee1ac 100644 --- a/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx +++ b/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx @@ -307,7 +307,8 @@ describe('ControlPanelsContainer', () => { props.form_data = { ...props.form_data, viz_type: 'line', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics', }; const { rerender } = render(, { @@ -327,7 +328,8 @@ describe('ControlPanelsContainer', () => { form_data: { ...props.form_data, viz_type: 'line', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, + matrixify_mode_rows: 'metrics', matrixify_dimension_columns: { dimension: 'country', values: ['USA', 'Canada'], @@ -381,7 +383,8 @@ describe('ControlPanelsContainer', () => { form_data: { ...props.form_data, viz_type: 'line', - matrixify_enable_horizontal_layout: true, + matrixify_enable: true, + matrixify_mode_columns: 'metrics', }, }; diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx index fba0cc3c0fd..0a1c79ab9fe 100644 --- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx +++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx @@ -816,8 +816,11 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { // Check if matrixify is enabled in form_data const matrixifyIsEnabled = - form_data.matrixify_enable_vertical_layout || - form_data.matrixify_enable_horizontal_layout; + form_data.matrixify_enable === true && + ((form_data.matrixify_mode_rows !== undefined && + form_data.matrixify_mode_rows !== 'disabled') || + (form_data.matrixify_mode_columns !== undefined && + form_data.matrixify_mode_columns !== 'disabled')); // Auto-switch to Matrixify tab when it's enabled useEffect(() => { diff --git a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx index 3417dab6476..c060c193c3c 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader/ExploreChartHeader.test.tsx @@ -549,7 +549,7 @@ describe('ExploreChartHeader', () => { const props = createProps({ formData: { ...createProps().chart.latestQueryFormData, - matrixify_enable_vertical_layout: true, + matrixify_enable: true, matrixify_mode_rows: 'metrics', matrixify_rows: [{ label: 'COUNT(*)', expressionType: 'SIMPLE' }], }, diff --git a/superset-frontend/src/explore/components/ExploreChartPanel/index.tsx b/superset-frontend/src/explore/components/ExploreChartPanel/index.tsx index b7bd508f959..3160189c3fb 100644 --- a/superset-frontend/src/explore/components/ExploreChartPanel/index.tsx +++ b/superset-frontend/src/explore/components/ExploreChartPanel/index.tsx @@ -455,8 +455,10 @@ const ExploreChartPanel = ({ })} {...(chart.chartStatus && { chartStatus: chart.chartStatus })} hideRowCount={ - formData?.matrixify_enable_vertical_layout === true || - formData?.matrixify_enable_horizontal_layout === true + (formData?.matrixify_mode_rows !== undefined && + formData?.matrixify_mode_rows !== 'disabled') || + (formData?.matrixify_mode_columns !== undefined && + formData?.matrixify_mode_columns !== 'disabled') } formData={formData} /> @@ -488,8 +490,8 @@ const ExploreChartPanel = ({ chart.chartUpdateEndTime, refreshCachedQuery, formData?.row_limit, - formData?.matrixify_enable_vertical_layout, - formData?.matrixify_enable_horizontal_layout, + formData?.matrixify_mode_rows, + formData?.matrixify_mode_columns, renderChart, theme.sizeUnit, ], diff --git a/superset-frontend/src/explore/components/controls/MatrixifyDimensionControl.tsx b/superset-frontend/src/explore/components/controls/MatrixifyDimensionControl.tsx index 3832b84d871..d1417b0f020 100644 --- a/superset-frontend/src/explore/components/controls/MatrixifyDimensionControl.tsx +++ b/superset-frontend/src/explore/components/controls/MatrixifyDimensionControl.tsx @@ -32,6 +32,7 @@ export interface MatrixifyDimensionControlValue { dimension: string; values: any[]; topNValues?: TopNValue[]; // Store topN values with their metric values + totalValueCount?: number; // Total number of distinct values for this dimension } interface MatrixifyDimensionControlProps { @@ -42,10 +43,11 @@ interface MatrixifyDimensionControlProps { description?: string; hovered?: boolean; renderTrigger?: boolean; - selectionMode?: 'members' | 'topn'; + selectionMode?: 'members' | 'topn' | 'all'; topNMetric?: string; topNValue?: number; topNOrder?: 'ASC' | 'DESC'; + allSortBy?: 'a_to_z' | 'z_to_a' | 'metric'; formData?: any; // For access to filters and time range validationErrors?: string[]; } @@ -64,6 +66,7 @@ export default function MatrixifyDimensionControl( topNMetric, topNValue, topNOrder = 'DESC', + allSortBy = 'a_to_z', formData, validationErrors, } = props; @@ -109,15 +112,19 @@ export default function MatrixifyDimensionControl( } }, [datasource]); - // Load dimension values when dimension changes + // Load dimension values when dimension changes (members mode, or all mode with A-Z/Z-A sort) + const isAllWithMetric = selectionMode === 'all' && allSortBy === 'metric'; useEffect(() => { if ( !value?.dimension || !datasource || !datasource.id || - selectionMode !== 'members' + (selectionMode !== 'members' && selectionMode !== 'all') || + isAllWithMetric ) { - setValueOptions([]); + if (selectionMode !== 'members' && !isAllWithMetric) { + setValueOptions([]); + } return undefined; } @@ -142,13 +149,43 @@ export default function MatrixifyDimensionControl( signal, endpoint, }); - const values = json.result || []; + let values = json.result || []; + + // Sort alphabetically for 'all' mode + if (selectionMode === 'all') { + const descending = allSortBy === 'z_to_a'; + values = [...values].sort((a: any, b: any) => { + const strA = String(a).toLowerCase(); + const strB = String(b).toLowerCase(); + if (strA < strB) return descending ? 1 : -1; + if (strA > strB) return descending ? -1 : 1; + return 0; + }); + } + setValueOptions( values.map((v: any) => ({ label: optionLabel(v), value: v, })), ); + + if (!signal.aborted) { + const MAX_ALL_DIMENSION_VALUES = 25; + const allValues = + selectionMode === 'all' + ? values.slice(0, MAX_ALL_DIMENSION_VALUES) + : value.values || []; + const updatedValue: MatrixifyDimensionControlValue = { + dimension: value.dimension, + values: allValues, + totalValueCount: values.length, + }; + if (value.topNValues) { + updatedValue.topNValues = value.topNValues; + } + onChange(updatedValue); + } } catch (error) { setValueOptions([]); } finally { @@ -161,7 +198,7 @@ export default function MatrixifyDimensionControl( return () => { controller.abort(); }; - }, [value?.dimension, datasource, selectionMode]); + }, [value?.dimension, datasource, selectionMode, allSortBy]); // Convert topNValue to number for consistent comparison const topNValueNum = useMemo(() => { @@ -176,17 +213,27 @@ export default function MatrixifyDimensionControl( return typeof topNValue === 'number' ? topNValue : null; }, [topNValue]); - // Load TopN values when in TopN mode + // Load TopN values when in TopN mode, or All + Metric sort useEffect(() => { - if (!value?.dimension || !datasource || selectionMode !== 'topn') { + const isTopN = selectionMode === 'topn'; + const isAllMetric = selectionMode === 'all' && allSortBy === 'metric'; + + if (!value?.dimension || !datasource || (!isTopN && !isAllMetric)) { return undefined; } - // If we don't have the required topN parameters, just return without loading - if (!topNMetric || !topNValueNum || topNValueNum <= 0) { + if (!topNMetric) { return undefined; } + // For topn mode, also require a valid limit + if (isTopN && (!topNValueNum || topNValueNum <= 0)) { + return undefined; + } + + const MAX_ALL_DIMENSION_VALUES = 25; + const limit = isAllMetric ? MAX_ALL_DIMENSION_VALUES : topNValueNum!; + const controller = new AbortController(); const { signal } = controller; @@ -199,14 +246,13 @@ export default function MatrixifyDimensionControl( datasource: datasourceId, column: value.dimension, metric: topNMetric, - limit: topNValueNum, + limit, sortAscending: topNOrder === 'ASC', filters: formData?.adhoc_filters || [], timeRange: formData?.time_range, }); if (!signal.aborted) { - // Always update with the new topN values const dimensionValues = extractDimensionValues(values); onChange({ dimension: value.dimension, @@ -217,7 +263,6 @@ export default function MatrixifyDimensionControl( } catch (error: any) { if (!signal.aborted) { setTopNError(error.message || t('Failed to load top values')); - // Clear values on error onChange({ dimension: value.dimension, values: [], @@ -236,8 +281,9 @@ export default function MatrixifyDimensionControl( value?.dimension, datasource, selectionMode, + allSortBy, topNMetric, - topNValueNum, // Use the converted/validated number + topNValueNum, topNOrder, formData?.adhoc_filters, formData?.time_range, diff --git a/superset-frontend/src/explore/components/controls/SwitchControl.tsx b/superset-frontend/src/explore/components/controls/SwitchControl.tsx new file mode 100644 index 00000000000..7823dac011d --- /dev/null +++ b/superset-frontend/src/explore/components/controls/SwitchControl.tsx @@ -0,0 +1,65 @@ +/** + * 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 { type ReactNode } from 'react'; +import { css } from '@apache-superset/core/theme'; +import { Switch } from '@superset-ui/core/components'; +import ControlHeader from '../ControlHeader'; + +interface SwitchControlProps { + value?: boolean; + label?: ReactNode; + description?: ReactNode; + hovered?: boolean; + onChange?: (value: boolean) => void; + validationErrors?: string[]; +} + +export default function SwitchControl({ + value = false, + onChange, + ...props +}: SwitchControlProps) { + const handleChange = (checked: boolean) => { + onChange?.(checked); + }; + + const switchNode = ( + + ); + + if (props.label) { + return ( +
+ handleChange(!value)} + /> +
+ ); + } + return switchNode; +} diff --git a/superset-frontend/src/explore/components/controls/VerticalRadioControl.tsx b/superset-frontend/src/explore/components/controls/VerticalRadioControl.tsx new file mode 100644 index 00000000000..ee382d7faf9 --- /dev/null +++ b/superset-frontend/src/explore/components/controls/VerticalRadioControl.tsx @@ -0,0 +1,104 @@ +/** + * 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 { type ReactNode } from 'react'; +import { css, useTheme } from '@apache-superset/core/theme'; +import { JsonValue } from '@superset-ui/core'; +import { Radio } from '@superset-ui/core/components/Radio'; +import { Space } from '@superset-ui/core/components/Space'; +import { Tooltip } from '@superset-ui/core/components/Tooltip'; +import { Icons } from '@superset-ui/core/components/Icons'; +import ControlHeader from '../ControlHeader'; + +interface RadioOption { + value: JsonValue; + label: ReactNode; + disabled?: boolean; + tooltip?: string; +} + +type RadioOptionTuple = [JsonValue, ReactNode]; + +interface VerticalRadioControlProps { + value?: JsonValue; + label?: ReactNode; + description?: ReactNode; + hovered?: boolean; + options: (RadioOption | RadioOptionTuple)[]; + onChange: (value: JsonValue) => void; + validationErrors?: string[]; +} + +function normalizeOption(option: RadioOption | RadioOptionTuple): RadioOption { + if (Array.isArray(option)) { + return { value: option[0], label: option[1] }; + } + return option; +} + +export default function VerticalRadioControl({ + value: initialValue, + options, + onChange, + ...props +}: VerticalRadioControlProps) { + const theme = useTheme(); + const normalizedOptions = options.map(normalizeOption); + const currentValue = initialValue ?? normalizedOptions[0]?.value; + + return ( +
+ + onChange(e.target.value)} + > + + {normalizedOptions.map( + ({ value: val, label, disabled = false, tooltip }) => ( + + {label} + {tooltip && ( + + + + )} + + ), + )} + + +
+ ); +} diff --git a/superset-frontend/src/explore/components/controls/index.ts b/superset-frontend/src/explore/components/controls/index.ts index 326da43ce84..1550df945e7 100644 --- a/superset-frontend/src/explore/components/controls/index.ts +++ b/superset-frontend/src/explore/components/controls/index.ts @@ -60,6 +60,8 @@ import TimeRangeControl from './TimeRangeControl'; import ColorBreakpointsControl from './ColorBreakpointsControl'; import MatrixifyDimensionControl from './MatrixifyDimensionControl'; import JSEditorControl from './JSEditorControl'; +import SwitchControl from './SwitchControl'; +import VerticalRadioControl from './VerticalRadioControl'; const extensionsRegistry = getExtensionsRegistry(); const DateFilterControlExtension = extensionsRegistry.get( @@ -109,6 +111,8 @@ const controlMap = { NumberControl, TimeRangeControl, MatrixifyDimensionControl, + SwitchControl, + VerticalRadioControl, ...sharedControlComponents, }; export default controlMap; diff --git a/superset-frontend/src/explore/controlPanels/sections.tsx b/superset-frontend/src/explore/controlPanels/sections.tsx index 816d3e18604..bac8cddaf75 100644 --- a/superset-frontend/src/explore/controlPanels/sections.tsx +++ b/superset-frontend/src/explore/controlPanels/sections.tsx @@ -277,57 +277,77 @@ export const NVD3TimeSeries: ControlPanelSectionConfig[] = [ function buildMatrixifySection( axis: 'columns' | 'rows', ): ControlPanelSectionConfig { - const baseControls = [ - [`matrixify_mode_${axis}`], - [`matrixify_${axis}`], - [`matrixify_dimension_selection_mode_${axis}`], - [`matrixify_dimension_${axis}`], - [`matrixify_topn_dimension_${axis}`], - [`matrixify_topn_value_${axis}`], - [`matrixify_topn_metric_${axis}`], - [`matrixify_topn_order_${axis}`], - ]; - - // Add enable checkbox at the beginning of each section - const enableControl = + const customizationControls = axis === 'rows' - ? 'matrixify_enable_vertical_layout' - : 'matrixify_enable_horizontal_layout'; - - baseControls.unshift([enableControl]); - - // Add specific controls for each axis - if (axis === 'rows') { - // Add show row labels after enable - baseControls.splice(1, 0, ['matrixify_show_row_labels']); - } else if (axis === 'columns') { - // Add show column headers after enable - baseControls.splice(1, 0, ['matrixify_show_column_headers']); - } + ? ['matrixify_show_row_labels', 'matrixify_row_height'] + : ['matrixify_show_column_headers', 'matrixify_fit_columns_dynamically']; return { label: axis === 'columns' - ? t('Horizontal layout (columns)') - : t('Vertical layout (rows)'), + ? t('Columns (horizontal layout)') + : t('Rows (vertical layout)'), expanded: true, tabOverride: 'matrixify', - controlSetRows: baseControls, + visibility: ({ controls }) => controls?.matrixify_enable?.value === true, + controlSetRows: [ + [`matrixify_mode_${axis}`], + [`matrixify_${axis}`], + [`matrixify_dimension_selection_mode_${axis}`], + [`matrixify_dimension_${axis}`], + [`matrixify_topn_dimension_${axis}`], + [`matrixify_topn_value_${axis}`], + [`matrixify_all_sort_by_${axis}`], + [`matrixify_topn_metric_${axis}`], + [`matrixify_topn_order_${axis}`], + [ + + {t('Customization and styling')} + , + ], + customizationControls, + ], }; } export const matrixifyRows = buildMatrixifySection('rows'); export const matrixifyColumns = buildMatrixifySection('columns'); +export const matrixifyEnableSection: ControlPanelSectionConfig = { + label: t('Matrixify'), + expanded: true, + tabOverride: 'matrixify', + controlSetRows: [ + [ + { + name: 'matrixify_enable', + config: { + type: 'SwitchControl', + label: t('Enable matrixify'), + default: false, + renderTrigger: true, + }, + }, + ], + ], +}; + export const matrixifyCells: ControlPanelSectionConfig = { label: t('Cell layout & styling'), expanded: true, tabOverride: 'matrixify', - visibility: ({ controls }) => - controls?.matrixify_enable_vertical_layout?.value === true || - controls?.matrixify_enable_horizontal_layout?.value === true, + visibility: ({ controls }) => { + if (controls?.matrixify_enable?.value !== true) return false; + const rowMode = controls?.matrixify_mode_rows?.value; + const colMode = controls?.matrixify_mode_columns?.value; + return ( + rowMode === 'metrics' || + rowMode === 'dimensions' || + colMode === 'metrics' || + colMode === 'dimensions' + ); + }, controlSetRows: [ - ['matrixify_row_height', 'matrixify_fit_columns_dynamically'], ['matrixify_charts_per_row'], ['matrixify_cell_title_template'], ], diff --git a/superset-frontend/src/pages/ChartList/ChartList.testHelpers.tsx b/superset-frontend/src/pages/ChartList/ChartList.testHelpers.tsx index edd32b909f6..1a27d5affeb 100644 --- a/superset-frontend/src/pages/ChartList/ChartList.testHelpers.tsx +++ b/superset-frontend/src/pages/ChartList/ChartList.testHelpers.tsx @@ -65,7 +65,7 @@ export const mockCharts = [ // Add form_data with matrixify enabled form_data: { viz_type: 'table', - matrixify_enable_vertical_layout: true, + matrixify_enable: true, matrixify_mode_rows: 'metrics', matrixify_rows: [{ label: 'COUNT(*)', expressionType: 'SIMPLE' }], },