Skip to content

Commit

Permalink
fix(multi-select): resolve issue with multiple execution of onFilterC…
Browse files Browse the repository at this point in the history
…hange callback
  • Loading branch information
mkrds committed Feb 16, 2023
1 parent 65a9cef commit f0112a7
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 8 deletions.
13 changes: 10 additions & 3 deletions src/components/select/multi-select/multi-select.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import isExpectedOption from "../utils/is-expected-option";
import isExpectedValue from "../utils/is-expected-value";
import isNavigationKey from "../utils/is-navigation-key";
import Logger from "../../../__internal__/utils/logger";
import useStableCallback from "../../../hooks/__internal__/useStableCallback";

let deprecateInputRefWarnTriggered = false;

Expand All @@ -44,7 +45,7 @@ const MultiSelect = React.forwardRef(
readOnly,
children,
onOpen,
onFilterChange,
onFilterChange: onFilterChangeProp,
onChange,
onClick,
onFocus,
Expand Down Expand Up @@ -326,11 +327,17 @@ const MultiSelect = React.forwardRef(
};
}, [handleGlobalClick]);

const onFilterChange = useStableCallback(onFilterChangeProp);
const isFirstRender = useRef(true);
useEffect(() => {
if (onFilterChange) {
if (onFilterChange && !isFirstRender.current) {
onFilterChange(filterText);
}
}, [filterText, onFilterChange]);
}, [onFilterChange, filterText]);

useEffect(() => {
isFirstRender.current = false;
}, []);

function handleTextboxClick(event) {
isMouseDownReported.current = false;
Expand Down
35 changes: 32 additions & 3 deletions src/components/select/multi-select/multi-select.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useRef } from "react";
import React, { useRef, useState } from "react";
import { act } from "react-dom/test-utils";
import { mount } from "enzyme";

Expand Down Expand Up @@ -805,8 +805,8 @@ describe("MultiSelect", () => {
});
});

describe('when the "onFilter" prop has been passed and the input text changed', () => {
it("then that prop should be called", () => {
describe('when the "onFilter" prop has been passed', () => {
it("then that prop should be invoked when the input text has changed", () => {
const filterText = "foo";
const onFilterChangeFn = jest.fn();
const wrapper = renderSelect({ onFilterChange: onFilterChangeFn });
Expand All @@ -816,6 +816,35 @@ describe("MultiSelect", () => {
.simulate("change", { target: { value: filterText } });
expect(onFilterChangeFn).toHaveBeenCalledWith(filterText);
});

it("then it should not be invoked when the component rerenders", () => {
const Component = () => {
const [onFilterChangeCalled, setOnFilterChangeCalled] = useState(false);

return (
<>
<div id="on-filter-called">
{onFilterChangeCalled ? "true" : "false"}
</div>
<MultiSelect
name="testSelect"
id="testSelect"
onFilterChange={() => setOnFilterChangeCalled(true)}
>
<Option value="opt1" text="red" />
<Option value="opt2" text="green" />
<Option value="opt3" text="blue" />
<Option value="opt4" text="black" />
</MultiSelect>
</>
);
};

const wrapper = mount(<Component />);
expect(wrapper.find("#on-filter-called").text()).toBe("false");
wrapper.setProps({});
expect(wrapper.find("#on-filter-called").text()).toBe("false");
});
});

describe('when the "onSelectListClose" is called in the SelectList', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/components/select/multi-select/multi-select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1310,8 +1310,8 @@ context("Tests for Multi Select component", () => {
.type(text)
.then(() => {
// eslint-disable-next-line no-unused-expressions
expect(callback).to.have.been.calledTwice;
expect(callback.getCalls()[1].args[0]).to.equals(text);
expect(callback).to.have.been.calledOnce;
expect(callback).to.have.been.calledWith(text);
});
});
});
Expand Down

0 comments on commit f0112a7

Please sign in to comment.