diff --git a/data/core.yaml b/data/core.yaml index 00f7ffa62..0b4afc557 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -346,6 +346,19 @@ en: not_downloaded: single: This feature can't be moved because parts of it have not yet been downloaded. multiple: These features can't be moved because parts of them have not yet been downloaded. + follow: + key: F + error: + needs_more_initial_nodes: This feature can't follow another feature because it isn't connected to enough consecutive points along another feature. Add another point manually to continue. + intersection_of_multiple_ways: + line: This feature can't follow a line because multiple lines are connected to the last two points. Add another point manually to continue. + area: This feature can't follow an area because multiple areas are connected to the last two points. Add another point manually to continue. + generic: This feature can't follow another feature because multiple features are connected to the last two points. Add another point manually to continue. + intersection_of_different_ways: + line: This feature can't follow the line because it is only connected at a single point. Add another point manually to continue. + area: This feature can't follow the area because it is only connected at a single point. Add another point manually to continue. + generic: This feature can't follow the other feature because they are only connected at a single point. Add another point manually to continue. + unknown: This feature can't follow another feature. reflect: title: long: Flip Long @@ -784,7 +797,7 @@ en: map_data: title: Map Data description: Map Data - key: F + key: U data_layers: Data Layers layers: osm: @@ -1762,10 +1775,12 @@ en: incompatible_source: title: Suspicious Sources tip: "Find features with suspicious source tags" - google: - feature: - message: '{feature} lists Google as a data source' - reference: "Google products are proprietary and must not be used as references." + feature: + message: '{feature} lists "{value}" as a data source' + reference: + amap: "Amap products are proprietary and must not be used as references." + baidu: "Baidu products are proprietary and must not be used as references." + google: "Google products are proprietary and must not be used as references." incorrect_name: message: '{feature} has the mistaken name "{name}"' message_language: '{feature} has the mistaken name "{name}" in {language}' @@ -2331,6 +2346,7 @@ en: split: "Split features at the selected points" reverse: "Reverse selected features" move: "Move selected features" + follow: "Follow a line or area" nudge: "Nudge selected features" nudge_more: "Nudge selected features by a lot" scale: "Scale selected features" diff --git a/data/shortcuts.json b/data/shortcuts.json index 4aacade84..8cf0f2d5f 100644 --- a/data/shortcuts.json +++ b/data/shortcuts.json @@ -283,6 +283,10 @@ "shortcuts": ["operations.move.key"], "text": "shortcuts.editing.operations.move" }, + { + "shortcuts": ["operations.follow.key"], + "text": "shortcuts.editing.operations.follow" + }, { "modifiers": ["⇧"], "shortcuts": ["↓", "↑", "←", "→"], diff --git a/modules/actions/join.js b/modules/actions/join.js index 75c7469c9..3515a243b 100644 --- a/modules/actions/join.js +++ b/modules/actions/join.js @@ -27,7 +27,6 @@ export function actionJoin(ids) { var action = function(graph) { var ways = ids.map(graph.entity, graph); - var survivorID = ways[0].id; // if any of the ways are sided (e.g. coastline, cliff, kerb) // sort them first so they establish the overall order - #6033 @@ -40,12 +39,15 @@ export function actionJoin(ids) { }); // Prefer to keep an existing way. - for (var i = 0; i < ways.length; i++) { - if (!ways[i].isNew()) { - survivorID = ways[i].id; - break; - } - } + // if there are mulitple existing ways, keep the oldest one + // the oldest way is determined by the ID of the way + const survivorID = ( + ways + .filter((way) => !way.isNew()) + .sort((a, b) => +a.osmId() - +b.osmId())[0] || ways[0] + ).id; + + var sequences = osmJoinWays(ways, graph); var joined = sequences[0]; diff --git a/modules/behavior/draw_way.js b/modules/behavior/draw_way.js index 84d6af211..0a7645d62 100644 --- a/modules/behavior/draw_way.js +++ b/modules/behavior/draw_way.js @@ -18,6 +18,7 @@ import { utilRebind } from '../util/rebind'; import { utilKeybinding } from '../util'; export function behaviorDrawWay(context, wayID, mode, startGraph) { + const keybinding = utilKeybinding('drawWay'); var dispatch = d3_dispatch('rejectedSelfIntersection'); @@ -412,6 +413,96 @@ export function behaviorDrawWay(context, wayID, mode, startGraph) { }); }; + /** + * @param {(typeof osmWay)[]} ways + * @returns {"line" | "area" | "generic"} + */ + function getFeatureType(ways) { + if (ways.every(way => way.isClosed())) return 'area'; + if (ways.every(way => !way.isClosed())) return 'line'; + return 'generic'; + } + + /** see PR #8671 */ + function followMode() { + if (_didResolveTempEdit) return; + + try { + + // get the last 2 added nodes. + // check if they are both part of only oneway (the same one) + // check if the ways that they're part of are the same way + // find index of the last two nodes, to determine the direction to travel around the existing way + // add the next node to the way we are drawing + + // if we're drawing an area, the first node = last node. + const isDrawingArea = _origWay.nodes[0] === _origWay.nodes.slice(-1)[0]; + + const [secondLastNodeId, lastNodeId] = _origWay.nodes.slice(isDrawingArea ? -3 : -2); + + if (!lastNodeId || !secondLastNodeId || !startGraph.hasEntity(lastNodeId) || !startGraph.hasEntity(secondLastNodeId)) { + context.ui().flash + .duration(4000) + .iconName('#iD-icon-no') + .label(t('operations.follow.error.needs_more_initial_nodes'))(); + return; + } + + const lastNodesParents = startGraph.parentWays(startGraph.entity(lastNodeId)); + const secondLastNodesParents = startGraph.parentWays(startGraph.entity(secondLastNodeId)); + + const featureType = getFeatureType(lastNodesParents); + + if (lastNodesParents.length !== 1 || secondLastNodesParents.length === 0) { + context.ui().flash + .duration(4000) + .iconName('#iD-icon-no') + .label(t(`operations.follow.error.intersection_of_multiple_ways.${featureType}`))(); + return; + } + + // Check if the last node's parent is also the parent of the second last node. + // The last node must only have one parent, but the second last node can have + // multiple parents. + if (!secondLastNodesParents.some(n => n.id === lastNodesParents[0].id)) { + context.ui().flash + .duration(4000) + .iconName('#iD-icon-no') + .label(t(`operations.follow.error.intersection_of_different_ways.${featureType}`))(); + return; + } + + const way = lastNodesParents[0]; + + const indexOfLast = way.nodes.indexOf(lastNodeId); + const indexOfSecondLast = way.nodes.indexOf(secondLastNodeId); + + // for a closed way, the first/last node is the same so it appears twice in the array, + // but indexOf always finds the first occurance. This is only an issue when following a way + // in descending order + const isDescendingPastZero = indexOfLast === way.nodes.length - 2 && indexOfSecondLast === 0; + + let nextNodeIndex = indexOfLast + (indexOfLast > indexOfSecondLast && !isDescendingPastZero ? 1 : -1); + // if we're following a closed way and we pass the first/last node, the next index will be -1 + if (nextNodeIndex === -1) nextNodeIndex = indexOfSecondLast === 1 ? way.nodes.length - 2 : 1; + + const nextNode = startGraph.entity(way.nodes[nextNodeIndex]); + + drawWay.addNode(nextNode, { + geometry: { type: 'Point', coordinates: nextNode.loc }, + id: nextNode.id, + properties: { target: true, entity: nextNode }, + }); + } catch (ex) { + context.ui().flash + .duration(4000) + .iconName('#iD-icon-no') + .label(t('operations.follow.error.unknown'))(); + } + } + + keybinding.on(t('operations.follow.key'), followMode); + d3_select(document).call(keybinding); // Finish the draw operation, removing the temporary edit. // If the way has enough nodes to be valid, it's selected. diff --git a/modules/presets/index.js b/modules/presets/index.js index 6770f3994..6487534a6 100644 --- a/modules/presets/index.js +++ b/modules/presets/index.js @@ -208,11 +208,8 @@ export function presetIndex() { let address; let best = -1; let match; - + let matchCandidates = []; let validLocations; - if (Array.isArray(loc)) { - validLocations = locationManager.locationsAt(loc); - } for (let k in tags) { // If any part of an address is present, allow fallback to "Address" preset - #4353 @@ -225,13 +222,11 @@ export function presetIndex() { for (let i = 0; i < keyMatches.length; i++) { const candidate = keyMatches[i]; - - // discard candidate preset if location is not valid at `loc` - if (validLocations && candidate.locationSetID) { - if (!validLocations[candidate.locationSetID]) continue; - } - const score = candidate.matchScore(tags); + if (score === -1){ + continue; + } + matchCandidates.push({score, candidate}); if (score > best) { best = score; match = candidate; @@ -239,6 +234,21 @@ export function presetIndex() { } } + if (match && match.locationSetID && match.locationSetID !== '+[Q2]' && Array.isArray(loc)){ + validLocations = locationManager.locationsAt(loc); + if (!validLocations[match.locationSetID]){ + matchCandidates.sort((a, b) => (a.score < b.score) ? 1 : -1); + for (let i = 0; i < matchCandidates.length; i++){ + const candidateScore = matchCandidates[i]; + if (!candidateScore.candidate.locationSetID || validLocations[candidateScore.candidate.locationSetID]){ + match = candidateScore.candidate; + best = candidateScore.score; + break; + } + } + } + } + if (address && (!match || match.isFallback())) { match = address; } diff --git a/modules/util/svg_paths_rtl_fix.js b/modules/util/svg_paths_rtl_fix.js index 74cfe28d7..b5e59dbae 100644 --- a/modules/util/svg_paths_rtl_fix.js +++ b/modules/util/svg_paths_rtl_fix.js @@ -1,5 +1,6 @@ // see https://github.com/openstreetmap/iD/pull/3707 // https://gist.github.com/mapmeld/556b09ddec07a2044c76e1ef45f01c60 +// fixed in Chromium 96.0 https://bugs.chromium.org/p/chromium/issues/detail?id=374526 import { WordShaper } from 'alif-toolkit'; diff --git a/modules/util/util.js b/modules/util/util.js index 388ba7261..e078647e6 100644 --- a/modules/util/util.js +++ b/modules/util/util.js @@ -225,8 +225,9 @@ export function utilDisplayName(entity) { export function utilDisplayNameForPath(entity) { var name = utilDisplayName(entity); var isFirefox = utilDetect().browser.toLowerCase().indexOf('firefox') > -1; + var isNewChromium = Number(utilDetect().version.split('.')[0]) >= 96.0; - if (!isFirefox && name && rtlRegex.test(name)) { + if (!isFirefox && !isNewChromium && name && rtlRegex.test(name)) { name = fixRTLTextForSvg(name); } diff --git a/modules/validations/incompatible_source.js b/modules/validations/incompatible_source.js index ed2041dde..154e458ca 100644 --- a/modules/validations/incompatible_source.js +++ b/modules/validations/incompatible_source.js @@ -4,65 +4,73 @@ import { validationIssue, validationIssueFix } from '../core/validation'; export function validationIncompatibleSource() { - var type = 'incompatible_source'; - var invalidSources = [ - { - id:'google', regex:'google', exceptRegex: 'books.google|Google Books|drive.google|googledrive|Google Drive' - } - ]; + const type = 'incompatible_source'; + const incompatibleRules = [ + { + id: 'amap', + regex: /(amap|autonavi|mapabc|高德)/i + }, + { + id: 'baidu', + regex: /(baidu|mapbar|百度)/i + }, + { + id: 'google', + regex: /google/i, + exceptRegex: /((books|drive)\.google|google\s?(books|drive|plus))/i + } + ]; - var validation = function checkIncompatibleSource(entity) { - var entitySources = entity.tags && entity.tags.source && entity.tags.source.split(';'); + const validation = function checkIncompatibleSource(entity) { + const entitySources = entity.tags && entity.tags.source && entity.tags.source.split(';'); + if (!entitySources) return []; - if (!entitySources) return []; + const entityID = entity.id; - var issues = []; - - invalidSources.forEach(function(invalidSource) { - - var hasInvalidSource = entitySources.some(function(source) { - if (!source.match(new RegExp(invalidSource.regex, 'i'))) return false; - if (invalidSource.exceptRegex && source.match(new RegExp(invalidSource.exceptRegex, 'i'))) return false; - return true; - }); - - if (!hasInvalidSource) return; - - issues.push(new validationIssue({ - type: type, - severity: 'warning', - message: function(context) { - var entity = context.hasEntity(this.entityIds[0]); - return entity ? t.html('issues.incompatible_source.' + invalidSource.id + '.feature.message', { - feature: utilDisplayLabel(entity, context.graph(), true /* verbose */) - }) : ''; - }, - reference: getReference(invalidSource.id), - entityIds: [entity.id], - dynamicFixes: function() { - return [ - new validationIssueFix({ - title: t.html('issues.fix.remove_proprietary_data.title') - }) - ]; - } - })); + return entitySources + .map(source => { + const matchRule = incompatibleRules.find(rule => { + if (!rule.regex.test(source)) return false; + if (rule.exceptRegex && rule.exceptRegex.test(source)) return false; + return true; }); - return issues; + if (!matchRule) return null; + + return new validationIssue({ + type: type, + severity: 'warning', + message: (context) => { + const entity = context.hasEntity(entityID); + return entity ? t.html('issues.incompatible_source.feature.message', { + feature: utilDisplayLabel(entity, context.graph(), true /* verbose */), + value: source + }) : ''; + }, + reference: getReference(matchRule.id), + entityIds: [entityID], + hash: source, + dynamicFixes: () => { + return [ + new validationIssueFix({ title: t.html('issues.fix.remove_proprietary_data.title') }) + ]; + } + }); + + }).filter(Boolean); - function getReference(id) { - return function showReference(selection) { - selection.selectAll('.issue-reference') - .data([0]) - .enter() - .append('div') - .attr('class', 'issue-reference') - .html(t.html('issues.incompatible_source.' + id + '.reference')); - }; - } + function getReference(id) { + return function showReference(selection) { + selection.selectAll('.issue-reference') + .data([0]) + .enter() + .append('div') + .attr('class', 'issue-reference') + .html(t.html(`issues.incompatible_source.reference.${id}`)); + }; + } }; validation.type = type; diff --git a/modules/validations/mismatched_geometry.js b/modules/validations/mismatched_geometry.js index 199cf238e..42fb0b4ef 100644 --- a/modules/validations/mismatched_geometry.js +++ b/modules/validations/mismatched_geometry.js @@ -220,8 +220,9 @@ export function validationMismatchedGeometry() { if (sourceGeom === 'area') targetGeoms.unshift('line'); + var asSource = presetManager.match(entity, graph); + var targetGeom = targetGeoms.find(nodeGeom => { - var asSource = presetManager.matchTags(entity.tags, sourceGeom); var asTarget = presetManager.matchTags(entity.tags, nodeGeom); if (!asSource || !asTarget || asSource === asTarget || diff --git a/package.json b/package.json index 336503409..d50098057 100644 --- a/package.json +++ b/package.json @@ -77,9 +77,9 @@ "@ideditor/temaki": "~4.4.0", "@mapbox/maki": "^6.0.0", "@rollup/plugin-babel": "^5.2.1", - "@rollup/plugin-commonjs": "^17.0.0", + "@rollup/plugin-commonjs": "^21.0.0", "@rollup/plugin-json": "^4.0.1", - "@rollup/plugin-node-resolve": "~11.2.0", + "@rollup/plugin-node-resolve": "~13.0.5", "autoprefixer": "^10.0.1", "btoa": "^1.2.1", "chai": "^4.1.0", diff --git a/test/spec/actions/join.js b/test/spec/actions/join.js index deb30e77f..fecc421de 100644 --- a/test/spec/actions/join.js +++ b/test/spec/actions/join.js @@ -424,6 +424,29 @@ describe('iD.actionJoin', function () { expect(graph.hasEntity('w-2')).to.be.undefined; }); + it('prefers to keep the oldest way', function () { + // n1 ==> n2 ++> n3 --> n4 + // ==> is existing, ++> is existing, --> is new + // Expected result: + // n1 ==> n2 ==> n3 ==> n4 + var graph = iD.coreGraph([ + iD.osmNode({ id: 'n1', loc: [0,0] }), + iD.osmNode({ id: 'n2', loc: [2,0] }), + iD.osmNode({ id: 'n3', loc: [4,0] }), + iD.osmNode({ id: 'n4', loc: [6,0] }), + iD.osmWay({ id: 'w1', nodes: ['n2', 'n3'] }), + iD.osmWay({ id: 'w2', nodes: ['n1', 'n2'] }), + iD.osmWay({ id: 'w-1', nodes: ['n3', 'n4'] }) + ]); + + graph = iD.actionJoin(['w1', 'w2', 'w-1'])(graph); + + // way 1 is the oldest (it has the lower id) so it kept that one + expect(graph.entity('w1').nodes).to.eql(['n1', 'n2', 'n3', 'n4']); + expect(graph.hasEntity('w2')).to.be.undefined; + expect(graph.hasEntity('w-1')).to.be.undefined; + }); + it('merges tags', function () { var graph = iD.coreGraph([ iD.osmNode({id: 'a', loc: [0,0]}),