Skip to content

Commit

Permalink
[Lens] Don't allow values outside of range for number of top values (e…
Browse files Browse the repository at this point in the history
  • Loading branch information
mbondyra authored Oct 1, 2020
1 parent 90d9334 commit 827b4a5
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,13 @@

import React from 'react';
import { i18n } from '@kbn/i18n';
import { EuiFormRow, EuiRange, EuiSelect } from '@elastic/eui';
import { IndexPatternColumn } from '../../indexpattern';
import { updateColumnParam } from '../../state_helpers';
import { DataType } from '../../../types';
import { OperationDefinition } from './index';
import { FieldBasedIndexPatternColumn } from './column_types';

type PropType<C> = C extends React.ComponentType<infer P> ? P : unknown;

// Add ticks to EuiRange component props
const FixedEuiRange = (EuiRange as unknown) as React.ComponentType<
PropType<typeof EuiRange> & {
ticks?: Array<{
label: string;
value: number;
}>;
}
>;
import { EuiFormRow, EuiSelect } from '@elastic/eui';
import { IndexPatternColumn } from '../../../indexpattern';
import { updateColumnParam } from '../../../state_helpers';
import { DataType } from '../../../../types';
import { OperationDefinition } from '../index';
import { FieldBasedIndexPatternColumn } from '../column_types';
import { ValuesRangeInput } from './values_range_input';

function ofName(name: string) {
return i18n.translate('xpack.lens.indexPattern.termsOf', {
Expand Down Expand Up @@ -182,30 +171,19 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
display="columnCompressed"
fullWidth
>
<FixedEuiRange
min={1}
max={20}
step={1}
<ValuesRangeInput
value={currentColumn.params.size}
showInput
showLabels
compressed
onChange={(
e: React.ChangeEvent<HTMLInputElement> | React.MouseEvent<HTMLButtonElement>
) =>
onChange={(value) => {
setState(
updateColumnParam({
state,
layerId,
currentColumn,
paramName: 'size',
value: Number((e.target as HTMLInputElement).value),
value,
})
)
}
aria-label={i18n.translate('xpack.lens.indexPattern.terms.size', {
defaultMessage: 'Number of values',
})}
);
}}
/>
</EuiFormRow>
<EuiFormRow
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,17 @@
*/

import React from 'react';
import { shallow } from 'enzyme';
import { act } from 'react-dom/test-utils';
import { shallow, mount } from 'enzyme';
import { EuiRange, EuiSelect } from '@elastic/eui';
import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public';
import { IStorageWrapper } from 'src/plugins/kibana_utils/public';
import { dataPluginMock } from '../../../../../../../src/plugins/data/public/mocks';
import { createMockedIndexPattern } from '../../mocks';
import { TermsIndexPatternColumn } from './terms';
import { termsOperation } from './index';
import { IndexPatternPrivateState, IndexPattern } from '../../types';
import { dataPluginMock } from '../../../../../../../../src/plugins/data/public/mocks';
import { createMockedIndexPattern } from '../../../mocks';
import { ValuesRangeInput } from './values_range_input';
import { TermsIndexPatternColumn } from '.';
import { termsOperation } from '../index';
import { IndexPatternPrivateState, IndexPattern } from '../../../types';

const defaultProps = {
storage: {} as IStorageWrapper,
Expand Down Expand Up @@ -479,7 +481,7 @@ describe('terms', () => {

it('should render current size value', () => {
const setStateSpy = jest.fn();
const instance = shallow(
const instance = mount(
<InlineOptions
{...defaultProps}
state={state}
Expand All @@ -490,12 +492,12 @@ describe('terms', () => {
/>
);

expect(instance.find(EuiRange).prop('value')).toEqual(3);
expect(instance.find(EuiRange).prop('value')).toEqual('3');
});

it('should update state with the size value', () => {
const setStateSpy = jest.fn();
const instance = shallow(
const instance = mount(
<InlineOptions
{...defaultProps}
state={state}
Expand All @@ -506,14 +508,10 @@ describe('terms', () => {
/>
);

instance.find(EuiRange).prop('onChange')!(
{
target: {
value: '7',
},
} as React.ChangeEvent<HTMLInputElement>,
true
);
act(() => {
instance.find(ValuesRangeInput).prop('onChange')!(7);
});

expect(setStateSpy).toHaveBeenCalledWith({
...state,
layers: {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React from 'react';
import { act } from 'react-dom/test-utils';
import { shallow } from 'enzyme';
import { EuiRange } from '@elastic/eui';
import { ValuesRangeInput } from './values_range_input';

jest.mock('react-use', () => ({
useDebounce: (fn: () => void) => fn(),
}));

describe('ValuesRangeInput', () => {
it('should render EuiRange correctly', () => {
const onChangeSpy = jest.fn();
const instance = shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);

expect(instance.find(EuiRange).prop('value')).toEqual('5');
});

it('should run onChange function on update', () => {
const onChangeSpy = jest.fn();
const instance = shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);
act(() => {
instance.find(EuiRange).prop('onChange')!(
{ currentTarget: { value: '7' } } as React.ChangeEvent<HTMLInputElement>,
true
);
});
expect(instance.find(EuiRange).prop('value')).toEqual('7');
// useDebounce runs on initialization and on change
expect(onChangeSpy.mock.calls.length).toBe(2);
expect(onChangeSpy.mock.calls[0][0]).toBe(5);
expect(onChangeSpy.mock.calls[1][0]).toBe(7);
});
it('should not run onChange function on update when value is out of 1-100 range', () => {
const onChangeSpy = jest.fn();
const instance = shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);
act(() => {
instance.find(EuiRange).prop('onChange')!(
{ currentTarget: { value: '107' } } as React.ChangeEvent<HTMLInputElement>,
true
);
});
instance.update();
expect(instance.find(EuiRange).prop('value')).toEqual('107');
// useDebounce only runs on initialization
expect(onChangeSpy.mock.calls.length).toBe(2);
expect(onChangeSpy.mock.calls[0][0]).toBe(5);
expect(onChangeSpy.mock.calls[1][0]).toBe(100);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { useState } from 'react';
import { useDebounce } from 'react-use';
import { i18n } from '@kbn/i18n';
import { EuiRange } from '@elastic/eui';

export const ValuesRangeInput = ({
value,
onChange,
}: {
value: number;
onChange: (value: number) => void;
}) => {
const MIN_NUMBER_OF_VALUES = 1;
const MAX_NUMBER_OF_VALUES = 100;

const [inputValue, setInputValue] = useState(String(value));
useDebounce(
() => {
if (inputValue === '') {
return;
}
const inputNumber = Number(inputValue);
onChange(Math.min(MAX_NUMBER_OF_VALUES, Math.max(inputNumber, MIN_NUMBER_OF_VALUES)));
},
256,
[inputValue]
);

return (
<EuiRange
min={MIN_NUMBER_OF_VALUES}
max={MAX_NUMBER_OF_VALUES}
step={1}
value={inputValue}
showInput
showLabels
compressed
onChange={({ currentTarget }) => setInputValue(currentTarget.value)}
aria-label={i18n.translate('xpack.lens.indexPattern.terms.size', {
defaultMessage: 'Number of values',
})}
/>
);
};

0 comments on commit 827b4a5

Please sign in to comment.