From 63cc23a3ed55c265012ff7288e95f9e5992e9b29 Mon Sep 17 00:00:00 2001 From: Paul Marbach Date: Wed, 23 Jul 2025 18:06:31 -0400 Subject: [PATCH] TableNG: Footer should render summary value in first column if present (#108550) * TableNG: Footer should render summary value in first column if present * fix issue where empty fields array meant no calcs were shown * fix a bug related to hidden fields * go away from fieldmatcher to use a simple inline solution --- .../src/components/Table/TableNG/TableNG.tsx | 18 +++++++--- .../components/Table/TableNG/hooks.test.ts | 33 +++++++++++------ .../src/components/Table/TableNG/hooks.ts | 36 ++++++++----------- 3 files changed, 50 insertions(+), 37 deletions(-) diff --git a/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx b/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx index 8c4e1e62959..c84fb2d4194 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx @@ -210,7 +210,11 @@ export function TableNG(props: TableNGProps) { }); // Create a map of column key to text wrap - const footerCalcs = useFooterCalcs(sortedRows, data, { enabled: hasFooter, footerOptions, isCountRowsSet }); + const footerCalcs = useFooterCalcs(sortedRows, visibleFields, { + enabled: hasFooter, + footerOptions, + isCountRowsSet, + }); const applyToRowBgFn = useMemo(() => getApplyToRowBgFn(data.fields, theme) ?? undefined, [data.fields, theme]); const renderRow = useMemo( @@ -791,6 +795,8 @@ const getGridStyles = ( '--rdg-header-background-color': transparent ? theme.colors.background.canvas : theme.colors.background.primary, '--rdg-border-color': theme.colors.border.weak, '--rdg-color': theme.colors.text.primary, + '--rdg-summary-border-color': theme.colors.border.weak, + '--rdg-summary-border-width': '1px', // note: this cannot have any transparency since default cells that // overlay/overflow on hover inherit this background and need to occlude cells below @@ -807,6 +813,12 @@ const getGridStyles = ( border: 'none', + '.rdg-cell': { + '&:last-child': { + borderInlineEnd: 'none', + }, + }, + // add a box shadow on hover and selection for all body cells '& > :not(.rdg-summary-row, .rdg-header-row) > .rdg-cell': { '&:hover, &[aria-selected=true]': { @@ -941,10 +953,6 @@ const getCellStyles = ( ...(shouldWrap && { whiteSpace: isMonospace ? 'pre' : 'pre-line' }), ...(isMonospace && { fontFamily: 'monospace' }), - '&:last-child': { - borderInlineEnd: 'none', - }, - // should omit if no cell actions, and no shouldOverflow '&:hover, &[aria-selected=true]': { '.table-cell-actions': { 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 afbc4fff691..4df9128205a 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts @@ -288,7 +288,7 @@ describe('TableNG hooks', () => { const { result } = renderHook(() => { const data = createDataFrame({ fields: [textField, numericField] }); cacheFieldDisplayNames([data]); - return useFooterCalcs(rows, data, { + return useFooterCalcs(rows, data.fields, { enabled: true, footerOptions: { show: true, reducer: ['sum'] }, }); @@ -301,7 +301,7 @@ describe('TableNG hooks', () => { const { result } = renderHook(() => { const data = createDataFrame({ fields: [textField, numericField] }); cacheFieldDisplayNames([data]); - return useFooterCalcs(rows, data, { + return useFooterCalcs(rows, data.fields, { enabled: true, footerOptions: { show: true, reducer: ['mean'] }, }); @@ -314,7 +314,7 @@ describe('TableNG hooks', () => { const { result } = renderHook(() => { const data = createDataFrame({ fields: [textField, textField] }); cacheFieldDisplayNames([data]); - return useFooterCalcs(rows, data, { + return useFooterCalcs(rows, data.fields, { enabled: true, footerOptions: { show: true, reducer: ['sum'] }, }); @@ -327,7 +327,7 @@ describe('TableNG hooks', () => { const { result } = renderHook(() => { const data = createDataFrame({ fields: [textField, numericField, numericField2] }); cacheFieldDisplayNames([data]); - return useFooterCalcs(rows, data, { + return useFooterCalcs(rows, data.fields, { enabled: true, footerOptions: undefined, }); @@ -340,7 +340,7 @@ describe('TableNG hooks', () => { const { result } = renderHook(() => { const data = createDataFrame({ fields: [textField, numericField, numericField2] }); cacheFieldDisplayNames([data]); - return useFooterCalcs(rows, data, { + return useFooterCalcs(rows, data.fields, { enabled: false, footerOptions: { show: true, reducer: ['sum'] }, }); @@ -353,7 +353,7 @@ describe('TableNG hooks', () => { const { result } = renderHook(() => { const data = createDataFrame({ fields: [textField, textField] }); cacheFieldDisplayNames([data]); - return useFooterCalcs(rows, data, { + return useFooterCalcs(rows, data.fields, { enabled: true, footerOptions: { show: true, reducer: undefined }, }); @@ -366,7 +366,7 @@ describe('TableNG hooks', () => { const { result } = renderHook(() => { const data = createDataFrame({ fields: [textField, numericField, numericField2] }); cacheFieldDisplayNames([data]); - return useFooterCalcs(rows, data, { + return useFooterCalcs(rows, data.fields, { enabled: true, footerOptions: { show: true, reducer: [] }, }); @@ -379,7 +379,7 @@ describe('TableNG hooks', () => { const { result } = renderHook(() => { const data = createDataFrame({ fields: [textField, numericField, numericField2] }); cacheFieldDisplayNames([data]); - return useFooterCalcs(rows, data, { + return useFooterCalcs(rows, data.fields, { enabled: true, footerOptions: { show: true, reducer: ['sum'], fields: ['Field2', 'Field3'] }, }); @@ -392,7 +392,7 @@ describe('TableNG hooks', () => { const { result } = renderHook(() => { const data = createDataFrame({ fields: [textField, numericField, numericField2] }); cacheFieldDisplayNames([data]); - return useFooterCalcs(rows, data, { + return useFooterCalcs(rows, data.fields, { enabled: true, footerOptions: { show: true, reducer: ['sum'], fields: ['Field1', 'Field2', 'Field3'] }, }); @@ -405,7 +405,7 @@ describe('TableNG hooks', () => { const { result } = renderHook(() => { const data = createDataFrame({ fields: [textField, numericField, numericField2] }); cacheFieldDisplayNames([data]); - return useFooterCalcs(rows, data, { + return useFooterCalcs(rows, data.fields, { enabled: true, footerOptions: { show: true, reducer: ['sum'], fields: ['Field1', 'Field 2'] }, }); @@ -413,6 +413,19 @@ describe('TableNG hooks', () => { expect(result.current).toEqual(['Total', '6', '13']); }); + + it('should not return the reducer label in the first column if there is a calc to render', () => { + const { result } = renderHook(() => { + const data = createDataFrame({ fields: [numericField, numericField2] }); + cacheFieldDisplayNames([data]); + return useFooterCalcs(rows, data.fields, { + enabled: true, + footerOptions: { show: true, reducer: ['sum'], fields: [] }, + }); + }); + + expect(result.current).toEqual(['6', '13']); + }); }); describe('useHeaderHeight', () => { diff --git a/packages/grafana-ui/src/components/Table/TableNG/hooks.ts b/packages/grafana-ui/src/components/Table/TableNG/hooks.ts index a11be3d165a..e5f30a68d27 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/hooks.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/hooks.ts @@ -2,16 +2,7 @@ import { useState, useMemo, useEffect, useCallback, useRef, useLayoutEffect, Ref import { Column, DataGridHandle, DataGridProps, SortColumn } from 'react-data-grid'; import { varPreLine } from 'uwrap'; -import { - DataFrame, - Field, - FieldMatcherID, - fieldReducers, - FieldType, - formattedValueToString, - getFieldMatcher, - reduceField, -} from '@grafana/data'; +import { Field, fieldReducers, FieldType, formattedValueToString, reduceField } from '@grafana/data'; import { useTheme2 } from '../../../themes/ThemeContext'; import { TableCellDisplayMode, TableColumnResizeActionCallback } from '../types'; @@ -264,7 +255,8 @@ export interface FooterCalcsOptions { export function useFooterCalcs( rows: TableRow[], - data: DataFrame, + // it's very important that this is the _visible_ fields. + fields: Field[], { enabled, footerOptions, isCountRowsSet }: FooterCalcsOptions ): string[] { return useMemo(() => { @@ -274,11 +266,9 @@ export function useFooterCalcs( return []; } - const fieldNameMatcher = footerOptions.fields - ? getFieldMatcher({ id: FieldMatcherID.byNames, options: { names: footerOptions.fields } }) - : undefined; + const fieldNameSet = footerOptions.fields?.length ? new Set(footerOptions.fields) : null; - return data.fields.map((field, index) => { + return fields.map((field, index) => { if (field.state?.calcs) { delete field.state?.calcs; } @@ -287,25 +277,27 @@ export function useFooterCalcs( return index === 0 ? `${rows.length}` : ''; } + let emptyValue = ''; if (index === 0) { const footerCalcReducer = footerReducers[0]; - return footerCalcReducer ? fieldReducers.get(footerCalcReducer).name : ''; + emptyValue = footerCalcReducer ? fieldReducers.get(footerCalcReducer).name : ''; } if (field.type !== FieldType.number) { - return ''; + return emptyValue; } // if field.display is undefined, don't throw const displayFn = field.display; if (!displayFn) { - return ''; + return emptyValue; } // If fields array is specified, only show footer for fields included in that array. - // the array can include either the display name or the field name. - if (fieldNameMatcher && !fieldNameMatcher(field, data, [data])) { - return ''; + // the array can include either the display name or the field name. we don't use a field matcher + // because that requires us to drill the data frame down here. + if (fieldNameSet && !fieldNameSet.has(getDisplayName(field)) && !fieldNameSet.has(field.name)) { + return emptyValue; } const calc = footerReducers[0]; @@ -319,7 +311,7 @@ export function useFooterCalcs( return formattedValueToString(displayFn(value)); }); - }, [data, enabled, footerOptions, isCountRowsSet, rows]); + }, [fields, enabled, footerOptions, isCountRowsSet, rows]); } interface TypographyCtx {