diff --git a/CHANGELOG.md b/CHANGELOG.md index dc516f5b0..52586f666 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,12 +43,14 @@ _Breaking developer changes, which may affect downstream projects or sites that #### :white_check_mark: Validation #### :bug: Bugfixes * Fix bug which made it impossible to change an object's preset from a sub-preset to the respective parents preset (e.g. from Driveway to Service Road) ([#9372]) +* Fix corruption of (directional) `cycleway` tags when editing a multi-selection ([#9423]) #### :hourglass: Performance #### :rocket: Presets * Clamp degree values in `direction` fields between 0 and 359 degrees ([#9386]) * Disable increment/decrement buttons on number fields if the input value is not numeric or when there is a multi-selection with conflicting values * Filter out misspelled taginfo suggestions in combo field ([#9397]) * Add `highway=busway` to 'Traffic Roads' group of map features ([#9413], thanks [@Rewinteer]) +* Rename `cycleway` field type to `directionalCombo` and make it reusable for arbitrary directional tags ([#9423]) #### :hammer: Development * Upgrade to Transifex API v3 ([#9375]) * Upgrade dependencies: `d3` to v7.7, `@ideditor/country-coder` to v5.1, `@ideditor/location-conflation` to v1.1, `esbuild` to v0.16 @@ -62,6 +64,7 @@ _Breaking developer changes, which may affect downstream projects or sites that [#9392]: https://github.com/openstreetmap/iD/pull/9392 [#9397]: https://github.com/openstreetmap/iD/issues/9397 [#9413]: https://github.com/openstreetmap/iD/pull/9413 +[#9423]: https://github.com/openstreetmap/iD/pull/9423 [@alanb43]: https://github.com/alanb43 [@Rewinteer]: https://github.com/Rewinteer diff --git a/css/80_app.css b/css/80_app.css index 4d5984c02..de9ea0a8c 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -1551,10 +1551,10 @@ a.hide-toggle { } -/* Field - Access, Cycleway +/* Field - Access, DirectionalCombo ------------------------------------------------------- */ .form-field-input-access, -.form-field-input-cycleway { +.form-field-input-directionalcombo { flex: 1 1 auto; display: flex; flex-flow: row wrap; diff --git a/modules/ui/entity_editor.js b/modules/ui/entity_editor.js index 435d2b37d..ba2956ab3 100644 --- a/modules/ui/entity_editor.js +++ b/modules/ui/entity_editor.js @@ -163,14 +163,19 @@ export function uiEntityEditor(context) { var tags = Object.assign({}, entity.tags); // shallow copy - for (var k in changed) { - if (!k) continue; - var v = changed[k]; - if (typeof v === 'object') { - // a "key only" tag change - tags[k] = tags[v.oldKey]; - } else if (v !== undefined || tags.hasOwnProperty(k)) { - tags[k] = v; + if (typeof changed === 'function') { + // a complex callback tag change + tags = changed(tags); + } else { + for (var k in changed) { + if (!k) continue; + var v = changed[k]; + if (typeof v === 'object') { + // a "key only" tag change + tags[k] = tags[v.oldKey]; + } else if (v !== undefined || tags.hasOwnProperty(k)) { + tags[k] = v; + } } } diff --git a/modules/ui/fields/combo.js b/modules/ui/fields/combo.js index 7cdfa43e7..b343aa293 100644 --- a/modules/ui/fields/combo.js +++ b/modules/ui/fields/combo.js @@ -115,14 +115,22 @@ export function uiFieldCombo(field, context) { } + function getLabelId(field, v) { + return field.hasTextForStringId(`options.${v}.title`) + ? `options.${v}.title` + : `options.${v}`; + } + + // returns the display value for a tag value // (for multiCombo, tval should be the key suffix, not the entire key) function displayValue(tval) { tval = tval || ''; var stringsField = field.resolveReference('stringsCrossReference'); - if (stringsField.hasTextForStringId('options.' + tval)) { - return stringsField.t('options.' + tval, { default: tval }); + const labelId = getLabelId(stringsField, tval); + if (stringsField.hasTextForStringId(labelId)) { + return stringsField.t(labelId, { default: tval }); } if (field.type === 'typeCombo' && tval.toLowerCase() === 'yes') { @@ -139,8 +147,9 @@ export function uiFieldCombo(field, context) { tval = tval || ''; var stringsField = field.resolveReference('stringsCrossReference'); - if (stringsField.hasTextForStringId('options.' + tval)) { - return stringsField.t.append('options.' + tval, { default: tval }); + const labelId = getLabelId(stringsField, tval); + if (stringsField.hasTextForStringId(labelId)) { + return stringsField.t(labelId, { default: tval }); } if (field.type === 'typeCombo' && tval.toLowerCase() === 'yes') { @@ -184,12 +193,13 @@ export function uiFieldCombo(field, context) { if (!(field.options || stringsField.options)) return []; return (field.options || stringsField.options).map(function(v) { + const labelId = getLabelId(stringsField, v); return { key: v, - value: stringsField.t('options.' + v, { default: v }), - title: v, - display: addComboboxIcons(stringsField.t.append('options.' + v, { default: v }), v), - klass: stringsField.hasTextForStringId('options.' + v) ? '' : 'raw-option' + value: stringsField.t(labelId, { default: v }), + title: stringsField.t(`options.${v}.description`, { default: v }), + display: addComboboxIcons(stringsField.t.append(labelId, { default: v }), v), + klass: stringsField.hasTextForStringId(labelId) ? '' : 'raw-option' }; }); } @@ -266,15 +276,17 @@ export function uiFieldCombo(field, context) { _container.classed('empty-combobox', data.length === 0); _comboData = data.concat(additionalOptions).map(function(d) { - var k = d.value; - if (_isMulti) k = k.replace(field.key, ''); - var isLocalizable = stringsField.hasTextForStringId('options.' + k); - var label = stringsField.t('options.' + k, { default: k }); + var v = d.value; + if (_isMulti) v = v.replace(field.key, ''); + const labelId = getLabelId(stringsField, v); + var isLocalizable = stringsField.hasTextForStringId(labelId); + var label = stringsField.t(labelId, { default: v }); return { - key: k, + key: v, value: label, - display: addComboboxIcons(stringsField.t.append('options.' + k, { default: k }), k), - title: isLocalizable ? k : (d.title !== label ? d.title : ''), + title: stringsField.t(`options.${v}.description`, { default: + isLocalizable ? v : (d.title !== label ? d.title : '') }), + display: addComboboxIcons(stringsField.t.append(labelId, { default: v }), v), klass: isLocalizable ? '' : 'raw-option' }; }); @@ -684,7 +696,8 @@ export function uiFieldCombo(field, context) { }).filter(Boolean); var showsValue = !isMixed && tags[field.key] && !(field.type === 'typeCombo' && tags[field.key] === 'yes'); - var isRawValue = showsValue && !stringsField.hasTextForStringId('options.' + tags[field.key]); + var isRawValue = showsValue && !stringsField.hasTextForStringId(`options.${tags[field.key]}`) + && !stringsField.hasTextForStringId(`options.${tags[field.key]}.title`); var isKnownValue = showsValue && !isRawValue; var isReadOnly = !_allowCustomValues || isKnownValue; diff --git a/modules/ui/fields/cycleway.js b/modules/ui/fields/cycleway.js deleted file mode 100644 index 1cd94e747..000000000 --- a/modules/ui/fields/cycleway.js +++ /dev/null @@ -1,170 +0,0 @@ -import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { select as d3_select } from 'd3-selection'; - -import { uiCombobox } from '../combobox'; -import { utilGetSetValue, utilNoAuto, utilRebind } from '../../util'; -import { t } from '../../core/localizer'; - - -export function uiFieldCycleway(field, context) { - var dispatch = d3_dispatch('change'); - var items = d3_select(null); - var wrap = d3_select(null); - var _tags; - - function cycleway(selection) { - - function stripcolon(s) { - return s.replace(':', ''); - } - - - wrap = selection.selectAll('.form-field-input-wrap') - .data([0]); - - wrap = wrap.enter() - .append('div') - .attr('class', 'form-field-input-wrap form-field-input-' + field.type) - .merge(wrap); - - - var div = wrap.selectAll('ul') - .data([0]); - - div = div.enter() - .append('ul') - .attr('class', 'rows') - .merge(div); - - var keys = ['cycleway:left', 'cycleway:right']; - - items = div.selectAll('li') - .data(keys); - - var enter = items.enter() - .append('li') - .attr('class', function(d) { return 'labeled-input preset-cycleway-' + stripcolon(d); }); - - enter - .append('span') - .attr('class', 'label preset-label-cycleway') - .attr('for', function(d) { return 'preset-input-cycleway-' + stripcolon(d); }) - .html(function(d) { return field.t.html('types.' + d); }); - - enter - .append('div') - .attr('class', 'preset-input-cycleway-wrap') - .append('input') - .attr('type', 'text') - .attr('class', function(d) { return 'preset-input-cycleway preset-input-' + stripcolon(d); }) - .call(utilNoAuto) - .each(function(d) { - d3_select(this) - .call(uiCombobox(context, 'cycleway-' + stripcolon(d)) - .data(cycleway.options(d)) - ); - }); - - items = items.merge(enter); - - // Update - wrap.selectAll('.preset-input-cycleway') - .on('change', change) - .on('blur', change); - } - - - function change(d3_event, key) { - - var newValue = context.cleanTagValue(utilGetSetValue(d3_select(this))); - - // don't override multiple values with blank string - if (!newValue && (Array.isArray(_tags.cycleway) || Array.isArray(_tags[key]))) return; - - if (newValue === 'none' || newValue === '') { newValue = undefined; } - - var otherKey = key === 'cycleway:left' ? 'cycleway:right' : 'cycleway:left'; - var otherValue = typeof _tags.cycleway === 'string' ? _tags.cycleway : _tags[otherKey]; - if (otherValue && Array.isArray(otherValue)) { - // we must always have an explicit value for comparison - otherValue = otherValue[0]; - } - if (otherValue === 'none' || otherValue === '') { otherValue = undefined; } - - var tag = {}; - - // If the left and right tags match, use the cycleway tag to tag both - // sides the same way - if (newValue === otherValue) { - tag = { - cycleway: newValue, - 'cycleway:left': undefined, - 'cycleway:right': undefined - }; - } else { - // Always set both left and right as changing one can affect the other - tag = { - cycleway: undefined - }; - tag[key] = newValue; - tag[otherKey] = otherValue; - } - - dispatch.call('change', this, tag); - } - - - cycleway.options = function() { - var stringsField = field.resolveReference('stringsCrossReference'); - return field.options.map(function(option) { - return { - title: stringsField.t('options.' + option + '.description'), - value: option - }; - }); - }; - - - cycleway.tags = function(tags) { - _tags = tags; - - // If cycleway is set, use that instead of individual values - var commonValue = typeof tags.cycleway === 'string' && tags.cycleway; - - utilGetSetValue(items.selectAll('.preset-input-cycleway'), function(d) { - if (commonValue) return commonValue; - return !tags.cycleway && typeof tags[d] === 'string' ? tags[d] : ''; - }) - .attr('title', function(d) { - if (Array.isArray(tags.cycleway) || Array.isArray(tags[d])) { - var vals = []; - if (Array.isArray(tags.cycleway)) { - vals = vals.concat(tags.cycleway); - } - if (Array.isArray(tags[d])) { - vals = vals.concat(tags[d]); - } - return vals.filter(Boolean).join('\n'); - } - return null; - }) - .attr('placeholder', function(d) { - if (Array.isArray(tags.cycleway) || Array.isArray(tags[d])) { - return t('inspector.multiple_values'); - } - return field.placeholder(); - }) - .classed('mixed', function(d) { - return Array.isArray(tags.cycleway) || Array.isArray(tags[d]); - }); - }; - - - cycleway.focus = function() { - var node = wrap.selectAll('input').node(); - if (node) node.focus(); - }; - - - return utilRebind(cycleway, dispatch, 'on'); -} diff --git a/modules/ui/fields/directional_combo.js b/modules/ui/fields/directional_combo.js new file mode 100644 index 000000000..91a280789 --- /dev/null +++ b/modules/ui/fields/directional_combo.js @@ -0,0 +1,122 @@ +import { dispatch as d3_dispatch } from 'd3-dispatch'; +import { select as d3_select } from 'd3-selection'; + +import { utilRebind } from '../../util'; +import { uiFieldCombo } from './combo'; + + +export function uiFieldDirectionalCombo(field, context) { + var dispatch = d3_dispatch('change'); + var items = d3_select(null); + var wrap = d3_select(null); + var _tags; + + var _combos = {}; + + function directionalCombo(selection) { + + function stripcolon(s) { + return s.replace(':', ''); + } + + + wrap = selection.selectAll('.form-field-input-wrap') + .data([0]); + + wrap = wrap.enter() + .append('div') + .attr('class', 'form-field-input-wrap form-field-input-' + field.type) + .merge(wrap); + + + var div = wrap.selectAll('ul') + .data([0]); + + div = div.enter() + .append('ul') + .attr('class', 'rows') + .merge(div); + + var keys = field.keys.slice(1); + + items = div.selectAll('li') + .data(keys); + + var enter = items.enter() + .append('li') + .attr('class', function(d) { return 'labeled-input preset-directionalcombo-' + stripcolon(d); }); + + enter + .append('span') + .attr('class', 'label preset-label-directionalcombo') + .attr('for', function(d) { return 'preset-input-directionalcombo-' + stripcolon(d); }) + .html(function(d) { return field.t.html('types.' + d); }); + + enter + .append('div') + .attr('class', 'preset-input-directionalcombo-wrap form-field-input-wrap') + .each(function(key) { + const subField = { + ...field, + type: 'combo', + key + }; + const combo = uiFieldCombo(subField, context); + combo.on('change', t => change(key, t[key])); + _combos[key] = combo; + d3_select(this).call(combo); + }); + + items = items.merge(enter); + + // Update + wrap.selectAll('.preset-input-directionalcombo') + .on('change', change) + .on('blur', change); + } + + + function change(key, newValue) { + const commonKey = field.keys[0]; + const otherKey = key === field.keys[1] ? field.keys[2] : field.keys[1]; + + dispatch.call('change', this, tags => { + const otherValue = tags[otherKey] || tags[commonKey]; + if (newValue === otherValue) { + // both tags match, use the common tag to tag both sides the same way + tags[commonKey] = newValue; + delete tags[key]; + delete tags[otherKey]; + } else { + // Always set both left and right as changing one can affect the other + tags[key] = newValue; + delete tags[commonKey]; + tags[otherKey] = otherValue; + } + return tags; + }); + } + + + directionalCombo.tags = function(tags) { + _tags = tags; + + const commonKey = field.keys[0]; + for (let key in _combos) { + const uniqueValues = [... new Set([] + .concat(_tags[commonKey]) + .concat(_tags[key]) + .filter(Boolean))]; + _combos[key].tags({ [key]: uniqueValues.length > 1 ? uniqueValues : uniqueValues[0] }); + } + }; + + + directionalCombo.focus = function() { + var node = wrap.selectAll('input').node(); + if (node) node.focus(); + }; + + + return utilRebind(directionalCombo, dispatch, 'on'); +} diff --git a/modules/ui/fields/index.js b/modules/ui/fields/index.js index 2c289641b..73733044b 100644 --- a/modules/ui/fields/index.js +++ b/modules/ui/fields/index.js @@ -3,7 +3,7 @@ export * from './combo'; export * from './input'; export * from './access'; export * from './address'; -export * from './cycleway'; +export * from './directional_combo'; export * from './lanes'; export * from './localized'; export * from './roadheight'; @@ -46,7 +46,7 @@ import { import { uiFieldAccess } from './access'; import { uiFieldAddress } from './address'; -import { uiFieldCycleway } from './cycleway'; +import { uiFieldDirectionalCombo } from './directional_combo'; import { uiFieldLanes } from './lanes'; import { uiFieldLocalized } from './localized'; import { uiFieldRoadheight } from './roadheight'; @@ -62,8 +62,9 @@ export var uiFields = { check: uiFieldCheck, colour: uiFieldColour, combo: uiFieldCombo, - cycleway: uiFieldCycleway, + cycleway: uiFieldDirectionalCombo, defaultCheck: uiFieldDefaultCheck, + directionalCombo: uiFieldDirectionalCombo, email: uiFieldEmail, identifier: uiFieldIdentifier, lanes: uiFieldLanes,