Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions components/src/Table/TableCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,16 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import { TableCell as MuiTableCell, styled, TableCellProps as MuiTableCellProps, Box, useTheme } from '@mui/material';
import {
TableCell as MuiTableCell,
styled,
TableCellProps as MuiTableCellProps,
Box,
useTheme,
Link,
} from '@mui/material';
import { ReactElement, useEffect, useRef } from 'react';
import { TableCellAlignment, TableDensity, getTableCellLayout } from './model/table-model';
import { DataLink, TableCellAlignment, TableDensity, getTableCellLayout } from './model/table-model';

const StyledMuiTableCell = styled(MuiTableCell)(({ theme }) => ({
padding: 0,
Expand Down Expand Up @@ -65,6 +72,7 @@ export interface TableCellProps extends Omit<MuiTableCellProps, 'tabIndex' | 'al
onFocusTrigger?: (e: React.MouseEvent<HTMLTableCellElement> | React.KeyboardEvent<HTMLTableCellElement>) => void;
color?: string;
backgroundColor?: string;
dataLink?: DataLink;
}

export function TableCell({
Expand All @@ -81,10 +89,10 @@ export function TableCell({
align,
color,
backgroundColor,
dataLink,
...otherProps
}: TableCellProps): ReactElement {
const theme = useTheme();

const elRef = useRef<HTMLTableCellElement>();

const isHeader = variant === 'head';
Expand Down Expand Up @@ -172,7 +180,17 @@ export function TableCell({
aria-label={description}
textAlign={align}
>
{children}
{dataLink ? (
<Link
href={dataLink.url}
target={dataLink.openNewTab ? '_blank' : '_self'}
rel={dataLink.openNewTab ? 'noopener noreferrer' : undefined}
>
{children}
</Link>
) : (
children
)}
</Box>
</StyledMuiTableCell>
);
Expand Down
17 changes: 14 additions & 3 deletions components/src/Table/VirtualizedTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import { Column, HeaderGroup, Row, flexRender } from '@tanstack/react-table';
import { Column, ColumnMeta, HeaderGroup, Row, flexRender } from '@tanstack/react-table';
import { Box, TablePagination, TableRow as MuiTableRow } from '@mui/material';
import { TableVirtuoso, TableComponents, TableVirtuosoHandle, TableVirtuosoProps } from 'react-virtuoso';
import { useRef, useMemo, ReactElement } from 'react';
Expand All @@ -22,7 +22,7 @@ import { TableHead } from './TableHead';
import { TableHeaderCell } from './TableHeaderCell';
import { TableCell, TableCellProps } from './TableCell';
import { VirtualizedTableContainer } from './VirtualizedTableContainer';
import { TableCellConfigs, TableProps, TableRowEventOpts } from './model/table-model';
import { TableCellConfigs, TableColumnConfig, TableProps, TableRowEventOpts } from './model/table-model';
import { useVirtualizedTableKeyboardNav } from './hooks/useVirtualizedTableKeyboardNav';
import { TableFoot } from './TableFoot';

Expand Down Expand Up @@ -72,6 +72,7 @@ export function VirtualizedTable<TableData>({
startIndex: 0,
endIndex: 0,
});

const setVisibleRange: TableVirtuosoProps<TableData, unknown>['rangeChanged'] = (newVisibleRange) => {
visibleRange.current = newVisibleRange;
};
Expand Down Expand Up @@ -218,7 +219,6 @@ export function VirtualizedTable<TableData>({
<>
{row.getVisibleCells().map((cell, i, cells) => {
const position: TableCellPosition = {
// Add 1 to the row index because the header is row 0
row: index + 1,
column: i,
};
Expand All @@ -229,6 +229,16 @@ export function VirtualizedTable<TableData>({
const cellRenderFn = cell.column.columnDef.cell;
const cellContent = typeof cellRenderFn === 'function' ? cellRenderFn(cellContext) : null;

/*
IMPORTANT:
If Variables exist in the link, they should have been translated by the plugin already. (Being developed at the moment)
Components have no access to any context (Which is intentional and correct)
We may want to add parameters to a link from neighboring cells in the future as well.
If this is the case, the value of the neighboring cells should be read from here and be replaced. (Bing discussed at the moment, not decided yet)
*/
const columnDefMeta = cell.column.columnDef.meta as ColumnMeta<TableData, unknown> &
Pick<TableColumnConfig<TableData>, 'dataLink'>;

const cellDescriptionDef = cell.column.columnDef.meta?.cellDescription;
let description: string | undefined = undefined;
if (typeof cellDescriptionDef === 'function') {
Expand Down Expand Up @@ -258,6 +268,7 @@ export function VirtualizedTable<TableData>({
description={description}
color={cellConfig?.textColor ?? undefined}
backgroundColor={cellConfig?.backgroundColor ?? undefined}
dataLink={columnDefMeta?.dataLink}
>
{cellConfig?.text || cellContent}
</TableCell>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ exports[`persesColumnToTanstackColumn transforms perses columns to tanstack colu
"meta": {
"align": "right",
"cellDescription": undefined,
"dataLink": undefined,
"headerDescription": undefined,
},
"minSize": 0,
Expand All @@ -77,6 +78,7 @@ exports[`persesColumnToTanstackColumn transforms perses columns to tanstack colu
"meta": {
"align": undefined,
"cellDescription": [Function],
"dataLink": undefined,
"headerDescription": "The total number of values.",
},
"size": 120,
Expand Down
2 changes: 2 additions & 0 deletions components/src/Table/model/table-model.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,14 @@ describe('persesColumnToTanstackColumn', () => {
header: 'Name',
width: 'auto',
align: 'right',
dataLink: undefined,
},
{
accessorKey: 'value',
header: 'Count',
headerDescription: 'The total number of values.',
width: 120,
dataLink: undefined,
cell: (data) => <strong>{data.getValue()}</strong>,
cellDescription: (data) => `Desc for ${data.getValue()}`,
},
Expand Down
18 changes: 12 additions & 6 deletions components/src/Table/model/table-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,15 @@ declare module '@tanstack/table-core' {
}
}

// Column link settings
// The URL could be set to a static link or could be constructed dynamically
// The URL may include reference to the variables or neighboring cells in the row
export interface DataLink {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, but we should adjust this interface with the comments from perses/plugins#488 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I answered the comment.
I understand the concern there. I am just wondering, if we should mix this kind of refactor with this feature.
Could you check and see if my answer makes sense? (I know that at some points we need to refactor the interface)

If a refactor is a must, should we move the new suggested DataLink interface under perses/ui/core?
DataLink is not bound to any component and is something general. I see no directory which could fit in Shared repo.

Copy link
Contributor

@AntoineThebaud AntoineThebaud Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FE counterpart of the BE interface shared in above comment is already in the core https://github.com/perses/perses/blob/main/ui/core/src/model/panels.ts#L17-L23

For now located in "panels" because added with the Panel links feature but we should soon have the Dashboard links that will also need the very same interface.

Still, e.g in the case of the Table panel links we shouldn't have the name attribute - as there's a single link to configure and not an array of them - so we should still have sth like this on Table plugin or shared side I guess:

type DataLink = Omit<Link, 'name'>;

url: string;
title?: string;
openNewTab: boolean;
}

// Only exposing a very simplified version of the very extensive column definitions
// possible with tanstack table to make it easier for us to control rendering
// and functionality.
Expand Down Expand Up @@ -324,11 +333,7 @@ export interface TableColumnConfig<TableData>
* Dynamic link setting. If available the the cell content should turn into
* a link with the value of the cell as the dynamic section
*/
dataLink?: {
url: string;
title?: string;
openNewTab: boolean;
};
dataLink?: DataLink;
}

/**
Expand All @@ -338,7 +343,7 @@ export function persesColumnsToTanstackColumns<TableData>(
columns: Array<TableColumnConfig<TableData>>
): Array<ColumnDef<TableData>> {
const tableCols: Array<ColumnDef<TableData>> = columns.map(
({ width, align, headerDescription, cellDescription, enableSorting, ...otherProps }) => {
({ width, align, headerDescription, cellDescription, enableSorting, dataLink, ...otherProps }) => {
// Tanstack Table does not support an "auto" value to naturally size to fit
// the space in a table. We translate our custom "auto" setting to 0 size
// for these columns, so it is easy to fall back to auto when rendering.
Expand Down Expand Up @@ -370,6 +375,7 @@ export function persesColumnsToTanstackColumns<TableData>(
align,
headerDescription,
cellDescription,
dataLink,
},
};

Expand Down