Fixes several a11y issues manifesting in Kibana (#2411)

Fixes several of misc a11y issues showing up in Kibana: - Poor labeling in `EuiSuperDatePicker` ([Kibana issue 38794](elastic/kibana#38794)) (this kind of snowballed into improving several things in the vicinity) - Fixes #2416: `EuiFormRow`s that render fields that also render a label end up rendering multiple labels for an input which breaks many assistive tech (this issue was found during the in progress Kibana rollout of automated a11y testing ([Kibana issue 43584](elastic/kibana#43584) ) - Fixes #2414: `EuiCodeEditor`s would render with the same ID which breaks many assistive tech (this issue was found during the in progress Kibana rollout of automated a11y testing)
elastic · Oct 11, 2019 · f475ce9 · f475ce9
1 parent df5b1b2
commit f475ce9
Show file tree

Hide file tree

Showing 22 changed files with 514 additions and 405 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,13 +4,17 @@
 - Added new `EuiColorStops` component ([#2360](https://github.com/elastic/eui/pull/2360))
 - Added `currency` glyph to 'EuiIcon' ([#2398](https://github.com/elastic/eui/pull/2398))
 - Migrate `EuiBreadcrumbs`, `EuiHeader` etc, and `EuiLink` to TypeScript ([#2391](https://github.com/elastic/eui/pull/2391))
+- Added `hasChildLabel` prop to `EuiFormRow` to avoid duplicate labels ([#2390](https://github.com/elastic/eui/pull/2390))
 - Added `component` prop to `EuiPageBody`, switching the default from `div` to `main` ([#2410](https://github.com/elastic/eui/pull/2410))
 - Added focus state to `EuiListGroupItem` ([#2406](https://github.com/elastic/eui/pull/2406))
 - Added `keyboardShorcut` glyph to 'EuiIcon ([#2413](https://github.com/elastic/eui/pull/2413))
 
 **Bug fixes**
 
-- Fix `EuiSelectable` to accept programmatic updates to its `options` prop ([#2390](https://github.com/elastic/eui/pull/2390))
+- Fixed `EuiSelectable` to accept programmatic updates to its `options` prop ([#2390](https://github.com/elastic/eui/pull/2390))
+- Fixed poor labeling in `EuiSuperDatePicker` ([#2390](https://github.com/elastic/eui/pull/2411))
+- Fixed `EuiCodeEditor`'s ID to be dynamic between renders ([#2390](https://github.com/elastic/eui/pull/2411))
+- Fixed `EuiCodeEditor` to not render multiple labels for some inputs ([#2390](https://github.com/elastic/eui/pull/2411))
 
 ## [`14.4.0`](https://github.com/elastic/eui/tree/v14.4.0)
 

diff --git a/src-docs/src/views/form_layouts/form_rows.js b/src-docs/src/views/form_layouts/form_rows.js
@@ -117,7 +117,10 @@ export default class extends Component {
           <EuiRange min={0} max={100} name="range" id="range" />
         </EuiFormRow>
 
-        <EuiFormRow label="Use a switch instead of a single checkbox">
+        <EuiFormRow
+          label="Use a switch instead of a single checkbox"
+          labelAppend="Some inputs also render their own labels, such as the switch, so they need the row label turned off because multiple labels break screen readers. "
+          hasChildLabel={false}>
           <EuiSwitch
             name="switch"
             label="Should we do this?"

diff --git a/src/components/code_editor/__snapshots__/code_editor.test.js.snap b/src/components/code_editor/__snapshots__/code_editor.test.js.snap
@@ -4,7 +4,7 @@ exports[`EuiCodeEditor behavior hint element should be disabled when the ui ace
 <div
   class="euiCodeEditorKeyboardHint"
   data-test-subj="codeEditorHint"
-  id="42"
+  id="htmlId"
   role="button"
   tabindex="0"
 >
@@ -25,7 +25,7 @@ exports[`EuiCodeEditor behavior hint element should be enabled when the ui ace b
 <div
   class="euiCodeEditorKeyboardHint"
   data-test-subj="codeEditorHint"
-  id="42"
+  id="htmlId"
   role="button"
   tabindex="0"
 >
@@ -46,7 +46,7 @@ exports[`EuiCodeEditor behavior hint element should be tabable 1`] = `
 <div
   class="euiCodeEditorKeyboardHint"
   data-test-subj="codeEditorHint"
-  id="42"
+  id="htmlId"
   role="button"
   tabindex="0"
 >
@@ -71,7 +71,7 @@ exports[`EuiCodeEditor is rendered 1`] = `
   <div
     class="euiCodeEditorKeyboardHint"
     data-test-subj="codeEditorHint"
-    id="42"
+    id="htmlId"
     role="button"
     tabindex="0"
   >
@@ -88,7 +88,7 @@ exports[`EuiCodeEditor is rendered 1`] = `
   </div>
   <div
     class=" ace_editor ace-tm testClass1 testClass2"
-    id="brace-editor"
+    id="htmlId"
     style="width: 500px; height: 500px;"
   >
     <textarea
@@ -187,7 +187,7 @@ exports[`EuiCodeEditor props aria attributes allows setting aria-describedby on
   <div
     class="euiCodeEditorKeyboardHint"
     data-test-subj="codeEditorHint"
-    id="42"
+    id="htmlId"
     role="button"
     tabindex="0"
   >
@@ -204,7 +204,7 @@ exports[`EuiCodeEditor props aria attributes allows setting aria-describedby on
   </div>
   <div
     class=" ace_editor ace-tm"
-    id="brace-editor"
+    id="htmlId"
     style="width: 500px; height: 500px;"
   >
     <textarea
@@ -303,7 +303,7 @@ exports[`EuiCodeEditor props aria attributes allows setting aria-labelledby on t
   <div
     class="euiCodeEditorKeyboardHint"
     data-test-subj="codeEditorHint"
-    id="42"
+    id="htmlId"
     role="button"
     tabindex="0"
   >
@@ -320,7 +320,7 @@ exports[`EuiCodeEditor props aria attributes allows setting aria-labelledby on t
   </div>
   <div
     class=" ace_editor ace-tm"
-    id="brace-editor"
+    id="htmlId"
     style="width: 500px; height: 500px;"
   >
     <textarea
@@ -419,7 +419,7 @@ exports[`EuiCodeEditor props isReadOnly renders alternate hint text 1`] = `
   <div
     class="euiCodeEditorKeyboardHint"
     data-test-subj="codeEditorHint"
-    id="42"
+    id="htmlId"
     role="button"
     tabindex="0"
   >
@@ -436,7 +436,7 @@ exports[`EuiCodeEditor props isReadOnly renders alternate hint text 1`] = `
   </div>
   <div
     class=" ace_editor ace-tm"
-    id="brace-editor"
+    id="htmlId"
     style="width: 500px; height: 500px;"
   >
     <textarea

diff --git a/src/components/code_editor/code_editor.js b/src/components/code_editor/code_editor.js
@@ -204,6 +204,7 @@ export class EuiCodeEditor extends Component {
         {prompt}
 
         <AceEditor
+          name={this.idGenerator()}
           ref={this.aceEditorRef}
           width={width}
           height={height}

diff --git a/src/components/code_editor/code_editor.test.js b/src/components/code_editor/code_editor.test.js
@@ -11,9 +11,7 @@ import {
 
 // Mock the htmlIdGenerator to generate predictable ids for snapshot tests
 jest.mock('../../services/accessibility/html_id_generator', () => ({
-  htmlIdGenerator: () => {
-    return () => 42;
-  },
+  htmlIdGenerator: () => () => 'htmlId',
 }));
 
 describe('EuiCodeEditor', () => {

diff --git a/src/components/date_picker/super_date_picker/date_popover/relative_tab.js b/src/components/date_picker/super_date_picker/date_popover/relative_tab.js
@@ -2,7 +2,7 @@ import PropTypes from 'prop-types';
 import React, { Component } from 'react';
 import dateMath from '@elastic/datemath';
 import { toSentenceCase } from '../../../../services/string/to_case';
-
+import { htmlIdGenerator } from '../../../../services';
 import { EuiFlexGroup, EuiFlexItem } from '../../../flex';
 import {
   EuiForm,
@@ -21,6 +21,8 @@ import {
   parseRelativeParts,
   toRelativeStringFromParts,
 } from '../relative_utils';
+import { EuiScreenReaderOnly } from '../../../accessibility';
+import { EuiI18n } from '../../../i18n';
 
 export class EuiRelativeTab extends Component {
   constructor(props) {
@@ -33,6 +35,8 @@ export class EuiRelativeTab extends Component {
     };
   }
 
+  generateId = htmlIdGenerator();
+
   onCountChange = evt => {
     const sanitizedValue = parseInt(evt.target.value, 10);
     this.setState(
@@ -69,6 +73,7 @@ export class EuiRelativeTab extends Component {
   };
 
   render() {
+    const relativeDateInputNumberDescriptionId = this.generateId();
     const isInvalid = this.state.count < 0;
     const parsedValue = dateMath.parse(this.props.value, {
       roundUp: this.props.roundUp,
@@ -81,45 +86,87 @@ export class EuiRelativeTab extends Component {
       <EuiForm className="euiDatePopoverContent__padded">
         <EuiFlexGroup gutterSize="s" responsive={false}>
           <EuiFlexItem>
-            <EuiFormRow
-              isInvalid={isInvalid}
-              error={isInvalid ? 'Must be >= 0' : null}>
-              <EuiFieldNumber
-                compressed
-                aria-label="Count of"
-                data-test-subj={'superDatePickerRelativeDateInputNumber'}
-                value={this.state.count}
-                onChange={this.onCountChange}
-                isInvalid={isInvalid}
-              />
-            </EuiFormRow>
+            <EuiI18n
+              tokens={[
+                'euiRelativeTab.numberInputError',
+                'euiRelativeTab.numberInputLabel',
+              ]}
+              defaults={['Must be >= 0', 'Time span amount']}>
+              {([numberInputError, numberInputLabel]) => (
+                <EuiFormRow
+                  isInvalid={isInvalid}
+                  error={isInvalid ? numberInputError : null}>
+                  <EuiFieldNumber
+                    compressed
+                    aria-label={numberInputLabel}
+                    aria-describedby={relativeDateInputNumberDescriptionId}
+                    data-test-subj={'superDatePickerRelativeDateInputNumber'}
+                    value={this.state.count}
+                    onChange={this.onCountChange}
+                    isInvalid={isInvalid}
+                  />
+                </EuiFormRow>
+              )}
+            </EuiI18n>
           </EuiFlexItem>
           <EuiFlexItem>
-            <EuiSelect
-              compressed
-              data-test-subj={'superDatePickerRelativeDateInputUnitSelector'}
-              value={this.state.unit}
-              options={relativeOptions}
-              onChange={this.onUnitChange}
-            />
+            <EuiI18n
+              token="euiRelativeTab.unitInputLabel"
+              default="Relative time span">
+              {unitInputLabel => (
+                <EuiSelect
+                  compressed
+                  aria-label={unitInputLabel}
+                  data-test-subj={
+                    'superDatePickerRelativeDateInputUnitSelector'
+                  }
+                  value={this.state.unit}
+                  options={relativeOptions}
+                  onChange={this.onUnitChange}
+                />
+              )}
+            </EuiI18n>
           </EuiFlexItem>
         </EuiFlexGroup>
         <EuiSpacer size="m" />
-        <EuiSwitch
-          data-test-subj={'superDatePickerRelativeDateRoundSwitch'}
-          label={`Round to the ${timeUnits[this.state.unit.substring(0, 1)]}`}
-          checked={this.state.round}
-          onChange={this.onRoundChange}
-        />
+        <EuiI18n
+          token="euiRelativeTab.roundingLabel"
+          default="Round to the {unit}"
+          values={{ unit: timeUnits[this.state.unit.substring(0, 1)] }}>
+          {roundingLabel => (
+            <EuiSwitch
+              data-test-subj={'superDatePickerRelativeDateRoundSwitch'}
+              label={roundingLabel}
+              checked={this.state.round}
+              onChange={this.onRoundChange}
+            />
+          )}
+        </EuiI18n>
+
         <EuiSpacer size="m" />
         <EuiFieldText
           compressed
           value={formatedValue}
           readOnly
           prepend={
-            <EuiFormLabel>{this.state.sentenceCasedPosition} date</EuiFormLabel>
+            <EuiFormLabel>
+              <EuiI18n
+                token="euiRelativeTab.relativeDate"
+                default="{position} date"
+                values={{ position: this.state.sentenceCasedPosition }}
+              />
+            </EuiFormLabel>
           }
         />
+        <EuiScreenReaderOnly id={relativeDateInputNumberDescriptionId}>
+          <p>
+            <EuiI18n
+              token="euiRelativeTab.fullDescription"
+              default="The unit is changeable. Currently set to {unit}."
+              values={{ unit: this.state.unit }}
+            />
+          </p>
+        </EuiScreenReaderOnly>
       </EuiForm>
     );
   }