Skip to content

Commit

Permalink
[8.11] [lens] color mapping general ux fixes (#167997) (#168161)
Browse files Browse the repository at this point in the history 
# Backport

This will backport the following commits from `main` to `8.11`:
- [[lens] color mapping general ux fixes
(#167997)](#167997)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marco
Vettorello","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-10-05T21:11:21Z","message":"[lens]
color mapping general ux fixes (#167997)\n\n## Summary\r\n\r\nFixes the
following issues with the Color mapping MVP
(see\r\n[here](https://github.com/elastic/kibana/issues/167506)):\r\n-
refactored the color contrast check and the RGB text input to
avoid\r\npopover flickering\r\n- replaced categorical/sequential icons
for color scales\r\n- add dot to complete the tooltip sentence.\r\n- fix
#167880 by adding a\r\ntooltip
to the warning
sign","sha":"6f62f7b5a65be6d99e9b3a0092304966b7a7bbf4","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Team:Visualizations","release_note:skip","Feature:Lens","backport:prev-minor","v8.11.0","v8.12.0"],"number":167997,"url":"https://github.com/elastic/kibana/pull/167997","mergeCommit":{"message":"[lens]
color mapping general ux fixes (#167997)\n\n## Summary\r\n\r\nFixes the
following issues with the Color mapping MVP
(see\r\n[here](https://github.com/elastic/kibana/issues/167506)):\r\n-
refactored the color contrast check and the RGB text input to
avoid\r\npopover flickering\r\n- replaced categorical/sequential icons
for color scales\r\n- add dot to complete the tooltip sentence.\r\n- fix
#167880 by adding a\r\ntooltip
to the warning
sign","sha":"6f62f7b5a65be6d99e9b3a0092304966b7a7bbf4"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/167997","number":167997,"mergeCommit":{"message":"[lens]
color mapping general ux fixes (#167997)\n\n## Summary\r\n\r\nFixes the
following issues with the Color mapping MVP
(see\r\n[here](https://github.com/elastic/kibana/issues/167506)):\r\n-
refactored the color contrast check and the RGB text input to
avoid\r\npopover flickering\r\n- replaced categorical/sequential icons
for color scales\r\n- add dot to complete the tooltip sentence.\r\n- fix
#167880 by adding a\r\ntooltip
to the warning
sign","sha":"6f62f7b5a65be6d99e9b3a0092304966b7a7bbf4"}}]}] BACKPORT-->

Co-authored-by: Marco Vettorello <[email protected]>
  • Loading branch information
@kibanamachine @markov00
kibanamachine and markov00 authored Oct 5, 2023
1 parent 36a023c commit 1873987
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 84 deletions.
17 changes: 14 additions & 3 deletions packages/kbn-coloring/src/shared_components/color_mapping/components/assignment/match.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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'
? []
Expand All @@ -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,
},
]
Expand All @@ -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,
};
});
Expand Down
2 changes: 1 addition & 1 deletion ...n-coloring/src/shared_components/color_mapping/components/color_picker/palette_colors.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
137 changes: 98 additions & 39 deletions ...s/kbn-coloring/src/shared_components/color_mapping/components/color_picker/rgb_picker.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand All @@ -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>
Expand All @@ -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>
Expand Down
6 changes: 2 additions & 4 deletions ...ring/src/shared_components/color_mapping/components/palette_selector/palette_selector.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down
17 changes: 0 additions & 17 deletions ...ing/src/shared_components/color_mapping/components/palette_selector/scale_categorical.tsx
View file

This file was deleted.

20 changes: 0 additions & 20 deletions ...ring/src/shared_components/color_mapping/components/palette_selector/scale_sequential.tsx
View file

This file was deleted.

0 comments on commit 1873987

Please sign in to comment.