You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/09/09 19:14:48 UTC
(superset) branch feat/ag-grid-zebra-striping created (now 6dae021eb15)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a change to branch feat/ag-grid-zebra-striping
in repository https://gitbox.apache.org/repos/asf/superset.git
at 6dae021eb15 feat(ag-grid-table): add zebra striping control
This branch includes the following new commits:
new 6dae021eb15 feat(ag-grid-table): add zebra striping control
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: feat(ag-grid-table): add zebra striping control
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch feat/ag-grid-zebra-striping
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 6dae021eb15dbe534893fc298418de42805c14b3
Author: Evan Rusackas <ev...@rusackas.com>
AuthorDate: Wed Sep 9 12:09:08 2026 -0700
feat(ag-grid-table): add zebra striping control
Table V2's shared ag-grid wrapper (ThemedAgGridReact) already renders a
subtle odd-row background unconditionally for every AG Grid instance in
Superset, including SQL Lab's results grid. It's just much subtler than
Table V1's overt Bootstrap-style striping, easy to miss side by side.
Add a zebra_striping checkbox (default false) so Table V2 charts can turn
that subtle background on or off per chart, reusing the same shared theme
token rather than introducing a new color, and scoped to this plugin via
ThemedAgGridReact's existing themeOverrides prop so SQL Lab's grid and
other consumers are unaffected. A migrated v1 chart gets zebra_striping
forced on, since v1's striping was never a per-chart toggle to begin with.
---
.../src/AgGridTable/index.tsx | 19 +++++++
.../src/AgGridTableChart.tsx | 3 ++
.../src/controlPanel.tsx | 14 +++++
.../src/transformProps.ts | 2 +
.../plugin-chart-ag-grid-table/src/types.ts | 2 +
.../src/utils/useColDefs.ts | 9 +++-
.../test/controlPanel.test.ts | 12 +++++
.../test/transformProps.test.ts | 25 +++++++++
.../test/utils/useColDefs.test.ts | 62 ++++++++++++++++++++++
.../migrations/shared/migrate_viz/processors.py | 8 +++
.../unit_tests/migrations/viz/table_v1_v2_test.py | 18 +++++++
11 files changed, 173 insertions(+), 1 deletion(-)
diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx
index a395f44c52b..4015532a7c1 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx
@@ -45,6 +45,7 @@ import {
SelectionChangedEvent,
} from '@superset-ui/core/components/ThemedAgGridReact';
import { t } from '@apache-superset/core/translation';
+import { useTheme } from '@apache-superset/core/theme';
import {
AgGridChartState,
DataRecordValue,
@@ -118,6 +119,7 @@ export interface AgGridTableProps {
gridRef?: RefObject<AgGridReact>;
chartState?: AgGridChartState;
onClientViewChange?: (snapshot: ClientViewSnapshot) => void;
+ zebraStriping: boolean;
}
ModuleRegistry.registerModules([AllCommunityModule, ClientSideRowModelModule]);
@@ -159,6 +161,7 @@ const AgGridDataTable: FunctionComponent<AgGridTableProps> = memo(
metricColumns = [],
chartState,
onClientViewChange,
+ zebraStriping,
}) => {
const gridRef = useRef<AgGridReact>(null);
const inputRef = useRef<HTMLInputElement>(null);
@@ -168,6 +171,21 @@ const AgGridDataTable: FunctionComponent<AgGridTableProps> = memo(
const hasCapturedInitialGridStateRef = useRef(false);
const filterOperationVersionRef = useRef(0);
+ const theme = useTheme();
+ // ThemedAgGridReact defaults every AG Grid instance (including SQL Lab's
+ // results grid) to a subtle striped background via this same token, so
+ // an explicit override is needed in both directions here rather than
+ // just turning it on: "off" has to opt out of that shared default, and
+ // "on" reuses the same token instead of inventing a chart-specific color.
+ const themeOverrides = useMemo(
+ () => ({
+ oddRowBackgroundColor: zebraStriping
+ ? theme.colorFillQuaternary
+ : 'transparent',
+ }),
+ [zebraStriping, theme.colorFillQuaternary],
+ );
+
const searchId = `search-${id}`;
const initialFilterModel = getInitialFilterModel(
@@ -668,6 +686,7 @@ const AgGridDataTable: FunctionComponent<AgGridTableProps> = memo(
<ThemedAgGridReact
ref={gridRef}
onGridReady={onGridReady}
+ themeOverrides={themeOverrides}
className="ag-container"
rowData={rowData}
headerHeight={36}
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 bbc5baf1e8f..e9d11770271 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
@@ -99,6 +99,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
metricSqlExpressions,
rawSummaryColumns,
showNumberedColumn,
+ zebraStriping,
onContextMenu,
formData,
} = props;
@@ -362,6 +363,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
conditionalFormatting: formData?.conditional_formatting,
comparisonColorEnabled: formData?.comparison_color_enabled,
comparisonColorScheme: formData?.comparison_color_scheme,
+ zebraStriping,
});
const isActiveFilterValue = useCallback(
@@ -730,6 +732,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
onColumnStateChange={handleColumnStateChange}
chartState={chartState}
onClientViewChange={handleClientViewChange}
+ zebraStriping={!!zebraStriping}
/>
</StyledChartContainer>
);
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 4d5a60d933d..4711a58009c 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
@@ -597,6 +597,20 @@ const config: ControlPanelConfig = {
},
},
],
+ [
+ {
+ name: 'zebra_striping',
+ config: {
+ type: 'CheckboxControl',
+ label: t('Zebra striping'),
+ renderTrigger: true,
+ default: false,
+ description: t(
+ 'Alternate the row background color to make rows easier to scan.',
+ ),
+ },
+ },
+ ],
[
{
name: 'column_config',
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 82fcbd3242a..fde606dae12 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
@@ -590,6 +590,7 @@ const transformProps = (
show_numbered_column: showNumberedColumn = false,
allow_rearrange_columns: allowRearrangeColumns = true,
allow_render_html: allowRenderHtml = true,
+ zebra_striping: zebraStriping = false,
} = formData;
// Calculate time comparison settings early since they're used in multiple places
@@ -950,6 +951,7 @@ const transformProps = (
chartState,
onChartStateChange,
showNumberedColumn,
+ zebraStriping,
onContextMenu,
};
};
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 90f617fdbae..85bc3485ee5 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
@@ -84,6 +84,7 @@ export type TableChartFormData = QueryFormData & {
allow_rearrange_columns?: boolean;
allow_render_html?: boolean;
show_numbered_column?: boolean;
+ zebra_striping?: boolean;
};
export interface TableChartProps extends ChartProps {
@@ -136,6 +137,7 @@ export interface AgGridTableChartTransformedProps<
onChartStateChange?: (chartState: JsonObject) => void;
chartState?: AgGridChartState;
showNumberedColumn: boolean;
+ zebraStriping: boolean;
onContextMenu?: (
clientX: number,
clientY: number,
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 05136466e61..c795f95ed63 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
@@ -84,6 +84,7 @@ type UseColDefsProps = {
conditionalFormatting?: ConditionalFormattingConfig[];
comparisonColorEnabled?: boolean;
comparisonColorScheme?: string;
+ zebraStriping?: boolean;
};
function getValueRange(
@@ -253,6 +254,7 @@ export const useColDefs = ({
conditionalFormatting,
comparisonColorEnabled,
comparisonColorScheme,
+ zebraStriping,
}: UseColDefsProps) => {
const theme = useTheme();
// transformProps.ts computes these fresh on every call (no memoization),
@@ -332,8 +334,12 @@ export const useColDefs = ({
valueFormatter: (p: ValueFormatterParams) => valueFormatter(p, col),
valueGetter: (p: ValueGetterParams) => valueGetter(p, col),
cellStyle: (p: CellClassParams) => {
+ // Mirrors the oddRowBackgroundColor override AgGridTable passes to
+ // ThemedAgGridReact for this same zebraStriping flag, so the color
+ // used for text-contrast here always matches the row's actual
+ // rendered background.
const cellSurfaceColor =
- p.node?.rowPinned != null
+ p.node?.rowPinned != null || !zebraStriping
? theme.colorBgBase
: p.rowIndex % 2 === 0
? theme.colorBgBase
@@ -476,6 +482,7 @@ export const useColDefs = ({
allowRenderHtml,
serverPagination,
alignPositiveNegative,
+ zebraStriping,
theme.colorBgBase,
theme.colorFillSecondary,
theme.colorFillQuaternary,
diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.ts
index 90368c8a6aa..12af40cd3d8 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.ts
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.ts
@@ -226,3 +226,15 @@ test('allow_render_html defaults to true, matching v1, and has no visibility gat
expect(control.config.renderTrigger).toBe(true);
expect(control.config.visibility).toBeUndefined();
});
+
+test('zebra_striping defaults to false and has no visibility gate', () => {
+ // v1 has no equivalent control at all (its striping is unconditional), so
+ // there's no "matching v1" default here -- new v2 charts default to v2's
+ // own subtle look, and the migration processor is what materializes True
+ // for charts migrated from v1.
+ const control = findControl(config, 'zebra_striping');
+ expect(control.config.type).toBe('CheckboxControl');
+ expect(control.config.default).toBe(false);
+ expect(control.config.renderTrigger).toBe(true);
+ expect(control.config.visibility).toBeUndefined();
+});
diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/transformProps.test.ts
index 923bb41e6a5..61c12b3bc06 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/transformProps.test.ts
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/transformProps.test.ts
@@ -432,6 +432,31 @@ test('allowRenderHtml is false when allow_render_html is explicitly false', () =
expect(result.allowRenderHtml).toBe(false);
});
+test('zebraStriping defaults to false when zebra_striping is unset', () => {
+ const props = createMockChartProps();
+ const result = transformProps(props);
+ expect(result.zebraStriping).toBe(false);
+});
+
+test('zebraStriping is true when zebra_striping is explicitly true', () => {
+ const props = createMockChartProps({
+ rawFormData: {
+ viz_type: 'table',
+ datasource: '1__table',
+ query_mode: QueryMode.Aggregate,
+ metrics: [],
+ percent_metrics: [],
+ column_config: {},
+ table_timestamp_format: '',
+ granularity_sqla: 'day',
+ time_range: 'No filter',
+ zebra_striping: true,
+ } as unknown as TableChartProps['rawFormData'],
+ });
+ const result = transformProps(props);
+ expect(result.zebraStriping).toBe(true);
+});
+
test('retains saved percentage rules with automatic bounds when server pagination is enabled', () => {
const props = createMockChartProps({
rawFormData: {
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 428bf4b8991..d6025e328d5 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
@@ -470,6 +470,7 @@ test('cellStyle uses striped odd-row surface for adaptive contrast', () => {
...defaultProps,
columns: [numericCol],
data: [{ count: 42 }, { count: 43 }],
+ zebraStriping: true,
columnColorFormatters: [
{
column: 'count',
@@ -516,6 +517,67 @@ test('cellStyle uses striped odd-row surface for adaptive contrast', () => {
});
});
+test('cellStyle uses a flat surface for contrast when zebra striping is disabled', () => {
+ // zebraStriping defaults to false/undefined -- AgGridTable's
+ // oddRowBackgroundColor override matches this by rendering a flat
+ // background in that case, so the contrast color computed here must not
+ // alternate either, or it'd pick a color meant for a stripe that isn't
+ // actually there.
+ const numericCol = makeColumn({
+ key: 'count',
+ label: 'Count',
+ dataType: GenericDataType.Numeric,
+ isNumeric: true,
+ isMetric: true,
+ });
+ const backgroundColor = 'rgba(0, 0, 0, 0.4)';
+
+ const { result } = renderHook(
+ () =>
+ useColDefs({
+ ...defaultProps,
+ columns: [numericCol],
+ data: [{ count: 42 }, { count: 43 }],
+ columnColorFormatters: [
+ {
+ column: 'count',
+ objectFormatting: ObjectFormattingEnum.BACKGROUND_COLOR,
+ getColorFromValue: (value: unknown) =>
+ typeof value === 'number' ? backgroundColor : undefined,
+ },
+ ],
+ }),
+ {
+ wrapper: makeThemeWrapper({
+ ...supersetTheme,
+ colorBgBase: '#ffffff',
+ colorFillQuaternary: '#000000',
+ }),
+ },
+ );
+
+ const cellStyle = getCellStyleFunction(result.current[0].cellStyle);
+ const expectedTextColor = getExpectedTextColor(
+ { backgroundColor },
+ '#ffffff',
+ );
+
+ expect(
+ getCellStyleResult(cellStyle, {
+ rowIndex: 0,
+ }),
+ ).toMatchObject({
+ '--ag-cell-value-color': expectedTextColor,
+ });
+ expect(
+ getCellStyleResult(cellStyle, {
+ rowIndex: 1,
+ }),
+ ).toMatchObject({
+ '--ag-cell-value-color': expectedTextColor,
+ });
+});
+
test('cellStyle exposes hover-specific adaptive contrast for formatted cells', () => {
const numericCol = makeColumn({
key: 'count',
diff --git a/superset/migrations/shared/migrate_viz/processors.py b/superset/migrations/shared/migrate_viz/processors.py
index 2f48fd8c47c..f854dd203cb 100644
--- a/superset/migrations/shared/migrate_viz/processors.py
+++ b/superset/migrations/shared/migrate_viz/processors.py
@@ -799,6 +799,14 @@ class MigrateTableChart(MigrateViz):
if "allow_rearrange_columns" not in self.data:
self.data["allow_rearrange_columns"] = False
+ # v1's TableChart always renders with Bootstrap-style zebra
+ # striping ("table-striped") -- there's no control for it, it's
+ # unconditional. v2's zebra_striping control defaults new charts to
+ # False (matching v2's own subtle-by-default look), so a migrated
+ # chart needs this materialized explicitly to keep its original
+ # striped appearance rather than silently losing it.
+ self.data["zebra_striping"] = True
+
def _build_aggregate_mode_query(
self, base_query_object: dict[str, Any], time_offsets: list[Any]
) -> tuple[list[Any], list[Any], Any, list[Any]]:
diff --git a/tests/unit_tests/migrations/viz/table_v1_v2_test.py b/tests/unit_tests/migrations/viz/table_v1_v2_test.py
index a1627954dbc..8c3c1b3b91c 100644
--- a/tests/unit_tests/migrations/viz/table_v1_v2_test.py
+++ b/tests/unit_tests/migrations/viz/table_v1_v2_test.py
@@ -44,6 +44,9 @@ SOURCE_FORM_DATA: dict[str, Any] = {
}
TARGET_FORM_DATA: dict[str, Any] = {
+ # zebra_striping is intentionally absent from SOURCE_FORM_DATA: v1 has
+ # no control for it (always striped), so the migration materializes it
+ # unconditionally rather than carrying over a source key.
"datasource": "1__table",
"any_other_key": "untouched",
"viz_type": "ag-grid-table",
@@ -62,6 +65,7 @@ TARGET_FORM_DATA: dict[str, Any] = {
"color_pn": True,
"allow_rearrange_columns": True,
"allow_render_html": True,
+ "zebra_striping": True,
"form_data_bak": SOURCE_FORM_DATA,
}
@@ -242,6 +246,20 @@ def test_migration_defaults_omitted_allow_rearrange_columns_to_false() -> None:
migrate_and_assert(MigrateTableChart, source, target)
+def test_migration_always_enables_zebra_striping() -> None:
+ """v1's TableChart always renders striped rows -- there's no control
+ for it, so no source key exists to carry over. v2's zebra_striping
+ control defaults new charts to False, so the migration must
+ unconditionally set it True for a migrated chart to keep its original
+ striped appearance."""
+ target: dict[str, Any] = {
+ **{k: v for k, v in TARGET_FORM_DATA.items() if k != "form_data_bak"},
+ "zebra_striping": True,
+ "form_data_bak": SOURCE_FORM_DATA,
+ }
+ migrate_and_assert(MigrateTableChart, SOURCE_FORM_DATA, target)
+
+
@pytest.mark.parametrize(
"auto_currency_form_data",
[