From e3841ea7eaba4eabd3769d425c59ca0e7fcde7d2 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Tue, 8 May 2018 13:48:12 -0600 Subject: [PATCH] [Input controls] Replace react-select with EuiComboBox (#17452) * replace react-select with EuiComboBox * remove constructor for ListControl component * get working with portal version * remove overflow visible from input_control_vis since its no longer needed * convert index pattern select to EuiComboBox * replace react-select with EuiComboBox for field select * group fields by type * remove esvm * remove on-foxus box around input cursor * fix jest tests * remove broken jest test * fix functional tests * review changes * remove componentWillMount from field_select * update snapshot changed from rebasing and getting new EUI version * use combo box clear and close buttons for clearing and closing * jsdoc syntax fix --- .../list_control_editor.test.js.snap | 10 +- .../range_control_editor.test.js.snap | 5 +- .../__tests__/get_index_pattern_mock.js | 11 ++ .../__tests__/get_index_patterns_mock.js | 16 +++ .../public/components/editor/controls_tab.js | 8 +- .../components/editor/controls_tab.test.js | 10 +- .../public/components/editor/field_select.js | 106 ++++++++++----- .../components/editor/index_pattern_select.js | 107 +++++++++++++-- .../components/editor/list_control_editor.js | 5 +- .../editor/list_control_editor.test.js | 43 ++---- .../components/editor/range_control_editor.js | 5 +- .../editor/range_control_editor.test.js | 39 ++---- .../input_control_vis.test.js.snap | 123 ++++++++---------- .../__snapshots__/list_control.test.js.snap | 81 +----------- .../__snapshots__/range_control.test.js.snap | 20 +-- .../public/components/vis/form_row.js | 6 +- .../public/components/vis/form_row.test.js | 12 +- .../components/vis/input_control_vis.js | 7 +- .../components/vis/input_control_vis.test.js | 28 +--- .../public/components/vis/list_control.js | 67 +++++----- .../components/vis/list_control.test.js | 27 ++-- .../public/components/vis/range_control.js | 2 +- .../filter_manager/phrase_filter_manager.js | 34 +++-- .../phrase_filter_manager.test.js | 38 +++++- .../public/control/list_control_factory.js | 8 +- .../input_control_vis/public/vis.less | 8 -- src/ui/public/styles/base.less | 6 + .../apps/visualize/_input_control_vis.js | 50 +++---- .../functional/page_objects/visualize_page.js | 64 +++++---- 29 files changed, 474 insertions(+), 472 deletions(-) create mode 100644 src/core_plugins/input_control_vis/public/components/editor/__tests__/get_index_pattern_mock.js create mode 100644 src/core_plugins/input_control_vis/public/components/editor/__tests__/get_index_patterns_mock.js diff --git a/src/core_plugins/input_control_vis/public/components/editor/__snapshots__/list_control_editor.test.js.snap b/src/core_plugins/input_control_vis/public/components/editor/__snapshots__/list_control_editor.test.js.snap index 8c6ab30f75c3..a0ee8da4407d 100644 --- a/src/core_plugins/input_control_vis/public/components/editor/__snapshots__/list_control_editor.test.js.snap +++ b/src/core_plugins/input_control_vis/public/components/editor/__snapshots__/list_control_editor.test.js.snap @@ -4,17 +4,18 @@ exports[`parentCandidates 1`] = `
{ + return Promise.resolve({ + id: 'mockIndexPattern', + title: 'mockIndexPattern', + fields: [ + { name: 'keywordField', type: 'string', aggregatable: true }, + { name: 'textField', type: 'string', aggregatable: false }, + { name: 'numberField', type: 'number', aggregatable: true } + ] + }); +}; diff --git a/src/core_plugins/input_control_vis/public/components/editor/__tests__/get_index_patterns_mock.js b/src/core_plugins/input_control_vis/public/components/editor/__tests__/get_index_patterns_mock.js new file mode 100644 index 000000000000..60dbc949c538 --- /dev/null +++ b/src/core_plugins/input_control_vis/public/components/editor/__tests__/get_index_patterns_mock.js @@ -0,0 +1,16 @@ +export const getIndexPatternsMock = () => { + return Promise.resolve([ + { + id: 'indexPattern1', + attributes: { + title: 'indexPattern1' + } + }, + { + id: 'indexPattern2', + attributes: { + title: 'indexPattern2' + } + } + ]); +}; diff --git a/src/core_plugins/input_control_vis/public/components/editor/controls_tab.js b/src/core_plugins/input_control_vis/public/components/editor/controls_tab.js index 6ffe115e1ce8..b71307148524 100644 --- a/src/core_plugins/input_control_vis/public/components/editor/controls_tab.js +++ b/src/core_plugins/input_control_vis/public/components/editor/controls_tab.js @@ -48,16 +48,16 @@ export class ControlsTab extends Component { this.setVisParam('controls', setControl(this.props.scope.vis.params.controls, controlIndex, updatedControl)); } - handleIndexPatternChange = (controlIndex, evt) => { + handleIndexPatternChange = (controlIndex, indexPatternId) => { const updatedControl = this.props.scope.vis.params.controls[controlIndex]; - updatedControl.indexPattern = evt.value; + updatedControl.indexPattern = indexPatternId; updatedControl.fieldName = ''; this.setVisParam('controls', setControl(this.props.scope.vis.params.controls, controlIndex, updatedControl)); } - handleFieldNameChange = (controlIndex, evt) => { + handleFieldNameChange = (controlIndex, fieldName) => { const updatedControl = this.props.scope.vis.params.controls[controlIndex]; - updatedControl.fieldName = evt.value; + updatedControl.fieldName = fieldName; this.setVisParam('controls', setControl(this.props.scope.vis.params.controls, controlIndex, updatedControl)); } diff --git a/src/core_plugins/input_control_vis/public/components/editor/controls_tab.test.js b/src/core_plugins/input_control_vis/public/components/editor/controls_tab.test.js index 0cc5dc7120c9..9f888ac7e70a 100644 --- a/src/core_plugins/input_control_vis/public/components/editor/controls_tab.test.js +++ b/src/core_plugins/input_control_vis/public/components/editor/controls_tab.test.js @@ -2,6 +2,7 @@ import React from 'react'; import sinon from 'sinon'; import { mount, shallow } from 'enzyme'; import { findTestSubject } from '@elastic/eui/lib/test'; +import { getIndexPatternMock } from './__tests__/get_index_pattern_mock'; import { ControlsTab, } from './controls_tab'; @@ -21,14 +22,7 @@ const savedObjectsClientMock = { } }; const indexPatternsMock = { - get: () => { - return Promise.resolve({ - fields: [ - { name: 'keywordField', type: 'string', aggregatable: true }, - { name: 'numberField', type: 'number', aggregatable: true } - ] - }); - } + get: getIndexPatternMock }; const scopeMock = { vis: { diff --git a/src/core_plugins/input_control_vis/public/components/editor/field_select.js b/src/core_plugins/input_control_vis/public/components/editor/field_select.js index 182d0f58096c..5a0813ccdc56 100644 --- a/src/core_plugins/input_control_vis/public/components/editor/field_select.js +++ b/src/core_plugins/input_control_vis/public/components/editor/field_select.js @@ -1,60 +1,102 @@ import _ from 'lodash'; import PropTypes from 'prop-types'; import React, { Component } from 'react'; -import Select from 'react-select'; import { EuiFormRow, + EuiComboBox, } from '@elastic/eui'; export class FieldSelect extends Component { constructor(props) { super(props); - // not storing activeIndexPatternId in react state - // 1) does not effect rendering - // 2) requires synchronous modification to avoid race condition - this.activeIndexPatternId = props.indexPatternId; + this._hasUnmounted = false; this.state = { - fields: [] + isLoading: false, + fields: [], + indexPatternId: props.indexPatternId, }; this.filterField = _.get(props, 'filterField', () => { return true; }); - this.loadFields(props.indexPatternId); + } + + componentWillUnmount() { + this._hasUnmounted = true; + } + + componentDidMount() { + this.loadFields(this.state.indexPatternId); } componentWillReceiveProps(nextProps) { if (this.props.indexPatternId !== nextProps.indexPatternId) { - this.activeIndexPatternId = nextProps.indexPatternId; - this.setState({ fields: [] }); this.loadFields(nextProps.indexPatternId); } } - async loadFields(indexPatternId) { + loadFields = (indexPatternId) => { + this.setState({ + isLoading: true, + fields: [], + indexPatternId + }, this.debouncedLoad.bind(null, indexPatternId)); + } + + debouncedLoad = _.debounce(async (indexPatternId) => { if (!indexPatternId || indexPatternId.length === 0) { return; } const indexPattern = await this.props.getIndexPattern(indexPatternId); - // props.indexPatternId may be updated before getIndexPattern returns - // ignore response when fetched index pattern does not match active index pattern - if (indexPattern.id !== this.activeIndexPatternId) { + if (this._hasUnmounted) { return; } - const fields = indexPattern.fields + // props.indexPatternId may be updated before getIndexPattern returns + // ignore response when fetched index pattern does not match active index pattern + if (indexPattern.id !== this.state.indexPatternId) { + return; + } + + const fieldsByTypeMap = new Map(); + const fields = []; + indexPattern.fields .filter(this.filterField) - .sort((a, b) => { - if (a.name < b.name) return -1; - if (a.name > b.name) return 1; - return 0; - }) - .map(function (field) { - return { label: field.name, value: field.name }; + .forEach(field => { + if (fieldsByTypeMap.has(field.type)) { + const fieldsList = fieldsByTypeMap.get(field.type); + fieldsList.push(field.name); + fieldsByTypeMap.set(field.type, fieldsList); + } else { + fieldsByTypeMap.set(field.type, [field.name]); + } }); - this.setState({ fields: fields }); + + fieldsByTypeMap.forEach((fieldsList, fieldType) => { + fields.push({ + label: fieldType, + options: fieldsList.sort().map(fieldName => { + return { value: fieldName, label: fieldName }; + }) + }); + }); + + fields.sort((a, b) => { + if (a.label < b.label) return -1; + if (a.label > b.label) return 1; + return 0; + }); + + this.setState({ + isLoading: false, + fields: fields + }); + }, 300); + + onChange = (selectedOptions) => { + this.props.onChange(_.get(selectedOptions, '0.value')); } render() { @@ -63,19 +105,25 @@ export class FieldSelect extends Component { } const selectId = `fieldSelect-${this.props.controlIndex}`; + + const selectedOptions = []; + if (this.props.fieldName) { + selectedOptions.push({ value: this.props.fieldName, label: this.props.fieldName }); + } + return ( - `; diff --git a/src/core_plugins/input_control_vis/public/components/vis/__snapshots__/range_control.test.js.snap b/src/core_plugins/input_control_vis/public/components/vis/__snapshots__/range_control.test.js.snap index 644f2285bb2d..24fd9d46902f 100644 --- a/src/core_plugins/input_control_vis/public/components/vis/__snapshots__/range_control.test.js.snap +++ b/src/core_plugins/input_control_vis/public/components/vis/__snapshots__/range_control.test.js.snap @@ -2,26 +2,8 @@ exports[`renders RangeControl 1`] = ` diff --git a/src/core_plugins/input_control_vis/public/components/vis/form_row.js b/src/core_plugins/input_control_vis/public/components/vis/form_row.js index 126483b09a2d..4e6bb54645c7 100644 --- a/src/core_plugins/input_control_vis/public/components/vis/form_row.js +++ b/src/core_plugins/input_control_vis/public/components/vis/form_row.js @@ -8,9 +8,9 @@ import { export function FormRow(props) { let control = props.children; - if (!props.control.isEnabled()) { + if (props.disableMsg) { control = ( - + {control} ); @@ -32,5 +32,5 @@ FormRow.propTypes = { id: PropTypes.string.isRequired, children: PropTypes.node.isRequired, controlIndex: PropTypes.number.isRequired, - control: PropTypes.object.isRequired, + disableMsg: PropTypes.string, }; diff --git a/src/core_plugins/input_control_vis/public/components/vis/form_row.test.js b/src/core_plugins/input_control_vis/public/components/vis/form_row.test.js index cd1439dbbcbe..28d5ec8edeec 100644 --- a/src/core_plugins/input_control_vis/public/components/vis/form_row.test.js +++ b/src/core_plugins/input_control_vis/public/components/vis/form_row.test.js @@ -6,15 +6,10 @@ import { } from './form_row'; test('renders enabled control', () => { - const enabledControl = { - id: 'mock-enabled-control', - isEnabled: () => { return true; }, - }; const component = shallow(
My Control
@@ -24,16 +19,11 @@ test('renders enabled control', () => { }); test('renders disabled control with tooltip', () => { - const disabledControl = { - id: 'mock-disabled-control', - isEnabled: () => { return false; }, - disabledReason: 'I am disabled for testing purposes' - }; const component = shallow(
My Control
diff --git a/src/core_plugins/input_control_vis/public/components/vis/input_control_vis.js b/src/core_plugins/input_control_vis/public/components/vis/input_control_vis.js index be5d3921a0b2..044e17dd5135 100644 --- a/src/core_plugins/input_control_vis/public/components/vis/input_control_vis.js +++ b/src/core_plugins/input_control_vis/public/components/vis/input_control_vis.js @@ -38,7 +38,12 @@ export class InputControlVis extends Component { case 'list': controlComponent = ( diff --git a/src/core_plugins/input_control_vis/public/components/vis/input_control_vis.test.js b/src/core_plugins/input_control_vis/public/components/vis/input_control_vis.test.js index 5eb22ba5a98f..e421ac528ffa 100644 --- a/src/core_plugins/input_control_vis/public/components/vis/input_control_vis.test.js +++ b/src/core_plugins/input_control_vis/public/components/vis/input_control_vis.test.js @@ -16,8 +16,7 @@ const mockListControl = { }, type: 'list', label: 'list control', - value: '', - getMultiSelectDelimiter: () => { return ','; }, + value: [], selectOptions: [ { label: 'choice1', value: 'choice1' }, { label: 'choice2', value: 'choice2' } @@ -159,28 +158,3 @@ test('resetControls', () => { sinon.assert.notCalled(submitFilters); sinon.assert.notCalled(stageFilter); }); - -test('stageFilter list control', () => { - const component = mount( { return true; }} - hasValues={() => { return true; }} - />); - const reactSelectInput = component.find(`#${mockListControl.id}`).hostNodes(); - reactSelectInput.simulate('change', { target: { value: 'choice1' } }); - reactSelectInput.simulate('keyDown', { keyCode: 9, key: 'Tab' }); - sinon.assert.notCalled(clearControls); - sinon.assert.notCalled(submitFilters); - sinon.assert.notCalled(resetControls); - const expectedControlIndex = 0; - const expectedControlValue = 'choice1'; - sinon.assert.calledWith(stageFilter, - expectedControlIndex, - expectedControlValue - ); -}); diff --git a/src/core_plugins/input_control_vis/public/components/vis/list_control.js b/src/core_plugins/input_control_vis/public/components/vis/list_control.js index 506d9faa332b..8caa4c60bf83 100644 --- a/src/core_plugins/input_control_vis/public/components/vis/list_control.js +++ b/src/core_plugins/input_control_vis/public/components/vis/list_control.js @@ -1,57 +1,44 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; -import Select from 'react-select'; import { FormRow } from './form_row'; import { EuiFieldText, + EuiComboBox, } from '@elastic/eui'; export class ListControl extends Component { - constructor(props) { - super(props); - this.handleOnChange = this.handleOnChange.bind(this); - this.truncate = this.truncate.bind(this); - } - - handleOnChange(evt) { - let newValue = ''; - if (evt) { - newValue = evt; - } - this.props.stageFilter(this.props.controlIndex, newValue); - } - - truncate(selected) { - if (selected.label.length <= 24) { - return selected.label; - } - return `${selected.label.substring(0, 23)}...`; + handleOnChange = (selectedOptions) => { + this.props.stageFilter(this.props.controlIndex, selectedOptions); } renderControl() { - if (!this.props.control.isEnabled()) { - // react-select clobbers the tooltip, so just returning a disabled input instead + if (this.props.disableMsg) { return ( ); } + const options = this.props.options.map(option => { + return { + label: option.label, + value: option.value, + ['data-test-subj']: `option_${option.value.replace(' ', '_')}` + }; + }); + return ( -