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] do not show border color for icon in legend when border width is zero #57501

Merged
merged 4 commits into from
Feb 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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

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 @@ -12,62 +12,30 @@ import { getMakiSymbolSvg, styleSvg, buildSrcUrl } from '../../symbol_utils';
export class SymbolIcon extends Component {
state = {
imgDataUrl: undefined,
prevSymbolId: undefined,
prevFill: undefined,
prevStroke: undefined,
prevStrokeWidth: undefined,
};

componentDidMount() {
this._isMounted = true;
this._loadSymbol(
this.props.symbolId,
this.props.fill,
this.props.stroke,
this.props.strokeWidth
);
}

componentDidUpdate() {
this._loadSymbol(
this.props.symbolId,
this.props.fill,
this.props.stroke,
this.props.strokeWidth
);
this._loadSymbol();
}

componentWillUnmount() {
this._isMounted = false;
}

async _loadSymbol(nextSymbolId, nextFill, nextStroke, nextStrokeWidth) {
if (
nextSymbolId === this.state.prevSymbolId &&
nextFill === this.state.prevFill &&
nextStroke === this.state.prevStroke &&
nextStrokeWidth === this.state.prevStrokeWidth
) {
return;
}

async _loadSymbol() {
let imgDataUrl;
try {
const svg = getMakiSymbolSvg(nextSymbolId);
const styledSvg = await styleSvg(svg, nextFill, nextStroke, nextStrokeWidth);
const svg = getMakiSymbolSvg(this.props.symbolId);
const styledSvg = await styleSvg(svg, this.props.fill, this.props.stroke);
imgDataUrl = buildSrcUrl(styledSvg);
} catch (error) {
// ignore failures - component will just not display an icon
return;
}

if (this._isMounted) {
this.setState({
imgDataUrl,
prevSymbolId: nextSymbolId,
prevFill: nextFill,
prevStroke: nextStroke,
prevStrokeWidth: nextStrokeWidth,
});
this.setState({ imgDataUrl });
}
}

Expand All @@ -80,7 +48,6 @@ export class SymbolIcon extends Component {
symbolId, // eslint-disable-line no-unused-vars
fill, // eslint-disable-line no-unused-vars
stroke, // eslint-disable-line no-unused-vars
strokeWidth, // eslint-disable-line no-unused-vars
...rest
} = this.props;

Expand All @@ -98,7 +65,6 @@ export class SymbolIcon extends Component {

SymbolIcon.propTypes = {
symbolId: PropTypes.string.isRequired,
fill: PropTypes.string.isRequired,
stroke: PropTypes.string.isRequired,
strokeWidth: PropTypes.string.isRequired,
fill: PropTypes.string,
stroke: PropTypes.string,
};
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ export function VectorIcon({ fillColor, isPointsOnly, isLinesOnly, strokeColor,

return (
<SymbolIcon
key={`${symbolId}${fillColor}${strokeColor}`}
symbolId={symbolId}
fill={style.fill}
stroke={style.stroke}
strokeWidth={style.strokeWidth}
fill={fillColor}
stroke={strokeColor}
/>
);
}
Expand All @@ -49,6 +49,6 @@ VectorIcon.propTypes = {
fillColor: PropTypes.string,
isPointsOnly: PropTypes.bool.isRequired,
isLinesOnly: PropTypes.bool.isRequired,
strokeColor: PropTypes.string.isRequired,
strokeColor: PropTypes.string,
symbolId: PropTypes.string,
};

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 @@ -80,11 +80,10 @@ export class IconSelect extends Component {
fullWidth
prepend={
<SymbolIcon
key={value}
className="mapIconSelectSymbol__inputButton"
symbolId={value}
fill={isDarkMode ? 'rgb(223, 229, 239)' : 'rgb(52, 55, 65)'}
stroke={isDarkMode ? 'rgb(255, 255, 255)' : 'rgb(0, 0, 0)'}
strokeWidth={'1px'}
/>
}
/>
Expand All @@ -100,10 +99,9 @@ export class IconSelect extends Component {
label,
prepend: (
<SymbolIcon
key={value}
symbolId={value}
fill={isDarkMode ? 'rgb(223, 229, 239)' : 'rgb(52, 55, 65)'}
stroke={isDarkMode ? 'rgb(255, 255, 255)' : 'rgb(0, 0, 0)'}
strokeWidth={'1px'}
/>
),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,6 @@ export class VectorStyleEditor extends Component {
this.props.handlePropertyChange(propertyName, styleDescriptor);
};

_hasBorder() {
const width = this.props.styleProperties[VECTOR_STYLES.LINE_WIDTH];
return width.isDynamic() ? width.isComplete() : width.getOptions().size !== 0;
}

_hasMarkerOrIcon() {
const iconSize = this.props.styleProperties[VECTOR_STYLES.ICON_SIZE];
return iconSize.isDynamic() || iconSize.getOptions().size > 0;
Expand Down Expand Up @@ -192,7 +187,7 @@ export class VectorStyleEditor extends Component {
const disabledByIconSize = isPointBorderColor && !this._hasMarkerOrIcon();
return (
<VectorStyleColorEditor
disabled={disabledByIconSize || !this._hasBorder()}
disabled={disabledByIconSize || !this.props.hasBorder}
disabledBy={disabledByIconSize ? VECTOR_STYLES.ICON_SIZE : VECTOR_STYLES.LINE_WIDTH}
swatches={DEFAULT_LINE_COLORS}
onStaticStyleChange={this._onStaticStyleChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,15 @@ export function buildSrcUrl(svgString) {
return domUrl.createObjectURL(svg);
}

export async function styleSvg(svgString, fill, stroke, strokeWidth) {
export async function styleSvg(svgString, fill, stroke) {
const svgXml = await parseXmlString(svgString);
let style = '';
if (fill) {
style += `fill:${fill};`;
}
if (stroke) {
style += `stroke:${stroke};`;
}
if (strokeWidth) {
style += `stroke-width:${strokeWidth};`;
style += `stroke-width:1;`;
}
if (style) svgXml.svg.$.style = style;
const builder = new xml2js.Builder();
Expand Down Expand Up @@ -119,8 +117,6 @@ export function getIconPaletteOptions(isDarkMode) {
className="mapIcon"
symbolId={iconId}
fill={isDarkMode ? 'rgb(223, 229, 239)' : 'rgb(52, 55, 65)'}
stroke={isDarkMode ? 'rgb(255, 255, 255)' : 'rgb(0, 0, 0)'}
strokeWidth={'1px'}
/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,12 @@ describe('styleSvg', () => {
);
});

it('Should add stroke style property to svg element', async () => {
it('Should add stroke and stroke-wdth style properties to svg element', async () => {
const unstyledSvgString =
'<svg version="1.1" width="11px" height="11px" viewBox="0 0 11 11"><path/></svg>';
const styledSvg = await styleSvg(unstyledSvgString, 'red', 'white');
expect(styledSvg.split('\n')[1]).toBe(
'<svg version="1.1" width="11px" height="11px" viewBox="0 0 11 11" style="fill:red;stroke:white;">'
);
});

it('Should add stroke-width style property to svg element', async () => {
const unstyledSvgString =
'<svg version="1.1" width="11px" height="11px" viewBox="0 0 11 11"><path/></svg>';
const styledSvg = await styleSvg(unstyledSvgString, 'red', 'white', '2px');
expect(styledSvg.split('\n')[1]).toBe(
'<svg version="1.1" width="11px" height="11px" viewBox="0 0 11 11" style="fill:red;stroke:white;stroke-width:2px;">'
'<svg version="1.1" width="11px" height="11px" viewBox="0 0 11 11" style="fill:red;stroke:white;stroke-width:1;">'
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ export class VectorStyle extends AbstractStyle {
];
}

_hasBorder() {
return this._lineWidthStyleProperty.isDynamic()
? this._lineWidthStyleProperty.isComplete()
: this._lineWidthStyleProperty.getOptions().size !== 0;
}

renderEditor({ layer, onStyleDescriptorChange }) {
const rawProperties = this.getRawProperties();
const handlePropertyChange = (propertyName, settings) => {
Expand Down Expand Up @@ -170,6 +176,7 @@ export class VectorStyle extends AbstractStyle {
onIsTimeAwareChange={onIsTimeAwareChange}
isTimeAware={this.isTimeAware()}
showIsTimeAware={propertiesWithFieldMeta.length > 0}
hasBorder={this._hasBorder()}
/>
);
}
Expand Down Expand Up @@ -423,12 +430,18 @@ export class VectorStyle extends AbstractStyle {

getIcon = () => {
const isLinesOnly = this._getIsLinesOnly();
const strokeColor = isLinesOnly
? extractColorFromStyleProperty(this._descriptor.properties[VECTOR_STYLES.LINE_COLOR], 'grey')
: extractColorFromStyleProperty(
this._descriptor.properties[VECTOR_STYLES.LINE_COLOR],
'none'
);
let strokeColor;
if (isLinesOnly) {
strokeColor = extractColorFromStyleProperty(
this._descriptor.properties[VECTOR_STYLES.LINE_COLOR],
'grey'
);
} else if (this._hasBorder()) {
strokeColor = extractColorFromStyleProperty(
this._descriptor.properties[VECTOR_STYLES.LINE_COLOR],
'none'
);
}
const fillColor = isLinesOnly
? null
: extractColorFromStyleProperty(
Expand Down