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 55b9d3e19ba..57001a4176e 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 @@ -61,6 +61,17 @@ describe('PillCell', () => { ` ); }); + + it('non-string values', () => { + expectHTML( + render(), + ` + 100 + 200 + 300 + ` + ); + }); }); describe('Color by value mappings', () => { 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 2b85560c792..4ba20f36605 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/Cells/PillCell.tsx +++ b/packages/grafana-ui/src/components/Table/TableNG/Cells/PillCell.tsx @@ -49,12 +49,12 @@ interface Pill { const SPLIT_RE = /\s*,\s*/; const TRANSPARENT = 'rgba(0,0,0,0)'; -function createPills(pillValues: string[], field: Field, theme: GrafanaTheme2): Pill[] { +function createPills(pillValues: unknown[], field: Field, theme: GrafanaTheme2): Pill[] { return pillValues.map((pill, index) => { const bgColor = getPillColor(pill, field, theme); const textColor = colorManipulator.getContrastRatio('#FFFFFF', bgColor) >= 4.5 ? '#FFFFFF' : '#000000'; return { - value: pill, + value: String(pill), key: `${pill}-${index}`, bgColor, color: textColor, @@ -62,7 +62,7 @@ function createPills(pillValues: string[], field: Field, theme: GrafanaTheme2): }); } -export function inferPills(rawValue: TableCellValue): string[] { +export function inferPills(rawValue: TableCellValue): unknown[] { if (rawValue === '' || rawValue == null) { return []; } @@ -81,7 +81,7 @@ export function inferPills(rawValue: TableCellValue): string[] { } // FIXME: this does not yet support "shades of a color" -function getPillColor(value: string, field: Field, theme: GrafanaTheme2): string { +function getPillColor(value: unknown, field: Field, theme: GrafanaTheme2): string { const cfg = field.config; if (cfg.mappings?.length ?? 0 > 0) { @@ -101,7 +101,7 @@ function getPillColor(value: string, field: Field, theme: GrafanaTheme2): string } } - return getColorByStringHash(colors, value); + return getColorByStringHash(colors, String(value)); } export const getStyles: TableCellStyles = (theme, { textWrap, shouldOverflow }) => 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 ce378961589..ad06992984e 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/hooks.test.ts @@ -477,7 +477,7 @@ describe('TableNG hooks', () => { }), columnWidths: [100, 100, 100], enabled: true, - typographyCtx: { ...typographyCtx, avgCharWidth: 5, wrappedCount: jest.fn(() => 2) }, + typographyCtx: { ...typographyCtx, avgCharWidth: 5, measureHeight: jest.fn(() => 38) }, sortColumns: [], }); }); @@ -486,7 +486,7 @@ describe('TableNG hooks', () => { }); it('should calculate the available width for a header cell based on the icons rendered within it', () => { - const countFn = jest.fn(() => 1); + const heightFn = jest.fn(() => 20); const { fields } = setupData(); @@ -512,13 +512,13 @@ describe('TableNG hooks', () => { fields: modifiedFields, columnWidths: [100, 100, 100], enabled: true, - typographyCtx: { ...typographyCtx, wrappedCount: countFn }, + typographyCtx: { ...typographyCtx, measureHeight: heightFn }, sortColumns: [], showTypeIcons: false, }); }); - expect(countFn).toHaveBeenCalledWith('Longer name that needs wrapping', 86, modifiedFields[0], -1); + expect(heightFn).toHaveBeenCalledWith('Longer name that needs wrapping', 86, modifiedFields[0], -1, 6); modifiedFields = fields.map((field) => { if (field.name === 'name') { @@ -543,13 +543,13 @@ describe('TableNG hooks', () => { fields: modifiedFields, columnWidths: [100, 100, 100], enabled: true, - typographyCtx: { ...typographyCtx, wrappedCount: countFn }, + typographyCtx: { ...typographyCtx, measureHeight: heightFn }, sortColumns: [{ columnKey: 'Longer name that needs wrapping', direction: 'ASC' }], showTypeIcons: true, }); }); - expect(countFn).toHaveBeenCalledWith('Longer name that needs wrapping', 26, modifiedFields[0], -1); + expect(heightFn).toHaveBeenCalledWith('Longer name that needs wrapping', 26, modifiedFields[0], -1, 6); }); }); @@ -685,9 +685,9 @@ describe('TableNG hooks', () => { }); }); - // we test the lineCounters and getRowHeight directly to check that all of that - // math is working correctly. we mainly want to confirm here that the - // cache is clearing and that the local logic in this hook works. + // we test the cell height measurerers and getRowHeight directly to check + //that all of that math is working correctly. we mainly want to confirm that + // the cache is clearing and that the local logic in this hook works. describe('wrapped columns', () => { let rows: TableRow[]; let fieldsWithWrappedText: Field[]; @@ -749,14 +749,14 @@ describe('TableNG hooks', () => { it('adjusts the width of the columns based on the cell padding and border', () => { fieldsWithWrappedText[0].values[0] = 'Annie Lennox'; - const wrappedCountFn = jest.fn(() => 2); - const estimateLinesFn = jest.fn(() => 2); + const measureHeightFn = jest.fn(() => 40); + const estimateHeightFn = jest.fn(() => 40); const { result } = renderHook(() => { const rowHeight = useRowHeight({ fields: fieldsWithWrappedText, columnWidths: [100, 100, 100], defaultHeight: 40, - typographyCtx: { ...typographyCtx, wrappedCount: wrappedCountFn, estimateLines: estimateLinesFn }, + typographyCtx: { ...typographyCtx, measureHeight: measureHeightFn, estimateHeight: estimateHeightFn }, hasNestedFrames: false, expandedRows: new Set(), }); @@ -768,11 +768,12 @@ describe('TableNG hooks', () => { expect(result.current(rows[0])).toEqual(expect.any(Number)); - expect(estimateLinesFn).toHaveBeenCalledWith( + expect(measureHeightFn).toHaveBeenCalledWith( 'Annie Lennox', 100 - TABLE.CELL_PADDING * 2 - TABLE.BORDER_RIGHT, fieldsWithWrappedText[0], - 0 + 0, + 22 ); }); }); diff --git a/packages/grafana-ui/src/components/Table/TableNG/hooks.ts b/packages/grafana-ui/src/components/Table/TableNG/hooks.ts index 84b9a277dad..5162aac82f8 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 { TableCellDisplayMode, TableColumnResizeActionCallback } from '../types'; +import { TableColumnResizeActionCallback } from '../types'; import { TABLE } from './constants'; import { FilterType, TableFooterCalc, TableRow, TableSortByFieldState, TableSummaryRow, TypographyCtx } from './types'; @@ -13,9 +13,8 @@ import { applySort, getColumnTypes, getRowHeight, - buildHeaderLineCounters, - buildRowLineCounters, - getCellOptions, + buildHeaderHeightMeasurers, + buildCellHeightMeasurers, } from './utils'; // Helper function to get displayed value @@ -341,7 +340,7 @@ export function useHeaderHeight({ }: UseHeaderHeightOptions): number { const perIconSpace = ICON_WIDTH + ICON_GAP; - const lineCounters = useMemo(() => buildHeaderLineCounters(fields, typographyCtx), [fields, typographyCtx]); + const measurers = useMemo(() => buildHeaderHeightMeasurers(fields, typographyCtx), [fields, typographyCtx]); const columnAvailableWidths = useMemo( () => @@ -369,16 +368,8 @@ export function useHeaderHeight({ if (!enabled) { return 0; } - return getRowHeight( - fields, - -1, - columnAvailableWidths, - TABLE.HEADER_HEIGHT, - lineCounters, - TABLE.LINE_HEIGHT, - TABLE.CELL_PADDING - ); - }, [fields, enabled, columnAvailableWidths, lineCounters]); + return getRowHeight(fields, -1, columnAvailableWidths, TABLE.HEADER_HEIGHT, measurers, TABLE.CELL_PADDING); + }, [fields, enabled, columnAvailableWidths, measurers]); return headerHeight; } @@ -400,8 +391,8 @@ export function useRowHeight({ expandedRows, typographyCtx, }: UseRowHeightOptions): NonNullable | ((row: TableRow) => number) { - const lineCounters = useMemo(() => buildRowLineCounters(fields, typographyCtx), [fields, typographyCtx]); - const hasWrappedCols = useMemo(() => lineCounters?.length ?? 0 > 0, [lineCounters]); + const measurers = useMemo(() => buildCellHeightMeasurers(fields, typographyCtx), [fields, typographyCtx]); + const hasWrappedCols = useMemo(() => measurers?.length ?? 0 > 0, [measurers]); const colWidths = useMemo(() => { const columnWidthAffordance = 2 * TABLE.CELL_PADDING + TABLE.BORDER_RIGHT; @@ -437,27 +428,11 @@ export function useRowHeight({ // regular rows let result = cache[row.__index]; if (!result) { - result = cache[row.__index] = getRowHeight( - fields, - row.__index, - colWidths, - defaultHeight, - lineCounters, - TABLE.LINE_HEIGHT, - (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; - } - ); + result = cache[row.__index] = getRowHeight(fields, row.__index, colWidths, defaultHeight, measurers); } return result; }; - }, [hasNestedFrames, hasWrappedCols, defaultHeight, fields, colWidths, lineCounters, expandedRows]); + }, [hasNestedFrames, hasWrappedCols, defaultHeight, fields, colWidths, measurers, expandedRows]); return rowHeight; } diff --git a/packages/grafana-ui/src/components/Table/TableNG/types.ts b/packages/grafana-ui/src/components/Table/TableNG/types.ts index 03705ef669d..3434b24dcf0 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/types.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/types.ts @@ -279,24 +279,30 @@ export interface TypographyCtx { fontFamily: string; letterSpacing: number; avgCharWidth: number; - estimateLines: LineCounter; - wrappedCount: LineCounter; + estimateHeight: MeasureCellHeight; + measureHeight: MeasureCellHeight; } -export type LineCounter = (value: unknown, width: number, field: Field, rowIdx: number) => number; -export interface LineCounterEntry { +export type MeasureCellHeight = ( + value: unknown, + width: number, + field: Field, + rowIdx: number, + lineHeight: number +) => number; +export interface MeasureCellHeightEntry { /** * given a values and the available width, returns the line count for that value */ - counter: LineCounter; + measure: MeasureCellHeight; /** * if getting an accurate line count is expensive, you can provide an estimate method - * which will be used when looping over the row. the counter method will only be invoked + * which will be used when looping over the row. the method will only be invoked * for the cell which is the maximum line count for the row. */ - estimate?: LineCounter; + estimate?: MeasureCellHeight; /** - * indicates which field indexes of the visible fields this line counter applies to. + * indicates which field indexes of the visible fields this measurer applies to. */ fieldIdxs: number[]; } diff --git a/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts b/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts index 5ebd14336b9..8ca2ba7870f 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/utils.test.ts @@ -18,7 +18,7 @@ import { BarGaugeDisplayMode, TableCellBackgroundDisplayMode, TableCellHeight } import { TableCellDisplayMode } from '../types'; import { COLUMN, TABLE } from './constants'; -import { LineCounterEntry } from './types'; +import { MeasureCellHeightEntry } from './types'; import { extractPixelValue, frameToRecords, @@ -34,15 +34,15 @@ import { getColumnTypes, computeColWidths, getRowHeight, - buildRowLineCounters, - buildHeaderLineCounters, - getTextLineEstimator, + buildCellHeightMeasurers, + buildHeaderHeightMeasurers, + getTextHeightEstimator, createTypographyContext, applySort, SINGLE_LINE_ESTIMATE_THRESHOLD, - wrapUwrapCount, - getDataLinksCounter, - getPillLineCounter, + getTextHeightMeasurerFromUwrapCount, + getDataLinksHeightMeasurer, + getPillCellHeightMeasurer, getDefaultRowHeight, getDisplayName, predicateByName, @@ -850,49 +850,49 @@ describe('TableNG utils', () => { ctx: expect.any(CanvasRenderingContext2D), fontFamily: 'sans-serif', letterSpacing: 0.15, - wrappedCount: expect.any(Function), - estimateLines: expect.any(Function), + measureHeight: expect.any(Function), + estimateHeight: expect.any(Function), avgCharWidth: expect.any(Number), }) ); - expect(ctx.wrappedCount('the quick brown fox jumps over the lazy dog', 100, field, 0)).toEqual( + expect(ctx.measureHeight('the quick brown fox jumps over the lazy dog', 100, field, 0, 20)).toEqual( expect.any(Number) ); - expect(ctx.estimateLines('the quick brown fox jumps over the lazy dog', 100, field, 0)).toEqual( + expect(ctx.estimateHeight('the quick brown fox jumps over the lazy dog', 100, field, 0, 20)).toEqual( expect.any(Number) ); }); }); - describe('wrapUwrapCount', () => { + describe('getTextHeightMeasurerFromUwrapCount', () => { 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); + const measureHeight = getTextHeightMeasurerFromUwrapCount(jest.fn(() => 2)); + expect(measureHeight('test string', 100, field, 0, 20)).toBe(40); }); - 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); + it("returns a single line's height for null or undefined values", () => { + const measureHeight = getTextHeightMeasurerFromUwrapCount(jest.fn(() => 2)); + expect(measureHeight(null, 100, field, 0, 20)).toBe(20); + expect(measureHeight(undefined, 100, field, 0, 20)).toBe(20); }); }); - describe('getTextLineEstimator', () => { - const counter = getTextLineEstimator(10); + describe('getTextHeightEstimator', () => { + const estimator = getTextHeightEstimator(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, field, 0)).toBe(-1); + expect(estimator('asdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdf', 5, field, 0, 22)).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, field, 0)).toBe(2.5); + expect(estimator('asdfas dfasdfasdf asdfasdfasdfa sdfasdfasdfasdf 23', 200, field, 0, 20)).toBe(60); }); }); - describe('getDataLinksCounter', () => { + describe('getDataLinksHeightMeasurer', () => { it('counts number of valid links using getCellLinks', () => { const field: Field = { name: 'test', @@ -911,58 +911,58 @@ describe('TableNG utils', () => { values: ['value1'], }; - const counter = getDataLinksCounter(); - expect(counter('my value', 100, field, 0)).toBe(2); + const measurer = getDataLinksHeightMeasurer(); + expect(measurer('my value', 100, field, 0, 20)).toBe(40); }); }); - describe('getPillLineCounter', () => { + describe('getPillCellHeightMeasurer', () => { 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); + const measurer = getPillCellHeightMeasurer(jest.fn((str) => str.length * 5)); + expect(measurer('tag1,tag2', 100, {} as Field, 0, 20)).toBe(20); + expect(measurer('tag1,tag2,tag3,tag4,tag5,tag6', 100, {} as Field, 0, 20)).toBe(68); }); 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); + const measurer = getPillCellHeightMeasurer(jest.fn((str) => str.length * 5)); + expect(measurer(null, 100, {} as Field, 0, 20)).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); + const measurer = getPillCellHeightMeasurer(jest.fn((str) => str.length * 5)); + expect(measurer('', 100, {} as Field, 0, 20)).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); + const measurer = getPillCellHeightMeasurer(widthMeasurement); + measurer('tag1,tag2,tag3,tag4,tag5,tag6', 100, {} as Field, 0, 20); + measurer('tag1,tag2', 100, {} as Field, 0, 20); + measurer('tag2', 200, {} as Field, 0, 20); + measurer('tag2,tag3,tag2,tag4,tag4,tag2,tag5', 300, {} as Field, 0, 20); expect(widthMeasurement).toHaveBeenCalledTimes(6); // Should only call for unique values }); }); - describe('buildHeaderLineCounters', () => { + describe('buildHeaderHeightMeasurers', () => { const ctx = { fontFamily: 'sans-serif', letterSpacing: 0.15, ctx: {} as CanvasRenderingContext2D, count: jest.fn(() => 2), avgCharWidth: 7, - wrappedCount: jest.fn(() => 2), - estimateLines: jest.fn(() => 2), + measureHeight: jest.fn(() => 2), + estimateHeight: jest.fn(() => 2), }; - it('returns an array of line counters for each column', () => { + it('returns an array of measurers for each column', () => { const fields: Field[] = [ { name: 'Name', type: FieldType.string, values: [], config: { custom: { wrapHeaderText: true } } }, { name: 'Age', type: FieldType.number, values: [], config: { custom: { wrapHeaderText: true } } }, ]; - const counters = buildHeaderLineCounters(fields, ctx); - expect(counters![0].counter).toEqual(expect.any(Function)); - expect(counters![0].fieldIdxs).toEqual([0, 1]); + const measurers = buildHeaderHeightMeasurers(fields, ctx); + expect(measurers![0].measure).toEqual(expect.any(Function)); + expect(measurers![0].fieldIdxs).toEqual([0, 1]); }); it('does not return the index of columns which are not wrapped', () => { @@ -971,8 +971,8 @@ describe('TableNG utils', () => { { name: 'Age', type: FieldType.number, values: [], config: { custom: { wrapHeaderText: true } } }, ]; - const counters = buildHeaderLineCounters(fields, ctx); - expect(counters![0].fieldIdxs).toEqual([1]); + const measurers = buildHeaderHeightMeasurers(fields, ctx); + expect(measurers![0].fieldIdxs).toEqual([1]); }); it('returns undefined if no columns are wrapped', () => { @@ -981,22 +981,22 @@ describe('TableNG utils', () => { { name: 'Age', type: FieldType.number, values: [], config: { custom: {} } }, ]; - const counters = buildHeaderLineCounters(fields, ctx); - expect(counters).toBeUndefined(); + const measurers = buildHeaderHeightMeasurers(fields, ctx); + expect(measurers).toBeUndefined(); }); }); - describe('buildRowLineCounters', () => { + describe('buildCellHeightMeasurers', () => { const ctx = { fontFamily: 'sans-serif', letterSpacing: 0.15, ctx: {} as CanvasRenderingContext2D, - wrappedCount: jest.fn(() => 2), - estimateLines: jest.fn(() => 2), + measureHeight: jest.fn(() => 2), + estimateHeight: jest.fn(() => 2), avgCharWidth: 7, }; - it('sets up text line counters for each text column if wrapping is on', () => { + it('sets up text height measurers for each text column if wrapping is on', () => { const fields: Field[] = [ { name: 'Name', type: FieldType.string, values: [], config: { custom: { cellOptions: { wrapText: true } } } }, { @@ -1006,9 +1006,9 @@ describe('TableNG utils', () => { config: { custom: { cellOptions: { wrapText: true } } }, }, ]; - const counters = buildRowLineCounters(fields, ctx); - expect(counters![0].counter).toEqual(expect.any(Function)); - expect(counters![0].fieldIdxs).toEqual([0, 1]); + const measurers = buildCellHeightMeasurers(fields, ctx); + expect(measurers![0].measure).toEqual(expect.any(Function)); + expect(measurers![0].fieldIdxs).toEqual([0, 1]); }); it('does not return the index of columns which are not wrapped', () => { @@ -1022,8 +1022,8 @@ describe('TableNG utils', () => { }, ]; - const counters = buildRowLineCounters(fields, ctx); - expect(counters![0].fieldIdxs).toEqual([1]); + const measurers = buildCellHeightMeasurers(fields, ctx); + expect(measurers![0].fieldIdxs).toEqual([1]); }); it('sets up line counting for pills if present and wrapping is on', () => { @@ -1035,12 +1035,12 @@ describe('TableNG utils', () => { 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]); + const measurers = buildCellHeightMeasurers(fields, ctx); + expect(measurers![0].estimate).toEqual(expect.any(Function)); + expect(measurers![0].estimate!('tag1,tag2', 100, fields[0], 0, 22)).toEqual(expect.any(Number)); + expect(measurers![0].measure).toEqual(expect.any(Function)); + expect(measurers![0].measure('tag1,tag2', 100, fields[0], 0, 22)).toEqual(expect.any(Number)); + expect(measurers![0].fieldIdxs).toEqual([0]); }); it('sets up line counting for datalinks if present and wrapping is on', () => { @@ -1056,10 +1056,10 @@ describe('TableNG utils', () => { ]), }, ]; - 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]); + const measurers = buildCellHeightMeasurers(fields, ctx); + expect(measurers![0].measure).toEqual(expect.any(Function)); + expect(measurers![0].measure('http://example.com/1', 100, fields[0], 0, 22)).toEqual(expect.any(Number)); + expect(measurers![0].fieldIdxs).toEqual([0]); }); it('does not enable text counting for non-string fields', () => { @@ -1068,9 +1068,9 @@ describe('TableNG utils', () => { { name: 'Age', type: FieldType.number, values: [], config: { custom: { cellOptions: { wrapText: true } } } }, ]; - const counters = buildRowLineCounters(fields, ctx); + const measurers = buildCellHeightMeasurers(fields, ctx); // empty array - we had one column that indicated it wraps, but it was numeric, so we just ignore it - expect(counters).toEqual([]); + expect(measurers).toEqual([]); }); it('returns an undefined if no columns are wrapped', () => { @@ -1079,14 +1079,14 @@ describe('TableNG utils', () => { { name: 'Age', type: FieldType.number, values: [], config: { custom: {} } }, ]; - const counters = buildRowLineCounters(fields, ctx); - expect(counters).toBeUndefined(); + const measurers = buildCellHeightMeasurers(fields, ctx); + expect(measurers).toBeUndefined(); }); }); describe('getRowHeight', () => { let fields: Field[]; - let counters: LineCounterEntry[]; + let measurers: MeasureCellHeightEntry[]; beforeEach(() => { fields = [ @@ -1103,91 +1103,109 @@ describe('TableNG utils', () => { config: { custom: { cellOptions: { wrapText: true } } }, }, ]; - counters = [ - { counter: jest.fn((value, _length: number) => String(value).split(' ').length), fieldIdxs: [0] }, // Mocked to count words as lines - { counter: jest.fn((value, _length: number) => Math.ceil(String(value).length / 3)), fieldIdxs: [1] }, // Mocked to return a line for every 3 digits of a number + measurers = [ + { + measure: jest.fn( + (value, _length, _field, _rowIdx, lineHeight) => String(value).split(' ').length * lineHeight + ), + fieldIdxs: [0], + }, // Mocked to count words as lines + { + measure: jest.fn( + (value, _length, _field, _rowIdx, lineHeight) => Math.ceil(String(value).length / 3) * lineHeight + ), + fieldIdxs: [1], + }, // Mocked to return a line for every 3 digits of a number ]; }); it('should use the default height for single-line rows', () => { // 1 line @ 20px, 10px vertical padding = 30, minimum is 36 - expect(getRowHeight(fields, 0, [30, 30], 36, counters, 20, 10)).toBe(36); + expect(getRowHeight(fields, 0, [30, 30], 36, measurers, 20, 10)).toBe(36); }); it('should use the default height for multi-line rows which are shorter than the default height', () => { // 3 lines @ 5px, 5px vertical padding = 20, minimum is 36 - expect(getRowHeight(fields, 3, [30, 30], 36, counters, 5, 5)).toBe(36); + expect(getRowHeight(fields, 3, [30, 30], 36, measurers, 5, 5)).toBe(36); }); - it('should return the row height using line counters for multi-line', () => { + it('should return the row height using line measurers for multi-line', () => { // 3 lines @ 20px ('longer', 'one', 'here'), 10px vertical padding - expect(getRowHeight(fields, 3, [30, 30], 36, counters, 20, 10)).toBe(70); + expect(getRowHeight(fields, 3, [30, 30], 36, measurers, 20, 10)).toBe(70); // 4 lines @ 15px (789 122 349 932), 15px vertical padding - expect(getRowHeight(fields, 4, [30, 30], 36, counters, 15, 15)).toBe(75); + expect(getRowHeight(fields, 4, [30, 30], 36, measurers, 15, 15)).toBe(75); }); 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, fields[0], 3); - expect(counters[1].counter).toHaveBeenCalledWith(123456, 60, fields[1], 3); + getRowHeight(fields, 3, [50, 60], 36, measurers, 20, 10); + expect(measurers[0].measure).toHaveBeenCalledWith('longer one here', 50, fields[0], 3, 20); + expect(measurers[1].measure).toHaveBeenCalledWith(123456, 60, fields[1], 3, 20); }); // 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, fields[0], -1); - expect(counters[1].counter).toHaveBeenCalledWith('Age', 60, fields[1], -1); + getRowHeight(fields, -1, [50, 60], 36, measurers, 20, 10); + expect(measurers[0].measure).toHaveBeenCalledWith('Name', 50, fields[0], -1, 20); + expect(measurers[1].measure).toHaveBeenCalledWith('Age', 60, fields[1], -1, 20); }); - it('should ignore columns which do not have line counters', () => { - const height = getRowHeight(fields, 3, [30, 30], 36, [counters[1]], 20, 10); + it('should ignore columns which do not have measurers', () => { + const height = getRowHeight(fields, 3, [30, 30], 36, [measurers[1]], 20, 10); // 2 lines @ 20px, 10px vertical padding (not 3 lines, since we don't line count Name) expect(height).toBe(50); }); - it('should return the default height if there are no counters to apply', () => { + it('should return the default height if there are no measurers to apply', () => { const height = getRowHeight(fields, 3, [30, 30], 36, [], 20, 10); expect(height).toBe(36); }); describe('estimations vs. precise counts', () => { beforeEach(() => { - counters = [ - { counter: jest.fn((value, _length: number) => String(value).split(' ').length), fieldIdxs: [0] }, // Mocked to count words as lines + measurers = [ { - estimate: jest.fn((value) => String(value).length), // Mocked to return a line for every digits of a number - counter: jest.fn((value, _length: number) => Math.ceil(String(value).length / 3)), + measure: jest.fn( + (value, _length, _field, _rowIdx, lineHeight) => String(value).split(' ').length * lineHeight + ), + fieldIdxs: [0], + }, // Mocked to count words as lines + { + measure: jest.fn( + (value, _length, _field, _rowIdx, lineHeight) => Math.ceil(String(value).length / 3) * lineHeight + ), + estimate: jest.fn((value, _length, _field, _rowIdx, lineHeight) => String(value).length * lineHeight), // Mocked to return a line for every digits of a number fieldIdxs: [1], - }, + }, // Mocked to return a line for every 3 digits of a number ]; }); // 2 lines @ 20px (123,456), 10px vertical padding. when we did this before, 'longer one here' would win, making it 70px. - // the `estimate` function is picking `123456` as the longer one now (6 lines), then the `counter` function is used + // the `estimate` function is picking `123456` as the longer one now (6 lines), then the `measure` function is used // to calculate the height (2 lines). this is a very forced case, but we just want to prove that it actually works. it('uses the estimate value rather than the precise value to select the row height', () => { - expect(getRowHeight(fields, 3, [30, 30], 36, counters, 20, 10)).toBe(50); + expect(getRowHeight(fields, 3, [30, 30], 36, measurers, 20, 10)).toBe(50); }); it('returns doesnt bother getting the precise count if the estimates are all below the threshold', () => { - jest.mocked(counters[0].counter).mockReturnValue(SINGLE_LINE_ESTIMATE_THRESHOLD - 0.3); - jest.mocked(counters[1].estimate!).mockReturnValue(SINGLE_LINE_ESTIMATE_THRESHOLD - 0.1); + jest.mocked(measurers[0].measure).mockReturnValue(SINGLE_LINE_ESTIMATE_THRESHOLD - 0.3); + jest.mocked(measurers[1].estimate!).mockReturnValue(SINGLE_LINE_ESTIMATE_THRESHOLD - 0.1); - expect(getRowHeight(fields, 3, [30, 30], 36, counters, 20, 10)).toBe(36); + expect(getRowHeight(fields, 3, [30, 30], 36, measurers, 20, 10)).toBe(36); - // this is what we really care about - we want to save on performance by not calling the counter in this case. - expect(counters[1].counter).not.toHaveBeenCalled(); + // this is what we really care about - we want to save on performance by not calling the measure in this case. + expect(measurers[1].measure).not.toHaveBeenCalled(); }); it('uses the precise count if the estimate is above the threshold, even if its below 1', () => { - // NOTE: if this fails, just change the test to use a different value besides 0.1 - expect(SINGLE_LINE_ESTIMATE_THRESHOLD + 0.1).toBeLessThan(1); + // NOTE: if this fails, just change the test to use a different value besides 1 + const thresholdOffset = 1; + expect(SINGLE_LINE_ESTIMATE_THRESHOLD + thresholdOffset).toBeLessThan(TABLE.LINE_HEIGHT); - jest.mocked(counters[0].counter).mockReturnValue(SINGLE_LINE_ESTIMATE_THRESHOLD - 0.3); - jest.mocked(counters[1].estimate!).mockReturnValue(SINGLE_LINE_ESTIMATE_THRESHOLD + 0.1); + jest.mocked(measurers[0].measure).mockReturnValue(SINGLE_LINE_ESTIMATE_THRESHOLD - thresholdOffset); + jest.mocked(measurers[1].estimate!).mockReturnValue(SINGLE_LINE_ESTIMATE_THRESHOLD + thresholdOffset); - expect(getRowHeight(fields, 3, [30, 30], 36, counters, 20, 10)).toBe(50); + expect(getRowHeight(fields, 3, [30, 30], 36, measurers, 20, 10)).toBe(50); }); }); }); diff --git a/packages/grafana-ui/src/components/Table/TableNG/utils.ts b/packages/grafana-ui/src/components/Table/TableNG/utils.ts index 3c36e150a3c..3eb89230a58 100644 --- a/packages/grafana-ui/src/components/Table/TableNG/utils.ts +++ b/packages/grafana-ui/src/components/Table/TableNG/utils.ts @@ -34,8 +34,8 @@ import { FrameToRowsConverter, Comparator, TypographyCtx, - LineCounter, - LineCounterEntry, + MeasureCellHeight, + MeasureCellHeightEntry, } from './types'; /* ---------------------------- Cell calculations --------------------------- */ @@ -112,29 +112,30 @@ export function createTypographyContext(fontSize: number, fontFamily: string, le fontFamily, letterSpacing, avgCharWidth, - estimateLines: getTextLineEstimator(avgCharWidth), - wrappedCount: wrapUwrapCount(count), + estimateHeight: getTextHeightEstimator(avgCharWidth), + measureHeight: getTextHeightMeasurerFromUwrapCount(count), }; } /** * @internal wraps the uwrap count function to ensure that it is given a string. */ -export function wrapUwrapCount(count: Count): LineCounter { - return (value, width) => { +export function getTextHeightMeasurerFromUwrapCount(count: Count): MeasureCellHeight { + return (value, width, _field, _rowIdx, lineHeight) => { if (value == null) { - return 1; + return lineHeight; } - return count(String(value), width); + const lines = count(String(value), width); + return lines * lineHeight; }; } /** - * @internal returns a line counter which guesstimates a number of lines in a text cell based on the typography context's avgCharWidth. + * @internal returns a measurer which guesstimates a number of lines in a text cell based on the typography context's avgCharWidth. */ -export function getTextLineEstimator(avgCharWidth: number): LineCounter { - return (value, width) => { +export function getTextHeightEstimator(avgCharWidth: number): MeasureCellHeight { + return (value, width, _field, _rowIdx, lineHeight) => { if (!value) { return -1; } @@ -147,20 +148,21 @@ export function getTextLineEstimator(avgCharWidth: number): LineCounter { } const charsPerLine = width / avgCharWidth; - return strValue.length / charsPerLine; + const lines = Math.ceil(strValue.length / charsPerLine); + return lines * lineHeight; }; } /** * @internal */ -export function getDataLinksCounter(): LineCounter { +export function getDataLinksHeightMeasurer(): MeasureCellHeight { 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) => { + return (_value, _width, field, _rowIdx, lineHeight) => { const cacheKey = getDisplayName(field); if (linksCountCache[cacheKey] === undefined) { let count = 0; @@ -172,7 +174,7 @@ export function getDataLinksCounter(): LineCounter { linksCountCache[cacheKey] = count; } - return linksCountCache[cacheKey]; + return linksCountCache[cacheKey] * lineHeight; }; } @@ -180,10 +182,10 @@ 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 { +export function getPillCellHeightMeasurer(measureWidth: (value: string) => number): MeasureCellHeight { const widthCache: Record = {}; - return (value, width) => { + return (value, width, _field, _rowIdx, lineHeight) => { if (value == null) { return 0; } @@ -197,10 +199,11 @@ export function getPillLineCounter(measureWidth: (value: string) => number): Lin let currentLineUse = width; for (const pillValue of pillValues) { - let rawWidth = widthCache[pillValue]; + const strPill = String(pillValue); + let rawWidth = widthCache[strPill]; if (rawWidth === undefined) { - rawWidth = measureWidth(pillValue); - widthCache[pillValue] = rawWidth; + rawWidth = measureWidth(strPill); + widthCache[strPill] = rawWidth; } const pillWidth = rawWidth + PILLS_SPACING; @@ -212,14 +215,19 @@ export function getPillLineCounter(measureWidth: (value: string) => number): Lin } } - return lines; + // default line height happens to be the height of a pill, but maybe we need a custom + // const here to make sure this doesn't get out of sync with the actual pill height. + return lines * lineHeight + (lines - 1) * PILLS_GAP; }; } /** - * @internal return a text line counter for every field which has wrapHeaderText enabled. + * @internal return a text measurer for every field which has wrapHeaderText enabled. */ -export function buildHeaderLineCounters(fields: Field[], typographyCtx: TypographyCtx): LineCounterEntry[] | undefined { +export function buildHeaderHeightMeasurers( + fields: Field[], + typographyCtx: TypographyCtx +): MeasureCellHeightEntry[] | undefined { const wrappedColIdxs = fields.reduce((acc: number[], field, idx) => { if (field.config?.custom?.wrapHeaderText) { acc.push(idx); @@ -233,19 +241,55 @@ export function buildHeaderLineCounters(fields: Field[], typographyCtx: Typograp // don't bother with estimating the line counts for the headers, because it's punishing // when we get it wrong and there won't be that many compared to how many rows a table might contain. - return [{ counter: typographyCtx.wrappedCount, fieldIdxs: wrappedColIdxs }]; + return [{ measure: typographyCtx.measureHeight, fieldIdxs: wrappedColIdxs }]; } const spaceRegex = /[\s-]/; /** - * @internal return a text line counter for every field which has wrapHeaderText enabled. we do this once as we're rendering + * @internal return a text height measurer for every field which has wrapHeaderText enabled. we do this once as we're rendering * the table, and then getRowHeight uses the output of this to caluclate the height of each row. */ -export function buildRowLineCounters(fields: Field[], typographyCtx: TypographyCtx): LineCounterEntry[] | undefined { - const result: Record = {}; +export function buildCellHeightMeasurers( + fields: Field[], + typographyCtx: TypographyCtx +): MeasureCellHeightEntry[] | undefined { + const result: Record = {}; let wrappedFields = 0; + const measurerFactory: Record< + TableCellDisplayMode.Auto | TableCellDisplayMode.DataLinks | TableCellDisplayMode.Pill, + () => [MeasureCellHeight, MeasureCellHeight?] + > = { + // for string fields, we estimate the length of a line using `avgCharWidth` to limit expensive calls `count`. + [TableCellDisplayMode.Auto]: () => [typographyCtx.measureHeight, typographyCtx.estimateHeight], + [TableCellDisplayMode.DataLinks]: () => [getDataLinksHeightMeasurer(), undefined], + // pills use a different font size, so they require their own typography context. + [TableCellDisplayMode.Pill]: () => { + const pillTypographyCtx = createTypographyContext( + PILLS_FONT_SIZE, + typographyCtx.fontFamily, + typographyCtx.letterSpacing + ); + return [ + getPillCellHeightMeasurer((value) => pillTypographyCtx.ctx.measureText(value).width), + getPillCellHeightMeasurer((value) => value.length * pillTypographyCtx.avgCharWidth), + ]; + }, + } as const; + + const setupMeasurerForIdx = (measurerFactoryKey: keyof typeof measurerFactory, fieldIdx: number) => { + if (!result[measurerFactoryKey]) { + const [measure, estimate] = measurerFactory[measurerFactoryKey](); + result[measurerFactoryKey] = { + measure, + estimate, + fieldIdxs: [], + }; + } + result[measurerFactoryKey].fieldIdxs.push(fieldIdx); + }; + for (let fieldIdx = 0; fieldIdx < fields.length; fieldIdx++) { const field = fields[fieldIdx]; if (shouldTextWrap(field)) { @@ -253,36 +297,11 @@ export function buildRowLineCounters(fields: Field[], typographyCtx: TypographyC const cellType = getCellOptions(field).type; if (cellType === TableCellDisplayMode.DataLinks) { - result.dataLinksCounter = result.dataLinksCounter ?? { - counter: getDataLinksCounter(), - fieldIdxs: [], - }; - result.dataLinksCounter.fieldIdxs.push(fieldIdx); + setupMeasurerForIdx(TableCellDisplayMode.DataLinks, 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, - fieldIdxs: [], - }; - result.textCounter.fieldIdxs.push(fieldIdx); + setupMeasurerForIdx(TableCellDisplayMode.Pill, fieldIdx); + } else if (field.type === FieldType.string) { + setupMeasurerForIdx(TableCellDisplayMode.Auto, fieldIdx); } } } @@ -294,10 +313,10 @@ export function buildRowLineCounters(fields: Field[], typographyCtx: TypographyC return Object.values(result); } -// in some cases, the estimator might return a value that is less than 1, but when measured by the counter, it actually +// in some cases, the estimator might return a value that is less than 1, but when calculated by the measurer, it actually // realizes that it's a multi-line cell. to avoid this, we want to give a little buffer away from 1 before we fully trust // the estimator to have told us that a cell is single-line. -export const SINGLE_LINE_ESTIMATE_THRESHOLD = 0.85; +export const SINGLE_LINE_ESTIMATE_THRESHOLD = 18.5; /** * @internal @@ -309,27 +328,25 @@ export function getRowHeight( rowIdx: number, columnWidths: number[], defaultHeight: number, - lineCounters?: LineCounterEntry[], + measurers?: MeasureCellHeightEntry[], lineHeight = TABLE.LINE_HEIGHT, - // 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 + verticalPadding = TABLE.CELL_PADDING * 2 ): number { - if (!lineCounters?.length) { + if (!measurers?.length) { return defaultHeight; } - let maxLines = -1; + let maxHeight = -1; let maxValue = ''; let maxWidth = 0; let maxField: Field | undefined; - let preciseCounter: LineCounter | undefined; + let preciseMeasurer: MeasureCellHeight | undefined; - for (const { estimate, counter, fieldIdxs } of lineCounters) { - // for some of the line counters, getting the precise count of the lines is expensive. those line counters - // set both an "estimate" and a "counter" function. if the cell we find to be the max was estimated, we will - // get the "true" value right before calculating the row height by hanging onto a reference to the counter fn. - const count = estimate ?? counter; + for (const { estimate, measure, fieldIdxs } of measurers) { + // for some of the cell height measurers, getting the precise height is expensive. those entries set + // both "estimate" and "measure" functions. if the cell we find to be the max was estimated, we will + // get the "true" value right before calculating the row height by keeping a reference to the measure fn. + const measurer = (estimate ?? measure) satisfies MeasureCellHeight; const isEstimating = estimate !== undefined; for (const fieldIdx of fieldIdxs) { @@ -338,38 +355,32 @@ export function getRowHeight( const cellValueRaw = rowIdx === -1 ? getDisplayName(field) : field.values[rowIdx]; if (cellValueRaw != null) { const colWidth = columnWidths[fieldIdx]; - const approxLines = count(cellValueRaw, colWidth, field, rowIdx); - if (approxLines > maxLines) { - maxLines = approxLines; + const estimatedHeight = measurer(cellValueRaw, colWidth, field, rowIdx, lineHeight); + if (estimatedHeight > maxHeight) { + maxHeight = estimatedHeight; maxValue = cellValueRaw; maxWidth = colWidth; maxField = field; - preciseCounter = isEstimating ? counter : undefined; + preciseMeasurer = isEstimating ? measure : undefined; } } } } // 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 (maxField === undefined || maxLines < SINGLE_LINE_ESTIMATE_THRESHOLD) { + // that the estimator correctly identified that no text wrapping is needed for this row, skipping the preciseMeasurer. + if (maxField === undefined || maxHeight < 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, maxField, rowIdx); + // the `preciseMeasurer` method to get the exact line count. + if (preciseMeasurer !== undefined) { + maxHeight = preciseMeasurer(maxValue, maxWidth, maxField, rowIdx, lineHeight); } - // 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); + // adjust for vertical padding, and clamp to a minimum default height + return Math.max(maxHeight + verticalPadding, defaultHeight); } /**