[Lens] Fix a11y issues & small styling issues (#161073)

## Summary Fixes the following issues: 1. Suggestions are not reachable by keyboard navigation (it was actually broken when fixing the [accesibility axe warning](https://github.com/elastic/kibana/issues/154787), I ran it now and it doesn't show now) 46c9eb70-4df7-462b-a878-b7d190aaf8c9 2. Dimension removal button is not reachable by keyboard navigation before: 5011679b-b05a-48ca-a877-bbc940607b56 after: 9d056512-a622-4c1c-aa5b-242e2762e9ae 3. The padding for the reordering groups got a distance between the empty dimension button (btw I have more little fixes for drag and drop look but want to submit it in a separate PR to not ping here Discover team): <img width="421" alt="Screenshot 2023-07-03 at 17 42 41" src="a1d22267-07b6-4179-89f9-b4c7ec5da030"> <img width="459" alt="Screenshot 2023-07-03 at 17 39 11" src="a7b697ca-4ff3-4c18-bf96-a8710615a9df"> 4. The focus outline for dataview selector and layer chart selector got a nicer rectangular shape before: <img width="343" alt="Screenshot 2023-07-03 at 10 50 52" src="0ca305b8-cdbf-4c43-b73d-0058046ab919"> <img width="360" alt="Screenshot 2023-07-03 at 17 41 18" src="54e61646-5d4e-4512-b6e1-6d758e2b7498"> after: <img width="350" alt="Screenshot 2023-07-03 at 11 11 01" src="8ddd81dd-8072-4cf8-a98b-ae5eb9998add"> <img width="415" alt="Screenshot 2023-07-03 at 17 39 51" src="1c36b3bd-cb15-4cb8-858e-92fc19e3181f"> 5. The focus outline was corrected for dimension buttons. I also refactored code here to make margins, paddings and gaps the property of the containers and not the items themselves. I think this way it's more correct. before: <img width="418" alt="Screenshot 2023-07-03 at 17 40 54" src="b8ce4a22-dc69-44a7-8689-70928340ac24"> after: 712e2ac8-f81d-4896-b384-164fc2854766
2025-04-23 17:28:26 -04:00 · 2023-07-11 15:51:58 +02:00 · 2023-07-11 15:51:58 +02:00 · 7e7293a520
commit 7e7293a520
parent f6f5986376
10 changed files with 29 additions and 43 deletions
--- a/src/plugins/visualization_ui_components/public/components/dimension_buttons/dimension_button.tsx
+++ b/src/plugins/visualization_ui_components/public/components/dimension_buttons/dimension_button.tsx
@ -58,20 +58,14 @@ export function DimensionButton({
      css={css`
        ${useEuiFontSize('s')}
        border-radius: ${euiThemeVars.euiBorderRadius};
+        position: relative;
+        line-height: 1;
+        overflow: hidden;
        display: flex;
        align-items: center;
-        overflow: hidden;
+        gap: ${euiThemeVars.euiSizeS};
        min-height: ${euiThemeVars.euiSizeXL};
-        position: relative;
-
-        &:hover,
-        &:focus {
-          .lnsLayerPanel__dimensionRemove {
-            visibility: visible;
-            opacity: 1;
-            transition: opacity ${euiThemeVars.euiAnimSpeedFast} ease-in-out;
-          }
-        }
+        padding: ${euiThemeVars.euiSizeXS} ${euiThemeVars.euiSizeS};
      `}
    >
      <EuiFlexGroup direction="row" alignItems="center" gutterSize="none" responsive={false}>
@ -120,12 +114,17 @@ export function DimensionButton({
        })}
        onClick={() => onRemoveClick(accessorConfig.columnId)}
        css={css`
-          margin-right: ${euiThemeVars.euiSizeS};
-          visibility: hidden;
-          opacity: 0;
          color: ${euiThemeVars.euiTextSubduedColor};
+          transition: ${euiThemeVars.euiAnimSpeedFast} ease-in-out;
+          transition-property: color, opacity, background-color, transform;
+          opacity: 0;

-          &:hover {
+          .domDragDrop:hover &,
+          .domDragDrop:focus-within & {
+            opacity: 1;
+          }
+          &:hover,
+          &:focus {
            color: ${euiThemeVars.euiColorDangerText};
          }
        `}
--- a/src/plugins/visualization_ui_components/public/components/dimension_buttons/dimension_button_icon.tsx
+++ b/src/plugins/visualization_ui_components/public/components/dimension_buttons/dimension_button_icon.tsx
@ -9,21 +9,12 @@
 import React from 'react';
 import { EuiFlexGroup, EuiFlexItem, EuiIcon } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
-import { css } from '@emotion/react';
-import { euiThemeVars } from '@kbn/ui-theme';
 import type { AccessorConfig, Message } from './types';

-const baseIconProps = {
-  css: css`
-    margin-left: ${euiThemeVars.euiSizeS};
-  `,
-} as const;
-
 const getIconFromAccessorConfig = (accessorConfig: AccessorConfig) => (
  <>
    {accessorConfig.triggerIconType === 'color' && accessorConfig.color && (
      <EuiIcon
-        {...baseIconProps}
        color={accessorConfig.color}
        type="stopFilled"
        aria-label={i18n.translate(
@ -39,7 +30,6 @@ const getIconFromAccessorConfig = (accessorConfig: AccessorConfig) => (
    )}
    {accessorConfig.triggerIconType === 'disabled' && (
      <EuiIcon
-        {...baseIconProps}
        type="stopSlash"
        color="subdued"
        aria-label={i18n.translate(
@ -52,7 +42,6 @@ const getIconFromAccessorConfig = (accessorConfig: AccessorConfig) => (
    )}
    {accessorConfig.triggerIconType === 'invisible' && (
      <EuiIcon
-        {...baseIconProps}
        type="eyeClosed"
        color="subdued"
        aria-label={i18n.translate(
@ -65,7 +54,6 @@ const getIconFromAccessorConfig = (accessorConfig: AccessorConfig) => (
    )}
    {accessorConfig.triggerIconType === 'aggregate' && (
      <EuiIcon
-        {...baseIconProps}
        type="fold"
        color="subdued"
        aria-label={i18n.translate(
@ -79,7 +67,6 @@ const getIconFromAccessorConfig = (accessorConfig: AccessorConfig) => (
    )}
    {accessorConfig.triggerIconType === 'colorBy' && (
      <EuiIcon
-        {...baseIconProps}
        type="color"
        color="subdued"
        aria-label={i18n.translate(
@ -92,7 +79,6 @@ const getIconFromAccessorConfig = (accessorConfig: AccessorConfig) => (
    )}
    {accessorConfig.triggerIconType === 'custom' && accessorConfig.customIcon && (
      <EuiIcon
-        {...baseIconProps}
        type={accessorConfig.customIcon}
        color={accessorConfig.color}
        aria-label={i18n.translate(
@ -116,7 +102,10 @@ export function DimensionButtonIcon({
  children: React.ReactChild;
 }) {
  let indicatorIcon = null;
-  if (severity || accessorConfig.triggerIconType !== 'none') {
+  if (
+    severity ||
+    (accessorConfig.triggerIconType !== 'none' && accessorConfig.triggerIconType !== undefined)
+  ) {
    indicatorIcon = (
      <>
        {accessorConfig.triggerIconType !== 'none' && (
@ -124,7 +113,7 @@ export function DimensionButtonIcon({
        )}
        {severity && (
          <EuiFlexItem grow={false}>
-            <EuiIcon {...baseIconProps} type={severity === 'error' ? 'error' : 'alert'} />
+            <EuiIcon type={severity === 'error' ? 'error' : 'alert'} />
          </EuiFlexItem>
        )}
      </>
@ -132,7 +121,7 @@ export function DimensionButtonIcon({
  }

  return (
-    <EuiFlexGroup gutterSize="none" alignItems="center" responsive={false}>
+    <EuiFlexGroup gutterSize="s" alignItems="center" responsive={false}>
      {indicatorIcon}
      <EuiFlexItem>{children}</EuiFlexItem>
    </EuiFlexGroup>
--- a/src/plugins/visualization_ui_components/public/components/dimension_buttons/empty_button.tsx
+++ b/src/plugins/visualization_ui_components/public/components/dimension_buttons/empty_button.tsx
@ -33,6 +33,7 @@ export const EmptyDimensionButton = ({
        width: 100%;
        border-radius: ${euiThemeVars.euiBorderRadius} !important;
        border: ${euiThemeVars.euiBorderWidthThin} dashed ${euiThemeVars.euiBorderColor} !important;
+        padding: ${euiThemeVars.euiSizeXS} ${euiThemeVars.euiSizeS};
      `}
      color="text" // as far as I can tell all this currently adds is the correct active background color
      size="s"
@ -40,13 +41,9 @@ export const EmptyDimensionButton = ({
      contentProps={{
        css: css`
          justify-content: flex-start;
+          gap: ${euiThemeVars.euiSizeS};
          padding: 0 !important;
          color: ${euiThemeVars.euiTextSubduedColor};
-          gap: 0;
-
-          .euiIcon {
-            margin-left: ${euiThemeVars.euiSizeS};
-          }
        `,
      }}
      aria-label={ariaLabel}
--- a/src/plugins/visualization_ui_components/public/components/dimension_buttons/trigger.tsx
+++ b/src/plugins/visualization_ui_components/public/components/dimension_buttons/trigger.tsx
@ -48,7 +48,6 @@ export const DimensionTrigger = ({
      color={color}
      css={css`
        width: 100%;
-        padding: ${euiThemeVars.euiSizeXS} ${euiThemeVars.euiSizeS};
        word-break: break-word;
        font-weight: ${euiThemeVars.euiFontWeightRegular};
      `}
--- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss
+++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.scss
@ -60,8 +60,8 @@
 }

 .lnsLayerPanel__group {
-  margin: (-$euiSizeS) (-$euiSize);
-  padding: $euiSizeS $euiSize;
+  margin: (-$euiSizeXS) (-$euiSize);
+  padding: $euiSizeXS $euiSize;
 }

 .lnsLayerPanel__styleEditor {
--- 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
@ -648,6 +648,7 @@ export function LayerPanel(
                        background-color: ${euiThemeVars.euiColorLightShade} !important;
                        border-color: transparent !important;
                        box-shadow: none !important;
+                        padding: 0 ${euiThemeVars.euiSizeS};
                      `}
                    >
                      <DimensionTrigger
--- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.test.tsx
+++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.test.tsx
@ -34,7 +34,7 @@ import { setChangesApplied } from '../../state_management/lens_slice';
 const SELECTORS = {
  APPLY_CHANGES_BUTTON: 'button[data-test-subj="lnsApplyChanges__suggestions"]',
  SUGGESTIONS_PANEL: '[data-test-subj="lnsSuggestionsPanel"]',
-  SUGGESTION_TILE_BUTTON: 'div[data-test-subj="lnsSuggestion"]',
+  SUGGESTION_TILE_BUTTON: 'button[data-test-subj="lnsSuggestion"]',
 };

 jest.mock('./suggestion_helpers');
--- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx
+++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx
@ -185,7 +185,7 @@ const SuggestionPreview = ({
          onClick={onSelect}
          aria-current={!!selected}
          aria-label={preview.title}
-          element="div"
+          element="button"
          role="listitem"
        >
          {preview.expression || preview.error ? (
--- a/x-pack/plugins/lens/public/shared_components/dataview_picker/trigger.tsx
+++ b/x-pack/plugins/lens/public/shared_components/dataview_picker/trigger.tsx
@ -101,7 +101,7 @@ export function TriggerButton({
      fullWidth
      {...colorProp}
      {...rest}
-      textProps={{ style: { width: '100%' } }}
+      textProps={{ style: { width: '100%', lineHeight: '100%' } }}
    >
      <TriggerLabel label={label} extraIcons={extraIcons} />
    </ToolbarButton>
--- a/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx
+++ b/x-pack/plugins/lens/public/visualizations/xy/xy_config_panel/layer_header.tsx
@ -235,6 +235,7 @@ const DataLayerHeaderTrigger = function ({
      onClick={onClick}
      fullWidth
      size="s"
+      textProps={{ style: { lineHeight: '100%' } }}
    >
      <>
        <EuiIcon type={currentVisType.icon} />