You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by gr...@apache.org on 2017/11/06 23:20:15 UTC
[incubator-superset] branch master updated: [Performance]
VirtualizedSelect for SelectControl and FilterBox (#3654)
This is an automated email from the ASF dual-hosted git repository.
graceguo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 9a49b1c [Performance] VirtualizedSelect for SelectControl and FilterBox (#3654)
9a49b1c is described below
commit 9a49b1c41dca13b9f5095e7cc8f3b931d01accf9
Author: Jeff Niu <je...@gmail.com>
AuthorDate: Mon Nov 6 15:20:13 2017 -0800
[Performance] VirtualizedSelect for SelectControl and FilterBox (#3654)
* Added virtualized select to SelectControl, allow onPaste to create new options
* Added unit tests
* Added virtualized/paste select to filterbox
---
.../javascripts/components/OnPasteSelect.jsx | 87 +++++++++++++++++
.../components/VirtualizedRendererWrap.jsx | 56 +++++++++++
.../explore/components/controls/SelectControl.jsx | 65 ++-----------
.../javascripts/components/OnPasteSelect_spec.jsx | 105 ++++++++++++++++++++
.../components/VirtualizedRendererWrap_spec.jsx | 106 +++++++++++++++++++++
.../explore/components/SelectControl_spec.jsx | 33 ++++++-
superset/assets/visualizations/filter_box.jsx | 12 ++-
7 files changed, 397 insertions(+), 67 deletions(-)
diff --git a/superset/assets/javascripts/components/OnPasteSelect.jsx b/superset/assets/javascripts/components/OnPasteSelect.jsx
new file mode 100644
index 0000000..b043bf3
--- /dev/null
+++ b/superset/assets/javascripts/components/OnPasteSelect.jsx
@@ -0,0 +1,87 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import Select from 'react-select';
+
+export default class OnPasteSelect extends React.Component {
+ onPaste(evt) {
+ if (!this.props.multi) {
+ return;
+ }
+ evt.preventDefault();
+ const clipboard = evt.clipboardData.getData('Text');
+ if (!clipboard) {
+ return;
+ }
+ const regex = `[${this.props.separator}]+`;
+ const values = clipboard.split(new RegExp(regex)).map(v => v.trim());
+ const validator = this.props.isValidNewOption;
+ const selected = this.props.value || [];
+ const existingOptions = {};
+ const existing = {};
+ this.props.options.forEach((v) => {
+ existingOptions[v[this.props.valueKey]] = 1;
+ });
+ let options = [];
+ selected.forEach((v) => {
+ options.push({ [this.props.labelKey]: v, [this.props.valueKey]: v });
+ existing[v] = 1;
+ });
+ options = options.concat(values
+ .filter((v) => {
+ const notExists = !existing[v];
+ existing[v] = 1;
+ return notExists && (validator ? validator({ [this.props.labelKey]: v }) : !!v);
+ })
+ .map((v) => {
+ const opt = { [this.props.labelKey]: v, [this.props.valueKey]: v };
+ if (!existingOptions[v]) {
+ this.props.options.unshift(opt);
+ }
+ return opt;
+ }),
+ );
+ if (options.length) {
+ if (this.props.onChange) {
+ this.props.onChange(options);
+ }
+ }
+ }
+ render() {
+ const SelectComponent = this.props.selectWrap;
+ const refFunc = (ref) => {
+ if (this.props.ref) {
+ this.props.ref(ref);
+ }
+ this.pasteInput = ref;
+ };
+ const inputProps = { onPaste: this.onPaste.bind(this) };
+ return (
+ <SelectComponent
+ {...this.props}
+ ref={refFunc}
+ inputProps={inputProps}
+ />
+ );
+ }
+}
+
+OnPasteSelect.propTypes = {
+ separator: PropTypes.string.isRequired,
+ selectWrap: PropTypes.func.isRequired,
+ ref: PropTypes.func,
+ onChange: PropTypes.func.isRequired,
+ valueKey: PropTypes.string.isRequired,
+ labelKey: PropTypes.string.isRequired,
+ options: PropTypes.array,
+ multi: PropTypes.bool.isRequired,
+ value: PropTypes.any,
+ isValidNewOption: PropTypes.func,
+};
+OnPasteSelect.defaultProps = {
+ separator: ',',
+ selectWrap: Select,
+ valueKey: 'value',
+ labelKey: 'label',
+ options: [],
+ multi: false,
+};
diff --git a/superset/assets/javascripts/components/VirtualizedRendererWrap.jsx b/superset/assets/javascripts/components/VirtualizedRendererWrap.jsx
new file mode 100644
index 0000000..3282112
--- /dev/null
+++ b/superset/assets/javascripts/components/VirtualizedRendererWrap.jsx
@@ -0,0 +1,56 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+
+export default function VirtualizedRendererWrap(renderer) {
+ function WrapperRenderer({
+ focusedOption,
+ focusOption,
+ key,
+ option,
+ selectValue,
+ style,
+ valueArray,
+ }) {
+ if (!option) {
+ return null;
+ }
+ const className = ['VirtualizedSelectOption'];
+ if (option === focusedOption) {
+ className.push('VirtualizedSelectFocusedOption');
+ }
+ if (option.disabled) {
+ className.push('VirtualizedSelectDisabledOption');
+ }
+ if (valueArray && valueArray.indexOf(option) >= 0) {
+ className.push('VirtualizedSelectSelectedOption');
+ }
+ if (option.className) {
+ className.push(option.className);
+ }
+ const events = option.disabled ? {} : {
+ onClick: () => selectValue(option),
+ onMouseEnter: () => focusOption(option),
+ };
+ return (
+ <div
+ className={className.join(' ')}
+ key={key}
+ style={Object.assign(option.style || {}, style)}
+ title={option.title}
+ {...events}
+ >
+ {renderer(option)}
+ </div>
+ );
+ }
+ WrapperRenderer.propTypes = {
+ focusedOption: PropTypes.object.isRequired,
+ focusOption: PropTypes.func.isRequired,
+ key: PropTypes.string,
+ option: PropTypes.object,
+ selectValue: PropTypes.func.isRequired,
+ style: PropTypes.object,
+ valueArray: PropTypes.array,
+ };
+ return WrapperRenderer;
+}
diff --git a/superset/assets/javascripts/explore/components/controls/SelectControl.jsx b/superset/assets/javascripts/explore/components/controls/SelectControl.jsx
index 51381d6..a82995d 100644
--- a/superset/assets/javascripts/explore/components/controls/SelectControl.jsx
+++ b/superset/assets/javascripts/explore/components/controls/SelectControl.jsx
@@ -1,8 +1,11 @@
import React from 'react';
import PropTypes from 'prop-types';
+import VirtualizedSelect from 'react-virtualized-select';
import Select, { Creatable } from 'react-select';
import ControlHeader from '../ControlHeader';
import { t } from '../../../locales';
+import VirtualizedRendererWrap from '../../../components/VirtualizedRendererWrap';
+import OnPasteSelect from '../../../components/OnPasteSelect';
const propTypes = {
choices: PropTypes.array,
@@ -37,55 +40,6 @@ const defaultProps = {
valueKey: 'value',
};
-// Handle `onPaste` so that users may paste in
-// options as comma-delimited, slightly modified from
-// https://github.com/JedWatson/react-select/issues/1672
-function pasteSelect(props) {
- let pasteInput;
- return (
- <Select
- {...props}
- ref={(ref) => {
- // Creatable requires a reference to its Select child
- if (props.ref) {
- props.ref(ref);
- }
- pasteInput = ref;
- }}
- inputProps={{
- onPaste: (evt) => {
- if (!props.multi) {
- return;
- }
- evt.preventDefault();
- // pull text from the clipboard and split by comma
- const clipboard = evt.clipboardData.getData('Text');
- if (!clipboard) {
- return;
- }
- const values = clipboard.split(/[,]+/).map(v => v.trim());
- const options = values
- .filter(value =>
- // Creatable validates options
- props.isValidNewOption ? props.isValidNewOption({ label: value }) : !!value,
- )
- .map(value => ({
- [props.labelKey]: value,
- [props.valueKey]: value,
- }));
- if (options.length) {
- pasteInput.selectValue(options);
- }
- },
- }}
- />
- );
-}
-pasteSelect.propTypes = {
- multi: PropTypes.bool,
- ref: PropTypes.func,
-};
-
export default class SelectControl extends React.PureComponent {
constructor(props) {
super(props);
@@ -161,23 +115,16 @@ export default class SelectControl extends React.PureComponent {
clearable: this.props.clearable,
isLoading: this.props.isLoading,
onChange: this.onChange,
- optionRenderer: this.props.optionRenderer,
+ optionRenderer: VirtualizedRendererWrap(this.props.optionRenderer),
valueRenderer: this.props.valueRenderer,
+ selectComponent: this.props.freeForm ? Creatable : Select,
};
- // Tab, comma or Enter will trigger a new option created for FreeFormSelect
- const selectWrap = this.props.freeForm ? (
- <Creatable {...selectProps}>
- {pasteSelect}
- </Creatable>
- ) : (
- pasteSelect(selectProps)
- );
return (
<div>
{this.props.showHeader &&
<ControlHeader {...this.props} />
}
- {selectWrap}
+ <OnPasteSelect {...selectProps} selectWrap={VirtualizedSelect} />
</div>
);
}
diff --git a/superset/assets/spec/javascripts/components/OnPasteSelect_spec.jsx b/superset/assets/spec/javascripts/components/OnPasteSelect_spec.jsx
new file mode 100644
index 0000000..949ca03
--- /dev/null
+++ b/superset/assets/spec/javascripts/components/OnPasteSelect_spec.jsx
@@ -0,0 +1,105 @@
+/* eslint-disable no-unused-expressions */
+import React from 'react';
+import sinon from 'sinon';
+import { expect } from 'chai';
+import { shallow } from 'enzyme';
+import { describe, it } from 'mocha';
+import VirtualizedSelect from 'react-virtualized-select';
+import Select, { Creatable } from 'react-select';
+
+import OnPasteSelect from '../../../javascripts/components/OnPasteSelect';
+
+const defaultProps = {
+ onChange: sinon.spy(),
+ multi: true,
+ isValidNewOption: sinon.spy(s => !!s.label),
+ value: [],
+ options: [
+ { value: 'United States', label: 'United States' },
+ { value: 'China', label: 'China' },
+ { value: 'India', label: 'India' },
+ { value: 'Canada', label: 'Canada' },
+ { value: 'Russian Federation', label: 'Russian Federation' },
+ { value: 'Japan', label: 'Japan' },
+ { value: 'Mexico', label: 'Mexico' },
+ ],
+};
+
+const defaultEvt = {
+ preventDefault: sinon.spy(),
+ clipboardData: {
+ getData: sinon.spy(() => ' United States, China , India, Canada, '),
+ },
+};
+
+describe('OnPasteSelect', () => {
+ let wrapper;
+ let props;
+ let evt;
+ let expected;
+ beforeEach(() => {
+ props = Object.assign({}, defaultProps);
+ wrapper = shallow(<OnPasteSelect {...props} />);
+ evt = Object.assign({}, defaultEvt);
+ });
+
+ it('renders the supplied selectWrap component', () => {
+ const select = wrapper.find(Select);
+ expect(select).to.have.lengthOf(1);
+ });
+
+ it('renders custom selectWrap components', () => {
+ props.selectWrap = Creatable;
+ wrapper = shallow(<OnPasteSelect {...props} />);
+ expect(wrapper.find(Creatable)).to.have.lengthOf(1);
+ props.selectWrap = VirtualizedSelect;
+ wrapper = shallow(<OnPasteSelect {...props} />);
+ expect(wrapper.find(VirtualizedSelect)).to.have.lengthOf(1);
+ });
+
+ describe('onPaste', () => {
+ it('calls onChange with pasted values', () => {
+ wrapper.instance().onPaste(evt);
+ expected = props.options.slice(0, 4);
+ expect(props.onChange.calledWith(expected)).to.be.true;
+ expect(evt.preventDefault.called).to.be.true;
+ expect(props.isValidNewOption.callCount).to.equal(5);
+ });
+
+ it('calls onChange without any duplicate values and adds new values', () => {
+ evt.clipboardData.getData = sinon.spy(() =>
+ 'China, China, China, China, Mexico, Mexico, Chi na, Mexico, ',
+ );
+ expected = [
+ props.options[1],
+ props.options[6],
+ { label: 'Chi na', value: 'Chi na' },
+ ];
+ wrapper.instance().onPaste(evt);
+ expect(props.onChange.calledWith(expected)).to.be.true;
+ expect(evt.preventDefault.called).to.be.true;
+ expect(props.isValidNewOption.callCount).to.equal(9);
+ expect(props.options[0].value).to.equal(expected[2].value);
+ props.options.splice(0, 1);
+ });
+
+ it('calls onChange with currently selected values and new values', () => {
+ props.value = ['United States', 'Canada', 'Mexico'];
+ evt.clipboardData.getData = sinon.spy(() =>
+ 'United States, Canada, Japan, India',
+ );
+ wrapper = shallow(<OnPasteSelect {...props} />);
+ expected = [
+ props.options[0],
+ props.options[3],
+ props.options[6],
+ props.options[5],
+ props.options[2],
+ ];
+ wrapper.instance().onPaste(evt);
+ expect(props.onChange.calledWith(expected)).to.be.true;
+ expect(evt.preventDefault.called).to.be.true;
+ expect(props.isValidNewOption.callCount).to.equal(11);
+ });
+ });
+});
diff --git a/superset/assets/spec/javascripts/components/VirtualizedRendererWrap_spec.jsx b/superset/assets/spec/javascripts/components/VirtualizedRendererWrap_spec.jsx
new file mode 100644
index 0000000..1cd4c4e
--- /dev/null
+++ b/superset/assets/spec/javascripts/components/VirtualizedRendererWrap_spec.jsx
@@ -0,0 +1,106 @@
+/* eslint-disable no-unused-expressions */
+import React from 'react';
+import sinon from 'sinon';
+import PropTypes from 'prop-types';
+import { expect } from 'chai';
+import { describe, it } from 'mocha';
+import { shallow } from 'enzyme';
+
+import VirtualizedRendererWrap from '../../../javascripts/components/VirtualizedRendererWrap';
+
+const defaultProps = {
+ focusedOption: { label: 'focusedOn', value: 'focusedOn' },
+ focusOption: sinon.spy(),
+ key: 'key1',
+ option: { label: 'option1', value: 'option1' },
+ selectValue: sinon.spy(),
+ valueArray: [],
+};
+
+function TestOption({ option }) {
+ return (
+ <span>{option.label}</span>
+ );
+}
+TestOption.propTypes = {
+ option: PropTypes.object.isRequired,
+};
+
+const defaultRenderer = opt => <TestOption option={opt} />;
+const RendererWrap = VirtualizedRendererWrap(defaultRenderer);
+
+describe('VirtualizedRendererWrap', () => {
+ let wrapper;
+ let props;
+ beforeEach(() => {
+ wrapper = shallow(<RendererWrap {...defaultProps} />);
+ props = Object.assign({}, defaultProps);
+ });
+
+ it('uses the provided renderer', () => {
+ const option = wrapper.find(TestOption);
+ expect(option).to.have.lengthOf(1);
+ });
+
+ it('renders nothing when no option is provided', () => {
+ props.option = null;
+ wrapper = shallow(<RendererWrap {...props} />);
+ const option = wrapper.find(TestOption);
+ expect(option).to.have.lengthOf(0);
+ });
+
+ it('renders unfocused, unselected options with the default class', () => {
+ const optionDiv = wrapper.find('div');
+ expect(optionDiv).to.have.lengthOf(1);
+ expect(optionDiv.props().className).to.equal('VirtualizedSelectOption');
+ });
+
+ it('renders focused option with the correct class', () => {
+ props.option = props.focusedOption;
+ wrapper = shallow(<RendererWrap {...props} />);
+ const optionDiv = wrapper.find('div');
+ expect(optionDiv.props().className).to.equal(
+ 'VirtualizedSelectOption VirtualizedSelectFocusedOption',
+ );
+ });
+
+ it('renders disabled option with the correct class', () => {
+ props.option.disabled = true;
+ wrapper = shallow(<RendererWrap {...props} />);
+ const optionDiv = wrapper.find('div');
+ expect(optionDiv.props().className).to.equal(
+ 'VirtualizedSelectOption VirtualizedSelectDisabledOption',
+ );
+ props.option.disabled = false;
+ });
+
+ it('renders selected option with the correct class', () => {
+ props.valueArray = [props.option, props.focusedOption];
+ wrapper = shallow(<RendererWrap {...props} />);
+ const optionDiv = wrapper.find('div');
+ expect(optionDiv.props().className).to.equal(
+ 'VirtualizedSelectOption VirtualizedSelectSelectedOption',
+ );
+ });
+
+ it('renders options with custom classes', () => {
+ props.option.className = 'CustomClass';
+ wrapper = shallow(<RendererWrap {...props} />);
+ const optionDiv = wrapper.find('div');
+ expect(optionDiv.props().className).to.equal(
+ 'VirtualizedSelectOption CustomClass',
+ );
+ });
+
+ it('calls focusedOption on its own option onMouseEnter', () => {
+ const optionDiv = wrapper.find('div');
+ optionDiv.simulate('mouseEnter');
+ expect(props.focusOption.calledWith(props.option)).to.be.true;
+ });
+
+ it('calls selectValue on its own option onClick', () => {
+ const optionDiv = wrapper.find('div');
+ optionDiv.simulate('click');
+ expect(props.selectValue.calledWith(props.option)).to.be.true;
+ });
+});
diff --git a/superset/assets/spec/javascripts/explore/components/SelectControl_spec.jsx b/superset/assets/spec/javascripts/explore/components/SelectControl_spec.jsx
index 7508eeb..d0eea9c 100644
--- a/superset/assets/spec/javascripts/explore/components/SelectControl_spec.jsx
+++ b/superset/assets/spec/javascripts/explore/components/SelectControl_spec.jsx
@@ -1,10 +1,13 @@
/* eslint-disable no-unused-expressions */
import React from 'react';
import Select, { Creatable } from 'react-select';
+import VirtualizedSelect from 'react-virtualized-select';
import sinon from 'sinon';
import { expect } from 'chai';
import { describe, it, beforeEach } from 'mocha';
import { shallow } from 'enzyme';
+import OnPasteSelect from '../../../../javascripts/components/OnPasteSelect';
+import VirtualizedRendererWrap from '../../../../javascripts/components/VirtualizedRendererWrap';
import SelectControl from '../../../../javascripts/explore/components/controls/SelectControl';
const defaultProps = {
@@ -26,19 +29,39 @@ describe('SelectControl', () => {
wrapper = shallow(<SelectControl {...defaultProps} />);
});
- it('renders a Select', () => {
- expect(wrapper.find(Select)).to.have.lengthOf(1);
+ it('renders an OnPasteSelect', () => {
+ expect(wrapper.find(OnPasteSelect)).to.have.lengthOf(1);
});
it('calls onChange when toggled', () => {
- const select = wrapper.find(Select);
+ const select = wrapper.find(OnPasteSelect);
select.simulate('change', { value: 50 });
expect(defaultProps.onChange.calledWith(50)).to.be.true;
});
- it('renders a Creatable for freeform', () => {
+ it('passes VirtualizedSelect as selectWrap', () => {
+ const select = wrapper.find(OnPasteSelect);
+ expect(select.props().selectWrap).to.equal(VirtualizedSelect);
+ });
+
+ it('passes Creatable as selectComponent when freeForm=true', () => {
wrapper = shallow(<SelectControl {...defaultProps} freeForm />);
- expect(wrapper.find(Creatable)).to.have.lengthOf(1);
+ const select = wrapper.find(OnPasteSelect);
+ expect(select.props().selectComponent).to.equal(Creatable);
+ });
+
+ it('passes Select as selectComponent when freeForm=false', () => {
+ const select = wrapper.find(OnPasteSelect);
+ expect(select.props().selectComponent).to.equal(Select);
+ });
+
+ it('wraps optionRenderer in a VirtualizedRendererWrap', () => {
+ const select = wrapper.find(OnPasteSelect);
+ const defaultOptionRenderer = SelectControl.defaultProps.optionRenderer;
+ const wrappedRenderer = VirtualizedRendererWrap(defaultOptionRenderer);
+ expect(select.props().optionRenderer).to.be.a('Function');
+ // different instances of wrapper with same inner renderer are unequal
+ expect(select.props().optionRenderer.name).to.equal(wrappedRenderer.name);
});
describe('getOptions', () => {
diff --git a/superset/assets/visualizations/filter_box.jsx b/superset/assets/visualizations/filter_box.jsx
index d09755a..bdcf978 100644
--- a/superset/assets/visualizations/filter_box.jsx
+++ b/superset/assets/visualizations/filter_box.jsx
@@ -3,13 +3,16 @@ import d3 from 'd3';
import React from 'react';
import PropTypes from 'prop-types';
import ReactDOM from 'react-dom';
-import Select from 'react-select';
+import VirtualizedSelect from 'react-virtualized-select';
+import { Creatable } from 'react-select';
import { Button } from 'react-bootstrap';
import DateFilterControl from '../javascripts/explore/components/controls/DateFilterControl';
import ControlRow from '../javascripts/explore/components/ControlRow';
import Control from '../javascripts/explore/components/Control';
import controls from '../javascripts/explore/stores/controls';
+import OnPasteSelect from '../javascripts/components/OnPasteSelect';
+import VirtualizedRendererWrap from '../javascripts/components/VirtualizedRendererWrap';
import './filter_box.css';
import { t } from '../javascripts/locales';
@@ -164,7 +167,7 @@ class FilterBox extends React.Component {
text: v,
metric: 0,
};
- this.props.filtersChoices[filterKey].push(addChoice);
+ this.props.filtersChoices[filterKey].unshift(addChoice);
}
}
}
@@ -177,7 +180,7 @@ class FilterBox extends React.Component {
return (
<div key={filter} className="m-b-5">
{this.props.datasource.verbose_map[filter] || filter}
- <Select.Creatable
+ <OnPasteSelect
placeholder={t('Select [%s]', filter)}
key={filter}
multi
@@ -195,6 +198,9 @@ class FilterBox extends React.Component {
return { value: opt.id, label: opt.id, style };
})}
onChange={this.changeFilter.bind(this, filter)}
+ selectComponent={Creatable}
+ selectWrap={VirtualizedSelect}
+ optionRenderer={VirtualizedRendererWrap(opt => opt.label)}
/>
</div>
);
--
To stop receiving notification emails like this one, please contact
['"commits@superset.apache.org" <co...@superset.apache.org>'].