From 18dff285d08ad9a88f527297a848fa9167501b17 Mon Sep 17 00:00:00 2001 From: Ming Gao Date: Mon, 29 Apr 2019 22:57:15 -0400 Subject: [PATCH] add validation for very close node on road addresses issue #6241 also made a change in inspector, so that when an untagged entity has issues, show the entity editor so that the issues are visible when an item is clicked in the issues pane. --- data/core.yaml | 7 ++ dist/locales/en.json | 23 +++--- modules/ui/inspector.js | 4 +- modules/validations/dupe_node_on_road.js | 92 ++++++++++++++++++++++++ modules/validations/index.js | 1 + 5 files changed, 112 insertions(+), 15 deletions(-) create mode 100644 modules/validations/dupe_node_on_road.js diff --git a/data/core.yaml b/data/core.yaml index 4f343ce37..1893dc9ed 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1403,6 +1403,11 @@ en: unknown_road: message: "{feature} has no classification" reference: "Roads without a specific type may not appear in maps or routing." + dupe_node_on_road: + title: Very close nodes on road + message: "Very close nodes on road" + ref_merge: "Nodes are less than 2 meters away; you may want to merge them." + ref_move_away: "Nodes are less than 2 meters away but not mergable; you may want to move them further apart." impossible_oneway: title: Impossible One-Ways tip: "Find route issues with one-way features" @@ -1491,6 +1496,8 @@ en: title: Use different levels use_tunnel: title: Use a tunnel + merge_nodes: + title: Merge these nodes intro: done: done ok: OK diff --git a/dist/locales/en.json b/dist/locales/en.json index aa8563761..e4671115f 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1737,6 +1737,12 @@ "message": "{feature} has no classification", "reference": "Roads without a specific type may not appear in maps or routing." }, + "dupe_node_on_road": { + "title": "Very close nodes on road", + "message": "Very close nodes on road", + "ref_merge": "Nodes are less than 2 meters away; you may want to merge them.", + "ref_move_away": "Nodes are less than 2 meters away but not mergable; you may want to move them further apart." + }, "impossible_oneway": { "title": "Impossible One-Ways", "tip": "Find route issues with one-way features", @@ -1863,6 +1869,9 @@ }, "use_tunnel": { "title": "Use a tunnel" + }, + "merge_nodes": { + "title": "Merge these nodes" } } }, @@ -9092,20 +9101,6 @@ "description": "Japan GSI ortho Imagery. Usually better than bing, but a bit older.", "name": "Japan GSI ortho Imagery" }, - "gsi.go.jp_airphoto": { - "attribution": { - "text": "GSI Japan" - }, - "description": "Japan GSI airphoto Imagery. Not fully orthorectified, but a bit newer and/or differently covered than GSI ortho Imagery.", - "name": "Japan GSI airphoto Imagery" - }, - "gsi.go.jp_seamlessphoto": { - "attribution": { - "text": "GSI Japan seamless photo" - }, - "description": "Japan GSI seamlessphoto Imagery. The collection of latest imageries of GSI ortho, airphoto, post disaster and others.", - "name": "Japan GSI seamlessphoto Imagery" - }, "gsi.go.jp_std_map": { "attribution": { "text": "GSI Japan" diff --git a/modules/ui/inspector.js b/modules/ui/inspector.js index 264b45380..d0fb78e42 100644 --- a/modules/ui/inspector.js +++ b/modules/ui/inspector.js @@ -52,8 +52,10 @@ export function uiInspector(context) { var hasNonGeometryTags = entity.hasNonGeometryTags(); var isTaglessOrIntersectionVertex = entity.geometry(context.graph()) === 'vertex' && (!hasNonGeometryTags && !entity.isHighwayIntersection(context.graph())); + var issues = context.validator().getEntityIssues(_entityID); // start with the preset list if the feature is new and untagged or is an uninteresting vertex - var showPresetList = (newFeature && !hasNonGeometryTags) || isTaglessOrIntersectionVertex; + var showPresetList = issues.length === 0 && + ((newFeature && !hasNonGeometryTags) || isTaglessOrIntersectionVertex); if (showPresetList) { wrap.style('right', '-100%'); diff --git a/modules/validations/dupe_node_on_road.js b/modules/validations/dupe_node_on_road.js new file mode 100644 index 000000000..78f6f7022 --- /dev/null +++ b/modules/validations/dupe_node_on_road.js @@ -0,0 +1,92 @@ +import { operationMerge } from '../operations/index'; +import { t } from '../util/locale'; +import { validationIssue, validationIssueFix } from '../core/validation'; +import { geoExtent } from '../geo'; + + +export function validationDupeNodeOnRoad() { + var type = 'dupe_node_on_road'; + + + function isNodeOnRoad(node, context) { + var parentWays = context.graph().parentWays(node); + for (var i = 0; i < parentWays.length; i++) { + if (parentWays[i].tags.highway) { + return true; + } + } + return false; + } + + function findDupeNode(node, context) { + var epsilon = 2e-5, + extent = geoExtent([ + [node.loc[0] - epsilon, node.loc[1] - epsilon], + [node.loc[0] + epsilon, node.loc[1] + epsilon] + ]); + var filteredEnts = context.intersects(extent); + for (var i = 0; i < filteredEnts.length; i++) { + var entity = filteredEnts[i]; + if (entity.type === 'node' && entity.id !== node.id && + Math.abs(node.loc[0] - entity.loc[0]) < epsilon && + Math.abs(node.loc[1] - entity.loc[1]) < epsilon && + isNodeOnRoad(entity, context) ) { + return entity; + } + } + return null; + } + + + var validation = function(entity, context) { + if (entity.type !== 'node' || !isNodeOnRoad(entity, context)) return []; + + var dupe = findDupeNode(entity, context); + if (dupe === null) return []; + + var mergable = !operationMerge([entity.id, dupe.id], context).disabled(); + var fixes = []; + if (mergable) { + fixes.push( + new validationIssueFix({ + icon: 'iD-icon-plus', + title: t('issues.fix.merge_nodes.title'), + onClick: function() { + var entities = this.issue.entities, + operation = operationMerge([entities[0].id, entities[1].id], context); + if (!operation.disabled()) { + operation(); + } + } + }) + ); + } + + return [new validationIssue({ + type: type, + severity: 'warning', + message: t('issues.dupe_node_on_road.message'), + reference: showReference, + entities: [entity, dupe], + fixes: fixes + })]; + + + function showReference(selection) { + var referenceText = mergable + ? t('issues.dupe_node_on_road.ref_merge') + : t('issues.dupe_node_on_road.ref_move_away'); + selection.selectAll('.issue-reference') + .data([0]) + .enter() + .append('div') + .attr('class', 'issue-reference') + .text(referenceText); + } + }; + + + validation.type = type; + + return validation; +} diff --git a/modules/validations/index.js b/modules/validations/index.js index 35862a874..a2dd6b6c9 100644 --- a/modules/validations/index.js +++ b/modules/validations/index.js @@ -1,6 +1,7 @@ export { validationAlmostJunction } from './almost_junction'; export { validationCrossingWays } from './crossing_ways'; export { validationDisconnectedWay } from './disconnected_way'; +export { validationDupeNodeOnRoad } from './dupe_node_on_road'; export { validationFixmeTag } from './fixme_tag'; export { validationGenericName } from './generic_name'; export { validationImpossibleOneway } from './impossible_oneway';