Skip to content

Commit

Permalink
[Lens] Fix default formatter for gauge charts (#179473)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nickofthyme
nickofthyme authored Apr 9, 2024
1 parent 7194ca2 commit 1d5a77a
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 75 deletions.
172 changes: 137 additions & 35 deletions src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { ColorStop } from '@kbn/coloring';
import { chartPluginMock } from '@kbn/charts-plugin/public/mocks';
import { fieldFormatsServiceMock } from '@kbn/field-formats-plugin/public/mocks';
import type { Datatable } from '@kbn/expressions-plugin/public';
import { DatatableColumn, DatatableRow } from '@kbn/expressions-plugin/common';
import { DatatableColumn, DatatableColumnMeta, DatatableRow } from '@kbn/expressions-plugin/common';
import { shallowWithIntl } from '@kbn/test-jest-helpers';
import {
GaugeRenderProps,
Expand All @@ -29,15 +29,7 @@ import {
ColorBandSimpleConfig,
Color,
} from '@elastic/charts';

jest.mock('@elastic/charts', () => {
const original = jest.requireActual('@elastic/charts');

return {
...original,
getSpecId: jest.fn(() => {}),
};
});
import { ExpressionValueVisDimension } from '@kbn/visualizations-plugin/common';

const numberColumn = (id = 'metric-accessor'): DatatableColumn => ({
id,
Expand All @@ -51,17 +43,8 @@ const numberColumn = (id = 'metric-accessor'): DatatableColumn => ({
},
});

jest.mock('@elastic/charts', () => {
const original = jest.requireActual('@elastic/charts');

return {
...original,
getSpecId: jest.fn(() => {}),
};
});

const chartsThemeService = chartPluginMock.createSetupContract().theme;
const paletteThemeService = chartPluginMock.createSetupContract().palettes;
const { theme: chartsThemeService, palettes: paletteThemeService } =
chartPluginMock.createSetupContract();
const formatService = fieldFormatsServiceMock.createStartContract();
const args: GaugeArguments = {
labelMajor: 'Gauge',
Expand Down Expand Up @@ -113,13 +96,17 @@ describe('GaugeComponent', function () {
};
});

beforeEach(() => {
jest.clearAllMocks();
});

it('renders the chart', () => {
const component = shallowWithIntl(<GaugeComponent {...wrapperProps} />);
expect(component.find(Chart)).toMatchSnapshot();
});

it('returns null when metric is not provided', async () => {
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand All @@ -134,7 +121,7 @@ describe('GaugeComponent', function () {
});

it('shows empty placeholder when minimum accessor equals maximum accessor', async () => {
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand All @@ -148,7 +135,7 @@ describe('GaugeComponent', function () {
expect(component.find('EmptyPlaceholder')).toHaveLength(1);
});
it('shows empty placeholder when minimum accessor value is greater maximum accessor value', async () => {
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand All @@ -162,7 +149,7 @@ describe('GaugeComponent', function () {
expect(component.find('EmptyPlaceholder')).toHaveLength(1);
});
it('when metric value is bigger than max, it takes maximum value', () => {
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand All @@ -180,7 +167,7 @@ describe('GaugeComponent', function () {

describe('labelMajor and labelMinor settings', () => {
it('displays no labelMajor and no labelMinor when no passed', () => {
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand All @@ -194,7 +181,7 @@ describe('GaugeComponent', function () {
expect(datum?.subtitle).toEqual('');
});
it('displays custom labelMajor and labelMinor when passed', () => {
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand All @@ -209,7 +196,7 @@ describe('GaugeComponent', function () {
expect(datum?.subtitle).toEqual('custom labelMinor');
});
it('displays auto labelMajor', () => {
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand Down Expand Up @@ -237,7 +224,7 @@ describe('GaugeComponent', function () {
rangeMax: 4,
},
};
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand Down Expand Up @@ -267,7 +254,7 @@ describe('GaugeComponent', function () {
rangeMax: 4,
},
};
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand Down Expand Up @@ -295,7 +282,7 @@ describe('GaugeComponent', function () {
rangeMax: 100,
},
};
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand Down Expand Up @@ -323,7 +310,7 @@ describe('GaugeComponent', function () {
rangeMax: 10,
},
};
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand Down Expand Up @@ -351,7 +338,7 @@ describe('GaugeComponent', function () {
rangeMax: 30,
},
};
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand Down Expand Up @@ -379,7 +366,7 @@ describe('GaugeComponent', function () {
rangeMax: 10,
},
};
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand Down Expand Up @@ -409,7 +396,7 @@ describe('GaugeComponent', function () {
rangeMax: 100,
},
};
const customProps = {
const customProps: GaugeRenderProps = {
...wrapperProps,
args: {
...wrapperProps.args,
Expand Down Expand Up @@ -442,4 +429,119 @@ describe('GaugeComponent', function () {
expect(settingsComponent.prop('ariaUseDefaultSummary')).toEqual(true);
});
});

describe('formatting', () => {
let baseFormattingProps: GaugeRenderProps;
const metricFormat: ExpressionValueVisDimension['format'] = {
id: 'bytes',
};
const tableFormatParams = {
id: 'number',
params: {
pattern: '0,00000',
},
};
const metricMetaParams: DatatableColumnMeta = {
type: 'number',
params: {
id: 'test',
params: {
pattern: '000,000.00',
},
},
};
const createColumnsWithMetricParams = (params?: DatatableColumnMeta['params']) =>
wrapperProps.data.columns.map((c) =>
c.id !== 'metric-accessor'
? c
: {
...c,
meta: {
...c.meta,
params,
},
}
);

beforeAll(() => {
baseFormattingProps = {
...wrapperProps,
args: {
...wrapperProps.args,
metric: {
...(wrapperProps.args.metric as ExpressionValueVisDimension),
accessor: {
id: 'metric-accessor',
name: 'metric-accessor',
meta: metricMetaParams,
},
format: metricFormat,
},
},
data: {
...wrapperProps.data,
columns: createColumnsWithMetricParams(tableFormatParams),
},
};
});

it('should use custom metric format params, if provided', () => {
shallowWithIntl(<GaugeComponent {...baseFormattingProps} />);
expect(formatService.deserialize).toBeCalledWith(metricFormat);
});

it('should use table metric format params, if provided', () => {
const customProps: GaugeRenderProps = {
...baseFormattingProps,
args: {
...baseFormattingProps.args,
metric: 'metric-accessor',
},
};
shallowWithIntl(<GaugeComponent {...customProps} />);
expect(formatService.deserialize).toBeCalledWith(tableFormatParams);
});

it('should use default metric format params, if no others provided', () => {
const testParams = {
id: 'test',
};
const customProps: GaugeRenderProps = {
...baseFormattingProps,
args: {
...baseFormattingProps.args,
metric: 'metric-accessor',
},
data: {
...baseFormattingProps.data,
columns: createColumnsWithMetricParams(testParams),
},
};

shallowWithIntl(<GaugeComponent {...customProps} />);
expect(formatService.deserialize).toBeCalledWith(testParams);
});

it('should use fallback if no default metric format and no others provided', () => {
const customProps: GaugeRenderProps = {
...baseFormattingProps,
args: {
...baseFormattingProps.args,
metric: 'metric-accessor',
},
data: {
...wrapperProps.data,
columns: createColumnsWithMetricParams(),
},
};

shallowWithIntl(<GaugeComponent {...customProps} />);
expect(formatService.deserialize).toBeCalledWith({
id: 'number',
params: {
pattern: '0,0.0',
},
});
});
});
});
11 changes: 8 additions & 3 deletions src/plugins/chart_expressions/expression_gauge/public/components/gauge_component.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import { FieldFormat } from '@kbn/field-formats-plugin/common';
import type { CustomPaletteState } from '@kbn/charts-plugin/public';
import { EmptyPlaceholder } from '@kbn/charts-plugin/public';
import { getOverridesFor } from '@kbn/chart-expressions-common';
import { findAccessor, isVisDimension } from '@kbn/visualizations-plugin/common/utils';
import {
findAccessor,
getFormatByAccessor,
isVisDimension,
} from '@kbn/visualizations-plugin/common/utils';
import { i18n } from '@kbn/i18n';
import {
GaugeRenderProps,
Expand Down Expand Up @@ -307,10 +311,11 @@ export const GaugeComponent: FC<GaugeRenderProps> = ({
? metricColumn?.meta?.params
: undefined;

const defaultMetricFormatParams = {
const defaultMetricFormatParams = (args.metric &&
getFormatByAccessor(args.metric, data.columns)) || {
id: 'number',
params: {
pattern: max - min > 5 ? `0,0` : `0,0.0`,
pattern: max - min > 5 ? '0,0' : '0,0.0',
},
};

Expand Down
9 changes: 0 additions & 9 deletions ...plugins/chart_expressions/expression_heatmap/public/components/heatmap_component.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,6 @@ import HeatmapComponent from './heatmap_component';
import { LegendSize } from '@kbn/visualizations-plugin/common';
import { FieldFormat } from '@kbn/field-formats-plugin/common';

jest.mock('@elastic/charts', () => {
const original = jest.requireActual('@elastic/charts');

return {
...original,
getSpecId: jest.fn(() => {}),
};
});

const actWithTimeout = (action: Function, timer: number = 1) =>
act(
() =>
Expand Down
9 changes: 0 additions & 9 deletions ...t_expressions/expression_partition_vis/public/components/partition_vis_component.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,6 @@ import { ChartTypes } from '../../common/types';
import { LegendSize } from '@kbn/visualizations-plugin/common';
import { cloneDeep } from 'lodash';

jest.mock('@elastic/charts', () => {
const original = jest.requireActual('@elastic/charts');

return {
...original,
getSpecId: jest.fn(() => {}),
};
});

const actWithTimeout = (action: Function, timer: number = 1) =>
act(
() =>
Expand Down
9 changes: 0 additions & 9 deletions ...plugins/chart_expressions/expression_partition_vis/public/utils/get_color_picker.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,6 @@ import { createMockBucketColumns, createMockVisData } from '../mocks';
const bucketColumns = createMockBucketColumns();
const visData = createMockVisData();

jest.mock('@elastic/charts', () => {
const original = jest.requireActual('@elastic/charts');

return {
...original,
getSpecId: jest.fn(() => {}),
};
});

describe('LegendColorPickerWrapper', () => {
const mockState = new Map();
const uiState = {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/visualizations/common/utils/accessors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export function getFormatByAccessor(
defaultColumnFormat?: SerializedFieldFormat
): SerializedFieldFormat | undefined {
return typeof dimension === 'string'
? getColumnByAccessor(dimension, columns)?.meta.params || defaultColumnFormat
? getColumnByAccessor(dimension, columns)?.meta?.params || defaultColumnFormat
: dimension.format || defaultColumnFormat;
}

Expand Down
Loading

0 comments on commit 1d5a77a

Please sign in to comment.