From 26d26f67d49842c5337e7848613e079a1a6a9c64 Mon Sep 17 00:00:00 2001 From: Paul Marbach Date: Wed, 23 Jul 2025 15:39:25 -0400 Subject: [PATCH] TableNG: Integrate hover and selection, DataLinkCell cleanup (#108353) * Table: Style cleanups (minus DataLinkCell word wrap) * kill JSONCell in favor of a custom display method and style overrides at TableNG * remove unused type for JSONCellProps * add increased specificity to CSS selector * remove inherit and rely on undefined * fix tests * shrink and optimize DataLinkCell * maybe * format files * better * classname * add Pills and DataLink cells to kitchen sink * add comment about align + justify, simplify datalinks targeting * simplify? * poke * tweak * revert * fix one more z-index conflict * clean up alignment tests * a couple more tests * make TableNG e2e tests more resilient to changes to the gdev dashboard --------- Co-authored-by: Leon Sorokin --- .../panel-table/table_kitchen_sink.json | 83 +++++++- .../panels-suite/table-kitchenSink.spec.ts | 92 +++++++-- .../Table/TableNG/Cells/DataLinksCell.tsx | 44 +--- .../Table/TableNG/Cells/JSONCell.tsx | 49 ----- .../Table/TableNG/Cells/SparklineCell.tsx | 9 +- .../Table/TableNG/Cells/renderers.test.tsx | 9 + .../Table/TableNG/Cells/renderers.tsx | 12 +- .../src/components/Table/TableNG/TableNG.tsx | 81 ++++++-- .../src/components/Table/TableNG/hooks.ts | 2 - .../src/components/Table/TableNG/types.ts | 7 - .../components/Table/TableNG/utils.test.ts | 191 ++++++++---------- .../src/components/Table/TableNG/utils.ts | 103 ++++++---- public/locales/en-US/grafana.json | 5 +- 13 files changed, 389 insertions(+), 298 deletions(-) delete mode 100644 packages/grafana-ui/src/components/Table/TableNG/Cells/JSONCell.tsx diff --git a/devenv/dev-dashboards/panel-table/table_kitchen_sink.json b/devenv/dev-dashboards/panel-table/table_kitchen_sink.json index 6123deb638d..c92924966ce 100644 --- a/devenv/dev-dashboards/panel-table/table_kitchen_sink.json +++ b/devenv/dev-dashboards/panel-table/table_kitchen_sink.json @@ -79,10 +79,6 @@ { "id": "max", "value": 100 - }, - { - "id": "custom.width", - "value": 300 } ] }, @@ -280,13 +276,75 @@ "type": "auto", "wrapText": true } + }, + { + "id": "custom.width", + "value": 300 } ] + }, + { + "matcher": { + "id": "byName", + "options": "Pills" + }, + "properties": [ + { + "id": "custom.cellOptions", + "value": { + "type": "pill" + } + }, + { + "id": "custom.width", + "value": 120 + } + ] + }, + { + "matcher": { + "id": "byName", + "options": "Data Link" + }, + "properties": [ + { + "id": "custom.cellOptions", + "value": { + "type": "data-links" + } + }, + { + "id": "links", + "value": [ + { + "targetBlank": true, + "title": "Product", + "url": "${__value.text}" + }, + { + "targetBlank": true, + "title": "Grafana", + "url": "https://grafana.com" + } + ] + }, + { + "id": "custom.width", + "value": 180 + } + ] + }, + { + "matcher": { + "id": "byName", + "options": "Gauge" + }, + "properties": [] } ] }, "gridPos": { - "h": 18, + "h": 15, "w": 24, "x": 0, "y": 0 @@ -297,7 +355,10 @@ "footer": { "countRows": false, "enablePagination": false, - "fields": "", + "fields": [ + "Min", + "Max" + ], "reducer": [ "max" ], @@ -318,7 +379,7 @@ "scenarioId": "random_walk_table" }, { - "csvContent": "Info,Image,Long Text\ndown,https://placecats.com/millie/300/300,\"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse tempus et augue et lacinia. Interdum et malesuada fames ac ante ipsum primis in faucibus. Donec eu pretium tortor. Cras venenatis sapien sed mauris gravida, ut scelerisque est fringilla. Cras lorem diam, facilisis nec malesuada in, vulputate vel enim. Etiam fringilla nisi quis felis blandit tincidunt. Cras id lacus ornare, ullamcorper nisl eget, bibendum odio. Pellentesque imperdiet, leo a imperdiet venenatis, ligula risus venenatis quam, vel euismod magna nisi sit amet leo.\"\nup,https://placecats.com/neo/300/300,\"Sed imperdiet eget diam sit amet fringilla. Curabitur quis lacus blandit, mollis diam non, accumsan tortor. Aliquam ac tellus eget dui facilisis tempor eu id nulla. Maecenas ultrices turpis eu elementum imperdiet. Fusce eget rhoncus mi, et egestas lectus. Mauris facilisis auctor enim sed malesuada. Maecenas placerat ultricies metus vitae viverra. In hac habitasse platea dictumst. Mauris ipsum nisl, dictum eu aliquam eleifend, rutrum id orci. Nullam eget dui et odio eleifend porttitor.\"\nup fast,https://placecats.com/bella/300/300,\"Proin ac libero vulputate ex vulputate pharetra ut vel lacus. Phasellus quis dolor sed leo finibus scelerisque. Ut vel finibus leo, sed viverra ipsum. Suspendisse vitae rutrum arcu. Donec sed tellus vel lectus bibendum vestibulum. Sed eu felis non velit dictum pulvinar eu et leo. Aenean et dignissim arcu. Nam luctus at neque quis efficitur. Fusce tempus at nibh a imperdiet. Nullam malesuada ac magna at facilisis. Duis pretium aliquam eros. Donec pharetra dignissim dolor non bibendum. Ut gravida mi id urna tempus, at ullamcorper felis vulputate. Duis congue augue ex, sed finibus leo ornare ut. Mauris non quam sodales, dignissim lorem eget, tincidunt mauris. Aliquam ut velit auctor, vestibulum metus sed, mollis massa.\"\ndown fast,https://placecats.com/neo_2/300/300,\"Nullam in pulvinar justo. Nunc dictum arcu ac pellentesque bibendum. Sed in erat turpis. Vestibulum eu orci ac ligula lobortis tempus. Fusce consectetur feugiat magna, eu tempor nibh vestibulum ac. Aliquam erat volutpat. Vivamus sit amet viverra enim. Quisque mollis odio nulla, nec vulputate sem placerat in. Etiam dolor sapien, pulvinar in accumsan at, consequat eget nisi. Nunc condimentum neque magna, congue consectetur dui efficitur interdum. Nam lobortis fringilla maximus. Vestibulum eu dui a velit condimentum eleifend consequat nec lectus.\"", + "csvContent": "Info,Image,Pills,Data Link,Long Text\ndown,https://placecats.com/millie/200/400,hello,https://grafana.com,\"Lorem ipsum dolor sit amet, consectetur adipiscing elit. Suspendisse tempus et augue et lacinia. Interdum et malesuada fames ac ante ipsum primis in faucibus. Donec eu pretium tortor. Cras venenatis sapien sed mauris gravida, ut scelerisque est fringilla. Cras lorem diam, facilisis nec malesuada in, vulputate vel enim. Etiam fringilla nisi quis felis blandit tincidunt. Cras id lacus ornare, ullamcorper nisl eget, bibendum odio. Pellentesque imperdiet, leo a imperdiet venenatis, ligula risus venenatis quam, vel euismod magna nisi sit amet leo.\"\nup,https://placecats.com/neo/200/400,\"[1,2,3,\"\"foo\"\",\"\"bar\"\"]\",https://grafana.com/solutions/kubernetes/,\"Sed imperdiet eget diam sit amet fringilla. Curabitur quis lacus blandit, mollis diam non, accumsan tortor. Aliquam ac tellus eget dui facilisis tempor eu id nulla. Maecenas ultrices turpis eu elementum imperdiet. Fusce eget rhoncus mi, et egestas lectus. Mauris facilisis auctor enim sed malesuada. Maecenas placerat ultricies metus vitae viverra. In hac habitasse platea dictumst. Mauris ipsum nisl, dictum eu aliquam eleifend, rutrum id orci. Nullam eget dui et odio eleifend porttitor.\"\nup fast,https://placecats.com/bella/200/400,\"foo,1,4,beep\",https://k6.io/,\"Proin ac libero vulputate ex vulputate pharetra ut vel lacus. Phasellus quis dolor sed leo finibus scelerisque. Ut vel finibus leo, sed viverra ipsum. Suspendisse vitae rutrum arcu. Donec sed tellus vel lectus bibendum vestibulum. Sed eu felis non velit dictum pulvinar eu et leo. Aenean et dignissim arcu. Nam luctus at neque quis efficitur. Fusce tempus at nibh a imperdiet. Nullam malesuada ac magna at facilisis. Duis pretium aliquam eros. Donec pharetra dignissim dolor non bibendum. Ut gravida mi id urna tempus, at ullamcorper felis vulputate. Duis congue augue ex, sed finibus leo ornare ut. Mauris non quam sodales, dignissim lorem eget, tincidunt mauris. Aliquam ut velit auctor, vestibulum metus sed, mollis massa.\"\ndown fast,https://placecats.com/neo_2/200/400,\"foo,bar,baz,a longer one,bim\",https://grafana.com/products/cloud/,\"Nullam in pulvinar justo. Nunc dictum arcu ac pellentesque bibendum. Sed in erat turpis. Vestibulum eu orci ac ligula lobortis tempus. Fusce consectetur feugiat magna, eu tempor nibh vestibulum ac. Aliquam erat volutpat. Vivamus sit amet viverra enim. Quisque mollis odio nulla, nec vulputate sem placerat in. Etiam dolor sapien, pulvinar in accumsan at, consequat eget nisi. Nunc condimentum neque magna, congue consectetur dui efficitur interdum. Nam lobortis fringilla maximus. Vestibulum eu dui a velit condimentum eleifend consequat nec lectus.\"", "datasource": { "type": "grafana-testdata-datasource", "uid": "PD8C576611E62080A" @@ -344,12 +405,14 @@ }, "includeByName": {}, "indexByName": { - "A": 7, + "A": 9, + "Data Link": 8, "Image": 5, "Info": 1, - "Long Text": 6, + "Long Text": 7, "Max A": 3, "Min A": 2, + "Pills": 6, "State A": 4, "Time A": 0 }, @@ -461,5 +524,5 @@ "timezone": "", "title": "Panel Tests - Table - Kitchen Sink", "uid": "dcb9f5e9-8066-4397-889e-864b99555dbb", - "version": 1 + "version": 2 } diff --git a/e2e-playwright/panels-suite/table-kitchenSink.spec.ts b/e2e-playwright/panels-suite/table-kitchenSink.spec.ts index 29fea517fae..7052ccfc510 100644 --- a/e2e-playwright/panels-suite/table-kitchenSink.spec.ts +++ b/e2e-playwright/panels-suite/table-kitchenSink.spec.ts @@ -2,14 +2,20 @@ import { Page, Locator } from '@playwright/test'; import { test, expect } from '@grafana/plugin-e2e'; +const DASHBOARD_UID = 'dcb9f5e9-8066-4397-889e-864b99555dbb'; + test.use({ - viewport: { width: 1600, height: 1080 }, + viewport: { width: 2000, height: 1080 }, featureToggles: { tableNextGen: true, }, }); // helper utils +const waitForTableLoad = async (loc: Page | Locator) => { + await expect(loc.locator('.rdg')).toBeVisible(); +}; + const getCell = async (loc: Page | Locator, rowIdx: number, colIdx: number) => loc .getByRole('row') @@ -22,6 +28,24 @@ const getCellHeight = async (loc: Page | Locator, rowIdx: number, colIdx: number return (await cell.boundingBox())?.height ?? 0; }; +const getColumnIdx = async (loc: Page | Locator, columnName: string) => { + // find the index of the column "Long text." The kitchen sink table will change over time, but + // we can just find the column programatically and use it throughout the test. + let result = -1; + const colCount = await loc.getByRole('columnheader').count(); + for (let colIdx = 0; colIdx < colCount; colIdx++) { + const cell = await getCell(loc, 0, colIdx); + if ((await cell.textContent()) === columnName) { + result = colIdx; + break; + } + } + if (result === -1) { + throw new Error(`Could not find the "${columnName}" column in the table`); + } + return result; +}; + test.describe( 'Panels test: Table - Kitchen Sink', { @@ -30,7 +54,7 @@ test.describe( () => { test('Tests word wrap, hover overflow, and cell inspect', async ({ gotoDashboardPage, selectors, page }) => { const dashboardPage = await gotoDashboardPage({ - uid: 'dcb9f5e9-8066-4397-889e-864b99555dbb', + uid: DASHBOARD_UID, queryParams: new URLSearchParams({ editPanel: '1' }), }); @@ -38,22 +62,28 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) ).toBeVisible(); + // to avoid a race condition when counting up , wait for react-data-grid to finish rendering. + await waitForTableLoad(page); + + const longTextColIdx = await getColumnIdx(page, 'Long Text'); + // text wrapping is enabled by default on this panel. - await expect(getCellHeight(page, 1, 5)).resolves.toBeGreaterThan(100); + await expect(getCellHeight(page, 1, longTextColIdx)).resolves.toBeGreaterThan(100); // toggle the lorem ipsum column's wrap text toggle and confirm that the height shrinks. await dashboardPage .getByGrafanaSelector(selectors.components.PanelEditor.OptionsPane.fieldLabel('Wrap text')) .last() .click(); - await expect(getCellHeight(page, 1, 5)).resolves.toBeLessThan(100); + await expect(getCellHeight(page, 1, longTextColIdx)).resolves.toBeLessThan(100); // test that hover overflow works. - const loremIpsumCell = await getCell(page, 1, 5); + const loremIpsumCell = await getCell(page, 1, longTextColIdx); + await loremIpsumCell.scrollIntoViewIfNeeded(); await loremIpsumCell.hover(); - await expect(getCellHeight(page, 1, 5)).resolves.toBeGreaterThan(100); - await (await getCell(page, 1, 6)).hover(); - await expect(getCellHeight(page, 1, 5)).resolves.toBeLessThan(100); + await expect(getCellHeight(page, 1, longTextColIdx)).resolves.toBeGreaterThan(100); + await (await getCell(page, 1, longTextColIdx + 1)).hover(); + await expect(getCellHeight(page, 1, longTextColIdx)).resolves.toBeLessThan(100); // enable cell inspect, confirm that hover no longer triggers. await dashboardPage @@ -64,7 +94,7 @@ test.describe( .locator('label[for="custom.inspect"]') .click(); await loremIpsumCell.hover(); - await expect(getCellHeight(page, 1, 5)).resolves.toBeLessThan(100); + await expect(getCellHeight(page, 1, longTextColIdx)).resolves.toBeLessThan(100); // click cell inspect, check that cell inspection pops open in the side as we'd expect. await loremIpsumCell.getByLabel('Inspect value').click(); @@ -75,7 +105,7 @@ test.describe( test('Tests visibility and display name via overrides', async ({ gotoDashboardPage, selectors, page }) => { const dashboardPage = await gotoDashboardPage({ - uid: 'dcb9f5e9-8066-4397-889e-864b99555dbb', + uid: DASHBOARD_UID, queryParams: new URLSearchParams({ editPanel: '1' }), }); @@ -107,7 +137,7 @@ test.describe( // hashtag testing pyramid. test('Tests sorting by column', async ({ gotoDashboardPage, selectors, page }) => { const dashboardPage = await gotoDashboardPage({ - uid: 'dcb9f5e9-8066-4397-889e-864b99555dbb', + uid: DASHBOARD_UID, queryParams: new URLSearchParams({ editPanel: '1' }), }); @@ -132,7 +162,7 @@ test.describe( test('Tests filtering within a column', async ({ gotoDashboardPage, selectors, page }) => { const dashboardPage = await gotoDashboardPage({ - uid: 'dcb9f5e9-8066-4397-889e-864b99555dbb', + uid: DASHBOARD_UID, queryParams: new URLSearchParams({ editPanel: '1' }), }); @@ -140,10 +170,14 @@ test.describe( dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) ).toBeVisible(); - const stateColumnHeader = page.getByRole('columnheader').filter({ hasText: 'Info' }); + await waitForTableLoad(page); + + const infoColumnIdx = await getColumnIdx(page, 'Info'); + + const stateColumnHeader = page.getByRole('columnheader').nth(infoColumnIdx); // get the first value in the "State" column, filter it out, then check that it went away. - const firstStateValue = (await (await getCell(page, 1, 1)).textContent())!; + const firstStateValue = (await (await getCell(page, 1, infoColumnIdx)).textContent())!; await stateColumnHeader .getByTestId(selectors.components.Panels.Visualization.TableNG.Filters.HeaderButton) .click(); @@ -162,7 +196,7 @@ test.describe( await expect(filterContainer).not.toBeVisible(); // did it actually filter out our value? - await expect(getCell(page, 1, 1)).resolves.not.toHaveText(firstStateValue); + await expect(getCell(page, 1, infoColumnIdx)).resolves.not.toHaveText(firstStateValue); }); test('Tests pagination, row height adjustment', async ({ gotoDashboardPage, selectors, page }) => { @@ -178,7 +212,7 @@ test.describe( }; const dashboardPage = await gotoDashboardPage({ - uid: 'dcb9f5e9-8066-4397-889e-864b99555dbb', + uid: DASHBOARD_UID, queryParams: new URLSearchParams({ editPanel: '1' }), }); @@ -251,12 +285,12 @@ test.describe( await page.getByRole('dialog').locator('#data-link-input [contenteditable="true"]').focus(); await page.getByRole('dialog').locator('#data-link-input [contenteditable="true"]').fill(url); await page.getByRole('dialog').locator('#data-link-input [contenteditable="true"]').blur(); - await page.getByRole('dialog').getByRole('button', { name: 'Save' }).click(); + await page.getByRole('dialog').locator('button[aria-disabled="false"]').filter({ hasText: 'Save' }).click(); await expect(page.getByRole('dialog')).not.toBeVisible(); }; const dashboardPage = await gotoDashboardPage({ - uid: 'dcb9f5e9-8066-4397-889e-864b99555dbb', + uid: DASHBOARD_UID, queryParams: new URLSearchParams({ editPanel: '1' }), }); @@ -271,8 +305,12 @@ test.describe( .last() .click(); + const infoColumnIdx = await getColumnIdx(page, 'Info'); + const pillColIdx = await getColumnIdx(page, 'Pills'); + const dataLinkColIdx = await getColumnIdx(page, 'Data Link'); + // Info column has a single DataLink by default. - const infoCell = await getCell(page, 1, 1); + const infoCell = await getCell(page, 1, infoColumnIdx); await expect(infoCell.locator('a')).toBeVisible(); expect(infoCell.locator('a')).toHaveAttribute('href'); expect(infoCell.locator('a')).not.toHaveAttribute('aria-haspopup'); @@ -283,6 +321,12 @@ test.describe( // add a DataLink to the whole table, all cells will now have a single link. const colCount = await page.getByRole('row').nth(1).getByRole('gridcell').count(); for (let colIdx = 0; colIdx < colCount; colIdx++) { + // - pills column currently does not support DataLinks. + // - we don't apply DataLinks to the DataLinks column itself, since they're rendered inside. + if (colIdx === pillColIdx || colIdx === dataLinkColIdx) { + continue; + } + const cell = await getCell(page, 1, colIdx); await expect(cell.locator('a')).toBeVisible(); expect(cell.locator('a')).toHaveAttribute('href'); @@ -297,12 +341,18 @@ test.describe( // loop thru the columns, click the links, observe that the tooltip appears, and close the tooltip. for (let colIdx = 0; colIdx < colCount; colIdx++) { const cell = await getCell(page, 1, colIdx); - if (colIdx === 1) { + if (colIdx === infoColumnIdx) { // the Info column should still have its single link. expect(cell.locator('a')).not.toHaveAttribute('aria-haspopup', 'menu'); continue; } + // - pills column currently does not support DataLinks. + // - we don't apply DataLinks to the DataLinks column itself, since they're rendered inside. + if (colIdx === pillColIdx || colIdx === dataLinkColIdx) { + continue; + } + await cell.locator('a').click({ force: true }); await expect(page.getByTestId(selectors.components.DataLinksActionsTooltip.tooltipWrapper)).toBeVisible(); @@ -316,7 +366,7 @@ test.describe( test('Empty Table panel', async ({ gotoDashboardPage, selectors }) => { const dashboardPage = await gotoDashboardPage({ - uid: 'dcb9f5e9-8066-4397-889e-864b99555dbb', + uid: DASHBOARD_UID, queryParams: new URLSearchParams({ editPanel: '2' }), }); diff --git a/packages/grafana-ui/src/components/Table/TableNG/Cells/DataLinksCell.tsx b/packages/grafana-ui/src/components/Table/TableNG/Cells/DataLinksCell.tsx index a83a623ee88..5fafb04765a 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/Cells/DataLinksCell.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/Cells/DataLinksCell.tsx @@ -1,42 +1,16 @@ -import { css } from '@emotion/css'; - -import { GrafanaTheme2 } from '@grafana/data'; - -import { useStyles2 } from '../../../../themes/ThemeContext'; import { DataLinksCellProps } from '../types'; import { getCellLinks } from '../utils'; export const DataLinksCell = ({ field, rowIdx }: DataLinksCellProps) => { - const styles = useStyles2(getStyles); + const links = getCellLinks(field, rowIdx); - const links = getCellLinks(field, rowIdx!); + if (!links?.length) { + return null; + } - return ( -
- {links && - links.map((link, idx) => { - return !link.href && link.onClick == null ? ( - - {link.title} - - ) : ( - // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions - - - {link.title} - - - ); - })} -
- ); + return links.map((link, idx) => ( + + {link.title} + + )); }; - -const getStyles = (theme: GrafanaTheme2) => ({ - linkCell: css({ - userSelect: 'text', - whiteSpace: 'nowrap', - fontWeight: theme.typography.fontWeightMedium, - paddingRight: theme.spacing(1.5), - }), -}); diff --git a/packages/grafana-ui/src/components/Table/TableNG/Cells/JSONCell.tsx b/packages/grafana-ui/src/components/Table/TableNG/Cells/JSONCell.tsx deleted file mode 100644 index ed66bdef90c..00000000000 --- a/packages/grafana-ui/src/components/Table/TableNG/Cells/JSONCell.tsx +++ /dev/null @@ -1,49 +0,0 @@ -import { css } from '@emotion/css'; -import { Property } from 'csstype'; - -import { GrafanaTheme2 } from '@grafana/data'; - -import { useStyles2 } from '../../../../themes/ThemeContext'; -import { MaybeWrapWithLink } from '../MaybeWrapWithLink'; -import { JSONCellProps } from '../types'; - -export const JSONCell = ({ value, justifyContent, field, rowIdx }: JSONCellProps) => { - const styles = useStyles2(getStyles, justifyContent); - - let displayValue = value; - - // Handle string values that might be JSON - if (typeof value === 'string') { - try { - const parsed = JSON.parse(value); - displayValue = JSON.stringify(parsed, null, ' '); - } catch { - displayValue = value; // Keep original if not valid JSON - } - } else { - // For non-string values, stringify them - try { - displayValue = JSON.stringify(value, null, ' '); - } catch (error) { - // Handle circular references or other stringify errors - displayValue = String(value); - } - } - - return ( -
- - {displayValue} - -
- ); -}; - -const getStyles = (theme: GrafanaTheme2, justifyContent: Property.JustifyContent) => ({ - jsonText: css({ - display: 'flex', - cursor: 'pointer', - fontFamily: 'monospace', - justifyContent: justifyContent, - }), -}); diff --git a/packages/grafana-ui/src/components/Table/TableNG/Cells/SparklineCell.tsx b/packages/grafana-ui/src/components/Table/TableNG/Cells/SparklineCell.tsx index 225790cb0bb..4979981995b 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/Cells/SparklineCell.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/Cells/SparklineCell.tsx @@ -12,6 +12,7 @@ import { isDataFrameWithValue, GrafanaTheme2, } from '@grafana/data'; +import { t } from '@grafana/i18n'; import { BarAlignment, GraphDrawStyle, @@ -46,10 +47,10 @@ export const defaultSparklineCellConfig: TableSparklineCellOptions = { export const SparklineCell = (props: SparklineCellProps) => { const { field, value, theme, timeRange, rowIdx, justifyContent, width } = props; const styles = useStyles2(getStyles, justifyContent); - const sparkline = getSparkline(value); + const sparkline = getSparkline(value, field); if (!sparkline) { - return <>{field.config.noValue || 'no data'}; + return <>{field.config.noValue || t('grafana-ui.table.sparkline.no-data', 'no data')}; } // Get the step from the first two values to null-fill the x-axis based on timerange @@ -117,11 +118,11 @@ export const SparklineCell = (props: SparklineCellProps) => { ); }; -function getSparkline(value: unknown): FieldSparkline | undefined { +function getSparkline(value: unknown, field: Field): FieldSparkline | undefined { if (Array.isArray(value)) { return { y: { - name: 'test', + name: `${field.name}-sparkline`, type: FieldType.number, values: value, config: {}, diff --git a/packages/grafana-ui/src/components/Table/TableNG/Cells/renderers.test.tsx b/packages/grafana-ui/src/components/Table/TableNG/Cells/renderers.test.tsx index 2f364bfa732..44221577315 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/Cells/renderers.test.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/Cells/renderers.test.tsx @@ -60,6 +60,15 @@ describe('TableNG Cells renderers', () => { config: {}, state: {}, display: jest.fn(() => ({ text: 'black', color: 'white', numeric: 0 })), + // @ts-ignore: this mock works fine for this test. + getLinks: jest.fn(() => [ + { + title: 'example', + href: 'http://example.com', + target: '_blank', + origin: {}, + }, + ]), }; } diff --git a/packages/grafana-ui/src/components/Table/TableNG/Cells/renderers.tsx b/packages/grafana-ui/src/components/Table/TableNG/Cells/renderers.tsx index fe4bc7e3f0c..8d444eddf40 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/Cells/renderers.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/Cells/renderers.tsx @@ -11,7 +11,6 @@ import { BarGaugeCell } from './BarGaugeCell'; import { DataLinksCell } from './DataLinksCell'; import { GeoCell } from './GeoCell'; import { ImageCell } from './ImageCell'; -import { JSONCell } from './JSONCell'; import { PillCell } from './PillCell'; import { SparklineCell } from './SparklineCell'; @@ -44,10 +43,6 @@ const SPARKLINE_RENDERER: TableCellRenderer = (props) => ( /> ); -const JSON_RENDERER: TableCellRenderer = (props) => ( - -); - const GEO_RENDERER: TableCellRenderer = (props) => ( ); @@ -86,7 +81,7 @@ const CUSTOM_RENDERER: TableCellRenderer = (props) => { const CELL_RENDERERS: Record = { [TableCellDisplayMode.Sparkline]: SPARKLINE_RENDERER, [TableCellDisplayMode.Gauge]: GAUGE_RENDERER, - [TableCellDisplayMode.JSONView]: JSON_RENDERER, + [TableCellDisplayMode.JSONView]: AUTO_RENDERER, [TableCellDisplayMode.Image]: IMAGE_RENDERER, [TableCellDisplayMode.DataLinks]: DATA_LINKS_RENDERER, [TableCellDisplayMode.Actions]: ACTIONS_RENDERER, @@ -121,12 +116,7 @@ export function getAutoRendererResult(field: Field): TableCellRenderer { const firstValue = field.values[0]; if (isDataFrame(firstValue) && isTimeSeriesFrame(firstValue)) { return SPARKLINE_RENDERER; - } else { - return JSON_RENDERER; } } - if (field.type === FieldType.other) { - return JSON_RENDERER; - } return AUTO_RENDERER; } diff --git a/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx b/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx index c23f3a7384b..8c4e1e62959 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx @@ -60,7 +60,6 @@ import { getDefaultRowHeight, getDisplayName, getIsNestedTable, - getTextAlign, getVisibleFields, shouldTextOverflow, getApplyToRowBgFn, @@ -72,6 +71,10 @@ import { isCellInspectEnabled, getCellLinks, withDataLinksActionsTooltip, + displayJsonValue, + getAlignment, + getJustifyContent, + TextAlign, } from './utils'; type CellRootRenderer = (key: React.Key, props: CellRendererProps) => React.ReactNode; @@ -185,7 +188,6 @@ export function TableNG(props: TableNGProps) { fields: visibleFields, hasNestedFrames, defaultHeight: defaultRowHeight, - headerHeight, expandedRows, typographyCtx, }); @@ -307,7 +309,16 @@ export function TableNG(props: TableNGProps) { field.display = getDisplayProcessor({ field, theme }); } - const justifyContent = getTextAlign(field); + // attach JSONCell custom display function to JSONView cell type + if (cellType === TableCellDisplayMode.JSONView || field.type === FieldType.other) { + field.display = displayJsonValue; + } + + // For some cells, "aligning" the cell will mean aligning the inline contents of the cell with + // the text-align css property, and for others, we'll use justify-content to align the cell + // contents with flexbox. We always just get both and provide both when styling the cell. + const textAlign = getAlignment(field); + const justifyContent = getJustifyContent(textAlign); const footerStyles = getFooterStyles(justifyContent); const displayName = getDisplayName(field); const headerCellClass = getHeaderCellStyles(theme, justifyContent); @@ -332,6 +343,7 @@ export function TableNG(props: TableNGProps) { const withTooltip = withDataLinksActionsTooltip(field, cellType); const canBeColorized = cellType === TableCellDisplayMode.ColorBackground || cellType === TableCellDisplayMode.ColorText; + const isMonospace = cellType === TableCellDisplayMode.JSONView; result.colsWithTooltip[displayName] = withTooltip; @@ -344,7 +356,16 @@ export function TableNG(props: TableNGProps) { case TableCellDisplayMode.ColorBackground: case TableCellDisplayMode.ColorText: case TableCellDisplayMode.DataLinks: - cellClass = getCellStyles(theme, justifyContent, shouldWrap, shouldOverflow, withTooltip, canBeColorized); + case TableCellDisplayMode.JSONView: + cellClass = getCellStyles( + theme, + textAlign, + shouldWrap, + shouldOverflow, + canBeColorized, + isMonospace, + cellType === TableCellDisplayMode.DataLinks + ); break; } @@ -786,7 +807,21 @@ const getGridStyles = ( border: 'none', - '.rdg-summary-row': { + // add a box shadow on hover and selection for all body cells + '& > :not(.rdg-summary-row, .rdg-header-row) > .rdg-cell': { + '&:hover, &[aria-selected=true]': { + boxShadow: theme.shadows.z2, + }, + // selected cells should appear below hovered cells. + '&:hover': { + zIndex: theme.zIndex.tooltip - 2, + }, + '&[aria-selected=true]': { + zIndex: theme.zIndex.tooltip - 3, + }, + }, + + '.rdg-header-row, .rdg-summary-row': { '.rdg-cell': { zIndex: theme.zIndex.tooltip - 1, paddingInline: TABLE.CELL_PADDING, @@ -887,35 +922,39 @@ const getHeaderCellStyles = (theme: GrafanaTheme2, justifyContent: Property.Just const getCellStyles = ( theme: GrafanaTheme2, - justifyContent: Property.JustifyContent, + textAlign: TextAlign, shouldWrap: boolean, shouldOverflow: boolean, - hasTooltip: boolean, - isColorized: boolean + isColorized: boolean, + isMonospace: boolean, + // TODO: replace this with cellTypeStyles: TemplateStringsArray object + isLinkCell: boolean ) => css({ display: 'flex', alignItems: 'center', - justifyContent, + textAlign, + justifyContent: getJustifyContent(textAlign), paddingInline: TABLE.CELL_PADDING, minHeight: '100%', backgroundClip: 'padding-box !important', // helps when cells have a bg color - ...(shouldWrap && { whiteSpace: 'pre-line' }), + ...(shouldWrap && { whiteSpace: isMonospace ? 'pre' : 'pre-line' }), + ...(isMonospace && { fontFamily: 'monospace' }), '&:last-child': { borderInlineEnd: 'none', }, // should omit if no cell actions, and no shouldOverflow - '&:hover': { + '&:hover, &[aria-selected=true]': { '.table-cell-actions': { display: 'flex', }, ...(shouldOverflow && { - zIndex: theme.zIndex.tooltip - 2, whiteSpace: 'pre-line', height: 'fit-content', minWidth: 'fit-content', + ...(isMonospace && { whiteSpace: 'pre' }), }), }, @@ -934,4 +973,22 @@ const getCellStyles = ( }, }), }, + + ...(isLinkCell && { + '> a': { + // display: 'inline', // textWrap ? 'block' : 'inline', + whiteSpace: 'nowrap', + paddingInline: theme.spacing(1), + borderRight: `2px solid ${theme.colors.border.medium}`, + + '&:first-of-type': { + paddingInlineStart: 0, + }, + + '&:last-of-type': { + borderRight: 'none', + paddingInlineEnd: 0, + }, + }, + }), }); diff --git a/packages/grafana-ui/src/components/Table/TableNG/hooks.ts b/packages/grafana-ui/src/components/Table/TableNG/hooks.ts index 0533c3cf3d0..a11be3d165a 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/hooks.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/hooks.ts @@ -453,7 +453,6 @@ interface UseRowHeightOptions { fields: Field[]; hasNestedFrames: boolean; defaultHeight: number; - headerHeight: number; expandedRows: Set; typographyCtx: TypographyCtx; } @@ -463,7 +462,6 @@ export function useRowHeight({ fields, hasNestedFrames, defaultHeight, - headerHeight, expandedRows, typographyCtx: { calcRowHeight, avgCharWidth }, }: UseRowHeightOptions): number | ((row: TableRow) => number) { diff --git a/packages/grafana-ui/src/components/Table/TableNG/types.ts b/packages/grafana-ui/src/components/Table/TableNG/types.ts index 742d495b806..bee921c1c71 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/types.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/types.ts @@ -219,13 +219,6 @@ export interface ImageCellProps { rowIdx: number; } -export interface JSONCellProps { - justifyContent: Property.JustifyContent; - value: TableCellValue; - field: Field; - rowIdx: number; -} - export interface DataLinksCellProps { field: Field; rowIdx: number; diff --git a/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts b/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts index 59e58541744..9f48dba7b42 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts @@ -25,129 +25,85 @@ import { getComparator, getDefaultRowHeight, getIsNestedTable, - getTextAlign, + getAlignment, + getJustifyContent, migrateTableDisplayModeToCellOptions, getColumnTypes, getMaxWrapCell, } from './utils'; describe('TableNG utils', () => { - describe('text alignment', () => { - it('should map alignment options to flex values', () => { - // Test 'left' alignment - const leftField = { - name: 'Value', - type: FieldType.string, - values: [], - config: { - custom: { - align: 'left', + describe('alignment', () => { + it.each(['left', 'center', 'right'] as const)('should return "%s" when configured', (align) => { + expect( + getAlignment({ + name: 'Value', + type: FieldType.string, + values: [], + config: { + custom: { + align, + }, }, - }, - }; - expect(getTextAlign(leftField)).toBe('flex-start'); + }) + ).toBe(align); + }); - // Test 'center' alignment - const centerField = { - name: 'Value', - type: FieldType.string, - values: [], - config: { - custom: { - align: 'center', + it.each([ + { type: FieldType.string, align: 'left' }, + { type: FieldType.number, align: 'right' }, + { type: FieldType.boolean, align: 'left' }, + { type: FieldType.time, align: 'left' }, + ])('should return "$align" for field type $type by default', ({ type, align }) => { + expect( + getAlignment({ + name: 'Test', + type, + values: [], + config: { + custom: {}, }, - }, - }; - expect(getTextAlign(centerField)).toBe('center'); + }) + ).toBe(align); + }); - // Test 'right' alignment - const rightField = { - name: 'Value', - type: FieldType.string, - values: [], - config: { - custom: { - align: 'right', + it.each([ + { cellType: undefined, align: 'right' }, + { cellType: TableCellDisplayMode.Auto, align: 'right' }, + { cellType: TableCellDisplayMode.ColorText, align: 'right' }, + { cellType: TableCellDisplayMode.ColorBackground, align: 'right' }, + { cellType: TableCellDisplayMode.Gauge, align: 'left' }, + { cellType: TableCellDisplayMode.JSONView, align: 'left' }, + { cellType: TableCellDisplayMode.DataLinks, align: 'left' }, + ])('numeric field should return "$align" for cell type "$cellType"', ({ align, cellType }) => { + expect( + getAlignment({ + name: 'Test', + type: FieldType.number, + values: [], + config: { + custom: { + ...(cellType !== undefined + ? { + cellOptions: { + type: cellType, + }, + } + : {}), + }, }, - }, - }; - expect(getTextAlign(rightField)).toBe('flex-end'); + }) + ).toBe(align); }); - it('should default to flex-start when no alignment specified', () => { - const field = { - name: 'Value', - type: FieldType.string, - values: [], - config: { - custom: {}, - }, - }; - expect(getTextAlign(field)).toBe('flex-start'); - }); - - it('should default to flex-start when no field is specified', () => { - expect(getTextAlign(undefined)).toBe('flex-start'); - }); - - it('should default to flex-end for number types', () => { - const field = { - name: 'Value', - type: FieldType.number, - values: [], - config: { - custom: {}, - }, - }; - expect(getTextAlign(field)).toBe('flex-end'); - }); - - it('should default to flex-start for string types', () => { - const field = { - name: 'String', - type: FieldType.string, - values: [], - config: { - custom: {}, - }, - }; - expect(getTextAlign(field)).toBe('flex-start'); - }); - - it('should default to flex-start for enum types', () => { - const field = { - name: 'Enum', - type: FieldType.enum, - values: [], - config: { - custom: {}, - }, - }; - expect(getTextAlign(field)).toBe('flex-start'); - }); - - it('should default to flex-start for time types', () => { - const field = { - name: 'Time', - type: FieldType.time, - values: [], - config: { - custom: {}, - }, - }; - expect(getTextAlign(field)).toBe('flex-start'); - }); - - it('should default to flex-start for boolean types', () => { - const field = { - name: 'Active', - type: FieldType.boolean, - values: [], - config: { - custom: {}, - }, - }; - expect(getTextAlign(field)).toBe('flex-start'); + describe('mapping to getJustifyContent', () => { + it.each([ + { align: 'left', expected: 'flex-start' }, + { align: 'center', expected: 'center' }, + { align: 'right', expected: 'flex-end' }, + ] as const)(`should map align "$align" to justifyContent "$expected"`, ({ align, expected }) => { + expect(getJustifyContent(align)).toBe(expected); + }); }); }); @@ -919,6 +875,21 @@ describe('TableNG utils', () => { expect(onClickHandler).not.toHaveBeenCalled(); } ); + + it('should filter out links which contain neither href nor onClick', () => { + const field: Field = { + name: 'test', + type: FieldType.string, + config: {}, + values: ['value1'], + getLinks: (): LinkModel[] => [ + { title: 'Invalid Link', target: '_blank', origin: { datasourceUid: 'test' } } as LinkModel, // No href or onClick + ], + }; + + const links = getCellLinks(field, 0); + expect(links).toEqual([]); + }); }); describe('extractPixelValue', () => { diff --git a/packages/grafana-ui/src/components/Table/TableNG/utils.ts b/packages/grafana-ui/src/components/Table/TableNG/utils.ts index e0b83527470..7ba9cabf855 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/utils.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/utils.ts @@ -11,9 +11,11 @@ import { LinkModel, DisplayValueAlignmentFactors, DataFrame, + DisplayProcessor, } from '@grafana/data'; import { BarGaugeDisplayMode, + FieldTextAlignment, TableCellBackgroundDisplayMode, TableCellDisplayMode, TableCellHeight, @@ -23,10 +25,10 @@ import { getTextColorForAlphaBackground } from '../../../utils/colors'; import { TableCellOptions } from '../types'; import { COLUMN, TABLE } from './constants'; -import { CellColors, TableRow, TableFieldOptionsType, ColumnTypes, FrameToRowsConverter, Comparator } from './types'; +import { CellColors, TableRow, ColumnTypes, FrameToRowsConverter, Comparator } from './types'; /* ---------------------------- Cell calculations --------------------------- */ -export type CellHeightCalculator = (text: string, cellWidth: number) => number; +export type CellNumLinesCalculator = (text: string, cellWidth: number) => number; /** * @internal @@ -128,46 +130,51 @@ export function getMaxWrapCell( * Returns true if text overflow handling should be applied to the cell. */ export function shouldTextOverflow(field: Field): boolean { - let type = getCellOptions(field).type; - - return ( - field.type === FieldType.string && + const cellOptions = getCellOptions(field); + const eligibleCellType = // Tech debt: Technically image cells are of type string, which is misleading (kinda?) - // so we need to ensure we don't apply overflow hover states for type image - type !== TableCellDisplayMode.Image && - type !== TableCellDisplayMode.Pill && - !shouldTextWrap(field) && - !isCellInspectEnabled(field) - ); + // so we need to ensurefield.type === FieldType.string we don't apply overflow hover states for type image + (field.type === FieldType.string && + cellOptions.type !== TableCellDisplayMode.Image && + cellOptions.type !== TableCellDisplayMode.Pill) || + // regardless of the underlying cell type, data links cells have text overflow. + cellOptions.type === TableCellDisplayMode.DataLinks; + + return eligibleCellType && !shouldTextWrap(field) && !isCellInspectEnabled(field); +} + +// we only want to infer justifyContent and textAlign for these cellTypes +const TEXT_CELL_TYPES = new Set([ + TableCellDisplayMode.Auto, + TableCellDisplayMode.ColorText, + TableCellDisplayMode.ColorBackground, +]); + +export type TextAlign = 'left' | 'right' | 'center'; + +/** + * @internal + * Returns the text-align value for inline-displayed cells for a field based on its type and configuration. + */ +export function getAlignment(field: Field): TextAlign { + const align: FieldTextAlignment | undefined = field.config.custom?.align; + + if (!align || align === 'auto') { + if (TEXT_CELL_TYPES.has(getCellOptions(field).type) && field.type === FieldType.number) { + return 'right'; + } + return 'left'; + } + + return align; } /** * @internal - * Returns the text alignment for a field based on its type and configuration. + * Returns the justify-content value for flex-displayed cells for a field based on its type and configuration. */ -export function getTextAlign(field?: Field): Property.JustifyContent { - if (!field) { - return 'flex-start'; - } - - if (field.config.custom) { - const custom: TableFieldOptionsType = field.config.custom; - - switch (custom.align) { - case 'right': - return 'flex-end'; - case 'left': - return 'flex-start'; - case 'center': - return 'center'; - } - } - - if (field.type === FieldType.number) { - return 'flex-end'; - } - - return 'flex-start'; +export function getJustifyContent(textAlign: TextAlign): Property.JustifyContent { + return textAlign === 'center' ? 'center' : textAlign === 'right' ? 'flex-end' : 'flex-start'; } const DEFAULT_CELL_OPTIONS = { type: TableCellDisplayMode.Auto } as const; @@ -628,3 +635,27 @@ export function withDataLinksActionsTooltip(field: Field, cellType: TableCellDis (field.config.links?.length ?? 0) + (field.config.actions?.length ?? 0) > 1 ); } + +export const displayJsonValue: DisplayProcessor = (value: unknown): DisplayValue => { + let displayValue: string; + + // Handle string values that might be JSON + if (typeof value === 'string') { + try { + const parsed = JSON.parse(value); + displayValue = JSON.stringify(parsed, null, ' '); + } catch { + displayValue = value; // Keep original if not valid JSON + } + } else { + // For non-string values, stringify them + try { + displayValue = JSON.stringify(value, null, ' '); + } catch (error) { + // Handle circular references or other stringify errors + displayValue = String(value); + } + } + + return { text: displayValue, numeric: Number.NaN }; +}; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 2d21e3f49bf..ddd03eccbf7 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -8600,7 +8600,10 @@ "no-data": "No data" }, "no-values-label": "No values", - "pagination-summary": "{{itemsRangeStart}} - {{displayedEnd}} of {{numRows}} rows" + "pagination-summary": "{{itemsRangeStart}} - {{displayedEnd}} of {{numRows}} rows", + "sparkline": { + "no-data": "no data" + } }, "tags": { "list-label": "Tags"