diff --git a/packages/grafana-ui/src/components/Table/Table.test.tsx b/packages/grafana-ui/src/components/Table/Table.test.tsx index 8d73374bc10..4b2322923d4 100644 --- a/packages/grafana-ui/src/components/Table/Table.test.tsx +++ b/packages/grafana-ui/src/components/Table/Table.test.tsx @@ -520,7 +520,7 @@ describe('Table', () => { }); describe('when mounted with nested data', () => { - it('then correct rows should be rendered and new table is rendered when expander is clicked', async () => { + beforeEach(() => { const nestedFrame = (idx: number) => applyOverrides( toDataFrame({ @@ -539,6 +539,7 @@ describe('Table', () => { ], }) ); + const defaultFrame = getDefaultDataFrame(); getTestContext({ @@ -555,6 +556,9 @@ describe('Table', () => { ], }), }); + }); + + it('then correct rows should be rendered and new table is rendered when expander is clicked', async () => { expect(getTable()).toBeInTheDocument(); expect(screen.getAllByRole('columnheader')).toHaveLength(4); expect(getColumnHeader(/time/)).toBeInTheDocument(); @@ -593,5 +597,64 @@ describe('Table', () => { expect(within(subTableRows1[2]).getByText(/17%_1/)).toBeInTheDocument(); expect(within(subTableRows1[2]).getByText(/humid_1/)).toBeInTheDocument(); }); + + it('then properly handle row expansion and sorting', async () => { + expect(getTable()).toBeInTheDocument(); + expect(screen.getAllByRole('columnheader')).toHaveLength(4); + expect(getColumnHeader(/time/)).toBeInTheDocument(); + expect(getColumnHeader(/temperature/)).toBeInTheDocument(); + expect(getColumnHeader(/img/)).toBeInTheDocument(); + + let rows = within(getTable()).getAllByRole('row'); + expect(rows).toHaveLength(5); + expect(getRowsData(rows)).toEqual([ + { time: '2021-01-01 00:00:00', temperature: '10', link: '${__value.text} interpolation' }, + { time: '2021-01-01 03:00:00', temperature: 'NaN', link: '${__value.text} interpolation' }, + { time: '2021-01-01 01:00:00', temperature: '11', link: '${__value.text} interpolation' }, + { time: '2021-01-01 02:00:00', temperature: '12', link: '${__value.text} interpolation' }, + ]); + + // Sort rows, and check the new order + const table = getTable(); + await userEvent.click(within(table).getAllByTitle('Toggle SortBy')[0]); + rows = within(table).getAllByRole('row'); + expect(rows).toHaveLength(5); + expect(getRowsData(rows)).toEqual([ + { time: '2021-01-01 00:00:00', temperature: '10', link: '${__value.text} interpolation' }, + { time: '2021-01-01 01:00:00', temperature: '11', link: '${__value.text} interpolation' }, + { time: '2021-01-01 02:00:00', temperature: '12', link: '${__value.text} interpolation' }, + { time: '2021-01-01 03:00:00', temperature: 'NaN', link: '${__value.text} interpolation' }, + ]); + + // No sub table exists before expending a row + let tables = screen.getAllByRole('table'); + expect(tables).toHaveLength(1); + + // Expand a row, and check its height + rows = within(getTable()).getAllByRole('row'); + await userEvent.click(within(rows[1]).getByLabelText('Expand row')); + tables = screen.getAllByRole('table'); + expect(tables).toHaveLength(3); + let subTable = screen.getAllByRole('table')[2]; + expect(subTable.style.height).toBe('108px'); + + // Sort again rows + tables = screen.getAllByRole('table'); + await userEvent.click(within(tables[0]).getAllByTitle('Toggle SortBy')[0]); + rows = within(table).getAllByRole('row'); + expect(rows).toHaveLength(5); + expect(getRowsData(rows)).toEqual([ + { time: '2021-01-01 03:00:00', temperature: 'NaN', link: '${__value.text} interpolation' }, + { time: '2021-01-01 02:00:00', temperature: '12', link: '${__value.text} interpolation' }, + { time: '2021-01-01 01:00:00', temperature: '11', link: '${__value.text} interpolation' }, + { time: '2021-01-01 00:00:00', temperature: '10', link: '${__value.text} interpolation' }, + ]); + + // Expand another row + rows = within(getTable()).getAllByRole('row'); + await userEvent.click(within(rows[1]).getByLabelText('Expand row')); + subTable = screen.getAllByRole('table')[2]; + expect(subTable.style.height).toBe('108px'); + }); }); }); diff --git a/packages/grafana-ui/src/components/Table/Table.tsx b/packages/grafana-ui/src/components/Table/Table.tsx index d7568efa1b7..398732e7922 100644 --- a/packages/grafana-ui/src/components/Table/Table.tsx +++ b/packages/grafana-ui/src/components/Table/Table.tsx @@ -110,8 +110,26 @@ export const Table = memo((props: Props) => { [data, width, columnMinWidth, footerItems, hasNestedData, isCountRowsSet] ); + // we need a ref to later store the `toggleAllRowsExpanded` function, returned by `useTable`. + // We cannot simply use a variable because we need to use such function in the initialization of + // `useTableStateReducer`, which is needed to construct options for `useTable` (the hook that returns + // `toggleAllRowsExpanded`), and if we used a variable, that variable would be undefined at the time + // we initialize `useTableStateReducer`. + const toggleAllRowsExpandedRef = useRef<(value?: boolean) => void>(); + // Internal react table state reducer - const stateReducer = useTableStateReducer(props); + const stateReducer = useTableStateReducer({ + ...props, + onSortByChange: (state) => { + // Collapse all rows. This prevents a known bug that causes the size of the rows to be incorrect due to + // using `VariableSizeList` and `useExpanded` together. + toggleAllRowsExpandedRef.current!(false); + + if (props.onSortByChange) { + props.onSortByChange(state); + } + }, + }); const options: any = useMemo( () => ({ @@ -142,9 +160,17 @@ export const Table = memo((props: Props) => { gotoPage, setPageSize, pageOptions, + toggleAllRowsExpanded, } = useTable(options, useFilters, useSortBy, useAbsoluteLayout, useResizeColumns, useExpanded, usePagination); const extendedState = state as GrafanaTableState; + toggleAllRowsExpandedRef.current = toggleAllRowsExpanded; + + const expandedRowsRepr = JSON.stringify(Object.keys(state.expanded)); + useEffect(() => { + // Reset the list size cache when the expanded rows change + listRef.current?.resetAfterIndex(0); + }, [expandedRowsRepr]); /* Footer value calculation is being moved in the Table component and the footerValues prop will be deprecated. @@ -213,11 +239,9 @@ export const Table = memo((props: Props) => { ); const RenderRow = useCallback( - ({ index: rowIndex, style }: { index: number; style: CSSProperties }) => { - let row = rows[rowIndex]; - if (enablePagination) { - row = page[rowIndex]; - } + ({ index, style }: { index: number; style: CSSProperties }) => { + const indexForPagination = rowIndexForPagination(index); + const row = rows[indexForPagination]; prepareRow(row); @@ -230,7 +254,7 @@ export const Table = memo((props: Props) => { @@ -251,18 +275,17 @@ export const Table = memo((props: Props) => { ); }, [ + rowIndexForPagination, rows, - enablePagination, prepareRow, + state.expanded, tableStyles, nestedDataField, - page, + width, + cellHeight, onCellFilterAdded, timeRange, data, - width, - cellHeight, - state.expanded, ] ); @@ -301,8 +324,9 @@ export const Table = memo((props: Props) => { const getItemSize = (index: number): number => { const indexForPagination = rowIndexForPagination(index); - if (state.expanded[indexForPagination] && nestedDataField) { - return getExpandedRowHeight(nestedDataField, indexForPagination, tableStyles); + const row = rows[indexForPagination]; + if (state.expanded[row.index] && nestedDataField) { + return getExpandedRowHeight(nestedDataField, index, tableStyles); } return tableStyles.rowHeight;