From 09341a0cd625eb155d5a2f42a103f43dcf41ae27 Mon Sep 17 00:00:00 2001 From: Jev Forsberg <46619047+baldm0mma@users.noreply.github.com> Date: Wed, 8 Mar 2023 06:57:01 -0700 Subject: [PATCH] TablePanel: fix footer bug; no footer calculated values after "hidden" column override (#64269) * baldm0mma/bug/tableFooter/ first commit * baldm0mma/bug/tableFooter/ investigation annotations * baldm0mma/bug/tableFooter/ solution * baldm0mma/bug/tableFooter/ rem conlogs in footerrow.txs * baldm0mma/bug/tableFooter/ rem conlogs in tablepanel.tsx * baldm0mma/bug/tableFooter/ rem conlgs in table.tsx * baldm0mma/bug/tableFooter/ rem conlogs in utils.ts * baldm0mma/bug/tableFooter/ reset return in footerRow.tsx * baldm0mma/bug/tableFooter/ rem unused anno in table.tsx * baldm0mma/bug/tableFooter/ rem unsed annos in utils.ts * baldm0mma/bug/tableFooter/ add addMissingColumnIndex * baldm0mma/bug/tableFooter/ add annos * baldm0mma/bug/tableFooter/ add annos * baldm0mma/bug/tableFooter/ / add annos * baldm0mma/bug/tableFooterFix/ update annos * baldm0mma/bug/tableFooterFix/ update spelling in utils * baldm0mma/bug/tableFooterFix/ rem unused condition in utils.ts * baldm0mma/bug/tableFooterFix/ update anno in utils.ts * Wrap comments and fix misspelling. * baldm0mma/bug/tableFooterFix/ add TSDoc * baldm0mma/bug/tableFooterFix/ update annotations in utils.ts --------- Co-authored-by: Kyle Cunningham --- .../grafana-ui/src/components/Table/utils.ts | 80 ++++++++++++++++--- 1 file changed, 68 insertions(+), 12 deletions(-) diff --git a/packages/grafana-ui/src/components/Table/utils.ts b/packages/grafana-ui/src/components/Table/utils.ts index 5f794438b66..d4ed918755b 100644 --- a/packages/grafana-ui/src/components/Table/utils.ts +++ b/packages/grafana-ui/src/components/Table/utils.ts @@ -99,7 +99,6 @@ export function getColumns( for (const [fieldIndex, field] of data.fields.entries()) { const fieldTableOptions = (field.config.custom || {}) as TableFieldOptions; - if (fieldTableOptions.hidden) { continue; } @@ -319,43 +318,73 @@ function toNumber(value: any): number { } export function getFooterItems( - filterFields: Array<{ id: string; field: Field }>, + filterFields: Array<{ id: string; field?: Field } | undefined>, values: any[number], options: TableFooterCalc, theme2: GrafanaTheme2 ): FooterItem[] { /* - Here, `filterFields` is passed to as the `headerGroups[0].headers` array that was destrcutured from the `useTable` hook. - Unfortunately, since the `headerGroups` object is data based ONLY on the rendered "non-hidden" column headers, - it will NOT include the Row Number column if it has been toggled off. This will shift the rendering of the footer left 1 column, - creating an off-by-one issue. This is why we test for a `field.id` of "0". If the condition is truthy, the togglable Row Number column is being rendered, - and we can proceed normally. If not, we must add the field data in its place so that the footer data renders in the expected column. + Here, `filterFields` is passed as the `headerGroups[0].headers` array + that was destructured from the `useTable` hook. Unfortunately, since + the `headerGroups` object is data based ONLY on the rendered "non-hidden" + column headers, it will NOT include the Row Number column if it has been + toggled off. This will shift the rendering of the footer left 1 column, + creating an off-by-one issue. This is why we test for a `field.id` of "0". + If the condition is truthy, the togglable Row Number column is being rendered, + and we can proceed normally. If not, we must add the field data in its place + so that the footer data renders in the expected column. */ - if (!filterFields.some((field) => field.id === '0')) { + if (!filterFields.some((field) => field?.id === '0')) { const length = values.length; // Build the additional field that will correct the off-by-one footer issue. const fieldToAdd = { id: '0', field: buildFieldsForOptionalRowNums(length) }; filterFields = [fieldToAdd, ...filterFields]; } + /* + The FooterItems[] are calculated using both the `headerGroups[0].headers` + (filterFields) and `rows` (values) destructured from the useTable() hook. + This cacluation is based on the data from each index in `filterFields` + array as well as the corresponding index in the `values` array. + When the user hides a column through an override, the getColumns() + hook is invoked, removes said hidden column, sends the updated column + data to the useTable() hook, which then builds `headerGroups[0].headers` + without the hidden column. However, it doesn't remove the hidden column + from the `row` data, instead it substututes the hidden column row data + with an `undefined` value. Therefore, the `row` array length never changes, + despite the `headerGroups[0].headers` length changing at every column removal. + This makes all footer reduce calculations AFTER the first hidden column + in the `headerGroups[0].headers` break, since the indexing of both + arrays is no longer in parity. + + So, here we simply recursively test for the "hidden" columns + from `headerGroups[0].headers`. Each column has an ID property that corresponds + to its own index, therefore if (`filterField.id` !== `String(index)`), + we know there is one or more hidden columns; at which point we update + the index with an ersatz placeholder with just an `id` property. + */ + addMissingColumnIndex(filterFields); + return filterFields.map((data, i) => { - if (data.field.type !== FieldType.number) { + // Then test for numerical data - this will filter out placeholder `filterFields` as well. + if (data?.field?.type !== FieldType.number) { // Show the reducer type ("Total", "Range", "Count", "Delta", etc) in the first non "Row Number" column, only if it cannot be numerically reduced. if (i === 1 && options.reducer && options.reducer.length > 0) { const reducer = fieldReducers.get(options.reducer[0]); return reducer.name; } - // Otherwise return `undefined`, which will render an . + // Render an . return undefined; } let newField = clone(data.field); - newField.values = new ArrayVector(values[i]); + newField.values = new ArrayVector(values[data.id]); newField.state = undefined; data.field = newField; + if (options.fields && options.fields.length > 0) { - const f = options.fields.find((f) => f === data.field.name); + const f = options.fields.find((f) => f === data?.field?.name); if (f) { return getFormattedValue(data.field, options.reducer, theme2); } @@ -477,3 +506,30 @@ export const defaultRowNumberColumnFieldData: Omit = { }, }, }; + +/** + * This recurses through an array of `filterFields` (Array<{ id: string; field?: Field } | undefined>) + * and adds back the missing indecies that are removed due to hiding a column through an panel override. + * This is necessary to create Array.length parity between the `filterFields` array and the `values` array (any[number]), + * since the footer value calculations are based on the corresponding index values of both arrays. + * + * @remarks + * This function uses the splice() method, and therefore mutates the array. + * + * @param columns - An array of `filterFields` (Array<{ id: string; field?: Field } | undefined>). + * @returns void; this function returns nothing; it only mutates values as a side effect. + */ +function addMissingColumnIndex(columns: Array<{ id: string; field?: Field } | undefined>): void { + const missingIndex = columns.findIndex((field, index) => field?.id !== String(index)); + + // Base case + if (missingIndex === -1) { + return; + } + + // Splice in missing column + columns.splice(missingIndex, 0, { id: String(missingIndex) }); + + // Recurse + addMissingColumnIndex(columns); +}