From 7e479689395650976f735cfd5528016048ec0652 Mon Sep 17 00:00:00 2001 From: Paul Marbach Date: Fri, 1 Aug 2025 12:27:53 -0400 Subject: [PATCH] TableNG: Wrap text for DataLinks and Pills (#108645) * TableNG: Wrap text for DataLinks and Pills; groundwork for max wrap length * disable editing max wrapped lines for now * disable wrap text line limit e2e * new i18n extract after commenting out input * wip * kill max wrapped lines for now * more cleanup * remove targeting classes added for max wrapped lines * fix Pill test * couple more style cleanups * fix e2es given these updates * add a couple tests * wip: tests * add tests * bump up capital letters in lorem ipsum * fix copy-pasta mistake * use a local count instead of getCellLinks * fix linting on test --- .betterer.results | 6 +- .../panels-suite/table-kitchenSink.spec.ts | 602 +++++++++--------- .../panels-suite/table-sparkline.spec.ts | 39 +- .../grafana-schema/src/common/common.gen.ts | 87 +-- packages/grafana-schema/src/common/table.cue | 25 +- .../Table/TableNG/Cells/PillCell.test.tsx | 152 ++--- .../Table/TableNG/Cells/PillCell.tsx | 92 ++- .../src/components/Table/TableNG/TableNG.tsx | 67 +- .../components/Table/TableNG/hooks.test.ts | 81 +-- .../src/components/Table/TableNG/hooks.ts | 13 +- .../src/components/Table/TableNG/types.ts | 11 +- .../components/Table/TableNG/utils.test.ts | 482 ++++++-------- .../src/components/Table/TableNG/utils.ts | 132 +++- .../table/cells/AutoCellOptionsEditor.tsx | 2 - .../ColorBackgroundCellOptionsEditor.tsx | 2 - .../table/table-new/TableCellOptionEditor.tsx | 21 +- .../table-new/cells/AutoCellOptionsEditor.tsx | 27 - .../ColorBackgroundCellOptionsEditor.tsx | 5 +- .../table-new/cells/TextWrapOptionsEditor.tsx | 29 + public/locales/en-US/grafana.json | 5 +- 20 files changed, 930 insertions(+), 950 deletions(-) delete mode 100644 public/app/plugins/panel/table/table-new/cells/AutoCellOptionsEditor.tsx create mode 100644 public/app/plugins/panel/table/table-new/cells/TextWrapOptionsEditor.tsx diff --git a/.betterer.results b/.betterer.results index 239af992de5..0493a87d6d3 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3919,9 +3919,6 @@ exports[`better eslint`] = { "public/app/plugins/panel/table/table-new/TableCellOptionEditor.tsx:5381": [ [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"] ], - "public/app/plugins/panel/table/table-new/cells/AutoCellOptionsEditor.tsx:5381": [ - [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"] - ], "public/app/plugins/panel/table/table-new/cells/BarGaugeCellOptionsEditor.tsx:5381": [ [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"], [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "1"] @@ -3939,6 +3936,9 @@ exports[`better eslint`] = { [0, 0, 0, "\'VerticalGroup\' import from \'@grafana/ui\' is restricted from being used by a pattern. Use Stack component instead.", "0"], [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "1"] ], + "public/app/plugins/panel/table/table-new/cells/TextWrapOptionsEditor.tsx:5381": [ + [0, 0, 0, "Add noMargin prop to Field components to remove built-in margins. Use layout components like Stack or Grid with the gap prop instead for consistent spacing.", "0"] + ], "public/app/plugins/panel/table/table-new/migrations.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], diff --git a/e2e-playwright/panels-suite/table-kitchenSink.spec.ts b/e2e-playwright/panels-suite/table-kitchenSink.spec.ts index f386fb4175c..c7947fcd281 100644 --- a/e2e-playwright/panels-suite/table-kitchenSink.spec.ts +++ b/e2e-playwright/panels-suite/table-kitchenSink.spec.ts @@ -4,12 +4,7 @@ import { test, expect } from '@grafana/plugin-e2e'; const DASHBOARD_UID = 'dcb9f5e9-8066-4397-889e-864b99555dbb'; -test.use({ - viewport: { width: 2000, height: 1080 }, - featureToggles: { - tableNextGen: true, - }, -}); +test.use({ viewport: { width: 2000, height: 1080 }, featureToggles: { tableNextGen: true } }); // helper utils const waitForTableLoad = async (loc: Page | Locator) => { @@ -46,337 +41,324 @@ const getColumnIdx = async (loc: Page | Locator, columnName: string) => { return result; }; -test.describe( - 'Panels test: Table - Kitchen Sink', - { - tag: ['@panels'], - }, - () => { - test('Tests word wrap, hover overflow, and cell inspect', async ({ gotoDashboardPage, selectors, page }) => { - const dashboardPage = await gotoDashboardPage({ - uid: DASHBOARD_UID, - queryParams: new URLSearchParams({ editPanel: '1' }), - }); +test.describe('Panels test: Table - Kitchen Sink', { tag: ['@panels', '@table'] }, () => { + test('Tests word wrap, hover overflow, and cell inspect', async ({ gotoDashboardPage, selectors, page }) => { + const dashboardPage = await gotoDashboardPage({ + uid: DASHBOARD_UID, + queryParams: new URLSearchParams({ editPanel: '1' }), + }); - await expect( - dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) - ).toBeVisible(); + await expect( + 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); + // 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'); + const longTextColIdx = await getColumnIdx(page, 'Long Text'); - // text wrapping is enabled by default on this panel. - await expect(getCellHeight(page, 1, longTextColIdx)).resolves.toBeGreaterThan(100); + // text wrapping is enabled by default on this panel. + 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, longTextColIdx)).resolves.toBeLessThan(100); + // FIXME very bad selector to get the correct "wrap text" toggle here. + // toggle the lorem ipsum column's wrap text toggle and confirm that the height shrinks. + await page + .locator('[id="Override 13"]') + .locator(`[aria-label="${selectors.components.PanelEditor.OptionsPane.fieldLabel('Wrap text')}"]`) + .click(); + await expect(getCellHeight(page, 1, longTextColIdx)).resolves.toBeLessThan(100); - // test that hover overflow works. - const loremIpsumCell = await getCell(page, 1, longTextColIdx); - await loremIpsumCell.scrollIntoViewIfNeeded(); - await loremIpsumCell.hover(); - 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); + // test that hover overflow works. + const loremIpsumCell = await getCell(page, 1, longTextColIdx); + await loremIpsumCell.scrollIntoViewIfNeeded(); + await loremIpsumCell.hover(); + 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. + // enable cell inspect, confirm that hover no longer triggers. + await dashboardPage + .getByGrafanaSelector(selectors.components.PanelEditor.OptionsPane.fieldLabel('Cell options Cell value inspect')) + .first() + .locator('label[for="custom.inspect"]') + .click(); + await loremIpsumCell.hover(); + 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(); + const loremIpsumText = await loremIpsumCell.textContent(); + expect(loremIpsumText).toBeDefined(); + await expect(page.getByRole('dialog').getByText(loremIpsumText!)).toBeVisible(); + }); + + test('Tests visibility and display name via overrides', async ({ gotoDashboardPage, selectors, page }) => { + const dashboardPage = await gotoDashboardPage({ + uid: DASHBOARD_UID, + queryParams: new URLSearchParams({ editPanel: '1' }), + }); + + await expect( + dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) + ).toBeVisible(); + + // confirm that "State" column is hidden by default. + expect(page.getByRole('row').nth(0)).not.toContainText('State'); + + // toggle the "State" column visibility and test that it appears before re-hiding it. + // FIXME this selector is utterly godawful, but there's no way to give testIds or aria-labels or anything to + // the panel editor builder. we should fix that to make e2e's easier to write for our team. + const hideStateColumnSwitch = page.locator('[id="Override 12"]').locator('label').last(); + await hideStateColumnSwitch.click(); + expect(page.getByRole('row').nth(0)).toContainText('State'); + + // now change the display name of the "State" column. + // FIXME it would be good to have a better selector here too. + const displayNameInput = page.locator('[id="Override 12"]').locator('input[value="State"]').last(); + await displayNameInput.fill('State (renamed)'); + await displayNameInput.press('Enter'); + expect(page.getByRole('row').nth(0)).toContainText('State (renamed)'); + }); + + // we test niche cases for sorting, filtering, pagination, etc. in a unit tests already. + // we mainly want to test the happiest paths for these in e2es as well to check for integration + // issues, but the unit tests can confirm that the internal logic works as expected much more quickly and thoroughly. + // hashtag testing pyramid. + test('Tests sorting by column', async ({ gotoDashboardPage, selectors, page }) => { + const dashboardPage = await gotoDashboardPage({ + uid: DASHBOARD_UID, + queryParams: new URLSearchParams({ editPanel: '1' }), + }); + + await expect( + dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) + ).toBeVisible(); + + // click the "State" column header to sort it. + const stateColumnHeader = await getCell(page, 0, 1); + + await stateColumnHeader.getByText('Info').click(); + await expect(stateColumnHeader).toHaveAttribute('aria-sort', 'ascending'); + expect(getCell(page, 1, 1)).resolves.toContainText('down'); // down or down fast + + await stateColumnHeader.getByText('Info').click(); + await expect(stateColumnHeader).toHaveAttribute('aria-sort', 'descending'); + expect(getCell(page, 1, 1)).resolves.toContainText('up'); // up or up fast + + await stateColumnHeader.getByText('Info').click(); + await expect(stateColumnHeader).not.toHaveAttribute('aria-sort'); + }); + + test('Tests filtering within a column', async ({ gotoDashboardPage, selectors, page }) => { + const dashboardPage = await gotoDashboardPage({ + uid: DASHBOARD_UID, + queryParams: new URLSearchParams({ editPanel: '1' }), + }); + + await expect( + dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) + ).toBeVisible(); + + 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, infoColumnIdx)).textContent())!; + await stateColumnHeader.getByTestId(selectors.components.Panels.Visualization.TableNG.Filters.HeaderButton).click(); + const filterContainer = dashboardPage.getByGrafanaSelector( + selectors.components.Panels.Visualization.TableNG.Filters.Container + ); + + await expect(filterContainer).toBeVisible(); + + // select all, then click the first value to unselect it, filtering it out. + await filterContainer.getByTestId(selectors.components.Panels.Visualization.TableNG.Filters.SelectAll).click(); + await filterContainer.getByTitle(firstStateValue, { exact: true }).locator('label').click(); + await filterContainer.getByRole('button', { name: 'Ok' }).click(); + + // make sure the filter container closed when we clicked "Ok". + await expect(filterContainer).not.toBeVisible(); + + // did it actually filter out our value? + await expect(getCell(page, 1, infoColumnIdx)).resolves.not.toHaveText(firstStateValue); + }); + + test('Tests pagination, row height adjustment', async ({ gotoDashboardPage, selectors, page }) => { + const rowRe = /([\d]+) - ([\d]+) of ([\d]+) rows/; + const getRowStatus = async (page: Page | Locator) => { + const text = (await page.getByText(rowRe).textContent()) ?? ''; + const match = text.match(rowRe); + return { + start: parseInt(match?.[1] ?? '0', 10), + end: parseInt(match?.[2] ?? '0', 10), + total: parseInt(match?.[3] ?? '0', 10), + }; + }; + + const dashboardPage = await gotoDashboardPage({ + uid: DASHBOARD_UID, + queryParams: new URLSearchParams({ editPanel: '1' }), + }); + + await expect( + dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) + ).toBeVisible(); + + await page + .getByLabel(selectors.components.PanelEditor.OptionsPane.fieldLabel(`Enable pagination`), { exact: true }) + .click(); + + // because of text wrapping, we're guaranteed to only be showing a single row when we enable pagination. + await expect(page.getByText(/([\d]+) - ([\d]+) of ([\d]+) rows/)).toBeVisible(); + + // FIXME horrible selector for the "Wrap text" toggle for the "Long text" column. + await page + .locator('[id="Override 13"]') + .locator(`[aria-label="${selectors.components.PanelEditor.OptionsPane.fieldLabel('Wrap text')}"]`) + .click(); + + // any number of rows that is not "1" is allowed here, we don't want to police the exact number of rows that + // are rendered since there are tons of factors which could effect this. we do want to grab this number for comparison + // in a second, though. + const smallRowStatus = await getRowStatus(page); + expect(smallRowStatus.end).toBeGreaterThan(1); + expect(page.getByRole('grid').getByRole('row')).toHaveCount(smallRowStatus.end + 1); + + // change cell height to Large + await dashboardPage + .getByGrafanaSelector(selectors.components.PanelEditor.OptionsPane.fieldLabel('Table Cell height')) + .locator('input') + .last() + .click(); + const largeRowStatus = await getRowStatus(page); + expect(largeRowStatus.end).toBeLessThan(smallRowStatus.end); + expect(page.getByRole('grid').getByRole('row')).toHaveCount(largeRowStatus.end + 1); + + // click a page over with the directional nav + await page.getByLabel('next page').click(); + const nextPageStatus = await getRowStatus(page); + expect(nextPageStatus.start).toBe(largeRowStatus.end + 1); + expect(nextPageStatus.end).toBe(largeRowStatus.end * 2); + expect(nextPageStatus.total).toBe(largeRowStatus.total); + + // click a page number + await page.getByTestId('data-testid panel content').getByRole('navigation').getByText('4', { exact: true }).click(); + const fourthPageStatus = await getRowStatus(page); + expect(fourthPageStatus.start).toBe(largeRowStatus.end * 3 + 1); + expect(fourthPageStatus.end).toBe(largeRowStatus.end * 4); + expect(fourthPageStatus.total).toBe(largeRowStatus.total); + }); + + test.skip('Tests DataLinks (single and multi) and actions', async ({ gotoDashboardPage, selectors, page }) => { + const addDataLink = async (title: string, url: string) => { await dashboardPage .getByGrafanaSelector( - selectors.components.PanelEditor.OptionsPane.fieldLabel('Cell options Cell value inspect') + selectors.components.PanelEditor.OptionsPane.fieldLabel('Data links and actions Data links') ) - .first() - .locator('label[for="custom.inspect"]') + .locator('button') + .filter({ hasText: 'Add link' }) .click(); - await loremIpsumCell.hover(); - 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(); - const loremIpsumText = await loremIpsumCell.textContent(); - expect(loremIpsumText).toBeDefined(); - await expect(page.getByRole('dialog').getByText(loremIpsumText!)).toBeVisible(); + // DataLinks dialog has popped open - fill it in and add a global datalink. + await expect(page.getByRole('dialog')).toBeVisible(); + await page.getByRole('dialog').locator('#link-title').fill(title); + 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').locator('button[aria-disabled="false"]').filter({ hasText: 'Save' }).click(); + await expect(page.getByRole('dialog')).not.toBeVisible(); + }; + + const dashboardPage = await gotoDashboardPage({ + uid: DASHBOARD_UID, + queryParams: new URLSearchParams({ editPanel: '1' }), }); - test('Tests visibility and display name via overrides', async ({ gotoDashboardPage, selectors, page }) => { - const dashboardPage = await gotoDashboardPage({ - uid: DASHBOARD_UID, - queryParams: new URLSearchParams({ editPanel: '1' }), - }); + await expect( + dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) + ).toBeVisible(); - await expect( - dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) - ).toBeVisible(); + // disable text wrapping for this test to make it easier to click the links, the long lorem ipsum + // can push the links off the screen. + // FIXME very bad selector to get the correct "wrap text" toggle here. + await page + .locator('[id="Override 13"]') + .locator(`[aria-label="${selectors.components.PanelEditor.OptionsPane.fieldLabel('Wrap text')}"]`) + .click(); - // confirm that "State" column is hidden by default. - expect(page.getByRole('row').nth(0)).not.toContainText('State'); + const infoColumnIdx = await getColumnIdx(page, 'Info'); + const pillColIdx = await getColumnIdx(page, 'Pills'); + const dataLinkColIdx = await getColumnIdx(page, 'Data Link'); - // toggle the "State" column visibility and test that it appears before re-hiding it. - // FIXME this selector is utterly godawful, but there's no way to give testIds or aria-labels or anything to - // the panel editor builder. we should fix that to make e2e's easier to write for our team. - const hideStateColumnSwitch = page.locator('[id="Override 12"]').locator('label').last(); - await hideStateColumnSwitch.click(); - expect(page.getByRole('row').nth(0)).toContainText('State'); + // Info column has a single DataLink by default. + 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'); - // now change the display name of the "State" column. - // FIXME it would be good to have a better selector here too. - const displayNameInput = page.locator('[id="Override 12"]').locator('input[value="State"]').last(); - await displayNameInput.fill('State (renamed)'); - await displayNameInput.press('Enter'); - expect(page.getByRole('row').nth(0)).toContainText('State (renamed)'); - }); + // now, add a DataLink to the whole table + await addDataLink('Test link', 'https://grafana.com'); - // we test niche cases for sorting, filtering, pagination, etc. in a unit tests already. - // we mainly want to test the happiest paths for these in e2es as well to check for integration - // issues, but the unit tests can confirm that the internal logic works as expected much more quickly and thoroughly. - // hashtag testing pyramid. - test('Tests sorting by column', async ({ gotoDashboardPage, selectors, page }) => { - const dashboardPage = await gotoDashboardPage({ - uid: DASHBOARD_UID, - queryParams: new URLSearchParams({ editPanel: '1' }), - }); + // 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; + } - await expect( - dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) - ).toBeVisible(); + const cell = await getCell(page, 1, colIdx); + await expect(cell.locator('a')).toBeVisible(); + expect(cell.locator('a')).toHaveAttribute('href'); + expect(cell.locator('a')).not.toHaveAttribute('aria-haspopup', 'menu'); + } - // click the "State" column header to sort it. - const stateColumnHeader = await getCell(page, 0, 1); + const headerContainer = dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.headerContainer); - await stateColumnHeader.getByText('Info').click(); - await expect(stateColumnHeader).toHaveAttribute('aria-sort', 'ascending'); - expect(getCell(page, 1, 1)).resolves.toContainText('down'); // down or down fast + // add another data link. now we'll check that the multi-link popups work. + await addDataLink('Another test link', 'https://grafana.com/foo'); - await stateColumnHeader.getByText('Info').click(); - await expect(stateColumnHeader).toHaveAttribute('aria-sort', 'descending'); - expect(getCell(page, 1, 1)).resolves.toContainText('up'); // up or up fast - - await stateColumnHeader.getByText('Info').click(); - await expect(stateColumnHeader).not.toHaveAttribute('aria-sort'); - }); - - test('Tests filtering within a column', async ({ gotoDashboardPage, selectors, page }) => { - const dashboardPage = await gotoDashboardPage({ - uid: DASHBOARD_UID, - queryParams: new URLSearchParams({ editPanel: '1' }), - }); - - await expect( - dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) - ).toBeVisible(); - - 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, infoColumnIdx)).textContent())!; - await stateColumnHeader - .getByTestId(selectors.components.Panels.Visualization.TableNG.Filters.HeaderButton) - .click(); - const filterContainer = dashboardPage.getByGrafanaSelector( - selectors.components.Panels.Visualization.TableNG.Filters.Container - ); - - await expect(filterContainer).toBeVisible(); - - // select all, then click the first value to unselect it, filtering it out. - await filterContainer.getByTestId(selectors.components.Panels.Visualization.TableNG.Filters.SelectAll).click(); - await filterContainer.getByTitle(firstStateValue, { exact: true }).locator('label').click(); - await filterContainer.getByRole('button', { name: 'Ok' }).click(); - - // make sure the filter container closed when we clicked "Ok". - await expect(filterContainer).not.toBeVisible(); - - // did it actually filter out our value? - await expect(getCell(page, 1, infoColumnIdx)).resolves.not.toHaveText(firstStateValue); - }); - - test('Tests pagination, row height adjustment', async ({ gotoDashboardPage, selectors, page }) => { - const rowRe = /([\d]+) - ([\d]+) of ([\d]+) rows/; - const getRowStatus = async (page: Page | Locator) => { - const text = (await page.getByText(rowRe).textContent()) ?? ''; - const match = text.match(rowRe); - return { - start: parseInt(match?.[1] ?? '0', 10), - end: parseInt(match?.[2] ?? '0', 10), - total: parseInt(match?.[3] ?? '0', 10), - }; - }; - - const dashboardPage = await gotoDashboardPage({ - uid: DASHBOARD_UID, - queryParams: new URLSearchParams({ editPanel: '1' }), - }); - - await expect( - dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) - ).toBeVisible(); - - await page - .getByLabel(selectors.components.PanelEditor.OptionsPane.fieldLabel(`Enable pagination`), { exact: true }) - .click(); - - // because of text wrapping, we're guaranteed to only be showing a single row when we enable pagination. - await expect(page.getByText(/([\d]+) - ([\d]+) of ([\d]+) rows/)).toBeVisible(); - - // disable text wrap and see the number of rows. - await dashboardPage - .getByGrafanaSelector(selectors.components.PanelEditor.OptionsPane.fieldLabel('Wrap text')) - .last() - .click(); - - // any number of rows that is not "1" is allowed here, we don't want to police the exact number of rows that - // are rendered since there are tons of factors which could effect this. we do want to grab this number for comparison - // in a second, though. - const smallRowStatus = await getRowStatus(page); - expect(smallRowStatus.end).toBeGreaterThan(1); - expect(page.getByRole('grid').getByRole('row')).toHaveCount(smallRowStatus.end + 1); - - // change cell height to Large - await dashboardPage - .getByGrafanaSelector(selectors.components.PanelEditor.OptionsPane.fieldLabel('Table Cell height')) - .locator('input') - .last() - .click(); - const largeRowStatus = await getRowStatus(page); - expect(largeRowStatus.end).toBeLessThan(smallRowStatus.end); - expect(page.getByRole('grid').getByRole('row')).toHaveCount(largeRowStatus.end + 1); - - // click a page over with the directional nav - await page.getByLabel('next page').click(); - const nextPageStatus = await getRowStatus(page); - expect(nextPageStatus.start).toBe(largeRowStatus.end + 1); - expect(nextPageStatus.end).toBe(largeRowStatus.end * 2); - expect(nextPageStatus.total).toBe(largeRowStatus.total); - - // click a page number - await page - .getByTestId('data-testid panel content') - .getByRole('navigation') - .getByText('4', { exact: true }) - .click(); - const fourthPageStatus = await getRowStatus(page); - expect(fourthPageStatus.start).toBe(largeRowStatus.end * 3 + 1); - expect(fourthPageStatus.end).toBe(largeRowStatus.end * 4); - expect(fourthPageStatus.total).toBe(largeRowStatus.total); - }); - - // TODO: skipping this test for now due to flakiness in adding DataLinks. - test.skip('Tests DataLinks (single and multi) and actions', async ({ gotoDashboardPage, selectors, page }) => { - const addDataLink = async (title: string, url: string) => { - await dashboardPage - .getByGrafanaSelector( - selectors.components.PanelEditor.OptionsPane.fieldLabel('Data links and actions Data links') - ) - .locator('button') - .filter({ hasText: 'Add link' }) - .click(); - - // DataLinks dialog has popped open - fill it in and add a global datalink. - await expect(page.getByRole('dialog')).toBeVisible(); - await page.getByRole('dialog').locator('#link-title').fill(title); - 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').locator('button[aria-disabled="false"]').filter({ hasText: 'Save' }).click(); - await expect(page.getByRole('dialog')).not.toBeVisible(); - }; - - const dashboardPage = await gotoDashboardPage({ - uid: DASHBOARD_UID, - queryParams: new URLSearchParams({ editPanel: '1' }), - }); - - await expect( - dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) - ).toBeVisible(); - - // disable text wrapping for this test to make it easier to click the links, the long lorem ipsum - // can push the links off the screen. - await dashboardPage - .getByGrafanaSelector(selectors.components.PanelEditor.OptionsPane.fieldLabel('Wrap text')) - .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, infoColumnIdx); - await expect(infoCell.locator('a')).toBeVisible(); - expect(infoCell.locator('a')).toHaveAttribute('href'); - expect(infoCell.locator('a')).not.toHaveAttribute('aria-haspopup'); - - // now, add a DataLink to the whole table - await addDataLink('Test link', 'https://grafana.com'); - - // 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'); + // 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 === infoColumnIdx) { + // the Info column should still have its single link. expect(cell.locator('a')).not.toHaveAttribute('aria-haspopup', 'menu'); + continue; } - const headerContainer = dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.headerContainer); - - // add another data link. now we'll check that the multi-link popups work. - await addDataLink('Another test link', 'https://grafana.com/foo'); - - // 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 === 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(); - - await headerContainer.click(); // convenient just to click the header to close the tooltip. - await expect(page.getByTestId(selectors.components.DataLinksActionsTooltip.tooltipWrapper)).not.toBeVisible(); + // - 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; } - // add an Action to the whole table and check that the action button is added to the tooltip. - // TODO -- saving for another day. + await cell.locator('a').click({ force: true }); + await expect(page.getByTestId(selectors.components.DataLinksActionsTooltip.tooltipWrapper)).toBeVisible(); + + await headerContainer.click(); // convenient just to click the header to close the tooltip. + await expect(page.getByTestId(selectors.components.DataLinksActionsTooltip.tooltipWrapper)).not.toBeVisible(); + } + + // add an Action to the whole table and check that the action button is added to the tooltip. + // TODO -- saving for another day. + }); + + test('Empty Table panel', async ({ gotoDashboardPage, selectors }) => { + const dashboardPage = await gotoDashboardPage({ + uid: DASHBOARD_UID, + queryParams: new URLSearchParams({ editPanel: '2' }), }); - test('Empty Table panel', async ({ gotoDashboardPage, selectors }) => { - const dashboardPage = await gotoDashboardPage({ - uid: DASHBOARD_UID, - queryParams: new URLSearchParams({ editPanel: '2' }), - }); - - await expect( - dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.PanelDataErrorMessage) - ).toBeVisible(); - await expect( - dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) - ).not.toBeVisible(); - }); - } -); + await expect( + dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.PanelDataErrorMessage) + ).toBeVisible(); + await expect( + dashboardPage.getByGrafanaSelector(selectors.components.Panels.Panel.title('Table - Kitchen Sink')) + ).not.toBeVisible(); + }); +}); diff --git a/e2e-playwright/panels-suite/table-sparkline.spec.ts b/e2e-playwright/panels-suite/table-sparkline.spec.ts index 16dc55f3df4..d2ac814e2df 100644 --- a/e2e-playwright/panels-suite/table-sparkline.spec.ts +++ b/e2e-playwright/panels-suite/table-sparkline.spec.ts @@ -1,29 +1,18 @@ import { test, expect } from '@grafana/plugin-e2e'; -test.use({ - viewport: { width: 1280, height: 1080 }, - featureToggles: { - tableNextGen: true, - }, -}); +test.use({ viewport: { width: 1280, height: 1080 }, featureToggles: { tableNextGen: true } }); -test.describe( - 'Panels test: Table - Sparkline', - { - tag: ['@panels'], - }, - () => { - test('Tests sparkline tables are successfully rendered', async ({ gotoDashboardPage, selectors, page }) => { - await gotoDashboardPage({ - uid: 'd6373b49-1957-4f00-9218-ee2120d3ecd9', - queryParams: new URLSearchParams({ editPanel: '2' }), - }); - - await expect(page.getByRole('grid')).toBeVisible(); - - const uplotCount = await page.locator('.uplot').count(); - const rowCount = await page.getByRole('row').count(); - expect(uplotCount).toBe(rowCount - 1); +test.describe('Panels test: Table - Sparkline', { tag: ['@panels', '@table'] }, () => { + test('Tests sparkline tables are successfully rendered', async ({ gotoDashboardPage, selectors, page }) => { + await gotoDashboardPage({ + uid: 'd6373b49-1957-4f00-9218-ee2120d3ecd9', + queryParams: new URLSearchParams({ editPanel: '2' }), }); - } -); + + await expect(page.getByRole('grid')).toBeVisible(); + + const uplotCount = await page.locator('.uplot').count(); + const rowCount = await page.getByRole('row').count(); + expect(uplotCount).toBe(rowCount - 1); + }); +}); diff --git a/packages/grafana-schema/src/common/common.gen.ts b/packages/grafana-schema/src/common/common.gen.ts index b98c83c9525..9b229c4d1e9 100644 --- a/packages/grafana-schema/src/common/common.gen.ts +++ b/packages/grafana-schema/src/common/common.gen.ts @@ -722,6 +722,16 @@ export enum TableCellBackgroundDisplayMode { Gradient = 'gradient', } +/** + * Whenever we add text wrapping, we should add all text wrapping options at once + */ +export interface TableWrapTextOptions { + /** + * if true, wrap the text content of the cell + */ + wrapText?: boolean; +} + /** * Sort by field state */ @@ -755,17 +765,15 @@ export const defaultTableFooterOptions: Partial = { /** * Auto mode table cell options */ -export interface TableAutoCellOptions { +export interface TableAutoCellOptions extends TableWrapTextOptions { type: TableCellDisplayMode.Auto; - wrapText?: boolean; } /** * Colored text cell options */ -export interface TableColorTextCellOptions { +export interface TableColorTextCellOptions extends TableWrapTextOptions { type: TableCellDisplayMode.ColorText; - wrapText?: boolean; } /** @@ -787,7 +795,7 @@ export interface TableImageCellOptions { /** * Show data links in the cell */ -export interface TableDataLinksCellOptions { +export interface TableDataLinksCellOptions extends TableWrapTextOptions { type: TableCellDisplayMode.DataLinks; } @@ -818,11 +826,14 @@ export interface TableSparklineCellOptions extends GraphFieldConfig { /** * Colored background cell options */ -export interface TableColoredBackgroundCellOptions { +export interface TableColoredBackgroundCellOptions extends TableWrapTextOptions { applyToRow?: boolean; mode?: TableCellBackgroundDisplayMode; type: TableCellDisplayMode.ColorBackground; - wrapText?: boolean; +} + +export interface TablePillCellOptions extends TableWrapTextOptions { + type: TableCellDisplayMode.Pill; } /** @@ -841,37 +852,6 @@ export enum TableCellHeight { */ export type TableCellOptions = (TableAutoCellOptions | TableSparklineCellOptions | TableBarGaugeCellOptions | TableColoredBackgroundCellOptions | TableColorTextCellOptions | TableImageCellOptions | TablePillCellOptions | TableDataLinksCellOptions | TableActionsCellOptions | TableJsonViewCellOptions); -/** - * Field options for each field within a table (e.g 10, "The String", 64.20, etc.) - * Generally defines alignment, filtering capabilties, display options, etc. - */ -export interface TableFieldOptions { - align: FieldTextAlignment; - cellOptions: TableCellOptions; - /** - * This field is deprecated in favor of using cellOptions - */ - displayMode?: TableCellDisplayMode; - filterable?: boolean; - hidden?: boolean; // ?? default is missing or false ?? - /** - * Hides any header for a column, useful for columns that show some static content or buttons. - */ - hideHeader?: boolean; - inspect: boolean; - minWidth?: number; - width?: number; - /** - * Enables text wrapping for column headers - */ - wrapHeaderText?: boolean; -} - -export const defaultTableFieldOptions: Partial = { - align: 'auto', - inspect: false, -}; - /** * Use UTC/GMT timezone */ @@ -987,10 +967,37 @@ export enum ComparisonOperation { NEQ = 'neq', } -export interface TablePillCellOptions { - type: TableCellDisplayMode.Pill; +/** + * Field options for each field within a table (e.g 10, "The String", 64.20, etc.) + * Generally defines alignment, filtering capabilties, display options, etc. + */ +export interface TableFieldOptions { + align: FieldTextAlignment; + cellOptions: TableCellOptions; + /** + * This field is deprecated in favor of using cellOptions + */ + displayMode?: TableCellDisplayMode; + filterable?: boolean; + hidden?: boolean; // ?? default is missing or false ?? + /** + * Hides any header for a column, useful for columns that show some static content or buttons. + */ + hideHeader?: boolean; + inspect: boolean; + minWidth?: number; + width?: number; + /** + * Enables text wrapping for column headers + */ + wrapHeaderText?: boolean; } +export const defaultTableFieldOptions: Partial = { + align: 'auto', + inspect: false, +}; + /** * A specific timezone from https://en.wikipedia.org/wiki/Tz_database */ diff --git a/packages/grafana-schema/src/common/table.cue b/packages/grafana-schema/src/common/table.cue index acf4a6b7480..0a15fb926b5 100644 --- a/packages/grafana-schema/src/common/table.cue +++ b/packages/grafana-schema/src/common/table.cue @@ -11,6 +11,12 @@ TableCellDisplayMode: "auto" | "color-text" | "color-background" | "color-backgr // or a gradient. TableCellBackgroundDisplayMode: "basic" | "gradient" @cuetsy(kind="enum",memberNames="Basic|Gradient") +// Whenever we add text wrapping, we should add all text wrapping options at once +TableWrapTextOptions: { + // if true, wrap the text content of the cell + wrapText?: bool +} @cuetsy(kind="interface") + // Sort by field state TableSortByFieldState: { // Sets the display name of the field to sort by @@ -31,14 +37,12 @@ TableFooterOptions: { // Auto mode table cell options TableAutoCellOptions: { type: TableCellDisplayMode & "auto" - wrapText?: bool -} @cuetsy(kind="interface") +} & TableWrapTextOptions @cuetsy(kind="interface") // Colored text cell options TableColorTextCellOptions: { type: TableCellDisplayMode & "color-text" - wrapText?: bool -} @cuetsy(kind="interface") +} & TableWrapTextOptions @cuetsy(kind="interface") // Json view cell options TableJsonViewCellOptions: { @@ -55,7 +59,7 @@ TableImageCellOptions: { // Show data links in the cell TableDataLinksCellOptions: { type: TableCellDisplayMode & "data-links" -} @cuetsy(kind="interface") +} & TableWrapTextOptions @cuetsy(kind="interface") // Show actions in the cell TableActionsCellOptions: { @@ -81,8 +85,11 @@ TableColoredBackgroundCellOptions: { type: TableCellDisplayMode & "color-background" mode?: TableCellBackgroundDisplayMode applyToRow?: bool - wrapText?: bool -} @cuetsy(kind="interface") +} & TableWrapTextOptions @cuetsy(kind="interface") + +TablePillCellOptions: { + type: TableCellDisplayMode & "pill" +} & TableWrapTextOptions @cuetsy(kind="interface") // Height of a table cell TableCellHeight: "sm" | "md" | "lg" | "auto" @cuetsy(kind="enum") @@ -108,7 +115,3 @@ TableFieldOptions: { // Enables text wrapping for column headers wrapHeaderText?: bool } @cuetsy(kind="interface") - -TablePillCellOptions: { - type: TableCellDisplayMode & "pill" -} @cuetsy(kind="interface") diff --git a/packages/grafana-ui/src/components/Table/TableNG/Cells/PillCell.test.tsx b/packages/grafana-ui/src/components/Table/TableNG/Cells/PillCell.test.tsx index b7249e88b2b..55b9d3e19ba 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/Cells/PillCell.test.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/Cells/PillCell.test.tsx @@ -1,54 +1,18 @@ import { render, RenderResult } from '@testing-library/react'; -import { DataFrame, Field, FieldType, GrafanaTheme2, MappingType, createTheme } from '@grafana/data'; -import { TableCellDisplayMode, TablePillCellOptions } from '@grafana/schema'; +import { Field, FieldType, MappingType, createTheme } from '@grafana/data'; -import { mockThemeContext } from '../../../../themes/ThemeContext'; - -import { PillCell, getStyles } from './PillCell'; +import { PillCell } from './PillCell'; describe('PillCell', () => { - let pillClass: string; - let restoreThemeContext: () => void; + const theme = createTheme(); - beforeEach(() => { - pillClass = getStyles(createTheme()).pill; - restoreThemeContext = mockThemeContext(createTheme()); - }); - - afterEach(() => { - restoreThemeContext(); - }); - - const mockCellOptions: TablePillCellOptions = { - type: TableCellDisplayMode.Pill, - }; - - const mockField: Field = { + const fieldWithValues = (values: unknown[]): Field => ({ name: 'test', type: FieldType.string, - values: [], + values: values, config: {}, - }; - - const mockFrame: DataFrame = { - name: 'test', - fields: [mockField], - length: 1, - }; - - const defaultProps = { - field: mockField, - justifyContent: 'flex-start' as const, - cellOptions: mockCellOptions, - rowIdx: 0, - frame: mockFrame, - height: 30, - width: 100, - theme: {} as GrafanaTheme2, - cellInspect: false, - showFilters: false, - }; + }); const ser = new XMLSerializer(); @@ -60,88 +24,84 @@ describe('PillCell', () => { // one class for lightTextPill, darkTextPill describe('Color by hash (classic palette)', () => { - const props = { ...defaultProps }; - it('single value', () => { expectHTML( - render(), - `value1` + render(), + `value1` ); }); it('empty string', () => { - expectHTML(render(), ''); + expectHTML(render(), ''); }); - // it('null', () => { - // expectHTML( - // render(), - // 'value1' - // ); - // }); + it('null', () => { + const { container } = render(); + expect(container).toBeEmptyDOMElement(); + }); it('CSV values', () => { expectHTML( - render(), + render(), ` - value1 - value2 - value3 + value1 + value2 + value3 ` ); }); it('JSON array values', () => { expectHTML( - render(), + render(), ` - value1 - value2 - value3 + value1 + value2 + value3 ` ); }); - - // TODO: handle null values? }); describe('Color by value mappings', () => { - const field: Field = { - ...mockField, - config: { - ...mockField.config, - mappings: [ - { - type: MappingType.ValueToText, - options: { - success: { color: '#00FF00' }, - error: { color: '#FF0000' }, - warning: { color: '#FFFF00' }, - }, - }, - ], - }, - display: (value: unknown) => ({ - text: String(value), - color: - value === 'success' ? '#00FF00' : value === 'error' ? '#FF0000' : value === 'warning' ? '#FFFF00' : '#FF780A', - numeric: 0, - }), - }; - - const props = { - ...defaultProps, - field, - }; - it('CSV values', () => { + const mockField = fieldWithValues(['success,error,warning,unknown']); + const field = { + ...mockField, + config: { + ...mockField.config, + mappings: [ + { + type: MappingType.ValueToText, + options: { + success: { color: '#00FF00' }, + error: { color: '#FF0000' }, + warning: { color: '#FFFF00' }, + }, + }, + ], + }, + display: (value: unknown) => ({ + text: String(value), + color: + value === 'success' + ? '#00FF00' + : value === 'error' + ? '#FF0000' + : value === 'warning' + ? '#FFFF00' + : '#FF780A', + numeric: 0, + }), + } satisfies Field; + expectHTML( - render(), + render(), ` - success - error - warning - unknown + success + error + warning + unknown ` ); }); diff --git a/packages/grafana-ui/src/components/Table/TableNG/Cells/PillCell.tsx b/packages/grafana-ui/src/components/Table/TableNG/Cells/PillCell.tsx index c1ca23b7f69..35b759aa224 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/Cells/PillCell.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/Cells/PillCell.tsx @@ -1,4 +1,3 @@ -import { css } from '@emotion/css'; import { useMemo } from 'react'; import { @@ -8,11 +7,36 @@ import { Field, getColorByStringHash, FALLBACK_COLOR, + fieldColorModeRegistry, } from '@grafana/data'; import { FieldColorModeId } from '@grafana/schema'; -import { useStyles2, useTheme2 } from '../../../../themes/ThemeContext'; -import { TableCellRendererProps } from '../types'; +import { PillCellProps, TableCellValue } from '../types'; + +export function PillCell({ rowIdx, field, theme }: PillCellProps) { + const value = field.values[rowIdx]; + const pills: Pill[] = useMemo(() => { + const pillValues = inferPills(value); + return pillValues.length > 0 ? createPills(pillValues, field, theme) : []; + }, [value, field, theme]); + + if (pills.length === 0) { + return null; + } + + return pills.map((pill) => ( + + {pill.value} + + )); +} interface Pill { value: string; @@ -21,6 +45,9 @@ interface Pill { color: string; } +const SPLIT_RE = /\s*,\s*/; +const TRANSPARENT = 'rgba(0,0,0,0)'; + function createPills(pillValues: string[], field: Field, theme: GrafanaTheme2): Pill[] { return pillValues.map((pill, index) => { const bgColor = getPillColor(pill, field, theme); @@ -34,38 +61,13 @@ function createPills(pillValues: string[], field: Field, theme: GrafanaTheme2): }); } -export function PillCell({ value, field }: TableCellRendererProps) { - const styles = useStyles2(getStyles); - const theme = useTheme2(); - - const pills: Pill[] = useMemo(() => { - const pillValues = inferPills(String(value)); - return createPills(pillValues, field, theme); - }, [value, field, theme]); - - return pills.map((pill) => ( - - {pill.value} - - )); -} - -const SPLIT_RE = /\s*,\s*/; -const TRANSPARENT = 'rgba(0,0,0,0)'; - -export function inferPills(value: string): string[] { - if (value === '') { +export function inferPills(rawValue: TableCellValue): string[] { + if (rawValue === '' || rawValue == null) { return []; } + const value = String(rawValue); + if (value[0] === '[') { try { return JSON.parse(value); @@ -77,6 +79,7 @@ export function inferPills(value: string): string[] { return value.trim().split(SPLIT_RE); } +// FIXME: this does not yet support "shades of a color" function getPillColor(value: string, field: Field, theme: GrafanaTheme2): string { const cfg = field.config; @@ -88,19 +91,14 @@ function getPillColor(value: string, field: Field, theme: GrafanaTheme2): string return theme.visualization.getColorByName(cfg.color?.fixedColor ?? FALLBACK_COLOR); } - // TODO: instead of classicColors we need to pull colors from theme, same way as FieldColorModeId.PaletteClassicByName (see fieldColor.ts) - return getColorByStringHash(classicColors, value); -} + let colors = classicColors; + const configuredColor = cfg.color; + if (configuredColor) { + const mode = fieldColorModeRegistry.get(configuredColor.mode); + if (typeof mode?.getColors === 'function') { + colors = mode.getColors(theme); + } + } -export const getStyles = (theme: GrafanaTheme2) => ({ - pill: css({ - display: 'inline-block', - padding: theme.spacing(0.25, 0.75), - marginInlineEnd: theme.spacing(0.5), - marginBlock: theme.spacing(0.5), - borderRadius: theme.shape.radius.default, - fontSize: theme.typography.bodySmall.fontSize, - lineHeight: theme.typography.bodySmall.lineHeight, - whiteSpace: 'nowrap', - }), -}); + return getColorByStringHash(colors, value); +} diff --git a/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx b/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx index 40cd49278d3..8d6c9bb5ef8 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx @@ -367,14 +367,15 @@ export function TableNG(props: TableNGProps) { case TableCellDisplayMode.ColorText: case TableCellDisplayMode.DataLinks: case TableCellDisplayMode.JSONView: + case TableCellDisplayMode.Pill: cellClass = getCellStyles( theme, + cellType, textAlign, shouldWrap, shouldOverflow, canBeColorized, - isMonospace, - cellType === TableCellDisplayMode.DataLinks + isMonospace ); break; } @@ -821,6 +822,7 @@ const getGridStyles = ( border: 'none', '.rdg-cell': { + padding: TABLE.CELL_PADDING, '&:last-child': { borderInlineEnd: 'none', }, @@ -843,8 +845,6 @@ const getGridStyles = ( '.rdg-header-row, .rdg-summary-row': { '.rdg-cell': { zIndex: theme.zIndex.tooltip - 1, - paddingInline: TABLE.CELL_PADDING, - paddingBlock: TABLE.CELL_PADDING, }, }, }), @@ -941,35 +941,36 @@ const getHeaderCellStyles = (theme: GrafanaTheme2, justifyContent: Property.Just const getCellStyles = ( theme: GrafanaTheme2, + cellType: TableCellDisplayMode, textAlign: TextAlign, shouldWrap: boolean, shouldOverflow: boolean, isColorized: boolean, - isMonospace: boolean, - // TODO: replace this with cellTypeStyles: TemplateStringsArray object - isLinkCell: boolean + isMonospace: boolean ) => css({ display: 'flex', alignItems: 'center', textAlign, justifyContent: getJustifyContent(textAlign), - paddingInline: TABLE.CELL_PADDING, minHeight: '100%', backgroundClip: 'padding-box !important', // helps when cells have a bg color + ...(shouldWrap && { whiteSpace: isMonospace ? 'pre' : 'pre-line' }), ...(isMonospace && { fontFamily: 'monospace' }), - // should omit if no cell actions, and no shouldOverflow '&:hover, &[aria-selected=true]': { '.table-cell-actions': { display: 'flex', }, ...(shouldOverflow && { - whiteSpace: 'pre-line', + zIndex: theme.zIndex.tooltip - 2, + whiteSpace: isMonospace ? 'pre' : 'pre-line', height: 'fit-content', minWidth: 'fit-content', - ...(isMonospace && { whiteSpace: 'pre' }), + ...(cellType === TableCellDisplayMode.Pill && { + flexWrap: 'wrap', + }), }), }, @@ -989,21 +990,39 @@ const getCellStyles = ( }), }, - ...(isLinkCell && { + ...(cellType === TableCellDisplayMode.DataLinks && { + ...(shouldWrap && { + flexDirection: 'column', + justifyContent: 'center', + alignItems: getJustifyContent(textAlign), + }), '> a': { - // display: 'inline', // textWrap ? 'block' : 'inline', + flexWrap: 'nowrap', + ...(!shouldWrap && { + paddingInline: theme.spacing(0.5), + borderRight: `2px solid ${theme.colors.border.medium}`, + '&:first-child': { + paddingInlineStart: 0, + }, + '&:last-child': { + paddingInlineEnd: 0, + borderRight: 'none', + }, + }), + }, + }), + + ...(cellType === TableCellDisplayMode.Pill && { + display: 'inline-flex', + gap: theme.spacing(0.5), + flexWrap: shouldWrap ? 'wrap' : 'nowrap', + '> span': { + display: 'flex', + padding: theme.spacing(0.25, 0.75), + borderRadius: theme.shape.radius.default, + fontSize: theme.typography.bodySmall.fontSize, + lineHeight: theme.typography.bodySmall.lineHeight, 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.test.ts b/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts index f7a1a1b858a..ce378961589 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts @@ -490,24 +490,26 @@ describe('TableNG hooks', () => { const { fields } = setupData(); + let modifiedFields = fields.map((field) => { + if (field.name === 'name') { + return { + ...field, + name: 'Longer name that needs wrapping', + config: { + ...field.config, + custom: { + ...field.config?.custom, + wrapHeaderText: true, + }, + }, + }; + } + return field; + }); + renderHook(() => { return useHeaderHeight({ - fields: fields.map((field) => { - if (field.name === 'name') { - return { - ...field, - name: 'Longer name that needs wrapping', - config: { - ...field.config, - custom: { - ...field.config?.custom, - wrapHeaderText: true, - }, - }, - }; - } - return field; - }), + fields: modifiedFields, columnWidths: [100, 100, 100], enabled: true, typographyCtx: { ...typographyCtx, wrappedCount: countFn }, @@ -516,27 +518,29 @@ describe('TableNG hooks', () => { }); }); - expect(countFn).toHaveBeenCalledWith('Longer name that needs wrapping', 86); + expect(countFn).toHaveBeenCalledWith('Longer name that needs wrapping', 86, modifiedFields[0], -1); + + modifiedFields = fields.map((field) => { + if (field.name === 'name') { + return { + ...field, + name: 'Longer name that needs wrapping', + config: { + ...field.config, + custom: { + ...field.config?.custom, + filterable: true, + wrapHeaderText: true, + }, + }, + }; + } + return field; + }); renderHook(() => { return useHeaderHeight({ - fields: fields.map((field) => { - if (field.name === 'name') { - return { - ...field, - name: 'Longer name that needs wrapping', - config: { - ...field.config, - custom: { - ...field.config?.custom, - filterable: true, - wrapHeaderText: true, - }, - }, - }; - } - return field; - }), + fields: modifiedFields, columnWidths: [100, 100, 100], enabled: true, typographyCtx: { ...typographyCtx, wrappedCount: countFn }, @@ -545,7 +549,7 @@ describe('TableNG hooks', () => { }); }); - expect(countFn).toHaveBeenCalledWith('Longer name that needs wrapping', 26); + expect(countFn).toHaveBeenCalledWith('Longer name that needs wrapping', 26, modifiedFields[0], -1); }); }); @@ -764,7 +768,12 @@ describe('TableNG hooks', () => { expect(result.current(rows[0])).toEqual(expect.any(Number)); - expect(estimateLinesFn).toHaveBeenCalledWith('Annie Lennox', 100 - TABLE.CELL_PADDING * 2 - TABLE.BORDER_RIGHT); + expect(estimateLinesFn).toHaveBeenCalledWith( + 'Annie Lennox', + 100 - TABLE.CELL_PADDING * 2 - TABLE.BORDER_RIGHT, + fieldsWithWrappedText[0], + 0 + ); }); }); }); diff --git a/packages/grafana-ui/src/components/Table/TableNG/hooks.ts b/packages/grafana-ui/src/components/Table/TableNG/hooks.ts index 70a790d9133..5c349bdd1ab 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/hooks.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/hooks.ts @@ -3,7 +3,7 @@ import { Column, DataGridHandle, DataGridProps, SortColumn } from 'react-data-gr import { Field, fieldReducers, FieldType, formattedValueToString, reduceField } from '@grafana/data'; -import { TableColumnResizeActionCallback } from '../types'; +import { TableCellDisplayMode, TableColumnResizeActionCallback } from '../types'; import { TABLE } from './constants'; import { FilterType, TableFooterCalc, TableRow, TableSortByFieldState, TableSummaryRow, TypographyCtx } from './types'; @@ -15,6 +15,7 @@ import { getRowHeight, buildHeaderLineCounters, buildRowLineCounters, + getCellOptions, } from './utils'; // Helper function to get displayed value @@ -437,7 +438,15 @@ export function useRowHeight({ defaultHeight, lineCounters, TABLE.LINE_HEIGHT, - TABLE.CELL_PADDING * 2 + (field, numLines) => { + // Pill cells have vertical padding between each row + if (getCellOptions(field).type === TableCellDisplayMode.Pill) { + return TABLE.CELL_PADDING * (numLines - 1) + TABLE.CELL_PADDING * 2; + } + + // default vertical padding for cells + return TABLE.CELL_PADDING * 2; + } ); } return result; diff --git a/packages/grafana-ui/src/components/Table/TableNG/types.ts b/packages/grafana-ui/src/components/Table/TableNG/types.ts index 1d736974189..d28bab7b668 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/types.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/types.ts @@ -248,6 +248,12 @@ export interface ActionCellProps { getActions: GetActionsFunctionLocal; } +export interface PillCellProps { + theme: GrafanaTheme2; + field: Field; + rowIdx: number; +} + // Comparator for sorting table values export type Comparator = (a: TableCellValue, b: TableCellValue) => number; @@ -264,13 +270,14 @@ export interface ScrollPosition { export interface TypographyCtx { ctx: CanvasRenderingContext2D; - font: string; + fontFamily: string; + letterSpacing: number; avgCharWidth: number; estimateLines: LineCounter; wrappedCount: LineCounter; } -export type LineCounter = (value: unknown, width: number) => number; +export type LineCounter = (value: unknown, width: number, field: Field, rowIdx: number) => number; export interface LineCounterEntry { /** * given a values and the available width, returns the line count for that value 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 e86de2a7009..4dd20cac215 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts @@ -5,6 +5,7 @@ import { createTheme, DataFrame, DataFrameWithValue, + DataLink, DisplayValue, Field, FieldType, @@ -40,23 +41,17 @@ import { createTypographyContext, applySort, SINGLE_LINE_ESTIMATE_THRESHOLD, + wrapUwrapCount, + getDataLinksCounter, + getPillLineCounter, } from './utils'; describe('TableNG utils', () => { 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, - }, - }, - }) - ).toBe(align); + expect(getAlignment({ name: 'Value', type: FieldType.string, values: [], config: { custom: { align } } })).toBe( + align + ); }); it.each([ @@ -65,16 +60,7 @@ describe('TableNG utils', () => { { 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: {}, - }, - }) - ).toBe(align); + expect(getAlignment({ name: 'Test', type, values: [], config: { custom: {} } })).toBe(align); }); it.each([ @@ -91,17 +77,7 @@ describe('TableNG utils', () => { name: 'Test', type: FieldType.number, values: [], - config: { - custom: { - ...(cellType !== undefined - ? { - cellOptions: { - type: cellType, - }, - } - : {}), - }, - }, + config: { custom: { ...(cellType !== undefined ? { cellOptions: { type: cellType } } : {}) } }, }) ).toBe(align); }); @@ -122,34 +98,17 @@ describe('TableNG utils', () => { colors: { isDark: true, mode: 'dark', - primary: { - text: '#FFFFFF', - main: '#FF0000', - }, - background: { - canvas: '#000000', - primary: '#111111', - }, - text: { - primary: '#FFFFFF', - }, - action: { - hover: '#FF0000', - }, + primary: { text: '#FFFFFF', main: '#FF0000' }, + background: { canvas: '#000000', primary: '#111111' }, + text: { primary: '#FFFFFF' }, + action: { hover: '#FF0000' }, }, } as unknown as GrafanaTheme2; it('should handle color background mode', () => { - const field = { - type: TableCellDisplayMode.ColorBackground as const, - mode: TableCellBackgroundDisplayMode.Basic, - }; + const field = { type: TableCellDisplayMode.ColorBackground as const, mode: TableCellBackgroundDisplayMode.Basic }; - const displayValue = { - text: '100', - numeric: 100, - color: '#ff0000', - }; + const displayValue = { text: '100', numeric: 100, color: '#ff0000' }; const colors = getCellColors(theme, field, displayValue); expect(colors.bgColor).toBe('rgb(255, 0, 0)'); @@ -162,11 +121,7 @@ describe('TableNG utils', () => { mode: TableCellBackgroundDisplayMode.Gradient, }; - const displayValue = { - text: '100', - numeric: 100, - color: '#ff0000', - }; + const displayValue = { text: '100', numeric: 100, color: '#ff0000' }; const colors = getCellColors(theme, field, displayValue); expect(colors.bgColor).toBe('linear-gradient(120deg, rgb(255, 54, 36), #ff0000)'); @@ -185,12 +140,7 @@ describe('TableNG utils', () => { const records = frameToRecords(frame); expect(records).toHaveLength(2); - expect(records[0]).toEqual({ - __depth: 0, - __index: 0, - time: 1, - value: 10, - }); + expect(records[0]).toEqual({ __depth: 0, __index: 0, time: 1, value: 10 }); }); }); @@ -203,36 +153,22 @@ describe('TableNG utils', () => { config: {}, values: [1, 22, 333, 4444], // No state property initially - display: (value: unknown) => ({ - text: String(value), - numeric: Number(value), - }), + display: (value: unknown) => ({ text: String(value), numeric: Number(value) }), }; // Create a display value - const displayValue: DisplayValue = { - text: '1', - numeric: 1, - }; + const displayValue: DisplayValue = { text: '1', numeric: 1 }; // Call getAlignmentFactor with the first row const result = getAlignmentFactor(field, displayValue, 0); // Verify the result has the text property - expect(result).toEqual( - expect.objectContaining({ - text: '1', - }) - ); + expect(result).toEqual(expect.objectContaining({ text: '1' })); // Verify that field.state was created and contains the alignment factor expect(field.state).toBeDefined(); expect(field.state?.alignmentFactors).toBeDefined(); - expect(field.state?.alignmentFactors).toEqual( - expect.objectContaining({ - text: '1', - }) - ); + expect(field.state?.alignmentFactors).toEqual(expect.objectContaining({ text: '1' })); }); it('should update alignment factor when a longer value is found', () => { @@ -242,39 +178,21 @@ describe('TableNG utils', () => { type: FieldType.number, config: {}, values: [1, 22, 333, 4444], - state: { - alignmentFactors: { - text: '1', - }, - }, - display: (value: unknown) => ({ - text: String(value), - numeric: Number(value), - }), + state: { alignmentFactors: { text: '1' } }, + display: (value: unknown) => ({ text: String(value), numeric: Number(value) }), }; // Create a display value that is longer than the existing alignment factor - const displayValue: DisplayValue = { - text: '4444', - numeric: 4444, - }; + const displayValue: DisplayValue = { text: '4444', numeric: 4444 }; // Call getAlignmentFactor const result = getAlignmentFactor(field, displayValue, 3); // Verify the result is updated to the longer value - expect(result).toEqual( - expect.objectContaining({ - text: '4444', - }) - ); + expect(result).toEqual(expect.objectContaining({ text: '4444' })); // Verify that field.state.alignmentFactors was updated - expect(field.state?.alignmentFactors).toEqual( - expect.objectContaining({ - text: '4444', - }) - ); + expect(field.state?.alignmentFactors).toEqual(expect.objectContaining({ text: '4444' })); }); it('should not update alignment factor when a shorter value is found', () => { @@ -284,39 +202,21 @@ describe('TableNG utils', () => { type: FieldType.number, config: {}, values: [1, 22, 333, 4444], - state: { - alignmentFactors: { - text: '4444', - }, - }, - display: (value: unknown) => ({ - text: String(value), - numeric: Number(value), - }), + state: { alignmentFactors: { text: '4444' } }, + display: (value: unknown) => ({ text: String(value), numeric: Number(value) }), }; // Create a display value that is shorter than the existing alignment factor - const displayValue: DisplayValue = { - text: '1', - numeric: 1, - }; + const displayValue: DisplayValue = { text: '1', numeric: 1 }; // Call getAlignmentFactor const result = getAlignmentFactor(field, displayValue, 0); // Verify the result is still the longer value - expect(result).toEqual( - expect.objectContaining({ - text: '4444', - }) - ); + expect(result).toEqual(expect.objectContaining({ text: '4444' })); // Verify that field.state.alignmentFactors was not changed - expect(field.state?.alignmentFactors).toEqual( - expect.objectContaining({ - text: '4444', - }) - ); + expect(field.state?.alignmentFactors).toEqual(expect.objectContaining({ text: '4444' })); }); it('should add alignment factor to existing field state', () => { @@ -334,38 +234,24 @@ describe('TableNG utils', () => { // Or if noValue is a valid property: // noValue: true }, - display: (value: unknown) => ({ - text: String(value), - numeric: Number(value), - }), + display: (value: unknown) => ({ text: String(value), numeric: Number(value) }), }; // Create a display value - const displayValue: DisplayValue = { - text: '1', - numeric: 1, - }; + const displayValue: DisplayValue = { text: '1', numeric: 1 }; // Call getAlignmentFactor with the first row const result = getAlignmentFactor(field, displayValue, 0); // Verify the result has the text property - expect(result).toEqual( - expect.objectContaining({ - text: '1', - }) - ); + expect(result).toEqual(expect.objectContaining({ text: '1' })); // Verify that field.state was preserved and alignment factor was added expect(field.state).toBeDefined(); // Check for the valid property we used expect(field.state?.calcs).toBeDefined(); expect(field.state?.alignmentFactors).toBeDefined(); - expect(field.state?.alignmentFactors).toEqual( - expect.objectContaining({ - text: '1', - }) - ); + expect(field.state?.alignmentFactors).toEqual(expect.objectContaining({ text: '1' })); }); it.todo('alignmentFactor.text = displayValue.text;'); @@ -398,11 +284,7 @@ describe('TableNG utils', () => { ]; const result = getColumnTypes(fields); - expect(result).toEqual({ - name: FieldType.string, - age: FieldType.number, - active: FieldType.boolean, - }); + expect(result).toEqual({ name: FieldType.string, age: FieldType.number, active: FieldType.boolean }); }); it('should recursively build column types when nested fields are present', () => { @@ -448,20 +330,13 @@ describe('TableNG utils', () => { const frame: DataFrame = { fields: [ { type: FieldType.string, name: 'stringCol', config: {}, values: [] }, - { - type: FieldType.nestedFrames, - name: 'nestedCol', - config: {}, - values: [], - }, + { type: FieldType.nestedFrames, name: 'nestedCol', config: {}, values: [] }, ], length: 0, name: 'test', }; - expect(getColumnTypes(frame.fields)).toEqual({ - stringCol: FieldType.string, - }); + expect(getColumnTypes(frame.fields)).toEqual({ stringCol: FieldType.string }); }); }); @@ -557,18 +432,12 @@ describe('TableNG utils', () => { describe('migrateTableDisplayModeToCellOptions', () => { it('should migrate basic to gauge mode', () => { const result = migrateTableDisplayModeToCellOptions(TableCellDisplayMode.BasicGauge); - expect(result).toEqual({ - type: TableCellDisplayMode.Gauge, - mode: BarGaugeDisplayMode.Basic, - }); + expect(result).toEqual({ type: TableCellDisplayMode.Gauge, mode: BarGaugeDisplayMode.Basic }); }); it('should migrate gradient-gauge to gauge mode with gradient', () => { const result = migrateTableDisplayModeToCellOptions(TableCellDisplayMode.GradientGauge); - expect(result).toEqual({ - type: TableCellDisplayMode.Gauge, - mode: BarGaugeDisplayMode.Gradient, - }); + expect(result).toEqual({ type: TableCellDisplayMode.Gauge, mode: BarGaugeDisplayMode.Gradient }); }); it('should migrate color-background to color background with gradient', () => { @@ -581,20 +450,13 @@ describe('TableNG utils', () => { it('should handle other display modes', () => { const result = migrateTableDisplayModeToCellOptions(TableCellDisplayMode.ColorText); - expect(result).toEqual({ - type: TableCellDisplayMode.ColorText, - }); + expect(result).toEqual({ type: TableCellDisplayMode.ColorText }); }); }); describe('getCellOptions', () => { it('should return default options when no custom config is provided', () => { - const field: Field = { - name: 'test', - type: FieldType.string, - config: {}, - values: [], - }; + const field: Field = { name: 'test', type: FieldType.string, config: {}, values: [] }; const options = getCellOptions(field); @@ -607,35 +469,21 @@ describe('TableNG utils', () => { name: 'test', type: FieldType.string, config: { - custom: { - cellOptions: { - type: TableCellDisplayMode.ColorText, - inspectEnabled: false, - wrapText: true, - }, - }, + custom: { cellOptions: { type: TableCellDisplayMode.ColorText, inspectEnabled: false, wrapText: true } }, }, values: [], }; const options = getCellOptions(field); - expect(options).toEqual({ - type: TableCellDisplayMode.ColorText, - inspectEnabled: false, - wrapText: true, - }); + expect(options).toEqual({ type: TableCellDisplayMode.ColorText, inspectEnabled: false, wrapText: true }); }); it('should handle legacy displayMode property', () => { const field: Field = { name: 'test', type: FieldType.string, - config: { - custom: { - displayMode: 'color-background', - }, - }, + config: { custom: { displayMode: 'color-background' } }, values: [], }; @@ -649,14 +497,7 @@ describe('TableNG utils', () => { const field: Field = { name: 'test', type: FieldType.string, - config: { - custom: { - displayMode: 'color-background', - cellOptions: { - type: TableCellDisplayMode.ColorText, - }, - }, - }, + config: { custom: { displayMode: 'color-background', cellOptions: { type: TableCellDisplayMode.ColorText } } }, values: [], }; @@ -689,13 +530,7 @@ describe('TableNG utils', () => { const field: Field = { name: 'test', type: FieldType.string, - config: { - custom: { - cellOptions: { - type: TableCellDisplayMode.JSONView, - }, - }, - }, + config: { custom: { cellOptions: { type: TableCellDisplayMode.JSONView } } }, values: [], }; @@ -707,12 +542,7 @@ describe('TableNG utils', () => { describe('getCellLinks', () => { it('should return undefined when field has no getLinks function', () => { - const field: Field = { - name: 'test', - type: FieldType.string, - config: {}, - values: ['value'], - }; + const field: Field = { name: 'test', type: FieldType.string, config: {}, values: ['value'] }; const links = getCellLinks(field, 0); expect(links).toEqual(undefined); @@ -987,36 +817,112 @@ describe('TableNG utils', () => { // actually executed the JS correctly. If you called `count` with a sensible value and width, // it wouldn't give you a very reasonable answer in Jest's DOM environment for some reason. it('creates the context using uwrap', () => { + const field: Field = { name: 'test', type: FieldType.string, config: {}, values: ['foo', 'bar', 'baz'] }; const ctx = createTypographyContext(14, 'sans-serif', 0.15); + expect(ctx).toEqual( expect.objectContaining({ - font: '14px sans-serif', ctx: expect.any(CanvasRenderingContext2D), + fontFamily: 'sans-serif', + letterSpacing: 0.15, wrappedCount: expect.any(Function), estimateLines: expect.any(Function), avgCharWidth: expect.any(Number), }) ); - expect(ctx.wrappedCount('the quick brown fox jumps over the lazy dog', 100)).toEqual(expect.any(Number)); - expect(ctx.estimateLines('the quick brown fox jumps over the lazy dog', 100)).toEqual(expect.any(Number)); + expect(ctx.wrappedCount('the quick brown fox jumps over the lazy dog', 100, field, 0)).toEqual( + expect.any(Number) + ); + expect(ctx.estimateLines('the quick brown fox jumps over the lazy dog', 100, field, 0)).toEqual( + expect.any(Number) + ); + }); + }); + + describe('wrapUwrapCount', () => { + const field: Field = { name: 'test', type: FieldType.string, config: {}, values: ['foo', 'bar', 'baz'] }; + + it('wraps the uwrap count function', () => { + const wrappedCount = wrapUwrapCount(jest.fn(() => 2)); + expect(wrappedCount('test string', 100, field, 0)).toBe(2); + }); + + it('returns 1 for null or undefined values', () => { + const wrappedCount = wrapUwrapCount(jest.fn(() => 2)); + expect(wrappedCount(null, 100, field, 0)).toBe(1); + expect(wrappedCount(undefined, 100, field, 0)).toBe(1); }); }); describe('getTextLineEstimator', () => { const counter = getTextLineEstimator(10); + const field: Field = { name: 'test', type: FieldType.string, config: {}, values: ['foo', 'bar', 'baz'] }; it('returns -1 if there are no strings or dashes within the string', () => { - expect(counter('asdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdf', 5)).toBe(-1); + expect(counter('asdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdf', 5, field, 0)).toBe(-1); }); it('calculates an approximate rendered height for the text based on the width and avgCharWidth', () => { - expect(counter('asdfas dfasdfasdf asdfasdfasdfa sdfasdfasdfasdf 23', 200)).toBe(2.5); + expect(counter('asdfas dfasdfasdf asdfasdfasdfa sdfasdfasdfasdf 23', 200, field, 0)).toBe(2.5); + }); + }); + + describe('getDataLinksCounter', () => { + it('counts number of valid links using getCellLinks', () => { + const field: Field = { + name: 'test', + type: FieldType.string, + config: { + links: [ + { title: 'Link 1', url: 'http://example.com/1' }, + { title: 'Invalid Link' } as DataLink, // No href or onClick + { + title: 'Link w', + url: 'asdf', + onClick: jest.fn(() => {}), + }, + ], + }, + values: ['value1'], + }; + + const counter = getDataLinksCounter(); + expect(counter('my value', 100, field, 0)).toBe(2); + }); + }); + + describe('getPillLineCounter', () => { + it('counts up the number of lines using the pill measuring method', () => { + const counter = getPillLineCounter(jest.fn((str) => str.length * 5)); + expect(counter('tag1,tag2', 100, {} as Field, 0)).toBe(1); + expect(counter('tag1,tag2,tag3,tag4,tag5,tag6', 100, {} as Field, 0)).toBe(3); + }); + + it('returns 0 if value is null', () => { + const counter = getPillLineCounter(jest.fn((str) => str.length * 5)); + expect(counter(null, 100, {} as Field, 0)).toBe(0); + }); + + it('returns 0 if no pills are inferred', () => { + const counter = getPillLineCounter(jest.fn((str) => str.length * 5)); + expect(counter('', 100, {} as Field, 0)).toBe(0); + }); + + it('caches the width measurement for the same value', () => { + const widthMeasurement = jest.fn((str) => str.length * 5); + const counter = getPillLineCounter(widthMeasurement); + counter('tag1,tag2,tag3,tag4,tag5,tag6', 100, {} as Field, 0); + counter('tag1,tag2', 100, {} as Field, 0); + counter('tag2', 200, {} as Field, 0); + counter('tag2,tag3,tag2,tag4,tag4,tag2,tag5', 300, {} as Field, 0); + expect(widthMeasurement).toHaveBeenCalledTimes(6); // Should only call for unique values }); }); describe('buildHeaderLineCounters', () => { const ctx = { - font: '14px sans-serif', + fontFamily: 'sans-serif', + letterSpacing: 0.15, ctx: {} as CanvasRenderingContext2D, count: jest.fn(() => 2), avgCharWidth: 7, @@ -1057,15 +963,15 @@ describe('TableNG utils', () => { describe('buildRowLineCounters', () => { const ctx = { - font: '14px sans-serif', + fontFamily: 'sans-serif', + letterSpacing: 0.15, ctx: {} as CanvasRenderingContext2D, - count: jest.fn(() => 2), wrappedCount: jest.fn(() => 2), estimateLines: jest.fn(() => 2), avgCharWidth: 7, }; - it('returns an array of line counters for each column', () => { + it('sets up text line counters for each text column if wrapping is on', () => { const fields: Field[] = [ { name: 'Name', type: FieldType.string, values: [], config: { custom: { cellOptions: { wrapText: true } } } }, { @@ -1095,6 +1001,42 @@ describe('TableNG utils', () => { expect(counters![0].fieldIdxs).toEqual([1]); }); + it('sets up line counting for pills if present and wrapping is on', () => { + const fields: Field[] = [ + { + name: 'Tags', + type: FieldType.string, + values: ['tag1,tag2', 'tag3', '["tag4","tag5","tag6"]'], + config: { custom: { cellOptions: { type: TableCellDisplayMode.Pill, wrapText: true } } }, + }, + ]; + const counters = buildRowLineCounters(fields, ctx); + expect(counters![0].estimate).toEqual(expect.any(Function)); + expect(counters![0].estimate!('tag1,tag2', 100, fields[0], 0)).toEqual(expect.any(Number)); + expect(counters![0].counter).toEqual(expect.any(Function)); + expect(counters![0].counter('tag1,tag2', 100, fields[0], 0)).toEqual(expect.any(Number)); + expect(counters![0].fieldIdxs).toEqual([0]); + }); + + it('sets up line counting for datalinks if present and wrapping is on', () => { + const fields: Field[] = [ + { + name: 'Links', + type: FieldType.string, + values: ['http://example.com/1', 'http://example.com/2'], + config: { custom: { cellOptions: { type: TableCellDisplayMode.DataLinks, wrapText: true } } }, + getLinks: jest.fn((): LinkModel[] => [ + { title: 'Link 1', href: 'http://example.com/1', target: '_blank', origin: { datasourceUid: 'test' } }, + { title: 'Link 2', href: 'http://example.com/2', target: '_self', origin: { datasourceUid: 'test' } }, + ]), + }, + ]; + const counters = buildRowLineCounters(fields, ctx); + expect(counters![0].counter).toEqual(expect.any(Function)); + expect(counters![0].counter('http://example.com/1', 100, fields[0], 0)).toEqual(expect.any(Number)); + expect(counters![0].fieldIdxs).toEqual([0]); + }); + it('does not enable text counting for non-string fields', () => { const fields: Field[] = [ { name: 'Name', type: FieldType.string, values: [], config: { custom: {} } }, @@ -1162,15 +1104,15 @@ describe('TableNG utils', () => { it('should take colWidths into account when calculating max wrap cell', () => { getRowHeight(fields, 3, [50, 60], 36, counters, 20, 10); - expect(counters[0].counter).toHaveBeenCalledWith('longer one here', 50); - expect(counters[1].counter).toHaveBeenCalledWith(123456, 60); + expect(counters[0].counter).toHaveBeenCalledWith('longer one here', 50, fields[0], 3); + expect(counters[1].counter).toHaveBeenCalledWith(123456, 60, fields[1], 3); }); // this is used to calc wrapped header height it('should use the display name if the rowIdx is -1', () => { getRowHeight(fields, -1, [50, 60], 36, counters, 20, 10); - expect(counters[0].counter).toHaveBeenCalledWith('Name', 50); - expect(counters[1].counter).toHaveBeenCalledWith('Age', 60); + expect(counters[0].counter).toHaveBeenCalledWith('Name', 50, fields[0], -1); + expect(counters[1].counter).toHaveBeenCalledWith('Age', 60, fields[1], -1); }); it('should ignore columns which do not have line counters', () => { @@ -1230,18 +1172,8 @@ describe('TableNG utils', () => { expect( computeColWidths( [ - { - name: 'A', - type: FieldType.string, - values: [], - config: { custom: { width: 100 } }, - }, - { - name: 'B', - type: FieldType.string, - values: [], - config: { custom: { width: 200 } }, - }, + { name: 'A', type: FieldType.string, values: [], config: { custom: { width: 100 } } }, + { name: 'B', type: FieldType.string, values: [], config: { custom: { width: 200 } } }, ], 500 ) @@ -1252,18 +1184,8 @@ describe('TableNG utils', () => { expect( computeColWidths( [ - { - name: 'A', - type: FieldType.string, - values: [], - config: {}, - }, - { - name: 'B', - type: FieldType.string, - values: [], - config: { custom: { width: 200 } }, - }, + { name: 'A', type: FieldType.string, values: [], config: {} }, + { name: 'B', type: FieldType.string, values: [], config: { custom: { width: 200 } } }, ], 500 ) @@ -1274,18 +1196,8 @@ describe('TableNG utils', () => { expect( computeColWidths( [ - { - name: 'A', - type: FieldType.string, - values: [], - config: { custom: { minWidth: 100 } }, - }, - { - name: 'B', - type: FieldType.string, - values: [], - config: { custom: { minWidth: 100 } }, - }, + { name: 'A', type: FieldType.string, values: [], config: { custom: { minWidth: 100 } } }, + { name: 'B', type: FieldType.string, values: [], config: { custom: { minWidth: 100 } } }, ], 100 ) @@ -1296,18 +1208,8 @@ describe('TableNG utils', () => { expect( computeColWidths( [ - { - name: 'A', - type: FieldType.string, - values: [], - config: {}, - }, - { - name: 'B', - type: FieldType.string, - values: [], - config: {}, - }, + { name: 'A', type: FieldType.string, values: [], config: {} }, + { name: 'B', type: FieldType.string, values: [], config: {} }, ], // we have two columns but have set the table to the width of one default column. COLUMN.DEFAULT_WIDTH @@ -1332,28 +1234,14 @@ describe('TableNG utils', () => { ], }); - const sortColumns: SortColumn[] = [ - { - columnKey: 'time', - direction: 'ASC', - }, - ]; + const sortColumns: SortColumn[] = [{ columnKey: 'time', direction: 'ASC' }]; const records = applySort(frameToRecords(frame), frame.fields, sortColumns); expect(records).toMatchObject([ - { - time: 1, - value: 20, - }, - { - time: 1, - value: 10, - }, - { - time: 2, - value: 30, - }, + { time: 1, value: 20 }, + { time: 1, value: 10 }, + { time: 2, value: 30 }, ]); }); }); diff --git a/packages/grafana-ui/src/components/Table/TableNG/utils.ts b/packages/grafana-ui/src/components/Table/TableNG/utils.ts index a24a361bfda..96e6c71c018 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/utils.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/utils.ts @@ -25,6 +25,7 @@ import { import { getTextColorForAlphaBackground } from '../../../utils/colors'; import { TableCellOptions } from '../types'; +import { inferPills } from './Cells/PillCell'; import { COLUMN, TABLE } from './constants'; import { CellColors, @@ -92,15 +93,19 @@ export function createTypographyContext(fontSize: number, fontFamily: string, le ctx.letterSpacing = `${letterSpacing}px`; ctx.font = font; + // 1/6 of the characters in this string are capitalized. Since the avgCharWidth is used for estimation, it's + // better that the estimation over-estimates the width than if it underestimates it, so we're a little on the + // aggressive side here and could even go more aggressive if we get complaints in the future. const txt = - "Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s."; + "Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s. 1234567890 ALL CAPS TO HELP WITH MEASUREMENT."; const txtWidth = ctx.measureText(txt).width; const avgCharWidth = txtWidth / txt.length + letterSpacing; const { count } = varPreLine(ctx); return { ctx, - font, + fontFamily, + letterSpacing, avgCharWidth, estimateLines: getTextLineEstimator(avgCharWidth), wrappedCount: wrapUwrapCount(count), @@ -108,7 +113,7 @@ export function createTypographyContext(fontSize: number, fontFamily: string, le } /** - * @internal + * @internal wraps the uwrap count function to ensure that it is given a string. */ export function wrapUwrapCount(count: Count): LineCounter { return (value, width) => { @@ -141,6 +146,71 @@ export function getTextLineEstimator(avgCharWidth: number): LineCounter { }; } +/** + * @internal + */ +export function getDataLinksCounter(): LineCounter { + const linksCountCache: Record = {}; + + // when we render links, we need to filter out the invalid links. since the call to `getLinks` is expensive, + // we'll cache the result and reuse it for every row in the table. this cache is cleared when line counts are + // rebuilt anytime from the `useRowHeight` hook, and that includes adding and removing data links. + return (_value, _width, field) => { + const cacheKey = getDisplayName(field); + if (linksCountCache[cacheKey] === undefined) { + let count = 0; + for (const l of field.config?.links ?? []) { + if (l.onClick || l.url) { + count += 1; + } + } + linksCountCache[cacheKey] = count; + } + + return linksCountCache[cacheKey]; + }; +} + +const PILLS_FONT_SIZE = 12; +const PILLS_SPACING = 12; // 6px horizontal padding on each side +const PILLS_GAP = 4; // gap between pills + +export function getPillLineCounter(measureWidth: (value: string) => number): LineCounter { + const widthCache: Record = {}; + + return (value, width) => { + if (value == null) { + return 0; + } + + const pillValues = inferPills(String(value)); + if (pillValues.length === 0) { + return 0; + } + + let lines = 0; + let currentLineUse = width; + + for (const pillValue of pillValues) { + let rawWidth = widthCache[pillValue]; + if (rawWidth === undefined) { + rawWidth = measureWidth(pillValue); + widthCache[pillValue] = rawWidth; + } + const pillWidth = rawWidth + PILLS_SPACING; + + if (currentLineUse + pillWidth + PILLS_GAP > width) { + lines++; + currentLineUse = pillWidth; + } else { + currentLineUse += pillWidth + PILLS_GAP; + } + } + + return lines; + }; +} + /** * @internal return a text line counter for every field which has wrapHeaderText enabled. */ @@ -175,12 +245,33 @@ export function buildRowLineCounters(fields: Field[], typographyCtx: TypographyC const field = fields[fieldIdx]; if (shouldTextWrap(field)) { wrappedFields++; - // TODO: Pills, DataLinks, and JSON will have custom line counters here. - // for string fields, we really want to find the longest field ahead of time to reduce the number of calls to `count`. - // calling `count` is going to get a perfectly accurate line count, but it is expensive, so we'd rather estimate the line - // count and call the counter only for the field which will take up the most space based on its - if (field.type === FieldType.string) { + const cellType = getCellOptions(field).type; + if (cellType === TableCellDisplayMode.DataLinks) { + result.dataLinksCounter = result.dataLinksCounter ?? { + counter: getDataLinksCounter(), + fieldIdxs: [], + }; + result.dataLinksCounter.fieldIdxs.push(fieldIdx); + } else if (cellType === TableCellDisplayMode.Pill) { + if (!result.pillCounter) { + const pillTypographyCtx = createTypographyContext( + PILLS_FONT_SIZE, + typographyCtx.fontFamily, + typographyCtx.letterSpacing + ); + + result.pillCounter = { + estimate: getPillLineCounter((value) => value.length * pillTypographyCtx.avgCharWidth), + counter: getPillLineCounter((value) => pillTypographyCtx.ctx.measureText(value).width), + fieldIdxs: [], + }; + } + result.pillCounter.fieldIdxs.push(fieldIdx); + } + + // for string fields, we estimate the length of a line using `avgCharWidth` to limit expensive calls `count`. + else if (field.type === FieldType.string) { result.textCounter = result.textCounter ?? { counter: typographyCtx.wrappedCount, estimate: typographyCtx.estimateLines, @@ -215,7 +306,9 @@ export function getRowHeight( defaultHeight: number, lineCounters?: LineCounterEntry[], lineHeight = TABLE.LINE_HEIGHT, - verticalPadding = 0 + // when this is a function, the field which was measured as the maximum size will be returned, as well as the + // calculated number of lines, so that the consumer can use it in case the vertical padding value differs field-by-field. + verticalPadding: number | ((field: Field, numLines: number) => number) = TABLE.CELL_PADDING ): number { if (!lineCounters?.length) { return defaultHeight; @@ -224,6 +317,7 @@ export function getRowHeight( let maxLines = -1; let maxValue = ''; let maxWidth = 0; + let maxField: Field | undefined; let preciseCounter: LineCounter | undefined; for (const { estimate, counter, fieldIdxs } of lineCounters) { @@ -239,11 +333,12 @@ export function getRowHeight( const cellValueRaw = rowIdx === -1 ? getDisplayName(field) : field.values[rowIdx]; if (cellValueRaw != null) { const colWidth = columnWidths[fieldIdx]; - const approxLines = count(cellValueRaw, colWidth); + const approxLines = count(cellValueRaw, colWidth, field, rowIdx); if (approxLines > maxLines) { maxLines = approxLines; maxValue = cellValueRaw; maxWidth = colWidth; + maxField = field; preciseCounter = isEstimating ? counter : undefined; } } @@ -252,18 +347,23 @@ export function getRowHeight( // if the value is -1 or the estimate for the max cell was less than the SINGLE_LINE_ESTIMATE_THRESHOLD, we trust // that the estimator correctly identified that no text wrapping is needed for this row, skipping the preciseCounter. - if (maxLines < SINGLE_LINE_ESTIMATE_THRESHOLD) { + if (maxField === undefined || maxLines < SINGLE_LINE_ESTIMATE_THRESHOLD) { return defaultHeight; } // if we finished this row height loop with an estimate, we need to call // the `preciseCounter` method to get the exact line count. if (preciseCounter !== undefined) { - maxLines = preciseCounter(maxValue, maxWidth); + maxLines = preciseCounter(maxValue, maxWidth, maxField, rowIdx); } - // we want a round number of lines for rendering - const totalHeight = Math.ceil(maxLines) * lineHeight + verticalPadding; + // round up to the nearest line before doing math + maxLines = Math.ceil(maxLines); + + // adjust for vertical padding and line height, and clamp to a minimum default height + const verticalPaddingValue = + typeof verticalPadding === 'function' ? verticalPadding(maxField, maxLines) : verticalPadding; + const totalHeight = maxLines * lineHeight + verticalPaddingValue; return Math.max(totalHeight, defaultHeight); } @@ -276,9 +376,7 @@ export function shouldTextOverflow(field: Field): boolean { const eligibleCellType = // Tech debt: Technically image cells are of type string, which is misleading (kinda?) // 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) || + (field.type === FieldType.string && cellOptions.type !== TableCellDisplayMode.Image) || // regardless of the underlying cell type, data links cells have text overflow. cellOptions.type === TableCellDisplayMode.DataLinks; diff --git a/public/app/plugins/panel/table/cells/AutoCellOptionsEditor.tsx b/public/app/plugins/panel/table/cells/AutoCellOptionsEditor.tsx index 7b108fbc155..e97d481e0b3 100644 --- a/public/app/plugins/panel/table/cells/AutoCellOptionsEditor.tsx +++ b/public/app/plugins/panel/table/cells/AutoCellOptionsEditor.tsx @@ -8,8 +8,6 @@ export const AutoCellOptionsEditor = ({ cellOptions, onChange, }: TableCellEditorProps) => { - // Handle row coloring changes - const onWrapTextChange = () => { cellOptions.wrapText = !cellOptions.wrapText; onChange(cellOptions); diff --git a/public/app/plugins/panel/table/cells/ColorBackgroundCellOptionsEditor.tsx b/public/app/plugins/panel/table/cells/ColorBackgroundCellOptionsEditor.tsx index afd6353a43e..33c3761493d 100644 --- a/public/app/plugins/panel/table/cells/ColorBackgroundCellOptionsEditor.tsx +++ b/public/app/plugins/panel/table/cells/ColorBackgroundCellOptionsEditor.tsx @@ -21,13 +21,11 @@ export const ColorBackgroundCellOptionsEditor = ({ onChange(cellOptions); }; - // Handle row coloring changes const onColorRowChange = () => { cellOptions.applyToRow = !cellOptions.applyToRow; onChange(cellOptions); }; - // Handle row coloring changes const onWrapTextChange = () => { cellOptions.wrapText = !cellOptions.wrapText; onChange(cellOptions); diff --git a/public/app/plugins/panel/table/table-new/TableCellOptionEditor.tsx b/public/app/plugins/panel/table/table-new/TableCellOptionEditor.tsx index 77e2acf0808..92cbbf0f264 100644 --- a/public/app/plugins/panel/table/table-new/TableCellOptionEditor.tsx +++ b/public/app/plugins/panel/table/table-new/TableCellOptionEditor.tsx @@ -4,14 +4,14 @@ import { useState } from 'react'; import { GrafanaTheme2 } from '@grafana/data'; import { t } from '@grafana/i18n'; -import { TableCellOptions } from '@grafana/schema'; +import { TableCellOptions, TableWrapTextOptions } from '@grafana/schema'; import { Combobox, ComboboxOption, Field, TableCellDisplayMode, useStyles2 } from '@grafana/ui'; -import { AutoCellOptionsEditor } from './cells/AutoCellOptionsEditor'; import { BarGaugeCellOptionsEditor } from './cells/BarGaugeCellOptionsEditor'; import { ColorBackgroundCellOptionsEditor } from './cells/ColorBackgroundCellOptionsEditor'; import { ImageCellOptionsEditor } from './cells/ImageCellOptionsEditor'; import { SparklineCellOptionsEditor } from './cells/SparklineCellOptionsEditor'; +import { TextWrapOptionsEditor } from './cells/TextWrapOptionsEditor'; // The props that any cell type editor are expected // to handle. In this case the generic type should @@ -26,6 +26,19 @@ interface Props { onChange: (v: TableCellOptions) => void; } +const TEXT_WRAP_CELL_TYPES = new Set([ + TableCellDisplayMode.Auto, + TableCellDisplayMode.Sparkline, + TableCellDisplayMode.ColorText, + TableCellDisplayMode.ColorBackground, + TableCellDisplayMode.DataLinks, + TableCellDisplayMode.Pill, +]); + +function isTextWrapCellType(value: TableCellOptions): value is TableCellOptions & TableWrapTextOptions { + return TEXT_WRAP_CELL_TYPES.has(value.type); +} + export const TableCellOptionEditor = ({ value, onChange }: Props) => { const cellType = value.type; const styles = useStyles2(getStyles); @@ -79,9 +92,7 @@ export const TableCellOptionEditor = ({ value, onChange }: Props) => { - {(cellType === TableCellDisplayMode.Auto || cellType === TableCellDisplayMode.ColorText) && ( - - )} + {isTextWrapCellType(value) && } {cellType === TableCellDisplayMode.Gauge && ( )} diff --git a/public/app/plugins/panel/table/table-new/cells/AutoCellOptionsEditor.tsx b/public/app/plugins/panel/table/table-new/cells/AutoCellOptionsEditor.tsx deleted file mode 100644 index 5aae9d65b7a..00000000000 --- a/public/app/plugins/panel/table/table-new/cells/AutoCellOptionsEditor.tsx +++ /dev/null @@ -1,27 +0,0 @@ -import { selectors } from '@grafana/e2e-selectors'; -import { t } from '@grafana/i18n'; -import { TableAutoCellOptions, TableColoredBackgroundCellOptions, TableColorTextCellOptions } from '@grafana/schema'; -import { Field, Switch } from '@grafana/ui'; - -import { TableCellEditorProps } from '../TableCellOptionEditor'; - -export const AutoCellOptionsEditor = ({ - cellOptions, - onChange, -}: TableCellEditorProps) => { - // Handle row coloring changes - const onWrapTextChange = () => { - cellOptions.wrapText = !cellOptions.wrapText; - onChange(cellOptions); - }; - - return ( - - - - ); -}; diff --git a/public/app/plugins/panel/table/table-new/cells/ColorBackgroundCellOptionsEditor.tsx b/public/app/plugins/panel/table/table-new/cells/ColorBackgroundCellOptionsEditor.tsx index b7d5d2ab3dc..33d85539ec4 100644 --- a/public/app/plugins/panel/table/table-new/cells/ColorBackgroundCellOptionsEditor.tsx +++ b/public/app/plugins/panel/table/table-new/cells/ColorBackgroundCellOptionsEditor.tsx @@ -6,7 +6,7 @@ import { Field, RadioButtonGroup, Switch } from '@grafana/ui'; import { TableCellEditorProps } from '../TableCellOptionEditor'; -import { AutoCellOptionsEditor } from './AutoCellOptionsEditor'; +import { TextWrapOptionsEditor } from './TextWrapOptionsEditor'; const colorBackgroundOpts: Array> = [ { value: TableCellBackgroundDisplayMode.Basic, label: 'Basic' }, @@ -21,7 +21,6 @@ export const ColorBackgroundCellOptionsEditor = ({ cellOptions.mode = v; onChange(cellOptions); }; - // Handle row coloring changes const onColorRowChange = () => { cellOptions.applyToRow = !cellOptions.applyToRow; onChange(cellOptions); @@ -54,7 +53,7 @@ export const ColorBackgroundCellOptionsEditor = ({ /> - { cellOptions.wrapText = updatedCellOptions.wrapText; diff --git a/public/app/plugins/panel/table/table-new/cells/TextWrapOptionsEditor.tsx b/public/app/plugins/panel/table/table-new/cells/TextWrapOptionsEditor.tsx new file mode 100644 index 00000000000..74ddaa092ea --- /dev/null +++ b/public/app/plugins/panel/table/table-new/cells/TextWrapOptionsEditor.tsx @@ -0,0 +1,29 @@ +import { selectors } from '@grafana/e2e-selectors'; +import { t } from '@grafana/i18n'; +import { TableCellOptions, TableWrapTextOptions } from '@grafana/schema'; +import { Field, Switch } from '@grafana/ui'; + +import { TableCellEditorProps } from '../TableCellOptionEditor'; + +export const TextWrapOptionsEditor = ({ + cellOptions, + onChange, +}: TableCellEditorProps) => { + // Handle row coloring changes + const onWrapTextChange = () => { + cellOptions.wrapText = !cellOptions.wrapText; + onChange(cellOptions); + }; + + return ( + <> + + + + + ); +}; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 339cb81d0d1..927b0c58839 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -12582,7 +12582,10 @@ "name-show-table-header": "Show table header", "name-wrap-header-text": "Wrap header text", "placeholder-column-width": "auto", - "placeholder-fields": "All Numeric Fields" + "placeholder-fields": "All Numeric Fields", + "text-wrap-options": { + "label-wrap-text": "Wrap text" + } }, "table-new": { "category-cell-options": "Cell options",