diff --git a/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/helpers.ts b/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/helpers.ts index 21d13655421e0..4b0df91dead5f 100644 --- a/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/helpers.ts +++ b/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/helpers.ts @@ -16,7 +16,16 @@ import { getColumnByAccessor, getFormatByAccessor } from '@kbn/visualizations-pl import { getFormatService, getPaletteService } from '../services'; import { getDataBoundsForPalette } from '../utils'; -function enhanceFieldFormat(serializedFieldFormat: SerializedFieldFormat | undefined) { +export interface FormatOverrides { + number?: { alwaysShowSign?: boolean }; + percent?: { alwaysShowSign?: boolean }; + bytes?: { alwaysShowSign?: boolean }; +} + +function enhanceFieldFormat( + serializedFieldFormat: SerializedFieldFormat | undefined, + formatOverrides: FormatOverrides | undefined +): SerializedFieldFormat { const formatId = serializedFieldFormat?.id || 'number'; if (formatId === 'duration' && !serializedFieldFormat?.params?.formatOverride) { return { @@ -31,17 +40,28 @@ function enhanceFieldFormat(serializedFieldFormat: SerializedFieldFormat | undef }, }; } + if (formatOverrides && formatId in formatOverrides) { + return { + ...serializedFieldFormat, + params: { + ...serializedFieldFormat?.params, + ...formatOverrides[formatId as keyof FormatOverrides], + }, + }; + } + return serializedFieldFormat ?? { id: formatId }; } export const getMetricFormatter = ( accessor: ExpressionValueVisDimension | string, - columns: Datatable['columns'] + columns: Datatable['columns'], + formatOverrides?: FormatOverrides | undefined ) => { const type = getColumnByAccessor(accessor, columns)?.meta.type; const defaultFormat = type ? { id: type } : undefined; const serializedFieldFormat = getFormatByAccessor(accessor, columns, defaultFormat); - const enhancedFieldFormat = enhanceFieldFormat(serializedFieldFormat); + const enhancedFieldFormat = enhanceFieldFormat(serializedFieldFormat, formatOverrides); return getFormatService().deserialize(enhancedFieldFormat).getConverterFor('text'); }; diff --git a/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.test.tsx b/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.test.tsx index bee7ea66d16f7..3ad2782776881 100644 --- a/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.test.tsx +++ b/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.test.tsx @@ -9,7 +9,7 @@ import React from 'react'; import userEvent from '@testing-library/user-event'; -import { render, screen, waitFor } from '@testing-library/react'; +import { act, render, screen, waitFor } from '@testing-library/react'; import { Datatable, DatatableColumn } from '@kbn/expressions-plugin/common'; import { MetricVis, MetricVisComponentProps } from './metric_vis'; import { MetricWTrend } from '@elastic/charts'; @@ -238,8 +238,12 @@ describe('MetricVisComponent', function () { }); async function waitForChartToRender(renderComplete: MetricVisComponentProps['renderComplete']) { - // wait for 1 rAF tick (~16ms) - jest.advanceTimersByTime(30); + // Interestingly we have to wrap this into an act() call to avoid + // issues with the React scheduling when testing + await act(async () => { + // wait for 1 rAF tick (~16ms) + jest.advanceTimersByTime(30); + }); // wait for render complete callback await waitFor(() => expect(renderComplete).toHaveBeenCalled()); } diff --git a/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.tsx b/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.tsx index f1870105a26d2..0a58231b2b517 100644 --- a/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.tsx +++ b/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/metric_vis.tsx @@ -184,14 +184,13 @@ export const MetricVis = ({ title: String(title), subtitle, icon: config.metric?.icon ? getIcon(config.metric?.icon) : undefined, - extra: ({ fontSize, color }) => ( + extra: ({ color }) => ( ( + extra: ({ color }) => ( = {}) { metric: { secondaryPrefix: '' }, }} getMetricFormatter={jest.fn(() => () => formattedValue)} - fontSize={16} {...props} /> ); @@ -257,9 +256,10 @@ describe('Secondary metric', () => { it.each(trendCombinationCompareToPrimary)( '[Compare to primary] should render a badge with the trend icon "$icon" and the formatted value (rawValue: $valueFinite, baseline: $baselineFinite)', ({ value, baseline, color, icon }) => { + const getMetricFormatterMock = jest.fn(() => (v: unknown) => String(v)); renderSecondaryMetric({ row: { [id]: value }, - getMetricFormatter: jest.fn(() => (v: unknown) => String(v)), + getMetricFormatter: getMetricFormatterMock, trendConfig: { icon: showIcon, value: showValue, @@ -280,6 +280,13 @@ describe('Secondary metric', () => { const el = screen.getByTitle(badgeText); expect(el).toBeInTheDocument(); expect(el).toHaveStyle(`--euiBadgeBackgroundColor: ${color}`); + expect(getMetricFormatterMock).toHaveBeenCalledWith( + expect.any(String), + expect.any(Object), + expect.objectContaining({ + number: expect.objectContaining({ alwaysShowSign: true }), + }) + ); } if (showValue) { expect( diff --git a/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/secondary_metric.tsx b/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/secondary_metric.tsx index 07f9b55055ab8..55bae8bbf5bde 100644 --- a/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/secondary_metric.tsx +++ b/src/platform/plugins/shared/chart_expressions/expression_metric/public/components/secondary_metric.tsx @@ -15,6 +15,7 @@ import type { DatatableColumn, DatatableRow } from '@kbn/expressions-plugin/comm import { type FieldFormatConvertFunction } from '@kbn/field-formats-plugin/common'; import { type VisParams } from '@kbn/visualizations-plugin/common'; import { getColumnByAccessor } from '@kbn/visualizations-plugin/common/utils'; +import { FormatOverrides } from './helpers'; export interface TrendConfig { icon: boolean; @@ -119,14 +120,12 @@ function SecondaryMetricValue({ formattedValue, trendConfig, color, - fontSize, formatter, }: { rawValue?: number | string; formattedValue?: string; trendConfig?: TrendConfig; color?: string; - fontSize: number; formatter?: FieldFormatConvertFunction; }) { const safeFormattedValue = formattedValue ?? notAvailable; @@ -152,6 +151,15 @@ function SecondaryMetricValue({ formatter, trendConfig.compareToPrimary ); + // If no value is shown and no icon should be shown (i.e. N/A) then do not render the badge at all + if (trendConfig.icon && !trendConfig.value && !icon) { + return ( + + ); + } return ( ; trendConfig?: TrendConfig; color?: string; - fontSize: number; - getMetricFormatter: (accessor: string, columns: DatatableColumn[]) => FieldFormatConvertFunction; + getMetricFormatter: ( + accessor: string, + columns: DatatableColumn[], + formatOverrides?: FormatOverrides | undefined + ) => FieldFormatConvertFunction; } function getMetricColumnAndFormatter( columns: SecondaryMetricProps['columns'], config: SecondaryMetricProps['config'], - getMetricFormatter: SecondaryMetricProps['getMetricFormatter'] + getMetricFormatter: SecondaryMetricProps['getMetricFormatter'], + formatOverrides: FormatOverrides | undefined ) { if (!config.dimensions.secondaryMetric) { return; } return { metricColumn: getColumnByAccessor(config.dimensions.secondaryMetric, columns), - metricFormatter: getMetricFormatter(config.dimensions.secondaryMetric, columns), + metricFormatter: getMetricFormatter( + config.dimensions.secondaryMetric, + columns, + formatOverrides + ), + }; +} + +function getEnhancedNumberSignFormatter( + trendConfig: TrendConfig | undefined +): FormatOverrides | undefined { + if (!trendConfig?.compareToPrimary) { + return; + } + const paramsOverride = { alwaysShowSign: true }; + return { + number: paramsOverride, + percent: paramsOverride, + bytes: paramsOverride, }; } @@ -214,10 +244,14 @@ export function SecondaryMetric({ getMetricFormatter, trendConfig, color, - fontSize, }: SecondaryMetricProps) { const { metricFormatter, metricColumn } = - getMetricColumnAndFormatter(columns, config, getMetricFormatter) || {}; + getMetricColumnAndFormatter( + columns, + config, + getMetricFormatter, + getEnhancedNumberSignFormatter(trendConfig) + ) || {}; const prefix = config.metric.secondaryPrefix ?? metricColumn?.name; const value = metricColumn ? row[metricColumn.id] : undefined; @@ -230,7 +264,6 @@ export function SecondaryMetric({ formattedValue={metricFormatter?.(value)} trendConfig={color ? undefined : trendConfig} color={color} - fontSize={fontSize} formatter={metricFormatter} /> diff --git a/src/platform/plugins/shared/field_formats/common/converters/numeral.ts b/src/platform/plugins/shared/field_formats/common/converters/numeral.ts index 100898fd558b6..6c74153fca36b 100644 --- a/src/platform/plugins/shared/field_formats/common/converters/numeral.ts +++ b/src/platform/plugins/shared/field_formats/common/converters/numeral.ts @@ -30,7 +30,9 @@ export abstract class NumeralFormat extends FieldFormat { abstract title: string; getParamDefaults = () => ({ - pattern: this.getConfig!(`format:${this.id}:defaultPattern`), + // While this should be always defined, it is not guaranteed in testing that the function is available + pattern: this.getConfig?.(`format:${this.id}:defaultPattern`), + alwaysShowSign: false, }); protected getConvertedValue(val: number | string | object): string { @@ -50,7 +52,12 @@ export abstract class NumeralFormat extends FieldFormat { (this.getConfig && this.getConfig(FORMATS_UI_SETTINGS.FORMAT_NUMBER_DEFAULT_LOCALE)) || 'en'; numeral.language(defaultLocale); - const formatted = numeralInst.set(val).format(this.param('pattern')); + let pattern: string = this.param('pattern'); + if (pattern && this.param('alwaysShowSign')) { + pattern = pattern.startsWith('+') || val === 0 ? pattern : `+ ${pattern}`; + } + + const formatted = numeralInst.set(val).format(pattern); numeral.language(previousLocale); diff --git a/src/platform/plugins/shared/field_formats/common/converters/percent.ts b/src/platform/plugins/shared/field_formats/common/converters/percent.ts index c091ba1866d5d..43cbd5e1216e3 100644 --- a/src/platform/plugins/shared/field_formats/common/converters/percent.ts +++ b/src/platform/plugins/shared/field_formats/common/converters/percent.ts @@ -26,6 +26,7 @@ export class PercentFormat extends NumeralFormat { getParamDefaults = () => ({ pattern: this.getConfig!(FORMATS_UI_SETTINGS.FORMAT_PERCENT_DEFAULT_PATTERN), fractional: true, + alwaysShowSign: false, }); textConvert: TextContextTypeConvert = (val: string | number) => {