Skip to content

Commit

Permalink
change layout of top values function (#95412) (#95546)
Browse files Browse the repository at this point in the history 
Co-authored-by: Joe Reuter <[email protected]>
  • Loading branch information
@kibanamachine @flash1293
kibanamachine and flash1293 authored Mar 26, 2021
1 parent 30abcf5 commit 394d674
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 103 deletions.
128 changes: 55 additions & 73 deletions x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@
* 2.0.
*/

import React, { useState } from 'react';
import React from 'react';
import { i18n } from '@kbn/i18n';
import {
EuiFormRow,
EuiSelect,
EuiSwitch,
EuiSwitchEvent,
EuiSpacer,
EuiPopover,
EuiButtonEmpty,
EuiText,
EuiAccordion,
EuiIconTip,
} from '@elastic/eui';
import { AggFunctionsMapping } from '../../../../../../../../src/plugins/data/public';
Expand All @@ -24,7 +22,7 @@ import { updateColumnParam, isReferenced } from '../../layer_helpers';
import { DataType } from '../../../../types';
import { OperationDefinition } from '../index';
import { FieldBasedIndexPatternColumn } from '../column_types';
import { ValuesRangeInput } from './values_range_input';
import { ValuesInput } from './values_input';
import { getEsAggsSuffix, getInvalidFieldMessage } from '../helpers';
import type { IndexPatternLayer } from '../../../types';

Expand Down Expand Up @@ -193,8 +191,6 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
paramEditor: function ParamEditor({ layer, updateLayer, currentColumn, columnId, indexPattern }) {
const hasRestrictions = indexPattern.hasRestrictions;

const [popoverOpen, setPopoverOpen] = useState(false);

const SEPARATOR = '$$$';
function toValue(orderBy: TermsIndexPatternColumn['params']['orderBy']) {
if (orderBy.type === 'alphabetical') {
Expand Down Expand Up @@ -237,7 +233,7 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
display="columnCompressed"
fullWidth
>
<ValuesRangeInput
<ValuesInput
value={currentColumn.params.size}
onChange={(value) => {
updateLayer(
Expand All @@ -251,71 +247,6 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
}}
/>
</EuiFormRow>
{!hasRestrictions && (
<EuiText textAlign="right">
<EuiPopover
ownFocus
button={
<EuiButtonEmpty
size="xs"
iconType="arrowDown"
iconSide="right"
onClick={() => {
setPopoverOpen(!popoverOpen);
}}
>
{i18n.translate('xpack.lens.indexPattern.terms.advancedSettings', {
defaultMessage: 'Advanced',
})}
</EuiButtonEmpty>
}
isOpen={popoverOpen}
closePopover={() => {
setPopoverOpen(false);
}}
>
<EuiSwitch
label={i18n.translate('xpack.lens.indexPattern.terms.otherBucketDescription', {
defaultMessage: 'Group other values as "Other"',
})}
compressed
data-test-subj="indexPattern-terms-other-bucket"
checked={Boolean(currentColumn.params.otherBucket)}
onChange={(e: EuiSwitchEvent) =>
updateLayer(
updateColumnParam({
layer,
columnId,
paramName: 'otherBucket',
value: e.target.checked,
})
)
}
/>
<EuiSpacer size="m" />
<EuiSwitch
label={i18n.translate('xpack.lens.indexPattern.terms.missingBucketDescription', {
defaultMessage: 'Include documents without this field',
})}
compressed
disabled={!currentColumn.params.otherBucket}
data-test-subj="indexPattern-terms-missing-bucket"
checked={Boolean(currentColumn.params.missingBucket)}
onChange={(e: EuiSwitchEvent) =>
updateLayer(
updateColumnParam({
layer,
columnId,
paramName: 'missingBucket',
value: e.target.checked,
})
)
}
/>
</EuiPopover>
<EuiSpacer size="s" />
</EuiText>
)}
<EuiFormRow
label={
<>
Expand Down Expand Up @@ -415,6 +346,57 @@ export const termsOperation: OperationDefinition<TermsIndexPatternColumn, 'field
})}
/>
</EuiFormRow>
{!hasRestrictions && (
<>
<EuiSpacer size="s" />
<EuiAccordion
id="lnsTermsAdvanced"
buttonContent={i18n.translate('xpack.lens.indexPattern.terms.advancedSettings', {
defaultMessage: 'Advanced',
})}
>
<EuiSpacer size="m" />
<EuiSwitch
label={i18n.translate('xpack.lens.indexPattern.terms.otherBucketDescription', {
defaultMessage: 'Group other values as "Other"',
})}
compressed
data-test-subj="indexPattern-terms-other-bucket"
checked={Boolean(currentColumn.params.otherBucket)}
onChange={(e: EuiSwitchEvent) =>
updateLayer(
updateColumnParam({
layer,
columnId,
paramName: 'otherBucket',
value: e.target.checked,
})
)
}
/>
<EuiSpacer size="m" />
<EuiSwitch
label={i18n.translate('xpack.lens.indexPattern.terms.missingBucketDescription', {
defaultMessage: 'Include documents without this field',
})}
compressed
disabled={!currentColumn.params.otherBucket}
data-test-subj="indexPattern-terms-missing-bucket"
checked={Boolean(currentColumn.params.missingBucket)}
onChange={(e: EuiSwitchEvent) =>
updateLayer(
updateColumnParam({
layer,
columnId,
paramName: 'missingBucket',
value: e.target.checked,
})
)
}
/>
</EuiAccordion>
</>
)}
</>
);
},
Expand Down
8 changes: 4 additions & 4 deletions ...k/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
import React from 'react';
import { act } from 'react-dom/test-utils';
import { shallow, mount } from 'enzyme';
import { EuiRange, EuiSelect, EuiSwitch, EuiSwitchEvent } from '@elastic/eui';
import { EuiFieldNumber, EuiSelect, EuiSwitch, EuiSwitchEvent } from '@elastic/eui';
import type { IUiSettingsClient, SavedObjectsClientContract, HttpSetup } from 'kibana/public';
import type { IStorageWrapper } from 'src/plugins/kibana_utils/public';
import { dataPluginMock } from '../../../../../../../../src/plugins/data/public/mocks';
import { createMockedIndexPattern } from '../../../mocks';
import { ValuesRangeInput } from './values_range_input';
import { ValuesInput } from './values_input';
import type { TermsIndexPatternColumn } from '.';
import { termsOperation } from '../index';
import { IndexPattern, IndexPatternLayer } from '../../../types';
Expand Down Expand Up @@ -888,7 +888,7 @@ describe('terms', () => {
/>
);

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

it('should update state with the size value', () => {
Expand All @@ -904,7 +904,7 @@ describe('terms', () => {
);

act(() => {
instance.find(ValuesRangeInput).prop('onChange')!(7);
instance.find(ValuesInput).prop('onChange')!(7);
});

expect(updateLayerSpy).toHaveBeenCalledWith({
Expand Down
38 changes: 18 additions & 20 deletions ...nitions/terms/values_range_input.test.tsx → ...s/definitions/terms/values_input.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,52 +8,50 @@
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';
import { EuiFieldNumber } from '@elastic/eui';
import { ValuesInput } from './values_input';

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

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

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

it('should not run onChange function on mount', () => {
const onChangeSpy = jest.fn();
shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);
shallow(<ValuesInput value={5} onChange={onChangeSpy} />);

expect(onChangeSpy.mock.calls.length).toBe(0);
});

it('should run onChange function on update', () => {
const onChangeSpy = jest.fn();
const instance = shallow(<ValuesRangeInput value={5} onChange={onChangeSpy} />);
const instance = shallow(<ValuesInput value={5} onChange={onChangeSpy} />);
act(() => {
instance.find(EuiRange).prop('onChange')!(
{ currentTarget: { value: '7' } } as React.ChangeEvent<HTMLInputElement>,
true
);
instance.find(EuiFieldNumber).prop('onChange')!({
currentTarget: { value: '7' },
} as React.ChangeEvent<HTMLInputElement>);
});
expect(instance.find(EuiRange).prop('value')).toEqual('7');
expect(instance.find(EuiFieldNumber).prop('value')).toEqual('7');
expect(onChangeSpy.mock.calls.length).toBe(1);
expect(onChangeSpy.mock.calls[0][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} />);
const instance = shallow(<ValuesInput value={5} onChange={onChangeSpy} />);
act(() => {
instance.find(EuiRange).prop('onChange')!(
{ currentTarget: { value: '107' } } as React.ChangeEvent<HTMLInputElement>,
true
);
instance.find(EuiFieldNumber).prop('onChange')!({
currentTarget: { value: '1007' },
} as React.ChangeEvent<HTMLInputElement>);
});
instance.update();
expect(instance.find(EuiRange).prop('value')).toEqual('107');
expect(instance.find(EuiFieldNumber).prop('value')).toEqual('1007');
expect(onChangeSpy.mock.calls.length).toBe(1);
expect(onChangeSpy.mock.calls[0][0]).toBe(100);
expect(onChangeSpy.mock.calls[0][0]).toBe(1000);
});
});
10 changes: 4 additions & 6 deletions .../definitions/terms/values_range_input.tsx → ...ations/definitions/terms/values_input.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@

import React, { useState } from 'react';
import { i18n } from '@kbn/i18n';
import { EuiRange } from '@elastic/eui';
import { EuiFieldNumber } from '@elastic/eui';
import { useDebounceWithOptions } from '../helpers';

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

const [inputValue, setInputValue] = useState(String(value));

Expand All @@ -36,13 +36,11 @@ export const ValuesRangeInput = ({
);

return (
<EuiRange
<EuiFieldNumber
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', {
Expand Down

0 comments on commit 394d674

Please sign in to comment.