Table: datalink to have text underline and support for image datalink (#34635) (#35029)

* Table: datalink to have text underline and support for image datalink

* fixes image oversize issue when using both image and link in a column

* fixes small nit

* extracted the getLink logic to be a standalone utility function

* Updates table tests to suit current structure

* fixes small syntax nit

* fixes bad typing issue

* annotes the getCellLinks logic as an internal utility function

* removes blank whitespace

* Tests: updates test cases to use getByRole

Co-authored-by: Hugo Häggmark <hugo.haggmark@gmail.com>
(cherry picked from commit 865eac309c)

Co-authored-by: Uchechukwu Obasi <obasiuche62@gmail.com>

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

@@ -5,10 +5,10 @@ import { TableCellDisplayMode, TableCellProps } from './types';
 import tinycolor from 'tinycolor2';
 import { TableStyles } from './styles';
 import { FilterActions } from './FilterActions';
-import { getTextColorForBackground } from '../../utils';
+import { getTextColorForBackground, getCellLinks } from '../../utils';
 
 export const DefaultCell: FC<TableCellProps> = (props) => {
-  const { field, cell, tableStyles, cellProps } = props;
+  const { field, cell, tableStyles, row, cellProps } = props;
 
   const displayValue = field.display!(cell.value);
 
@@ -22,9 +22,16 @@ export const DefaultCell: FC<TableCellProps> = (props) => {
   const cellStyle = getCellStyle(tableStyles, field, displayValue);
   const showFilters = field.config.filterable;
 
+  const { link, onClick } = getCellLinks(field, row);
+
   return (
     <div {...cellProps} className={cellStyle}>
-      <div className={tableStyles.cellText}>{value}</div>
+      {!link && <div className={tableStyles.cellText}>{value}</div>}
+      {link && (
+        <a href={link.href} onClick={onClick} target={link.target} title={link.title} className={tableStyles.cellLink}>
+          {value}
+        </a>
+      )}
       {showFilters && cell.value !== undefined && <FilterActions {...props} />}
     </div>
   );

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

@@ -1,14 +1,28 @@
 import React, { FC } from 'react';
+import { getCellLinks } from '../../utils';
 import { TableCellProps } from './types';
 
 export const ImageCell: FC<TableCellProps> = (props) => {
-  const { field, cell, tableStyles, cellProps } = props;
+  const { field, cell, tableStyles, row, cellProps } = props;
 
   const displayValue = field.display!(cell.value);
 
+  const { link, onClick } = getCellLinks(field, row);
+
   return (
     <div {...cellProps} className={tableStyles.cellContainer}>
-      <img src={displayValue.text} className={tableStyles.imageCell} />
+      {!link && <img src={displayValue.text} className={tableStyles.imageCell} />}
+      {link && (
+        <a
+          href={link.href}
+          onClick={onClick}
+          target={link.target}
+          title={link.title}
+          className={tableStyles.imageCellLink}
+        >
+          <img src={displayValue.text} className={tableStyles.imageCell} />
+        </a>
+      )}
     </div>
   );
 };

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

@@ -98,6 +98,10 @@ function getColumnHeader(name: string | RegExp): HTMLElement {
   return within(getTable()).getByRole('columnheader', { name });
 }
 
+function getLinks(row: HTMLElement): HTMLElement[] {
+  return within(row).getAllByRole('link');
+}
+
 describe('Table', () => {
   describe('when mounted without data', () => {
     it('then no data to show should be displayed', () => {
@@ -118,20 +122,27 @@ 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]).getByRole('cell', { name: '2021-01-01 00:00:00' })).toBeInTheDocument();
+      expect(within(rows[1]).getByText('2021-01-01 00:00:00')).toBeInTheDocument();
-      expect(within(rows[1]).getByRole('cell', { name: '10' })).toBeInTheDocument();
+      expect(getLinks(rows[1])).toHaveLength(2);
-      expect(within(rows[2]).getByRole('cell', { name: '2021-01-01 01:00:00' })).toBeInTheDocument();
+      expect(within(rows[2]).getByText('2021-01-01 01:00:00')).toBeInTheDocument();
-      expect(within(rows[2]).getByRole('cell', { name: '11' })).toBeInTheDocument();
+      expect(getLinks(rows[2])).toHaveLength(2);
-      expect(within(rows[3]).getByRole('cell', { name: '2021-01-01 02:00:00' })).toBeInTheDocument();
+      expect(within(rows[3]).getByText('2021-01-01 02:00:00')).toBeInTheDocument();
-      expect(within(rows[3]).getByRole('cell', { name: '12' })).toBeInTheDocument();
+      expect(getLinks(rows[3])).toHaveLength(2);
-      expect(within(rows[1]).getByRole('cell', { name: '10' }).closest('a')).toHaveAttribute('href', '10');
+      expect(rowOneLink()).toHaveTextContent('10');
-      expect(within(rows[2]).getByRole('cell', { name: '11' }).closest('a')).toHaveAttribute('href', '11');
+      expect(rowOneLink()).toHaveAttribute('href', '10');
-      expect(within(rows[3]).getByRole('cell', { name: '12' }).closest('a')).toHaveAttribute('href', '12');
+      expect(rowTwoLink()).toHaveTextContent('11');
+      expect(rowTwoLink()).toHaveAttribute('href', '11');
+      expect(rowThreeLink()).toHaveTextContent('12');
+      expect(rowThreeLink()).toHaveAttribute('href', '12');
     });
   });
 
-  describe('when sorting with columnheader', () => {
+  describe('when sorting with column header', () => {
     it('then correct rows should be rendered', () => {
       getTestContext();
 
@@ -140,26 +151,19 @@ describe('Table', () => {
 
       const rows = within(getTable()).getAllByRole('row');
       expect(rows).toHaveLength(4);
-      expect(within(rows[1]).getByRole('cell', { name: '2021-01-01 02:00:00' })).toBeInTheDocument();
+      const rowOneLink = () => getLinks(rows[1])[0];
-      expect(within(rows[1]).getByRole('cell', { name: '12' })).toBeInTheDocument();
+      const rowTwoLink = () => getLinks(rows[2])[0];
-      expect(within(rows[2]).getByRole('cell', { name: '2021-01-01 01:00:00' })).toBeInTheDocument();
+      const rowThreeLink = () => getLinks(rows[3])[0];
-      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', () => {
+      expect(within(rows[1]).getByText('2021-01-01 02:00:00')).toBeInTheDocument();
-      it('then correct row data should be in link', () => {
+      expect(rowOneLink()).toHaveTextContent('12');
-        getTestContext();
+      expect(rowOneLink()).toHaveAttribute('href', '12');
-
+      expect(within(rows[2]).getByText('2021-01-01 01:00:00')).toBeInTheDocument();
-        userEvent.click(within(getColumnHeader(/temperature/)).getByText(/temperature/i));
+      expect(rowTwoLink()).toHaveTextContent('11');
-        userEvent.click(within(getColumnHeader(/temperature/)).getByText(/temperature/i));
+      expect(rowTwoLink()).toHaveAttribute('href', '11');
-
+      expect(within(rows[3]).getByText('2021-01-01 00:00:00')).toBeInTheDocument();
-        const rows = within(getTable()).getAllByRole('row');
+      expect(rowThreeLink()).toHaveTextContent('10');
-        expect(within(rows[1]).getByRole('cell', { name: '12' }).closest('a')).toHaveAttribute('href', '12');
+      expect(rowThreeLink()).toHaveAttribute('href', '10');
-        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');
-      });
     });
   });
 });

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

@@ -185,7 +185,6 @@ export const Table: FC<Props> = memo((props: Props) => {
               onCellFilterAdded={onCellFilterAdded}
               columnIndex={index}
               columnCount={row.cells.length}
-              dataRowIndex={row.index}
             />
           ))}
         </div>

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

@@ -1,6 +1,6 @@
-import React, { FC, MouseEventHandler } from 'react';
+import React, { FC } from 'react';
 import { Cell } from 'react-table';
-import { Field, LinkModel } from '@grafana/data';
+import { Field } from '@grafana/data';
 import { TableFilterActionCallback } from './types';
 import { TableStyles } from './styles';
 
@@ -11,19 +11,9 @@ export interface Props {
   onCellFilterAdded?: TableFilterActionCallback;
   columnIndex: number;
   columnCount: number;
-  /** Index before table sort */
-  dataRowIndex: number;
 }
 
-export const TableCell: FC<Props> = ({
+export const TableCell: FC<Props> = ({ cell, field, tableStyles, onCellFilterAdded, columnIndex, columnCount }) => {
-  cell,
-  field,
-  tableStyles,
-  onCellFilterAdded,
-  columnIndex,
-  columnCount,
-  dataRowIndex,
-}) => {
   const cellProps = cell.getCellProps();
 
   if (!field.display) {
@@ -42,33 +32,11 @@ export const TableCell: FC<Props> = ({
     innerWidth -= tableStyles.lastChildExtraPadding;
   }
 
-  const link: LinkModel | undefined = field.getLinks?.({
+  return cell.render('Cell', {
-    valueRowIndex: dataRowIndex,
-  })[0];
-
-  let onClick: MouseEventHandler<HTMLAnchorElement> | undefined;
-  if (link?.onClick) {
-    onClick = (event) => {
-      // Allow opening in new tab
-      if (!(event.ctrlKey || event.metaKey || event.shiftKey) && link!.onClick) {
-        event.preventDefault();
-        link!.onClick(event);
-      }
-    };
-  }
-
-  const renderedCell = cell.render('Cell', {
     field,
     tableStyles,
     onCellFilterAdded,
     cellProps,
     innerWidth,
-  });
+  }) as React.ReactElement;
-  return link ? (
-    <a href={link.href} onClick={onClick} target={link.target} title={link.title} className={tableStyles.cellLink}>
-      {renderedCell}
-    </a>
-  ) : (
-    <>{renderedCell}</>
-  );
 };

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

@@ -103,6 +103,12 @@ export const getTableStyles = (theme: GrafanaTheme2) => {
       white-space: nowrap;
       text-decoration: underline;
     `,
+    imageCellLink: css`
+      cursor: pointer;
+      overflow: hidden;
+      width: 100%;
+      height: 100%;
+    `,
     headerFilter: css`
       label: headerFilter;
       cursor: pointer;

packages/grafana-ui/src/utils/index.ts

@@ -4,6 +4,7 @@ export * from './slate';
 export * from './dataLinks';
 export * from './tags';
 export * from './scrollbar';
+export * from './table';
 export * from './measureText';
 export * from './useForceUpdate';
 export { SearchFunctionType } from './searchFunctions';

packages/grafana-ui/src/utils/table.ts

@@ -0,0 +1,30 @@
+import { Field, LinkModel } from '@grafana/data';
+import { MouseEventHandler } from 'react';
+import { Row } from 'react-table';
+
+/**
+ * @internal
+ */
+export const getCellLinks = (field: Field, row: Row<any>) => {
+  let link: LinkModel<any> | undefined;
+  let onClick: MouseEventHandler<HTMLAnchorElement> | undefined;
+  if (field.getLinks) {
+    link = field.getLinks({
+      valueRowIndex: row.index,
+    })[0];
+  }
+
+  if (link && link.onClick) {
+    onClick = (event) => {
+      // Allow opening in new tab
+      if (!(event.ctrlKey || event.metaKey || event.shiftKey) && link!.onClick) {
+        event.preventDefault();
+        link!.onClick(event);
+      }
+    };
+  }
+  return {
+    link,
+    onClick,
+  };
+};