-
Notifications
You must be signed in to change notification settings - Fork 841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use react-virtualized to virtualize EuiComboBox options list #670
Changes from all commits
48d13c8
c0722c6
09730ba
9fa163f
7380a41
fd0214a
b024db6
d18690e
e417a18
85550ff
68c9826
fde8a5b
f4426ab
c355fed
47fa3ef
cc4451d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,10 @@ import Async from './async'; | |
const asyncSource = require('!!raw-loader!./async'); | ||
const asyncHtml = renderToHtml(Async); | ||
|
||
import Virtualized from './virtualized'; | ||
const virtualizedSource = require('!!raw-loader!./virtualized'); | ||
const virtualizedHtml = renderToHtml(Virtualized); | ||
|
||
export const ComboBoxExample = { | ||
title: 'Combo Box', | ||
intro: ( | ||
|
@@ -94,6 +98,23 @@ export const ComboBoxExample = { | |
}], | ||
props: { EuiComboBox }, | ||
demo: <ComboBox />, | ||
}, { | ||
title: 'Virtualized', | ||
source: [{ | ||
type: GuideSectionTypes.JS, | ||
code: virtualizedSource, | ||
}, { | ||
type: GuideSectionTypes.HTML, | ||
code: virtualizedHtml, | ||
}], | ||
text: ( | ||
<p> | ||
<EuiCode>EuiComboBoxList</EuiCode> uses <Link to="https://github.com/bvaughn/react-virtualized">react-virtualized</Link>{' '} | ||
to only render visible options to be super fast no matter how many options there are. | ||
</p> | ||
), | ||
props: { EuiComboBox }, | ||
demo: <Virtualized />, | ||
}, { | ||
title: 'Containers', | ||
source: [{ | ||
|
@@ -140,11 +161,19 @@ export const ComboBoxExample = { | |
code: renderOptionHtml, | ||
}], | ||
text: ( | ||
<p> | ||
You can provide a <EuiCode>renderOption</EuiCode> prop which will accept <EuiCode>option</EuiCode> | ||
and <EuiCode>searchValue</EuiCode> arguments. Use the <EuiCode>value</EuiCode> prop of the | ||
<EuiCode>option</EuiCode> object to store metadata about the option for use in this callback. | ||
</p> | ||
<Fragment> | ||
<p> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will fix some more spacing issues: <p>
You can provide a <EuiCode>renderOption</EuiCode> prop which will accept <EuiCode>option</EuiCode>{' '}
and <EuiCode>searchValue</EuiCode> arguments. Use the <EuiCode>value</EuiCode> prop of the{' '}
<EuiCode>option</EuiCode> object to store metadata about the option for use in this callback.
</p> |
||
You can provide a <EuiCode>renderOption</EuiCode> prop which will accept <EuiCode>option</EuiCode>{' '} | ||
and <EuiCode>searchValue</EuiCode> arguments. Use the <EuiCode>value</EuiCode> prop of the{' '} | ||
<EuiCode>option</EuiCode> object to store metadata about the option for use in this callback. | ||
</p> | ||
|
||
<p> | ||
<strong>Note:</strong> virtualization (above) requires that each option have the same height. | ||
Ensure that you render the options so that wrapping text is truncated instead of causing | ||
the height of the option to change. | ||
</p> | ||
</Fragment> | ||
), | ||
props: { EuiComboBox }, | ||
demo: <RenderOption />, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import React, { Component } from 'react'; | ||
|
||
import { | ||
EuiComboBox, | ||
} from '../../../../src/components'; | ||
|
||
export default class extends Component { | ||
constructor(props) { | ||
super(props); | ||
|
||
this.options = []; | ||
let groupOptions = []; | ||
for (let i=1; i < 5000; i++) { | ||
groupOptions.push({ label: `option${i}` }); | ||
if (i % 25 === 0) { | ||
this.options.push({ | ||
label: `Options ${i - (groupOptions.length - 1)} to ${i}`, | ||
options: groupOptions | ||
}); | ||
groupOptions = []; | ||
} | ||
} | ||
|
||
this.state = { | ||
selectedOptions: [], | ||
}; | ||
} | ||
|
||
onChange = (selectedOptions) => { | ||
this.setState({ | ||
selectedOptions, | ||
}); | ||
}; | ||
|
||
render() { | ||
const { selectedOptions } = this.state; | ||
return ( | ||
<EuiComboBox | ||
placeholder="Select or create options" | ||
options={this.options} | ||
selectedOptions={selectedOptions} | ||
onChange={this.onChange} | ||
/> | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
* from the tab order with tabindex="-1" so that we can control the keyboard navigation interface. | ||
*/ | ||
|
||
import { throttle } from 'lodash'; | ||
import React, { | ||
Component, | ||
} from 'react'; | ||
|
@@ -38,6 +39,7 @@ export class EuiComboBox extends Component { | |
onCreateOption: PropTypes.func, | ||
renderOption: PropTypes.func, | ||
isInvalid: PropTypes.bool, | ||
rowHeight: PropTypes.number, | ||
} | ||
|
||
static defaultProps = { | ||
|
@@ -50,18 +52,17 @@ export class EuiComboBox extends Component { | |
|
||
const initialSearchValue = ''; | ||
const { options, selectedOptions } = props; | ||
const { matchingOptions, optionToGroupMap } = this.getMatchingOptions(options, selectedOptions, initialSearchValue); | ||
const matchingOptions = this.getMatchingOptions(options, selectedOptions, initialSearchValue); | ||
|
||
this.state = { | ||
searchValue: initialSearchValue, | ||
isListOpen: false, | ||
listPosition: 'bottom', | ||
activeOptionIndex: undefined, | ||
}; | ||
|
||
// Cached derived state. | ||
this.matchingOptions = matchingOptions; | ||
this.optionToGroupMap = optionToGroupMap; | ||
this.activeOptionIndex = undefined; | ||
this.listBounds = undefined; | ||
|
||
// Refs. | ||
|
@@ -122,6 +123,7 @@ export class EuiComboBox extends Component { | |
this.optionsList.style.width = `${comboBoxBounds.width}px`; | ||
|
||
this.setState({ | ||
width: comboBoxBounds.width, | ||
listPosition: position, | ||
}); | ||
}; | ||
|
@@ -149,7 +151,7 @@ export class EuiComboBox extends Component { | |
tabbableItems[comboBoxIndex + amount].focus(); | ||
}; | ||
|
||
incrementActiveOptionIndex = amount => { | ||
incrementActiveOptionIndex = throttle(amount => { | ||
// If there are no options available, reset the focus. | ||
if (!this.matchingOptions.length) { | ||
this.clearActiveOption(); | ||
|
@@ -161,33 +163,49 @@ export class EuiComboBox extends Component { | |
if (!this.hasActiveOption()) { | ||
// If this is the beginning of the user's keyboard navigation of the menu, then we'll focus | ||
// either the first or last item. | ||
nextActiveOptionIndex = amount < 0 ? this.options.length - 1 : 0; | ||
nextActiveOptionIndex = amount < 0 ? this.matchingOptions.length - 1 : 0; | ||
} else { | ||
nextActiveOptionIndex = this.activeOptionIndex + amount; | ||
nextActiveOptionIndex = this.state.activeOptionIndex + amount; | ||
|
||
if (nextActiveOptionIndex < 0) { | ||
nextActiveOptionIndex = this.options.length - 1; | ||
} else if (nextActiveOptionIndex === this.options.length) { | ||
nextActiveOptionIndex = this.matchingOptions.length - 1; | ||
} else if (nextActiveOptionIndex === this.matchingOptions.length) { | ||
nextActiveOptionIndex = 0; | ||
} | ||
} | ||
|
||
this.activeOptionIndex = nextActiveOptionIndex; | ||
this.focusActiveOption(); | ||
}; | ||
// Group titles are included in option list but are not selectable | ||
// Skip group title options | ||
const direction = amount > 0 ? 1 : -1; | ||
while (this.matchingOptions[nextActiveOptionIndex].isGroupLabelOption) { | ||
nextActiveOptionIndex = nextActiveOptionIndex + direction; | ||
|
||
if (nextActiveOptionIndex < 0) { | ||
nextActiveOptionIndex = this.matchingOptions.length - 1; | ||
} else if (nextActiveOptionIndex === this.matchingOptions.length) { | ||
nextActiveOptionIndex = 0; | ||
} | ||
} | ||
|
||
this.setState({ | ||
activeOptionIndex: nextActiveOptionIndex, | ||
}); | ||
}, 200); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 200 feels a bit sluggish to me -- 100 feels snappier, can we use that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. with 100 I was still getting the keydown to be faster than the rendering and I could hold the key down and the focus option would not keep up |
||
|
||
hasActiveOption = () => { | ||
return this.activeOptionIndex !== undefined; | ||
return this.state.activeOptionIndex !== undefined; | ||
}; | ||
|
||
clearActiveOption = () => { | ||
this.activeOptionIndex = undefined; | ||
this.setState({ | ||
activeOptionIndex: undefined, | ||
}); | ||
}; | ||
|
||
focusActiveOption = () => { | ||
// If an item is focused, focus it. | ||
if (this.hasActiveOption()) { | ||
this.options[this.activeOptionIndex].focus(); | ||
if (this.hasActiveOption() && this.options[this.state.activeOptionIndex]) { | ||
this.options[this.state.activeOptionIndex].focus(); | ||
} | ||
}; | ||
|
||
|
@@ -366,6 +384,8 @@ export class EuiComboBox extends Component { | |
onComboBoxClick = () => { | ||
// When the user clicks anywhere on the box, enter the interaction state. | ||
this.searchInput.focus(); | ||
// If the user does this from a state in which an option has focus, then we need to clear it. | ||
this.clearActiveOption(); | ||
}; | ||
|
||
onComboBoxFocus = (e) => { | ||
|
@@ -379,7 +399,9 @@ export class EuiComboBox extends Component { | |
// and we need to update the index. | ||
const optionIndex = this.options.indexOf(e.target); | ||
if (optionIndex !== -1) { | ||
this.activeOptionIndex = optionIndex; | ||
this.setState({ | ||
activeOptionIndex: optionIndex, | ||
}); | ||
} | ||
}; | ||
|
||
|
@@ -392,6 +414,12 @@ export class EuiComboBox extends Component { | |
|
||
comboBoxRef = node => { | ||
this.comboBox = node; | ||
if (this.comboBox) { | ||
const comboBoxBounds = this.comboBox.getBoundingClientRect(); | ||
this.setState({ | ||
width: comboBoxBounds.width, | ||
}); | ||
} | ||
}; | ||
|
||
autoSizeInputRef = node => { | ||
|
@@ -407,11 +435,7 @@ export class EuiComboBox extends Component { | |
}; | ||
|
||
optionRef = (index, node) => { | ||
// Sometimes the node is null. | ||
if (node) { | ||
// Store all options. | ||
this.options[index] = node; | ||
} | ||
this.options[index] = node; | ||
}; | ||
|
||
componentDidMount() { | ||
|
@@ -436,12 +460,14 @@ export class EuiComboBox extends Component { | |
|
||
// Calculate and cache the options which match the searchValue, because we use this information | ||
// in multiple places and it would be expensive to calculate repeatedly. | ||
const { matchingOptions, optionToGroupMap } = this.getMatchingOptions(options, selectedOptions, nextState.searchValue); | ||
const matchingOptions = this.getMatchingOptions(options, selectedOptions, nextState.searchValue); | ||
this.matchingOptions = matchingOptions; | ||
this.optionToGroupMap = optionToGroupMap; | ||
|
||
if (!matchingOptions.length) { | ||
this.clearActiveOption(); | ||
// Prevent endless setState -> componentWillUpdate -> setState loop. | ||
if (nextState.hasActiveOption) { | ||
this.clearActiveOption(); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -470,10 +496,11 @@ export class EuiComboBox extends Component { | |
onSearchChange, // eslint-disable-line no-unused-vars | ||
async, // eslint-disable-line no-unused-vars | ||
isInvalid, | ||
rowHeight, | ||
...rest | ||
} = this.props; | ||
|
||
const { searchValue, isListOpen, listPosition } = this.state; | ||
const { searchValue, isListOpen, listPosition, width, activeOptionIndex } = this.state; | ||
|
||
const classes = classNames('euiComboBox', className, { | ||
'euiComboBox-isOpen': isListOpen, | ||
|
@@ -494,7 +521,6 @@ export class EuiComboBox extends Component { | |
onCreateOption={onCreateOption} | ||
searchValue={searchValue} | ||
matchingOptions={this.matchingOptions} | ||
optionToGroupMap={this.optionToGroupMap} | ||
listRef={this.optionsListRef} | ||
optionRef={this.optionRef} | ||
onOptionClick={this.onOptionClick} | ||
|
@@ -504,6 +530,10 @@ export class EuiComboBox extends Component { | |
updatePosition={this.updateListPosition} | ||
position={listPosition} | ||
renderOption={renderOption} | ||
width={width} | ||
scrollToIndex={activeOptionIndex} | ||
onScroll={this.focusActiveOption} | ||
rowHeight={rowHeight} | ||
/> | ||
</EuiPortal> | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we tweak this to fix some spacing and a typo: