Table Panel: Revert row number column changes (#65470)

* baldm0mma/revert

* baldm0mma/revert

* baldm0mma/revert/ update tests

* baldm0mma/revert/ add back isCountRowsSet condition

* baldm0mma/revert/ update tests

* baldm0mma/revert update annos

packages/grafana-ui/src/components/Table/Table.test.tsx

@@ -152,38 +152,6 @@ describe('Table', () => {
     });
   });
 
-  describe('when `showRowNums` is toggled', () => {
-    const showRowNumsTestContext = {
-      data: toDataFrame({
-        name: 'A',
-        fields: [
-          {
-            name: 'number',
-            type: FieldType.number,
-            values: [1, 1, 1, 2, 2, 3, 4, 5],
-            config: {
-              custom: {
-                filterable: true,
-              },
-            },
-          },
-        ],
-      }),
-    };
-
-    it('should render the (fields.length) rows when `showRowNums` is untoggled', () => {
-      getTestContext({ ...showRowNumsTestContext, showRowNums: false });
-
-      expect(screen.getAllByRole('columnheader')).toHaveLength(1);
-    });
-
-    it('should render (fields.length + 1) rows row when `showRowNums` is toggled', () => {
-      getTestContext({ ...showRowNumsTestContext, showRowNums: true });
-
-      expect(screen.getAllByRole('columnheader')).toHaveLength(2);
-    });
-  });
-
   describe('when mounted with footer', () => {
     it('then footer should be displayed', () => {
       const footerValues = ['a', 'b', 'c'];

packages/grafana-ui/src/components/Table/Table.tsx

@@ -1,4 +1,3 @@
-import { clone } from 'lodash';
 import React, { CSSProperties, memo, useCallback, useEffect, useMemo, useRef, useState, UIEventHandler } from 'react';
 import {
   Cell,
@@ -12,7 +11,7 @@ import {
 } from 'react-table';
 import { VariableSizeList } from 'react-window';
 
-import { DataFrame, Field, ReducerID } from '@grafana/data';
+import { Field, ReducerID } from '@grafana/data';
 import { TableCellHeight } from '@grafana/schema';
 
 import { useTheme2 } from '../../themes';
@@ -33,7 +32,6 @@ import {
   getFooterItems,
   createFooterCalculationValues,
   EXPANDER_WIDTH,
-  buildFieldsForOptionalRowNums,
 } from './utils';
 
 const COLUMN_MIN_WIDTH = 150;
@@ -51,7 +49,6 @@ export const Table = memo((props: Props) => {
     columnMinWidth = COLUMN_MIN_WIDTH,
     noHeader,
     resizable = true,
-    showRowNums,
     initialSortBy,
     footerOptions,
     showTypeIcons,
@@ -95,7 +92,7 @@ export const Table = memo((props: Props) => {
     if (!data.fields.length) {
       return [];
     }
-    // as we only use this to fake the length of our data set for react-table we need to make sure we always return an array
+    // As we only use this to fake the length of our data set for react-table we need to make sure we always return an array
     // filled with values at each index otherwise we'll end up trying to call accessRow for null|undefined value in
     // https://github.com/tannerlinsley/react-table/blob/7be2fc9d8b5e223fc998af88865ae86a88792fdb/src/hooks/useTable.js#L585
     return Array(data.length).fill(0);
@@ -111,8 +108,7 @@ export const Table = memo((props: Props) => {
 
   // React-table column definitions
   const memoizedColumns = useMemo(
-    () =>
+    () => getColumns(data, width, columnMinWidth, !!subData?.length, footerItems, isCountRowsSet),
-      getColumns(addRowNumbersFieldToData(data), width, columnMinWidth, !!subData?.length, footerItems, isCountRowsSet),
     [data, width, columnMinWidth, footerItems, subData, isCountRowsSet]
   );
 
@@ -125,19 +121,20 @@ export const Table = memo((props: Props) => {
       data: memoizedData,
       disableResizing: !resizable,
       stateReducer: stateReducer,
-      initialState: getInitialState(initialSortBy, showRowNums, memoizedColumns),
+      initialState: getInitialState(initialSortBy, memoizedColumns),
       autoResetFilters: false,
       sortTypes: {
         number: sortNumber, // the builtin number type on react-table does not handle NaN values
         'alphanumeric-insensitive': sortCaseInsensitive, // should be replace with the builtin string when react-table is upgraded, see https://github.com/tannerlinsley/react-table/pull/3235
       },
     }),
-    [initialSortBy, showRowNums, memoizedColumns, memoizedData, resizable, stateReducer]
+    [initialSortBy, memoizedColumns, memoizedData, resizable, stateReducer]
   );
 
   const {
     getTableProps,
     headerGroups,
+    footerGroups,
     rows,
     prepareRow,
     totalColumnsWidth,
@@ -146,16 +143,10 @@ export const Table = memo((props: Props) => {
     gotoPage,
     setPageSize,
     pageOptions,
-    setHiddenColumns,
   } = useTable(options, useFilters, useSortBy, useAbsoluteLayout, useResizeColumns, useExpanded, usePagination);
 
   const extendedState = state as GrafanaTableState;
 
-  // Hide Row Number column on toggle
-  useEffect(() => {
-    !!showRowNums ? setHiddenColumns([]) : setHiddenColumns(['0']);
-  }, [showRowNums, setHiddenColumns]);
-
   /*
     Footer value calculation is being moved in the Table component and the footerValues prop will be deprecated.
     The footerValues prop is still used in the Table component for backwards compatibility. Adding the
@@ -180,23 +171,13 @@ export const Table = memo((props: Props) => {
 
     if (isCountRowsSet) {
       const footerItemsCountRows: FooterItem[] = [];
-      /*
+      footerItemsCountRows[0] = headerGroups[0]?.headers[0]?.filteredRows.length.toString() ?? data.length.toString();
-        Update the 1st index of the empty array with the row length, which will then default the 0th index to `undefined`,
-        which will therefore account for our Row Numbers column, and render the count in the following column.
-        This will work with tables with only a single column as well, since our Row Numbers column is being prepended reguardless.
-      */
-      footerItemsCountRows[1] = headerGroups[0]?.headers[0]?.filteredRows.length.toString() ?? data.length.toString();
       setFooterItems(footerItemsCountRows);
       return;
     }
 
     const footerItems = getFooterItems(
-      /*
-        The `headerGroups` object is NOT based on the `data.fields`, but instead on the currently rendered headers in the Table,
-        which may or may not include the Row Numbers column.
-      */
       headerGroups[0].headers as unknown as Array<{ id: string; field: Field }>,
-      // The `rows` object, on the other hand, is based on the `data.fields` data, and therefore ALWAYS include the Row Numbers column data.
       createFooterCalculationValues(rows),
       footerOptions,
       theme
@@ -281,27 +262,16 @@ export const Table = memo((props: Props) => {
         <div {...row.getRowProps({ style })} className={tableStyles.row}>
           {/*add the subtable to the DOM first to prevent a 1px border CSS issue on the last cell of the row*/}
           {renderSubTable(rowIndex)}
-          {row.cells.map((cell: Cell, index: number) => {
+          {row.cells.map((cell: Cell, index: number) => (
-            /*
+            <TableCell
-              Here we test if the `row.cell` is of id === "0"; only if the user has toggled ON `Show row numbers` in the panelOptions panel will this cell exist.
+              key={index}
-              This cell had already been built, but with undefined values. This is so we can now update our empty/undefined `cell.value` to the current `rowIndex + 1`.
+              tableStyles={tableStyles}
-              This will assure that on sort, our row numbers don't also sort; but instewad stay in their respective rows.
+              cell={cell}
-            */
+              onCellFilterAdded={onCellFilterAdded}
-            if (cell.column.id === '0') {
+              columnIndex={index}
-              cell.value = rowIndex + 1;
+              columnCount={row.cells.length}
-            }
+            />
-
+          ))}
-            return (
-              <TableCell
-                key={index}
-                tableStyles={tableStyles}
-                cell={cell}
-                onCellFilterAdded={onCellFilterAdded}
-                columnIndex={index}
-                columnCount={row.cells.length}
-              />
-            );
-          })}
         </div>
       );
     },
@@ -341,19 +311,6 @@ export const Table = memo((props: Props) => {
     );
   }
 
-  // This adds the `Field` data needed to display a column with Row Numbers.
-  function addRowNumbersFieldToData(data: DataFrame): DataFrame {
-    /*
-      The `length` prop in a DataFrame tells us the amount of rows of data that will appear in our table;
-      with that we can build the correct buffered incrementing values for our Row Number column data.
-    */
-    const rowField: Field = buildFieldsForOptionalRowNums(data.length);
-    // Clone data to avoid unwanted mutation.
-    const clonedData = clone(data);
-    clonedData.fields = [rowField, ...data.fields];
-    return clonedData;
-  }
-
   const getItemSize = (index: number): number => {
     if (state.expanded[index]) {
       const rowSubData = subData?.find((frame) => frame.meta?.custom?.parentRowIndex === index);
@@ -406,11 +363,7 @@ export const Table = memo((props: Props) => {
             <FooterRow
               isPaginationVisible={Boolean(enablePagination)}
               footerValues={footerItems}
-              /*
+              footerGroups={footerGroups}
-                The `headerGroups` and `footerGroups` objects destructured from the `useTable` hook are perfectly equivalent, in deep value, but not reference.
-                So we can use `headerGroups` here for building the footer, and no longer have a need for `footerGroups`.
-              */
-              footerGroups={headerGroups}
               totalColumnsWidth={totalColumnsWidth}
               tableStyles={tableStyles}
             />

packages/grafana-ui/src/components/Table/reducer.ts

@@ -65,12 +65,10 @@ export function useTableStateReducer({ onColumnResize, onSortByChange, data }: P
 
 export function getInitialState(
   initialSortBy: Props['initialSortBy'],
-  initialShowRowNumbers: Props['showRowNums'],
   columns: GrafanaTableColumn[]
 ): Partial<GrafanaTableState> {
   const state: Partial<GrafanaTableState> = {
     toggleRowExpandedCounter: 0,
-    hiddenColumns: initialShowRowNumbers ? [] : ['0'],
   };
 
   if (initialSortBy) {

packages/grafana-ui/src/components/Table/types.ts

@@ -77,7 +77,6 @@ export interface Props {
   noHeader?: boolean;
   showTypeIcons?: boolean;
   resizable?: boolean;
-  showRowNums?: boolean;
   initialSortBy?: TableSortByFieldState[];
   onColumnResize?: TableColumnResizeActionCallback;
   onSortByChange?: TableSortByActionCallback;

packages/grafana-ui/src/components/Table/utils.test.tsx

@@ -12,8 +12,6 @@ import {
   sortNumber,
   sortOptions,
   valuesToOptions,
-  buildBufferedEmptyValues,
-  buildFieldsForOptionalRowNums,
 } from './utils';
 
 function getData() {
@@ -377,30 +375,6 @@ describe('Table utils', () => {
     });
   });
 
-  describe('buildBufferedEmptyValues', () => {
-    it('should build a buffered VectorArray of empty values the length of the number passed to it as an argument', () => {
-      const arrayVectorLength = 10;
-      const bufferedArray = buildBufferedEmptyValues(arrayVectorLength);
-      expect(bufferedArray).toBeInstanceOf(ArrayVector);
-
-      // Convert back into a standard array type.
-      const nonBufferedArray = Array.from(bufferedArray);
-      expect(nonBufferedArray[0]).toEqual(undefined);
-      expect(nonBufferedArray[nonBufferedArray.length - 1]).toEqual(undefined);
-    });
-  });
-
-  describe('buildFieldsForOptionalRowNums', () => {
-    it('should prepend a Field to a `DataFrame.field` so row numbers can be calculated and rendered', () => {
-      const builtField = buildFieldsForOptionalRowNums(10);
-
-      expect(builtField['name']).toEqual(' ');
-      expect(builtField['type']).toEqual(FieldType.string);
-      expect(typeof builtField['display']).toBe('function');
-      expect(typeof builtField['config']).toBe('object');
-    });
-  });
-
   describe('getFilteredOptions', () => {
     describe('when called without filterValues', () => {
       it('then it should return an empty array', () => {

packages/grafana-ui/src/components/Table/utils.ts

@@ -43,7 +43,6 @@ import {
 } from './types';
 
 export const EXPANDER_WIDTH = 50;
-export const OPTIONAL_ROW_NUMBER_COLUMN_WIDTH = 50;
 
 export function getTextAlign(field?: Field): Property.JustifyContent {
   if (!field) {
@@ -165,25 +164,6 @@ export function getColumns(
   return columns;
 }
 
-/*
-  Build `Field` data for row numbers and prepend to the field array;
-  this way, on other column's sort, the row numbers will persist in their proper place.
-*/
-export function buildFieldsForOptionalRowNums(totalRows: number): Field {
-  return {
-    ...defaultRowNumberColumnFieldData,
-    values: buildBufferedEmptyValues(totalRows),
-  };
-}
-
-/*
-  This gives us an empty buffered ArrayVector of the desired length to match the table data.
-  It is simply a data placeholder for the Row Number column data.
-*/
-export function buildBufferedEmptyValues(totalRows: number): ArrayVector<string> {
-  return new ArrayVector(new Array(totalRows));
-}
-
 export function getCellComponent(displayMode: TableCellDisplayMode, field: Field): CellComponent {
   switch (displayMode) {
     case TableCellDisplayMode.ColorText:
@@ -338,24 +318,6 @@ export function getFooterItems(
   options: TableFooterCalc,
   theme2: GrafanaTheme2
 ): FooterItem[] {
-  /*
-    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')) {
-    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.
@@ -383,8 +345,8 @@ export function getFooterItems(
   return filterFields.map((data, i) => {
     // 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.
+      // Show the reducer in the first column
-      if (i === 1 && options.reducer && options.reducer.length > 0) {
+      if (i === 0 && options.reducer && options.reducer.length > 0) {
         const reducer = fieldReducers.get(options.reducer[0]);
         return reducer.name;
       }
@@ -494,34 +456,6 @@ export function migrateTableDisplayModeToCellOptions(displayMode: TableCellDispl
   }
 }
 
-/*
-  For building the column data for the togglable Row Number field.
-  `values` property is omitted, as it will be added at a later time.
-*/
-export const defaultRowNumberColumnFieldData: Omit<Field, 'values'> = {
-  /*
-    Single whitespace as value for `name` property so as to render an empty/invisible column header;
-    without the single whitespace, falsey headers (empty strings) are given a default name of "Value".
-  */
-  name: ' ',
-  display: function (value) {
-    return {
-      numeric: Number(value),
-      text: value != null ? String(value) : '',
-    };
-  },
-  type: FieldType.string,
-  config: {
-    color: { mode: 'thresholds' },
-    custom: {
-      align: 'auto',
-      cellOptions: { type: 'auto' },
-      inspect: false,
-      width: OPTIONAL_ROW_NUMBER_COLUMN_WIDTH,
-    },
-  },
-};
-
 /**
  * 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.

public/app/plugins/panel/table/TablePanel.tsx

@@ -5,7 +5,6 @@ import { DataFrame, FieldMatcherID, getFrameDisplayName, PanelProps, SelectableV
 import { PanelDataErrorView } from '@grafana/runtime';
 import { Select, Table, usePanelContext, useTheme2 } from '@grafana/ui';
 import { TableSortByFieldState } from '@grafana/ui/src/components/Table/types';
-import { OPTIONAL_ROW_NUMBER_COLUMN_WIDTH } from '@grafana/ui/src/components/Table/utils';
 
 import { PanelOptions } from './panelcfg.gen';
 
@@ -19,7 +18,7 @@ export function TablePanel(props: Props) {
   const frames = data.series;
   const mainFrames = frames.filter((f) => f.meta?.custom?.parentRowIndex === undefined);
   const subFrames = frames.filter((f) => f.meta?.custom?.parentRowIndex !== undefined);
-  const count = mainFrames.length;
+  const count = mainFrames?.length;
   const hasFields = mainFrames[0]?.fields.length;
   const currentIndex = getCurrentFrameIndex(mainFrames, options);
   const main = mainFrames[currentIndex];
@@ -43,12 +42,11 @@ export function TablePanel(props: Props) {
     <Table
       height={tableHeight}
       // This calculation is to accommodate the optionally rendered Row Numbers Column
-      width={options.showRowNums ? width : width + OPTIONAL_ROW_NUMBER_COLUMN_WIDTH}
+      width={width}
       data={main}
       noHeader={!options.showHeader}
       showTypeIcons={options.showTypeIcons}
       resizable={true}
-      showRowNums={options.showRowNums}
       initialSortBy={options.sortBy}
       onSortByChange={(sortBy) => onSortByChange(sortBy, props)}
       onColumnResize={(displayName, resizedWidth) => onColumnResize(displayName, resizedWidth, props)}

public/app/plugins/panel/table/module.tsx

@@ -126,11 +126,6 @@ export const plugin = new PanelPlugin<PanelOptions, TableFieldOptions>(TablePane
           ],
         },
       })
-      .addBooleanSwitch({
-        path: 'showRowNums',
-        name: 'Show row numbers',
-        defaultValue: defaultPanelOptions.showRowNums,
-      })
       .addBooleanSwitch({
         path: 'footer.show',
         category: [footerCategory],

public/app/plugins/panel/table/panelcfg.cue

@@ -30,8 +30,6 @@ composableKinds: PanelCfg: {
 							frameIndex: number | *0
 							// Controls whether the panel should show the header
 							showHeader: bool | *true
-							// Controls whether the columns should be numbered
-							showRowNums?: bool | *false
 							// Controls whether the header should show icons for the column types
 							showTypeIcons?: bool | *false
 							// Used to control row sorting

public/app/plugins/panel/table/panelcfg.gen.ts

@@ -29,10 +29,6 @@ export interface PanelOptions {
    * Controls whether the panel should show the header
    */
   showHeader: boolean;
-  /**
-   * Controls whether the columns should be numbered
-   */
-  showRowNums?: boolean;
   /**
    * Controls whether the header should show icons for the column types
    */
@@ -61,7 +57,6 @@ export const defaultPanelOptions: Partial<PanelOptions> = {
   },
   frameIndex: 0,
   showHeader: true,
-  showRowNums: false,
   showTypeIcons: false,
   sortBy: [],
 };