From 596672818f7bc95e0b1be128f7ade25f4840ff8a Mon Sep 17 00:00:00 2001 From: Josh Farrant Date: Mon, 3 Feb 2025 13:18:50 +0000 Subject: [PATCH] ActionMenu falsy value bugfix (#912) * allow falsey values to trigger item selection callback * add a test to assert that falsey items can be selected * add changeset --- .changeset/kind-years-repair.md | 5 +++++ .../react/src/ActionMenu/ActionMenu.test.tsx | 19 +++++++++++++++++++ packages/react/src/ActionMenu/ActionMenu.tsx | 8 +++----- 3 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 .changeset/kind-years-repair.md diff --git a/.changeset/kind-years-repair.md b/.changeset/kind-years-repair.md new file mode 100644 index 000000000..9a0628953 --- /dev/null +++ b/.changeset/kind-years-repair.md @@ -0,0 +1,5 @@ +--- +'@primer/react-brand': patch +--- + +Fixed a bug in the `ActionMenu` component where items with falsy values (eg `""`) would not trigger the `onSelect` callback when selected. diff --git a/packages/react/src/ActionMenu/ActionMenu.test.tsx b/packages/react/src/ActionMenu/ActionMenu.test.tsx index c397e4878..a4ee55ff1 100644 --- a/packages/react/src/ActionMenu/ActionMenu.test.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.test.tsx @@ -349,4 +349,23 @@ describe('ActionMenu', () => { {timeout: 100}, ) }) + + it('should allow falsey items to be selected', () => { + const mockOnSelect = jest.fn() + + const {getByRole} = render( + + Open menu + + Test string + Empty string + + , + ) + + fireEvent.click(getByRole('button', {name: 'Open menu'})) + fireEvent.click(getByRole('menuitemradio', {name: 'Empty string'})) + + expect(mockOnSelect).toHaveBeenCalledWith('') + }) }) diff --git a/packages/react/src/ActionMenu/ActionMenu.tsx b/packages/react/src/ActionMenu/ActionMenu.tsx index f75eea889..96b2e3632 100644 --- a/packages/react/src/ActionMenu/ActionMenu.tsx +++ b/packages/react/src/ActionMenu/ActionMenu.tsx @@ -166,11 +166,9 @@ const _ActionMenuRoot = memo( const handleItemSelection = useCallback( (newValue: string) => { - if (newValue) { - handleOnSelect(newValue) - toggleMenu() - anchorElementRef.current?.focus() - } + handleOnSelect(newValue) + toggleMenu() + anchorElementRef.current?.focus() }, [handleOnSelect, toggleMenu, anchorElementRef], )