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

[V2] live region implementation #2581

Merged
merged 18 commits into from
Jul 3, 2018
Merged

Conversation

gwyneplaine
Copy link
Collaborator

@gwyneplaine gwyneplaine commented May 2, 2018

This PR includes work to substitute the existing minimal aria-label based accessibility implementation with an aria-live-region.

We remove the following labels:

Group

  • aria-labelledby
  • aria-label
  • aria-expanded
  • role

Menu

  • aria-multiselectable
  • role
  • id
  • innerRef (innerRef is now a prop exposed from Menu, I'm happy to revert this change as while logical it is not consistent with the the api we expose for the rest of our components)

Option

  • aria-selected
  • role

aria live region Implementation

  • Added an A11yText element with two children
    • a string representing value events (select-option, deselect-option etc)
    • a string representing context
      • focused-value
      • focused-option
      • results
      • instructions for what to do depending on where the user is focused in the select.
  • See inline code comments for details

@gwyneplaine gwyneplaine force-pushed the v2-live-region-implementation branch from a140d2b to 03c33e3 Compare June 5, 2018 03:54
@gwyneplaine gwyneplaine force-pushed the v2-live-region-implementation branch 2 times, most recently from a2e58fd to 9525702 Compare June 19, 2018 00:53
@gwyneplaine gwyneplaine changed the title WIP V2 live region implementation [V2] live region implementation Jun 19, 2018
@@ -4,6 +4,15 @@ import React, { Component, type ElementRef, type Node } from 'react';

import { createFilter } from './filters';
import { DummyInput, ScrollBlock, ScrollCaptor } from './internal/index';
import {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We import in helpers for constructing the relevant aria live messages. These can eventually be exposed as a command api like so

export const a11yCommands = {
 valueFocus,
 optionFocus, 
 resultContext 
 instructions 
 valueEvent
}

@@ -224,7 +232,7 @@ export const defaultProps = {
pageSize: 5,
placeholder: 'Select...',
screenReaderStatus: ({ count }: { count: number }) =>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Keeping screenReaderStatus for now to not break the existing accessibility API, but i think we should do away with this in favor of the more verbose pattern specified above.

@@ -236,12 +244,22 @@ type MenuOptions = {
};

type State = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Internally we now have two new pieces of state information.

  • ariaLiveContext: (a string value holding instructions for assistive tech based on the context of the user (in menu, focused on value, in input)
  • ariaLiveSelection: a string value holding instructions for assistive tech based on value events that occur (select-option, deselect-option, pop-value etc.)

@gwyneplaine gwyneplaine force-pushed the v2-live-region-implementation branch 4 times, most recently from 9993071 to b3697d1 Compare June 19, 2018 01:22
let focusedIndex = selectValue.indexOf(focusedValue);
if (!focusedValue) {
focusedIndex = -1;
this.announceAriaLiveContext({ event: 'value' });
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if there is no currently focusedValue we can infer that the user is moving from the input to a focused value. We want to inform the user of the shift in context and what to do from this point onwards, hence the ariaLiveContext invocation here.

src/Select.js Outdated
@@ -536,9 +571,11 @@ export default class Select extends Component<Props, State> {
popValue = () => {
const { onChange } = this.props;
const { selectValue } = this.state;
const lastSelectedValue = selectValue[selectValue.length - 1];
this.announceAriaLiveSelection({ event: 'pop-value', context: { value: this.getOptionLabel(lastSelectedValue) } });
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we invoke announceAriaLiveSelection for any events to do with setting or unsetting values from the control.

@@ -536,9 +571,11 @@ export default class Select extends Component<Props, State> {
popValue = () => {
const { onChange } = this.props;
const { selectValue } = this.state;
const lastSelectedValue = selectValue[selectValue.length - 1];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just boyscouting, so we don't attempt to retrieve the same value from the array twice.

@gwyneplaine gwyneplaine force-pushed the v2-live-region-implementation branch 2 times, most recently from 2fc8985 to 695120a Compare June 19, 2018 01:29
@@ -635,6 +672,16 @@ export default class Select extends Component<Props, State> {
// ==============================
// Helpers
// ==============================
announceAriaLiveSelection = ({ event, context }: { event: string, context: ValueEventContext }) => {
this.setState({
ariaLiveSelection: valueEventAriaMessage(event, context),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this calls the valueEventAriaMessage helper with the passed in event and context to resolve to the next value of ariaLiveSelection in state.

}
announceAriaLiveContext = ({ event, context }: { event: string, context?: InstructionsContext }) => {
this.setState({
ariaLiveContext: instructionsAriaMessage(event, { ...context, label: this.props['aria-label'] }),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this calls the instructionsAriaMessage helper with the passed in event and context to resolve to the next value of ariaLiveContext in state.

{screenReaderStatus({ count: this.countOptions() })}
</A11yText>
);
constructAriaLiveMessage () {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

each invocation of constructAriaLiveMessage creates an aria live message to represent the following:

  • focusedValue
  • focusedOption
  • results (amount of options available) and inputvalue
  • ariaLiveContext

We construct this every time on render, in order for us to not have to imperatively infer value focus or option focus change in event calls or within componentWillReceiveProps. valueFocus and optionFocus can change due to any number of user interactions and non user interaction based side-effects from async loading to controlled prop changes outside of our intended scope. Likewise with results.

@@ -1481,6 +1506,16 @@ export default class Select extends Component<Props, State> {
}
}

renderLiveRegion () {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if the input (and hence the select) is not focused, don't render the A11yText elements.

@@ -0,0 +1,30 @@
export type InstructionsContext = { isSearchable?: boolean, isMulti?: boolean, label?: string };
export type ValueEventContext = { value: string };
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's actually better here to pass in value as an object and the function getOptionLabel as a function argument here.

This solves for Creatable select, where we need to be able to either fire an announcementEvent from the wrapping Creatable HoC, or infer whether or not a value isNew to change the returned event string. However this also requires that the user apply getOptionLabel manually, which doesn't seem great, given that it's a prop that they've passed in with the implicit understanding that we'd be handling application for them.

}
};

export const valueFocusAriaMessage = ({ focusedValue, getOptionLabel, selectValue }) => `value ${getOptionLabel(focusedValue)} focused, ${selectValue.indexOf(focusedValue) + 1} of ${selectValue.length}.`;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

see above for argument against this way of doing things. Of course, passing in the focusedValue as a string here, means that instead of getOptionLabel and selectValue, we'll probably have to pass through selectValueSize (number) and focusedValueIndex (number).

};

export const valueFocusAriaMessage = ({ focusedValue, getOptionLabel, selectValue }) => `value ${getOptionLabel(focusedValue)} focused, ${selectValue.indexOf(focusedValue) + 1} of ${selectValue.length}.`;
export const optionFocusAriaMessage = ({ focusedOption, getOptionLabel, options }) => `option ${getOptionLabel(focusedOption)} focused, ${options.indexOf(focusedOption) + 1} of ${options.length}.`;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same as above


export const valueFocusAriaMessage = ({ focusedValue, getOptionLabel, selectValue }) => `value ${getOptionLabel(focusedValue)} focused, ${selectValue.indexOf(focusedValue) + 1} of ${selectValue.length}.`;
export const optionFocusAriaMessage = ({ focusedOption, getOptionLabel, options }) => `option ${getOptionLabel(focusedOption)} focused, ${options.indexOf(focusedOption) + 1} of ${options.length}.`;
export const resultsAriaMessage = ({ inputValue, screenReaderMessage }) => `${screenReaderMessage} for search term ${inputValue}.`;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we ingest a screenReaderMessage which is the returned value of calling the screenReaderStatus prop.

@gwyneplaine gwyneplaine force-pushed the v2-live-region-implementation branch from a53f33c to b8fc4c3 Compare July 3, 2018 02:18
@gwyneplaine gwyneplaine merged commit b3fb7d7 into v2 Jul 3, 2018
@gwyneplaine gwyneplaine deleted the v2-live-region-implementation branch July 3, 2018 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant