From 6089a6aaea975c115c41d924c960042ae1813117 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Sun, 18 May 2014 13:27:15 -0700 Subject: [PATCH] Better solution for maintaining selection --- js/id/actions/restrict_turn.js | 12 +---- js/id/geo/intersection.js | 78 +++++++++++++----------------- js/id/ui/preset/restrictions.js | 25 +++------- test/spec/actions/restrict_turn.js | 34 ------------- test/spec/geo/intersection.js | 56 +++++++++------------ 5 files changed, 67 insertions(+), 138 deletions(-) diff --git a/js/id/actions/restrict_turn.js b/js/id/actions/restrict_turn.js index aaac9b734..eeefee3e6 100644 --- a/js/id/actions/restrict_turn.js +++ b/js/id/actions/restrict_turn.js @@ -29,9 +29,7 @@ // be assigned a new ID. // iD.actions.RestrictTurn = function(turn, projection, restrictionId) { - var dispatch = d3.dispatch('split'); - - function action(graph) { + return function(graph) { var from = graph.entity(turn.from.way), via = graph.entity(turn.via.node), to = graph.entity(turn.to.way); @@ -42,8 +40,6 @@ iD.actions.RestrictTurn = function(turn, projection, restrictionId) { graph = iD.actions.Split(via.id, [newFromId]) .limitWays([from.id])(graph); - dispatch.split(from.id, newFromId, graph); - var newFrom = graph.entity(newFromId); if (newFrom.nodes.indexOf(turn.from.node) !== -1) from = newFrom; @@ -57,8 +53,6 @@ iD.actions.RestrictTurn = function(turn, projection, restrictionId) { graph = iD.actions.Split(via.id, [newToId]) .limitWays([to.id])(graph); - dispatch.split(to.id, newToId, graph); - var newTo = graph.entity(newToId); if (newTo.nodes.indexOf(turn.to.node) !== -1) to = newTo; @@ -81,7 +75,5 @@ iD.actions.RestrictTurn = function(turn, projection, restrictionId) { {id: to.id, type: 'way', role: 'to'} ] })); - } - - return d3.rebind(action, dispatch, 'on'); + }; }; diff --git a/js/id/geo/intersection.js b/js/id/geo/intersection.js index dab5b2fb2..e2dace3d6 100644 --- a/js/id/geo/intersection.js +++ b/js/id/geo/intersection.js @@ -35,11 +35,11 @@ iD.geo.Intersection = function(graph, vertexId) { graph: graph }; - intersection.turns = function(wayId) { - if (!wayId) + intersection.turns = function(fromNodeID) { + if (!fromNodeID) return []; - var way = graph.entity(wayId); + var way = _.find(highways, function(way) { return way.contains(fromNodeID); }); if (way.first() === vertex.id && way.tags.oneway === 'yes') return []; if (way.last() === vertex.id && way.tags.oneway === '-1') @@ -64,58 +64,48 @@ iD.geo.Intersection = function(graph, vertexId) { return iD.geo.Turn(turn); } - var via = {node: vertex.id}, - ways = [], + var from = { + node: way.nodes[way.first() === vertex.id ? 1 : way.nodes.length - 2], + way: way.id.split(/-(a|b)/)[0] + }, + via = {node: vertex.id}, turns = []; - if (way.affix(vertexId)) { - ways = [way]; - } else { - ways = [graph.entity(way.id + '-a'), graph.entity(way.id + '-b')]; - } + highways.forEach(function(parent) { + if (parent === way) + return; - ways.forEach(function(way) { - var from = { - node: way.nodes[way.first() === vertex.id ? 1 : way.nodes.length - 2], - way: way.id.split(/-(a|b)/)[0] - }; + var index = parent.nodes.indexOf(vertex.id); - highways.forEach(function(parent) { - if (parent === way) - return; - - var index = parent.nodes.indexOf(vertex.id); - - // backward - if (parent.first() !== vertex.id && parent.tags.oneway !== 'yes') { - turns.push(withRestriction({ - from: from, - via: via, - to: {node: parent.nodes[index - 1], way: parent.id.split(/-(a|b)/)[0]} - })); - } - - // forward - if (parent.last() !== vertex.id && parent.tags.oneway !== '-1') { - turns.push(withRestriction({ - from: from, - via: via, - to: {node: parent.nodes[index + 1], way: parent.id.split(/-(a|b)/)[0]} - })); - } - }); - - // U-turn - if (way.tags.oneway !== 'yes' && way.tags.oneway !== '-1') { + // backward + if (parent.first() !== vertex.id && parent.tags.oneway !== 'yes') { turns.push(withRestriction({ from: from, via: via, - to: from, - u: true + to: {node: parent.nodes[index - 1], way: parent.id.split(/-(a|b)/)[0]} + })); + } + + // forward + if (parent.last() !== vertex.id && parent.tags.oneway !== '-1') { + turns.push(withRestriction({ + from: from, + via: via, + to: {node: parent.nodes[index + 1], way: parent.id.split(/-(a|b)/)[0]} })); } }); + // U-turn + if (way.tags.oneway !== 'yes' && way.tags.oneway !== '-1') { + turns.push(withRestriction({ + from: from, + via: via, + to: from, + u: true + })); + } + return turns; }; diff --git a/js/id/ui/preset/restrictions.js b/js/id/ui/preset/restrictions.js index 10e3c4ed7..ef31b77aa 100644 --- a/js/id/ui/preset/restrictions.js +++ b/js/id/ui/preset/restrictions.js @@ -1,7 +1,7 @@ iD.ui.preset.restrictions = function(field, context) { var event = d3.dispatch('change'), vertexID, - selectedID; + fromNodeID; function restrictions(selection) { var wrap = selection.selectAll('.preset-input-wrap') @@ -44,7 +44,7 @@ iD.ui.preset.restrictions = function(field, context) { surface .call(vertices, graph, [vertex], filter, extent, z) .call(lines, graph, intersection.highways, filter) - .call(turns, graph, intersection.turns(selectedID)); + .call(turns, graph, intersection.turns(fromNodeID)); surface .on('click.restrictions', click) @@ -55,9 +55,9 @@ iD.ui.preset.restrictions = function(field, context) { .selectAll('.selected') .classed('selected', false); - if (selectedID) { + if (fromNodeID) { surface - .selectAll('.' + selectedID) + .selectAll('.' + _.find(intersection.highways, function(way) { return way.contains(fromNodeID); }).id) .classed('selected', true); } @@ -72,7 +72,7 @@ iD.ui.preset.restrictions = function(field, context) { function click() { var datum = d3.event.target.__data__; if (datum instanceof iD.Entity) { - selectedID = datum.id; + fromNodeID = datum.nodes[(datum.first() === vertexID) ? 1 : datum.nodes.length - 2]; render(); } else if (datum instanceof iD.geo.Turn) { if (datum.restriction) { @@ -81,17 +81,8 @@ iD.ui.preset.restrictions = function(field, context) { t('operations.restriction.annotation.delete')); } else { context.perform( - iD.actions.RestrictTurn(datum, projection) - .on('split', split), + iD.actions.RestrictTurn(datum, projection), t('operations.restriction.annotation.create')); - - function split(oldID, newID, graph) { - if (graph.entity(newID).contains(datum.from.node)) { - selectedID = newID; - } else if (graph.entity(oldID).contains(datum.from.node)) { - selectedID = oldID; - } - } } } } @@ -124,7 +115,7 @@ iD.ui.preset.restrictions = function(field, context) { function mouseout() { wrap.selectAll('.restriction-help') .text(t('operations.restriction.help.' + - (selectedID ? 'toggle' : 'select'))); + (fromNodeID ? 'toggle' : 'select'))); } function render() { @@ -134,7 +125,7 @@ iD.ui.preset.restrictions = function(field, context) { restrictions.entity = function(_) { if (!vertexID || vertexID !== _.id) { - selectedID = null; + fromNodeID = null; vertexID = _.id; } }; diff --git a/test/spec/actions/restrict_turn.js b/test/spec/actions/restrict_turn.js index a403a8a17..302f14cad 100644 --- a/test/spec/actions/restrict_turn.js +++ b/test/spec/actions/restrict_turn.js @@ -236,38 +236,4 @@ describe("iD.actions.RestrictTurn", function() { }, projection, 'r')(graph); expect(u.entity('r').tags.restriction).to.equal('no_u_turn'); }); - - it('emits split events', function() { - // x - // | - // u====*====w - // | - // y - var graph = iD.Graph([ - iD.Node({id: '*'}), - iD.Node({id: 'u'}), - iD.Node({id: 'w'}), - iD.Node({id: 'x'}), - iD.Node({id: 'y'}), - iD.Way({id: '=', nodes: ['u', '*', 'w']}), - iD.Way({id: '-', nodes: ['x', '*', 'y']}) - ]), - action = iD.actions.RestrictTurn({ - from: {node: 'u', way: '=', newID: '=='}, - via: {node: '*'}, - to: {node: 'x', way: '-', newID: '--'}, - restriction: 'no_left_turn' - }); - - var splits = []; - - action.on('split', function(a, b, graph) { - expect(graph).to.be.instanceOf(iD.Graph); - splits.push([a, b]); - }); - - action(graph); - - expect(splits).to.eql([['=', '=='], ['-', '--']]); - }); }); diff --git a/test/spec/geo/intersection.js b/test/spec/geo/intersection.js index bcef423b5..62170e601 100644 --- a/test/spec/geo/intersection.js +++ b/test/spec/geo/intersection.js @@ -63,7 +63,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['*', 'w'], tags: {highway: 'residential'}}) ]), - turns = iD.geo.Intersection(graph, '*').turns('='); + turns = iD.geo.Intersection(graph, '*').turns('u'); expect(turns.length).to.eql(2); expect(turns[0]).to.eql({ @@ -82,7 +82,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['w', '*'], tags: {highway: 'residential'}}) ]), - turns = iD.geo.Intersection(graph, '*').turns('='); + turns = iD.geo.Intersection(graph, '*').turns('u'); expect(turns.length).to.eql(2); expect(turns[0]).to.eql({ @@ -92,7 +92,7 @@ describe("iD.geo.Intersection", function() { }); }); - it("permits turns fom a way in both directions", function() { + it("permits turns from a way that must be split", function() { // w // | // u===* @@ -106,9 +106,9 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['w', '*', 'x'], tags: {highway: 'residential'}}) ]), - turns = iD.geo.Intersection(graph, '*').turns('-'); + turns = iD.geo.Intersection(graph, '*').turns('w'); - expect(turns.length).to.eql(6); + expect(turns.length).to.eql(3); expect(turns[0]).to.eql({ from: {node: 'w', way: '-'}, via: {node: '*'}, @@ -125,25 +125,9 @@ describe("iD.geo.Intersection", function() { to: {node: 'w', way: '-'}, u: true }); - expect(turns[3]).to.eql({ - from: {node: 'x', way: '-'}, - via: {node: '*'}, - to: {node: 'u', way: '='} - }); - expect(turns[4]).to.eql({ - from: {node: 'x', way: '-'}, - via: {node: '*'}, - to: {node: 'w', way: '-'} - }); - expect(turns[5]).to.eql({ - from: {node: 'x', way: '-'}, - via: {node: '*'}, - to: {node: 'x', way: '-'}, - u: true - }); }); - it("permits turns to a way in both directions", function() { + it("permits turns to a way that must be split", function() { // w // | // u===* @@ -157,7 +141,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['w', '*', 'x'], tags: {highway: 'residential'}}) ]), - turns = iD.geo.Intersection(graph, '*').turns('='); + turns = iD.geo.Intersection(graph, '*').turns('u'); expect(turns.length).to.eql(3); expect(turns[0]).to.eql({ @@ -170,6 +154,12 @@ describe("iD.geo.Intersection", function() { via: {node: '*'}, to: {node: 'x', way: '-'} }); + expect(turns[2]).to.eql({ + from: {node: 'u', way: '='}, + via: {node: '*'}, + to: {node: 'u', way: '='}, + u: true + }); }); it("permits turns from a oneway forward", function() { @@ -181,7 +171,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential', oneway: 'yes'}}), iD.Way({id: '-', nodes: ['*', 'w'], tags: {highway: 'residential'}}) ]), - turns = iD.geo.Intersection(graph, '*').turns('='); + turns = iD.geo.Intersection(graph, '*').turns('u'); expect(turns).to.eql([{ from: {node: 'u', way: '='}, @@ -199,7 +189,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['*', 'u'], tags: {highway: 'residential', oneway: '-1'}}), iD.Way({id: '-', nodes: ['*', 'w'], tags: {highway: 'residential'}}) ]), - turns = iD.geo.Intersection(graph, '*').turns('='); + turns = iD.geo.Intersection(graph, '*').turns('u'); expect(turns).to.eql([{ from: {node: 'u', way: '='}, @@ -217,7 +207,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['*', 'u'], tags: {highway: 'residential', oneway: 'yes'}}), iD.Way({id: '-', nodes: ['*', 'w'], tags: {highway: 'residential'}}) ]); - expect(iD.geo.Intersection(graph, '*').turns('=')).to.eql([]); + expect(iD.geo.Intersection(graph, '*').turns('u')).to.eql([]); }); it("omits turns from a reverse oneway forward", function() { @@ -229,7 +219,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential', oneway: '-1'}}), iD.Way({id: '-', nodes: ['*', 'w'], tags: {highway: 'residential'}}) ]); - expect(iD.geo.Intersection(graph, '*').turns('=')).to.eql([]); + expect(iD.geo.Intersection(graph, '*').turns('u')).to.eql([]); }); it("permits turns onto a oneway forward", function() { @@ -241,7 +231,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['*', 'w'], tags: {highway: 'residential', oneway: 'yes'}}) ]), - turns = iD.geo.Intersection(graph, '*').turns('='); + turns = iD.geo.Intersection(graph, '*').turns('u'); expect(turns.length).to.eql(2); expect(turns[0]).to.eql({ @@ -260,7 +250,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['w', '*'], tags: {highway: 'residential', oneway: '-1'}}) ]), - turns = iD.geo.Intersection(graph, '*').turns('='); + turns = iD.geo.Intersection(graph, '*').turns('u'); expect(turns.length).to.eql(2); expect(turns[0]).to.eql({ @@ -279,7 +269,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['w', '*'], tags: {highway: 'residential', oneway: 'yes'}}) ]); - expect(iD.geo.Intersection(graph, '*').turns('=').length).to.eql(1); + expect(iD.geo.Intersection(graph, '*').turns('u').length).to.eql(1); }); it("omits turns onto a reverse oneway forward", function() { @@ -291,7 +281,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['*', 'w'], tags: {highway: 'residential', oneway: '-1'}}) ]); - expect(iD.geo.Intersection(graph, '*').turns('=').length).to.eql(1); + expect(iD.geo.Intersection(graph, '*').turns('u').length).to.eql(1); }); it("includes U-turns", function() { @@ -303,7 +293,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['*', 'w'], tags: {highway: 'residential'}}) ]), - turns = iD.geo.Intersection(graph, '*').turns('='); + turns = iD.geo.Intersection(graph, '*').turns('u'); expect(turns.length).to.eql(2); expect(turns[1]).to.eql({ @@ -328,7 +318,7 @@ describe("iD.geo.Intersection", function() { {id: '*', role: 'via', type: 'node'} ]}) ]), - turns = iD.geo.Intersection(graph, '*').turns('='); + turns = iD.geo.Intersection(graph, '*').turns('u'); expect(turns.length).to.eql(2); expect(turns[0]).to.eql({