diff --git a/change/@fluentui-react-tree-48fe2b57-5a5f-4455-b90c-0810d0a2ba03.json b/change/@fluentui-react-tree-48fe2b57-5a5f-4455-b90c-0810d0a2ba03.json new file mode 100644 index 0000000000000..e67dfaa02ea17 --- /dev/null +++ b/change/@fluentui-react-tree-48fe2b57-5a5f-4455-b90c-0810d0a2ba03.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: add accessible names to Tree selection controls", + "packageName": "@fluentui/react-tree", + "email": "paulmardling@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-tree/library/src/components/Tree/Tree.test.tsx b/packages/react-components/react-tree/library/src/components/Tree/Tree.test.tsx index 4f5bc730ece6c..39c3f68e16128 100644 --- a/packages/react-components/react-tree/library/src/components/Tree/Tree.test.tsx +++ b/packages/react-components/react-tree/library/src/components/Tree/Tree.test.tsx @@ -1,8 +1,10 @@ import * as React from 'react'; -import { render } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import { isConformant } from '../../testing/isConformant'; import { Tree } from './Tree'; import { TreeItem } from '../TreeItem/TreeItem'; +import { TreeItemLayout } from '../TreeItemLayout/TreeItemLayout'; +import { TreeItemPersonaLayout } from '../TreeItemPersonaLayout/TreeItemPersonaLayout'; describe('Tree', () => { isConformant({ @@ -37,4 +39,118 @@ describe('Tree', () => { expect(handleOpenChange).toHaveBeenNthCalledWith(2, expect.any(Object), expect.objectContaining({ open: false })); }); + + describe('selection control accessibility', () => { + it.each([ + { + case: 'multiselect', + selectionMode: 'multiselect', + role: 'checkbox', + layout: Item 1, + accessibleName: 'Item 1', + }, + { + case: 'single', + selectionMode: 'single', + role: 'radio', + layout: Item 1, + accessibleName: 'Item 1', + }, + { + case: 'TreeItemPersonaLayout', + selectionMode: 'multiselect', + role: 'checkbox', + layout: Jane Doe, + accessibleName: 'Jane Doe', + }, + ] as const)('exposes a named $role selector ($case)', ({ selectionMode, role, layout, accessibleName }) => { + render( + + + {layout} + + , + ); + + expect(screen.getByRole(role, { name: accessibleName })).toBeTruthy(); + }); + + it.each([ + { + case: 'TreeItemLayout', + layout: Item 1, + mainId: 'custom-main', + accessibleName: 'Item 1', + }, + { + case: 'TreeItemPersonaLayout', + layout: Jane Doe, + mainId: 'custom-persona-main', + accessibleName: 'Jane Doe', + }, + ] as const)('preserves a consumer-provided main ID ($case)', ({ layout, mainId, accessibleName }) => { + render( + + + {layout} + + , + ); + + expect(screen.getByRole('checkbox', { name: accessibleName }).getAttribute('aria-labelledby')).toBe(mainId); + expect(document.getElementById(mainId)?.textContent).toBe(accessibleName); + }); + + it('preserves a consumer-provided layout ID', () => { + render( + + + + Item 1 + + + , + ); + + const selector = screen.getByRole('checkbox', { name: 'Item 1' }); + const mainId = selector.getAttribute('aria-labelledby'); + const layout = document.getElementById('custom-layout'); + const main = document.getElementById('lower-priority-main-id'); + + expect(layout?.classList.contains('fui-TreeItemLayout')).toBe(true); + expect(mainId).toBe('lower-priority-main-id'); + expect(main?.classList.contains('fui-TreeItemLayout__main')).toBe(true); + }); + + it('preserves a consumer-provided selector aria-label', () => { + render( + + + Item 1 + + , + ); + + const selector = screen.getByRole('checkbox', { name: 'Custom selector label' }); + expect(selector.getAttribute('aria-label')).toBe('Custom selector label'); + expect(selector.hasAttribute('aria-labelledby')).toBe(false); + }); + + it('preserves a consumer-provided selector aria-labelledby', () => { + render( + <> + External selector label + + + Item 1 + + + , + ); + + expect(screen.getByRole('checkbox', { name: 'External selector label' }).getAttribute('aria-labelledby')).toBe( + 'external-selector-label', + ); + }); + }); }); diff --git a/packages/react-components/react-tree/library/src/components/TreeItemLayout/useTreeItemLayout.tsx b/packages/react-components/react-tree/library/src/components/TreeItemLayout/useTreeItemLayout.tsx index bccb8bfe85fcf..0804e495e46b0 100644 --- a/packages/react-components/react-tree/library/src/components/TreeItemLayout/useTreeItemLayout.tsx +++ b/packages/react-components/react-tree/library/src/components/TreeItemLayout/useTreeItemLayout.tsx @@ -9,6 +9,7 @@ import { useEventCallback, elementContains, useControllableState, + useId, } from '@fluentui/react-utilities'; import { useTreeItemContext_unstable, useTreeContext_unstable } from '../../contexts'; import type { @@ -66,6 +67,32 @@ export const useTreeItemLayout_unstable = ( const checked = useTreeItemContext_unstable(ctx => ctx.checked); const isBranch = useTreeItemContext_unstable(ctx => ctx.itemType === 'branch'); + const mainShorthand = isResolvedShorthand(main) ? main : undefined; + const mainContentId = useId('fui-TreeItemLayout__main-', mainShorthand?.id); + + // Link the selector to the main content unless the consumer provided an accessible name + const selectorShorthand = isResolvedShorthand(props.selector) ? props.selector : undefined; + const selectorAriaLabelledBy = + selectorShorthand?.['aria-label'] || selectorShorthand?.['aria-labelledby'] ? undefined : mainContentId; + + const selector = slot.optional(props.selector, { + renderByDefault: selectionMode !== 'none', + defaultProps: { + checked, + tabIndex: -1, + ref: selectionRef, + 'aria-labelledby': selectorAriaLabelledBy, + // casting here to a union between checkbox and radio + // since ref is not present on the selector signature + // FIXME: look into Slot type to see if we can make this work + } as CheckboxProps | RadioProps, + elementType: (selectionMode === 'multiselect' ? Checkbox : Radio) as React.ElementType, + }); + const mainSlot = slot.always(main, { + defaultProps: { id: selector ? mainContentId : undefined }, + elementType: 'div', + }); + // FIXME: Asserting is required here, as converting this to RefObject on context type would be a breaking change // eslint-disable-next-line react-hooks/refs assertIsRefObject(treeItemRef); @@ -142,29 +169,18 @@ export const useTreeItemLayout_unstable = ( [setIsActionsVisible, onActionVisibilityChange, treeItemRef, isNavigatingWithKeyboard, targetDocument], ); + const expandIconRefs = useMergedRefs(expandIconRef); const expandIcon = slot.optional(props.expandIcon, { renderByDefault: isBranch, defaultProps: { children: , 'aria-hidden': true, + ref: expandIconRefs, }, elementType: 'div', }); - const expandIconRefs = useMergedRefs(expandIcon?.ref, expandIconRef); - if (expandIcon) { - expandIcon.ref = expandIconRefs; - } const arrowNavigationProps = useArrowNavigationGroup({ circular: navigationMode === 'tree', axis: 'horizontal' }); - const actions = isActionsVisible - ? slot.optional(props.actions, { - defaultProps: { ...arrowNavigationProps, role: 'toolbar' }, - elementType: 'div', - }) - : undefined; - delete actions?.visible; - delete actions?.onVisibilityChange; - const actionsRefs = useMergedRefs(actions?.ref, actionsRef, actionsRefInternal); const handleActionsBlur = useEventCallback((event: React.FocusEvent) => { if (isResolvedShorthand(props.actions)) { props.actions.onBlur?.(event); @@ -177,10 +193,19 @@ export const useTreeItemLayout_unstable = ( } as Extract); setIsActionsVisible(isRelatedTargetFromActions); }); - if (actions) { - actions.ref = actionsRefs; - actions.onBlur = handleActionsBlur; - } + + const actionsRefs = useMergedRefs(actionsRef, actionsRefInternal); + const actions = isActionsVisible + ? slot.optional(props.actions, { + defaultProps: { + ...arrowNavigationProps, + role: 'toolbar', + ref: actionsRefs, + onBlur: handleActionsBlur, + }, + elementType: 'div', + }) + : undefined; const hasActions = Boolean(props.actions); @@ -233,26 +258,12 @@ export const useTreeItemLayout_unstable = ( }, ), iconBefore: slot.optional(iconBefore, { elementType: 'div' }), - main: slot.always(main, { elementType: 'div' }), + main: mainSlot, iconAfter: slot.optional(iconAfter, { elementType: 'div' }), aside: !isActionsVisible ? slot.optional(props.aside, { elementType: 'div' }) : undefined, actions, expandIcon, - selector: slot.optional(props.selector, { - renderByDefault: selectionMode !== 'none', - defaultProps: { - checked, - tabIndex: -1, - 'aria-hidden': true, - ref: selectionRef, - // casting here to a union between checkbox and radio - // since ref is not present on the selector signature - // FIXME: look into Slot type to see if we can make this work - } as CheckboxProps | RadioProps, - elementType: (selectionMode === 'multiselect' ? Checkbox : Radio) as React.ElementType< - CheckboxProps | RadioProps - >, - }), + selector, }; }; diff --git a/packages/react-components/react-tree/library/src/components/TreeItemPersonaLayout/useTreeItemPersonaLayout.ts b/packages/react-components/react-tree/library/src/components/TreeItemPersonaLayout/useTreeItemPersonaLayout.ts index 9a338ee629b3c..73e0e0ee41392 100644 --- a/packages/react-components/react-tree/library/src/components/TreeItemPersonaLayout/useTreeItemPersonaLayout.ts +++ b/packages/react-components/react-tree/library/src/components/TreeItemPersonaLayout/useTreeItemPersonaLayout.ts @@ -52,7 +52,10 @@ export const useTreeItemPersonaLayout_unstable = ( selector: (selectionMode === 'multiselect' ? Checkbox : Radio) as React.ElementType, }, avatarSize: treeAvatarSize[size], - main: slot.always(main, { defaultProps: { children }, elementType: 'div' }), + main: slot.always(main, { + defaultProps: { children, id: treeItemLayoutState.main.id }, + elementType: 'div', + }), media: slot.always(media, { elementType: 'div' }), description: slot.optional(description, { elementType: 'div' }), };