Plugins: Fix sorting issue with expandable rows (#75553)

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');
+    });
   });
 });

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) => {
             <ExpandedRow
               nestedData={nestedDataField}
               tableStyles={tableStyles}
-              rowIndex={row.index}
+              rowIndex={index}
               width={width}
               cellHeight={cellHeight}
             />
@@ -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;