From 6aa7a2eda4a0500b5af79013a282322d9ab96578 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Sat, 11 May 2024 21:56:08 +0100 Subject: [PATCH 1/3] Ensure specified trigger id is used for menu aria-labelledby --- .../src/components/Menu/Menu.test.tsx | 19 +++++++++++++++++++ .../src/components/Menu/useMenu.tsx | 3 ++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/react-components/react-menu/src/components/Menu/Menu.test.tsx b/packages/react-components/react-menu/src/components/Menu/Menu.test.tsx index 58d3fe512e16d5..07284b21447433 100644 --- a/packages/react-components/react-menu/src/components/Menu/Menu.test.tsx +++ b/packages/react-components/react-menu/src/components/Menu/Menu.test.tsx @@ -397,4 +397,23 @@ describe('Menu', () => { expect(container.querySelector('#second')?.getAttribute('aria-checked')).toBe('true'); expect(container.querySelector('#third')?.getAttribute('aria-checked')).toBe('false'); }); + + it('should render correct aria-labelledby attribute from corresponding trigger id', () => { + const customId = 'myTrigger'; + const { getByRole } = render( + + + + + + + Item + + + , + ); + + // Assert + expect(getByRole('menu')).toHaveAttribute('aria-labelledby', customId); + }); }); diff --git a/packages/react-components/react-menu/src/components/Menu/useMenu.tsx b/packages/react-components/react-menu/src/components/Menu/useMenu.tsx index 507b8f072ccadd..41a2f853f5ad85 100644 --- a/packages/react-components/react-menu/src/components/Menu/useMenu.tsx +++ b/packages/react-components/react-menu/src/components/Menu/useMenu.tsx @@ -54,7 +54,6 @@ export const useMenu_unstable = (props: MenuProps): MenuState => { defaultCheckedValues, mountNode = null, } = props; - const triggerId = useId('menu'); const [contextTarget, setContextTarget] = usePositioningMouseTarget(); const positioningState = { @@ -88,6 +87,8 @@ export const useMenu_unstable = (props: MenuProps): MenuState => { menuPopover = children[0]; } + const triggerId = menuTrigger?.props?.children?.props?.id ?? useId('menu'); + const { targetRef: triggerRef, containerRef: menuPopoverRef } = usePositioning(positioningState); // TODO Better way to narrow types ? From 1d56d82a8fbe95ad18f9cf7a476a97499374cd18 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Sun, 12 May 2024 07:47:41 +0100 Subject: [PATCH 2/3] Fix rules-of-hooks lint error --- ...ui-react-menu-cd75874c-429a-4082-8ba3-de74b0cd50b8.json | 7 +++++++ .../react-menu/src/components/Menu/useMenu.tsx | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 change/@fluentui-react-menu-cd75874c-429a-4082-8ba3-de74b0cd50b8.json diff --git a/change/@fluentui-react-menu-cd75874c-429a-4082-8ba3-de74b0cd50b8.json b/change/@fluentui-react-menu-cd75874c-429a-4082-8ba3-de74b0cd50b8.json new file mode 100644 index 00000000000000..9caf8fadba2135 --- /dev/null +++ b/change/@fluentui-react-menu-cd75874c-429a-4082-8ba3-de74b0cd50b8.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Bug fix for #18318 which ensures that a specified id attribute placed on the first trigger child is used for the menu aria-labelledby when provided", + "packageName": "@fluentui/react-menu", + "email": "ijackson145@gmail.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-menu/src/components/Menu/useMenu.tsx b/packages/react-components/react-menu/src/components/Menu/useMenu.tsx index 41a2f853f5ad85..75d056d5c1f776 100644 --- a/packages/react-components/react-menu/src/components/Menu/useMenu.tsx +++ b/packages/react-components/react-menu/src/components/Menu/useMenu.tsx @@ -54,6 +54,7 @@ export const useMenu_unstable = (props: MenuProps): MenuState => { defaultCheckedValues, mountNode = null, } = props; + let triggerId = useId('menu'); const [contextTarget, setContextTarget] = usePositioningMouseTarget(); const positioningState = { @@ -86,8 +87,7 @@ export const useMenu_unstable = (props: MenuProps): MenuState => { } else if (children.length === 1) { menuPopover = children[0]; } - - const triggerId = menuTrigger?.props?.children?.props?.id ?? useId('menu'); + if (menuTrigger?.props?.children?.props?.id) triggerId = menuTrigger.props.children.props.id; const { targetRef: triggerRef, containerRef: menuPopoverRef } = usePositioning(positioningState); From 40ce96c336ebf87a1e33780096e7b546ef5dd1ea Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Sun, 12 May 2024 08:14:39 +0100 Subject: [PATCH 3/3] Wrap if statement in curly braces --- .../react-menu/src/components/Menu/useMenu.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-components/react-menu/src/components/Menu/useMenu.tsx b/packages/react-components/react-menu/src/components/Menu/useMenu.tsx index 75d056d5c1f776..7ecc02b78f646b 100644 --- a/packages/react-components/react-menu/src/components/Menu/useMenu.tsx +++ b/packages/react-components/react-menu/src/components/Menu/useMenu.tsx @@ -87,7 +87,9 @@ export const useMenu_unstable = (props: MenuProps): MenuState => { } else if (children.length === 1) { menuPopover = children[0]; } - if (menuTrigger?.props?.children?.props?.id) triggerId = menuTrigger.props.children.props.id; + if (menuTrigger?.props?.children?.props?.id) { + triggerId = menuTrigger.props.children.props.id; + } const { targetRef: triggerRef, containerRef: menuPopoverRef } = usePositioning(positioningState);