[Lens] Show validation feedback on top values out of bounds number of…

… values (elastic#110222) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
stephmilovic · Aug 31, 2021 · 3b81205 · 3b81205
1 parent c568a43
commit 3b81205
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 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
@@ -348,27 +348,19 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
     });
     return (
       <>
-        <EuiFormRow
-          label={i18n.translate('xpack.lens.indexPattern.terms.size', {
-            defaultMessage: 'Number of values',
-          })}
-          display="columnCompressed"
-          fullWidth
-        >
-          <ValuesInput
-            value={currentColumn.params.size}
-            onChange={(value) => {
-              updateLayer(
-                updateColumnParam({
-                  layer,
-                  columnId,
-                  paramName: 'size',
-                  value,
-                })
-              );
-            }}
-          />
-        </EuiFormRow>
+        <ValuesInput
+          value={currentColumn.params.size}
+          onChange={(value) => {
+            updateLayer(
+              updateColumnParam({
+                layer,
+                columnId,
+                paramName: 'size',
+                value,
+              })
+            );
+          }}
+        />
         <EuiFormRow
           label={
             <>

diff --git a/...ns/lens/public/indexpattern_datasource/operations/definitions/terms/values_input.test.tsx b/...ns/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(<ValuesInput value={5} onChange={onChangeSpy} />);
     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(<ValuesInput value={-5} onChange={jest.fn()} />);
+
+    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<HTMLInputElement>);
+    });
+    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(<ValuesInput value={123} onChange={jest.fn()} />);
+
+    function changeAndBlur(newValue: string) {
+      act(() => {
+        instance.find(EuiFieldNumber).prop('onChange')!({
+          currentTarget: { value: newValue },
+        } as React.ChangeEvent<HTMLInputElement>);
+      });
+      instance.update();
+      act(() => {
+        instance.find(EuiFieldNumber).prop('onBlur')!({} as React.FocusEvent<HTMLInputElement>);
+      });
+      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/...plugins/lens/public/indexpattern_datasource/operations/definitions/terms/values_input.tsx b/...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 (
-    <EuiFieldNumber
-      min={MIN_NUMBER_OF_VALUES}
-      max={MAX_NUMBER_OF_VALUES}
-      step={1}
-      value={inputValue}
-      compressed
-      onChange={({ currentTarget }) => setInputValue(currentTarget.value)}
-      aria-label={i18n.translate('xpack.lens.indexPattern.terms.size', {
+    <EuiFormRow
+      label={i18n.translate('xpack.lens.indexPattern.terms.size', {
         defaultMessage: 'Number of values',
       })}
-    />
+      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
+      }
+    >
+      <EuiFieldNumber
+        min={MIN_NUMBER_OF_VALUES}
+        max={MAX_NUMBER_OF_VALUES}
+        step={1}
+        value={inputValue}
+        compressed
+        isInvalid={isHigherThanMax || isLowerThanMin}
+        onChange={({ currentTarget }) => 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)))
+          );
+        }}
+      />
+    </EuiFormRow>
   );
 };