Skip to content

Commit c2cc7de

Browse files
authored
fix: update FilterableMultiSelect onMenuChange behavior (#18606)
* fix: update FilterableMultiSelect onMenuChange behavior * fix: address accessibility todo
1 parent 77f9c6c commit c2cc7de

File tree

2 files changed

+63
-5
lines changed

2 files changed

+63
-5
lines changed

packages/react/src/components/MultiSelect/FilterableMultiSelect.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright IBM Corp. 2016, 2023
2+
* Copyright IBM Corp. 2016, 2025
33
*
44
* This source code is licensed under the Apache-2.0 license found in the
55
* LICENSE file in the root directory of this source tree.
@@ -358,6 +358,7 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
358358
ref: ForwardedRef<HTMLDivElement>
359359
) {
360360
const { isFluid } = useContext(FormContext);
361+
const isFirstRender = useRef(true);
361362
const [isFocused, setIsFocused] = useState<boolean>(false);
362363
const [isOpen, setIsOpen] = useState<boolean>(!!open);
363364
const [prevOpen, setPrevOpen] = useState<boolean>(!!open);
@@ -521,8 +522,16 @@ const FilterableMultiSelect = React.forwardRef(function FilterableMultiSelect<
521522
}
522523

523524
useEffect(() => {
524-
onMenuChange?.(isOpen);
525-
}, [isOpen, onMenuChange]);
525+
if (isFirstRender.current) {
526+
isFirstRender.current = false;
527+
528+
if (open) {
529+
onMenuChange?.(isOpen);
530+
}
531+
} else {
532+
onMenuChange?.(isOpen);
533+
}
534+
}, [isOpen, onMenuChange, open]);
526535

527536
const {
528537
getToggleButtonProps,

packages/react/src/components/MultiSelect/__tests__/FilterableMultiSelect-test.js

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright IBM Corp. 2016, 2023
2+
* Copyright IBM Corp. 2016, 2025
33
*
44
* This source code is licensed under the Apache-2.0 license found in the
55
* LICENSE file in the root directory of this source tree.
@@ -536,7 +536,7 @@ describe('FilterableMultiSelect', () => {
536536
item ? (
537537
<span className="test-element" data-testid="custom-id-item">
538538
{item.text}{' '}
539-
<span role="img" alt="fire">
539+
<span role="img" aria-label="fire">
540540
{' '}
541541
🔥
542542
</span>
@@ -922,4 +922,53 @@ describe('FilterableMultiSelect', () => {
922922
`${prefix}--multi-select--filterable--input-focused`
923923
);
924924
});
925+
926+
it('should call `onMenuChange` exactly once on mount when `open` prop is provided', async () => {
927+
render(<FilterableMultiSelect {...mockProps} open />);
928+
await waitForPosition();
929+
930+
expect(mockProps.onMenuChange).toHaveBeenCalledTimes(1);
931+
expect(mockProps.onMenuChange).toHaveBeenCalledWith(true);
932+
});
933+
934+
it('should not re-trigger `onMenuChange` on re-render if `open` prop remains unchanged', async () => {
935+
const { rerender } = render(<FilterableMultiSelect {...mockProps} open />);
936+
await waitForPosition();
937+
938+
// Initially called once on mount.
939+
expect(mockProps.onMenuChange).toHaveBeenCalledTimes(1);
940+
expect(mockProps.onMenuChange).toHaveBeenCalledWith(true);
941+
942+
// Rerender with the same open prop value.
943+
rerender(<FilterableMultiSelect {...mockProps} open />);
944+
await waitForPosition();
945+
946+
// The callback should not be called again.
947+
expect(mockProps.onMenuChange).toHaveBeenCalledTimes(1);
948+
});
949+
950+
it('should not call `onMenuChange` on mount when uncontrolled', async () => {
951+
render(<FilterableMultiSelect {...mockProps} />);
952+
await waitForPosition();
953+
954+
// Since uncontrolled mode only fires on interactions, expect no call on
955+
// mount.
956+
expect(mockProps.onMenuChange).not.toHaveBeenCalled();
957+
});
958+
959+
it('should call `onMenuChange` when user interactions trigger state changes', async () => {
960+
render(<FilterableMultiSelect {...mockProps} />);
961+
await waitForPosition();
962+
963+
// Open the menu by clicking the combobox.
964+
await userEvent.click(screen.getByRole('combobox'));
965+
expect(mockProps.onMenuChange).toHaveBeenCalledTimes(1);
966+
expect(mockProps.onMenuChange).toHaveBeenCalledWith(true);
967+
mockProps.onMenuChange.mockClear();
968+
969+
// Close the menu by clicking outside of it.
970+
await userEvent.click(document.body);
971+
expect(mockProps.onMenuChange).toHaveBeenCalledTimes(1);
972+
expect(mockProps.onMenuChange).toHaveBeenCalledWith(false);
973+
});
925974
});

0 commit comments

Comments
 (0)