From b3d78e800ea0cf22ed8c3ebb9c1129b56728090b Mon Sep 17 00:00:00 2001
From: Jeramy Soucy <jeramy.soucy@elastic.co>
Date: Thu, 30 Mar 2023 11:59:45 -0400
Subject: [PATCH] Improves screen reader features of combo boxes in edit role
 screen (#153808)

## Summary

Related to #27749

While the [EuiComboBox
rebuild](https://github.com/elastic/eui/issues/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.
---
 .../cluster_privileges.test.tsx.snap          |  1 +
 .../elasticsearch_privileges.test.tsx.snap    | 37 +++++++---------
 .../privileges/es/cluster_privileges.tsx      |  6 +++
 .../es/elasticsearch_privileges.tsx           | 43 +++++++++---------
 .../privileges/es/index_privilege_form.tsx    | 44 +++++++++----------
 .../nav_control/components/spaces_menu.tsx    | 13 +++---
 6 files changed, 72 insertions(+), 72 deletions(-)

diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/cluster_privileges.test.tsx.snap b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/cluster_privileges.test.tsx.snap
index dd27fe13e84a3..ae57c00324ccd 100644
--- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/cluster_privileges.test.tsx.snap
+++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/cluster_privileges.test.tsx.snap
@@ -6,6 +6,7 @@ exports[`it renders without crashing 1`] = `
     key="clusterPrivs"
   >
     <EuiComboBox
+      aria-label="Cluster privileges"
       async={false}
       compressed={false}
       data-test-subj="cluster-privileges-combobox"
diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap
index 5c4097aacd325..25196b9929f5a 100644
--- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap
+++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/__snapshots__/elasticsearch_privileges.test.tsx.snap
@@ -100,28 +100,21 @@ exports[`it renders without crashing 1`] = `
       </h3>
     }
   >
-    <EuiFormRow
-      describedByIds={Array []}
-      display="row"
-      hasChildLabel={true}
-      hasEmptyLabelSpace={true}
-      labelType="label"
-    >
-      <EuiComboBox
-        async={false}
-        compressed={false}
-        fullWidth={false}
-        isClearable={true}
-        isDisabled={false}
-        onChange={[Function]}
-        onCreateOption={[Function]}
-        options={Array []}
-        placeholder="Add a user…"
-        selectedOptions={Array []}
-        singleSelection={false}
-        sortMatchesBy="none"
-      />
-    </EuiFormRow>
+    <EuiComboBox
+      aria-label="Run as privileges"
+      async={false}
+      compressed={false}
+      fullWidth={false}
+      isClearable={true}
+      isDisabled={false}
+      onChange={[Function]}
+      onCreateOption={[Function]}
+      options={Array []}
+      placeholder="Add a user…"
+      selectedOptions={Array []}
+      singleSelection={false}
+      sortMatchesBy="none"
+    />
   </EuiDescribedFormGroup>
   <EuiSpacer />
   <EuiTitle
diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/cluster_privileges.tsx b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/cluster_privileges.tsx
index 0e44970d8ef7a..4eb9827990849 100644
--- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/cluster_privileges.tsx
+++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/cluster_privileges.tsx
@@ -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';
 
@@ -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}
diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/elasticsearch_privileges.tsx b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/elasticsearch_privileges.tsx
index 045c4d924c426..6ae205c62cfa7 100644
--- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/elasticsearch_privileges.tsx
+++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/elasticsearch_privileges.tsx
@@ -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 />
diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/index_privilege_form.tsx b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/index_privilege_form.tsx
index fbc440c0ed984..934de8921ec88 100644
--- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/index_privilege_form.tsx
+++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/es/index_privilege_form.tsx
@@ -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>
@@ -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>
diff --git a/x-pack/plugins/spaces/public/nav_control/components/spaces_menu.tsx b/x-pack/plugins/spaces/public/nav_control/components/spaces_menu.tsx
index 1d54c639d84d8..adad431cd05ea 100644
--- a/x-pack/plugins/spaces/public/nav_control/components/spaces_menu.tsx
+++ b/x-pack/plugins/spaces/public/nav_control/components/spaces_menu.tsx
@@ -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';
@@ -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', {
@@ -105,7 +108,7 @@ class SpacesMenuUI extends Component<Props> {
           }}
         >
           {(list, search) => (
-            <>
+            <Fragment>
               <EuiPopoverTitle paddingSize="s">
                 {search ||
                   i18n.translate('xpack.spaces.navControl.spacesMenu.selectSpacesTitle', {
@@ -113,11 +116,11 @@ class SpacesMenuUI extends Component<Props> {
                   })}
               </EuiPopoverTitle>
               {list}
-            </>
+            </Fragment>
           )}
         </EuiSelectable>
         <EuiPopoverFooter paddingSize="s">{this.renderManageButton()}</EuiPopoverFooter>
-      </>
+      </Fragment>
     );
   }