Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Apr 3, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we also have this variable in x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js. Maybe we should add it to x-pack/plugins/maps/common/constants.ts and import from there?


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