From 6780ecb75b0c578233c4a233c8b46b59c1751f21 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 4 Dec 2012 14:55:32 -0500 Subject: [PATCH 1/4] Add whitespace, cleanup, fix global leaks --- js/id/actions/modes.js | 48 ++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/js/id/actions/modes.js b/js/id/actions/modes.js index b29860ebc..4f8166e22 100644 --- a/js/id/actions/modes.js +++ b/js/id/actions/modes.js @@ -12,6 +12,7 @@ iD.modes._node = function(ll) { iD.modes.AddPlace = { id: 'add-place', title: '+ Place', + enter: function() { var surface = this.map.surface; @@ -32,6 +33,7 @@ iD.modes.AddPlace = { this.controller.exit(); }.bind(this)); }, + exit: function() { this.map.surface .on('click.addplace', null); @@ -44,6 +46,7 @@ iD.modes.AddPlace = { iD.modes.AddRoad = { id: 'add-road', title: '+ Road', + enter: function() { this.map.dblclickEnable(false); var surface = this.map.surface; @@ -90,6 +93,7 @@ iD.modes.AddRoad = { this.history.perform(iD.actions.addWayNode(way, node)); console.log(this.history.graph().entities); } + this.controller.enter(iD.modes.DrawRoad(way.id, direction)); } @@ -112,18 +116,18 @@ iD.modes.AddRoad = { iD.modes.DrawRoad = function(way_id, direction) { return { enter: function() { - var push = (direction === 'forward') ? 'push' : 'unshift', - pop = (direction === 'forward') ? 'pop' : 'shift'; this.map.dblclickEnable(false); this.map.dragEnable(false); - var surface = this.map.surface, - nextnode = iD.modes._node([NaN, NaN]); - var nextnode_id = nextnode.id; - - var way = this.history.graph().entity(way_id), + var push = (direction === 'forward') ? 'push' : 'unshift', + pop = (direction === 'forward') ? 'pop' : 'shift', + surface = this.map.surface, + nextnode = iD.modes._node([NaN, NaN]), + nextnode_id = nextnode.id, + way = this.history.graph().entity(way_id), firstNode = way.nodes[0], lastNode = _.last(way.nodes); + way.nodes[push](nextnode_id); this.history.perform(iD.actions.addWayNode(way, nextnode)); @@ -137,12 +141,16 @@ iD.modes.DrawRoad = function(way_id, direction) { } function click() { - var t = d3.select(d3.event.target); d3.event.stopPropagation(); + + var node, + t = d3.select(d3.event.target); + if (t.data() && t.data()[0] && t.data()[0].type === 'node') { if (t.data()[0].id == firstNode || t.data()[0].id == lastNode) { var l = this.history.graph().entity(way.nodes[pop]()); this.history.perform(iD.actions.removeWayNode(way, l)); + // If this is drawing a loop and this is not the drawing // end of the stick, finish the circle if (direction === 'forward' && t.data()[0].id == firstNode) { @@ -154,9 +162,11 @@ iD.modes.DrawRoad = function(way_id, direction) { this.history.perform(iD.actions.addWayNode(way, this.history.graph().entity(lastNode))); } + delete way.tags.elastic; this.history.perform(iD.actions.changeTags(way, way.tags)); this.map.selectEntity(way); + // End by clicking on own tail return this.controller.exit(); } else { @@ -174,11 +184,14 @@ iD.modes.DrawRoad = function(way_id, direction) { node = iD.modes._node(this.map.projection.invert( d3.mouse(surface.node()))); } + var old = this.history.graph().entity(way.nodes[pop]()); this.history.perform(iD.actions.removeWayNode(way, old)); + way.nodes[push](node.id); this.history.perform(iD.actions.addWayNode(way, node)); way.nodes = way.nodes.slice(); + this.controller.enter(iD.modes.DrawRoad(way_id, direction)); } @@ -189,6 +202,7 @@ iD.modes.DrawRoad = function(way_id, direction) { this.controller.exit(); }.bind(this)); }, + exit: function() { this.map.surface.on('mousemove.drawroad', null) .on('click.drawroad', null); @@ -204,11 +218,13 @@ iD.modes.DrawRoad = function(way_id, direction) { iD.modes.AddArea = { id: 'add-area', title: '+ Area', + way: function() { return iD.Way({ tags: { building: 'yes', area: 'yes', elastic: 'true' } }); }, + enter: function() { this.map.dblclickEnable(false); @@ -252,6 +268,7 @@ iD.modes.AddArea = { this.controller.exit(); }.bind(this)); }, + exit: function() { window.setTimeout(function() { this.map.dblclickEnable(true); @@ -267,11 +284,10 @@ iD.modes.DrawArea = function(way_id) { enter: function() { this.map.dblclickEnable(false); - nextnode = iD.modes._node([NaN, NaN]); - var surface = this.map.surface, way = this.history.graph().entity(way_id), firstnode_id = _.first(way.nodes), + nextnode = iD.modes._node([NaN, NaN]), nextnode_id = nextnode.id; way.nodes.push(nextnode_id); @@ -288,17 +304,23 @@ iD.modes.DrawArea = function(way_id) { } function click() { - var t = d3.select(d3.event.target); d3.event.stopPropagation(); + + var node, + t = d3.select(d3.event.target); + if (t.data() && t.data()[0] && t.data()[0].type === 'node') { if (t.data()[0].id == firstnode_id) { var l = this.history.graph().entity(way.nodes.pop()); this.history.perform(iD.actions.removeWayNode(way, l)); + way.nodes.push(way.nodes[0]); this.history.perform(iD.actions.addWayNode(way, this.history.graph().entity(way.nodes[0]))); + delete way.tags.elastic; this.history.perform(iD.actions.changeTags(way, way.tags)); + // End by clicking on own tail return this.controller.exit(); } else { @@ -309,11 +331,14 @@ iD.modes.DrawArea = function(way_id) { node = iD.modes._node(this.map.projection.invert( d3.mouse(surface.node()))); } + var old = this.history.graph().entity(way.nodes.pop()); this.history.perform(iD.actions.removeWayNode(way, old)); + way.nodes.push(node.id); this.history.perform(iD.actions.addWayNode(way, node)); way.nodes = way.nodes.slice(); + this.controller.enter(iD.modes.DrawArea(way_id)); } @@ -324,6 +349,7 @@ iD.modes.DrawArea = function(way_id) { surface.on('click.drawarea', click.bind(this)) .on('mousemove.drawarea', mousemove.bind(this)); }, + exit: function() { this.map.surface.on('mousemove.drawarea', null) .on('click.drawarea', null); From 12d02e0a6bf79f7b6bf2dec5011521f7c04d6cd2 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 4 Dec 2012 16:57:52 -0500 Subject: [PATCH 2/4] Copy entity in Graph#fetch This shouldn't be necessary, but someone is modifying them in place and it's causing problems elsewhere. --- js/id/graph/graph.js | 2 +- test/spec/graph/graph.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/js/id/graph/graph.js b/js/id/graph/graph.js index 1410dea41..5ef7daef1 100644 --- a/js/id/graph/graph.js +++ b/js/id/graph/graph.js @@ -89,7 +89,7 @@ iD.Graph.prototype = { // Resolve the id references in a way, replacing them with actual objects. fetch: function(id) { var entity = this.entities[id], nodes = []; - if (!entity.nodes || !entity.nodes.length) return entity; + if (!entity.nodes || !entity.nodes.length) return iD.Entity(entity); // TODO: shouldn't be necessary for (var i = 0, l = entity.nodes.length; i < l; i++) { nodes[i] = this.fetch(entity.nodes[i]); } diff --git a/test/spec/graph/graph.js b/test/spec/graph/graph.js index b026a8be3..487bdd0f7 100644 --- a/test/spec/graph/graph.js +++ b/test/spec/graph/graph.js @@ -59,7 +59,7 @@ describe('Graph', function() { var node = iD.Node({id: "n1"}), way = iD.Way({id: "w1", nodes: ["n1"]}), graph = iD.Graph({n1: node, w1: way}); - expect(graph.fetch("w1").nodes).to.eql([node]); + expect(graph.fetch("w1").nodes[0].id).to.equal("n1"); }); }); From a3165440089c3cf0cbcb4988750fc3aea57d62e6 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 4 Dec 2012 16:58:54 -0500 Subject: [PATCH 3/4] Use datum() --- js/id/actions/modes.js | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/js/id/actions/modes.js b/js/id/actions/modes.js index 4f8166e22..f95ca931a 100644 --- a/js/id/actions/modes.js +++ b/js/id/actions/modes.js @@ -61,9 +61,9 @@ iD.modes.AddRoad = { way = iD.Way({ tags: { highway: 'residential', elastic: 'true' } }); // connect a way to an existing way - if (t.data() && t.data()[0] && t.data()[0].type === 'node') { + if (t.datum() && t.datum().type === 'node') { // continue an existing way - var id = t.data()[0].id; + var id = t.datum().id; var parents = this.history.graph().parents(id); if (parents.length && parents[0].nodes[0] === id) { way = parents[0]; @@ -73,13 +73,13 @@ iD.modes.AddRoad = { way = parents[0]; start = false; } - node = t.data()[0]; + node = t.datum(); // snap into an existing way - } else if (t.data() && t.data()[0] && t.data()[0].type === 'way') { - var index = iD.util.geo.chooseIndex(t.data()[0], d3.mouse(surface.node()), this.map); + } else if (t.data() && t.datum() && t.datum().type === 'way') { + var index = iD.util.geo.chooseIndex(t.datum(), d3.mouse(surface.node()), this.map); node = iD.modes._node(this.map.projection.invert( d3.mouse(surface.node()))); - var connectedWay = this.history.graph().entity(t.data()[0].id); + var connectedWay = this.history.graph().entity(t.datum().id); connectedWay.nodes.splice(index, 0, node.id); this.history.perform(iD.actions.addWayNode(connectedWay, node)); } else { @@ -146,18 +146,18 @@ iD.modes.DrawRoad = function(way_id, direction) { var node, t = d3.select(d3.event.target); - if (t.data() && t.data()[0] && t.data()[0].type === 'node') { - if (t.data()[0].id == firstNode || t.data()[0].id == lastNode) { + if (t.datum() && t.datum().type === 'node') { + if (t.datum().id == firstNode || t.datum().id == lastNode) { var l = this.history.graph().entity(way.nodes[pop]()); this.history.perform(iD.actions.removeWayNode(way, l)); // If this is drawing a loop and this is not the drawing // end of the stick, finish the circle - if (direction === 'forward' && t.data()[0].id == firstNode) { + if (direction === 'forward' && t.datum().id == firstNode) { way.nodes[push](firstNode); this.history.perform(iD.actions.addWayNode(way, this.history.graph().entity(firstNode))); - } else if (direction === 'backward' && t.data()[0].id == lastNode) { + } else if (direction === 'backward' && t.datum().id == lastNode) { way.nodes[push](lastNode); this.history.perform(iD.actions.addWayNode(way, this.history.graph().entity(lastNode))); @@ -171,13 +171,13 @@ iD.modes.DrawRoad = function(way_id, direction) { return this.controller.exit(); } else { // connect a way to an existing way - node = t.data()[0]; + node = t.datum(); } - } else if (t.data() && t.data()[0] && t.data()[0].type === 'way') { - var index = iD.modes.chooseIndex(t.data()[0], d3.mouse(surface.node()), this.map); + } else if (t.datum() && t.datum().type === 'way') { + var index = iD.modes.chooseIndex(t.datum(), d3.mouse(surface.node()), this.map); node = iD.modes._node(this.map.projection.invert( d3.mouse(surface.node()))); - var connectedWay = this.history.graph().entity(t.data()[0].id); + var connectedWay = this.history.graph().entity(t.datum().id); connectedWay.nodes.splice(1, 0, node.id); this.history.perform(iD.actions.addWayNode(connectedWay, node)); } else { @@ -248,8 +248,8 @@ iD.modes.AddArea = { node, way = this.way(); // connect a way to an existing way - if (t.data() && t.data()[0] && t.data()[0].type === 'node') { - node = t.data()[0]; + if (t.datum() && t.datum().type === 'node') { + node = t.datum(); } else { node = iD.modes._node(this.map.projection.invert( d3.mouse(surface.node()))); @@ -309,8 +309,8 @@ iD.modes.DrawArea = function(way_id) { var node, t = d3.select(d3.event.target); - if (t.data() && t.data()[0] && t.data()[0].type === 'node') { - if (t.data()[0].id == firstnode_id) { + if (t.datum() && t.datum().type === 'node') { + if (t.datum().id == firstnode_id) { var l = this.history.graph().entity(way.nodes.pop()); this.history.perform(iD.actions.removeWayNode(way, l)); @@ -325,7 +325,7 @@ iD.modes.DrawArea = function(way_id) { return this.controller.exit(); } else { // connect a way to an existing way - node = t.data()[0]; + node = t.datum(); } } else { node = iD.modes._node(this.map.projection.invert( From 36ea03044025f33a40cd7785c4e8faa5a8ad60e0 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 4 Dec 2012 17:31:09 -0500 Subject: [PATCH 4/4] Don't change graph in origin() --- js/id/renderer/map.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index bbcf9d4c8..2226a3c3a 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -19,13 +19,6 @@ iD.Map = function() { dragbehavior = d3.behavior.drag() .origin(function(entity) { if (!dragEnabled) return { x: 0, y: 0 }; - if (entity.accuracy) { - var index = entity.index, wayid = entity.way; - entity = iD.Node(entity); - var connectedWay = history.graph().entity(wayid); - connectedWay.nodes.splice(index, 0, entity.id); - map.perform(iD.actions.addWayNode(connectedWay, entity)); - } var p = projection(ll2a(entity)); return { x: p[0], y: p[1] }; }) @@ -33,6 +26,14 @@ iD.Map = function() { d3.event.sourceEvent.stopPropagation(); if (!dragging) { + if (entity.accuracy) { + var index = entity.index, wayid = entity.way; + entity = iD.Node(entity); + var connectedWay = history.graph().entity(wayid); + connectedWay.nodes.splice(index, 0, entity.id); + history.perform(iD.actions.addWayNode(connectedWay, entity)); + } + dragging = iD.util.trueObj([entity.id].concat( _.pluck(history.graph().parents(entity.id), 'id'))); history.perform(iD.actions.noop());