Skip to content

Commit

Permalink
[Maps] Safely handle empty string and invalid strings from EuiColorPi…
Browse files Browse the repository at this point in the history
…cker (#62507)

* [Maps] Safely handle empty string and invalid strings from EuiColorPicker

* move RGBA_0000 to constants
  • Loading branch information
nreese authored Apr 3, 2020
1 parent 9ed69ce commit a5526c8
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export class ColorMapSelect extends Component {
<ColorStopsOrdinal
colorStops={this.state.customColorMap}
onChange={this._onCustomColorMapChange}
swatches={this.props.swatches}
/>
);
} else
Expand All @@ -108,6 +109,7 @@ export class ColorMapSelect extends Component {
field={this.props.styleProperty.getField()}
getValueSuggestions={this.props.styleProperty.getValueSuggestions}
onChange={this._onCustomColorMapChange}
swatches={this.props.swatches}
/>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,61 +8,8 @@ import _ from 'lodash';
import React from 'react';
import { removeRow, isColorInvalid } from './color_stops_utils';
import { i18n } from '@kbn/i18n';
import { EuiButtonIcon, EuiColorPicker, EuiFlexGroup, EuiFlexItem, EuiFormRow } from '@elastic/eui';

function getColorStopRow({ index, errors, stopInput, onColorChange, color, deleteButton, onAdd }) {
const colorPickerButtons = (
<div>
{deleteButton}
<EuiButtonIcon
iconType="plusInCircle"
color="primary"
aria-label="Add"
title="Add"
onClick={onAdd}
/>
</div>
);
return (
<EuiFormRow
key={index}
className="mapColorStop"
isInvalid={errors.length !== 0}
error={errors}
display="rowCompressed"
>
<EuiFlexGroup alignItems="center" gutterSize="xs">
<EuiFlexItem grow={false} className="mapStyleSettings__fixedBox">
{stopInput}
</EuiFlexItem>
<EuiFlexItem>
<EuiColorPicker
onChange={onColorChange}
color={color}
compressed
append={colorPickerButtons}
/>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFormRow>
);
}

export function getDeleteButton(onRemove) {
return (
<EuiButtonIcon
iconType="trash"
color="danger"
aria-label={i18n.translate('xpack.maps.styles.colorStops.deleteButtonAriaLabel', {
defaultMessage: 'Delete',
})}
title={i18n.translate('xpack.maps.styles.colorStops.deleteButtonLabel', {
defaultMessage: 'Delete',
})}
onClick={onRemove}
/>
);
}
import { EuiButtonIcon, EuiFlexGroup, EuiFlexItem, EuiFormRow } from '@elastic/eui';
import { MbValidatedColorPicker } from './mb_validated_color_picker';

export const ColorStops = ({
onChange,
Expand All @@ -72,6 +19,7 @@ export const ColorStops = ({
renderStopInput,
addNewRow,
canDeleteStop,
swatches,
}) => {
function getStopInput(stop, index) {
const onStopChange = newStopValue => {
Expand Down Expand Up @@ -134,10 +82,56 @@ export const ColorStops = ({
isInvalid: isStopsInvalid(newColorStops),
});
};
deleteButton = getDeleteButton(onRemove);
deleteButton = (
<EuiButtonIcon
iconType="trash"
color="danger"
aria-label={i18n.translate('xpack.maps.styles.colorStops.deleteButtonAriaLabel', {
defaultMessage: 'Delete',
})}
title={i18n.translate('xpack.maps.styles.colorStops.deleteButtonLabel', {
defaultMessage: 'Delete',
})}
onClick={onRemove}
/>
);
}

return getColorStopRow({ index, errors, stopInput, onColorChange, color, deleteButton, onAdd });
const colorPickerButtons = (
<div>
{deleteButton}
<EuiButtonIcon
iconType="plusInCircle"
color="primary"
aria-label="Add"
title="Add"
onClick={onAdd}
/>
</div>
);
return (
<EuiFormRow
key={index}
className="mapColorStop"
isInvalid={errors.length !== 0}
error={errors}
display="rowCompressed"
>
<EuiFlexGroup alignItems="center" gutterSize="xs">
<EuiFlexItem grow={false} className="mapStyleSettings__fixedBox">
{stopInput}
</EuiFlexItem>
<EuiFlexItem>
<MbValidatedColorPicker
onChange={onColorChange}
color={color}
swatches={swatches}
append={colorPickerButtons}
/>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFormRow>
);
});

return <div>{rows}</div>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const ColorStopsCategorical = ({
field,
onChange,
getValueSuggestions,
swatches,
}) => {
const getStopError = (stop, index) => {
let count = 0;
Expand Down Expand Up @@ -81,6 +82,7 @@ export const ColorStopsCategorical = ({
renderStopInput={renderStopInput}
canDeleteStop={canDeleteStop}
addNewRow={addCategoricalRow}
swatches={swatches}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { i18n } from '@kbn/i18n';
export const ColorStopsOrdinal = ({
colorStops = [{ stop: 0, color: DEFAULT_CUSTOM_COLOR }],
onChange,
swatches,
}) => {
const getStopError = (stop, index) => {
let error;
Expand Down Expand Up @@ -69,6 +70,7 @@ export const ColorStopsOrdinal = ({
renderStopInput={renderStopInput}
canDeleteStop={canDeleteStop}
addNewRow={addOrdinalRow}
swatches={swatches}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export function DynamicColorForm({
onDynamicStyleChange,
staticDynamicSelect,
styleProperty,
swatches,
}) {
const styleOptions = styleProperty.getOptions();

Expand Down Expand Up @@ -101,6 +102,7 @@ export function DynamicColorForm({
useCustomColorMap={_.get(styleOptions, 'useCustomColorRamp', false)}
styleProperty={styleProperty}
showColorMapTypeToggle={showColorMapTypeToggle}
swatches={swatches}
/>
);
} else if (styleProperty.isCategorical()) {
Expand All @@ -118,6 +120,7 @@ export function DynamicColorForm({
useCustomColorMap={_.get(styleOptions, 'useCustomColorPalette', false)}
styleProperty={styleProperty}
showColorMapTypeToggle={showColorMapTypeToggle}
swatches={swatches}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import React, { Component } from 'react';
import { isValidHex, EuiColorPicker, EuiFormControlLayoutProps } from '@elastic/eui';

export const RGBA_0000 = 'rgba(0,0,0,0)';

interface Props {
onChange: (color: string) => void;
color: string;
swatches?: string[];
append?: EuiFormControlLayoutProps['append'];
}

interface State {
colorInputValue: string;
}

// EuiColorPicker treats '' or invalid colors as transparent.
// Mapbox logs errors for '' or invalid colors.
// MbValidatedColorPicker is a wrapper around EuiColorPicker that reconciles the behavior difference
// between the two by returning a Mapbox safe RGBA_0000 for '' or invalid colors
// while keeping invalid state local so EuiColorPicker's input properly handles text input.
export class MbValidatedColorPicker extends Component<Props, State> {
state = {
colorInputValue: this.props.color === RGBA_0000 ? '' : this.props.color,
};

_onColorChange = (color: string) => {
// reflect all user input, whether valid or not
this.setState({ colorInputValue: color });
// Only surface mapbox valid input to caller
this.props.onChange(isValidHex(color) ? color : RGBA_0000);
};

render() {
return (
<EuiColorPicker
onChange={this._onColorChange}
color={this.state.colorInputValue}
swatches={this.props.swatches}
append={this.props.append}
compressed
/>
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
*/

import React from 'react';
import { EuiColorPicker, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import { MbValidatedColorPicker } from './mb_validated_color_picker';

export function StaticColorForm({
onStaticStyleChange,
Expand All @@ -23,11 +24,10 @@ export function StaticColorForm({
{staticDynamicSelect}
</EuiFlexItem>
<EuiFlexItem>
<EuiColorPicker
<MbValidatedColorPicker
onChange={onColorChange}
color={styleProperty.getOptions().color}
swatches={swatches}
compressed
/>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@ import {
EuiTextColor,
} from '@elastic/eui';
import { Category } from '../components/legend/category';
import { COLOR_MAP_TYPE } from '../../../../../common/constants';
import { COLOR_MAP_TYPE, RGBA_0000 } from '../../../../../common/constants';
import { isCategoricalStopsInvalid } from '../components/color/color_stops_utils';

const EMPTY_STOPS = { stops: [], defaultColor: null };
const RGBA_0000 = 'rgba(0,0,0,0)';

export class DynamicColorProperty extends DynamicStyleProperty {
syncCircleColorWithMb(mbLayerId, mbMap, alpha) {
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/maps/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,5 @@ export enum SCALING_TYPES {
CLUSTERS = 'CLUSTERS',
TOP_HITS = 'TOP_HITS',
}

export const RGBA_0000 = 'rgba(0,0,0,0)';

0 comments on commit a5526c8

Please sign in to comment.