Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
nickpeihl committed Mar 29, 2022
1 parent e822e0b commit 827c7c2
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 123 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/maps/public/classes/layers/layer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export interface ILayer {
cloneDescriptor(): Promise<LayerDescriptor>;
renderStyleEditor(
onStyleDescriptorChange: (styleDescriptor: StyleDescriptor) => void,
onCustomIconChange: (customIcons: CustomIcon[]) => void
onCustomIconsChange: (customIcons: CustomIcon[]) => void
): ReactElement<any> | null;
getInFlightRequestTokens(): symbol[];
getPrevRequestToken(dataId: string): symbol | undefined;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const defaultProps = {
},
],
customIcons: [],
onCustomIconsChange: () => {},
};

test('Should render default props', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ interface Props {
customIconStops?: IconStop[];
iconPaletteId: string | null;
customIcons: CustomIcon[];
onCustomIconsChange?: (customIcons: CustomIcon[]) => void;
onCustomIconsChange: (customIcons: CustomIcon[]) => void;
onChange: ({ customIconStops, iconPaletteId, useCustomIconMap }: StyleOptionChanges) => void;
styleProperty: IDynamicStyleProperty<IconDynamicOptions>;
useCustomIconMap?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,127 +51,128 @@ export function IconStops({
onCustomIconsChange,
customIcons,
}) {
return iconStops.map(({ stop, icon, iconSource }, index) => {
const iconInfo =
iconSource === ICON_SOURCE.CUSTOM
? customIcons.find(({ symbolId }) => symbolId === icon)
: getMakiSymbol(icon);
if (iconInfo === undefined) return;
const { svg, label } = iconInfo;
const onIconSelect = ({ selectedIconId }) => {
const newIconStops = [...iconStops];
newIconStops[index] = {
...iconStops[index],
icon: selectedIconId,
return iconStops
.map(({ stop, icon, iconSource }, index) => {
const iconInfo =
iconSource === ICON_SOURCE.CUSTOM
? customIcons.find(({ symbolId }) => symbolId === icon)
: getMakiSymbol(icon);
if (iconInfo === undefined) return;
const { svg, label } = iconInfo;
const onIconSelect = ({ selectedIconId }) => {
const newIconStops = [...iconStops];
newIconStops[index] = {
...iconStops[index],
icon: selectedIconId,
};
onChange({ customStops: newIconStops });
};
onChange({ customStops: newIconStops });
};
const onStopChange = (newStopValue) => {
const newIconStops = [...iconStops];
newIconStops[index] = {
...iconStops[index],
stop: newStopValue,
const onStopChange = (newStopValue) => {
const newIconStops = [...iconStops];
newIconStops[index] = {
...iconStops[index],
stop: newStopValue,
};
onChange({
customStops: newIconStops,
isInvalid: isDuplicateStop(newStopValue, iconStops),
});
};
onChange({
customStops: newIconStops,
isInvalid: isDuplicateStop(newStopValue, iconStops),
});
};
const onAdd = () => {
onChange({
customStops: [
...iconStops.slice(0, index + 1),
{
stop: '',
icon: getFirstUnusedSymbol(iconStops),
},
...iconStops.slice(index + 1),
],
});
};
const onRemove = () => {
onChange({
customStops: [...iconStops.slice(0, index), ...iconStops.slice(index + 1)],
});
};
const onAdd = () => {
onChange({
customStops: [
...iconStops.slice(0, index + 1),
{
stop: '',
icon: getFirstUnusedSymbol(iconStops),
},
...iconStops.slice(index + 1),
],
});
};
const onRemove = () => {
onChange({
customStops: [...iconStops.slice(0, index), ...iconStops.slice(index + 1)],
});
};

let deleteButton;
if (iconStops.length > 2 && index !== 0) {
deleteButton = (
<EuiButtonIcon
iconType="trash"
color="danger"
aria-label={i18n.translate('xpack.maps.styles.iconStops.deleteButtonAriaLabel', {
defaultMessage: 'Delete',
})}
title={i18n.translate('xpack.maps.styles.iconStops.deleteButtonLabel', {
defaultMessage: 'Delete',
})}
onClick={onRemove}
/>
);
}

let deleteButton;
if (iconStops.length > 2 && index !== 0) {
deleteButton = (
<EuiButtonIcon
iconType="trash"
color="danger"
aria-label={i18n.translate('xpack.maps.styles.iconStops.deleteButtonAriaLabel', {
defaultMessage: 'Delete',
})}
title={i18n.translate('xpack.maps.styles.iconStops.deleteButtonLabel', {
defaultMessage: 'Delete',
})}
onClick={onRemove}
/>
const iconStopButtons = (
<div>
{deleteButton}
<EuiButtonIcon
iconType="plusInCircle"
color="primary"
aria-label="Add"
title="Add"
onClick={onAdd}
/>
</div>
);
}

const iconStopButtons = (
<div>
{deleteButton}
<EuiButtonIcon
iconType="plusInCircle"
color="primary"
aria-label="Add"
title="Add"
onClick={onAdd}
/>
</div>
);
const errors = [];
// TODO check for duplicate values and add error messages here

const errors = [];
// TODO check for duplicate values and add error messages here
const stopInput =
index === 0 ? (
<EuiFieldText
aria-label={getOtherCategoryLabel()}
placeholder={getOtherCategoryLabel()}
disabled
compressed
/>
) : (
<StopInput
key={field.getName()} // force new component instance when field changes
field={field}
getValueSuggestions={getValueSuggestions}
value={stop}
onChange={onStopChange}
/>
);

const stopInput =
index === 0 ? (
<EuiFieldText
aria-label={getOtherCategoryLabel()}
placeholder={getOtherCategoryLabel()}
disabled
compressed
/>
) : (
<StopInput
key={field.getName()} // force new component instance when field changes
field={field}
getValueSuggestions={getValueSuggestions}
value={stop}
onChange={onStopChange}
/>
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>
<IconSelect
onCustomIconsChange={onCustomIconsChange}
customIcons={customIcons}
onChange={onIconSelect}
icon={{ value: icon, svg, label }}
append={iconStopButtons}
/>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFormRow>
);

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>
<IconSelect
onCustomIconsChange={onCustomIconsChange}
customIcons={customIcons}
onChange={onIconSelect}
icon={{ value: icon, svg, label }}
append={iconStopButtons}
/>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFormRow>
);
})
.filter((stop) => {
return stop !== undefined;
});
})
.filter((stop) => {
return stop !== undefined;
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,6 @@ export class VectorStyleEditor extends Component<Props, State> {
this.props.handlePropertyChange(propertyName, styleDescriptor);
};

_onCustomIconsChange = (icons: CustomIcon[]) => {
this.props.onCustomIconsChange(icons);
};

_hasMarkerOrIcon() {
const iconSize = this.props.styleProperties[VECTOR_STYLES.ICON_SIZE];
return iconSize.isDynamic() || (iconSize as StaticSizeProperty).getOptions().size > 0;
Expand Down Expand Up @@ -402,7 +398,7 @@ export class VectorStyleEditor extends Component<Props, State> {
customIcons={this.props.customIcons}
onStaticStyleChange={this._onStaticStyleChange}
onDynamicStyleChange={this._onDynamicStyleChange}
onCustomIconsChange={this._onCustomIconsChange}
onCustomIconsChange={this.props.onCustomIconsChange}
styleProperty={
this.props.styleProperties[VECTOR_STYLES.ICON] as IStyleProperty<
IconDynamicOptions | IconStaticOptions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,7 @@ export class VectorStyle implements IVectorStyle {

getIconSvg(symbolId: string) {
const meta = this._getIconMeta(symbolId);
return meta ? meta.svg : undefined
return meta ? meta.svg : undefined;
}

_getSymbolId() {
Expand Down

0 comments on commit 827c7c2

Please sign in to comment.