diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx index a065ff7216e..24bc28e5322 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx @@ -86,6 +86,7 @@ export default function TableChart( onChartStateChange, chartState, metricSqlExpressions, + showNumberedColumn, } = props; const [searchOptions, setSearchOptions] = useState([]); @@ -230,6 +231,9 @@ export default function TableChart( : (columns as InputColumn[]), data, serverPagination, + serverPaginationData, + serverPageLength, + showNumberedColumn: showNumberedColumn && !emitCrossFilters, isRawRecords, defaultAlignPN: alignPositiveNegative, showCellBars, diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/consts.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/consts.ts index 29d43a17aca..888686e450a 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/consts.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/consts.ts @@ -42,3 +42,5 @@ export const FILTER_CONDITION_BODY_INDEX = { FIRST: 0, SECOND: 1, } as const; + +export const ROW_NUMBER_COL_ID = '__row_number__'; diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx index 292672d73cb..6759e95052a 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx @@ -496,6 +496,18 @@ const config: ControlPanelConfig = { label: t('Visual formatting'), expanded: true, controlSetRows: [ + [ + { + name: 'show_numbered_column', + config: { + type: 'CheckboxControl', + label: t('Add numbered column'), + renderTrigger: true, + default: false, + description: t('Whether to display the numbered column'), + }, + }, + ], [ { name: 'column_config', diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/styles/index.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/styles/index.tsx index 7e5131bdf17..3bb714e03a2 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/styles/index.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/styles/index.tsx @@ -419,5 +419,11 @@ export const StyledChartContainer = styled.div<{ color: ${theme.colorTextQuaternary}; } } + + .ag-header-center { + .ag-header-cell-label { + justify-content: center; + } + } `} `; diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts index 33ac8aa0183..1ad19b06b57 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts @@ -507,6 +507,7 @@ const transformProps = ( conditional_formatting: conditionalFormatting, comparison_color_enabled: comparisonColorEnabled = false, comparison_color_scheme: comparisonColorScheme = ColorSchemeEnum.Green, + show_numbered_column: showNumberedColumn = false, } = formData; const allowRearrangeColumns = true; @@ -785,6 +786,7 @@ const transformProps = ( metricSqlExpressions, chartState, onChartStateChange, + showNumberedColumn, }; }; diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts index e3d6bf07079..f24085e2795 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts @@ -81,6 +81,7 @@ export type TableChartFormData = QueryFormData & { time_grain_sqla?: TimeGranularity; column_config?: Record; allow_rearrange_columns?: boolean; + show_numbered_column?: boolean; }; export interface TableChartProps extends ChartProps { @@ -131,6 +132,7 @@ export interface AgGridTableChartTransformedProps< metricSqlExpressions: Record; onChartStateChange?: (chartState: JsonObject) => void; chartState?: AgGridChartState; + showNumberedColumn: boolean; } export interface SortState { diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useColDefs.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useColDefs.ts index cc99891602f..1ade82887d4 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useColDefs.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useColDefs.ts @@ -17,9 +17,10 @@ * specific language governing permissions and limitations * under the License. */ +import { t } from '@apache-superset/core/translation'; import { ColDef } from '@superset-ui/core/components/ThemedAgGridReact'; import { useCallback, useMemo } from 'react'; -import { DataRecord, DataRecordValue } from '@superset-ui/core'; +import { DataRecord, DataRecordValue, JsonObject } from '@superset-ui/core'; import { GenericDataType } from '@apache-superset/core/common'; import { useTheme } from '@apache-superset/core/theme'; import { ColorFormatters } from '@superset-ui/chart-controls'; @@ -41,7 +42,7 @@ import { getAggFunc } from './getAggFunc'; import { TextCellRenderer } from '../renderers/TextCellRenderer'; import { NumericCellRenderer } from '../renderers/NumericCellRenderer'; import CustomHeader from '../AgGridTable/components/CustomHeader'; -import { NOOP_FILTER_COMPARATOR } from '../consts'; +import { NOOP_FILTER_COMPARATOR, ROW_NUMBER_COL_ID } from '../consts'; import { valueFormatter, valueGetter } from './formatValue'; import getCellStyle from './getCellStyle'; @@ -53,6 +54,9 @@ type UseColDefsProps = { columns: InputColumn[]; data: InputData[]; serverPagination: boolean; + serverPaginationData: JsonObject; + serverPageLength: number; + showNumberedColumn: boolean; isRawRecords: boolean; defaultAlignPN: boolean; showCellBars: boolean; @@ -216,6 +220,9 @@ export const useColDefs = ({ columns, data, serverPagination, + serverPaginationData, + serverPageLength, + showNumberedColumn, isRawRecords, defaultAlignPN, showCellBars, @@ -459,5 +466,61 @@ export const useColDefs = ({ }, []); }, [stringifiedCols, getCommonColProps]); - return colDefs; + const rawPageSize = serverPaginationData?.pageSize ?? serverPageLength; + const pageSize = rawPageSize && rawPageSize > 0 ? rawPageSize : data.length; + const currentPage = serverPaginationData?.currentPage ?? 0; + const maxVisibleRowNumber = serverPagination + ? currentPage * pageSize + data.length + : data.length; + const rowIndexLength = `${Math.max(maxVisibleRowNumber, 1)}`.length; + const rowNumberCol = useMemo( + () => ({ + headerName: t('№'), + headerClass: 'ag-header-center', + field: ROW_NUMBER_COL_ID, + valueGetter: params => { + if (params.node?.rowPinned != null) return ''; + if (serverPagination && serverPaginationData) { + return currentPage * pageSize + (params.node?.rowIndex ?? 0) + 1; + } + return (params.node?.rowIndex ?? 0) + 1; + }, + headerStyle: { + backgroundColor: theme.colorFillTertiary, + fontSize: '1em', + color: theme.colorTextTertiary, + }, + width: 30 + rowIndexLength * 6, + minWidth: 30 + rowIndexLength * 6, + sortable: false, + filter: false, + pinned: 'left' as const, + lockPosition: true, + suppressNavigable: true, + resizable: false, + suppressMovable: true, + suppressSizeToFit: true, + cellStyle: { + backgroundColor: theme.colorFillTertiary, + padding: '0', + textAlign: 'center', + fontSize: '0.9em', + color: theme.colorTextTertiary, + }, + }), + [ + currentPage, + pageSize, + rowIndexLength, + serverPagination, + serverPaginationData, + theme.colorFillTertiary, + theme.colorTextTertiary, + ], + ); + + return useMemo( + () => (showNumberedColumn ? [rowNumberCol, ...colDefs] : colDefs), + [showNumberedColumn, rowNumberCol, colDefs], + ); }; diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/useColDefs.test.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/useColDefs.test.ts index 46d531a80c6..bfbf9327eb6 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/useColDefs.test.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/useColDefs.test.ts @@ -28,6 +28,7 @@ import tinycolor from 'tinycolor2'; import { createElement, type ComponentProps, ReactNode } from 'react'; import { useColDefs } from '../../src/utils/useColDefs'; import { InputColumn } from '../../src/types'; +import { ROW_NUMBER_COL_ID } from '../../src/consts'; type TestCellStyleFunc = (params: unknown) => unknown; @@ -122,6 +123,9 @@ const defaultThemeWrapper = makeThemeWrapper(supersetTheme); const defaultProps = { data: [{ test_col: 'value' }], serverPagination: false, + serverPaginationData: {}, + serverPageLength: 0, + showNumberedColumn: false, isRawRecords: true, defaultAlignPN: false, showCellBars: false, @@ -136,6 +140,12 @@ const defaultProps = { slice_id: 1, }; +const basePropsNumericColumns = { + ...defaultProps, + columns: [makeColumn({ key: 'a', label: 'A' })], + data: [{ a: 1 }, { a: 2 }, { a: 3 }], +}; + test('boolean columns use agCheckboxCellRenderer', () => { const booleanCol = makeColumn({ key: 'is_active', @@ -828,3 +838,233 @@ test('cellStyle respects explicit horizontal alignment overrides', () => { textAlign: 'center', }); }); + +test('is not added when showNumberedColumn is false', () => { + const { result } = renderHook( + () => useColDefs({ ...basePropsNumericColumns, showNumberedColumn: false }), + { wrapper: defaultThemeWrapper }, + ); + expect(result.current.some(col => col.field === ROW_NUMBER_COL_ID)).toBe( + false, + ); + expect(result.current.length).toBe(1); +}); + +test('is added as first column when showNumberedColumn is true', () => { + const { result } = renderHook( + () => useColDefs({ ...basePropsNumericColumns, showNumberedColumn: true }), + { wrapper: defaultThemeWrapper }, + ); + expect(result.current[0].field).toBe(ROW_NUMBER_COL_ID); + expect(result.current[0].headerName).toBe('№'); + expect(result.current.length).toBe(2); +}); + +test('width defaults to 36 when maxVisibleRowNumber is 0 (empty data)', () => { + const emptyProps = { + ...basePropsNumericColumns, + data: [], + showNumberedColumn: true, + }; + const { result } = renderHook(() => useColDefs(emptyProps), { + wrapper: defaultThemeWrapper, + }); + expect(result.current[0].width).toBe(36); +}); + +test('width uses server pagination row count when available', () => { + const serverProps = { + ...basePropsNumericColumns, + serverPagination: true, + serverPaginationData: { currentPage: 0, pageSize: 5 }, + data: [{ a: 1 }, { a: 2 }], + showNumberedColumn: true, + }; + const { result } = renderHook(() => useColDefs(serverProps), { + wrapper: defaultThemeWrapper, + }); + + expect(result.current[0].width).toBe(36); + + const laterPageProps = { + ...serverProps, + serverPaginationData: { currentPage: 3, pageSize: 5 }, + data: [{ a: 21 }, { a: 22 }], + }; + const { result: resultLater } = renderHook(() => useColDefs(laterPageProps), { + wrapper: defaultThemeWrapper, + }); + expect(resultLater.current[0].width).toBe(42); +}); + +test('valueGetter returns row numbers without server pagination', () => { + const { result } = renderHook( + () => useColDefs({ ...basePropsNumericColumns, showNumberedColumn: true }), + { wrapper: defaultThemeWrapper }, + ); + const colDef = result.current[0]; + const { valueGetter } = colDef; + expect(valueGetter).toBeDefined(); + expect(typeof valueGetter).toBe('function'); + + const getter = valueGetter as (params: { + node?: { rowIndex?: number }; + data?: Record; + }) => number; + + const params = (rowIndex: number) => ({ + node: { rowIndex }, + data: basePropsNumericColumns.data[rowIndex], + }); + + expect(getter(params(0))).toBe(1); + expect(getter(params(1))).toBe(2); + expect(getter(params(2))).toBe(3); +}); + +test('valueGetter respects server pagination', () => { + const serverProps = { + ...basePropsNumericColumns, + serverPagination: true, + serverPaginationData: { currentPage: 2, pageSize: 5 }, + data: [{ a: 11 }, { a: 12 }], + }; + const { result } = renderHook( + () => useColDefs({ ...serverProps, showNumberedColumn: true }), + { wrapper: defaultThemeWrapper }, + ); + const { valueGetter } = result.current[0]; + expect(typeof valueGetter).toBe('function'); + + const getter = valueGetter as (params: { + node?: { rowIndex?: number }; + data?: Record; + }) => number; + + expect(getter({ node: { rowIndex: 0 } })).toBe(11); + expect(getter({ node: { rowIndex: 1 } })).toBe(12); +}); + +test('has correct static column properties', () => { + const { result } = renderHook( + () => useColDefs({ ...basePropsNumericColumns, showNumberedColumn: true }), + { wrapper: defaultThemeWrapper }, + ); + const colDef = result.current[0]; + expect(colDef.sortable).toBe(false); + expect(colDef.filter).toBe(false); + expect(colDef.pinned).toBe('left'); + expect(colDef.lockPosition).toBe(true); + expect(colDef.suppressNavigable).toBe(true); + expect(colDef.suppressSizeToFit).toBe(true); + expect(colDef.resizable).toBe(false); + expect(colDef.suppressMovable).toBe(true); + expect(colDef.headerStyle).toBeDefined(); + expect(colDef.cellStyle).toBeDefined(); +}); + +test('pageSize priority chain works correctly', () => { + const props1 = { + ...basePropsNumericColumns, + serverPagination: true, + serverPaginationData: { currentPage: 2, pageSize: 10 }, + serverPageLength: 5, + data: Array.from({ length: 3 }, () => ({ a: 1 })), + showNumberedColumn: true, + }; + const { result: res1 } = renderHook(() => useColDefs(props1), { + wrapper: defaultThemeWrapper, + }); + + expect(res1.current[0].width).toBe(42); + + const props2 = { + ...props1, + serverPaginationData: { currentPage: 2 }, + serverPageLength: 20, + }; + const { result: res2 } = renderHook(() => useColDefs(props2), { + wrapper: defaultThemeWrapper, + }); + + expect(res2.current[0].width).toBe(42); +}); + +test('column width adapts to max row number length in client mode', () => { + const base = { + ...basePropsNumericColumns, + showNumberedColumn: true, + serverPagination: false, + }; + + const props9 = { ...base, data: Array.from({ length: 9 }, () => ({ a: 1 })) }; + const { result: res9 } = renderHook(() => useColDefs(props9), { + wrapper: defaultThemeWrapper, + }); + expect(res9.current[0].width).toBe(36); + + const props10 = { + ...base, + data: Array.from({ length: 10 }, () => ({ a: 1 })), + }; + const { result: res10 } = renderHook(() => useColDefs(props10), { + wrapper: defaultThemeWrapper, + }); + expect(res10.current[0].width).toBe(42); + + const props100 = { + ...base, + data: Array.from({ length: 100 }, () => ({ a: 1 })), + }; + const { result: res100 } = renderHook(() => useColDefs(props100), { + wrapper: defaultThemeWrapper, + }); + expect(res100.current[0].width).toBe(48); +}); + +test('row number column uses theme variables for styling', () => { + const { result } = renderHook( + () => useColDefs({ ...basePropsNumericColumns, showNumberedColumn: true }), + { wrapper: defaultThemeWrapper }, + ); + const colDef = result.current[0]; + expect(colDef.cellStyle).toMatchObject({ + backgroundColor: expect.any(String), + padding: '0', + textAlign: 'center', + fontSize: '0.9em', + color: expect.any(String), + }); + expect(colDef.headerStyle).toMatchObject({ + backgroundColor: expect.any(String), + fontSize: '1em', + color: expect.any(String), + }); +}); + +test('valueGetter handles missing node or rowIndex gracefully', () => { + const { result } = renderHook( + () => useColDefs({ ...basePropsNumericColumns, showNumberedColumn: true }), + { wrapper: defaultThemeWrapper }, + ); + const getter = result.current[0].valueGetter as (params: { + node?: { rowIndex?: number }; + }) => number; + expect(getter({})).toBe(1); + expect(getter({ node: {} })).toBe(1); + + const serverProps = { + ...basePropsNumericColumns, + serverPagination: true, + serverPaginationData: { currentPage: 2, pageSize: 5 }, + showNumberedColumn: true, + }; + const { result: serverResult } = renderHook(() => useColDefs(serverProps), { + wrapper: defaultThemeWrapper, + }); + const serverGetter = serverResult.current[0].valueGetter as (params: { + node?: { rowIndex?: number }; + }) => number; + expect(serverGetter({})).toBe(2 * 5 + 1); + expect(serverGetter({ node: {} })).toBe(11); +});