Skip to content

Commit

Permalink
Improves screen reader features of combo boxes in edit role screen (#…
Browse files Browse the repository at this point in the history
…153808)

## Summary

Related to #27749

While the [EuiComboBox
rebuild](elastic/eui#2841) is in progress,
this PR addresses missing aria properties and fixes option announcements
for the `run as user` combo. It additionally adds an aria-label for the
Spaces Navigation EuiSelectable that I found was missing.

### Testing
- Add all sample data
- Edit a non-reserved role
- Turn on a screen reader (NVDA is recommended, but VoiceOver will do)
- Tab through the controls on the Edit role screen. When you arrive in a
combo box you may or may not hear an announcement declaring the name of
the combo box (this is expected, especially with VoiceOver).
- With focus in a combo box press the down arrow key. Verify the options
are announced as you traverse them with the down and up arrow keys.
- If there was success in option announcement, press escape key and
verify that the name of the combo box is announced. You may have to
press escape twice when using NVDA.

Note: It seems to be the consensus that this is best we can do with the
existing implementation of EuiComboBox, which is partly why it is being
rebuilt with the EuiSelectable component.
  • Loading branch information
jeramysoucy authored Mar 30, 2023
1 parent 327dd49 commit b3d78e8
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 72 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { EuiComboBox, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import _ from 'lodash';
import React, { Component } from 'react';

import { i18n } from '@kbn/i18n';

import type { Role } from '../../../../../../common/model';
import { isRoleReadOnly } from '../../../../../../common/model';

Expand Down Expand Up @@ -41,6 +43,10 @@ export class ClusterPrivileges extends Component<Props, {}> {
return (
<EuiFlexItem key={'clusterPrivs'}>
<EuiComboBox
aria-label={i18n.translate(
'xpack.security.management.editRole.clusterPrivilegeForm.clusterPrivilegesAriaLabel',
{ defaultMessage: 'Cluster privileges' }
)}
data-test-subj={'cluster-privileges-combobox'}
options={options}
selectedOptions={selectedOptions}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,27 +118,28 @@ export class ElasticsearchPrivileges extends Component<Props, {}> {
</p>
}
>
<EuiFormRow hasEmptyLabelSpace>
<EuiComboBox
placeholder={
this.props.editable
? i18n.translate(
'xpack.security.management.editRole.elasticSearchPrivileges.addUserTitle',
{ defaultMessage: 'Add a user…' }
)
: undefined
}
options={this.props.runAsUsers.map((username) => ({
id: username,
label: username,
isGroupLabelOption: false,
}))}
selectedOptions={this.props.role.elasticsearch.run_as.map((u) => ({ label: u }))}
onCreateOption={this.onCreateRunAsOption}
onChange={this.onRunAsUserChange}
isDisabled={!editable}
/>
</EuiFormRow>
<EuiComboBox
aria-label={i18n.translate(
'xpack.security.management.editRole.elasticSearchPrivileges.runAsPrivilegesAriaLabel',
{ defaultMessage: 'Run as privileges' }
)}
placeholder={
this.props.editable
? i18n.translate(
'xpack.security.management.editRole.elasticSearchPrivileges.addUserTitle',
{ defaultMessage: 'Add a user…' }
)
: undefined
}
options={this.props.runAsUsers.map((username) => ({
label: username,
isGroupLabelOption: false,
}))}
selectedOptions={this.props.role.elasticsearch.run_as.map((u) => ({ label: u }))}
onCreateOption={this.onCreateRunAsOption}
onChange={this.onRunAsUserChange}
isDisabled={!editable}
/>
</EuiDescribedFormGroup>

<EuiSpacer />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,18 +241,16 @@ export class IndexPrivilegeForm extends Component<Props, State> {
) : undefined
}
>
<Fragment>
<EuiComboBox
data-test-subj={`fieldInput${this.props.formIndex}`}
options={this.state.flsOptions.map(toOption)}
selectedOptions={grant.map(toOption)}
onCreateOption={this.onCreateGrantedField}
onChange={this.onGrantedFieldsChange}
isDisabled={this.props.isRoleReadOnly}
async={true}
isLoading={this.state.isFieldListLoading}
/>
</Fragment>
<EuiComboBox
data-test-subj={`fieldInput${this.props.formIndex}`}
options={this.state.flsOptions.map(toOption)}
selectedOptions={grant.map(toOption)}
onCreateOption={this.onCreateGrantedField}
onChange={this.onGrantedFieldsChange}
isDisabled={this.props.isRoleReadOnly}
async={true}
isLoading={this.state.isFieldListLoading}
/>
</EuiFormRow>
</EuiFlexItem>
<EuiFlexItem>
Expand All @@ -266,18 +264,16 @@ export class IndexPrivilegeForm extends Component<Props, State> {
fullWidth={true}
className="indexPrivilegeForm__deniedFieldsRow"
>
<Fragment>
<EuiComboBox
data-test-subj={`deniedFieldInput${this.props.formIndex}`}
options={this.state.flsOptions.map(toOption)}
selectedOptions={except.map(toOption)}
onCreateOption={this.onCreateDeniedField}
onChange={this.onDeniedFieldsChange}
isDisabled={isRoleReadOnly}
async={true}
isLoading={this.state.isFieldListLoading}
/>
</Fragment>
<EuiComboBox
data-test-subj={`deniedFieldInput${this.props.formIndex}`}
options={this.state.flsOptions.map(toOption)}
selectedOptions={except.map(toOption)}
onCreateOption={this.onCreateDeniedField}
onChange={this.onDeniedFieldsChange}
isDisabled={isRoleReadOnly}
async={true}
isLoading={this.state.isFieldListLoading}
/>
</EuiFormRow>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type {
EuiSelectableOnChangeEvent,
EuiSelectableSearchableSearchProps,
} from '@elastic/eui/src/components/selectable/selectable';
import React, { Component, lazy, Suspense } from 'react';
import React, { Component, Fragment, lazy, Suspense } from 'react';

import type { ApplicationStart, Capabilities } from '@kbn/core/public';
import { i18n } from '@kbn/i18n';
Expand Down Expand Up @@ -85,8 +85,11 @@ class SpacesMenuUI extends Component<Props> {
};

return (
<>
<Fragment>
<EuiSelectable
aria-label={i18n.translate('xpack.spaces.navControl.spacesMenu.spacesAriaLabel', {
defaultMessage: 'Spaces',
})}
id={this.props.id}
className={'spcMenu'}
title={i18n.translate('xpack.spaces.navControl.spacesMenu.changeCurrentSpaceTitle', {
Expand All @@ -105,19 +108,19 @@ class SpacesMenuUI extends Component<Props> {
}}
>
{(list, search) => (
<>
<Fragment>
<EuiPopoverTitle paddingSize="s">
{search ||
i18n.translate('xpack.spaces.navControl.spacesMenu.selectSpacesTitle', {
defaultMessage: 'Your spaces',
})}
</EuiPopoverTitle>
{list}
</>
</Fragment>
)}
</EuiSelectable>
<EuiPopoverFooter paddingSize="s">{this.renderManageButton()}</EuiPopoverFooter>
</>
</Fragment>
);
}

Expand Down

0 comments on commit b3d78e8

Please sign in to comment.