Skip to content

Commit 3f57751

Browse files
authored
fix(react-combobox): use role attribute instead of classname for active descendant (microsoft#36109)
1 parent b9cadd1 commit 3f57751

8 files changed

Lines changed: 61 additions & 6 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "fix: use role attribute instead of classname for active descendant",
4+
"packageName": "@fluentui/react-combobox",
5+
"email": "dmytrokirpa@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

packages/react-components/react-combobox/library/etc/react-combobox.api.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@ export type DropdownState = ComponentState<DropdownSlots> & Omit<ComboboxBaseSta
150150
activeDescendantController: ActiveDescendantImperativeRef;
151151
};
152152

153+
// @public
154+
export function isComboboxOptionElement(element: HTMLElement): boolean;
155+
153156
// @public
154157
export const Listbox: ForwardRefComponent<ListboxProps>;
155158

packages/react-components/react-combobox/library/src/components/Combobox/useCombobox.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import type {
2626
} from './Combobox.types';
2727
import { useListboxSlot } from '../../utils/useListboxSlot';
2828
import { useInputTriggerSlot } from './useInputTriggerSlot';
29-
import { optionClassNames } from '../Option/useOptionStyles.styles';
29+
import { isComboboxOptionElement } from '../../utils/isComboboxOptionElement';
3030

3131
/**
3232
* Create the base state required to render Combobox, without design-only props.
@@ -47,7 +47,7 @@ export const useComboboxBase_unstable = (
4747
activeParentRef,
4848
controller: activeDescendantController,
4949
} = useActiveDescendant<HTMLInputElement, HTMLDivElement>({
50-
matchOption: el => el.classList.contains(optionClassNames.root),
50+
matchOption: isComboboxOptionElement,
5151
});
5252
const comboboxInternalState = useComboboxBaseState({ ...props, editable: true, activeDescendantController });
5353
const { appearance: _appearance, size: _size, ...baseState } = comboboxInternalState;

packages/react-components/react-combobox/library/src/components/Dropdown/useDropdown.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ import { Listbox } from '../Listbox/Listbox';
1919
import type { DropdownBaseProps, DropdownBaseState, DropdownProps, DropdownState } from './Dropdown.types';
2020
import { useListboxSlot } from '../../utils/useListboxSlot';
2121
import { useButtonTriggerSlot } from './useButtonTriggerSlot';
22-
import { optionClassNames } from '../Option/useOptionStyles.styles';
2322
import type { ComboboxOpenEvents } from '../Combobox/Combobox.types';
23+
import { isComboboxOptionElement } from '../../utils/isComboboxOptionElement';
2424

2525
/**
2626
* Create the base state required to render Dropdown, without design-only props.
@@ -41,7 +41,7 @@ export const useDropdownBase_unstable = (
4141
activeParentRef,
4242
controller: activeDescendantController,
4343
} = useActiveDescendant<HTMLButtonElement, HTMLDivElement>({
44-
matchOption: el => el.classList.contains(optionClassNames.root),
44+
matchOption: isComboboxOptionElement,
4545
});
4646

4747
const dropdownInternalState = useComboboxBaseState({ ...props, activeDescendantController, freeform: false });

packages/react-components/react-combobox/library/src/components/Listbox/useListbox.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ import type { ListboxProps, ListboxState } from './Listbox.types';
1919
import { getDropdownActionFromKey } from '../../utils/dropdownKeyActions';
2020
import { useOptionCollection } from '../../utils/useOptionCollection';
2121
import { useSelection } from '../../utils/useSelection';
22-
import { optionClassNames } from '../Option/useOptionStyles.styles';
2322
import { ListboxContext, useListboxContext_unstable } from '../../contexts/ListboxContext';
2423
import { useOnKeyboardNavigationChange } from '@fluentui/react-tabster';
24+
import { isComboboxOptionElement } from '../../utils/isComboboxOptionElement';
2525

2626
// eslint-disable-next-line @typescript-eslint/naming-convention
2727
const UNSAFE_noLongerUsed = {
@@ -50,7 +50,7 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref<HTMLElem
5050
activeParentRef,
5151
controller,
5252
} = useActiveDescendant<HTMLInputElement, HTMLDivElement>({
53-
matchOption: el => el.classList.contains(optionClassNames.root),
53+
matchOption: isComboboxOptionElement,
5454
});
5555

5656
const hasListboxContext = useHasParentContext(ListboxContext);

packages/react-components/react-combobox/library/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,4 @@ export { useButtonTriggerSlot } from './components/Dropdown/useButtonTriggerSlot
7676
export { useInputTriggerSlot } from './components/Combobox/useInputTriggerSlot';
7777
export { useListboxSlot } from './utils/useListboxSlot';
7878
export type { ComboboxBaseState, ComboboxBaseProps } from './utils/ComboboxBase.types';
79+
export { isComboboxOptionElement } from './utils/isComboboxOptionElement';
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { isComboboxOptionElement } from './isComboboxOptionElement';
2+
3+
/**
4+
* Helper function to create an element with a specified role
5+
*/
6+
function createElementWithRole(tagName: string, role?: string) {
7+
const element = document.createElement(tagName);
8+
if (role) {
9+
element.setAttribute('role', role);
10+
}
11+
return element;
12+
}
13+
14+
describe('isComboboxOptionElement', () => {
15+
it('returns true for elements with role="option"', () => {
16+
const element = createElementWithRole('div', 'option');
17+
expect(isComboboxOptionElement(element)).toBe(true);
18+
});
19+
20+
it('returns true for elements with role="menuitemcheckbox"', () => {
21+
const element = createElementWithRole('div', 'menuitemcheckbox');
22+
expect(isComboboxOptionElement(element)).toBe(true);
23+
});
24+
25+
it('returns false for elements without role attribute', () => {
26+
const element = createElementWithRole('div');
27+
expect(isComboboxOptionElement(element)).toBe(false);
28+
});
29+
30+
it('returns false for elements with other role values', () => {
31+
const element = createElementWithRole('div', 'listbox');
32+
expect(isComboboxOptionElement(element)).toBe(false);
33+
});
34+
});
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/**
2+
* Checks whether the given element is a combobox option element.
3+
* Supports elements with role="option" or role="menuitemcheckbox".
4+
*
5+
* @param element - the element to check
6+
* @returns true if the element has a valid combobox option role, false otherwise
7+
*/
8+
export function isComboboxOptionElement(element: HTMLElement): boolean {
9+
return element.role === 'option' || element.role === 'menuitemcheckbox';
10+
}

0 commit comments

Comments
 (0)