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

Kashkovsky intl a11y, attempt 2 #4161

Closed
wants to merge 9 commits into from
Closed
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
60 changes: 44 additions & 16 deletions packages/react-select/src/Select.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ import {
resultsAriaMessage,
valueEventAriaMessage,
instructionsAriaMessage,
type AccessibilityProp,
type AccessibilityConfig,
type InstructionsContext,
type ValueEventContext,
type ValueEventType,
type InstructionEventType,
} from './accessibility/index';

import {
Expand Down Expand Up @@ -75,6 +79,8 @@ type FormatOptionLabelMeta = {
};

export type Props = {
/* Custom ARIA message functions */
accessibility?: AccessibilityProp,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this is a bit of semantics, but I believe accessibility is a bit vague for a prop.

Would you be willing to consider something more along the lines of ariaLiveConfig? I think it better encapsulates the functionality.

Additionally, this would provide more clarity and avoids confusion with perhaps other things aria/accessibility related.

/* Aria label (for assistive tech) */
'aria-label'?: string,
/* HTML ID of an element that should be used as the label (for assistive tech) */
Expand Down Expand Up @@ -130,6 +136,8 @@ export type Props = {
filterOption:
| (({ label: string, value: string, data: OptionType }, string) => boolean)
| null,
/* Sets the form attribute on the input */
form?: string,
/*
Formats group labels in the menu as React components

Expand Down Expand Up @@ -243,8 +251,6 @@ export type Props = {
tabSelectsValue: boolean,
/* The value of the select; reflected by the selected option */
value: ValueType,
/* Sets the form attribute on the input */
form?: string,
};

export const defaultProps = {
Expand Down Expand Up @@ -285,6 +291,13 @@ export const defaultProps = {
styles: {},
tabIndex: '0',
tabSelectsValue: true,
accessibility: {
valueFocusAriaMessage,
optionFocusAriaMessage,
resultsAriaMessage,
valueEventAriaMessage,
instructionsAriaMessage
},
};

type MenuOptions = {
Expand Down Expand Up @@ -322,7 +335,7 @@ export default class Select extends Component<Props, State> {

// Misc. Instance Properties
// ------------------------------

accessibility: AccessibilityConfig;
blockOptionHover: boolean = false;
isComposing: boolean = false;
clearFocusValueOnUpdate: boolean = false;
Expand Down Expand Up @@ -364,6 +377,7 @@ export default class Select extends Component<Props, State> {
super(props);
const { value } = props;
this.cacheComponents = memoizeOne(this.cacheComponents, isEqual).bind(this);
this.accessibility = this.getAccessibilityConfig(props.accessibility);
this.cacheComponents(props.components);
this.instancePrefix =
'react-select-' + (this.props.instanceId || ++instanceId);
Expand Down Expand Up @@ -404,6 +418,7 @@ export default class Select extends Component<Props, State> {
const { options, value, menuIsOpen, inputValue } = this.props;
// re-cache custom components
this.cacheComponents(nextProps.components);
this.accessibility = this.getAccessibilityConfig(nextProps.accessibility);
// rebuild the menu options
if (
nextProps.value !== value ||
Expand Down Expand Up @@ -823,26 +838,36 @@ export default class Select extends Component<Props, State> {
// ==============================
// Helpers
// ==============================
getAccessibilityConfig (accessibilityObj?: AccessibilityProp): AccessibilityConfig {
return {
valueFocusAriaMessage,
optionFocusAriaMessage,
resultsAriaMessage,
valueEventAriaMessage,
instructionsAriaMessage,
...accessibilityObj,
};
};
announceAriaLiveSelection = ({
event,
context,
}: {
event: string,
event: ValueEventType,
context: ValueEventContext,
}) => {
this.setState({
ariaLiveSelection: valueEventAriaMessage(event, context),
ariaLiveSelection: this.accessibility.valueEventAriaMessage(event, context),
});
};
announceAriaLiveContext = ({
event,
context,
}: {
event: string,
event: InstructionEventType,
context?: InstructionsContext,
}) => {
this.setState({
ariaLiveContext: instructionsAriaMessage(event, {
ariaLiveContext: this.accessibility.instructionsAriaMessage(event, {
...context,
label: this.props['aria-label'],
}),
Expand Down Expand Up @@ -1380,24 +1405,23 @@ export default class Select extends Component<Props, State> {
const { options, menuIsOpen, inputValue, screenReaderStatus } = this.props;

// An aria live message representing the currently focused value in the select.
const focusedValueMsg = focusedValue
? valueFocusAriaMessage({
const focusedValueMsg = focusedValue && this.accessibility
? this.accessibility.valueFocusAriaMessage({
focusedValue,
getOptionLabel: this.getOptionLabel,
selectValue,
})
: '';
// An aria live message representing the currently focused option in the select.
const focusedOptionMsg =
focusedOption && menuIsOpen
? optionFocusAriaMessage({
focusedOption && menuIsOpen ? this.accessibility.optionFocusAriaMessage({
focusedOption,
getOptionLabel: this.getOptionLabel,
options,
})
: '';
// An aria live message representing the set of focusable results and current searchterm/inputvalue.
const resultsMsg = resultsAriaMessage({
const resultsMsg = this.accessibility.resultsAriaMessage({
inputValue,
screenReaderMessage: screenReaderStatus({ count: this.countOptions() }),
});
Expand Down Expand Up @@ -1815,10 +1839,14 @@ export default class Select extends Component<Props, State> {
renderLiveRegion() {

Choose a reason for hiding this comment

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

It might be good to abstract this to a separate component at this time. What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your feedback!
I am not sure. Looking at other render helpers, renderLiveRegion seems to line up pretty well? On the other hand one could argue for a decoupling exercise of the whole Select class, being ~1600 lines tall. But perhaps that work should be logged in a separate work package. What are you thoughts?

Choose a reason for hiding this comment

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

I agree! You are following the convent. Also agree that this is 1600 lines. So maybe another PR would be good to refactor some of the render* function into separate components.

if (!this.state.isFocused) return null;
return (
<A11yText aria-live="polite">
<p id="aria-selection-event">&nbsp;{this.state.ariaLiveSelection}</p>
<p id="aria-context">&nbsp;{this.constructAriaLiveMessage()}</p>
</A11yText>
<span>
<A11yText aria-live="assertive">
<p id="aria-selection-event">&nbsp;{this.state.ariaLiveSelection}</p>
</A11yText>
<A11yText aria-live="polite">
<p id="aria-context">&nbsp;{this.constructAriaLiveMessage()}</p>
</A11yText>
</span>
);
}

Expand Down
38 changes: 38 additions & 0 deletions packages/react-select/src/__tests__/Select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1862,6 +1862,44 @@ test('accessibility > interacting with disabled options shows correct A11yText',
);
});

test('accessibility > A11yTexts can be provided through accessibility prop', () => {

const accessibility = {
valueEventAriaMessage: (
event,
context
) => {
const { value, isDisabled } = context;
if (event === 'select-option' && !isDisabled) {
return `CUSTOM: option ${value} is selected.`;
}
}
};

let { container } = render(
<Select
{...BASIC_PROPS}
accessibility={accessibility}
options={OPTIONS}
inputValue={''}
menuIsOpen
/>
);
const liveRegionEventId = '#aria-selection-event';
fireEvent.focus(container.querySelector('.react-select__input input'));

let menu = container.querySelector('.react-select__menu');
fireEvent.keyDown(menu, { keyCode: 40, key: 'ArrowDown' });
fireEvent.keyDown(container.querySelector('.react-select__menu'), {
keyCode: 13,
key: 'Enter',
});

expect(container.querySelector(liveRegionEventId).textContent).toMatch(
'CUSTOM: option 0 is selected.'
);
});

test('accessibility > screenReaderStatus function prop > to pass custom text to A11yText', () => {
const screenReaderStatus = ({ count }) =>
`There are ${count} options available`;
Expand Down
46 changes: 43 additions & 3 deletions packages/react-select/src/accessibility/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,38 @@

import { type OptionType, type OptionsType } from '../types';

export type AccessibilityProp = {
valueFocusAriaMessage?: (args: {
focusedValue: OptionType,
getOptionLabel: (data: OptionType) => string,
selectValue: OptionsType
}) => string,
optionFocusAriaMessage?: (args: {
focusedOption: OptionType,
getOptionLabel: (data: OptionType) => string,
options: OptionsType
}) => string,
resultsAriaMessage?: (args: { inputValue: string, screenReaderMessage: string }) => string,
valueEventAriaMessage?: (event: ValueEventType, context: ValueEventContext) => string,
instructionsAriaMessage?: (event: InstructionEventType, context?: InstructionsContext) => string
};

export type AccessibilityConfig = {
valueFocusAriaMessage: (args: {
focusedValue: OptionType,
getOptionLabel: (data: OptionType) => string,
selectValue: OptionsType
}) => string,
optionFocusAriaMessage: (args: {
focusedOption: OptionType,
getOptionLabel: (data: OptionType) => string,
options: OptionsType
}) => string,
resultsAriaMessage: (args: { inputValue: string, screenReaderMessage: string }) => string,
valueEventAriaMessage: (event: ValueEventType, context: ValueEventContext) => string,
instructionsAriaMessage: (event: InstructionEventType, context?: InstructionsContext) => string
}

export type InstructionsContext = {
isSearchable?: boolean,
isMulti?: boolean,
Expand All @@ -10,8 +42,10 @@ export type InstructionsContext = {
};
export type ValueEventContext = { value: string, isDisabled?: boolean };

export type InstructionEventType = 'menu' | 'input' | 'value';

export const instructionsAriaMessage = (
event: string,
event: InstructionEventType,
context?: InstructionsContext = {}
) => {
const { isSearchable, isMulti, label, isDisabled } = context;
Expand All @@ -26,22 +60,28 @@ export const instructionsAriaMessage = (
}`;
case 'value':
return 'Use left and right to toggle between focused values, press Backspace to remove the currently focused value';
default:
return '';
}
};

export type ValueEventType = 'deselect-option' | 'pop-value' | 'remove-value' | 'select-option';

export const valueEventAriaMessage = (
event: string,
event: ValueEventType,
context: ValueEventContext
) => {
const { value, isDisabled } = context;
if (!value) return;
if (!value) return '';
switch (event) {
case 'deselect-option':
case 'pop-value':
case 'remove-value':
return `option ${value}, deselected.`;
case 'select-option':
return isDisabled ? `option ${value} is disabled. Select another option.` : `option ${value}, selected.`;
default:
return '';
}
};

Expand Down