From 329d6a11faf985e18e51d4c2851dd3abc20c21f7 Mon Sep 17 00:00:00 2001 From: Paul Marbach Date: Tue, 28 Oct 2025 17:50:46 -0400 Subject: [PATCH] Table: Fix cell inspect for Sparkline and inferred JSON cells (#113059) * Table: Sparkline Cell inspect support * update to better support FieldType.other structures * clean up styling a bit for empty case * fix test import * add test for no x case for sparkline * fix merge mistake * fix test import --- eslint-suppressions.json | 5 - .../components/Table/TableCellInspector.tsx | 41 ++--- .../Table/TableNG/Cells/SparklineCell.tsx | 38 +---- .../src/components/Table/TableNG/TableNG.tsx | 1 - .../TableNG/__snapshots__/utils.test.ts.snap | 109 +++++++++++++ .../TableNG/components/TableCellActions.tsx | 38 +---- .../src/components/Table/TableNG/types.ts | 2 +- .../components/Table/TableNG/utils.test.ts | 151 ++++++++++++++++++ .../src/components/Table/TableNG/utils.ts | 76 ++++++++- 9 files changed, 359 insertions(+), 102 deletions(-) create mode 100644 packages/grafana-ui/src/components/Table/TableNG/__snapshots__/utils.test.ts.snap diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 708c8f9fbcc..fdcf848a354 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -927,11 +927,6 @@ "count": 1 } }, - "packages/grafana-ui/src/components/Table/TableCellInspector.tsx": { - "@typescript-eslint/no-explicit-any": { - "count": 1 - } - }, "packages/grafana-ui/src/components/Table/TableNG/TableNG.test.tsx": { "@typescript-eslint/no-explicit-any": { "count": 2 diff --git a/packages/grafana-ui/src/components/Table/TableCellInspector.tsx b/packages/grafana-ui/src/components/Table/TableCellInspector.tsx index ed7f9111773..10169328ba4 100644 --- a/packages/grafana-ui/src/components/Table/TableCellInspector.tsx +++ b/packages/grafana-ui/src/components/Table/TableCellInspector.tsx @@ -1,8 +1,10 @@ -import { isString } from 'lodash'; +import { css } from '@emotion/css'; import { useState } from 'react'; +import { GrafanaTheme2 } from '@grafana/data'; import { t, Trans } from '@grafana/i18n'; +import { useStyles2 } from '../../themes/ThemeContext'; import { ClipboardButton } from '../ClipboardButton/ClipboardButton'; import { Drawer } from '../Drawer/Drawer'; import { Stack } from '../Layout/Stack/Stack'; @@ -17,34 +19,15 @@ export enum TableCellInspectorMode { interface TableCellInspectorProps { // eslint-disable-next-line @typescript-eslint/no-explicit-any - value: any; + value: string; onDismiss: () => void; mode: TableCellInspectorMode; } export function TableCellInspector({ value, onDismiss, mode }: TableCellInspectorProps) { - let displayValue = value; const [currentMode, setMode] = useState(mode); - - if (isString(value)) { - const trimmedValue = value.trim(); - // Exclude numeric strings like '123' from being displayed in code/JSON mode - if (trimmedValue[0] === '{' || trimmedValue[0] === '[' || mode === 'code') { - try { - value = JSON.parse(value); - displayValue = JSON.stringify(value, null, ' '); - } catch (error: any) { - // Display helpful error to help folks diagnose json errors - console.log( - 'Failed to parse JSON in Table cell inspector (this will cause JSON to not print nicely): ', - error.message - ); - } - } - } else { - displayValue = JSON.stringify(value); - } - let text = displayValue; + const text = value.trim(); + const styles = useStyles2(getStyles); const tabs = [ { @@ -81,15 +64,23 @@ export function TableCellInspector({ value, onDismiss, mode }: TableCellInspecto height={500} language="json" showLineNumbers={true} - showMiniMap={(text && text.length) > 100} + showMiniMap={(text ? text.length : 0) > 100} value={text} readOnly={true} wordWrap={true} /> ) : ( - 
{text}
+ 
{text}
)} ); } + +// TODO: should we have different empty styles? +const getStyles = (theme: GrafanaTheme2) => ({ + textContainer: css({ + color: theme.colors.text.secondary, + minHeight: 42, + }), +}); diff --git a/packages/grafana-ui/src/components/Table/TableNG/Cells/SparklineCell.tsx b/packages/grafana-ui/src/components/Table/TableNG/Cells/SparklineCell.tsx index 08f8b8b6cd1..acedb2091a7 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/Cells/SparklineCell.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/Cells/SparklineCell.tsx @@ -1,15 +1,7 @@ import { css } from '@emotion/css'; import * as React from 'react'; -import { - FieldType, - FieldConfig, - getMinMaxAndDelta, - FieldSparkline, - isDataFrame, - Field, - isDataFrameWithValue, -} from '@grafana/data'; +import { FieldConfig, getMinMaxAndDelta, Field, isDataFrameWithValue } from '@grafana/data'; import { t } from '@grafana/i18n'; import { BarAlignment, @@ -26,7 +18,7 @@ import { measureText } from '../../../../utils/measureText'; import { FormattedValueDisplay } from '../../../FormattedValueDisplay/FormattedValueDisplay'; import { Sparkline } from '../../../Sparkline/Sparkline'; import { SparklineCellProps, TableCellStyles } from '../types'; -import { getAlignmentFactor, getCellOptions } from '../utils'; +import { getAlignmentFactor, getCellOptions, prepareSparklineValue } from '../utils'; export const defaultSparklineCellConfig: TableSparklineCellOptions = { type: TableCellDisplayMode.Sparkline, @@ -43,7 +35,7 @@ export const defaultSparklineCellConfig: TableSparklineCellOptions = { export const SparklineCell = (props: SparklineCellProps) => { const { field, value, theme, timeRange, rowIdx, width } = props; - const sparkline = getSparkline(value, field); + const sparkline = prepareSparklineValue(value, field); if (!sparkline) { return <>{field.config.noValue || t('grafana-ui.table.sparkline.no-data', 'no data')}; @@ -102,30 +94,6 @@ export const SparklineCell = (props: SparklineCellProps) => { ); }; -function getSparkline(value: unknown, field: Field): FieldSparkline | undefined { - if (Array.isArray(value)) { - return { - y: { - name: `${field.name}-sparkline`, - type: FieldType.number, - values: value, - config: {}, - }, - }; - } - - if (isDataFrame(value)) { - const timeField = value.fields.find((x) => x.type === FieldType.time); - const numberField = value.fields.find((x) => x.type === FieldType.number); - - if (timeField && numberField) { - return { x: timeField, y: numberField }; - } - } - - return; -} - function getTableSparklineCellOptions(field: Field): TableSparklineCellOptions { let options = getCellOptions(field); if (options.type === TableCellDisplayMode.Auto) { diff --git a/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx b/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx index f5fb37b68d9..5f64c58773a 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/TableNG.tsx @@ -563,7 +563,6 @@ export function TableNG(props: TableNGProps) { ( + ({ field, value, setInspectCell, onCellFilterAdded, className, cellInspect, showFilters }: TableCellActionsProps) => ( // stopping propagation to prevent clicks within the actions menu from triggering the cell click events // for things like the data links tooltip. // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions @@ -31,24 +17,8 @@ export const TableCellActions = memo( name="eye" aria-label={t('grafana-ui.table.cell-inspect-tooltip', 'Inspect value')} onClick={() => { - let inspectValue = value; - let mode = TableCellInspectorMode.text; - - if (field.type === FieldType.geo && value instanceof Geometry) { - inspectValue = new WKT().writeGeometry(value, { - featureProjection: 'EPSG:3857', - dataProjection: 'EPSG:4326', - }); - mode = TableCellInspectorMode.code; - } - if (cellOptions.type === TableCellDisplayMode.JSONView) { - mode = TableCellInspectorMode.code; - } - - setInspectCell({ - value: String(inspectValue ?? ''), - mode, - }); + const [inspectValue, mode] = buildInspectValue(value, field); + setInspectCell({ value: inspectValue, mode }); }} /> )} diff --git a/packages/grafana-ui/src/components/Table/TableNG/types.ts b/packages/grafana-ui/src/components/Table/TableNG/types.ts index 376fe895ba2..c4d63b2826b 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/types.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/types.ts @@ -167,12 +167,12 @@ export type InspectCellProps = { rowIdx?: number; value: string; mode?: TableCellInspectorMode.code | TableCellInspectorMode.text; + preformatted?: boolean; }; export interface TableCellActionsProps { field: Field; value: TableCellValue; - cellOptions: TableCellOptions; displayName: string; cellInspect: boolean; showFilters: boolean; 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 c6060814abe..d788ccdcb63 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts @@ -1,3 +1,4 @@ +import { Point } from 'ol/geom'; import { SortColumn } from 'react-data-grid'; import { @@ -49,6 +50,8 @@ import { parseStyleJson, calculateFooterHeight, displayJsonValue, + prepareSparklineValue, + buildInspectValue, } from './utils'; describe('TableNG utils', () => { @@ -1531,4 +1534,152 @@ describe('TableNG utils', () => { expect(parseStyleJson('{"notARealStyle": "someValue"}')).toEqual({ notARealStyle: 'someValue' }); }); }); + + describe('prepareSparklineValue', () => { + it('should return an array of numbers when given an array of numbers', () => { + expect( + prepareSparklineValue([1, 2, 3, 4, 5], { + name: 'test', + type: FieldType.number, + values: [1, 2, 3, 4, 5], + config: {}, + }) + ).toEqual({ + y: { + name: `test-sparkline`, + type: FieldType.number, + values: [1, 2, 3, 4, 5], + config: {}, + }, + }); + }); + + it('should parse the x and y values from a dataframe', () => { + const frame = createDataFrame({ + fields: [ + { name: 'x', type: FieldType.time, values: [0, 1000, 2000, 3000, 4000] }, + { name: 'y', type: FieldType.number, values: [10, 20, 30, 40, 50] }, + ], + }); + expect( + prepareSparklineValue(frame, { + name: 'test', + type: FieldType.frame, + values: [frame], + config: {}, + }) + ).toEqual({ + x: { + name: 'x', + type: FieldType.time, + values: [0, 1000, 2000, 3000, 4000], + config: {}, + }, + y: { + name: 'y', + type: FieldType.number, + values: [10, 20, 30, 40, 50], + config: {}, + }, + }); + }); + + it('should return undefined for non-array and non-dataframe values', () => { + expect( + prepareSparklineValue('not an array or dataframe', { + name: 'test', + type: FieldType.string, + values: ['a', 'b', 'c'], + config: {}, + }) + ).toBeUndefined(); + }); + }); + + describe('buildInspectValue', () => { + const numberFieldWithNulls: Field = { + name: 'numbers-with-nulls', + type: FieldType.number, + values: [0, 1, 2, null, NaN], + config: {}, + }; + const stringField: Field = { + name: 'string', + type: FieldType.string, + values: ['foo', 'bar', 'baz', null], + config: {}, + }; + const jsonStringField: Field = { + ...stringField, + config: { custom: { cellOptions: { type: TableCellDisplayMode.JSONView } } }, + }; + const booleanField: Field = { + name: 'boolean-field', + type: FieldType.boolean, + values: [true, false, true], + config: {}, + }; + const sparklineField: Field = { + name: 'sparkline-field', + type: FieldType.frame, + values: [ + createDataFrame({ + fields: [ + { name: 'x', type: FieldType.time, values: [0, 1000, 2000] }, + { name: 'y', type: FieldType.number, values: [10, 20, 30] }, + ], + }), + ], + config: {}, + }; + const sparklineFieldNoX: Field = { + name: 'sparkline-field-no-x', + type: FieldType.other, + values: [[2, 4, 6, 8, 10]], + config: { + custom: { cellOptions: { type: TableCellDisplayMode.Sparkline } }, + }, + }; + const arrayField: Field = { + name: 'array-field', + type: FieldType.other, + values: [ + ['foo', 'bar', 'baz'], + ['one', 'two', 'three'], + ], + config: {}, + }; + const objectField: Field = { + name: 'array-field', + type: FieldType.other, + values: [ + { foo: true, b: 'baz' }, + { foo: false, b: 'qux' }, + ], + config: {}, + }; + const geoField: Field = { + name: 'geo-field', + type: FieldType.geo, + values: [new Point([0, -74.1])], + config: {}, + }; + it.each([ + { name: 'numbers', input: { valueIdx: 0, field: numberFieldWithNulls } }, + { name: 'string', input: { valueIdx: 0, field: stringField } }, + { name: 'string w/ JSON', input: { valueIdx: 2, field: jsonStringField } }, + { name: 'boolean', input: { valueIdx: 0, field: booleanField } }, + { name: 'NaN', input: { valueIdx: 4, field: numberFieldWithNulls } }, + { name: 'null', input: { valueIdx: 3, field: numberFieldWithNulls } }, + { name: 'null w/ JSON', input: { valueIdx: 3, field: jsonStringField } }, + { name: 'undefined', input: { valueIdx: 6, field: numberFieldWithNulls } }, + { name: 'sparkline', input: { valueIdx: 0, field: sparklineField } }, + { name: 'sparkline (no x)', input: { valueIdx: 0, field: sparklineFieldNoX } }, + { name: 'array', input: { valueIdx: 0, field: arrayField } }, + { name: 'object', input: { valueIdx: 0, field: objectField } }, + { name: 'geo', input: { valueIdx: 0, field: geoField } }, + ])('should handle $name', ({ input: { field, valueIdx = 0 } }) => { + expect(buildInspectValue(field.values[valueIdx], field)).toMatchSnapshot(); + }); + }); }); diff --git a/packages/grafana-ui/src/components/Table/TableNG/utils.ts b/packages/grafana-ui/src/components/Table/TableNG/utils.ts index d2dd402217d..e3ab23b3e0e 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/utils.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/utils.ts @@ -1,5 +1,7 @@ import { Property } from 'csstype'; import memoize from 'micro-memoize'; +import WKT from 'ol/format/WKT'; +import Geometry from 'ol/geom/Geometry'; import { CSSProperties } from 'react'; import { SortColumn } from 'react-data-grid'; import tinycolor from 'tinycolor2'; @@ -15,6 +17,8 @@ import { DisplayValueAlignmentFactors, DataFrame, DisplayProcessor, + isDataFrame, + FieldSparkline, DecimalCount, } from '@grafana/data'; import { @@ -26,10 +30,11 @@ import { } from '@grafana/schema'; import { getTextColorForAlphaBackground } from '../../../utils/colors'; +import { TableCellInspectorMode } from '../TableCellInspector'; import { TableCellOptions } from '../types'; import { inferPills } from './Cells/PillCell'; -import { AutoCellRenderer, getCellRenderer } from './Cells/renderers'; +import { AutoCellRenderer, getAutoRendererDisplayMode, getCellRenderer } from './Cells/renderers'; import { COLUMN, TABLE } from './constants'; import { TableRow, @@ -991,6 +996,75 @@ export const displayJsonValue: (field: Field) => DisplayProcessor = (field: Fiel }; }; +export function prepareSparklineValue(value: unknown, field: Field): FieldSparkline | undefined { + if (Array.isArray(value)) { + return { + y: { + name: `${field.name}-sparkline`, + type: FieldType.number, + values: value, + config: {}, + }, + }; + } + + if (isDataFrame(value)) { + const timeField = value.fields.find((x) => x.type === FieldType.time); + const numberField = value.fields.find((x) => x.type === FieldType.number); + + if (timeField && numberField) { + return { x: timeField, y: numberField }; + } + } + + return; +} + +function isPlainObject(value: unknown): value is object { + return typeof value === 'object' && value != null && !Array.isArray(value); +} + +export function buildInspectValue(value: unknown, field: Field): [string, TableCellInspectorMode] { + const cellOptions = getCellOptions(field); + + let inspectValue: string; + let mode = TableCellInspectorMode.text; + + if (field.type === FieldType.geo && value instanceof Geometry) { + inspectValue = new WKT().writeGeometry(value, { + featureProjection: 'EPSG:3857', + dataProjection: 'EPSG:4326', + }); + mode = TableCellInspectorMode.code; + } else if ( + cellOptions.type === TableCellDisplayMode.Sparkline || + getAutoRendererDisplayMode(field) === TableCellDisplayMode.Sparkline + ) { + // rather than JSON.stringify this, manually format it to make the coordinate tuples more legible to the user. + const fieldSparkline = prepareSparklineValue(value, field); + inspectValue = '['; + if (fieldSparkline != null) { + // if an x value exists, render as a tuple [x,y], otherwise just y + const buildValString: (idx: number) => string = + fieldSparkline.x != null + ? (idx) => `[${fieldSparkline.x!.values[idx] ?? 'null'}, ${fieldSparkline.y.values[idx] ?? 'null'}]` + : (idx) => `${fieldSparkline.y.values[idx] ?? 'null'}`; + for (let i = 0; i < fieldSparkline.y.values.length; i++) { + inspectValue += `\n ${buildValString(i)}${i === fieldSparkline.y.values.length - 1 ? '\n' : ','}`; + } + } + inspectValue += ']'; + mode = TableCellInspectorMode.code; + } else if (cellOptions.type === TableCellDisplayMode.JSONView || Array.isArray(value) || isPlainObject(value)) { + inspectValue = JSON.stringify(value, null, ' '); + mode = TableCellInspectorMode.code; + } else { + inspectValue = String(value ?? ''); + } + + return [inspectValue, mode]; +} + export function getSummaryCellTextAlign(textAlign: TextAlign, cellType: TableCellDisplayMode): TextAlign { // gauge is weird. left-aligned gauge has the viz on the left and its numbers on the right, and vice-versa. // if you center-aligned your gauge... ok.