From cec75f02246527e9df17139a22de0067926dbc46 Mon Sep 17 00:00:00 2001 From: Michael Marcialis Date: Fri, 14 May 2021 12:57:45 -0400 Subject: [PATCH 1/8] update label and fix alignment --- .../datatable_visualization/components/dimension_editor.scss | 3 +++ .../datatable_visualization/components/dimension_editor.tsx | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.scss diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.scss b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.scss new file mode 100644 index 0000000000000..d7664b9d2da16 --- /dev/null +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.scss @@ -0,0 +1,3 @@ +.lnsDynamicColoringRow { + align-items: center; +} \ No newline at end of file diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx index fc2d0bcc90511..09a72953e5cc0 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/dimension_editor.tsx @@ -26,6 +26,7 @@ import { PalettePanelContainer } from './palette_panel_container'; import { findMinMaxByColumnId } from './shared_utils'; import { applyPaletteParams } from '../../shared_components/coloring/utils'; import { defaultParams, FIXED_PROGRESSION } from '../../shared_components/coloring/constants'; +import './dimension_editor.scss'; const idPrefix = htmlIdGenerator()(); @@ -242,10 +243,11 @@ export function TableDimensionEditor( {hasDynamicColoring && ( From 8b917874cc1fa73b50448a992eb2576d6210efdc Mon Sep 17 00:00:00 2001 From: Michael Marcialis Date: Fri, 14 May 2021 13:09:26 -0400 Subject: [PATCH 2/8] change tooltip comp and remove extraneous span --- .../components/palette_configuration.tsx | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx index 434e849730feb..0be42cd113fe4 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx @@ -12,8 +12,7 @@ import { htmlIdGenerator, EuiButtonGroup, EuiSuperSelect, - EuiToolTip, - EuiIcon, + EuiIconTip, EuiButtonEmpty, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; @@ -184,22 +183,22 @@ export function CustomizablePalette({ - - {i18n.translate('xpack.lens.table.dynamicColoring.rangeType.label', { - defaultMessage: 'Value type', - })}{' '} - - - + <> + {i18n.translate('xpack.lens.table.dynamicColoring.rangeType.label', { + defaultMessage: 'Value type', + })}{' '} + + } display="rowCompressed" > From b3349a86dcac92efaeb316f5b568f1da1bfc9a97 Mon Sep 17 00:00:00 2001 From: Michael Marcialis Date: Fri, 14 May 2021 13:23:53 -0400 Subject: [PATCH 3/8] change EuiButtonEmpty to EuiLink; rm styles --- .../components/palette_configuration.scss | 4 - .../components/palette_configuration.tsx | 81 +++++++++++-------- 2 files changed, 46 insertions(+), 39 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.scss b/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.scss index e81e679b17dc3..c6b14c5c5f9a3 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.scss +++ b/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.scss @@ -4,8 +4,4 @@ .lnsPalettePanel__section { padding: $euiSizeS; -} - -.lnsPalettePanel__reverseButton { - margin-top: -4px; } \ No newline at end of file diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx index 0be42cd113fe4..500a80a13a7ec 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx @@ -11,9 +11,13 @@ import { EuiFormRow, htmlIdGenerator, EuiButtonGroup, + EuiFlexGroup, + EuiFlexItem, EuiSuperSelect, + EuiIcon, EuiIconTip, - EuiButtonEmpty, + EuiLink, + EuiText, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { PalettePicker } from './palette_picker'; @@ -277,40 +281,47 @@ export function CustomizablePalette({ defaultMessage: 'Color stops', })} labelAppend={ - { - const params: CustomPaletteParams = { reverse: !activePalette.params?.reverse }; - if (isCurrentPaletteCustom) { - params.colorStops = reversePalette(colorStopsToShow); - params.stops = getPaletteStops( - palettes, - { - ...(activePalette?.params || {}), - colorStops: params.colorStops, - }, - { dataBounds } - ); - } else { - params.stops = reversePalette( - activePalette?.params?.stops || - getPaletteStops( - palettes, - { ...activePalette.params, ...params }, - { prevPalette: activePalette.name, dataBounds } - ) - ); - } - setPalette(mergePaletteParams(activePalette, params)); - }} - > - {i18n.translate('xpack.lens.table.dynamicColoring.reverse.label', { - defaultMessage: 'Reverse colors', - })} - + + { + const params: CustomPaletteParams = { reverse: !activePalette.params?.reverse }; + if (isCurrentPaletteCustom) { + params.colorStops = reversePalette(colorStopsToShow); + params.stops = getPaletteStops( + palettes, + { + ...(activePalette?.params || {}), + colorStops: params.colorStops, + }, + { dataBounds } + ); + } else { + params.stops = reversePalette( + activePalette?.params?.stops || + getPaletteStops( + palettes, + { ...activePalette.params, ...params }, + { prevPalette: activePalette.name, dataBounds } + ) + ); + } + setPalette(mergePaletteParams(activePalette, params)); + }} + > + + + + + + {i18n.translate('xpack.lens.table.dynamicColoring.reverse.label', { + defaultMessage: 'Reverse colors', + })} + + + + } > Date: Fri, 14 May 2021 13:31:26 -0400 Subject: [PATCH 4/8] remove unneeded EuiFlexGroup; add flush and spacer --- .../coloring/color_stops.tsx | 69 ++++++++++--------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx index 8372d8dc9395d..62fbbeaacf0ca 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx @@ -15,6 +15,7 @@ import { EuiFlexItem, EuiFlexGroup, EuiButtonEmpty, + EuiSpacer, } from '@elastic/eui'; import { DEFAULT_COLOR } from './constants'; import { getDataMinMax, getStepValue, isValidColor } from './utils'; @@ -192,40 +193,40 @@ export const CustomStops = ({ })} - - - { - const newColorStops = [...localColorStops]; - const length = newColorStops.length; - const { max } = getDataMinMax(rangeType, dataBounds); - const step = getStepValue( - colorStops, - newColorStops.map(({ color, stop }) => ({ color, stop: Number(stop) })), - max - ); - const prevColor = localColorStops[length - 1].color || DEFAULT_COLOR; - const newStop = step + Number(localColorStops[length - 1].stop); - newColorStops.push({ - color: prevColor, - stop: String(newStop), - }); - setLocalColorStops(newColorStops); - }} - > - {i18n.translate('xpack.lens.dynamicColoring.customPalette.addColorStop', { - defaultMessage: 'Add color stop', - })} - - - + + + + { + const newColorStops = [...localColorStops]; + const length = newColorStops.length; + const { max } = getDataMinMax(rangeType, dataBounds); + const step = getStepValue( + colorStops, + newColorStops.map(({ color, stop }) => ({ color, stop: Number(stop) })), + max + ); + const prevColor = localColorStops[length - 1].color || DEFAULT_COLOR; + const newStop = step + Number(localColorStops[length - 1].stop); + newColorStops.push({ + color: prevColor, + stop: String(newStop), + }); + setLocalColorStops(newColorStops); + }} + > + {i18n.translate('xpack.lens.dynamicColoring.customPalette.addColorStop', { + defaultMessage: 'Add color stop', + })} + ); }; From 68f54431d68ce295ff3ed3312926391e4bc61e1b Mon Sep 17 00:00:00 2001 From: Michael Marcialis Date: Fri, 14 May 2021 13:34:28 -0400 Subject: [PATCH 5/8] center color stops content --- .../lens/public/shared_components/coloring/color_stops.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx index 62fbbeaacf0ca..8c447cbd4b89c 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx @@ -105,7 +105,7 @@ export const CustomStops = ({ } }} > - + + @@ -152,6 +153,7 @@ export const CustomStops = ({ onBlur={() => setPopoverInFocus(false)} /> + Date: Fri, 14 May 2021 14:57:15 -0400 Subject: [PATCH 6/8] change nested EuiFormRow to EuiFlexItem; see TODOs --- .../components/palette_configuration.tsx | 1 + .../coloring/color_stops.tsx | 240 +++++++++--------- 2 files changed, 123 insertions(+), 118 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx index 500a80a13a7ec..bf0b039b83a20 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx @@ -277,6 +277,7 @@ export function CustomizablePalette({ /> - - - {localColorStops.map(({ color, stop }, index) => { - const prevStopValue = Number(localColorStops[index - 1]?.stop ?? -Infinity); - const nextStopValue = Number(localColorStops[index + 1]?.stop ?? Infinity); - const errorMessages = []; - // do not show color error messages if number field is already in error - if (!isValidColor(color) && errorMessages.length === 0) { - errorMessages.push( - i18n.translate('xpack.lens.dynamicColoring.customPalette.hexWarningLabel', { - defaultMessage: 'Color must provide a valid hex value', - }) - ); - } - return ( - ) => { - // sort the stops when the focus leaves the row container - const shouldSort = Number(stop) > nextStopValue || prevStopValue > Number(stop); - const isFocusStillInContent = - (e.currentTarget as Node)?.contains(e.relatedTarget as Node) || popoverInFocus; - if (shouldSort && !isFocusStillInContent) { - setLocalColorStops( - [...localColorStops].sort( - ({ stop: stopA }, { stop: stopB }) => Number(stopA) - Number(stopB) - ) - ); - } - }} - > - - - { - const newStopString = target.value.trim(); - const newColorStops = [...localColorStops]; - newColorStops[index] = { - color, - stop: newStopString, - }; - setLocalColorStops(newColorStops); - }} - append={rangeType === 'percent' ? '%' : undefined} + + {localColorStops.map(({ color, stop }, index) => { + const prevStopValue = Number(localColorStops[index - 1]?.stop ?? -Infinity); + const nextStopValue = Number(localColorStops[index + 1]?.stop ?? Infinity); + + // TODO: Commented out the below error message code because 1) the individual color stops no longer use an `EuiFormRow` element and 2) I don't think such error message are needed, as the `EuiColorPicker` dropdown already has the hex field displaying such an error in the case of an invalid hex value. Instead of having a redundant error message, can we instead change it so that if an invalid hex value remains on blur of the `EuiColorPicker`, we simply revert to last known good hex value? + // const errorMessages = []; + // // do not show color error messages if number field is already in error + // if (!isValidColor(color) && errorMessages.length === 0) { + // errorMessages.push( + // i18n.translate('xpack.lens.dynamicColoring.customPalette.hexWarningLabel', { + // defaultMessage: 'Color must provide a valid hex value', + // }) + // ); + // } + + return ( + ) => { + // sort the stops when the focus leaves the row container + const shouldSort = Number(stop) > nextStopValue || prevStopValue > Number(stop); + const isFocusStillInContent = + (e.currentTarget as Node)?.contains(e.relatedTarget as Node) || popoverInFocus; + if (shouldSort && !isFocusStillInContent) { + setLocalColorStops( + [...localColorStops].sort( + ({ stop: stopA }, { stop: stopB }) => Number(stopA) - Number(stopB) + ) + ); + } + }} + > + + + { + const newStopString = target.value.trim(); + const newColorStops = [...localColorStops]; + newColorStops[index] = { + color, + stop: newStopString, + }; + setLocalColorStops(newColorStops); + }} + append={rangeType === 'percent' ? '%' : undefined} + aria-label={i18n.translate( + 'xpack.lens.dynamicColoring.customPalette.stopAriaLabel', + { + defaultMessage: 'Stop {index}', + values: { + index: index + 1, + }, + } + )} + /> + + + + { + const newColorStops = [...localColorStops]; + newColorStops[index] = { color: newColor, stop }; + setLocalColorStops(newColorStops); + }} + secondaryInputDisplay="top" + color={color} + isInvalid={!isValidColor(color)} + showAlpha + compressed + onFocus={() => setPopoverInFocus(true)} + onBlur={() => setPopoverInFocus(false)} + /> + + + + + - - - - { - const newColorStops = [...localColorStops]; - newColorStops[index] = { color: newColor, stop }; - setLocalColorStops(newColorStops); - }} - secondaryInputDisplay="top" - color={color} - isInvalid={!isValidColor(color)} - showAlpha - compressed - onFocus={() => setPopoverInFocus(true)} - onBlur={() => setPopoverInFocus(false)} - /> - - - - - { - const newColorStops = localColorStops.filter((_, i) => i !== index); - setLocalColorStops(newColorStops); - }} - data-test-subj={`${dataTestPrefix}_dynamicColoring_removeStop_${index}`} - isDisabled={!shouldEnableDelete} - /> - - - - - ); - })} - + onClick={() => { + const newColorStops = localColorStops.filter((_, i) => i !== index); + setLocalColorStops(newColorStops); + }} + data-test-subj={`${dataTestPrefix}_dynamicColoring_removeStop_${index}`} + isDisabled={!shouldEnableDelete} + /> + + + + + ); + })} From 8a563dc6834e48eac7372a6ca8b6c9a61651e7d0 Mon Sep 17 00:00:00 2001 From: Michael Marcialis Date: Fri, 14 May 2021 15:00:50 -0400 Subject: [PATCH 7/8] Make EuiSuperSelect compressed --- .../datatable_visualization/components/palette_configuration.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx index bf0b039b83a20..aacf0cd8deeb6 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/palette_configuration.tsx @@ -133,6 +133,7 @@ export function CustomizablePalette({ > Date: Fri, 14 May 2021 15:41:30 -0400 Subject: [PATCH 8/8] clean up --- .../components/palette_panel_container.tsx | 2 +- .../lens/public/shared_components/coloring/color_stops.tsx | 2 +- .../public/shared_components/coloring/palette_configuration.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/palette_panel_container.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/palette_panel_container.tsx index c3c094999e656..1371fbe73ef84 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/palette_panel_container.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/palette_panel_container.tsx @@ -86,7 +86,7 @@ export function PalettePanelContainer({ > {i18n.translate('xpack.lens.table.palettePanelTitle', { - defaultMessage: 'Color palette settings', + defaultMessage: 'Edit color', })} diff --git a/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx b/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx index 055a1714f681f..382b060020e2f 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx +++ b/x-pack/plugins/lens/public/shared_components/coloring/color_stops.tsx @@ -92,7 +92,7 @@ export const CustomStops = ({ const prevStopValue = Number(localColorStops[index - 1]?.stop ?? -Infinity); const nextStopValue = Number(localColorStops[index + 1]?.stop ?? Infinity); - // TODO: Commented out the below error message code because 1) the individual color stops no longer use an `EuiFormRow` element and 2) I don't think such error message are needed, as the `EuiColorPicker` dropdown already has the hex field displaying such an error in the case of an invalid hex value. Instead of having a redundant error message, can we instead change it so that if an invalid hex value remains on blur of the `EuiColorPicker`, we simply revert to last known good hex value? + // TODO: Commented out the below error message code because 1) the individual color stops no longer use an `EuiFormRow` component and 2) I don't think such error message are needed, as the `EuiColorPicker` dropdown already has the hex field displaying such an error in the case of an invalid hex value. Instead of having a redundant error message, can we instead change it so that if an invalid hex value remains on blur of the `EuiColorPicker`, we simply revert to last known good hex value? // const errorMessages = []; // // do not show color error messages if number field is already in error // if (!isValidColor(color) && errorMessages.length === 0) { diff --git a/x-pack/plugins/lens/public/shared_components/coloring/palette_configuration.tsx b/x-pack/plugins/lens/public/shared_components/coloring/palette_configuration.tsx index 2433c8eb4ff23..12491a1797825 100644 --- a/x-pack/plugins/lens/public/shared_components/coloring/palette_configuration.tsx +++ b/x-pack/plugins/lens/public/shared_components/coloring/palette_configuration.tsx @@ -126,7 +126,7 @@ export function CustomizablePalette({ >