Skip to content

Commit

Permalink
Keep the currently selected item selected in the dropdown in single s…
Browse files Browse the repository at this point in the history
…election mode (#1391)

* Keep the currently selected item selected in the dropdown in single selection mode

* Ensure activeOptionIndex only changes in single selection mode
  • Loading branch information
trevan authored and chandlerprall committed Jan 3, 2019
1 parent 121101f commit 8b81242
Show file tree
Hide file tree
Showing 5 changed files with 216 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## [`master`](https://github.com/elastic/eui/tree/master)

- Convert the other of the services to TypeScript ([#1392](https://github.com/elastic/eui/pull/1392))
- Changed single selection to select existing option in the list ([#1391](https://github.com/elastic/eui/pull/1391))

## [`6.0.1`](https://github.com/elastic/eui/tree/v6.0.1)

Expand Down
140 changes: 139 additions & 1 deletion src/components/combo_box/__snapshots__/combo_box.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ exports[`props singleSelection is rendered 1`] = `
<EuiComboBoxInput
autoSizeInputRef={[Function]}
compressed={false}
focusedOptionId={null}
focusedOptionId="htmlid__option-2"
fullWidth={false}
hasSelectedOptions={true}
inputRef={[Function]}
Expand Down Expand Up @@ -409,3 +409,141 @@ exports[`props singleSelection is rendered 1`] = `
/>
</div>
`;

exports[`props singleSelection selects existing option when opened 1`] = `
<div
aria-expanded={true}
aria-haspopup="listbox"
className="euiComboBox euiComboBox-isOpen"
onBlur={[Function]}
onKeyDown={[Function]}
role="combobox"
>
<EuiComboBoxInput
autoSizeInputRef={[Function]}
compressed={false}
focusedOptionId="htmlid__option-2"
fullWidth={false}
hasSelectedOptions={true}
inputRef={[Function]}
isListOpen={true}
noIcon={false}
onChange={[Function]}
onClear={[Function]}
onClick={[Function]}
onCloseListClick={[Function]}
onFocus={[Function]}
onOpenListClick={[Function]}
onRemoveOption={[Function]}
rootId={[Function]}
searchValue=""
selectedOptions={
Array [
Object {
"label": "Mimas",
},
]
}
singleSelection={true}
toggleButtonRef={[Function]}
updatePosition={[Function]}
value="Mimas"
/>
<EuiPortal>
<EuiComboBoxOptionsList
activeOptionIndex={2}
areAllOptionsSelected={false}
data-test-subj=""
fullWidth={false}
getSelectedOptionForSearchValue={[Function]}
listRef={[Function]}
matchingOptions={
Array [
Object {
"data-test-subj": "titanOption",
"label": "Titan",
},
Object {
"label": "Enceladus",
},
Object {
"label": "Mimas",
},
Object {
"label": "Dione",
},
Object {
"label": "Iapetus",
},
Object {
"label": "Phoebe",
},
Object {
"label": "Rhea",
},
Object {
"label": "Pandora is one of Saturn's moons, named for a Titaness of Greek mythology",
},
Object {
"label": "Tethys",
},
Object {
"label": "Hyperion",
},
]
}
onOptionClick={[Function]}
onOptionEnterKey={[Function]}
optionRef={[Function]}
options={
Array [
Object {
"data-test-subj": "titanOption",
"label": "Titan",
},
Object {
"label": "Enceladus",
},
Object {
"label": "Mimas",
},
Object {
"label": "Dione",
},
Object {
"label": "Iapetus",
},
Object {
"label": "Phoebe",
},
Object {
"label": "Rhea",
},
Object {
"label": "Pandora is one of Saturn's moons, named for a Titaness of Greek mythology",
},
Object {
"label": "Tethys",
},
Object {
"label": "Hyperion",
},
]
}
position="bottom"
rootId={[Function]}
rowHeight={27}
scrollToIndex={2}
searchValue=""
selectedOptions={
Array [
Object {
"label": "Mimas",
},
]
}
updatePosition={[Function]}
/>
</EuiPortal>
</div>
`;
50 changes: 40 additions & 10 deletions src/components/combo_box/combo_box.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,24 @@ export class EuiComboBox extends Component {
super(props);

const initialSearchValue = '';
const { options, selectedOptions } = props;
const { options, selectedOptions, singleSelection } = props;

this.state = {
matchingOptions: getMatchingOptions(options, selectedOptions, initialSearchValue, props.async),
matchingOptions: getMatchingOptions(options, selectedOptions, initialSearchValue, props.async, singleSelection),
listElement: undefined,
searchValue: initialSearchValue,
isListOpen: false,
listPosition: 'bottom',
activeOptionIndex: undefined,
};

// ensure that the currently selected single option is active if it is in the matchingOptions
if (singleSelection && selectedOptions.length === 1) {
if (this.state.matchingOptions.includes(selectedOptions[0])) {
this.state.activeOptionIndex = this.state.matchingOptions.indexOf(selectedOptions[0]);
}
}

this.rootId = htmlIdGenerator();

// Refs.
Expand Down Expand Up @@ -396,8 +403,15 @@ 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();

// If the user does this from a state in which an option has focus, then we need to reset it or clear it.
if (this.props.singleSelection && this.props.selectedOptions.length === 1) {
this.setState({
activeOptionIndex: this.state.matchingOptions.indexOf(this.props.selectedOptions[0]),
});
} else {
this.clearActiveOption();
}
};

onOpenListClick = () => {
Expand Down Expand Up @@ -457,18 +471,27 @@ export class EuiComboBox extends Component {
}

static getDerivedStateFromProps(nextProps, prevState) {
const { options, selectedOptions } = nextProps;
const { options, selectedOptions, singleSelection } = nextProps;
const { searchValue } = prevState;

// 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 = getMatchingOptions(options, selectedOptions, searchValue, nextProps.async);
const matchingOptions = getMatchingOptions(options, selectedOptions, searchValue, nextProps.async, singleSelection);
// ensure that the currently selected single option is active if it is in the matchingOptions
if (singleSelection && selectedOptions.length === 1) {
let nextActiveOptionIndex;
if (matchingOptions.includes(selectedOptions[0])) {
nextActiveOptionIndex = matchingOptions.indexOf(selectedOptions[0]);
}
return { matchingOptions, activeOptionIndex: nextActiveOptionIndex };
}

return { matchingOptions };
}

updateMatchingOptionsIfDifferent(newMatchingOptions) {
const { matchingOptions } = this.state;
const { matchingOptions, activeOptionIndex } = this.state;
const { singleSelection, selectedOptions } = this.props;

let areOptionsDifferent = false;

Expand All @@ -485,7 +508,14 @@ export class EuiComboBox extends Component {

if (areOptionsDifferent) {
this.options = [];
this.setState({ matchingOptions: newMatchingOptions });
let nextActiveOptionIndex = activeOptionIndex;
// ensure that the currently selected single option is active if it is in the matchingOptions
if (singleSelection && selectedOptions.length === 1) {
if (newMatchingOptions.includes(selectedOptions[0])) {
nextActiveOptionIndex = newMatchingOptions.indexOf(selectedOptions[0]);
}
}
this.setState({ matchingOptions: newMatchingOptions, activeOptionIndex: nextActiveOptionIndex });

if (!newMatchingOptions.length) {
// Prevent endless setState -> componentWillUpdate -> setState loop.
Expand All @@ -497,13 +527,13 @@ export class EuiComboBox extends Component {
}

componentDidUpdate() {
const { options, selectedOptions } = this.props;
const { options, selectedOptions, singleSelection } = this.props;
const { searchValue } = this.state;

// React 16.3 has a bug (fixed in 16.4) where getDerivedStateFromProps
// isn't called after a state change, and we track `searchValue` in state
// instead we need to react to a change in searchValue here
this.updateMatchingOptionsIfDifferent(getMatchingOptions(options, selectedOptions, searchValue, this.props.async));
this.updateMatchingOptionsIfDifferent(getMatchingOptions(options, selectedOptions, searchValue, this.props.async, singleSelection));
}

componentWillUnmount() {
Expand Down
32 changes: 23 additions & 9 deletions src/components/combo_box/combo_box.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,16 +97,30 @@ describe('props', () => {
});
});

test('singleSelection is rendered', () => {
const component = shallow(
<EuiComboBox
options={options}
selectedOptions={[options[2]]}
singleSelection={true}
/>
);
describe('singleSelection', () => {
test('is rendered', () => {
const component = shallow(
<EuiComboBox
options={options}
selectedOptions={[options[2]]}
singleSelection={true}
/>
);

expect(component).toMatchSnapshot();
expect(component).toMatchSnapshot();
});
test('selects existing option when opened', () => {
const component = shallow(
<EuiComboBox
options={options}
selectedOptions={[options[2]]}
singleSelection={true}
/>
);

component.setState({ isListOpen: true });
expect(component).toMatchSnapshot();
});
});

test('isDisabled is rendered', () => {
Expand Down
19 changes: 13 additions & 6 deletions src/components/combo_box/matching_options.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ export const getSelectedOptionForSearchValue = (searchValue, selectedOptions) =>
return selectedOptions.find(option => option.label.toLowerCase() === normalizedSearchValue);
};

const collectMatchingOption = (accumulator, option, selectedOptions, normalizedSearchValue, isPreFiltered) => {
// Only show options which haven't yet been selected.
const collectMatchingOption = (accumulator, option, selectedOptions, normalizedSearchValue, isPreFiltered, showPrevSelected) => {
// Only show options which haven't yet been selected unless requested.
const selectedOption = getSelectedOptionForSearchValue(option.label, selectedOptions);
if (selectedOption) {
if (selectedOption && !showPrevSelected) {
return false;
}

Expand All @@ -38,15 +38,22 @@ const collectMatchingOption = (accumulator, option, selectedOptions, normalizedS
}
};

export const getMatchingOptions = (options, selectedOptions, searchValue, isPreFiltered) => {
export const getMatchingOptions = (options, selectedOptions, searchValue, isPreFiltered, showPrevSelected) => {
const normalizedSearchValue = searchValue.trim().toLowerCase();
const matchingOptions = [];

options.forEach(option => {
if (option.options) {
const matchingOptionsForGroup = [];
option.options.forEach(groupOption => {
collectMatchingOption(matchingOptionsForGroup, groupOption, selectedOptions, normalizedSearchValue, isPreFiltered);
collectMatchingOption(
matchingOptionsForGroup,
groupOption,
selectedOptions,
normalizedSearchValue,
isPreFiltered,
showPrevSelected
);
});
if (matchingOptionsForGroup.length > 0) {
// Add option for group label
Expand All @@ -55,7 +62,7 @@ export const getMatchingOptions = (options, selectedOptions, searchValue, isPreF
matchingOptions.push(...matchingOptionsForGroup);
}
} else {
collectMatchingOption(matchingOptions, option, selectedOptions, normalizedSearchValue, isPreFiltered);
collectMatchingOption(matchingOptions, option, selectedOptions, normalizedSearchValue, isPreFiltered, showPrevSelected);
}
});
return matchingOptions;
Expand Down

0 comments on commit 8b81242

Please sign in to comment.