Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "fix: button semantics for Combobox chevron, and pointer styles",
"packageName": "@fluentui/react-combobox",
"email": "sarah.higley@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,74 @@ describe('Combobox', () => {
expect(container.querySelector('[role=listbox]')).not.toBeNull();
});

/* Expand icon name */
it('Sets the chevron button name off the aria-label prop', () => {
const { container } = render(
<Combobox aria-label="test label">
<Option>Red</Option>
<Option>Green</Option>
<Option>Blue</Option>
</Combobox>,
);

const chevronButton = container.querySelector('[role=button]');
expect(chevronButton?.getAttribute('aria-label')).toEqual('Open test label');
});

it('Sets the chevron button name off the aria-labelledby prop', () => {
const { container } = render(
<Combobox aria-labelledby="testId">
<Option>Red</Option>
<Option>Green</Option>
<Option>Blue</Option>
</Combobox>,
);

const chevronButton = container.querySelector('[role=button]');
const chevronId = chevronButton?.id;
expect(chevronButton?.getAttribute('aria-label')).toEqual('Open');
expect(chevronButton?.getAttribute('aria-labelledby')).toEqual(chevronId + ' testId');
});

it('Defaults to "Open" as the chevron label', () => {
const { container } = render(
<Combobox>
<Option>Red</Option>
<Option>Green</Option>
<Option>Blue</Option>
</Combobox>,
);

const chevronButton = container.querySelector('[role=button]');
expect(chevronButton?.getAttribute('aria-label')).toEqual('Open');
expect(chevronButton?.getAttribute('aria-labelledby')).toBeFalsy();
});

it('Respects author-provided labels for the chevron button', () => {
const renderedCombobox = render(
<Combobox aria-label="not used" aria-labelledby="not-used" expandIcon={{ 'aria-label': 'test label' }}>
<Option>Red</Option>
<Option>Green</Option>
<Option>Blue</Option>
</Combobox>,
);

const chevronButton = renderedCombobox.container.querySelector('[role=button]');
expect(chevronButton?.getAttribute('aria-label')).toEqual('test label');
expect(chevronButton?.getAttribute('aria-labelledby')).toBeFalsy();

renderedCombobox.rerender(
<Combobox aria-label="not used" aria-labelledby="not-used" expandIcon={{ 'aria-labelledby': 'testId' }}>
<Option>Red</Option>
<Option>Green</Option>
<Option>Blue</Option>
</Combobox>,
);

expect(chevronButton?.getAttribute('aria-label')).toBeFalsy();
expect(chevronButton?.getAttribute('aria-labelledby')).toEqual('testId');
});

Comment thread
sopranopillow marked this conversation as resolved.
/* open/close tests */
it('opens the popup on click', () => {
const { getByRole } = render(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ exports[`Combobox renders a default state 1`] = `
value=""
/>
<span
aria-expanded="false"
aria-label="Open"
class="fui-Combobox__expandIcon"
role="button"
>
<svg
aria-hidden="true"
Expand All @@ -40,15 +43,18 @@ exports[`Combobox renders an open listbox 1`] = `
class="fui-Combobox"
>
<input
aria-activedescendant="fluent-option1"
aria-activedescendant="fluent-option11"
aria-expanded="true"
class="fui-Combobox__input"
role="combobox"
type="text"
value=""
/>
<span
aria-expanded="true"
aria-label="Open"
class="fui-Combobox__expandIcon"
role="button"
>
<svg
aria-hidden="true"
Expand All @@ -73,7 +79,7 @@ exports[`Combobox renders an open listbox 1`] = `
<div
aria-selected="false"
class="fui-Option"
id="fluent-option1"
id="fluent-option11"
role="option"
>
<span
Expand All @@ -100,7 +106,7 @@ exports[`Combobox renders an open listbox 1`] = `
<div
aria-selected="false"
class="fui-Option"
id="fluent-option2"
id="fluent-option12"
role="option"
>
<span
Expand All @@ -127,7 +133,7 @@ exports[`Combobox renders an open listbox 1`] = `
<div
aria-selected="false"
class="fui-Option"
id="fluent-option3"
id="fluent-option13"
role="option"
>
<span
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
resolveShorthand,
mergeCallbacks,
useEventCallback,
useId,
useMergedRefs,
} from '@fluentui/react-utilities';
import { getDropdownActionFromKey } from '../../utils/dropdownKeyActions';
Expand Down Expand Up @@ -44,6 +45,7 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
value,
} = baseState;
const { disabled, freeform, multiselect } = props;
const comboId = useId('combobox-');

const { primary: triggerNativeProps, root: rootNativeProps } = getPartitionedNativeProps({
props,
Expand Down Expand Up @@ -200,7 +202,9 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
expandIcon: resolveShorthand(props.expandIcon, {
required: true,
defaultProps: {
'aria-expanded': open,
children: <ChevronDownIcon />,
role: 'button',
},
}),
...baseState,
Expand Down Expand Up @@ -233,6 +237,29 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLIn
if (state.expandIcon) {
state.expandIcon.onMouseDown = onExpandIconMouseDown;
state.expandIcon.onClick = onExpandIconClick;

// If there is no explicit aria-label, calculate default accName attribute for expandIcon button,
// using the following steps:
// 1. If there is an aria-label, it is "Open [aria-label]"
// 2. If there is an aria-labelledby, it is "Open [aria-labelledby target]" (using aria-labelledby + ids)
// 3. If there is no aria-label/ledby attr, it falls back to "Open"
// We can't fall back to a label/htmlFor name because of https://github.com/w3c/accname/issues/179
const hasExpandLabel = state.expandIcon['aria-label'] || state.expandIcon['aria-labelledby'];
const defaultOpenString = 'Open'; // this is english-only since it is the fallback
Comment thread
spmonahan marked this conversation as resolved.
if (!hasExpandLabel) {
if (props['aria-labelledby']) {
const chevronId = state.expandIcon.id ?? `${comboId}-chevron`;
const chevronLabelledBy = `${chevronId} ${state.input['aria-labelledby']}`;

state.expandIcon['aria-label'] = defaultOpenString;
state.expandIcon.id = chevronId;
state.expandIcon['aria-labelledby'] = chevronLabelledBy;
} else if (props['aria-label']) {
state.expandIcon['aria-label'] = `${defaultOpenString} ${props['aria-label']}`;
} else {
state.expandIcon['aria-label'] = defaultOpenString;
}
}
}

return state;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ const useIconStyles = makeStyles({
icon: {
boxSizing: 'border-box',
color: tokens.colorNeutralStrokeAccessible,
cursor: 'pointer',
display: 'block',
fontSize: tokens.fontSizeBase500,

Expand All @@ -222,6 +223,7 @@ const useIconStyles = makeStyles({
},
disabled: {
color: tokens.colorNeutralForegroundDisabled,
cursor: 'not-allowed',
},
});

Expand Down