Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accessibility - screenreader announces "blank" while reading the options #5758

Merged
merged 17 commits into from
Nov 6, 2023
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
14 changes: 14 additions & 0 deletions .changeset/blue-kings-serve.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
'react-select': minor
---

1. Added 'aria-activedescendant' for input and functionality to calculate it;
2. Added role 'option' and 'aria-selected' for option;
3. Added role 'listbox' for menu;
4. Added tests for 'aria-activedescendant';
5. Changes in aria-live region:

- the instructions how to use select will be announced only one time when user focuses the input for the first time.
- instructions for menu or selected value will be announced only once after focusing them.
- removed aria-live for focused option because currently with correct aria-attributes it will be announced by screenreader natively as well as the status of this option (active or disabled).
- separated ariaContext into ariaFocused, ariaResults, ariaGuidance to avoid announcing redundant information and higlight only current change.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@
"jest-in-case": "^1.0.2",
"prettier": "^2.2.1",
"style-loader": "^0.23.1",
"typescript": "^4.1.3"
"typescript": "^4.1.3",
"user-agent-data-types": "^0.4.2"
},
"scripts": {
"build": "preconstruct build",
Expand Down
125 changes: 116 additions & 9 deletions packages/react-select/src/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import LiveRegion from './components/LiveRegion';
import { createFilter, FilterOptionOption } from './filters';
import { DummyInput, ScrollManager, RequiredInput } from './internal/index';
import { AriaLiveMessages, AriaSelection } from './accessibility/index';
import { isAppleDevice } from './accessibility/helpers';

import {
classNames,
Expand Down Expand Up @@ -329,12 +330,15 @@ interface State<
inputIsHidden: boolean;
isFocused: boolean;
focusedOption: Option | null;
focusedOptionId: string | null;
focusableOptionsWithIds: FocusableOptionWithId<Option>[];
focusedValue: Option | null;
selectValue: Options<Option>;
clearFocusValueOnUpdate: boolean;
prevWasFocused: boolean;
inputIsHiddenAfterUpdate: boolean | null | undefined;
prevProps: Props<Option, IsMulti, Group> | void;
instancePrefix: string;
}

interface CategorizedOption<Option> {
Expand All @@ -347,6 +351,11 @@ interface CategorizedOption<Option> {
index: number;
}

interface FocusableOptionWithId<Option> {
data: Option;
id: string;
}

interface CategorizedGroup<Option, Group extends GroupBase<Option>> {
type: 'group';
data: Group;
Expand Down Expand Up @@ -441,6 +450,31 @@ function buildFocusableOptionsFromCategorizedOptions<
);
}

function buildFocusableOptionsWithIds<Option, Group extends GroupBase<Option>>(
Copy link
Contributor Author

@Ke1sy Ke1sy Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function calculates focusable options with id as id is needed in 'aria-activedescendant'.

categorizedOptions: readonly CategorizedGroupOrOption<Option, Group>[],
optionId: string
) {
return categorizedOptions.reduce<FocusableOptionWithId<Option>[]>(
(optionsAccumulator, categorizedOption) => {
if (categorizedOption.type === 'group') {
optionsAccumulator.push(
...categorizedOption.options.map((option) => ({
data: option.data,
id: `${optionId}-${categorizedOption.index}-${option.index}`,
}))
);
} else {
optionsAccumulator.push({
data: categorizedOption.data,
id: `${optionId}-${categorizedOption.index}`,
});
}
return optionsAccumulator;
},
[]
);
}

function buildFocusableOptions<
Option,
IsMulti extends boolean,
Expand Down Expand Up @@ -499,6 +533,17 @@ function getNextFocusedOption<
? lastFocusedOption
: options[0];
}

const getFocusedOptionId = <Option,>(
focusableOptionsWithIds: FocusableOptionWithId<Option>[],
focusedOption: Option
) => {
const focusedOptionId = focusableOptionsWithIds.find(
(option) => option.data === focusedOption
)?.id;
return focusedOptionId || null;
};

const getOptionLabel = <
Option,
IsMulti extends boolean,
Expand Down Expand Up @@ -587,6 +632,8 @@ export default class Select<
state: State<Option, IsMulti, Group> = {
ariaSelection: null,
focusedOption: null,
focusedOptionId: null,
focusableOptionsWithIds: [],
focusedValue: null,
inputIsHidden: false,
isFocused: false,
Expand All @@ -595,6 +642,7 @@ export default class Select<
prevWasFocused: false,
inputIsHiddenAfterUpdate: undefined,
prevProps: undefined,
instancePrefix: '',
};

// Misc. Instance Properties
Expand All @@ -605,10 +653,10 @@ export default class Select<
commonProps: any; // TODO
initialTouchX = 0;
initialTouchY = 0;
instancePrefix = '';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved instancePrefix to state to be able to use it inside of getDerivedStateFromProps

openAfterFocus = false;
scrollToFocusedOptionOnUpdate = false;
userIsDragging?: boolean;
isAppleDevice = isAppleDevice();

// Refs
// ------------------------------
Expand All @@ -635,15 +683,21 @@ export default class Select<

constructor(props: Props<Option, IsMulti, Group>) {
super(props);
this.instancePrefix =
this.state.instancePrefix =
'react-select-' + (this.props.instanceId || ++instanceId);
this.state.selectValue = cleanValue(props.value);

// Set focusedOption if menuIsOpen is set on init (e.g. defaultMenuIsOpen)
if (props.menuIsOpen && this.state.selectValue.length) {
const focusableOptionsWithIds: FocusableOptionWithId<Option>[] =
this.getFocusableOptionsWithIds();
const focusableOptions = this.buildFocusableOptions();
const optionIndex = focusableOptions.indexOf(this.state.selectValue[0]);
this.state.focusableOptionsWithIds = focusableOptionsWithIds;
this.state.focusedOption = focusableOptions[optionIndex];
this.state.focusedOptionId = getFocusedOptionId(
Copy link
Contributor Author

@Ke1sy Ke1sy Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need update focusedOptionId each time when focusedOption is updated. This works if menuIsOpen on init

focusableOptionsWithIds,
focusableOptions[optionIndex]
);
}
}

Expand All @@ -658,6 +712,7 @@ export default class Select<
ariaSelection,
isFocused,
prevWasFocused,
instancePrefix,
} = state;
const { options, value, menuIsOpen, inputValue, isMulti } = props;
const selectValue = cleanValue(value);
Expand All @@ -672,13 +727,28 @@ export default class Select<
const focusableOptions = menuIsOpen
? buildFocusableOptions(props, selectValue)
: [];

const focusableOptionsWithIds = menuIsOpen
? buildFocusableOptionsWithIds(
buildCategorizedOptions(props, selectValue),
`${instancePrefix}-option`
)
: [];

const focusedValue = clearFocusValueOnUpdate
? getNextFocusedValue(state, selectValue)
: null;
const focusedOption = getNextFocusedOption(state, focusableOptions);
const focusedOptionId = getFocusedOptionId(
focusableOptionsWithIds,
focusedOption
);

newMenuOptionsState = {
selectValue,
focusedOption,
focusedOptionId,
focusableOptionsWithIds,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recalculation of focusableOptionsWithIds and focusedOptionId after some dependencies has changed (line 720)

focusedValue,
clearFocusValueOnUpdate: false,
};
Expand Down Expand Up @@ -801,6 +871,7 @@ export default class Select<
action: 'menu-close',
prevInputValue: this.props.inputValue,
});

this.props.onMenuClose();
}
onInputChange(newValue: string, actionMeta: InputActionMeta) {
Expand Down Expand Up @@ -844,6 +915,7 @@ export default class Select<
inputIsHiddenAfterUpdate: false,
focusedValue: null,
focusedOption: focusableOptions[openAtIndex],
focusedOptionId: this.getFocusedOptionId(focusableOptions[openAtIndex]),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after search update

},
() => this.onMenuOpen()
);
Expand Down Expand Up @@ -921,6 +993,7 @@ export default class Select<
this.setState({
focusedOption: options[nextFocus],
focusedValue: null,
focusedOptionId: this.getFocusedOptionId(options[nextFocus]),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after keyboard navigation

});
}
onChange = (
Expand All @@ -941,7 +1014,9 @@ export default class Select<
const { closeMenuOnSelect, isMulti, inputValue } = this.props;
this.onInputChange('', { action: 'set-value', prevInputValue: inputValue });
if (closeMenuOnSelect) {
this.setState({ inputIsHiddenAfterUpdate: !isMulti });
this.setState({
inputIsHiddenAfterUpdate: !isMulti,
});
this.onMenuClose();
}
// when the select value should change, we should reset focusedValue
Expand Down Expand Up @@ -1050,6 +1125,20 @@ export default class Select<
};
}

getFocusedOptionId = (focusedOption: Option) => {
return getFocusedOptionId(
this.state.focusableOptionsWithIds,
focusedOption
);
};

getFocusableOptionsWithIds = () => {
return buildFocusableOptionsWithIds(
buildCategorizedOptions(this.props, this.state.selectValue),
this.getElementId('option')
);
};

getValue = () => this.state.selectValue;

cx = (...args: any) => classNames(this.props.classNamePrefix, ...args);
Expand Down Expand Up @@ -1114,7 +1203,7 @@ export default class Select<
| 'placeholder'
| 'live-region'
) => {
return `${this.instancePrefix}-${element}`;
return `${this.state.instancePrefix}-${element}`;
};

getComponents = () => {
Expand Down Expand Up @@ -1437,7 +1526,13 @@ export default class Select<
if (this.blockOptionHover || this.state.focusedOption === focusedOption) {
return;
}
this.setState({ focusedOption });
const options = this.getFocusableOptions();
const focusedOptionIndex = options.indexOf(focusedOption!);
this.setState({
focusedOption,
focusedOptionId:
focusedOptionIndex > -1 ? this.getFocusedOptionId(focusedOption) : null,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating of focusedOptionId (aria-activedescendant) during hover

});
};
shouldHideSelectedOptions = () => {
return shouldHideSelectedOptions(this.props);
Expand Down Expand Up @@ -1536,7 +1631,9 @@ export default class Select<
return;
case 'Escape':
if (menuIsOpen) {
this.setState({ inputIsHiddenAfterUpdate: false });
this.setState({
inputIsHiddenAfterUpdate: false,
});
this.onInputChange('', {
action: 'menu-close',
prevInputValue: inputValue,
Expand Down Expand Up @@ -1624,9 +1721,12 @@ export default class Select<
'aria-labelledby': this.props['aria-labelledby'],
'aria-required': required,
role: 'combobox',
'aria-activedescendant': this.isAppleDevice
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When server-side rendering this hydration throws an error
Warning: Extra attributes from the server: aria-activedescendant
Setting either both to empty sting '' or undefined solves
'aria-activedescendant': this.isAppleDevice ? '' : this.state.focusedOptionId || ''
or
'aria-activedescendant': this.isAppleDevice ? undefined : this.state.focusedOptionId || undefined

Copy link

@khanakia khanakia Nov 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any update on the issue i am also facing the error after update to 5.8.0?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same. Setting instanceID helped fixed className hydration errors but aria-activedescendant still throws.

? undefined
: this.state.focusedOptionId || '',

...(menuIsOpen && {
'aria-controls': this.getElementId('listbox'),
'aria-owns': this.getElementId('listbox'),
}),
...(!isSearchable && {
'aria-readonly': true,
Expand Down Expand Up @@ -1891,6 +1991,8 @@ export default class Select<
onMouseMove: onHover,
onMouseOver: onHover,
tabIndex: -1,
role: 'option',
'aria-selected': this.isAppleDevice ? undefined : isSelected, // is not supported on Apple devices
};

return (
Expand Down Expand Up @@ -1970,7 +2072,6 @@ export default class Select<
innerProps={{
onMouseDown: this.onMenuMouseDown,
onMouseMove: this.onMenuMouseMove,
id: this.getElementId('listbox'),
}}
isLoading={isLoading}
placement={placement}
Expand All @@ -1988,6 +2089,11 @@ export default class Select<
this.getMenuListRef(instance);
scrollTargetRef(instance);
}}
innerProps={{
role: 'listbox',
'aria-multiselectable': commonProps.isMulti,
id: this.getElementId('listbox'),
}}
isLoading={isLoading}
maxHeight={maxHeight}
focusedOption={focusedOption}
Expand Down Expand Up @@ -2079,6 +2185,7 @@ export default class Select<
isFocused={isFocused}
selectValue={selectValue}
focusableOptions={focusableOptions}
isAppleDevice={this.isAppleDevice}
/>
);
}
Expand Down
Loading