You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/08/13 19:00:08 UTC
(superset) branch fix-table-time-comparison-summary-color-crash created (now 2f55cbefd11)
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a change to branch fix-table-time-comparison-summary-color-crash
in repository https://gitbox.apache.org/repos/asf/superset.git
at 2f55cbefd11 fix(plugin-chart-table): prevent crash when summary row + time comparison + comparison color formatting are combined
This branch includes the following new commits:
new 2f55cbefd11 fix(plugin-chart-table): prevent crash when summary row + time comparison + comparison color formatting are combined
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: fix(plugin-chart-table): prevent crash when summary row + time comparison + comparison color formatting are combined
Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch fix-table-time-comparison-summary-color-crash
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 2f55cbefd11120aa061cc6110c0a8f34b7f3a64c
Author: sadpandajoe <jc...@gmail.com>
AuthorDate: Wed Aug 12 22:41:52 2026 +0000
fix(plugin-chart-table): prevent crash when summary row + time comparison + comparison color formatting are combined
Guard the row-indexed comparison-color-formatter lookups
(basicColorFormatters/basicColorColumnFormatters) in the Cell renderer
with optional chaining on the array access itself. Previously only the
final property read was optional-chained, so any row for which the
formatter array has no corresponding entry crashed with
`TypeError: Cannot read properties of undefined (reading 'Main
<metric>')`.
---
.../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,