From 3b81205a23be2a6a7a4e0d0d374591ef09a22e4a Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Tue, 31 Aug 2021 15:23:18 +0200 Subject: [PATCH] [Lens] Show validation feedback on top values out of bounds number of values (#110222) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../operations/definitions/terms/index.tsx | 34 ++++------ .../definitions/terms/values_input.test.tsx | 56 +++++++++++++++- .../definitions/terms/values_input.tsx | 66 ++++++++++++++++--- 3 files changed, 123 insertions(+), 33 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx index cfe190261b53..6489a43cf2f8 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx @@ -348,27 +348,19 @@ export const termsOperation: OperationDefinition - - { - updateLayer( - updateColumnParam({ - layer, - columnId, - paramName: 'size', - value, - }) - ); - }} - /> - + { + updateLayer( + updateColumnParam({ + layer, + columnId, + paramName: 'size', + value, + }) + ); + }} + /> diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_input.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_input.test.tsx index 4303695d6e29..ac7397fb582a 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_input.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_input.test.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { shallow } from 'enzyme'; -import { EuiFieldNumber } from '@elastic/eui'; +import { EuiFieldNumber, EuiFormRow } from '@elastic/eui'; import { ValuesInput } from './values_input'; jest.mock('react-use/lib/useDebounce', () => (fn: () => void) => fn()); @@ -41,7 +41,7 @@ describe('Values', () => { expect(onChangeSpy.mock.calls[0][0]).toBe(7); }); - it('should not run onChange function on update when value is out of 1-100 range', () => { + it('should not run onChange function on update when value is out of 1-1000 range', () => { const onChangeSpy = jest.fn(); const instance = shallow(); act(() => { @@ -54,4 +54,56 @@ describe('Values', () => { expect(onChangeSpy.mock.calls.length).toBe(1); expect(onChangeSpy.mock.calls[0][0]).toBe(1000); }); + + it('should show an error message when the value is out of bounds', () => { + const instance = shallow(); + + expect(instance.find(EuiFieldNumber).prop('isInvalid')).toBeTruthy(); + expect(instance.find(EuiFormRow).prop('error')).toEqual( + expect.arrayContaining([expect.stringMatching('Value is lower')]) + ); + + act(() => { + instance.find(EuiFieldNumber).prop('onChange')!({ + currentTarget: { value: '1007' }, + } as React.ChangeEvent); + }); + instance.update(); + + expect(instance.find(EuiFieldNumber).prop('isInvalid')).toBeTruthy(); + expect(instance.find(EuiFormRow).prop('error')).toEqual( + expect.arrayContaining([expect.stringMatching('Value is higher')]) + ); + }); + + it('should fallback to last valid value on input blur', () => { + const instance = shallow(); + + function changeAndBlur(newValue: string) { + act(() => { + instance.find(EuiFieldNumber).prop('onChange')!({ + currentTarget: { value: newValue }, + } as React.ChangeEvent); + }); + instance.update(); + act(() => { + instance.find(EuiFieldNumber).prop('onBlur')!({} as React.FocusEvent); + }); + instance.update(); + } + + changeAndBlur('-5'); + + expect(instance.find(EuiFieldNumber).prop('isInvalid')).toBeFalsy(); + expect(instance.find(EuiFieldNumber).prop('value')).toBe('1'); + + changeAndBlur('5000'); + + expect(instance.find(EuiFieldNumber).prop('isInvalid')).toBeFalsy(); + expect(instance.find(EuiFieldNumber).prop('value')).toBe('1000'); + + changeAndBlur(''); + // as we're not handling the onChange state, it fallbacks to the value prop + expect(instance.find(EuiFieldNumber).prop('value')).toBe('123'); + }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_input.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_input.tsx index a4c0f8f1c50e..96b92686f762 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_input.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_input.tsx @@ -7,7 +7,7 @@ import React, { useState } from 'react'; import { i18n } from '@kbn/i18n'; -import { EuiFieldNumber } from '@elastic/eui'; +import { EuiFieldNumber, EuiFormRow } from '@elastic/eui'; import { useDebounceWithOptions } from '../../../../shared_components'; export const ValuesInput = ({ @@ -35,17 +35,63 @@ export const ValuesInput = ({ [inputValue] ); + const isEmptyString = inputValue === ''; + const isHigherThanMax = !isEmptyString && Number(inputValue) > MAX_NUMBER_OF_VALUES; + const isLowerThanMin = !isEmptyString && Number(inputValue) < MIN_NUMBER_OF_VALUES; + return ( - setInputValue(currentTarget.value)} - aria-label={i18n.translate('xpack.lens.indexPattern.terms.size', { + + display="columnCompressed" + fullWidth + isInvalid={isHigherThanMax || isLowerThanMin} + error={ + isHigherThanMax + ? [ + i18n.translate('xpack.lens.indexPattern.terms.sizeLimitMax', { + defaultMessage: + 'Value is higher than the maximum {max}, the maximum value is used instead.', + values: { + max: MAX_NUMBER_OF_VALUES, + }, + }), + ] + : isLowerThanMin + ? [ + i18n.translate('xpack.lens.indexPattern.terms.sizeLimitMin', { + defaultMessage: + 'Value is lower than the minimum {min}, the minimum value is used instead.', + values: { + min: MIN_NUMBER_OF_VALUES, + }, + }), + ] + : null + } + > + setInputValue(currentTarget.value)} + aria-label={i18n.translate('xpack.lens.indexPattern.terms.size', { + defaultMessage: 'Number of values', + })} + onBlur={() => { + if (inputValue === '') { + return setInputValue(String(value)); + } + const inputNumber = Number(inputValue); + setInputValue( + String(Math.min(MAX_NUMBER_OF_VALUES, Math.max(inputNumber, MIN_NUMBER_OF_VALUES))) + ); + }} + /> + ); };