Tables: fix sorting for NaN values (#40938)

Fixes #37236

Signed-off-by: Derik Evangelista <derik.evangelista@grafana.com>

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

@@ -11,7 +11,7 @@ function getDefaultDataFrame(): DataFrame {
       {
         name: 'time',
         type: FieldType.time,
-        values: [1609459200000, 1609462800000, 1609466400000],
+        values: [1609459200000, 1609470000000, 1609462800000, 1609466400000],
         config: {
           custom: {
             filterable: false,
@@ -21,7 +21,7 @@ function getDefaultDataFrame(): DataFrame {
       {
         name: 'temperature',
         type: FieldType.number,
-        values: [10, 11, 12],
+        values: [10, NaN, 11, 12],
         config: {
           custom: {
             filterable: false,
@@ -106,6 +106,19 @@ function getLinks(row: HTMLElement): HTMLElement[] {
   return within(row).getAllByRole('link');
 }
 
+function getRowsData(rows: HTMLElement[]): Object[] {
+  let content = [];
+  for (let i = 1; i < rows.length; i++) {
+    const row = getLinks(rows[i])[0];
+    content.push({
+      time: within(rows[i]).getByText(/2021*/).textContent,
+      temperature: row.textContent,
+      link: row.getAttribute('href'),
+    });
+  }
+  return content;
+}
+
 describe('Table', () => {
   describe('when mounted without data', () => {
     it('then no data to show should be displayed', () => {
@@ -126,23 +139,13 @@ describe('Table', () => {
       expect(getColumnHeader(/img/)).toBeInTheDocument();
 
       const rows = within(getTable()).getAllByRole('row');
-      const rowOneLink = () => getLinks(rows[1])[0];
+      expect(rows).toHaveLength(5);
-      const rowTwoLink = () => getLinks(rows[2])[0];
+      expect(getRowsData(rows)).toEqual([
-      const rowThreeLink = () => getLinks(rows[3])[0];
+        { time: '2021-01-01 00:00:00', temperature: '10', link: '10' },
-
+        { time: '2021-01-01 03:00:00', temperature: 'NaN', link: 'NaN' },
-      expect(rows).toHaveLength(4);
+        { time: '2021-01-01 01:00:00', temperature: '11', link: '11' },
-      expect(within(rows[1]).getByText('2021-01-01 00:00:00')).toBeInTheDocument();
+        { time: '2021-01-01 02:00:00', temperature: '12', link: '12' },
-      expect(getLinks(rows[1])).toHaveLength(2);
+      ]);
-      expect(within(rows[2]).getByText('2021-01-01 01:00:00')).toBeInTheDocument();
-      expect(getLinks(rows[2])).toHaveLength(2);
-      expect(within(rows[3]).getByText('2021-01-01 02:00:00')).toBeInTheDocument();
-      expect(getLinks(rows[3])).toHaveLength(2);
-      expect(rowOneLink()).toHaveTextContent('10');
-      expect(rowOneLink()).toHaveAttribute('href', '10');
-      expect(rowTwoLink()).toHaveTextContent('11');
-      expect(rowTwoLink()).toHaveAttribute('href', '11');
-      expect(rowThreeLink()).toHaveTextContent('12');
-      expect(rowThreeLink()).toHaveAttribute('href', '12');
     });
   });
 
@@ -163,20 +166,13 @@ describe('Table', () => {
       userEvent.click(within(getColumnHeader(/temperature/)).getByText(/temperature/i));
 
       const rows = within(getTable()).getAllByRole('row');
-      expect(rows).toHaveLength(4);
+      expect(rows).toHaveLength(5);
-      const rowOneLink = () => getLinks(rows[1])[0];
+      expect(getRowsData(rows)).toEqual([
-      const rowTwoLink = () => getLinks(rows[2])[0];
+        { time: '2021-01-01 02:00:00', temperature: '12', link: '12' },
-      const rowThreeLink = () => getLinks(rows[3])[0];
+        { time: '2021-01-01 01:00:00', temperature: '11', link: '11' },
-
+        { time: '2021-01-01 00:00:00', temperature: '10', link: '10' },
-      expect(within(rows[1]).getByText('2021-01-01 02:00:00')).toBeInTheDocument();
+        { time: '2021-01-01 03:00:00', temperature: 'NaN', link: 'NaN' },
-      expect(rowOneLink()).toHaveTextContent('12');
+      ]);
-      expect(rowOneLink()).toHaveAttribute('href', '12');
-      expect(within(rows[2]).getByText('2021-01-01 01:00:00')).toBeInTheDocument();
-      expect(rowTwoLink()).toHaveTextContent('11');
-      expect(rowTwoLink()).toHaveAttribute('href', '11');
-      expect(within(rows[3]).getByText('2021-01-01 00:00:00')).toBeInTheDocument();
-      expect(rowThreeLink()).toHaveTextContent('10');
-      expect(rowThreeLink()).toHaveAttribute('href', '10');
     });
   });
 });

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

@@ -160,7 +160,7 @@ export const Table: FC<Props> = memo((props: Props) => {
       stateReducer: stateReducer,
       initialState: getInitialState(initialSortBy, memoizedColumns),
       sortTypes: {
-        number: sortNumber, // should be replace with the builtin number when react-table is upgraded, see https://github.com/tannerlinsley/react-table/pull/3235
+        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
       },
     }),

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

@@ -450,6 +450,9 @@ describe('Table utils', () => {
       ${{ values: [Number.NEGATIVE_INFINITY] }} | ${{ values: [1] }}                        | ${-1}
       ${{ values: ['infinIty'] }}               | ${{ values: [1] }}                        | ${-1}
       ${{ values: ['infinIty'] }}               | ${{ values: [-1] }}                       | ${-1}
+      ${{ values: [1] }}                        | ${{ values: [NaN] }}                      | ${1}
+      ${{ values: [NaN] }}                      | ${{ values: [NaN] }}                      | ${0}
+      ${{ values: [NaN] }}                      | ${{ values: [1] }}                        | ${-1}
     `("when called with a: '$a.toString', b: '$b.toString' then result should be '$expected'", ({ a, b, expected }) => {
       expect(sortNumber(a, b, '0')).toEqual(expected);
     });

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

@@ -238,13 +238,13 @@ export function sortNumber(rowA: Row<any>, rowB: Row<any>, id: string) {
 }
 
 function toNumber(value: any): number {
-  if (typeof value === 'number') {
-    return value;
-  }
-
   if (value === null || value === undefined || value === '' || isNaN(value)) {
     return Number.NEGATIVE_INFINITY;
   }
 
+  if (typeof value === 'number') {
+    return value;
+  }
+
   return Number(value);
 }