[Lens] accessibility screen reader issues (#84395) (#84894)

* [Lens] accessibility screen reader issues * fix i18n * fix: no aria-label on divs * cr fixes Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
elastic · Dec 3, 2020 · 36a62c1 · 36a62c1
1 parent b9e0552
commit 36a62c1
Show file tree

Hide file tree

Showing 12 changed files with 184 additions and 101 deletions.
diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx
@@ -94,7 +94,7 @@ function LayerPanels(
           {...props}
           key={layerId}
           layerId={layerId}
-          dataTestSubj={`lns-layerPanel-${index}`}
+          index={index}
           visualizationState={visualizationState}
           updateVisualization={setVisualizationState}
           updateDatasource={updateDatasource}

diff --git a/...ugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss b/...ugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss
@@ -13,7 +13,39 @@
   animation: euiFlyout $euiAnimSpeedNormal $euiAnimSlightResistance;
 }
 
-.lnsDimensionContainer__footer,
-.lnsDimensionContainer__header {
+.lnsDimensionContainer__footer {
   padding: $euiSizeS;
 }
+
+.lnsDimensionContainer__header {
+  padding: $euiSizeS $euiSizeXS;
+}
+
+.lnsDimensionContainer__headerTitle {
+  padding: $euiSizeS $euiSizeXS;
+  cursor: pointer;
+
+  &:hover {
+    text-decoration: underline;
+  }
+}
+
+.lnsDimensionContainer__headerLink {
+  &:focus-within {
+    background-color: transparentize($euiColorVis1, .9);
+
+    .lnsDimensionContainer__headerTitle {
+      text-decoration: underline;
+    }
+  }
+}
+
+.lnsDimensionContainer__backIcon {
+  &:hover {
+    transform: none !important; // sass-lint:disable-line no-important
+  }
+
+  &:focus {
+    background-color: transparent;
+  }
+}
diff --git a/...lugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx b/...lugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx
@@ -10,7 +10,9 @@ import {
   EuiFlyoutHeader,
   EuiFlyoutFooter,
   EuiTitle,
+  EuiButtonIcon,
   EuiButtonEmpty,
+  EuiFlexGroup,
   EuiFlexItem,
   EuiFocusTrap,
   EuiOutsideClickDetector,
@@ -54,24 +56,42 @@ export function DimensionContainer({
           className="lnsDimensionContainer"
         >
           <EuiFlyoutHeader hasBorder className="lnsDimensionContainer__header">
-            <EuiTitle size="xs">
-              <EuiButtonEmpty
-                onClick={closeFlyout}
-                data-test-subj="lns-indexPattern-dimensionContainerTitle"
-                id="lnsDimensionContainerTitle"
-                iconType="sortLeft"
-                flush="left"
-              >
-                <strong>
-                  {i18n.translate('xpack.lens.configure.configurePanelTitle', {
-                    defaultMessage: '{groupLabel} configuration',
-                    values: {
-                      groupLabel,
-                    },
+            <EuiFlexGroup
+              gutterSize="none"
+              alignItems="center"
+              className="lnsDimensionContainer__headerLink"
+              onClick={closeFlyout}
+            >
+              <EuiFlexItem grow={false}>
+                <EuiButtonIcon
+                  color="text"
+                  data-test-subj="lns-indexPattern-dimensionContainerBack"
+                  className="lnsDimensionContainer__backIcon"
+                  onClick={closeFlyout}
+                  iconType="sortLeft"
+                  aria-label={i18n.translate('xpack.lens.dimensionContainer.closeConfiguration', {
+                    defaultMessage: 'Close configuration',
                   })}
-                </strong>
-              </EuiButtonEmpty>
-            </EuiTitle>
+                />
+              </EuiFlexItem>
+              <EuiFlexItem grow={true}>
+                <EuiTitle size="xs">
+                  <h2
+                    id="lnsDimensionContainerTitle"
+                    className="lnsDimensionContainer__headerTitle"
+                  >
+                    <strong>
+                      {i18n.translate('xpack.lens.configure.configurePanelTitle', {
+                        defaultMessage: '{groupLabel} configuration',
+                        values: {
+                          groupLabel,
+                        },
+                      })}
+                    </strong>
+                  </h2>
+                </EuiTitle>
+              </EuiFlexItem>
+            </EuiFlexGroup>
           </EuiFlyoutHeader>
           <EuiFlexItem className="eui-yScrollWithShadows" grow={1}>
             {panel}

diff --git a/...k/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx b/...k/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx
@@ -58,7 +58,7 @@ describe('LayerPanel', () => {
       onRemoveLayer: jest.fn(),
       dispatch: jest.fn(),
       core: coreMock.createStart(),
-      dataTestSubj: 'lns_layerPanel-0',
+      index: 0,
     };
   }
 

diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx
@@ -29,6 +29,12 @@ import { DimensionContainer } from './dimension_container';
 import { ColorIndicator } from './color_indicator';
 import { PaletteIndicator } from './palette_indicator';
 
+const triggerLinkA11yText = (label: string) =>
+  i18n.translate('xpack.lens.configure.editConfig', {
+    defaultMessage: 'Click to edit configuration for {label} or drag to move',
+    values: { label },
+  });
+
 const initialActiveDimensionState = {
   isNew: false,
 };
@@ -58,7 +64,7 @@ function isSameConfiguration(config1: unknown, config2: unknown) {
 export function LayerPanel(
   props: Exclude<ConfigPanelWrapperProps, 'state' | 'setState'> & {
     layerId: string;
-    dataTestSubj: string;
+    index: number;
     isOnlyLayer: boolean;
     updateVisualization: StateSetter<unknown>;
     updateDatasource: (datasourceId: string, newState: unknown) => void;
@@ -75,7 +81,7 @@ export function LayerPanel(
     initialActiveDimensionState
   );
 
-  const { framePublicAPI, layerId, isOnlyLayer, onRemoveLayer, dataTestSubj } = props;
+  const { framePublicAPI, layerId, isOnlyLayer, onRemoveLayer, index } = props;
   const datasourcePublicAPI = framePublicAPI.datasourceLayers[layerId];
 
   useEffect(() => {
@@ -125,7 +131,11 @@ export function LayerPanel(
   const columnLabelMap = layerDatasource.uniqueLabels(layerDatasourceConfigProps.state);
   return (
     <ChildDragDropProvider {...dragDropContext}>
-      <EuiPanel data-test-subj={dataTestSubj} className="lnsLayerPanel" paddingSize="s">
+      <EuiPanel
+        data-test-subj={`lns-layerPanel-${index}`}
+        className="lnsLayerPanel"
+        paddingSize="s"
+      >
         <EuiFlexGroup gutterSize="s" alignItems="flexStart" responsive={false}>
           <EuiFlexItem grow={false} className="lnsLayerPanel__settingsFlexItem">
             <LayerSettings
@@ -180,14 +190,10 @@ export function LayerPanel(
 
         <EuiSpacer size="m" />
 
-        {groups.map((group, index) => {
+        {groups.map((group, groupIndex) => {
           const newId = generateId();
           const isMissing = !isEmptyLayer && group.required && group.accessors.length === 0;
 
-          const triggerLinkA11yText = i18n.translate('xpack.lens.configure.editConfig', {
-            defaultMessage: 'Click to edit configuration or drag to move',
-          });
-
           return (
             <EuiFormRow
               className={
@@ -198,7 +204,7 @@ export function LayerPanel(
               fullWidth
               label={<div className="lnsLayerPanel__groupLabel">{group.groupLabel}</div>}
               labelType="legend"
-              key={index}
+              key={groupIndex}
               isInvalid={isMissing}
               error={
                 isMissing ? (
@@ -327,8 +333,8 @@ export function LayerPanel(
                                 });
                               }
                             }}
-                            aria-label={triggerLinkA11yText}
-                            title={triggerLinkA11yText}
+                            aria-label={triggerLinkA11yText(columnLabelMap[accessor])}
+                            title={triggerLinkA11yText(columnLabelMap[accessor])}
                           >
                             <ColorIndicator accessorConfig={accessorConfig}>
                               <NativeRenderer
@@ -351,11 +357,13 @@ export function LayerPanel(
                             aria-label={i18n.translate(
                               'xpack.lens.indexPattern.removeColumnLabel',
                               {
-                                defaultMessage: 'Remove configuration',
+                                defaultMessage: 'Remove configuration from "{groupLabel}"',
+                                values: { groupLabel: group.groupLabel },
                               }
                             )}
                             title={i18n.translate('xpack.lens.indexPattern.removeColumnLabel', {
-                              defaultMessage: 'Remove configuration',
+                              defaultMessage: 'Remove configuration from "{groupLabel}"',
+                              values: { groupLabel: group.groupLabel },
                             })}
                             onClick={() => {
                               trackUiEvent('indexpattern_dimension_removed');
@@ -435,6 +443,13 @@ export function LayerPanel(
                           contentProps={{
                             className: 'lnsLayerPanel__triggerTextContent',
                           }}
+                          aria-label={i18n.translate(
+                            'xpack.lens.indexPattern.removeColumnAriaLabel',
+                            {
+                              defaultMessage: 'Drop a field or click to add to {groupLabel}',
+                              values: { groupLabel: group.groupLabel },
+                            }
+                          )}
                           data-test-subj="lns-empty-dimension"
                           onClick={() => {
                             if (activeId) {
@@ -535,6 +550,17 @@ export function LayerPanel(
               iconType="trash"
               color="danger"
               data-test-subj="lnsLayerRemove"
+              aria-label={
+                isOnlyLayer
+                  ? i18n.translate('xpack.lens.resetLayerAriaLabel', {
+                      defaultMessage: 'Reset layer {index}',
+                      values: { index: index + 1 },
+                    })
+                  : i18n.translate('xpack.lens.deleteLayerAriaLabel', {
+                      defaultMessage: `Delete layer {index}`,
+                      values: { index: index + 1 },
+                    })
+              }
               onClick={() => {
                 // If we don't blur the remove / clear button, it remains focused
                 // which is a strange UX in this case. e.target.blur doesn't work
@@ -554,7 +580,7 @@ export function LayerPanel(
                     defaultMessage: 'Reset layer',
                   })
                 : i18n.translate('xpack.lens.deleteLayer', {
-                    defaultMessage: 'Delete layer',
+                    defaultMessage: `Delete layer`,
                   })}
             </EuiButtonEmpty>
           </EuiFlexItem>

diff --git a/...lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx b/...lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx
@@ -124,7 +124,9 @@ export function WorkspacePanelWrapper({
         <EuiScreenReaderOnly>
           <h1 id="lns_ChartTitle" data-test-subj="lns_ChartTitle">
             {title ||
-              i18n.translate('xpack.lens.chartTitle.unsaved', { defaultMessage: 'Unsaved' })}
+              i18n.translate('xpack.lens.chartTitle.unsaved', {
+                defaultMessage: 'Unsaved visualization',
+              })}
           </h1>
         </EuiScreenReaderOnly>
         <EuiPageContentBody className="lnsWorkspacePanelWrapper__pageContentBody">

diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/field_item.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/field_item.tsx
@@ -181,49 +181,56 @@ export const InnerFieldItem = function InnerFieldItem(props: FieldItemProps) {
     />
   );
   return (
-    <EuiPopover
-      ownFocus
-      className="lnsFieldItem__popoverAnchor"
-      display="block"
-      data-test-subj="lnsFieldListPanelField"
-      container={document.querySelector<HTMLElement>('.application') || undefined}
-      button={
-        <DragDrop
-          label={field.displayName}
-          value={value}
-          data-test-subj={`lnsFieldListPanelField-${field.name}`}
-          draggable
-        >
-          <FieldButton
-            className={`lnsFieldItem lnsFieldItem--${field.type} lnsFieldItem--${
-              exists ? 'exists' : 'missing'
-            }`}
-            isActive={infoIsOpen}
-            onClick={togglePopover}
-            aria-label={i18n.translate('xpack.lens.indexPattern.fieldStatsButtonAriaLabel', {
-              defaultMessage: '{fieldName}: {fieldType}. Hit enter for a field preview.',
-              values: {
-                fieldName: field.displayName,
-                fieldType: field.type,
-              },
-            })}
-            fieldIcon={lensFieldIcon}
-            fieldName={
-              <EuiHighlight search={wrapOnDot(highlight)}>
-                {wrapOnDot(field.displayName)}
-              </EuiHighlight>
-            }
-            fieldInfoIcon={lensInfoIcon}
-          />
-        </DragDrop>
-      }
-      isOpen={infoIsOpen}
-      closePopover={() => setOpen(false)}
-      anchorPosition="rightUp"
-      panelClassName="lnsFieldItem__fieldPanel"
-    >
-      <FieldItemPopoverContents {...state} {...props} />
-    </EuiPopover>
+    <li>
+      <EuiPopover
+        ownFocus
+        className="lnsFieldItem__popoverAnchor"
+        display="block"
+        data-test-subj="lnsFieldListPanelField"
+        container={document.querySelector<HTMLElement>('.application') || undefined}
+        button={
+          <DragDrop
+            label={field.displayName}
+            value={value}
+            data-test-subj={`lnsFieldListPanelField-${field.name}`}
+            draggable
+          >
+            <FieldButton
+              className={`lnsFieldItem lnsFieldItem--${field.type} lnsFieldItem--${
+                exists ? 'exists' : 'missing'
+              }`}
+              isActive={infoIsOpen}
+              onClick={togglePopover}
+              buttonProps={{
+                ['aria-label']: i18n.translate(
+                  'xpack.lens.indexPattern.fieldStatsButtonAriaLabel',
+                  {
+                    defaultMessage: '{fieldName}: {fieldType}. Hit enter for a field preview.',
+                    values: {
+                      fieldName: field.displayName,
+                      fieldType: field.type,
+                    },
+                  }
+                ),
+              }}
+              fieldIcon={lensFieldIcon}
+              fieldName={
+                <EuiHighlight search={wrapOnDot(highlight)}>
+                  {wrapOnDot(field.displayName)}
+                </EuiHighlight>
+              }
+              fieldInfoIcon={lensInfoIcon}
+            />
+          </DragDrop>
+        }
+        isOpen={infoIsOpen}
+        closePopover={() => setOpen(false)}
+        anchorPosition="rightUp"
+        panelClassName="lnsFieldItem__fieldPanel"
+      >
+        <FieldItemPopoverContents {...state} {...props} />
+      </EuiPopover>
+    </li>
   );
 };