Table: Avoid creating links in cells when data links have empty href (#103666)
* Table: Avoid creating links in cells when data links have empty href * Copy logic to TableNG * Do not add non-clickable links to context menu * Don’t pass if undefined * Add tests to cover datalink logic changes * Trigger Build --------- Co-authored-by: Kristina Durivage <kristina.durivage@grafana.com> Co-authored-by: Adela Almasan <adela.almasan@grafana.com>
This commit is contained in:
co-authored by
Kristina Durivage
Adela Almasan
parent
56d67f9ffc
commit
9d6ce37f68
@@ -485,7 +485,10 @@ export const getLinksSupplier =
|
||||
if (href) {
|
||||
href = locationUtil.assureBaseUrl(href.replace(/\n/g, ''));
|
||||
href = replaceVariables(href, dataLinkScopedVars, VariableFormatID.UriEncode);
|
||||
href = locationUtil.processUrl(href);
|
||||
|
||||
if (href?.length > 0) {
|
||||
href = locationUtil.processUrl(href);
|
||||
}
|
||||
}
|
||||
|
||||
if (link.onClick) {
|
||||
|
||||
@@ -8,17 +8,20 @@ export const DataLinksCell = (props: TableCellProps) => {
|
||||
|
||||
return (
|
||||
<div {...cellProps} className={tableStyles.cellContainerText}>
|
||||
{links &&
|
||||
links.map((link, idx) => {
|
||||
return (
|
||||
// eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
|
||||
<span key={idx} className={tableStyles.cellLink} onClick={link.onClick}>
|
||||
<a href={link.href} target={link.target}>
|
||||
{link.title}
|
||||
</a>
|
||||
</span>
|
||||
);
|
||||
})}
|
||||
{links?.map((link, idx) => {
|
||||
return !link.href && link.onClick == null ? (
|
||||
<span key={idx} className={tableStyles.cellLinkEmpty}>
|
||||
{link.title}
|
||||
</span>
|
||||
) : (
|
||||
// eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
|
||||
<span key={idx} className={tableStyles.cellLink} onClick={link.onClick}>
|
||||
<a href={link.href} target={link.target}>
|
||||
{link.title}
|
||||
</a>
|
||||
</span>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
@@ -23,7 +23,8 @@ export const DefaultCell = (props: TableCellProps) => {
|
||||
const showFilters = props.onCellFilterAdded && field.config.filterable;
|
||||
const showActions = (showFilters && cell.value !== undefined) || inspectEnabled;
|
||||
const cellOptions = getCellOptions(field);
|
||||
const hasLinks = Boolean(getCellLinks(field, row)?.length);
|
||||
const cellLinks = getCellLinks(field, row);
|
||||
const hasLinks = cellLinks?.some((link) => link.href || link.onClick != null);
|
||||
const clearButtonStyle = useStyles2(clearLinkButtonStyles);
|
||||
let value: string | ReactElement;
|
||||
|
||||
@@ -79,7 +80,9 @@ export const DefaultCell = (props: TableCellProps) => {
|
||||
return (
|
||||
<div key={key} {...rest} className={cellStyle}>
|
||||
{hasLinks ? (
|
||||
<DataLinksContextMenu links={() => getCellLinks(field, row) || []}>
|
||||
<DataLinksContextMenu
|
||||
links={() => getCellLinks(field, row)?.filter((link) => link.href || link.onClick != null) || []}
|
||||
>
|
||||
{(api) => {
|
||||
if (api.openMenu) {
|
||||
return (
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
import { render, screen } from '@testing-library/react';
|
||||
|
||||
import { Field, FieldType, LinkModel } from '@grafana/data';
|
||||
import { TableCellDisplayMode } from '@grafana/schema';
|
||||
|
||||
import AutoCell from './AutoCell';
|
||||
|
||||
describe('AutoCell', () => {
|
||||
describe('Displays data Links', () => {
|
||||
const getFieldWithLinks = (links: LinkModel[]): Field => {
|
||||
return {
|
||||
name: 'Category',
|
||||
type: FieldType.string,
|
||||
values: ['A', 'B', 'A', 'B', 'A'],
|
||||
config: {
|
||||
custom: {
|
||||
cellOptions: {
|
||||
type: TableCellDisplayMode.Auto,
|
||||
wrapText: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
display: (value: unknown) => ({
|
||||
text: String(value),
|
||||
numeric: 0,
|
||||
color: undefined,
|
||||
prefix: undefined,
|
||||
suffix: undefined,
|
||||
}),
|
||||
state: {},
|
||||
getLinks: () => links,
|
||||
};
|
||||
};
|
||||
|
||||
it('shows multiple datalinks in a context menu behind a button', () => {
|
||||
const linksForField = [
|
||||
{ href: 'http://asdasd.com', title: 'Test Title' } as LinkModel,
|
||||
{ href: 'http://asdasd2.com', title: 'Test Title2' } as LinkModel,
|
||||
];
|
||||
|
||||
jest.mock('../utils', () => ({
|
||||
getCellLinks: () => linksForField,
|
||||
}));
|
||||
|
||||
const field = getFieldWithLinks(linksForField);
|
||||
|
||||
render(
|
||||
<AutoCell
|
||||
value="test"
|
||||
field={field}
|
||||
justifyContent="normal"
|
||||
rowIdx={0}
|
||||
cellOptions={{ type: TableCellDisplayMode.Auto }}
|
||||
/>
|
||||
);
|
||||
const submitButton = screen.getByRole('button');
|
||||
expect(submitButton).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('does not show button for menu for multiple links if one is invalid', () => {
|
||||
const linksForField = [
|
||||
{ href: 'http://asdasd.com', title: 'Test Title' } as LinkModel,
|
||||
{ title: 'Test Title2' } as LinkModel,
|
||||
];
|
||||
|
||||
jest.mock('../utils', () => ({
|
||||
getCellLinks: () => linksForField,
|
||||
}));
|
||||
|
||||
const field = getFieldWithLinks(linksForField);
|
||||
|
||||
render(
|
||||
<AutoCell
|
||||
value="test"
|
||||
field={field}
|
||||
justifyContent="normal"
|
||||
rowIdx={0}
|
||||
cellOptions={{ type: TableCellDisplayMode.Auto }}
|
||||
/>
|
||||
);
|
||||
const submitButton = screen.queryByRole('button');
|
||||
expect(submitButton).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -15,13 +15,16 @@ export default function AutoCell({ value, field, justifyContent, rowIdx, cellOpt
|
||||
|
||||
const displayValue = field.display!(value);
|
||||
const formattedValue = formattedValueToString(displayValue);
|
||||
const hasLinks = Boolean(getCellLinks(field, rowIdx)?.length);
|
||||
const cellLinks = getCellLinks(field, rowIdx);
|
||||
const hasLinks = cellLinks?.some((link) => link.href || link.onClick != null);
|
||||
const clearButtonStyle = useStyles2(clearLinkButtonStyles);
|
||||
|
||||
return (
|
||||
<div className={styles.cell}>
|
||||
{hasLinks ? (
|
||||
<DataLinksContextMenu links={() => getCellLinks(field, rowIdx) || []}>
|
||||
<DataLinksContextMenu
|
||||
links={() => getCellLinks(field, rowIdx)?.filter((link) => link.href || link.onClick != null) || []}
|
||||
>
|
||||
{(api) => {
|
||||
if (api.openMenu) {
|
||||
return (
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
import { render, screen } from '@testing-library/react';
|
||||
|
||||
import { Field, FieldType, LinkModel } from '@grafana/data';
|
||||
import { TableCellDisplayMode } from '@grafana/schema';
|
||||
|
||||
import { DataLinksCell } from './DataLinksCell';
|
||||
|
||||
describe('DataLinksCell', () => {
|
||||
describe('Displays data Links', () => {
|
||||
const getFieldWithLinks = (links: LinkModel[]): Field => {
|
||||
return {
|
||||
name: 'Category',
|
||||
type: FieldType.string,
|
||||
values: ['A', 'B', 'A', 'B', 'A'],
|
||||
config: {
|
||||
custom: {
|
||||
cellOptions: {
|
||||
type: TableCellDisplayMode.Auto,
|
||||
wrapText: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
display: (value: unknown) => ({
|
||||
text: String(value),
|
||||
numeric: 0,
|
||||
color: undefined,
|
||||
prefix: undefined,
|
||||
suffix: undefined,
|
||||
}),
|
||||
state: {},
|
||||
getLinks: () => links,
|
||||
};
|
||||
};
|
||||
|
||||
it('shows multiple datalinks in separate spans', () => {
|
||||
const linksForField = [
|
||||
{ href: 'http://asdasd.com', title: 'Test Title' } as LinkModel,
|
||||
{ href: 'http://asdasd2.com', title: 'Test Title2' } as LinkModel,
|
||||
];
|
||||
|
||||
jest.mock('../utils', () => ({
|
||||
getCellLinks: () => linksForField,
|
||||
}));
|
||||
|
||||
const field = getFieldWithLinks(linksForField);
|
||||
|
||||
render(<DataLinksCell field={field} rowIdx={0} />);
|
||||
|
||||
linksForField.forEach((link) => {
|
||||
expect(screen.getByRole('link', { name: link.title })).toHaveAttribute('href', link.href);
|
||||
});
|
||||
});
|
||||
|
||||
it('Does not create a link if href is missing from link', () => {
|
||||
const linksForField = [
|
||||
{ href: 'http://asdasd.com', title: 'Test Title' } as LinkModel,
|
||||
{ title: 'Test Title2' } as LinkModel,
|
||||
];
|
||||
|
||||
jest.mock('../utils', () => ({
|
||||
getCellLinks: () => linksForField,
|
||||
}));
|
||||
|
||||
const field = getFieldWithLinks(linksForField);
|
||||
|
||||
render(<DataLinksCell field={field} rowIdx={0} />);
|
||||
|
||||
linksForField.forEach((link) => {
|
||||
if (link.href !== undefined) {
|
||||
expect(screen.getByRole('link', { name: link.title })).toHaveAttribute('href', link.href);
|
||||
} else {
|
||||
expect(screen.queryByRole('link', { name: link.title })).not.toBeInTheDocument();
|
||||
expect(screen.getByText(link.title)).toBeInTheDocument();
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -15,7 +15,11 @@ export const DataLinksCell = ({ field, rowIdx }: DataLinksCellProps) => {
|
||||
<div>
|
||||
{links &&
|
||||
links.map((link, idx) => {
|
||||
return (
|
||||
return !link.href && link.onClick == null ? (
|
||||
<span key={idx} className={styles.cellLinkEmpty}>
|
||||
{link.title}
|
||||
</span>
|
||||
) : (
|
||||
// eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
|
||||
<span key={idx} className={styles.linkCell} onClick={link.onClick}>
|
||||
<a href={link.href} target={link.target}>
|
||||
@@ -46,4 +50,12 @@ const getStyles = (theme: GrafanaTheme2) => ({
|
||||
color: theme.colors.text.link,
|
||||
},
|
||||
}),
|
||||
cellLinkEmpty: css({
|
||||
overflow: 'hidden',
|
||||
textOverflow: 'ellipsis',
|
||||
userSelect: 'text',
|
||||
whiteSpace: 'nowrap',
|
||||
fontWeight: theme.typography.fontWeightMedium,
|
||||
paddingRight: theme.spacing(1.5),
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -192,6 +192,14 @@ export function useTableStyles(theme: GrafanaTheme2, cellHeightOption: TableCell
|
||||
color: theme.colors.text.link,
|
||||
},
|
||||
}),
|
||||
cellLinkEmpty: css({
|
||||
overflow: 'hidden',
|
||||
textOverflow: 'ellipsis',
|
||||
userSelect: 'text',
|
||||
whiteSpace: 'nowrap',
|
||||
fontWeight: theme.typography.fontWeightMedium,
|
||||
paddingRight: theme.spacing(1.5),
|
||||
}),
|
||||
cellLinkForColoredCell: css({
|
||||
cursor: 'pointer',
|
||||
overflow: 'hidden',
|
||||
|
||||
Reference in New Issue
Block a user