From 595204312c5407e46c54b063bc73bdfca614ebde Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 24 Sep 2025 14:09:44 -0500 Subject: [PATCH 1/2] fix selection state in Menu items with ContextualHelpTrigger and isUnavailable={false} --- .../@react-spectrum/menu/src/MenuItem.tsx | 6 ++- .../menu/stories/MenuTrigger.stories.tsx | 35 ++++++++++++++++ .../menu/test/MenuTrigger.test.js | 40 +++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/menu/src/MenuItem.tsx b/packages/@react-spectrum/menu/src/MenuItem.tsx index 7277dc253cc..519e514787d 100644 --- a/packages/@react-spectrum/menu/src/MenuItem.tsx +++ b/packages/@react-spectrum/menu/src/MenuItem.tsx @@ -64,7 +64,11 @@ export function MenuItem(props: MenuItemProps): JSX.Element { } let isDisabled = state.disabledKeys.has(key); - let isSelectable = !isSubmenuTrigger && state.selectionManager.selectionMode !== 'none'; + let isContextualHelpTrigger = isSubmenuTrigger && isUnavailable !== undefined; + let isSelectable = ( + (isContextualHelpTrigger ? !isUnavailable : !isSubmenuTrigger) && + state.selectionManager.selectionMode !== 'none' + ); let isSelected = isSelectable && state.selectionManager.isSelected(key); let itemref = useRef(null); let ref = useObjectRef(useMemo(() => mergeRefs(itemref, triggerRef), [itemref, triggerRef])); diff --git a/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx b/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx index 29f611fcc7a..7233a2f7a29 100644 --- a/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx +++ b/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx @@ -786,6 +786,41 @@ export let MenuItemUnavailable: StoryObj = { ) }; +export let MenuItemUnavailableWithSelection: StoryObj = { + render: () => render( + + One + + Two + + hello + Is it me you're looking for? + + + + Two point five + + hello + Is it me you're looking for? + + + Three + + + Four + Shut the door + + + hello + Is it me you're looking for? +
Learn more
+
+
+ Five +
+ ) +}; + export let MenuItemUnavailableDynamic: StoryObj = { render: () => render( diff --git a/packages/@react-spectrum/menu/test/MenuTrigger.test.js b/packages/@react-spectrum/menu/test/MenuTrigger.test.js index 567ff686c51..7e3bc0bb323 100644 --- a/packages/@react-spectrum/menu/test/MenuTrigger.test.js +++ b/packages/@react-spectrum/menu/test/MenuTrigger.test.js @@ -617,6 +617,46 @@ describe('MenuTrigger', function () { expect(tree.queryByRole('dialog')).toBeNull(); }); + it('shows selection state for available ContextualHelpTrigger items', async function () { + render( + + + Menu + + One + + + Hello + Is it me you're looking for? + + + Lionel Richie says: + I can see it in your eyes + + + + + + ); + + act(() => {jest.runAllTimers();}); + + let menu = screen.getByRole('menu'); + let availableItem = within(menu).getByRole('menuitemradio', {name: 'Hello'}); + + expect(availableItem).toBeVisible(); + expect(availableItem).toHaveAttribute('aria-checked', 'false'); + expect(availableItem).not.toHaveClass('is-selected'); + fireEvent.click(availableItem); + act(() => {jest.runAllTimers();}); + + expect(availableItem).toHaveAttribute('aria-checked', 'true'); + expect(availableItem).toHaveClass('is-selected'); + expect(within(availableItem).getByRole('img', {hidden: true})).toHaveClass('spectrum-Menu-checkmark'); + expect(within(availableItem).queryByLabelText('Unavailable')).toBeNull(); + expect(availableItem).not.toHaveAttribute('aria-haspopup', 'dialog'); + }); + it('can open a sub dialog with keyboard', async function () { renderTree(); let menu = await openMenu(); From a205e07eb8f1f9b0b1d77d98fe70c2da574b41d3 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Wed, 24 Sep 2025 14:17:09 -0500 Subject: [PATCH 2/2] cleanup test --- packages/@react-spectrum/menu/test/MenuTrigger.test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/@react-spectrum/menu/test/MenuTrigger.test.js b/packages/@react-spectrum/menu/test/MenuTrigger.test.js index 7e3bc0bb323..8714c0765bf 100644 --- a/packages/@react-spectrum/menu/test/MenuTrigger.test.js +++ b/packages/@react-spectrum/menu/test/MenuTrigger.test.js @@ -646,14 +646,10 @@ describe('MenuTrigger', function () { expect(availableItem).toBeVisible(); expect(availableItem).toHaveAttribute('aria-checked', 'false'); - expect(availableItem).not.toHaveClass('is-selected'); fireEvent.click(availableItem); act(() => {jest.runAllTimers();}); expect(availableItem).toHaveAttribute('aria-checked', 'true'); - expect(availableItem).toHaveClass('is-selected'); - expect(within(availableItem).getByRole('img', {hidden: true})).toHaveClass('spectrum-Menu-checkmark'); - expect(within(availableItem).queryByLabelText('Unavailable')).toBeNull(); expect(availableItem).not.toHaveAttribute('aria-haspopup', 'dialog'); });