From 5735b2509d7ed5383980c313980e5ba07fa855f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ky=E2=84=93e=20Hensel?= Date: Thu, 30 Jan 2025 21:25:01 +1100 Subject: [PATCH] ignore relations by default in the extract operation (#9816) using Shift+E allows the node **and its relations** to be extracted (the old behaviour) --- CHANGELOG.md | 2 ++ data/core.yaml | 1 + data/shortcuts.json | 5 +++++ modules/actions/extract.js | 11 ++++++++--- modules/behavior/operation.js | 3 ++- modules/operations/extract.js | 6 ++++-- test/spec/actions/extract.js | 13 ++++++++++--- 7 files changed, 32 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 363c13336..c204559a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ _Breaking developer changes, which may affect downstream projects or sites that * Fix flickering when switching between background imagery layers, make switching backgrounds snappier * Prevent password managers from autofilling tag fields ([#10508], thanks [@michaelabon]) #### :scissors: Operations +* When extracting a node from a way (shortcut: E), the relations are now preserved by default. Extracting a node without its relations is still possible using ⇧ Shift E ([#9816], thanks [@k-yle]) #### :camera: Street-Level #### :white_check_mark: Validation #### :bug: Bugfixes @@ -65,6 +66,7 @@ _Breaking developer changes, which may affect downstream projects or sites that * Migrate unit tests from karma to vitest ([#10452]) [#9013]: https://github.com/openstreetmap/iD/issues/9013 +[#9816]: https://github.com/openstreetmap/iD/issues/9816 [#10452]: https://github.com/openstreetmap/iD/pull/10452 [#10459]: https://github.com/openstreetmap/iD/pull/10459 [#10488]: https://github.com/openstreetmap/iD/pull/10488 diff --git a/data/core.yaml b/data/core.yaml index 497f81ebb..cff0346c8 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -2413,6 +2413,7 @@ en: merge: "Combine (merge) selected features" disconnect: "Disconnect selected features" extract: "Extract a point from a feature" + extract_without_relations: "Extract a point from a feature, without extracting its relations" split: "Split features at the selected points" reverse: "Reverse selected features" move: "Move selected features" diff --git a/data/shortcuts.json b/data/shortcuts.json index 8cf0f2d5f..233078b57 100644 --- a/data/shortcuts.json +++ b/data/shortcuts.json @@ -271,6 +271,11 @@ "shortcuts": ["operations.extract.key"], "text": "shortcuts.editing.operations.extract" }, + { + "modifiers": ["⇧"], + "shortcuts": ["operations.extract.key"], + "text": "shortcuts.editing.operations.extract_without_relations" + }, { "shortcuts": ["operations.split.key"], "text": "shortcuts.editing.operations.split" diff --git a/modules/actions/extract.js b/modules/actions/extract.js index 9834d9aa0..a18bae112 100644 --- a/modules/actions/extract.js +++ b/modules/actions/extract.js @@ -7,17 +7,19 @@ export function actionExtract(entityID, projection) { var extractedNodeID; - var action = function(graph) { + /** @param {boolean} shiftKeyPressed */ + var action = function(graph, shiftKeyPressed) { var entity = graph.entity(entityID); if (entity.type === 'node') { - return extractFromNode(entity, graph); + return extractFromNode(entity, graph, shiftKeyPressed); } return extractFromWayOrRelation(entity, graph); }; - function extractFromNode(node, graph) { + /** @param {boolean} shiftKeyPressed */ + function extractFromNode(node, graph, shiftKeyPressed) { extractedNodeID = node.id; @@ -31,7 +33,10 @@ export function actionExtract(entityID, projection) { return accGraph.replace(parentWay.replaceNode(entityID, replacement.id)); }, graph); + if (!shiftKeyPressed) return graph; + // Process any relations too + // but only if the user holds down the shift key while triggering the operation. return graph.parentRelations(node) .reduce(function(accGraph, parentRel) { return accGraph.replace(parentRel.replaceMember(node, replacement)); diff --git a/modules/behavior/operation.js b/modules/behavior/operation.js index b10fcd3cb..c6e53bd9a 100644 --- a/modules/behavior/operation.js +++ b/modules/behavior/operation.js @@ -4,6 +4,7 @@ export function behaviorOperation(context) { var _operation; + /** @param {KeyboardEvent} d3_event */ function keypress(d3_event) { // prevent operations during low zoom selection if (!context.map().withinEditableZoom()) return; @@ -29,7 +30,7 @@ export function behaviorOperation(context) { .label(_operation.annotation() || _operation.title)(); if (_operation.point) _operation.point(null); - _operation(); + _operation(d3_event); } } diff --git a/modules/operations/extract.js b/modules/operations/extract.js index 43ddfde7a..67c296e5e 100644 --- a/modules/operations/extract.js +++ b/modules/operations/extract.js @@ -32,11 +32,13 @@ export function operationExtract(context, selectedIDs) { return actionExtract(entityID, context.projection); }).filter(Boolean); + /** @param {KeyboardEvent | undefined} d3_event */ + var operation = function (d3_event) { + const shiftKeyPressed = d3_event?.shiftKey || false; - var operation = function () { var combinedAction = function(graph) { _actions.forEach(function(action) { - graph = action(graph); + graph = action(graph, shiftKeyPressed); }); return graph; }; diff --git a/test/spec/actions/extract.js b/test/spec/actions/extract.js index e11547501..e1fb74c57 100644 --- a/test/spec/actions/extract.js +++ b/test/spec/actions/extract.js @@ -579,8 +579,15 @@ describe('iD.actionExtract', function () { ]); }); + it('does not extract relations by default', () => { + const assertionGraph = iD.actionExtract('b')(graph); + + const targetNode = assertionGraph.entity('b'); + expect(assertionGraph.parentRelations(targetNode).length).to.eql(1); + }); + it('detached node not a member of relation', function () { - var assertionGraph = iD.actionExtract('b')(graph); + var assertionGraph = iD.actionExtract('b')(graph, true); var targetNode = assertionGraph.entity('b'); // Confirm is not a member of the relation @@ -588,7 +595,7 @@ describe('iD.actionExtract', function () { }); it('new node is a member of relation', function () { - var assertionGraph = iD.actionExtract('b')(graph); + var assertionGraph = iD.actionExtract('b')(graph, true); // Find the new node var targetWay = assertionGraph.entity('-'); @@ -603,7 +610,7 @@ describe('iD.actionExtract', function () { }); it('Relation membership has the same properties', function () { - var assertionGraph = iD.actionExtract('b')(graph); + var assertionGraph = iD.actionExtract('b')(graph, true); // Find the new node var targetWay = assertionGraph.entity('-');