From 61c160b30f9b68b0f5c2b89102ad62330789e5be Mon Sep 17 00:00:00 2001 From: Paul Marbach Date: Thu, 21 Aug 2025 16:49:53 -0400 Subject: [PATCH] Table: Fields and Column Widths arrays can get out-of-sync on length (#109997) * Table: Fields and Column Widths arrays can get out-of-sync on length * slight cleanup for e2e * remove a couple of page desctructures --- .../panels-suite/table-kitchenSink.spec.ts | 9 +++++++++ .../src/components/Table/TableNG/hooks.test.ts | 14 ++++++++++++++ .../src/components/Table/TableNG/hooks.ts | 10 ++++++++-- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/e2e-playwright/panels-suite/table-kitchenSink.spec.ts b/e2e-playwright/panels-suite/table-kitchenSink.spec.ts index 7b5f1d84967..fc24bfd6832 100644 --- a/e2e-playwright/panels-suite/table-kitchenSink.spec.ts +++ b/e2e-playwright/panels-suite/table-kitchenSink.spec.ts @@ -133,6 +133,15 @@ test.describe('Panels test: Table - Kitchen Sink', { tag: ['@panels', '@table'] await displayNameInput.fill('State (renamed)'); await displayNameInput.press('Enter'); expect(page.getByRole('row').nth(0)).toContainText('State (renamed)'); + + // toggle the "State" column visibility again to hide it again. this confirms that we avoid bugs related to + // array lengths between the fields array and the column widths array. + await hideStateColumnSwitch.click(); + expect(page.getByRole('row').nth(0)).not.toContainText('State'); + + // since the previous assertion is just for the absence of text, let's also confirm that the table is + // actually still on the page and that an error has not been throw. + await waitForTableLoad(page); }); // we test niche cases for sorting, filtering, pagination, etc. in a unit tests already. diff --git a/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts b/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts index b1351ae11b0..a45dfc12125 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts @@ -551,6 +551,20 @@ describe('TableNG hooks', () => { expect(heightFn).toHaveBeenCalledWith('Longer name that needs wrapping', 26, modifiedFields[0], -1, 22); }); + + it('does not throw if a field has been deleted but the colWidth has not yet been updated', () => { + const { fields } = setupData(); + const { result } = renderHook(() => { + return useHeaderHeight({ + fields, + columnWidths: [100, 100, 100, 100], + enabled: true, + typographyCtx, + sortColumns: [], + }); + }); + expect(result.current).toBe(28); + }); }); describe('useRowHeight', () => { diff --git a/packages/grafana-ui/src/components/Table/TableNG/hooks.ts b/packages/grafana-ui/src/components/Table/TableNG/hooks.ts index 6cd4d836355..7a1be693d8c 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/hooks.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/hooks.ts @@ -353,13 +353,19 @@ export function useHeaderHeight({ const columnAvailableWidths = useMemo( () => columnWidths.map((c, idx) => { + if (idx >= fields.length) { + return 0; // no width available for this column yet + } + let width = c - 2 * TABLE.CELL_PADDING - TABLE.BORDER_RIGHT; + const field = fields[idx]; + // filtering icon - if (fields[idx]?.config?.custom?.filterable) { + if (field.config?.custom?.filterable) { width -= perIconSpace; } // sorting icon - if (sortColumns.some((col) => col.columnKey === getDisplayName(fields[idx]))) { + if (sortColumns.some((col) => col.columnKey === getDisplayName(field))) { width -= perIconSpace; } // type icon