From 6881205d43a56a6446e55182191a75c4ae23a78a Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Mon, 1 Jan 2018 22:37:10 -0500
Subject: [PATCH] All touch targets are GeoJSON now

This makes the code a bit more consistent and lets us avoid some
hacky and probably non-performant things:
- abusing CSS classes in the draw/drag datum functions (classed `.target`)
  (is this thing target? just check d.properties)
- regexing the id for `-nope$`
  (is this thing a nope target? just check d.properties)
- using context.hasEntity to get a the real entity
  (is this thing a real osmEntity? just check d.properties)
- fixes code like the restriction editor which uses fake ids for split ways
---
 css/20_map.css                    |  1 +
 modules/behavior/drag.js          | 15 +++--------
 modules/behavior/draw.js          |  9 ++++---
 modules/behavior/draw_way.js      | 12 ++++-----
 modules/behavior/hover.js         | 16 +++++------
 modules/behavior/select.js        |  2 +-
 modules/modes/drag_node.js        | 22 +++++++++------
 modules/svg/areas.js              |  4 +--
 modules/svg/helpers.js            | 45 +++++++++++++++++++------------
 modules/svg/lines.js              |  4 +--
 modules/svg/midpoints.js          | 23 +++++++++++++---
 modules/svg/points.js             | 24 +++++++++++++----
 modules/svg/vertices.js           | 37 +++++++++++++++++--------
 modules/ui/fields/restrictions.js |  4 +++
 14 files changed, 140 insertions(+), 78 deletions(-)

diff --git a/css/20_map.css b/css/20_map.css
index 0c0905799..d162eef12 100644
--- a/css/20_map.css
+++ b/css/20_map.css
@@ -240,6 +240,7 @@ g.turn circle {
 
 .form-field-restrictions .vertex {
     cursor: auto !important;
+    pointer-events: none;
 }
 
 .lasso #map {
diff --git a/modules/behavior/drag.js b/modules/behavior/drag.js
index b9ea9c0af..f32c952ce 100644
--- a/modules/behavior/drag.js
+++ b/modules/behavior/drag.js
@@ -36,7 +36,6 @@ export function behaviorDrag() {
     var dispatch = d3_dispatch('start', 'move', 'end');
     var _origin = null;
     var _selector = '';
-    var _filter = null;
     var _event;
     var _target;
     var _surface;
@@ -162,9 +161,10 @@ export function behaviorDrag() {
                 var root = this;
                 var target = d3_event.target;
                 for (; target && target !== root; target = target.parentNode) {
-                    if (target[matchesSelector](_selector) &&
-                            (!_filter || _filter(target.__data__))) {
-                        return dragstart.call(target, target.__data__);
+                    var datum = target.__data__;
+                    var entity = datum && datum.properties && datum.properties.entity;
+                    if (entity && target[matchesSelector](_selector)) {
+                        return dragstart.call(target, entity);
                     }
                 }
             };
@@ -190,13 +190,6 @@ export function behaviorDrag() {
     };
 
 
-    drag.filter = function(_) {
-        if (!arguments.length) return _filter;
-        _filter = _;
-        return drag;
-    };
-
-
     drag.origin = function (_) {
         if (!arguments.length) return _origin;
         _origin = _;
diff --git a/modules/behavior/draw.js b/modules/behavior/draw.js
index 77315e03b..5866d3c26 100644
--- a/modules/behavior/draw.js
+++ b/modules/behavior/draw.js
@@ -44,6 +44,8 @@ export function behaviorDraw(context) {
     var _lastMouse = null;
 
 
+    // related code
+    // - `mode/drag_node.js` `datum()`
     function datum() {
         if (d3_event.altKey) return {};
 
@@ -54,11 +56,10 @@ export function behaviorDraw(context) {
             element = d3_event.target;
         }
 
-        // When drawing, connect only to things classed as targets..
+        // When drawing, snap only to touch targets..
         // (this excludes area fills and active drawing elements)
-        var selection = d3_select(element);
-        if (!selection.classed('target')) return {};
-        return selection.datum();
+        var d = element.__data__;
+        return (d && d.properties && d.properties.target) ? d : {};
     }
 
 
diff --git a/modules/behavior/draw_way.js b/modules/behavior/draw_way.js
index 4a49856e9..6c43c6ea9 100644
--- a/modules/behavior/draw_way.js
+++ b/modules/behavior/draw_way.js
@@ -39,13 +39,14 @@ export function behaviorDrawWay(context, wayId, index, mode, startGraph) {
     // - `behavior/draw.js`      `click()`
     // - `behavior/draw_way.js`  `move()`
     function move(datum) {
+        var nodeLoc = datum && datum.properties && datum.properties.entity && datum.properties.entity.loc;
         var nodeGroups = datum && datum.properties && datum.properties.nodes;
         var loc = context.map().mouseCoordinates();
 
-        if (datum.loc) {   // snap to node/vertex - a real entity or a nope target with a `loc`
-            loc = datum.loc;
+        if (nodeLoc) {   // snap to node/vertex - a point target with `.loc`
+            loc = nodeLoc;
 
-        } else if (nodeGroups) {   // snap to way - a line touch target or nope target with nodes
+        } else if (nodeGroups) {   // snap to way - a line target with `.nodes`
             var best = Infinity;
             for (var i = 0; i < nodeGroups.length; i++) {
                 var childNodes = nodeGroups[i].map(function(id) { return context.entity(id); });
@@ -169,9 +170,8 @@ export function behaviorDrawWay(context, wayId, index, mode, startGraph) {
 
 
     // Accept the current position of the drawing node and continue drawing.
-    drawWay.add = function(loc, datum) {
-        if ((datum && datum.id && /-nope$/.test(datum.id)) ||
-            context.surface().classed('nope')) {
+    drawWay.add = function(loc, d) {
+        if ((d && d.properties && d.properties.nope) || context.surface().classed('nope')) {
             return;   // can't click here
         }
 
diff --git a/modules/behavior/hover.js b/modules/behavior/hover.js
index 1a496ee2d..1dbe142d3 100644
--- a/modules/behavior/hover.js
+++ b/modules/behavior/hover.js
@@ -71,15 +71,15 @@ export function behaviorHover(context) {
 
         function mouseover() {
             if (_buttonDown) return;
-            var _target = d3_event.target;
-            enter(_target ? _target.__data__ : null);
+            var target = d3_event.target;
+            enter(target ? target.__data__ : null);
         }
 
 
         function mouseout() {
             if (_buttonDown) return;
-            var _target = d3_event.relatedTarget;
-            enter(_target ? _target.__data__ : null);
+            var target = d3_event.relatedTarget;
+            enter(target ? target.__data__ : null);
         }
 
 
@@ -97,16 +97,16 @@ export function behaviorHover(context) {
         }
 
 
-        function enter(d) {
-            if (d === _target) return;
-            _target = d;
+        function enter(datum) {
+            if (datum === _target) return;
+            _target = datum;
 
             _selection.selectAll('.hover')
                 .classed('hover', false);
             _selection.selectAll('.hover-suppressed')
                 .classed('hover-suppressed', false);
 
-            var entity = _target && _target.id && context.hasEntity(_target.id);
+            var entity = datum && datum.properties && datum.properties.entity;
             if (entity && entity.id !== _newId) {
 
                 // If drawing a way, don't hover on a node that was just placed. #3974
diff --git a/modules/behavior/select.js b/modules/behavior/select.js
index 98799e625..9a9c0b34d 100644
--- a/modules/behavior/select.js
+++ b/modules/behavior/select.js
@@ -115,7 +115,7 @@ export function behaviorSelect(context) {
         var datum = d3_event.target.__data__ || (lastMouse && lastMouse.target.__data__);
         var mode = context.mode();
 
-        var entity = datum && datum.id && context.hasEntity(datum.id);
+        var entity = datum && datum.properties && datum.properties.entity;
         if (entity) datum = entity;
 
         if (datum && datum.type === 'midpoint') {
diff --git a/modules/modes/drag_node.js b/modules/modes/drag_node.js
index fff0f08a0..faf603f66 100644
--- a/modules/modes/drag_node.js
+++ b/modules/modes/drag_node.js
@@ -100,7 +100,7 @@ export function modeDragNode(context) {
             var midpoint = entity;
             entity = osmNode();
             context.perform(actionAddMidpoint(midpoint, entity));
-            entity = context.entity(entity.id);  // get post-action entity
+            entity = context.entity(entity.id);   // get post-action entity
 
             var vertex = context.surface().selectAll('.' + entity.id);
             drag.target(vertex.node(), entity);
@@ -119,12 +119,17 @@ export function modeDragNode(context) {
     }
 
 
+    // related code
+    // - `behavior/draw.js` `datum()`
     function datum() {
         var event = d3_event && d3_event.sourceEvent;
-        if (!event || event.altKey || !d3_select(event.target).classed('target')) {
+        if (!event || event.altKey) {
             return {};
         } else {
-            return event.target.__data__ || {};
+            // When dragging, snap only to touch targets..
+            // (this excludes area fills and active drawing elements)
+            var d = event.target.__data__;
+            return (d && d.properties && d.properties.target) ? d : {};
         }
     }
 
@@ -142,12 +147,13 @@ export function modeDragNode(context) {
             // - `behavior/draw.js`      `click()`
             // - `behavior/draw_way.js`  `move()`
             var d = datum();
+            var nodeLoc = d && d.properties && d.properties.entity && d.properties.entity.loc;
             var nodeGroups = d && d.properties && d.properties.nodes;
 
-            if (d.loc) {    // snap to node/vertex - a real entity or a nope target with a `loc`
-                loc = d.loc;
+            if (nodeLoc) {    // snap to node/vertex - a point target with `.loc`
+                loc = nodeLoc;
 
-            } else if (nodeGroups) {   // snap to way - a line touch target or nope target with nodes
+            } else if (nodeGroups) {   // snap to way - a line target with `.nodes`
                 var best = Infinity;
                 for (var i = 0; i < nodeGroups.length; i++) {
                     var childNodes = nodeGroups[i].map(function(id) { return context.entity(id); });
@@ -250,8 +256,8 @@ export function modeDragNode(context) {
         if (_isCancelled) return;
 
         var d = datum();
-        var nope = (d && d.id && /-nope$/.test(d.id)) || context.surface().classed('nope');
-        var target = d && d.id && context.hasEntity(d.id);   // entity to snap to
+        var nope = (d && d.properties && d.properties.nope) || context.surface().classed('nope');
+        var target = d && d.properties && d.properties.entity;   // entity to snap to
 
         if (nope) {   // bounce back
             context.perform(
diff --git a/modules/svg/areas.js b/modules/svg/areas.js
index 90f35eb57..8826ddcad 100644
--- a/modules/svg/areas.js
+++ b/modules/svg/areas.js
@@ -59,7 +59,7 @@ export function svgAreas(projection, context) {
 
         // Targets allow hover and vertex snapping
         var targets = selection.selectAll('.area.target-allowed')
-            .filter(filter)
+            .filter(function(d) { return filter(d.properties.entity); })
             .data(data.targets, function key(d) { return d.id; });
 
         // exit
@@ -76,7 +76,7 @@ export function svgAreas(projection, context) {
 
         // NOPE
         var nopes = selection.selectAll('.area.target-nope')
-            .filter(function(d) { return filter({ id: d.properties.originalID }); })
+            .filter(function(d) { return filter(d.properties.entity); })
             .data(data.nopes, function key(d) { return d.id; });
 
         // exit
diff --git a/modules/svg/helpers.js b/modules/svg/helpers.js
index 40f28d0a6..3ca51ea8d 100644
--- a/modules/svg/helpers.js
+++ b/modules/svg/helpers.js
@@ -155,11 +155,17 @@ export function svgPath(projection, graph, isArea) {
 
 
 export function svgPointTransform(projection) {
-    return function(entity) {
+    var svgpoint = function(entity) {
         // http://jsperf.com/short-array-join
         var pt = projection(entity.loc);
         return 'translate(' + pt[0] + ',' + pt[1] + ')';
     };
+
+    svgpoint.geojson = function(d) {
+        return svgpoint(d.properties.entity);
+    };
+
+    return svgpoint;
 }
 
 
@@ -184,7 +190,7 @@ export function svgSegmentWay(way, graph, activeID) {
     var coords = [];
     var nodes = [];
     var startType = null;   // 0 = active, 1 = passive, 2 = adjacent
-    var currType = null;
+    var currType = null;    // 0 = active, 1 = passive, 2 = adjacent
     var node;
 
     for (var i = 0; i < way.nodes.length; i++) {
@@ -244,29 +250,34 @@ export function svgSegmentWay(way, graph, activeID) {
 
     if (coordGroups.passive.length) {
         features.passive.push({
-            'type': 'Feature',
-            'id': way.id,
-            'properties': {
-                'nodes': nodeGroups.passive
+            type: 'Feature',
+            id: way.id,
+            properties: {
+                target: true,
+                entity: way,
+                nodes: nodeGroups.passive
             },
-            'geometry': {
-                'type': 'MultiLineString',
-                'coordinates': coordGroups.passive
+            geometry: {
+                type: 'MultiLineString',
+                coordinates: coordGroups.passive
             }
         });
     }
 
     if (coordGroups.active.length) {
         features.active.push({
-            'type': 'Feature',
-            'id': way.id + '-nope',   // break the ids on purpose
-            'properties': {
-                'originalID': way.id,
-                'nodes': nodeGroups.active
+            type: 'Feature',
+            id: way.id + '-nope',   // break the ids on purpose
+            properties: {
+                target: true,
+                entity: way,
+                nodes: nodeGroups.active,
+                nope: true,
+                originalID: way.id
             },
-            'geometry': {
-                'type': 'MultiLineString',
-                'coordinates': coordGroups.active
+            geometry: {
+                type: 'MultiLineString',
+                coordinates: coordGroups.active
             }
         });
     }
diff --git a/modules/svg/lines.js b/modules/svg/lines.js
index 67e433ab8..727d0178b 100644
--- a/modules/svg/lines.js
+++ b/modules/svg/lines.js
@@ -55,7 +55,7 @@ export function svgLines(projection, context) {
 
         // Targets allow hover and vertex snapping
         var targets = selection.selectAll('.line.target-allowed')
-            .filter(filter)
+            .filter(function(d) { return filter(d.properties.entity); })
             .data(data.targets, function key(d) { return d.id; });
 
         // exit
@@ -72,7 +72,7 @@ export function svgLines(projection, context) {
 
         // NOPE
         var nopes = selection.selectAll('.line.target-nope')
-            .filter(function(d) { return filter({ id: d.properties.originalID }); })
+            .filter(function(d) { return filter(d.properties.entity); })
             .data(data.nopes, function key(d) { return d.id; });
 
         // exit
diff --git a/modules/svg/midpoints.js b/modules/svg/midpoints.js
index bcf9495b2..0f68d949c 100644
--- a/modules/svg/midpoints.js
+++ b/modules/svg/midpoints.js
@@ -18,9 +18,26 @@ export function svgMidpoints(projection, context) {
 
     function drawTargets(selection, graph, entities, filter) {
         var fillClass = context.getDebug('target') ? 'pink ' : 'nocolor ';
+        var getTransform = svgPointTransform(projection).geojson;
+
+        var data = entities.map(function(midpoint) {
+            return {
+                type: 'Feature',
+                id: midpoint.id,
+                properties: {
+                    target: true,
+                    entity: midpoint
+                },
+                geometry: {
+                    type: 'Point',
+                    coordinates: midpoint.loc
+                }
+            };
+        });
+
         var targets = selection.selectAll('.midpoint.target')
-            .filter(filter)
-            .data(entities, function key(d) { return d.id; });
+            .filter(function(d) { return filter(d.properties.entity); })
+            .data(data, function key(d) { return d.id; });
 
         // exit
         targets.exit()
@@ -32,7 +49,7 @@ export function svgMidpoints(projection, context) {
             .attr('r', targetRadius)
             .merge(targets)
             .attr('class', function(d) { return 'node midpoint target ' + fillClass + d.id; })
-            .attr('transform', svgPointTransform(projection));
+            .attr('transform', getTransform);
     }
 
 
diff --git a/modules/svg/points.js b/modules/svg/points.js
index 0b3366ab3..c2f1fc0eb 100644
--- a/modules/svg/points.js
+++ b/modules/svg/points.js
@@ -29,13 +29,27 @@ export function svgPoints(projection, context) {
 
     function drawTargets(selection, graph, entities, filter) {
         var fillClass = context.getDebug('target') ? 'pink ' : 'nocolor ';
-        var passive = entities.filter(function(d) {
-            return d.id !== context.activeID();
+        var getTransform = svgPointTransform(projection).geojson;
+        var activeID = context.activeID();
+        var data = [];
+
+        entities.forEach(function(node) {
+            if (activeID === node.id) return;   // draw no target on the activeID
+
+            data.push({
+                type: 'Feature',
+                id: node.id,
+                properties: {
+                    target: true,
+                    entity: node
+                },
+                geometry: node.asGeoJSON()
+            });
         });
 
         var targets = selection.selectAll('.point.target')
-            .filter(filter)
-            .data(passive, function key(d) { return d.id; });
+            .filter(function(d) { return filter(d.properties.entity); })
+            .data(data, function key(d) { return d.id; });
 
         // exit
         targets.exit()
@@ -50,7 +64,7 @@ export function svgPoints(projection, context) {
             .attr('height', 30)
             .merge(targets)
             .attr('class', function(d) { return 'node point target ' + fillClass + d.id; })
-            .attr('transform', svgPointTransform(projection));
+            .attr('transform', getTransform);
     }
 
 
diff --git a/modules/svg/vertices.js b/modules/svg/vertices.js
index 1a5ed0a88..c6843a3af 100644
--- a/modules/svg/vertices.js
+++ b/modules/svg/vertices.js
@@ -184,20 +184,35 @@ export function svgVertices(projection, context) {
     function drawTargets(selection, graph, entities, filter) {
         var targetClass = context.getDebug('target') ? 'pink ' : 'nocolor ';
         var nopeClass = context.getDebug('target') ? 'red ' : 'nocolor ';
+        var getTransform = svgPointTransform(projection).geojson;
         var activeID = context.activeID();
         var data = { targets: [], nopes: [] };
 
         entities.forEach(function(node) {
             if (activeID === node.id) return;   // draw no target on the activeID
 
-            var currType = svgPassiveVertex(node, graph, activeID);
-            if (currType !== 0) {
-                data.targets.push(node);     // passive or adjacent - allow to connect
+            var vertexType = svgPassiveVertex(node, graph, activeID);
+            if (vertexType !== 0) {              // passive or adjacent - allow to connect
+                data.targets.push({
+                    type: 'Feature',
+                    id: node.id,
+                    properties: {
+                        target: true,
+                        entity: node
+                    },
+                    geometry: node.asGeoJSON()
+                });
             } else {
                 data.nopes.push({
-                    id: node.id + '-nope',   // not a real osmNode, break the id on purpose
-                    originalID: node.id,
-                    loc: node.loc
+                    type: 'Feature',
+                    id: node.id + '-nope',   // break the ids on purpose
+                    properties: {
+                        target: true,
+                        entity: node,
+                        nope: true,
+                        originalID: node.id
+                    },
+                    geometry: node.asGeoJSON()
                 });
             }
         });
@@ -205,7 +220,7 @@ export function svgVertices(projection, context) {
 
         // Targets allow hover and vertex snapping
         var targets = selection.selectAll('.vertex.target-allowed')
-            .filter(filter)
+            .filter(function(d) { return filter(d.properties.entity); })
             .data(data.targets, function key(d) { return d.id; });
 
         // exit
@@ -218,12 +233,12 @@ export function svgVertices(projection, context) {
             .attr('r', function(d) { return (_radii[d.id] || radiuses.shadow[3]); })
             .merge(targets)
             .attr('class', function(d) { return 'node vertex target target-allowed ' + targetClass + d.id; })
-            .attr('transform', svgPointTransform(projection));
+            .attr('transform', getTransform);
 
 
         // NOPE
         var nopes = selection.selectAll('.vertex.target-nope')
-            .filter(function(d) { return filter({ id: d.originalID }); })
+            .filter(function(d) { return filter(d.properties.entity); })
             .data(data.nopes, function key(d) { return d.id; });
 
         // exit
@@ -233,10 +248,10 @@ export function svgVertices(projection, context) {
         // enter/update
         nopes.enter()
             .append('circle')
-            .attr('r', function(d) { return (_radii[d.id.replace('-nope','')] || radiuses.shadow[3]); })
+            .attr('r', function(d) { return (_radii[d.properties.originalID] || radiuses.shadow[3]); })
             .merge(nopes)
             .attr('class', function(d) { return 'node vertex target target-nope ' + nopeClass + d.id; })
-            .attr('transform', svgPointTransform(projection));
+            .attr('transform', getTransform);
     }
 
 
diff --git a/modules/ui/fields/restrictions.js b/modules/ui/fields/restrictions.js
index f56bc868d..dde654d45 100644
--- a/modules/ui/fields/restrictions.js
+++ b/modules/ui/fields/restrictions.js
@@ -154,9 +154,13 @@ export function uiFieldRestrictions(field, context) {
                 .call(breathe);
 
             var datum = d3_event.target.__data__;
+            var entity = datum && datum.properties && datum.properties.entity;
+            if (entity) datum = entity;
+
             if (datum instanceof osmEntity) {
                 fromNodeID = intersection.adjacentNodeId(datum.id);
                 render();
+
             } else if (datum instanceof osmTurn) {
                 if (datum.restriction) {
                     context.perform(