From 5e1d0a8851dc18893b663b44be14875f60c4e69b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 7 Apr 2021 16:06:51 +0200 Subject: [PATCH] Table: Fixes table data links so they refer to correct row after sorting (#32571) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Table: Fixes table data links so they refer to correct row after sorting * Tests: adds some basic tests incl sorting * Refactor: removes dependeny on app Co-authored-by: Hugo Häggmark --- .../src/components/Table/Table.test.tsx | 167 ++++++++++++++++++ .../grafana-ui/src/components/Table/Table.tsx | 2 +- .../src/components/Table/TableCell.tsx | 7 +- 3 files changed, 172 insertions(+), 4 deletions(-) create mode 100644 packages/grafana-ui/src/components/Table/Table.test.tsx diff --git a/packages/grafana-ui/src/components/Table/Table.test.tsx b/packages/grafana-ui/src/components/Table/Table.test.tsx new file mode 100644 index 00000000000..69b5af5c6a1 --- /dev/null +++ b/packages/grafana-ui/src/components/Table/Table.test.tsx @@ -0,0 +1,167 @@ +import React from 'react'; +import { render, screen, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { applyFieldOverrides, DataFrame, FieldType, toDataFrame } from '@grafana/data'; + +import { Props, Table } from './Table'; +import { getTheme } from '../../themes'; + +function getDefaultDataFrame(): DataFrame { + const dataFrame = toDataFrame({ + name: 'A', + fields: [ + { + name: 'time', + type: FieldType.time, + values: [1609459200000, 1609462800000, 1609466400000], + config: { + custom: { + filterable: false, + }, + }, + }, + { + name: 'temperature', + type: FieldType.number, + values: [10, 11, 12], + config: { + custom: { + filterable: false, + }, + links: [ + { + targetBlank: true, + title: 'Value link', + url: '${__value.text}', + }, + ], + }, + }, + { + name: 'img', + type: FieldType.string, + values: ['data:image/png;base64,1', 'data:image/png;base64,2', 'data:image/png;base64,3'], + config: { + custom: { + filterable: false, + displayMode: 'image', + }, + links: [ + { + targetBlank: true, + title: 'Image link', + url: '${__value.text}', + }, + ], + }, + }, + ], + }); + const dataFrames = applyFieldOverrides({ + data: [dataFrame], + fieldConfig: { + defaults: {}, + overrides: [], + }, + replaceVariables: (value, vars, format) => { + return vars && value === '${__value.text}' ? vars['__value'].value.text : value; + }, + timeZone: 'utc', + theme: getTheme(), + }); + return dataFrames[0]; +} + +function getTestContext(propOverrides: Partial = {}) { + const onSortByChange = jest.fn(); + const onCellFilterAdded = jest.fn(); + const onColumnResize = jest.fn(); + const props: Props = { + ariaLabel: 'aria-label', + data: getDefaultDataFrame(), + height: 600, + width: 800, + onSortByChange, + onCellFilterAdded, + onColumnResize, + }; + + Object.assign(props, propOverrides); + const { rerender } = render(); + + return { rerender, onSortByChange, onCellFilterAdded, onColumnResize }; +} + +function getTable(): HTMLElement { + return screen.getByRole('table'); +} + +function getColumnHeader(name: string | RegExp): HTMLElement { + return within(getTable()).getByRole('columnheader', { name }); +} + +describe('Table', () => { + describe('when mounted without data', () => { + it('then no data to show should be displayed', () => { + getTestContext({ data: toDataFrame([]) }); + expect(getTable()).toBeInTheDocument(); + expect(screen.queryByRole('row')).not.toBeInTheDocument(); + expect(screen.getByText(/no data to show/i)).toBeInTheDocument(); + }); + }); + + describe('when mounted with data', () => { + it('then correct rows should be rendered', () => { + getTestContext(); + expect(getTable()).toBeInTheDocument(); + expect(screen.getAllByRole('columnheader')).toHaveLength(3); + expect(getColumnHeader(/time/)).toBeInTheDocument(); + expect(getColumnHeader(/temperature/)).toBeInTheDocument(); + expect(getColumnHeader(/img/)).toBeInTheDocument(); + + const rows = within(getTable()).getAllByRole('row'); + expect(rows).toHaveLength(4); + expect(within(rows[1]).getByRole('cell', { name: '2021-01-01 00:00:00' })).toBeInTheDocument(); + expect(within(rows[1]).getByRole('cell', { name: '10' })).toBeInTheDocument(); + expect(within(rows[2]).getByRole('cell', { name: '2021-01-01 01:00:00' })).toBeInTheDocument(); + expect(within(rows[2]).getByRole('cell', { name: '11' })).toBeInTheDocument(); + expect(within(rows[3]).getByRole('cell', { name: '2021-01-01 02:00:00' })).toBeInTheDocument(); + expect(within(rows[3]).getByRole('cell', { name: '12' })).toBeInTheDocument(); + expect(within(rows[1]).getByRole('cell', { name: '10' }).closest('a')).toHaveAttribute('href', '10'); + expect(within(rows[2]).getByRole('cell', { name: '11' }).closest('a')).toHaveAttribute('href', '11'); + expect(within(rows[3]).getByRole('cell', { name: '12' }).closest('a')).toHaveAttribute('href', '12'); + }); + }); + + describe('when sorting with columnheader', () => { + it('then correct rows should be rendered', () => { + getTestContext(); + + userEvent.click(within(getColumnHeader(/temperature/)).getByText(/temperature/i)); + userEvent.click(within(getColumnHeader(/temperature/)).getByText(/temperature/i)); + + const rows = within(getTable()).getAllByRole('row'); + expect(rows).toHaveLength(4); + expect(within(rows[1]).getByRole('cell', { name: '2021-01-01 02:00:00' })).toBeInTheDocument(); + expect(within(rows[1]).getByRole('cell', { name: '12' })).toBeInTheDocument(); + expect(within(rows[2]).getByRole('cell', { name: '2021-01-01 01:00:00' })).toBeInTheDocument(); + expect(within(rows[2]).getByRole('cell', { name: '11' })).toBeInTheDocument(); + expect(within(rows[3]).getByRole('cell', { name: '2021-01-01 00:00:00' })).toBeInTheDocument(); + expect(within(rows[3]).getByRole('cell', { name: '10' })).toBeInTheDocument(); + }); + + describe('and clicking on links', () => { + it('then correct row data should be in link', () => { + getTestContext(); + + userEvent.click(within(getColumnHeader(/temperature/)).getByText(/temperature/i)); + userEvent.click(within(getColumnHeader(/temperature/)).getByText(/temperature/i)); + + const rows = within(getTable()).getAllByRole('row'); + expect(within(rows[1]).getByRole('cell', { name: '12' }).closest('a')).toHaveAttribute('href', '12'); + expect(within(rows[2]).getByRole('cell', { name: '11' }).closest('a')).toHaveAttribute('href', '11'); + expect(within(rows[3]).getByRole('cell', { name: '10' }).closest('a')).toHaveAttribute('href', '10'); + }); + }); + }); +}); diff --git a/packages/grafana-ui/src/components/Table/Table.tsx b/packages/grafana-ui/src/components/Table/Table.tsx index 766089df8f8..2672ead0744 100644 --- a/packages/grafana-ui/src/components/Table/Table.tsx +++ b/packages/grafana-ui/src/components/Table/Table.tsx @@ -183,7 +183,7 @@ export const Table: FC = memo((props: Props) => { onCellFilterAdded={onCellFilterAdded} columnIndex={index} columnCount={row.cells.length} - rowIndex={rowIndex} + dataRowIndex={row.index} /> ))} diff --git a/packages/grafana-ui/src/components/Table/TableCell.tsx b/packages/grafana-ui/src/components/Table/TableCell.tsx index abbd77cfd9f..8a45d367668 100644 --- a/packages/grafana-ui/src/components/Table/TableCell.tsx +++ b/packages/grafana-ui/src/components/Table/TableCell.tsx @@ -11,7 +11,8 @@ export interface Props { onCellFilterAdded?: TableFilterActionCallback; columnIndex: number; columnCount: number; - rowIndex: number; + /** Index before table sort */ + dataRowIndex: number; } export const TableCell: FC = ({ @@ -21,7 +22,7 @@ export const TableCell: FC = ({ onCellFilterAdded, columnIndex, columnCount, - rowIndex, + dataRowIndex, }) => { const cellProps = cell.getCellProps(); @@ -42,7 +43,7 @@ export const TableCell: FC = ({ } const link: LinkModel | undefined = field.getLinks?.({ - valueRowIndex: rowIndex, + valueRowIndex: dataRowIndex, })[0]; let onClick: MouseEventHandler | undefined;