[lens] color mapping general ux fixes (elastic#167997)

## Summary Fixes the following issues with the Color mapping MVP (see [here](elastic#167506)): - refactored the color contrast check and the RGB text input to avoid popover flickering - replaced categorical/sequential icons for color scales - add dot to complete the tooltip sentence. - fix elastic#167880 by adding a tooltip to the warning sign
dej611 · Oct 17, 2023 · 7b89b11 · 7b89b11
1 parent 5a27b7d
commit 7b89b11
Show file tree

Hide file tree

Showing 6 changed files with 115 additions and 84 deletions.
diff --git a/packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/match.tsx b/packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/match.tsx
@@ -7,7 +7,7 @@
  */
 
 import React from 'react';
-import { EuiComboBox, EuiFlexItem, EuiIcon } from '@elastic/eui';
+import { EuiComboBox, EuiFlexItem, EuiIcon, EuiToolTip } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { MULTI_FIELD_KEY_SEPARATOR } from '@kbn/data-plugin/common';
 import { euiThemeVars } from '@kbn/ui-theme';
@@ -26,6 +26,13 @@ export const Match: React.FC<{
   specialTokens: Map<unknown, string>;
   assignmentValuesCounter: Map<string | string[], number>;
 }> = ({ index, rule, updateValue, editable, options, specialTokens, assignmentValuesCounter }) => {
+  const duplicateWarning = i18n.translate(
+    'coloring.colorMapping.assignments.duplicateCategoryWarning',
+    {
+      defaultMessage:
+        'This category has already been assigned a different color. Only the first matching assignment will be used.',
+    }
+  );
   const selectedOptions =
     rule.type === 'auto'
       ? []
@@ -36,7 +43,9 @@ export const Match: React.FC<{
             value: rule.values,
             append:
               (assignmentValuesCounter.get(rule.values) ?? 0) > 1 ? (
-                <EuiIcon size="s" type="warning" color={euiThemeVars.euiColorWarningText} />
+                <EuiToolTip position="bottom" content={duplicateWarning}>
+                  <EuiIcon size="s" type="warning" color={euiThemeVars.euiColorWarningText} />
+                </EuiToolTip>
               ) : undefined,
           },
         ]
@@ -47,7 +56,9 @@ export const Match: React.FC<{
             value,
             append:
               (assignmentValuesCounter.get(value) ?? 0) > 1 ? (
-                <EuiIcon size="s" type="warning" color={euiThemeVars.euiColorWarningText} />
+                <EuiToolTip position="bottom" content={duplicateWarning}>
+                  <EuiIcon size="s" type="warning" color={euiThemeVars.euiColorWarningText} />
+                </EuiToolTip>
               ) : undefined,
           };
         });

diff --git a/...n-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx b/...n-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx
@@ -94,7 +94,7 @@ export function PaletteColors({
               position="bottom"
               content={i18n.translate('coloring.colorMapping.colorPicker.themeAwareColorsTooltip', {
                 defaultMessage:
-                  'The provided neutral colors are theme-aware and will change appropriately when switching between light and dark themes',
+                  'The provided neutral colors are theme-aware and will change appropriately when switching between light and dark themes.',
               })}
             >
               <EuiIcon tabIndex={0} type="questionInCircle" />

diff --git a/...s/kbn-coloring/src/shared_components/color_mapping/components/color_picker/rgb_picker.tsx b/...s/kbn-coloring/src/shared_components/color_mapping/components/color_picker/rgb_picker.tsx
@@ -6,12 +6,21 @@
  * Side Public License, v 1.
  */
 
-import { EuiColorPicker, EuiFieldText, EuiFlexGroup, EuiFlexItem, EuiFormRow } from '@elastic/eui';
+import {
+  EuiColorPicker,
+  EuiFieldText,
+  EuiFlexGroup,
+  EuiFlexItem,
+  EuiFormRow,
+  EuiIcon,
+  EuiToolTip,
+} from '@elastic/eui';
 import React, { useState } from 'react';
 import useDebounce from 'react-use/lib/useDebounce';
 import chromajs from 'chroma-js';
 import { css } from '@emotion/react';
 import { euiThemeVars } from '@kbn/ui-theme';
+import { i18n } from '@kbn/i18n';
 import { ColorMapping } from '../../config';
 
 import { hasEnoughContrast } from '../../color/color_math';
@@ -54,8 +63,8 @@ export function RGBPicker({
     darkContrast === false ? 'dark' : undefined,
   ].filter(Boolean);
 
-  const isColorTextValid = chromajs.valid(colorTextInput);
-  const colorHasContrast = lightContrast && darkContrast;
+  const isColorTextInvalid = !chromajs.valid(colorTextInput);
+  const colorHasLowContrast = !lightContrast || !darkContrast;
 
   // debounce setting the color from the rgb picker by 500ms
   useDebounce(
@@ -67,6 +76,16 @@ export function RGBPicker({
     500,
     [color, customColorMappingColor]
   );
+  const invalidColor = isColorTextInvalid
+    ? euiThemeVars.euiColorDanger
+    : colorHasLowContrast
+    ? euiThemeVars.euiColorWarning
+    : '';
+  const invalidColorText = isColorTextInvalid
+    ? euiThemeVars.euiColorDangerText
+    : colorHasLowContrast
+    ? euiThemeVars.euiColorWarningText
+    : '';
   return (
     <EuiFlexGroup direction="column" gutterSize="s" style={{ padding: 8 }}>
       <EuiFlexItem>
@@ -86,56 +105,96 @@ export function RGBPicker({
       <EuiFlexItem>
         <div
           css={
-            !colorHasContrast && isColorTextValid
+            isColorTextInvalid || colorHasLowContrast
               ? css`
-                  svg {
-                    fill: ${euiThemeVars.euiColorWarningText} !important;
-                  }
                   input {
                     background-image: linear-gradient(
                       to top,
-                      ${euiThemeVars.euiColorWarning},
-                      ${euiThemeVars.euiColorWarning} 2px,
+                      ${invalidColor},
+                      ${invalidColor} 2px,
                       transparent 2px,
                       transparent 100%
                     ) !important;
+                    background-size: 100%;
+                    padding-right: 32px;
                   }
                   .euiFormErrorText {
-                    color: ${euiThemeVars.euiColorWarningText} !important;
+                    color: ${invalidColorText} !important;
                   }
                 `
               : undefined
           }
         >
-          <EuiFormRow
-            isInvalid={!isColorTextValid || !colorHasContrast}
-            error={
-              !isColorTextValid
-                ? `Please input a valid color hex code`
-                : !colorHasContrast
-                ? `This color has a low contrast in ${errorMessage} mode${
-                    errorMessage.length > 1 ? 's' : ''
-                  }`
-                : undefined
-            }
-          >
-            <EuiFieldText
-              placeholder="Please enter an hex color code"
-              value={colorTextInput}
-              compressed
-              isInvalid={!isColorTextValid || !colorHasContrast}
-              onChange={(e) => {
-                const textColor = e.currentTarget.value;
-                setColorTextInput(textColor);
-                if (chromajs.valid(textColor)) {
-                  setCustomColorMappingColor({
-                    type: 'colorCode',
-                    colorCode: chromajs(textColor).hex(),
-                  });
-                }
-              }}
-              aria-label="hex color input"
-            />
+          <EuiFormRow>
+            <EuiFlexGroup
+              css={css`
+                position: relative;
+              `}
+            >
+              <EuiFlexItem>
+                <EuiFieldText
+                  placeholder="#FF00FF"
+                  value={colorTextInput}
+                  compressed
+                  onChange={(e) => {
+                    const textColor = e.currentTarget.value;
+                    setColorTextInput(textColor);
+                    if (chromajs.valid(textColor)) {
+                      setCustomColorMappingColor({
+                        type: 'colorCode',
+                        colorCode: chromajs(textColor).hex(),
+                      });
+                    }
+                  }}
+                  aria-label={i18n.translate(
+                    'coloring.colorMapping.colorPicker.hexColorinputAriaLabel',
+                    {
+                      defaultMessage: 'hex color input',
+                    }
+                  )}
+                />
+              </EuiFlexItem>
+              {(isColorTextInvalid || colorHasLowContrast) && (
+                <div
+                  css={css`
+                    position: absolute;
+                    right: 8px;
+                    top: 6px;
+                  `}
+                >
+                  <EuiToolTip
+                    position="bottom"
+                    content={
+                      isColorTextInvalid
+                        ? i18n.translate('coloring.colorMapping.colorPicker.invalidColorHex', {
+                            defaultMessage: 'Please use a valid color hex code',
+                          })
+                        : colorHasLowContrast
+                        ? i18n.translate('coloring.colorMapping.colorPicker.lowContrastColor', {
+                            defaultMessage: `This color has a low contrast in {themes} {errorModes, plural, one {mode} other {# modes}}`,
+                            values: {
+                              themes: errorMessage.join(','),
+                              errorModes: errorMessage.length,
+                            },
+                          })
+                        : undefined
+                    }
+                  >
+                    <EuiIcon
+                      tabIndex={0}
+                      type="warning"
+                      color={
+                        isColorTextInvalid
+                          ? euiThemeVars.euiColorDangerText
+                          : colorHasLowContrast
+                          ? euiThemeVars.euiColorWarningText
+                          : euiThemeVars.euiColorPrimary
+                      }
+                    />
+                  </EuiToolTip>
+                </div>
+              )}
+            </EuiFlexGroup>
           </EuiFormRow>
         </div>
       </EuiFlexItem>

diff --git a/...ring/src/shared_components/color_mapping/components/palette_selector/palette_selector.tsx b/...ring/src/shared_components/color_mapping/components/palette_selector/palette_selector.tsx
@@ -17,8 +17,6 @@ import {
   EuiFormRow,
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
-import { ScaleCategoricalIcon } from './scale_categorical';
-import { ScaleSequentialIcon } from './scale_sequential';
 
 import { RootState, updatePalette } from '../../state/color_mapping';
 import { ColorMapping } from '../../config';
@@ -228,14 +226,14 @@ export function PaletteSelector({
                   label: i18n.translate('coloring.colorMapping.paletteSelector.categoricalLabel', {
                     defaultMessage: `Categorical`,
                   }),
-                  iconType: ScaleCategoricalIcon,
+                  iconType: 'palette',
                 },
                 {
                   id: `gradient`,
                   label: i18n.translate('coloring.colorMapping.paletteSelector.sequentialLabel', {
                     defaultMessage: `Sequential`,
                   }),
-                  iconType: ScaleSequentialIcon,
+                  iconType: 'gradient',
                 },
               ]}
               isFullWidth

diff --git a/...ing/src/shared_components/color_mapping/components/palette_selector/scale_categorical.tsx b/...ing/src/shared_components/color_mapping/components/palette_selector/scale_categorical.tsx
diff --git a/...ring/src/shared_components/color_mapping/components/palette_selector/scale_sequential.tsx b/...ring/src/shared_components/color_mapping/components/palette_selector/scale_sequential.tsx