Skip to content

Commit

Permalink
[Fix][Perf] String filter freezes browser when loading a large dataset (
Browse files Browse the repository at this point in the history 
#2012)

use infinite scroll to improve the performance of string filter on a large dataset
add IntersectionObserver mockup for browser test; update dropdown-list for testing

Signed-off-by: Ihor Dykhta dikhta.igor@gmail.com
Co-authored-by: Xun Li <lixun910@gmail.com>
  • Loading branch information
@igorDykhta @lixun910
igorDykhta and lixun910 authored Nov 22, 2022
1 parent 1214bd9 commit d0bcaa8
Show file tree
Hide file tree
Showing 6 changed files with 247 additions and 49 deletions.
98 changes: 95 additions & 3 deletions src/components/src/common/item-selector/dropdown-list.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import React, {Component, ElementType} from 'react';
import classNames from 'classnames';
import styled from 'styled-components';
import {INIT_FILTER_ITEMS_IN_DROPDOWN} from '@kepler.gl/constants';

export const classList = {
list: 'list-selector',
Expand Down Expand Up @@ -53,6 +54,10 @@ const DropdownListWrapper = styled.div<DropdownListWrapperProps>`
${props => (props.light ? props.theme.dropdownListLT : props.theme.dropdownList)};
`;

const DropdownFooterWrapper = styled.div`
height: '0px';
`;

interface DropdownListProps {
options?: any[];
allowCustomValues?: number;
Expand All @@ -71,7 +76,11 @@ interface DropdownListProps {
fixedOptions?: any[];
}

export default class DropdownList extends Component<DropdownListProps> {
interface DropdownListState {
options: Array<any> | null;
}

export default class DropdownList extends Component<DropdownListProps, DropdownListState> {
static defaultProps = {
customClasses: {},
customListItemComponent: ListItem,
Expand All @@ -84,6 +93,87 @@ export default class DropdownList extends Component<DropdownListProps> {
selectionIndex: null
};

initNumberOfOptions: number;
page: number;
prevY: number;
loadingRef: React.RefObject<HTMLDivElement>;
observer: IntersectionObserver | undefined;

constructor(props) {
super(props);

this.state = {options: []};
this.initNumberOfOptions = INIT_FILTER_ITEMS_IN_DROPDOWN;
this.page = 0;
this.prevY = 0;
this.loadingRef = React.createRef();
}

componentDidMount() {
const options = this._getOptions(this.page);
this.setState({options});

const divOptions = {
root: null,
rootMargin: '0%',
threshold: 1.0
};

if (this.loadingRef.current) {
this.observer = new IntersectionObserver(this.handleObserver, divOptions);
this.observer.observe(this.loadingRef.current);
}
}

getSnapshotBeforeUpdate(prevProps: DropdownListProps, prevState: DropdownListState) {
if (prevProps.options?.length !== this.props.options?.length) {
// check if user searching, reset state.options at the first time
const options = this._getOptions(0);
this.setState({options});
}
return null;
}

// prevent console warning: getSnapshotBeforeUpdate() should be used with componentDidUpdate().
componentDidUpdate(prevProps, prevState, snapshot) {}

componentWillUnmount() {
if (this.loadingRef.current) {
this.observer?.unobserve(this.loadingRef.current);
}
}

handleObserver = entities => {
const y = entities[0].boundingClientRect.y;
if (this.prevY > y) {
const options = this._getOptions(this.page);
if (options) this.setState({options});
}
this.prevY = y;
};

_getOptions(page) {
if(!this.props.options){
return [];
}

const n = this.props.options.length;
if (n === 0) {
return [];
}
const start = page * this.initNumberOfOptions;
const end = start + this.initNumberOfOptions > n ? n : start + this.initNumberOfOptions;

if (start < end && end <= n) {
this.page = page + 1;
// in case of user searching, props.options will be updated
// so "page" value will be set to 0 and previous state.options will be discarded
return [...(page > 0 ? (this.state.options || []) : []), ...this.props.options.slice(start, end)];
}

return null;
}

_onClick(result, event) {
event.preventDefault();
this.props.onOptionSelected?.(result, event);
Expand All @@ -100,7 +190,7 @@ export default class DropdownList extends Component<DropdownListProps> {

// Don't render if there are no options to display
if (!this.props.options?.length && allowCustomValues <= 0) {
return false;
return <div />;;
}

const valueOffset = Array.isArray(fixedOptions) ? fixedOptions.length : 0;
Expand Down Expand Up @@ -133,7 +223,7 @@ export default class DropdownList extends Component<DropdownListProps> {
</div>
) : null}

{this.props.options?.map((value, i) => (
{this.state.options?.map((value, i) => (
<div
className={classNames(classList.listItem, {
hover: this.props.selectionIndex === i + valueOffset
Expand All @@ -145,6 +235,8 @@ export default class DropdownList extends Component<DropdownListProps> {
<CustomListItemComponent value={value} displayOption={display} />
</div>
))}

<DropdownFooterWrapper ref={this.loadingRef} />
</DropdownListWrapper>
);
}
Expand Down
62 changes: 16 additions & 46 deletions src/components/src/common/item-selector/typeahead.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,40 +94,21 @@ function generateSearchFunction(props: TypeaheadProps) {
? Accessor.generateAccessor(filterOption)
: Accessor.IDENTITY_FN;

return (value, options) =>
fuzzy.filter(value, options, {extract: mapper}).map(res => options[res.index]);
return (value, options) => {
return fuzzy.filter(value, options, {extract: mapper}).map(res => options[res.index]);
};
}

function getOptionsForValue(value, props, state) {
const {options, showOptionsWhenEmpty} = props;

if (!props.searchable) {
// directly pass through options if can not be searched
return options;
}
if (shouldSkipSearch(value, state, showOptionsWhenEmpty)) {
return options;
}

function searchOptionsOnInput(inputValue, props) {
const searchOptions = generateSearchFunction(props);
return searchOptions(value, options);
}

function shouldSkipSearch(input, state, showOptionsWhenEmpty) {
const emptyValue = !input || input.trim().length === 0;

// this.state must be checked because it may not be defined yet if this function
// is called from within getInitialState
const isFocused = state && state.isFocused;
return !(showOptionsWhenEmpty && isFocused) && emptyValue;
return searchOptions(inputValue, props.options);
}

interface TypeaheadProps {
name?: string;
customClasses?: any;
maxVisible?: number;
resultsTruncatedMessage?: string;
options?: ReadonlyArray<string | number | boolean | object>;
options?: ReadonlyArray<string | number | boolean | object | undefined>;
fixedOptions?: ReadonlyArray<string | number | boolean | object> | null;
allowCustomValues?: number;
initialValue?: string;
Expand Down Expand Up @@ -158,11 +139,12 @@ interface TypeaheadProps {
inputIcon: ElementType;
className?: string;
selectedItems?: any[] | null;
// deprecated
maxVisible?: number;
}

interface TypeaheadState {
/** @type {ReadonlyArray<string>} */
searchResults: (string | undefined)[];
searchResults: ReadonlyArray<string | number | boolean | object | undefined>;

// This should be called something else, 'entryValue'
entryValue?: string;
Expand Down Expand Up @@ -209,19 +191,12 @@ class Typeahead extends Component<TypeaheadProps, TypeaheadState> {
resultsTruncatedMessage: null
};

static getDerivedStateFromProps(props, state) {
// invoked after a component is instantiated as well as before it is re-rendered
const searchResults = getOptionsForValue(state.entryValue, props, state);

return {searchResults};
}

constructor(props) {
super(props);

this.state = {
/** @type {ReadonlyArray<string>} */
searchResults: [],
// initiate searchResults with options
searchResults: this.props.options || [],

// This should be called something else, 'entryValue'
entryValue: this.props.value || this.props.initialValue,
Expand Down Expand Up @@ -273,14 +248,8 @@ class Typeahead extends Component<TypeaheadProps, TypeaheadState> {
return (
<CustomListComponent
fixedOptions={this.props.fixedOptions}
options={
this.props.maxVisible
? this.state.searchResults.slice(0, this.props.maxVisible)
: this.state.searchResults
}
areResultsTruncated={
this.props.maxVisible && this.state.searchResults.length > this.props.maxVisible
}
options={this.state.searchResults}
areResultsTruncated={false}
resultsTruncatedMessage={this.props.resultsTruncatedMessage}
onOptionSelected={this._onOptionSelected}
allowCustomValues={this.props.allowCustomValues}
Expand Down Expand Up @@ -322,7 +291,8 @@ class Typeahead extends Component<TypeaheadProps, TypeaheadState> {
if (this.props.searchable) {
// reset entry input
this.setState({
searchResults: getOptionsForValue('', this.props, this.state),
// reset search options when selection has been made
searchResults: this.props.options || [],
selection: '',
entryValue: ''
});
Expand All @@ -337,7 +307,7 @@ class Typeahead extends Component<TypeaheadProps, TypeaheadState> {
const value = this.entry.current?.value;

this.setState({
searchResults: getOptionsForValue(value, this.props, this.state),
searchResults: searchOptionsOnInput(value, this.props),
selection: '',
entryValue: value
});
Expand Down
3 changes: 3 additions & 0 deletions src/constants/src/default-settings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,9 @@ export const PLOT_TYPES = keyMirror({
lineChart: null
});

// Filter
export const INIT_FILTER_ITEMS_IN_DROPDOWN = 100;

// GPU Filtering
/**
* Max number of filter value buffers that deck.gl provides
Expand Down
100 changes: 100 additions & 0 deletions test/browser/components/common/item-selector-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,103 @@ test('Components -> ItemSelector.render', t => {
t.deepEqual(onChange.args[0], ['additive'], 'should call additive');
t.end();
});

test('Components -> ItemSelector.render over 100 options', t => {
let wrapper;
const onChange = sinon.spy();
const randomOptions = Array.from({length: 120}, () =>
Math.random()
.toString(16)
.substr(2, 8)
);
const props = {
selectedItems: '',
options: randomOptions,
multiSelect: false,
searchable: false,
onChange
};

t.doesNotThrow(() => {
wrapper = mountWithTheme(
<IntlWrapper>
<ItemSelector {...props} />
</IntlWrapper>
);
}, 'Show not fail without props');

t.equal(wrapper.find('.item-selector__dropdown').length, 1, 'should render DropdownSelect');
t.equal(wrapper.find(Typeahead).length, 0, 'should not render Typeahead');
t.equal(
wrapper
.find('.item-selector__dropdown')
.at(0)
.find('.list__item__anchor')
.text(),
'',
'should render no select value'
);

// click dropdown select
wrapper
.find('.item-selector__dropdown')
.at(0)
.simulate('click');

t.equal(wrapper.find(Typeahead).length, 1, 'should render Typeahead');
t.equal(wrapper.find(DropdownList).length, 1, 'should render 1 Typeahead');
t.equal(
wrapper
.find(DropdownList)
.at(0)
.find(ListItem).length,
100,
'should render first 100 ListItem'
);

// mockup scroll by triggering handleObserver() of IntersectionObserver
const mockedEntries = [
{
isIntersecting: true,
boundingClientRect: {
x: 77,
y: 77,
width: 231,
height: 0,
top: 777,
right: 308,
bottom: 777,
left: 77
}
}
];
const dropdown = wrapper.find(DropdownList).instance();
dropdown.prevY = 100;
dropdown.handleObserver(mockedEntries);
// update component
wrapper.update();

t.equal(
wrapper
.find(DropdownList)
.at(0)
.find(ListItem).length,
120,
'should render all 120 ListItem'
);

// mockup scroll again
dropdown.prevY = 110;
dropdown.handleObserver(mockedEntries);
wrapper.update();
t.equal(
wrapper
.find(DropdownList)
.at(0)
.find(ListItem).length,
120,
'should still render all 120 ListItem'
);

t.end();
});
13 changes: 13 additions & 0 deletions test/browser/components/helpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,16 @@ export function useTraceUpdate(props) {
prev.current = props;
});
}

export function useTraceUpdateClass(props, prev) {
const changedProps = Object.entries(props).reduce((ps, [k, v]) => {
if (prev[k] !== v) {
ps[k] = [prev[k], v];
}
return ps;
}, {});
if (Object.keys(changedProps).length > 0) {
// eslint-disable-next-line no-console
console.log('Changed props:', changedProps);
}
}
Loading

0 comments on commit d0bcaa8

Please sign in to comment.