[Lens] accessibility screen reader issues (#84395)

* [Lens] accessibility screen reader issues * fix i18n * fix: no aria-label on divs * cr fixes Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
2020-12-03 13:28:41 +01:00 · 2020-12-03 13:28:41 +01:00 · 487eb2e4e4
parent 3cb26ebe8d
commit 487eb2e4e4
12 changed files with 184 additions and 101 deletions
--- 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}
--- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss
+++ b/x-pack/plugins/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;
+  }
+}
--- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx
+++ b/x-pack/plugins/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}
--- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx
+++ b/x-pack/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,
    };
  }

--- 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>
--- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.tsx
+++ b/x-pack/plugins/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">
--- 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>
  );
 };

--- a/x-pack/plugins/lens/public/indexpattern_datasource/field_list.tsx
+++ b/x-pack/plugins/lens/public/indexpattern_datasource/field_list.tsx
@ -125,19 +125,21 @@ export function FieldList({
      onScroll={throttle(lazyScroll, 100)}
    >
      <div className="lnsIndexPatternFieldList__accordionContainer">
-        {Object.entries(fieldGroups)
-          .filter(([, { showInAccordion }]) => !showInAccordion)
-          .flatMap(([, { fields }]) =>
-            fields.map((field) => (
-              <FieldItem
-                {...fieldProps}
-                exists={exists(field)}
-                field={field}
-                hideDetails={true}
-                key={field.name}
-              />
-            ))
-          )}
+        <ul>
+          {Object.entries(fieldGroups)
+            .filter(([, { showInAccordion }]) => !showInAccordion)
+            .flatMap(([, { fields }]) =>
+              fields.map((field) => (
+                <FieldItem
+                  {...fieldProps}
+                  exists={exists(field)}
+                  field={field}
+                  hideDetails={true}
+                  key={field.name}
+                />
+              ))
+            )}
+        </ul>
        <EuiSpacer size="s" />
        {Object.entries(fieldGroups)
          .filter(([, { showInAccordion }]) => showInAccordion)
--- a/x-pack/plugins/lens/public/indexpattern_datasource/fields_accordion.tsx
+++ b/x-pack/plugins/lens/public/indexpattern_datasource/fields_accordion.tsx
@ -113,9 +113,9 @@ export const InnerFieldsAccordion = function InnerFieldsAccordion({
      <EuiSpacer size="s" />
      {hasLoaded &&
        (!!fieldsCount ? (
-          <div className="lnsInnerIndexPatternDataPanel__fieldItems">
+          <ul className="lnsInnerIndexPatternDataPanel__fieldItems">
            {paginatedFields && paginatedFields.map(renderField)}
-          </div>
+          </ul>
        ) : (
          renderCallout
        ))}
--- a/x-pack/plugins/translations/translations/ja-JP.json
+++ b/x-pack/plugins/translations/translations/ja-JP.json
@ -10579,14 +10579,12 @@
    "xpack.lens.chartSwitch.dataLossDescription": "このチャートに切り替えると構成の一部が失われます",
    "xpack.lens.chartSwitch.dataLossLabel": "データ喪失",
    "xpack.lens.chartSwitch.noResults": "{term}の結果が見つかりませんでした。",
-    "xpack.lens.chartTitle.unsaved": "未保存",
    "xpack.lens.configPanel.chartType": "チャートタイプ",
    "xpack.lens.configPanel.color.tooltip.auto": "カスタム色を指定しない場合、Lensは自動的に色を選択します。",
    "xpack.lens.configPanel.color.tooltip.custom": "［自動］モードに戻すには、カスタム色をオフにしてください。",
    "xpack.lens.configPanel.color.tooltip.disabled": "レイヤーに「内訳条件」が含まれている場合は、個別の系列をカスタム色にできません。",
    "xpack.lens.configPanel.selectVisualization": "ビジュアライゼーションを選択してください",
    "xpack.lens.configure.configurePanelTitle": "{groupLabel}構成",
-    "xpack.lens.configure.editConfig": "クリックして構成を編集するか、ドラッグして移動",
    "xpack.lens.configure.emptyConfig": "フィールドを破棄、またはクリックして追加",
    "xpack.lens.configure.invalidConfigTooltip": "無効な構成です。",
    "xpack.lens.configure.invalidConfigTooltipClick": "詳細はクリックしてください。",
@ -10725,7 +10723,6 @@
    "xpack.lens.indexPattern.ranges.lessThanPrepend": "&lt;",
    "xpack.lens.indexPattern.ranges.lessThanTooltip": "より小さい",
    "xpack.lens.indexPattern.records": "記録",
-    "xpack.lens.indexPattern.removeColumnLabel": "構成を削除",
    "xpack.lens.indexpattern.suggestions.nestingChangeLabel": "各 {outerOperation} の {innerOperation}",
    "xpack.lens.indexpattern.suggestions.overallLabel": "全体の {operation}",
    "xpack.lens.indexpattern.suggestions.overTimeLabel": "一定時間",
--- a/x-pack/plugins/translations/translations/zh-CN.json
+++ b/x-pack/plugins/translations/translations/zh-CN.json
@ -10592,14 +10592,12 @@
    "xpack.lens.chartSwitch.dataLossDescription": "切换到此图表将会丢失部分配置",
    "xpack.lens.chartSwitch.dataLossLabel": "数据丢失",
    "xpack.lens.chartSwitch.noResults": "找不到 {term} 的结果。",
-    "xpack.lens.chartTitle.unsaved": "未保存",
    "xpack.lens.configPanel.chartType": "图表类型",
    "xpack.lens.configPanel.color.tooltip.auto": "Lens 自动为您选取颜色，除非您指定定制颜色。",
    "xpack.lens.configPanel.color.tooltip.custom": "清除定制颜色以返回到“自动”模式。",
    "xpack.lens.configPanel.color.tooltip.disabled": "当图层包括“细分依据”，各个系列无法定制颜色。",
    "xpack.lens.configPanel.selectVisualization": "选择可视化",
    "xpack.lens.configure.configurePanelTitle": "{groupLabel} 配置",
-    "xpack.lens.configure.editConfig": "单击以编辑配置或进行拖移",
    "xpack.lens.configure.emptyConfig": "放置字段或单击以添加",
    "xpack.lens.configure.invalidConfigTooltip": "配置无效。",
    "xpack.lens.configure.invalidConfigTooltipClick": "单击了解更多详情。",
@ -10738,7 +10736,6 @@
    "xpack.lens.indexPattern.ranges.lessThanPrepend": "&lt;",
    "xpack.lens.indexPattern.ranges.lessThanTooltip": "小于",
    "xpack.lens.indexPattern.records": "记录",
-    "xpack.lens.indexPattern.removeColumnLabel": "移除配置",
    "xpack.lens.indexpattern.suggestions.nestingChangeLabel": "每个 {outerOperation} 的 {innerOperation}",
    "xpack.lens.indexpattern.suggestions.overallLabel": "{operation} - 总体",
    "xpack.lens.indexpattern.suggestions.overTimeLabel": "时移",
--- a/x-pack/test/functional/page_objects/lens_page.ts
+++ b/x-pack/test/functional/page_objects/lens_page.ts
@ -204,7 +204,7 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont

    // closes the dimension editor flyout
    async closeDimensionEditor() {
-      await testSubjects.click('lns-indexPattern-dimensionContainerTitle');
+      await testSubjects.click('lns-indexPattern-dimensionContainerBack');
    },

    /**