Skip to content

Commit 9e32b4a

Browse files
committed
Add utilIsColorValid to check whether a color can be shown
re: #1219 re: openstreetmap/iD#9424
1 parent 8e971ba commit 9e32b4a

File tree

5 files changed

+46
-11
lines changed

5 files changed

+46
-11
lines changed

modules/ui/feature_list.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { osmEntity } from '../osm/entity';
77
import { uiIcon } from './icon';
88
import { uiCmd } from './cmd';
99

10-
import { utilHighlightEntities, utilNoAuto } from '../util';
10+
import { utilHighlightEntities, utilIsColorValid, utilNoAuto } from '../util';
1111

1212

1313
export function uiFeatureList(context) {
@@ -303,8 +303,8 @@ export function uiFeatureList(context) {
303303
label
304304
.append('span')
305305
.attr('class', 'entity-name')
306-
.classed('has-color', d => d.entity && d.entity.type === 'relation' && d.entity.tags.colour)
307-
.style('border-color', d => d.entity && d.entity.type === 'relation' && d.entity.tags.colour)
306+
.classed('has-color', d => !!_getColor(d.entity))
307+
.style('border-color', d => _getColor(d.entity))
308308
.text(d => d.name);
309309

310310
enter
@@ -319,6 +319,12 @@ export function uiFeatureList(context) {
319319
}
320320

321321

322+
function _getColor(entity) {
323+
const val = entity?.type === 'relation' && entity?.tags.colour;
324+
return (val && utilIsColorValid(val)) ? val : null;
325+
}
326+
327+
322328
function mouseover(d3_event, d) {
323329
if (!d.id || d.id === -1) return;
324330
utilHighlightEntities([d.id], true, context);

modules/ui/sections/raw_member_editor.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { osmEntity } from '../../osm';
1010
import { uiIcon } from '../icon';
1111
import { uiCombobox } from '../combobox';
1212
import { uiSection } from '../section';
13-
import { utilHighlightEntities, utilNoAuto } from '../../util';
13+
import { utilHighlightEntities, utilIsColorValid, utilNoAuto } from '../../util';
1414

1515
const MAX_MEMBERS = 1000;
1616

@@ -194,8 +194,8 @@ export function uiSectionRawMemberEditor(context) {
194194
labelLink
195195
.append('span')
196196
.attr('class', 'member-entity-name')
197-
.classed('has-color', d => d.member.type === 'relation' && d.member.tags.colour)
198-
.style('border-color', d => d.member.type === 'relation' && d.member.tags.colour)
197+
.classed('has-color', d => !!_getColor(d.member))
198+
.style('border-color', d => _getColor(d.member))
199199
.text(d => (d.member ? l10n.displayName(d.member.tags) : ''));
200200

201201
label
@@ -335,6 +335,12 @@ export function uiSectionRawMemberEditor(context) {
335335
};
336336

337337

338+
function _getColor(entity) {
339+
const val = entity?.type === 'relation' && entity?.tags.colour;
340+
return (val && utilIsColorValid(val)) ? val : null;
341+
}
342+
343+
338344
function _bindCombo(d, i, nodes) {
339345
let row = d3_select(nodes[i]);
340346
let role = row.selectAll('input.member-role');

modules/ui/sections/raw_membership_editor.js

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { uiIcon } from '../icon';
1010
import { uiCombobox } from '../combobox';
1111
import { uiSection } from '../section';
1212
import { uiTooltip } from '../tooltip';
13-
import { utilNoAuto, utilHighlightEntities } from '../../util';
13+
import { utilNoAuto, utilIsColorValid, utilHighlightEntities } from '../../util';
1414

1515
const MAX_MEMBERSHIPS = 1000;
1616

@@ -134,6 +134,12 @@ export function uiSectionRawMembershipEditor(context) {
134134
}
135135

136136

137+
function _getColor(entity) {
138+
const val = entity?.type === 'relation' && entity?.tags.colour;
139+
return (val && utilIsColorValid(val)) ? val : null;
140+
}
141+
142+
137143
function changeRole(d3_event, d) {
138144
if (d === 0) return; // called on newrow (shouldn't happen)
139145
if (_inChange) return; // avoid accidental recursive call iD#5731
@@ -245,15 +251,16 @@ export function uiSectionRawMembershipEditor(context) {
245251
var matched = presets.match(entity, graph);
246252
var presetName = (matched && matched.name()) || l10n.t('inspector.relation');
247253
var entityName = l10n.displayName(entity.tags) || '';
254+
var color = _getColor(entity);
248255

249256
return selection => {
250257
selection
251258
.append('b')
252259
.text(presetName + ' ');
253260
selection
254261
.append('span')
255-
.classed('has-color', entity.tags.colour)
256-
.style('border-color', entity.tags.colour)
262+
.classed('has-color', !!color)
263+
.style('border-color', color)
257264
.text(entityName);
258265
};
259266
}
@@ -362,8 +369,8 @@ export function uiSectionRawMembershipEditor(context) {
362369
labelLink
363370
.append('span')
364371
.attr('class', 'member-entity-name')
365-
.classed('has-color', d => d.relation.tags.colour)
366-
.style('border-color', d => d.relation.tags.colour)
372+
.classed('has-color', d => !!_getColor(d.relation))
373+
.style('border-color', d => _getColor(d.relation))
367374
.text(function(d) {
368375
const matched = presets.match(d.relation, editor.staging.graph);
369376
// hide the network from the name if there is NSI match

modules/util/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ export { utilFastMouse } from './util';
33
export { utilFetchResponse, FetchError } from './fetch_response';
44
export { utilFunctor } from './util';
55
export { utilGetSetValue } from './get_set_value';
6+
export { utilIsColorValid } from './util';
67
export { utilHighlightEntities } from './util';
78
export { utilKeybinding } from './keybinding';
89
export { utilNoAuto } from './util';

modules/util/util.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,21 @@ export function utilHighlightEntities(ids, highlighted, context) {
3939
}
4040

4141

42+
// Returns whether value looks like a valid color we can display
43+
export function utilIsColorValid(value) {
44+
// OSM only supports hex or named colors
45+
if (!value.match(/^(#([0-9a-fA-F]{3}){1,2}|\w+)$/)) {
46+
return false;
47+
}
48+
// see https://stackoverflow.com/a/68217760/1627467
49+
if (!CSS.supports('color', value) || ['unset', 'inherit', 'initial', 'revert'].includes(value)) {
50+
return false;
51+
}
52+
53+
return true;
54+
}
55+
56+
4257
// `utilSetTransform`
4358
// Applies a CSS transformation to the given selection
4459
export function utilSetTransform(selection, x, y, scale, rotate) {

0 commit comments

Comments
 (0)