From 9e32b4a23cb9b39a3fc245df1ff18c813c7a3321 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 17 Nov 2023 23:48:25 -0500 Subject: [PATCH] Add utilIsColorValid to check whether a color can be shown re: #1219 re: https://github.com/openstreetmap/iD/pull/9424 --- modules/ui/feature_list.js | 12 +++++++++--- modules/ui/sections/raw_member_editor.js | 12 +++++++++--- modules/ui/sections/raw_membership_editor.js | 17 ++++++++++++----- modules/util/index.js | 1 + modules/util/util.js | 15 +++++++++++++++ 5 files changed, 46 insertions(+), 11 deletions(-) diff --git a/modules/ui/feature_list.js b/modules/ui/feature_list.js index 74c1b52b3e..ee6f5f92df 100644 --- a/modules/ui/feature_list.js +++ b/modules/ui/feature_list.js @@ -7,7 +7,7 @@ import { osmEntity } from '../osm/entity'; import { uiIcon } from './icon'; import { uiCmd } from './cmd'; -import { utilHighlightEntities, utilNoAuto } from '../util'; +import { utilHighlightEntities, utilIsColorValid, utilNoAuto } from '../util'; export function uiFeatureList(context) { @@ -303,8 +303,8 @@ export function uiFeatureList(context) { label .append('span') .attr('class', 'entity-name') - .classed('has-color', d => d.entity && d.entity.type === 'relation' && d.entity.tags.colour) - .style('border-color', d => d.entity && d.entity.type === 'relation' && d.entity.tags.colour) + .classed('has-color', d => !!_getColor(d.entity)) + .style('border-color', d => _getColor(d.entity)) .text(d => d.name); enter @@ -319,6 +319,12 @@ export function uiFeatureList(context) { } + function _getColor(entity) { + const val = entity?.type === 'relation' && entity?.tags.colour; + return (val && utilIsColorValid(val)) ? val : null; + } + + function mouseover(d3_event, d) { if (!d.id || d.id === -1) return; utilHighlightEntities([d.id], true, context); diff --git a/modules/ui/sections/raw_member_editor.js b/modules/ui/sections/raw_member_editor.js index 09a0b1d1c4..680db654a9 100644 --- a/modules/ui/sections/raw_member_editor.js +++ b/modules/ui/sections/raw_member_editor.js @@ -10,7 +10,7 @@ import { osmEntity } from '../../osm'; import { uiIcon } from '../icon'; import { uiCombobox } from '../combobox'; import { uiSection } from '../section'; -import { utilHighlightEntities, utilNoAuto } from '../../util'; +import { utilHighlightEntities, utilIsColorValid, utilNoAuto } from '../../util'; const MAX_MEMBERS = 1000; @@ -194,8 +194,8 @@ export function uiSectionRawMemberEditor(context) { labelLink .append('span') .attr('class', 'member-entity-name') - .classed('has-color', d => d.member.type === 'relation' && d.member.tags.colour) - .style('border-color', d => d.member.type === 'relation' && d.member.tags.colour) + .classed('has-color', d => !!_getColor(d.member)) + .style('border-color', d => _getColor(d.member)) .text(d => (d.member ? l10n.displayName(d.member.tags) : '')); label @@ -335,6 +335,12 @@ export function uiSectionRawMemberEditor(context) { }; + function _getColor(entity) { + const val = entity?.type === 'relation' && entity?.tags.colour; + return (val && utilIsColorValid(val)) ? val : null; + } + + function _bindCombo(d, i, nodes) { let row = d3_select(nodes[i]); let role = row.selectAll('input.member-role'); diff --git a/modules/ui/sections/raw_membership_editor.js b/modules/ui/sections/raw_membership_editor.js index e412b2d3e8..50f02453fb 100644 --- a/modules/ui/sections/raw_membership_editor.js +++ b/modules/ui/sections/raw_membership_editor.js @@ -10,7 +10,7 @@ import { uiIcon } from '../icon'; import { uiCombobox } from '../combobox'; import { uiSection } from '../section'; import { uiTooltip } from '../tooltip'; -import { utilNoAuto, utilHighlightEntities } from '../../util'; +import { utilNoAuto, utilIsColorValid, utilHighlightEntities } from '../../util'; const MAX_MEMBERSHIPS = 1000; @@ -134,6 +134,12 @@ export function uiSectionRawMembershipEditor(context) { } + function _getColor(entity) { + const val = entity?.type === 'relation' && entity?.tags.colour; + return (val && utilIsColorValid(val)) ? val : null; + } + + function changeRole(d3_event, d) { if (d === 0) return; // called on newrow (shouldn't happen) if (_inChange) return; // avoid accidental recursive call iD#5731 @@ -245,6 +251,7 @@ export function uiSectionRawMembershipEditor(context) { var matched = presets.match(entity, graph); var presetName = (matched && matched.name()) || l10n.t('inspector.relation'); var entityName = l10n.displayName(entity.tags) || ''; + var color = _getColor(entity); return selection => { selection @@ -252,8 +259,8 @@ export function uiSectionRawMembershipEditor(context) { .text(presetName + ' '); selection .append('span') - .classed('has-color', entity.tags.colour) - .style('border-color', entity.tags.colour) + .classed('has-color', !!color) + .style('border-color', color) .text(entityName); }; } @@ -362,8 +369,8 @@ export function uiSectionRawMembershipEditor(context) { labelLink .append('span') .attr('class', 'member-entity-name') - .classed('has-color', d => d.relation.tags.colour) - .style('border-color', d => d.relation.tags.colour) + .classed('has-color', d => !!_getColor(d.relation)) + .style('border-color', d => _getColor(d.relation)) .text(function(d) { const matched = presets.match(d.relation, editor.staging.graph); // hide the network from the name if there is NSI match diff --git a/modules/util/index.js b/modules/util/index.js index fac67acdd2..bb6c5425e8 100644 --- a/modules/util/index.js +++ b/modules/util/index.js @@ -3,6 +3,7 @@ export { utilFastMouse } from './util'; export { utilFetchResponse, FetchError } from './fetch_response'; export { utilFunctor } from './util'; export { utilGetSetValue } from './get_set_value'; +export { utilIsColorValid } from './util'; export { utilHighlightEntities } from './util'; export { utilKeybinding } from './keybinding'; export { utilNoAuto } from './util'; diff --git a/modules/util/util.js b/modules/util/util.js index 50cc6c1f32..1f9f96ef38 100644 --- a/modules/util/util.js +++ b/modules/util/util.js @@ -39,6 +39,21 @@ export function utilHighlightEntities(ids, highlighted, context) { } +// Returns whether value looks like a valid color we can display +export function utilIsColorValid(value) { + // OSM only supports hex or named colors + if (!value.match(/^(#([0-9a-fA-F]{3}){1,2}|\w+)$/)) { + return false; + } + // see https://stackoverflow.com/a/68217760/1627467 + if (!CSS.supports('color', value) || ['unset', 'inherit', 'initial', 'revert'].includes(value)) { + return false; + } + + return true; +} + + // `utilSetTransform` // Applies a CSS transformation to the given selection export function utilSetTransform(selection, x, y, scale, rotate) {