From 6a515576f11a9d120bcabed917dbab84714df0d7 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Tue, 1 Oct 2019 18:21:03 +0200 Subject: [PATCH] Add an "Extract this point" quick fix for points-as-vertices validation warnings (re: #6319) --- data/core.yaml | 2 ++ dist/locales/en.json | 3 +++ modules/core/validator.js | 22 +++++++++-------- modules/validations/mismatched_geometry.js | 28 +++++++++++++++++++++- 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 4096936cc..bd64aeae5 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1530,6 +1530,8 @@ en: title: Continue drawing from end delete_feature: title: Delete this feature + extract_point: + title: Extract this point ignore_issue: title: Ignore this issue merge_close_vertices: diff --git a/dist/locales/en.json b/dist/locales/en.json index 066291a01..0f1e4e8e5 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1907,6 +1907,9 @@ "delete_feature": { "title": "Delete this feature" }, + "extract_point": { + "title": "Extract this point" + }, "ignore_issue": { "title": "Ignore this issue" }, diff --git a/modules/core/validator.js b/modules/core/validator.js index 6a30221e8..28d49ec4a 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -372,19 +372,20 @@ export function coreValidator(context) { var checkParentRels = [entity]; - if (entity.type === 'node') { // include parent ways + if (entity.type === 'node') { graph.parentWays(entity).forEach(function(parentWay) { - acc.add(parentWay.id); + acc.add(parentWay.id); // include parent ways checkParentRels.push(parentWay); }); - } else if (entity.type === 'relation') { // include members + } else if (entity.type === 'relation') { entity.members.forEach(function(member) { - acc.add(member.id); + acc.add(member.id); // include members }); - } else if (entity.type === 'way') { // include connected ways + } else if (entity.type === 'way') { entity.nodes.forEach(function(nodeID) { + acc.add(nodeID); // include child nodes graph._parentWays[nodeID].forEach(function(wayID) { - acc.add(wayID); + acc.add(wayID); // include connected ways }); }); } @@ -460,13 +461,14 @@ export function coreValidator(context) { var createdAndModifiedEntityIDs = difference.extantIDs(true); // created/modified (true = w/relation members) var entityIDsToCheck = entityIDsToValidate(createdAndModifiedEntityIDs, currGraph); - // "validate" deleted entities in order to update their related entities + // check modified and deleted entities against the old graph in order to update their related entities // (e.g. deleting the only highway connected to a road should create a disconnected highway issue) - var deletedEntityIDs = difference.deleted().map(function(entity) { return entity.id; }); - var entityIDsToCheckForDeleted = entityIDsToValidate(deletedEntityIDs, oldGraph); + var modifiedAndDeletedEntityIDs = difference.deleted().concat(difference.modified()) + .map(function(entity) { return entity.id; }); + var entityIDsToCheckForOldGraph = entityIDsToValidate(modifiedAndDeletedEntityIDs, oldGraph); // concat the sets - entityIDsToCheckForDeleted.forEach(entityIDsToCheck.add, entityIDsToCheck); + entityIDsToCheckForOldGraph.forEach(entityIDsToCheck.add, entityIDsToCheck); validateEntities(entityIDsToCheck); // dispatches 'validated' }; diff --git a/modules/validations/mismatched_geometry.js b/modules/validations/mismatched_geometry.js index 17163c00c..4d2880078 100644 --- a/modules/validations/mismatched_geometry.js +++ b/modules/validations/mismatched_geometry.js @@ -1,6 +1,8 @@ import { actionAddVertex } from '../actions/add_vertex'; import { actionChangeTags } from '../actions/change_tags'; import { actionMergeNodes } from '../actions/merge_nodes'; +import { actionExtract } from '../actions/extract'; +import { modeSelect } from '../modes/select'; import { osmNodeGeometriesForTags } from '../osm/tags'; import { geoHasSelfIntersections, geoSphericalDistance } from '../geo'; import { t } from '../util/locale'; @@ -169,6 +171,22 @@ export function validationMismatchedGeometry(context) { } else if (geometry === 'vertex' && !allowedGeometries.vertex && allowedGeometries.point) { + var extractOnClick = null; + if (!context.hasHiddenConnections(entity.id) && + !actionExtract(entity.id, context.projection).disabled(context.graph())) { + + extractOnClick = function(context) { + var entityId = this.issue.entityIds[0]; + var action = actionExtract(entityId, context.projection); + context.perform( + action, + t('operations.extract.annotation.single') + ); + // re-enter mode to trigger updates + context.enter(modeSelect(context, [action.getExtractedNodeID()])); + }; + } + return new validationIssue({ type: type, subtype: 'point_as_vertex', @@ -187,7 +205,15 @@ export function validationMismatchedGeometry(context) { .attr('class', 'issue-reference') .text(t('issues.point_as_vertex.reference')); }, - entityIds: [entity.id] + entityIds: [entity.id], + fixes: [ + new validationIssueFix({ + icon: 'iD-operation-extract', + title: t('issues.fix.extract_point.title'), + onClick: extractOnClick + }) + ], + hash: typeof extractOnClick, // avoid stale extraction fix }); }