[Lens] Text based languages fixes and UX improvements (#144306)

* [Lens] Fixes wrong label color on field duplication

* Update meta information on selected field update

* Mark as incompatible all the non numeric fields for the metrics fields selection

* Restrict dropping non mumeric fields on metric dimensions

* Update functional tests
This commit is contained in:
Stratoula Kalafateli 2022-11-03 08:44:53 +02:00 committed by GitHub
parent 9e58af54a8
commit a6ab758e5f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 173 additions and 29 deletions

View file

@ -6,11 +6,9 @@
*/
import React from 'react';
import type { DatatableColumn } from '@kbn/expressions-plugin/public';
import { FieldPicker, FieldOptionValue } from '../../shared_components/field_picker';
import { FieldSelect, FieldSelectProps } from './field_select';
import { type FieldOptionCompatible, FieldSelect, FieldSelectProps } from './field_select';
import { shallowWithIntl as shallow } from '@kbn/test-jest-helpers';
const fields = [
@ -20,6 +18,7 @@ const fields = [
meta: {
type: 'date',
},
compatible: true,
},
{
name: 'bytes',
@ -27,6 +26,7 @@ const fields = [
meta: {
type: 'number',
},
compatible: true,
},
{
name: 'memory',
@ -34,8 +34,9 @@ const fields = [
meta: {
type: 'number',
},
compatible: true,
},
] as DatatableColumn[];
] as FieldOptionCompatible[];
describe('Layer Data Panel', () => {
let defaultProps: FieldSelectProps;
@ -75,7 +76,7 @@ describe('Layer Data Panel', () => {
label: 'Available fields',
options: [
{
compatible: true,
compatible: 1,
exists: true,
label: 'timestamp',
value: {
@ -85,7 +86,7 @@ describe('Layer Data Panel', () => {
},
},
{
compatible: true,
compatible: 1,
exists: true,
label: 'bytes',
value: {
@ -95,7 +96,7 @@ describe('Layer Data Panel', () => {
},
},
{
compatible: true,
compatible: 1,
exists: true,
label: 'memory',
value: {

View file

@ -13,10 +13,14 @@ import { FieldPicker, FieldOptionValue, FieldOption } from '../../shared_compone
import type { TextBasedLayerColumn } from './types';
import type { DataType } from '../../types';
export interface FieldOptionCompatible extends DatatableColumn {
compatible: boolean;
}
export interface FieldSelectProps extends EuiComboBoxProps<EuiComboBoxOptionOption['value']> {
selectedField?: TextBasedLayerColumn;
onChoose: (choice: FieldOptionValue) => void;
existingFields: DatatableColumn[];
existingFields: FieldOptionCompatible[];
}
export function FieldSelect({
@ -26,19 +30,21 @@ export function FieldSelect({
['data-test-subj']: dataTestSub,
}: FieldSelectProps) {
const memoizedFieldOptions = useMemo(() => {
const availableFields = existingFields.map((field) => {
const dataType = field?.meta?.type as DataType;
return {
compatible: true,
exists: true,
label: field.name,
value: {
type: 'field' as FieldOptionValue['type'],
field: field.name,
dataType,
},
};
});
const availableFields = existingFields
.map((field) => {
const dataType = field?.meta?.type as DataType;
return {
compatible: field.compatible ? 1 : 0,
exists: true,
label: field.name,
value: {
type: 'field' as FieldOptionValue['type'],
field: field.name,
dataType,
},
};
})
.sort((a, b) => b.compatible - a.compatible);
return [
{
label: i18n.translate('xpack.lens.indexPattern.availableFieldsLabel', {

View file

@ -77,7 +77,7 @@ const expectedIndexPatterns = {
const indexPatterns = expectedIndexPatterns;
describe('IndexPattern Data Source', () => {
describe('Textbased Data Source', () => {
let baseState: TextBasedPrivateState;
let TextBasedDatasource: Datasource<TextBasedPrivateState, TextBasedPersistedState>;
@ -189,6 +189,88 @@ describe('IndexPattern Data Source', () => {
});
});
describe('#getDropProps', () => {
it('should return undefined if source is not present', () => {
const props = {
target: {
layerId: 'a',
groupId: 'groupId',
columnId: 'col1',
filterOperations: jest.fn(),
},
state: baseState,
indexPatterns,
};
expect(TextBasedDatasource.getDropProps(props)).toBeUndefined();
});
it('should return undefined if target group not allows non numeric fields', () => {
const newState = {
...baseState,
layers: {
a: {
columns: [],
allColumns: [
{
columnId: 'col1',
fieldName: 'Test 1',
meta: {
type: 'string',
},
},
],
query: { sql: 'SELECT * FROM foo' },
index: 'foo',
},
},
} as unknown as TextBasedPrivateState;
const props = {
target: {
layerId: 'a',
groupId: 'groupId',
columnId: 'col1',
filterOperations: jest.fn(),
isMetricDimension: true,
},
source: {
id: 'col1',
field: 'Test 1',
humanData: {
label: 'Test 1',
},
},
state: newState,
indexPatterns,
};
expect(TextBasedDatasource.getDropProps(props)).toBeUndefined();
});
it('should return props if field is allowed to be dropped', () => {
const props = {
target: {
layerId: 'a',
groupId: 'groupId',
columnId: 'col1',
filterOperations: jest.fn(),
isMetricDimension: true,
},
source: {
id: 'col1',
field: 'Test 1',
humanData: {
label: 'Test 1',
},
},
state: baseState,
indexPatterns,
};
expect(TextBasedDatasource.getDropProps(props)).toStrictEqual({
dropTypes: ['field_add'],
nextLabel: 'Test 1',
});
});
});
describe('#insertLayer', () => {
it('should insert an empty layer into the previous state', () => {
expect(TextBasedDatasource.insertLayer(baseState, 'newLayer')).toEqual({

View file

@ -383,11 +383,9 @@ export function getTextBasedDatasource({
customLabel = selectedField?.fieldName;
}
const columnExists = props.state.fieldList.some((f) => f.name === customLabel);
render(
<EuiButtonEmpty
color={columnExists ? 'primary' : 'danger'}
color={customLabel ? 'primary' : 'danger'}
onClick={() => {}}
data-test-subj="lns-dimensionTrigger-textBased"
>
@ -412,6 +410,19 @@ export function getTextBasedDatasource({
const selectedField = props.state.layers[props.layerId]?.allColumns?.find(
(column) => column.columnId === props.columnId
);
const updatedFields = fields.map((f) => {
return {
...f,
compatible: props.isMetricDimension
? props.filterOperations({
dataType: f.meta.type as DataType,
isBucketed: Boolean(f?.meta?.type !== 'number'),
scale: 'ordinal',
})
: true,
};
});
render(
<KibanaThemeProvider theme$={core.theme.theme$}>
<EuiFormRow
@ -423,7 +434,7 @@ export function getTextBasedDatasource({
className="lnsIndexPatternDimensionEditor--padded"
>
<FieldSelect
existingFields={fields}
existingFields={updatedFields}
selectedField={selectedField}
onChoose={(choice) => {
const meta = fields.find((f) => f.name === choice.field)?.meta;
@ -457,12 +468,12 @@ export function getTextBasedDatasource({
columns: props.state.layers[props.layerId].columns.map((col) =>
col.columnId !== props.columnId
? col
: { ...col, fieldName: choice.field }
: { ...col, fieldName: choice.field, meta }
),
allColumns: props.state.layers[props.layerId].allColumns.map((col) =>
col.columnId !== props.columnId
? col
: { ...col, fieldName: choice.field }
: { ...col, fieldName: choice.field, meta }
),
},
},
@ -522,10 +533,16 @@ export function getTextBasedDatasource({
},
getDropProps: (props) => {
const { source } = props;
const { source, target, state } = props;
if (!source) {
return;
}
if (target && target.isMetricDimension) {
const layerId = target.layerId;
const currentLayer = state.layers[layerId];
const field = currentLayer.allColumns.find((f) => f.columnId === source.id);
if (field?.meta?.type !== 'number') return;
}
const label = source.field as string;
return { dropTypes: ['field_add'], nextLabel: label };
},

View file

@ -605,6 +605,7 @@ export function LayerPanel(
filterOperations: group.filterOperations,
prioritizedOperation: group.prioritizedOperation,
isNewColumn: true,
isMetricDimension: group?.isMetricDimension,
indexPatternId: layerDatasource
? layerDatasource.getUsedDataView(layerDatasourceState, layerId)
: activeVisualization.getUsedDataView?.(visualizationState, layerId),
@ -717,6 +718,7 @@ export function LayerPanel(
groupId: activeGroup.groupId,
hideGrouping: activeGroup.hideGrouping,
filterOperations: activeGroup.filterOperations,
isMetricDimension: activeGroup?.isMetricDimension,
dimensionGroups,
toggleFullscreen,
isFullscreen,

View file

@ -612,6 +612,7 @@ export type DatasourceDimensionEditorProps<T = unknown> = DatasourceDimensionPro
dimensionGroups: VisualizationDimensionGroupConfig[];
toggleFullscreen: () => void;
isFullscreen: boolean;
isMetricDimension?: boolean;
layerType: LayerType | undefined;
supportStaticValue: boolean;
paramEditorCustomProps?: ParamEditorCustomProps;
@ -636,6 +637,7 @@ export interface DragDropOperation {
filterOperations: (operation: OperationMetadata) => boolean;
indexPatternId?: string;
isNewColumn?: boolean;
isMetricDimension?: boolean;
prioritizedOperation?: string;
}
@ -793,6 +795,8 @@ export type VisualizationDimensionGroupConfig = SharedDimensionProps & {
// need a special flag to know when to pass the previous column on duplicating
requiresPreviousColumnOnDuplicate?: boolean;
supportStaticValue?: boolean;
// used by text based datasource to restrict the field selection only to number fields for the metric dimensions
isMetricDimension?: boolean;
paramEditorCustomProps?: ParamEditorCustomProps;
enableFormatSelector?: boolean;
formatSelectorOptions?: FormatSelectorOptions; // only relevant if supportFieldFormat is true

View file

@ -299,6 +299,7 @@ export const getDatatableVisualization = ({
}),
supportsMoreColumns: true,
filterOperations: (op) => !op.isBucketed,
isMetricDimension: true,
requiredMinDimensionCount: 1,
dataTestSubj: 'lnsDatatable_metrics',
enableDimensionEditor: true,

View file

@ -102,6 +102,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.METRIC,
groupLabel: 'Metric',
isMetricDimension: true,
accessors: [{ columnId: 'metric-accessor', triggerIcon: 'none' }],
filterOperations: isNumericDynamicMetric,
supportsMoreColumns: false,
@ -118,6 +119,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.MIN,
groupLabel: 'Minimum value',
isMetricDimension: true,
accessors: [{ columnId: 'min-accessor' }],
filterOperations: isNumericMetric,
supportsMoreColumns: false,
@ -135,6 +137,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.MAX,
groupLabel: 'Maximum value',
isMetricDimension: true,
accessors: [{ columnId: 'max-accessor' }],
filterOperations: isNumericMetric,
supportsMoreColumns: false,
@ -152,6 +155,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.GOAL,
groupLabel: 'Goal value',
isMetricDimension: true,
accessors: [{ columnId: 'goal-accessor' }],
filterOperations: isNumericMetric,
supportsMoreColumns: false,
@ -184,6 +188,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.METRIC,
groupLabel: 'Metric',
isMetricDimension: true,
accessors: [],
filterOperations: isNumericDynamicMetric,
supportsMoreColumns: true,
@ -200,6 +205,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.MIN,
groupLabel: 'Minimum value',
isMetricDimension: true,
accessors: [{ columnId: 'min-accessor' }],
filterOperations: isNumericMetric,
supportsMoreColumns: false,
@ -217,6 +223,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.MAX,
groupLabel: 'Maximum value',
isMetricDimension: true,
accessors: [],
filterOperations: isNumericMetric,
supportsMoreColumns: true,
@ -234,6 +241,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.GOAL,
groupLabel: 'Goal value',
isMetricDimension: true,
accessors: [],
filterOperations: isNumericMetric,
supportsMoreColumns: true,
@ -272,6 +280,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.METRIC,
groupLabel: 'Metric',
isMetricDimension: true,
accessors: [{ columnId: 'metric-accessor', triggerIcon: 'none' }],
filterOperations: isNumericDynamicMetric,
supportsMoreColumns: false,
@ -288,6 +297,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.MIN,
groupLabel: 'Minimum value',
isMetricDimension: true,
accessors: [{ columnId: 'min-accessor' }],
filterOperations: isNumericMetric,
supportsMoreColumns: false,
@ -305,6 +315,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.MAX,
groupLabel: 'Maximum value',
isMetricDimension: true,
accessors: [{ columnId: 'max-accessor' }],
filterOperations: isNumericMetric,
supportsMoreColumns: false,
@ -322,6 +333,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.GOAL,
groupLabel: 'Goal value',
isMetricDimension: true,
accessors: [{ columnId: 'goal-accessor' }],
filterOperations: isNumericMetric,
supportsMoreColumns: false,
@ -365,6 +377,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.METRIC,
groupLabel: 'Metric',
isMetricDimension: true,
accessors: [{ columnId: 'metric-accessor', triggerIcon: 'none' }],
filterOperations: isNumericDynamicMetric,
supportsMoreColumns: false,
@ -381,6 +394,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.MIN,
groupLabel: 'Minimum value',
isMetricDimension: true,
accessors: [{ columnId: 'min-accessor' }],
filterOperations: isNumericMetric,
supportsMoreColumns: false,
@ -400,6 +414,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.MAX,
groupLabel: 'Maximum value',
isMetricDimension: true,
accessors: [{ columnId: 'max-accessor' }],
filterOperations: isNumericMetric,
supportsMoreColumns: false,
@ -419,6 +434,7 @@ describe('gauge', () => {
},
groupId: GROUP_ID.GOAL,
groupLabel: 'Goal value',
isMetricDimension: true,
accessors: [{ columnId: 'goal-accessor' }],
filterOperations: isNumericMetric,
supportsMoreColumns: false,

View file

@ -244,6 +244,7 @@ export const getGaugeVisualization = ({
defaultMessage: 'Value',
}),
},
isMetricDimension: true,
accessors: metricAccessor
? [
palette
@ -282,6 +283,7 @@ export const getGaugeVisualization = ({
defaultMessage: 'Value',
}),
},
isMetricDimension: true,
accessors: state.minAccessor ? [{ columnId: state.minAccessor }] : [],
filterOperations: isNumericMetric,
supportsMoreColumns: !state.minAccessor,
@ -308,6 +310,7 @@ export const getGaugeVisualization = ({
defaultMessage: 'Value',
}),
},
isMetricDimension: true,
accessors: state.maxAccessor ? [{ columnId: state.maxAccessor }] : [],
filterOperations: isNumericMetric,
supportsMoreColumns: !state.maxAccessor,
@ -334,6 +337,7 @@ export const getGaugeVisualization = ({
defaultMessage: 'Value',
}),
},
isMetricDimension: true,
accessors: state.goalAccessor ? [{ columnId: state.goalAccessor }] : [],
filterOperations: isNumericMetric,
supportsMoreColumns: !state.goalAccessor,

View file

@ -156,6 +156,7 @@ describe('heatmap', () => {
},
groupId: GROUP_ID.CELL,
groupLabel: 'Cell value',
isMetricDimension: true,
accessors: [
{
columnId: 'v-accessor',
@ -214,6 +215,7 @@ describe('heatmap', () => {
},
groupId: GROUP_ID.CELL,
groupLabel: 'Cell value',
isMetricDimension: true,
accessors: [],
filterOperations: isCellValueSupported,
supportsMoreColumns: true,
@ -270,6 +272,7 @@ describe('heatmap', () => {
},
groupId: GROUP_ID.CELL,
groupLabel: 'Cell value',
isMetricDimension: true,
accessors: [
{
columnId: 'v-accessor',

View file

@ -229,6 +229,7 @@ export const getHeatmapVisualization = ({
]
: [],
filterOperations: isCellValueSupported,
isMetricDimension: true,
supportsMoreColumns: !state.valueAccessor,
enableDimensionEditor: true,
requiredMinDimensionCount: 1,

View file

@ -223,6 +223,7 @@ export const getLegacyMetricVisualization = ({
defaultMessage: 'Value',
}),
},
isMetricDimension: true,
groupLabel: i18n.translate('xpack.lens.metric.label', {
defaultMessage: 'Metric',
}),

View file

@ -20,6 +20,7 @@ Object {
},
"groupId": "metric",
"groupLabel": "Primary metric",
"isMetricDimension": true,
"paramEditorCustomProps": Object {
"headingLabel": "Value",
},
@ -41,6 +42,7 @@ Object {
},
"groupId": "secondaryMetric",
"groupLabel": "Secondary metric",
"isMetricDimension": true,
"paramEditorCustomProps": Object {
"headingLabel": "Value",
},

View file

@ -141,6 +141,7 @@ const getMetricLayerConfiguration = (
: [],
supportsMoreColumns: !props.state.metricAccessor,
filterOperations: isSupportedDynamicMetric,
isMetricDimension: true,
enableDimensionEditor: true,
enableFormatSelector: true,
formatSelectorOptions: formatterOptions,
@ -166,6 +167,7 @@ const getMetricLayerConfiguration = (
: [],
supportsMoreColumns: !props.state.secondaryMetricAccessor,
filterOperations: isSupportedDynamicMetric,
isMetricDimension: true,
enableDimensionEditor: true,
enableFormatSelector: true,
formatSelectorOptions: formatterOptions,

View file

@ -272,6 +272,7 @@ export const getPieVisualization = ({
groupLabel: i18n.translate('xpack.lens.pie.groupsizeLabel', {
defaultMessage: 'Size by',
}),
isMetricDimension: true,
dimensionEditorGroupLabel: i18n.translate('xpack.lens.pie.groupSizeLabel', {
defaultMessage: 'Size',
}),

View file

@ -347,6 +347,7 @@ export const getXyVisualization = ({
groupLabel: getAxisName('y', { isHorizontal }),
accessors: mappedAccessors,
filterOperations: isNumericDynamicMetric,
isMetricDimension: true,
supportsMoreColumns: true,
requiredMinDimensionCount: 1,
dataTestSubj: 'lnsXY_yDimensionPanel',