From af225e3370efa493fb46f1c88ebb571c22814d12 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Fri, 29 Oct 2021 04:35:52 -0600 Subject: [PATCH] Tables: fix sorting for NaN values (#40938) (#41054) Fixes #37236 (cherry picked from commit 97e0e12f4019a1460349339607a5bd287383b83a) Co-authored-by: Derik Evangelista --- .../src/components/Table/Table.test.tsx | 62 +++++++++---------- .../grafana-ui/src/components/Table/Table.tsx | 2 +- .../src/components/Table/utils.test.ts | 3 + .../grafana-ui/src/components/Table/utils.ts | 8 +-- 4 files changed, 37 insertions(+), 38 deletions(-) diff --git a/packages/grafana-ui/src/components/Table/Table.test.tsx b/packages/grafana-ui/src/components/Table/Table.test.tsx index b6dbb7e2d6b..eeb86e323f7 100644 --- a/packages/grafana-ui/src/components/Table/Table.test.tsx +++ b/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]; - const rowTwoLink = () => getLinks(rows[2])[0]; - const rowThreeLink = () => getLinks(rows[3])[0]; - - expect(rows).toHaveLength(4); - expect(within(rows[1]).getByText('2021-01-01 00:00:00')).toBeInTheDocument(); - 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'); + expect(rows).toHaveLength(5); + expect(getRowsData(rows)).toEqual([ + { time: '2021-01-01 00:00:00', temperature: '10', link: '10' }, + { time: '2021-01-01 03:00:00', temperature: 'NaN', link: 'NaN' }, + { time: '2021-01-01 01:00:00', temperature: '11', link: '11' }, + { time: '2021-01-01 02:00:00', temperature: '12', link: '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); - const rowOneLink = () => getLinks(rows[1])[0]; - const rowTwoLink = () => getLinks(rows[2])[0]; - const rowThreeLink = () => getLinks(rows[3])[0]; - - expect(within(rows[1]).getByText('2021-01-01 02:00:00')).toBeInTheDocument(); - 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'); + expect(rows).toHaveLength(5); + expect(getRowsData(rows)).toEqual([ + { time: '2021-01-01 02:00:00', temperature: '12', link: '12' }, + { time: '2021-01-01 01:00:00', temperature: '11', link: '11' }, + { time: '2021-01-01 00:00:00', temperature: '10', link: '10' }, + { time: '2021-01-01 03:00:00', temperature: 'NaN', link: 'NaN' }, + ]); }); }); }); diff --git a/packages/grafana-ui/src/components/Table/Table.tsx b/packages/grafana-ui/src/components/Table/Table.tsx index cc6772b9047..493c85862dd 100644 --- a/packages/grafana-ui/src/components/Table/Table.tsx +++ b/packages/grafana-ui/src/components/Table/Table.tsx @@ -160,7 +160,7 @@ export const Table: FC = 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 }, }), diff --git a/packages/grafana-ui/src/components/Table/utils.test.ts b/packages/grafana-ui/src/components/Table/utils.test.ts index fbe95206592..a8319dcb0d8 100644 --- a/packages/grafana-ui/src/components/Table/utils.test.ts +++ b/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); }); diff --git a/packages/grafana-ui/src/components/Table/utils.ts b/packages/grafana-ui/src/components/Table/utils.ts index 9ce18afdcd8..56c10571c3c 100644 --- a/packages/grafana-ui/src/components/Table/utils.ts +++ b/packages/grafana-ui/src/components/Table/utils.ts @@ -238,13 +238,13 @@ export function sortNumber(rowA: Row, rowB: Row, 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); }