From 43784e2eff766577b0bb39d5f2010c0d8f968903 Mon Sep 17 00:00:00 2001 From: Martin Raifer Date: Thu, 25 May 2023 19:19:09 +0200 Subject: [PATCH] take entity loc into account when resolving fields via parent preset, fixes #9524 this necessary when a regional preset (e.g. from NSI) is supposed to inherit fields from a parent preset, but the direct parent does NOT apply at the location of the entity to be added/edited. In that case we need to search for a potential regional variant of the parent preset. --- CHANGELOG.md | 4 ++- modules/actions/change_preset.js | 7 ++--- modules/modes/add_area.js | 14 +++++----- modules/modes/add_line.js | 13 +++++---- modules/modes/add_point.js | 18 ++++++++----- modules/presets/index.js | 3 --- modules/presets/preset.js | 40 +++++++++++++++++----------- modules/ui/fields/localized.js | 2 +- modules/ui/sections/preset_fields.js | 10 +++++-- 9 files changed, 68 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d5df8b02..52125745d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ _Breaking developer changes, which may affect downstream projects or sites that #### :rocket: Presets * Render "oneway" arrows for features with `waterway=pressurized`, `waterway=spillway`, `seamark:type=two-way_route` or `seamark:type=recommended_traffic_lane` ([#9492], thanks [@k-yle]) * Render "right-side" arrows for features with lifecycle prefixes ([#9493], thanks [@k-yle]) +* Take regional variants of parent presets into account when resolving preset fields ([#9524]) * Render "right-side" arrows for `man_made=quay` features #### :hammer: Development * Upgrade dependencies: `fortawesome` to v6.4, `which-polygon` to v2.2.1, `glob` to v9.2, `temaki` to v5.4, `marked` to v4.3, `core-js-bundle` to v3.30, `osm-auth` to v2.1 @@ -72,8 +73,9 @@ _Breaking developer changes, which may affect downstream projects or sites that [#9483]: https://github.com/openstreetmap/iD/pull/9483 [#9492]: https://github.com/openstreetmap/iD/pull/9492 [#9493]: https://github.com/openstreetmap/iD/pull/9493 -[#9520]: https://github.com/openstreetmap/iD/pull/9520 [#9501]: https://github.com/openstreetmap/iD/pull/9501 +[#9520]: https://github.com/openstreetmap/iD/pull/9520 +[#9524]: https://github.com/openstreetmap/iD/issues/9524 [#9630]: https://github.com/openstreetmap/iD/pull/9630 [#9637]: https://github.com/openstreetmap/iD/pull/9637 [#9638]: https://github.com/openstreetmap/iD/pull/9638 diff --git a/modules/actions/change_preset.js b/modules/actions/change_preset.js index 13f02596e..09bd4e3f3 100644 --- a/modules/actions/change_preset.js +++ b/modules/actions/change_preset.js @@ -3,6 +3,7 @@ export function actionChangePreset(entityID, oldPreset, newPreset, skipFieldDefa var entity = graph.entity(entityID); var geometry = entity.geometry(graph); var tags = entity.tags; + const loc = entity.extent(graph).center(); // preserve tags that the new preset might care about, if any var preserveKeys; @@ -15,14 +16,14 @@ export function actionChangePreset(entityID, oldPreset, newPreset, skipFieldDefa // only if old preset is not a sub-preset of the new one: // preserve tags for which the new preset has a field // https://github.com/openstreetmap/iD/issues/9372 - newPreset.fields().concat(newPreset.moreFields()) + newPreset.fields(loc).concat(newPreset.moreFields(loc)) .filter(f => f.matchGeometry(geometry)) .map(f => f.key).filter(Boolean) .forEach(key => preserveKeys.push(key)); } } - if (oldPreset) tags = oldPreset.unsetTags(tags, geometry, preserveKeys); - if (newPreset) tags = newPreset.setTags(tags, geometry, skipFieldDefaults); + if (oldPreset) tags = oldPreset.unsetTags(tags, geometry, preserveKeys, loc); + if (newPreset) tags = newPreset.setTags(tags, geometry, skipFieldDefaults, loc); return graph.replace(entity.update({tags: tags})); }; diff --git a/modules/modes/add_area.js b/modules/modes/add_area.js index b253d425f..7932feec0 100644 --- a/modules/modes/add_area.js +++ b/modules/modes/add_area.js @@ -15,9 +15,11 @@ export function modeAddArea(context, mode) { .on('startFromWay', startFromWay) .on('startFromNode', startFromNode); - var defaultTags = { area: 'yes' }; - if (mode.preset) defaultTags = mode.preset.setTags(defaultTags, 'area'); - + function defaultTags(loc) { + var defaultTags = { area: 'yes' }; + if (mode.preset) defaultTags = mode.preset.setTags(defaultTags, 'area', false, loc); + return defaultTags; + } function actionClose(wayId) { return function (graph) { @@ -29,7 +31,7 @@ export function modeAddArea(context, mode) { function start(loc) { var startGraph = context.graph(); var node = osmNode({ loc: loc }); - var way = osmWay({ tags: defaultTags }); + var way = osmWay({ tags: defaultTags(loc) }); context.perform( actionAddEntity(node), @@ -45,7 +47,7 @@ export function modeAddArea(context, mode) { function startFromWay(loc, edge) { var startGraph = context.graph(); var node = osmNode({ loc: loc }); - var way = osmWay({ tags: defaultTags }); + var way = osmWay({ tags: defaultTags(loc) }); context.perform( actionAddEntity(node), @@ -61,7 +63,7 @@ export function modeAddArea(context, mode) { function startFromNode(node) { var startGraph = context.graph(); - var way = osmWay({ tags: defaultTags }); + var way = osmWay({ tags: defaultTags(node.loc) }); context.perform( actionAddEntity(way), diff --git a/modules/modes/add_line.js b/modules/modes/add_line.js index 42e9d5db9..426f4ce4b 100644 --- a/modules/modes/add_line.js +++ b/modules/modes/add_line.js @@ -15,14 +15,17 @@ export function modeAddLine(context, mode) { .on('startFromWay', startFromWay) .on('startFromNode', startFromNode); - var defaultTags = {}; - if (mode.preset) defaultTags = mode.preset.setTags(defaultTags, 'line'); + function defaultTags(loc) { + var defaultTags = {}; + if (mode.preset) defaultTags = mode.preset.setTags(defaultTags, 'line', false, loc); + return defaultTags; + } function start(loc) { var startGraph = context.graph(); var node = osmNode({ loc: loc }); - var way = osmWay({ tags: defaultTags }); + var way = osmWay({ tags: defaultTags(loc) }); context.perform( actionAddEntity(node), @@ -37,7 +40,7 @@ export function modeAddLine(context, mode) { function startFromWay(loc, edge) { var startGraph = context.graph(); var node = osmNode({ loc: loc }); - var way = osmWay({ tags: defaultTags }); + var way = osmWay({ tags: defaultTags(loc) }); context.perform( actionAddEntity(node), @@ -52,7 +55,7 @@ export function modeAddLine(context, mode) { function startFromNode(node) { var startGraph = context.graph(); - var way = osmWay({ tags: defaultTags }); + var way = osmWay({ tags: defaultTags(node.loc) }); context.perform( actionAddEntity(way), diff --git a/modules/modes/add_point.js b/modules/modes/add_point.js index 3ad4a301b..deff506a9 100644 --- a/modules/modes/add_point.js +++ b/modules/modes/add_point.js @@ -19,12 +19,15 @@ export function modeAddPoint(context, mode) { .on('cancel', cancel) .on('finish', cancel); - var defaultTags = {}; - if (mode.preset) defaultTags = mode.preset.setTags(defaultTags, 'point'); + function defaultTags(loc) { + var defaultTags = {}; + if (mode.preset) defaultTags = mode.preset.setTags(defaultTags, 'point', false, loc); + return defaultTags; + } function add(loc) { - var node = osmNode({ loc: loc, tags: defaultTags }); + var node = osmNode({ loc: loc, tags: defaultTags(loc) }); context.perform( actionAddEntity(node), @@ -36,7 +39,7 @@ export function modeAddPoint(context, mode) { function addWay(loc, edge) { - var node = osmNode({ tags: defaultTags }); + var node = osmNode({ tags: defaultTags(loc) }); context.perform( actionAddMidpoint({loc: loc, edge: edge}, node), @@ -54,14 +57,15 @@ export function modeAddPoint(context, mode) { function addNode(node) { - if (Object.keys(defaultTags).length === 0) { + const _defaultTags = defaultTags(node.loc); + if (Object.keys(_defaultTags).length === 0) { enterSelectMode(node); return; } var tags = Object.assign({}, node.tags); // shallow copy - for (var key in defaultTags) { - tags[key] = defaultTags[key]; + for (var key in _defaultTags) { + tags[key] = _defaultTags[key]; } context.perform( diff --git a/modules/presets/index.js b/modules/presets/index.js index 8996aae12..2caf97549 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -163,9 +163,6 @@ export function presetIndex() { // Rebuild universal fields array _universal = Object.values(_fields).filter(field => field.universal); - // Reset all the preset fields - they'll need to be resolved again - Object.values(_presets).forEach(preset => preset.resetFields()); - // Rebuild geometry index _geometryIndex = { point: {}, vertex: {}, line: {}, area: {}, relation: {} }; _this.collection.forEach(preset => { diff --git a/modules/presets/preset.js b/modules/presets/preset.js index fbf3b841f..57a0c8b6b 100644 --- a/modules/presets/preset.js +++ b/modules/presets/preset.js @@ -1,7 +1,10 @@ +import { isEqual } from 'lodash'; + import { t } from '../core/localizer'; import { osmAreaKeys, osmAreaKeysExceptions } from '../osm/tags'; import { utilArrayUniq, utilObjectOmit } from '../util'; import { utilSafeClassName } from '../util/util'; +import { locationManager } from '../core/LocationManager'; // @@ -13,8 +16,6 @@ export function presetPreset(presetID, preset, addable, allFields, allPresets) { allPresets = allPresets || {}; let _this = Object.assign({}, preset); // shallow copy let _addable = addable || false; - let _resolvedFields; // cache - let _resolvedMoreFields; // cache let _searchName; // cache let _searchNameStripped; // cache let _searchAliases; // cache @@ -40,11 +41,9 @@ export function presetPreset(presetID, preset, addable, allFields, allPresets) { _this.originalMoreFields = (_this.moreFields || []); - _this.fields = () => _resolvedFields || (_resolvedFields = resolveFields('fields')); + _this.fields = loc => resolveFields('fields', loc); - _this.moreFields = () => _resolvedMoreFields || (_resolvedMoreFields = resolveFields('moreFields')); - - _this.resetFields = () => _resolvedFields = _resolvedMoreFields = null; + _this.moreFields = loc => resolveFields('moreFields', loc); _this.tags = _this.tags || {}; @@ -219,13 +218,13 @@ export function presetPreset(presetID, preset, addable, allFields, allPresets) { }; - _this.unsetTags = (tags, geometry, ignoringKeys, skipFieldDefaults) => { + _this.unsetTags = (tags, geometry, ignoringKeys, skipFieldDefaults, loc) => { // allow manually keeping some tags let removeTags = ignoringKeys ? utilObjectOmit(_this.removeTags, ignoringKeys) : _this.removeTags; tags = utilObjectOmit(tags, Object.keys(removeTags)); if (geometry && !skipFieldDefaults) { - _this.fields().forEach(field => { + _this.fields(loc).forEach(field => { if (field.matchGeometry(geometry) && field.key && field.default === tags[field.key] && (!ignoringKeys || ignoringKeys.indexOf(field.key) === -1)) { @@ -239,7 +238,7 @@ export function presetPreset(presetID, preset, addable, allFields, allPresets) { }; - _this.setTags = (tags, geometry, skipFieldDefaults) => { + _this.setTags = (tags, geometry, skipFieldDefaults, loc) => { const addTags = _this.addTags; tags = Object.assign({}, tags); // shallow copy @@ -277,7 +276,7 @@ export function presetPreset(presetID, preset, addable, allFields, allPresets) { } if (geometry && !skipFieldDefaults) { - _this.fields().forEach(field => { + _this.fields(loc).forEach(field => { if (field.matchGeometry(geometry) && field.key && !tags[field.key] && field.default) { tags[field.key] = field.default; } @@ -290,14 +289,14 @@ export function presetPreset(presetID, preset, addable, allFields, allPresets) { // For a preset without fields, use the fields of the parent preset. // Replace {preset} placeholders with the fields of the specified presets. - function resolveFields(which) { + function resolveFields(which, loc) { const fieldIDs = (which === 'fields' ? _this.originalFields : _this.originalMoreFields); let resolved = []; fieldIDs.forEach(fieldID => { const match = fieldID.match(referenceRegex); if (match !== null) { // a presetID wrapped in braces {} - resolved = resolved.concat(inheritFields(match[1], which)); + resolved = resolved.concat(inheritFields(allPresets[match[1]], which)); } else if (allFields[fieldID]) { // a normal fieldID resolved.push(allFields[fieldID]); } else { @@ -310,7 +309,19 @@ export function presetPreset(presetID, preset, addable, allFields, allPresets) { const endIndex = _this.id.lastIndexOf('/'); const parentID = endIndex && _this.id.substring(0, endIndex); if (parentID) { - resolved = inheritFields(parentID, which); + let parent = allPresets[parentID]; + if (loc) { + const validHere = locationManager.locationSetsAt(loc); + if (!validHere[parent.locationSetID]) { + // this is a preset for which a regional variant of the main preset exists + const candidateIDs = Object.keys(allPresets).filter(k => k.startsWith(parentID)); + parent = allPresets[candidateIDs.find(candidateID => { + const candidate = allPresets[candidateID]; + return validHere[candidate.locationSetID] && isEqual(candidate.tags, parent.tags); + })]; + } + } + resolved = inheritFields(parent, which); } } @@ -318,8 +329,7 @@ export function presetPreset(presetID, preset, addable, allFields, allPresets) { // returns an array of fields to inherit from the given presetID, if found - function inheritFields(presetID, which) { - const parent = allPresets[presetID]; + function inheritFields(parent, which) { if (!parent) return []; if (which === 'fields') { diff --git a/modules/ui/fields/localized.js b/modules/ui/fields/localized.js index 631475823..8726ce8c5 100644 --- a/modules/ui/fields/localized.js +++ b/modules/ui/fields/localized.js @@ -96,7 +96,7 @@ export function uiFieldLocalized(field, context) { var preset = presetManager.match(entity, context.graph()); if (preset) { var isSuggestion = preset.suggestion; - var fields = preset.fields(); + var fields = preset.fields(entity.extent(context.graph()).center()); var showsBrandField = fields.some(function(d) { return d.id === 'brand'; }); var showsOperatorField = fields.some(function(d) { return d.id === 'operator'; }); var setsName = preset.addTags.name; diff --git a/modules/ui/sections/preset_fields.js b/modules/ui/sections/preset_fields.js index c694c4b58..4b11edbc4 100644 --- a/modules/ui/sections/preset_fields.js +++ b/modules/ui/sections/preset_fields.js @@ -4,6 +4,7 @@ import { presetManager } from '../../presets'; import { t, localizer } from '../../core/localizer'; import { utilArrayIdentical } from '../../util/array'; import { utilArrayUnion, utilRebind } from '../../util'; +import { geoExtent } from '../../geo/extent'; import { uiField } from '../field'; import { uiFormFields } from '../form_fields'; import { uiSection } from '../section'; @@ -32,6 +33,11 @@ export function uiSectionPresetFields(context) { return geoms; }, {})); + const loc = _entityIDs.reduce(function(extent, entityID) { + var entity = context.graph().entity(entityID); + return extent.extend(entity.extent(context.graph())); + }, geoExtent()).center(); + var presetsManager = presetManager; var allFields = []; @@ -39,8 +45,8 @@ export function uiSectionPresetFields(context) { var sharedTotalFields; _presets.forEach(function(preset) { - var fields = preset.fields(); - var moreFields = preset.moreFields(); + var fields = preset.fields(loc); + var moreFields = preset.moreFields(loc); allFields = utilArrayUnion(allFields, fields); allMoreFields = utilArrayUnion(allMoreFields, moreFields);