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/08/24 18:45:42 UTC
(superset) branch master updated: fix(plugin-chart-table): guard row-indexed comparison-color lookups against undefined entries (#43139)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new a8a8b51afb8 fix(plugin-chart-table): guard row-indexed comparison-color lookups against undefined entries (#43139)
a8a8b51afb8 is described below
commit a8a8b51afb8b5170baab3a0d1506e25d629fa500
Author: Joe Li <jo...@preset.io>
AuthorDate: Mon Aug 24 11:45:25 2026 -0700
fix(plugin-chart-table): guard row-indexed comparison-color lookups against undefined entries (#43139)
---
.../plugins/plugin-chart-table/src/TableChart.tsx | 12 ++---
.../plugin-chart-table/test/TableChart.test.tsx | 54 ++++++++++++++++++++++
2 files changed, 60 insertions(+), 6 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
index 1f6d634e597..2045c3d8a0e 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
@@ -1069,10 +1069,10 @@ export default function TableChart<D extends DataRecord = DataRecord>(
const originKey = column.key.substring(column.label.length).trim();
if (!hasColumnColorFormatters && hasBasicColorFormatters) {
backgroundColor =
- basicColorFormatters[row.index][originKey]?.backgroundColor;
+ basicColorFormatters[row.index]?.[originKey]?.backgroundColor;
arrow =
column.label === comparisonLabels[0]
- ? basicColorFormatters[row.index][originKey]?.mainArrow
+ ? basicColorFormatters[row.index]?.[originKey]?.mainArrow
: '';
}
@@ -1134,11 +1134,11 @@ export default function TableChart<D extends DataRecord = DataRecord>(
basicColorColumnFormatters?.length > 0
) {
backgroundColor =
- basicColorColumnFormatters[row.index][column.key]
+ basicColorColumnFormatters[row.index]?.[column.key]
?.backgroundColor || backgroundColor;
arrow =
column.label === comparisonLabels[0]
- ? basicColorColumnFormatters[row.index][column.key]?.mainArrow
+ ? basicColorColumnFormatters[row.index]?.[column.key]?.mainArrow
: '';
}
const rowSurfaceColor =
@@ -1197,7 +1197,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
let arrowStyles = css`
color: ${
basicColorFormatters &&
- basicColorFormatters[row.index][originKey]?.arrowColor ===
+ basicColorFormatters[row.index]?.[originKey]?.arrowColor ===
ColorSchemeEnum.Green
? theme.colorSuccess
: theme.colorError
@@ -1211,7 +1211,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
) {
arrowStyles = css`
color: ${
- basicColorColumnFormatters[row.index][column.key]
+ basicColorColumnFormatters[row.index]?.[column.key]
?.arrowColor === ColorSchemeEnum.Green
? theme.colorSuccess
: theme.colorError
diff --git a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
index 80441dab9c4..8a11044e2d8 100644
--- a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
@@ -20,6 +20,7 @@ import '@testing-library/jest-dom';
import {
getTextColorForBackground,
ObjectFormattingEnum,
+ ColorSchemeEnum,
} from '@superset-ui/chart-controls';
import { supersetTheme } from '@apache-superset/core/theme';
import {
@@ -2075,6 +2076,59 @@ describe('plugin-chart-table', () => {
});
});
+ test('does not crash when a comparison-color-formatter array has no entry for a rendered row', () => {
+ // Regression test: the per-cell comparison-color lookups in the Cell
+ // renderer (`basicColorFormatters`/`basicColorColumnFormatters`,
+ // indexed by `row.index`) must stay safe even if those arrays ever
+ // end up with fewer entries than the number of rendered rows -- e.g.
+ // when "Show summary" is combined with time comparison and a
+ // comparison-based conditional color scheme ("Green for increase,
+ // red for decrease") applied to a Time Comparison column. Without
+ // the `?.` guard on the array-index lookup, this throws
+ // `TypeError: Cannot read properties of undefined (reading 'Main
+ // metric_1')`.
+ const propsInput = {
+ ...testData.comparison,
+ rawFormData: {
+ ...testData.comparison.rawFormData,
+ conditional_formatting: [
+ { column: 'Main metric_1', colorScheme: ColorSchemeEnum.Green },
+ ],
+ },
+ };
+ const transformedProps = transformProps(propsInput);
+ expect(transformedProps.data).toHaveLength(2);
+ expect(transformedProps.basicColorColumnFormatters).toHaveLength(2);
+
+ // Simulate the row-count mismatch: the formatter array has an entry
+ // for only the first row, matching the shape of the bug (an entry
+ // missing for one of the rendered rows).
+ const propsWithMissingFormatterEntry = {
+ ...transformedProps,
+ basicColorColumnFormatters:
+ transformedProps.basicColorColumnFormatters!.slice(0, 1),
+ };
+
+ expect(() =>
+ render(
+ <TableChart {...propsWithMissingFormatterEntry} sticky={false} />,
+ ),
+ ).not.toThrow();
+
+ // the row that still has a formatter entry keeps its comparison
+ // background color and arrow: the "Main metric_1" cell for the
+ // first row (value 100) renders before the derived "△ metric_1"
+ // cell that happens to share the same value and aria label.
+ const [styledCell] = screen.getAllByTitle('100');
+ expect(styledCell).toHaveTextContent('↑100');
+ expect(getComputedStyle(styledCell).background).toContain(
+ 'rgba(0, 150, 0, 0.2)',
+ );
+
+ // the row missing a formatter entry still renders its raw value
+ expect(screen.getAllByTitle('110').length).toBeGreaterThan(0);
+ });
+
test('preserves client-side search text across temporal table rerenders', async () => {
const formDataWithSearch = {
...testData.basic.formData,