Skip to content

Commit

Permalink
[EuiComboBox] Fix onSearchChange bug with incorrect `hasMatchingOpt…
Browse files Browse the repository at this point in the history
…ions` data (#7334)
  • Loading branch information
cee-chen authored Nov 2, 2023
1 parent 5cab9cb commit 197e104
Show file tree
Hide file tree
Showing 7 changed files with 607 additions and 598 deletions.
18 changes: 9 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,13 @@
"@emotion/react": "^11.11.0",
"@faker-js/faker": "^8.0.2",
"@pmmmwh/react-refresh-webpack-plugin": "^0.5.3",
"@storybook/addon-essentials": "^7.3.1",
"@storybook/addon-interactions": "^7.3.1",
"@storybook/addon-links": "^7.3.1",
"@storybook/blocks": "^7.3.1",
"@storybook/react": "^7.3.1",
"@storybook/react-webpack5": "^7.3.1",
"@storybook/testing-library": "^0.1.0",
"@storybook/addon-essentials": "^7.5.2",
"@storybook/addon-interactions": "^7.5.2",
"@storybook/addon-links": "^7.5.2",
"@storybook/blocks": "^7.5.2",
"@storybook/react": "^7.5.2",
"@storybook/react-webpack5": "^7.5.2",
"@storybook/testing-library": "^0.2.2",
"@svgr/core": "8.0.0",
"@svgr/plugin-jsx": "^8.0.1",
"@svgr/plugin-svgo": "^8.0.1",
Expand Down Expand Up @@ -192,7 +192,7 @@
"eslint-plugin-prettier": "^4.2.1",
"eslint-plugin-react": "^7.32.2",
"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-storybook": "^0.6.13",
"eslint-plugin-storybook": "^0.6.15",
"expose-gc": "^1.0.0",
"file-loader": "^6.1.0",
"findup": "^0.1.5",
Expand Down Expand Up @@ -238,7 +238,7 @@
"sass-loader": "^13.2.0",
"shelljs": "^0.8.4",
"start-server-and-test": "^1.11.3",
"storybook": "^7.3.1",
"storybook": "^7.5.2",
"style-loader": "^3.3.1",
"stylelint": "^15.7.0",
"stylelint-config-prettier-scss": "^1.0.0",
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/combo_box/async.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export default () => {
// Create the option if it doesn't exist.
if (
flattenedOptions.findIndex(
(option) => option.value.trim().toLowerCase() === normalizedSearchValue
(option) => option.label.trim().toLowerCase() === normalizedSearchValue
) === -1
) {
// Simulate creating this option on the server.
Expand Down
68 changes: 68 additions & 0 deletions src/components/combo_box/combo_box.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { Meta, StoryObj } from '@storybook/react';
import React, { useCallback, useState } from 'react';

import { EuiComboBox, EuiComboBoxProps } from './combo_box';

const options = [
{ label: 'Item 1' },
{ label: 'Item 2' },
{ label: 'Item 3' },
{ label: 'Item 4' },
{ label: 'Item 5' },
];

const meta: Meta<EuiComboBoxProps<{}>> = {
title: 'EuiComboBox',
// @ts-ignore typescript shenanigans
component: EuiComboBox,
args: {
options: options,
selectedOptions: [options[0]],
singleSelection: false,
},
argTypes: {
singleSelection: {
control: 'radio',
options: [false, true, 'asPlainText'],
},
append: { control: 'text' },
prepend: { control: 'text' },
},
};

export default meta;
type Story = StoryObj<EuiComboBoxProps<{}>>;

export const Playground: Story = {
// The render function is a component, eslint just doesn't know it
/* eslint-disable react-hooks/rules-of-hooks */
render: ({ singleSelection, ...args }) => {
const [selectedOptions, setSelectedOptions] = useState(
args.selectedOptions
);
const onChange = useCallback((newOptions: any[]) => {
setSelectedOptions(newOptions);
}, []);
return (
<EuiComboBox
singleSelection={
// @ts-ignore Specific to Storybook control
singleSelection === 'asPlainText'
? { asPlainText: true }
: Boolean(singleSelection)
}
{...args}
selectedOptions={selectedOptions}
onChange={onChange}
/>
);
},
};
24 changes: 17 additions & 7 deletions src/components/combo_box/combo_box.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,22 @@ describe('EuiComboBox', () => {

expect(getByRole('combobox')).toBe(inputRefCallback.mock.calls[0][0]);
});

test('onSearchChange', () => {
const onSearchChange = jest.fn();
const { getByTestSubject, queryAllByRole } = render(
<EuiComboBox options={options} onSearchChange={onSearchChange} />
);
const input = getByTestSubject('comboBoxSearchInput');

fireEvent.change(input, { target: { value: 'no results' } });
expect(onSearchChange).toHaveBeenCalledWith('no results', false);
expect(queryAllByRole('option')).toHaveLength(0);

fireEvent.change(input, { target: { value: 'titan' } });
expect(onSearchChange).toHaveBeenCalledWith('titan', true);
expect(queryAllByRole('option')).toHaveLength(2);
});
});

it('does not show multiple checkmarks with duplicate labels', async () => {
Expand Down Expand Up @@ -519,19 +535,14 @@ describe('EuiComboBox', () => {
});

describe('sortMatchesBy', () => {
const onSearchChange = jest.fn();
const sortMatchesByOptions = [
{ label: 'Something is Disabled' },
...options,
];

test('"none"', () => {
const { getByTestSubject, getAllByRole } = render(
<EuiComboBox
options={sortMatchesByOptions}
onSearchChange={onSearchChange}
sortMatchesBy="none"
/>
<EuiComboBox options={sortMatchesByOptions} sortMatchesBy="none" />
);
fireEvent.change(getByTestSubject('comboBoxSearchInput'), {
target: { value: 'di' },
Expand All @@ -547,7 +558,6 @@ describe('EuiComboBox', () => {
const { getByTestSubject, getAllByRole } = render(
<EuiComboBox
options={sortMatchesByOptions}
onSearchChange={onSearchChange}
sortMatchesBy="startsWith"
/>
);
Expand Down
13 changes: 7 additions & 6 deletions src/components/combo_box/combo_box.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -638,13 +638,14 @@ export class EuiComboBox<T> extends Component<
) => {
const { onSearchChange, delimiter } = this.props;

if (onSearchChange) {
const hasMatchingOptions = this.state.matchingOptions.length > 0;
onSearchChange(searchValue, hasMatchingOptions);
}

this.setState({ searchValue }, () => {
if (searchValue && this.state.isListOpen === false) this.openList();
if (searchValue && this.state.isListOpen === false) {
this.openList();
}
if (onSearchChange) {
const hasMatchingOptions = this.state.matchingOptions.length > 0;
onSearchChange(searchValue, hasMatchingOptions);
}
});
if (delimiter && searchValue.endsWith(delimiter)) {
this.setCustomOptions(false);
Expand Down
3 changes: 3 additions & 0 deletions upcoming_changelogs/7334.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
**Bug fixes**

- Fixed `EuiComboBox`'s `onSearchChange` callback to pass the correct `hasMatchingOptions` value
Loading

0 comments on commit 197e104

Please sign in to comment.