Skip to content

Commit

Permalink
[EuiSearchBar] Fix multiSelect: false filter behavior (#6577)
Browse files Browse the repository at this point in the history
* Fix broken behavior for `multiSelect: false` filters

- the diff being returned was catching on the first item (now unflagged) and sending that instead

- instead of using an unnecessary `.find`, use our new 3rd `changedOption` arg from `EuiSelectable` to immediately get the changed/selected option

- nb: this does require flipping all ternaries on `onOptionClick` b/c the new `changedOption` correctly sends the new updated state

* Add regression tests for both multiselect false/or/true behavior

* changelog
  • Loading branch information
cee-chen authored Feb 6, 2023
1 parent 096e35f commit 3c2a9a9
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
* Side Public License, v 1.
*/

import React from 'react';
/// <reference types="../../../../cypress/support"/>

import React, { useState } from 'react';
import { requiredProps } from '../../../test';
import {
FieldValueSelectionFilter,
Expand Down Expand Up @@ -135,31 +137,90 @@ describe('FieldValueSelectionFilter', () => {
);
});

it('uses multi-select OR', () => {
const props: FieldValueSelectionFilterProps = {
...requiredProps,
index: 0,
onChange: () => {},
query: Query.parse(''),
config: {
type: 'field_value_selection',
field: 'tag',
name: 'Tag',
multiSelect: 'or',
available: () => false,
loadingMessage: 'loading...',
noOptionsMessage: 'oops...',
searchThreshold: 5,
options: () => Promise.resolve([]),
},
describe('multi-select testing', () => {
const FieldValueSelectionFilterWithState = ({
multiSelect,
}: {
multiSelect: 'or' | boolean;
}) => {
const [query, setQuery] = useState(Query.parse(''));
const onChange = (newQuery) => setQuery(newQuery);

const props: FieldValueSelectionFilterProps = {
...requiredProps,
index: 0,
onChange,
query,
config: {
type: 'field_value_selection',
field: 'tag',
name: 'Tag',
multiSelect,
options: staticOptions,
},
};

return <FieldValueSelectionFilter {...props} />;
};

cy.mount(<FieldValueSelectionFilter {...props} />);
cy.get('button').click();
cy.get('[data-test-subj="euiSelectableMessage"]').should(
'have.text',
'oops...'
);
it('uses multi-select OR', () => {
cy.mount(<FieldValueSelectionFilterWithState multiSelect="or" />);
cy.get('button').click();

cy.get('li[role="option"][title="feature"]')
.should('have.attr', 'aria-checked', 'false')
.click();
cy.get('.euiNotificationBadge').should('have.text', '1');
cy.get('li[role="option"][title="feature"]').should(
'have.attr',
'aria-checked',
'true'
);
// Popover should still be open when multiselect is true/or
cy.get('li[role="option"][title="Bug"]')
.should('have.attr', 'aria-checked', 'false')
.click();
cy.get('.euiNotificationBadge').should('have.text', '2');
cy.get('li[role="option"][title="Bug"]').should(
'have.attr',
'aria-checked',
'true'
);
});

it('uses multi-select false', () => {
cy.mount(<FieldValueSelectionFilterWithState multiSelect={false} />);
cy.get('button').click();

cy.get('li[role="option"][title="feature"]')
.should('have.attr', 'aria-checked', 'false')
.click();
cy.get('.euiNotificationBadge').should('have.text', '1');
cy.get('li[role="option"][title="feature"]').should(
'have.attr',
'aria-checked',
'true'
);

// Multiselect false should close the popover, so we need to re-open it
cy.get('button').click();
cy.get('li[role="option"][title="Bug"]')
.should('have.attr', 'aria-checked', 'false')
.click();
// Filter count should have remained at 1
cy.get('.euiNotificationBadge').should('have.text', '1');
cy.get('li[role="option"][title="Bug"]').should(
'have.attr',
'aria-checked',
'true'
);
// 'featured' should now be unchecked
cy.get('li[role="option"][title="feature"]').should(
'have.attr',
'aria-checked',
'false'
);
});
});

it('has inactive filters, field is global', () => {
Expand Down
27 changes: 12 additions & 15 deletions src/components/search_bar/filters/field_value_selection_filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -259,23 +259,23 @@ export class FieldValueSelectionFilter extends Component<
if (!multiSelect && autoClose) {
this.closePopover();
const query = checked
? this.props.query.removeSimpleFieldClauses(field)
: this.props.query
? this.props.query
.removeSimpleFieldClauses(field)
.addSimpleFieldValue(field, value, true, operator);
.addSimpleFieldValue(field, value, true, operator)
: this.props.query.removeSimpleFieldClauses(field);

this.props.onChange(query);
} else {
if (multiSelect === 'or') {
const query = checked
? this.props.query.removeOrFieldValue(field, value)
: this.props.query.addOrFieldValue(field, value, true, operator);
? this.props.query.addOrFieldValue(field, value, true, operator)
: this.props.query.removeOrFieldValue(field, value);

this.props.onChange(query);
} else {
const query = checked
? this.props.query.removeSimpleFieldValue(field, value)
: this.props.query.addSimpleFieldValue(field, value, true, operator);
? this.props.query.addSimpleFieldValue(field, value, true, operator)
: this.props.query.removeSimpleFieldValue(field, value);

this.props.onChange(query);
}
Expand Down Expand Up @@ -404,15 +404,12 @@ export class FieldValueSelectionFilter extends Component<
listProps={{
isVirtualized: isOverSearchThreshold || false,
}}
onChange={(options) => {
const diff = items.find(
(item, index) => item.checked !== options[index].checked
);
if (diff) {
onChange={(options, event, changedOption) => {
if (changedOption.data) {
this.onOptionClick(
diff.data.optionField,
diff.data.value,
diff.checked
changedOption.data.optionField,
changedOption.data.value,
changedOption.checked
);
}
}}
Expand Down
3 changes: 3 additions & 0 deletions upcoming_changelogs/6577.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed a bug with `EuiSearchBar` where filters with `multiSelect: false` were not able to select a new option when an option was already selected

0 comments on commit 3c2a9a9

Please sign in to comment.