From 98a1dfbad42152834706fe916bde088352e6009b Mon Sep 17 00:00:00 2001 From: Alex Spencer <52186778+alexjonspencer1@users.noreply.github.com> Date: Thu, 27 Mar 2025 14:02:57 -0600 Subject: [PATCH] TableNG: Bugfixes (#102905) * fixes: sort persistence, sort trigger panel dirty state, use field display names * fix: for nested tables, use column.name for header text * chore: fix location of cache display names --- .betterer.results | 3 +- .../Table/TableNG/Cells/HeaderCell.tsx | 25 ++++----- .../src/components/Table/TableNG/TableNG.tsx | 52 ++++++++++++++++--- .../src/components/Table/TableNG/utils.ts | 2 + .../panel/table/table-new/TablePanel.tsx | 3 ++ 5 files changed, 64 insertions(+), 21 deletions(-) diff --git a/.betterer.results b/.betterer.results index d7d99266d8e..d050edb043e 100644 --- a/.betterer.results +++ b/.betterer.results @@ -694,7 +694,8 @@ exports[`better eslint`] = { "packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Do not use any type assertions.", "1"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"] + [0, 0, 0, "Do not use any type assertions.", "2"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "3"] ], "packages/grafana-ui/src/components/Table/TableNG/utils.test.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/packages/grafana-ui/src/components/Table/TableNG/Cells/HeaderCell.tsx b/packages/grafana-ui/src/components/Table/TableNG/Cells/HeaderCell.tsx index 824a71f9186..39ae4da25e7 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/Cells/HeaderCell.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/Cells/HeaderCell.tsx @@ -44,7 +44,7 @@ const HeaderCell: React.FC = ({ crossFilterRows, showTypeIcons, }) => { - const styles = useStyles2(getStyles); + const styles = useStyles2(getStyles, justifyContent); const headerRef = useRef(null); let isColumnFilterable = filterable; @@ -99,7 +99,7 @@ const HeaderCell: React.FC = ({ return (
{ @@ -110,13 +110,9 @@ const HeaderCell: React.FC = ({ > {isColumnFilterable && ( @@ -134,7 +130,12 @@ const HeaderCell: React.FC = ({ ); }; -const getStyles = (theme: GrafanaTheme2) => ({ +const getStyles = (theme: GrafanaTheme2, justifyContent: Property.JustifyContent) => ({ + headerCell: css({ + display: 'flex', + gap: theme.spacing(0.5), + justifyContent, + }), headerCellLabel: css({ border: 'none', padding: 0, @@ -146,7 +147,6 @@ const getStyles = (theme: GrafanaTheme2) => ({ fontWeight: theme.typography.fontWeightMedium, display: 'flex', alignItems: 'center', - marginRight: theme.spacing(0.5), color: theme.colors.text.secondary, gap: theme.spacing(1), @@ -155,9 +155,6 @@ const getStyles = (theme: GrafanaTheme2) => ({ color: theme.colors.text.link, }, }), - sortIcon: css({ - marginLeft: theme.spacing(0.5), - }), }); export { HeaderCell }; diff --git a/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx b/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx index 58a1c83fc64..70019b5a9f7 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx @@ -1,7 +1,14 @@ import 'react-data-grid/lib/styles.css'; import { css } from '@emotion/css'; import { useMemo, useState, useLayoutEffect, useCallback, useRef, useEffect } from 'react'; -import DataGrid, { RenderCellProps, RenderRowProps, Row, SortColumn, DataGridHandle } from 'react-data-grid'; +import DataGrid, { + RenderCellProps, + RenderRowProps, + Row, + SortColumn, + DataGridHandle, + SortDirection, +} from 'react-data-grid'; import { useMeasure } from 'react-use'; import { @@ -65,14 +72,29 @@ export function TableNG(props: TableNGProps) { fieldConfig, footerOptions, height, + initialSortBy, noHeader, onColumnResize, + onSortByChange, width, data, enableSharedCrosshair, showTypeIcons, } = props; + const initialSortColumns = useMemo(() => { + const initialSort = initialSortBy?.map(({ displayName, desc }) => { + const matchingField = data.fields.find(({ state }) => state?.displayName === displayName); + const columnKey = matchingField?.name || displayName; + + return { + columnKey, + direction: (desc ? 'DESC' : 'ASC') as SortDirection, + }; + }); + return initialSort ?? []; + }, []); // eslint-disable-line react-hooks/exhaustive-deps + /* ------------------------------- Local state ------------------------------ */ const [revId, setRevId] = useState(0); const [contextMenuProps, setContextMenuProps] = useState<{ @@ -89,7 +111,7 @@ export function TableNG(props: TableNGProps) { // This state will trigger re-render for recalculating row heights const [, setResizeTrigger] = useState(0); const [, setReadyForRowHeightCalc] = useState(false); - const [sortColumns, setSortColumns] = useState([]); + const [sortColumns, setSortColumns] = useState(initialSortColumns); const [expandedRows, setExpandedRows] = useState([]); const [isNestedTable, setIsNestedTable] = useState(false); const scrollPositionRef = useRef({ x: 0, y: 0 }); @@ -100,7 +122,7 @@ export function TableNG(props: TableNGProps) { const crossFilterRows = useRef>({}); const headerCellRefs = useRef>({}); // TODO: This ref persists sortColumns between renders. setSortColumns is still used to trigger re-render - const sortColumnsRef = useRef(sortColumns); + const sortColumnsRef = useRef(initialSortColumns); const prevProps = useRef(props); const calcsRef = useRef([]); const [paginationWrapperRef, { height: paginationHeight }] = useMeasure(); @@ -364,6 +386,7 @@ export function TableNG(props: TableNGProps) { filter, headerCellRefs, isCountRowsSet, + onSortByChange, osContext, rows, // INFO: sortedRows is for correct row indexing for cell background coloring @@ -438,6 +461,13 @@ export function TableNG(props: TableNGProps) { }; }; + // Reset sortColumns when initialSortBy changes + useEffect(() => { + if (initialSortColumns.length > 0) { + setSortColumns(initialSortColumns); + } + }, [initialSortColumns]); + // Restore scroll position after re-renders useEffect(() => { if (tableRef.current?.element) { @@ -566,6 +596,7 @@ export function mapFrameToDataGrid({ filter, headerCellRefs, isCountRowsSet, + onSortByChange, osContext, rows, sortedRows, @@ -752,9 +783,18 @@ export function mapFrameToDataGrid({ column={column} rows={rows} field={field} - onSort={(columnKey, direction, isMultiSort) => - handleSort(columnKey, direction, isMultiSort, setSortColumns, sortColumnsRef) - } + onSort={(columnKey, direction, isMultiSort) => { + handleSort(columnKey, direction, isMultiSort, setSortColumns, sortColumnsRef); + + // Update panel context with the new sort order + if (onSortByChange) { + const sortByFields = sortColumnsRef.current.map(({ columnKey, direction }) => ({ + displayName: columnKey, + desc: direction === 'DESC', + })); + onSortByChange(sortByFields); + } + }} direction={sortDirection} justifyContent={justifyColumnContent} filter={filter} diff --git a/packages/grafana-ui/src/components/Table/TableNG/utils.ts b/packages/grafana-ui/src/components/Table/TableNG/utils.ts index 5b98a0692d3..addb25edb6d 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/utils.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/utils.ts @@ -22,6 +22,7 @@ import { TableCellDisplayMode, TableCellHeight, TableCellOptions, + TableSortByFieldState, } from '@grafana/schema'; import { TableCellInspectorMode } from '../..'; @@ -477,6 +478,7 @@ export interface MapFrameToGridOptions extends TableNGProps { filter: FilterType; headerCellRefs: React.MutableRefObject>; isCountRowsSet: boolean; + onSortByChange?: (sortBy: TableSortByFieldState[]) => void; osContext: OffscreenCanvasRenderingContext2D | null; rows: TableRow[]; sortedRows: TableRow[]; diff --git a/public/app/plugins/panel/table/table-new/TablePanel.tsx b/public/app/plugins/panel/table/table-new/TablePanel.tsx index f613a36217f..018fbed4146 100644 --- a/public/app/plugins/panel/table/table-new/TablePanel.tsx +++ b/public/app/plugins/panel/table/table-new/TablePanel.tsx @@ -10,6 +10,7 @@ import { PanelProps, SelectableValue, Field, + cacheFieldDisplayNames, } from '@grafana/data'; import { config, PanelDataErrorView } from '@grafana/runtime'; import { Select, usePanelContext, useTheme2 } from '@grafana/ui'; @@ -26,6 +27,8 @@ interface Props extends PanelProps {} export function TablePanel(props: Props) { const { data, height, width, options, fieldConfig, id, timeRange, replaceVariables } = props; + cacheFieldDisplayNames(data.series); + const theme = useTheme2(); const panelContext = usePanelContext(); const frames = hasDeprecatedParentRowIndex(data.series)