From 7a8f50c74e54e6749fc0f4c64e0644657044a0ef Mon Sep 17 00:00:00 2001
From: Bryan Housel <bryan@mapbox.com>
Date: Mon, 18 Dec 2017 22:54:49 -0500
Subject: [PATCH] More fixes for drawing/snapping, don't draw touch targets for
 activeIDs

---
 css/20_map.css               |  2 +-
 css/70_fills.css             |  4 +++
 modules/behavior/draw.js     | 17 +++++++-----
 modules/behavior/draw_way.js | 51 ++++++++++++++++++------------------
 modules/core/context.js      |  7 +++++
 modules/modes/drag_node.js   | 17 +++++++-----
 modules/modes/draw_area.js   |  8 ++++--
 modules/modes/draw_line.js   | 10 ++++---
 modules/renderer/map.js      |  6 ++++-
 modules/svg/areas.js         |  5 +++-
 modules/svg/lines.js         |  5 +++-
 modules/svg/points.js        |  6 ++++-
 modules/svg/vertices.js      |  6 ++++-
 13 files changed, 95 insertions(+), 49 deletions(-)

diff --git a/css/20_map.css b/css/20_map.css
index 38106a223..87fea9309 100644
--- a/css/20_map.css
+++ b/css/20_map.css
@@ -26,7 +26,7 @@
 .way.target {
     pointer-events: stroke;
     fill: none;
-    stroke-width: 10;
+    stroke-width: 12;
     stroke-opacity: 0.8;
     stroke: currentColor;
 }
diff --git a/css/70_fills.css b/css/70_fills.css
index 3587bbee8..1c536510c 100644
--- a/css/70_fills.css
+++ b/css/70_fills.css
@@ -33,5 +33,9 @@
 .fill-partial path.area.fill {
     fill-opacity: 0;
     stroke-width: 60px;
+    pointer-events: none;
+}
+.mode-browse .fill-partial path.area.fill,
+.mode-select .fill-partial path.area.fill {
     pointer-events: visibleStroke;
 }
diff --git a/modules/behavior/draw.js b/modules/behavior/draw.js
index 61ca00af5..4bce319dd 100644
--- a/modules/behavior/draw.js
+++ b/modules/behavior/draw.js
@@ -48,11 +48,17 @@ export function behaviorDraw(context) {
     function datum() {
         if (d3_event.altKey) return {};
 
+        var element;
         if (d3_event.type === 'keydown') {
-            return (_lastMouse && _lastMouse.target.__data__) || {};
+            element = _lastMouse && _lastMouse.target;
         } else {
-            return d3_event.target.__data__ || {};
+            element = d3_event.target;
         }
+
+        // When drawing, connect only to things classed as targets..
+        // (this excludes area fills and active drawing elements)
+        var selection = d3_select(element);
+        return (selection.classed('target') && element.__data__) || {};
     }
 
 
@@ -116,8 +122,7 @@ export function behaviorDraw(context) {
 
 
     function click() {
-        var trySnap = geoViewportEdge(context.mouse, context.map().dimensions()) !== null;
-
+        var trySnap = geoViewportEdge(context.mouse(), context.map().dimensions()) === null;
         if (trySnap) {
             // If we're not at the edge of the viewport, try to snap..
             // See also: `modes/drag_node.js doMove()`
@@ -128,8 +133,8 @@ export function behaviorDraw(context) {
                 dispatch.call('clickNode', this, d);
                 return;
 
-            // Snap to a way (not an area fill)
-            } else if (d.type === 'way' && !d3_select(d3_event.sourceEvent.target).classed('fill')) {
+            // Snap to a way
+            } else if (d.type === 'way') {
                 var choice = geoChooseEdge(context.childNodes(d), context.mouse(), context.projection);
                 var edge = [d.nodes[choice.index - 1], d.nodes[choice.index]];
                 dispatch.call('clickWay', this, choice.loc, edge);
diff --git a/modules/behavior/draw_way.js b/modules/behavior/draw_way.js
index 57ecfb193..045b8a855 100644
--- a/modules/behavior/draw_way.js
+++ b/modules/behavior/draw_way.js
@@ -13,7 +13,8 @@ import { behaviorDraw } from './draw';
 
 import {
     geoChooseEdge,
-    geoEdgeEqual
+    geoEdgeEqual,
+    geoViewportEdge
 } from '../geo';
 
 import {
@@ -31,17 +32,18 @@ import { utilEntitySelector } from '../util';
 
 export function behaviorDrawWay(context, wayId, index, mode, startGraph) {
 
-    var origWay = context.entity(wayId),
-        isArea = context.geometry(wayId) === 'area',
-        tempEdits = 0,
-        annotation = t((origWay.isDegenerate() ?
-            'operations.start.annotation.' :
-            'operations.continue.annotation.') + context.geometry(wayId)),
-        draw = behaviorDraw(context),
-        startIndex,
-        start,
-        end,
-        segment;
+    var origWay = context.entity(wayId);
+    var isArea = context.geometry(wayId) === 'area';
+    var tempEdits = 0;
+    var annotation = t((origWay.isDegenerate() ?
+        'operations.start.annotation.' :
+        'operations.continue.annotation.') + context.geometry(wayId));
+    var draw = behaviorDraw(context);
+    var _activeIDs = [];
+    var startIndex;
+    var start;
+    var end;
+    var segment;
 
 
     // initialize the temporary drawing entities
@@ -49,7 +51,8 @@ export function behaviorDrawWay(context, wayId, index, mode, startGraph) {
         startIndex = typeof index === 'undefined' ? origWay.nodes.length - 1 : 0;
         start = osmNode({ id: 'nStart', loc: context.entity(origWay.nodes[startIndex]).loc });
         end = osmNode({ id: 'nEnd', loc: context.map().mouseCoordinates() });
-        segment = osmWay({ id: 'wTemp',
+        segment = osmWay({
+            id: 'wTemp',
             nodes: typeof index === 'undefined' ? [start.id, end.id] : [end.id, start.id],
             tags: _clone(origWay.tags)
         });
@@ -70,20 +73,11 @@ export function behaviorDrawWay(context, wayId, index, mode, startGraph) {
 
     function move(datum) {
         var loc;
-
         if (datum.type === 'node' && datum.id !== end.id) {
             loc = datum.loc;
 
         } else if (datum.type === 'way') {
-            var dims = context.map().dimensions(),
-                mouse = context.mouse(),
-                pad = 5,
-                trySnap = mouse[0] > pad && mouse[0] < dims[0] - pad &&
-                    mouse[1] > pad && mouse[1] < dims[1] - pad;
-
-            if (trySnap) {
-                loc = geoChooseEdge(context.childNodes(datum), context.mouse(), context.projection).loc;
-            }
+            loc = geoChooseEdge(context.childNodes(datum), context.mouse(), context.projection).loc;
         }
 
         if (!loc) {
@@ -110,8 +104,8 @@ export function behaviorDrawWay(context, wayId, index, mode, startGraph) {
 
 
     function setActiveElements() {
-        var active = isArea ? [wayId, end.id] : [segment.id, start.id, end.id];
-        context.surface().selectAll(utilEntitySelector(active))
+        _activeIDs = isArea ? [wayId, end.id] : [segment.id, start.id, end.id];
+        context.surface().selectAll(utilEntitySelector(_activeIDs))
             .classed('active', true);
     }
 
@@ -326,6 +320,13 @@ export function behaviorDrawWay(context, wayId, index, mode, startGraph) {
     };
 
 
+    drawWay.activeIDs = function() {
+        if (!arguments.length) return _activeIDs;
+        // no assign
+        return drawWay;
+    };
+
+
     drawWay.tail = function(text) {
         draw.tail(text);
         return drawWay;
diff --git a/modules/core/context.js b/modules/core/context.js
index 95146e248..e7f0deacf 100644
--- a/modules/core/context.js
+++ b/modules/core/context.js
@@ -255,6 +255,13 @@ export function coreContext() {
             return [];
         }
     };
+    context.activeIDs = function() {
+        if (mode && mode.activeIDs) {
+            return mode.activeIDs();
+        } else {
+            return [];
+        }
+    };
 
 
     /* Behaviors */
diff --git a/modules/modes/drag_node.js b/modules/modes/drag_node.js
index 915706bbd..e39832b98 100644
--- a/modules/modes/drag_node.js
+++ b/modules/modes/drag_node.js
@@ -123,7 +123,7 @@ export function modeDragNode(context) {
 
     function datum() {
         var event = d3_event && d3_event.sourceEvent;
-        if (!event || event.altKey) {
+        if (!event || event.altKey || !d3_select(event.target).classed('target')) {
             return {};
         } else {
             return event.target.__data__ || {};
@@ -147,11 +147,8 @@ export function modeDragNode(context) {
             if (d.type === 'node' && d.id !== entity.id) {
                 loc = d.loc;
 
-            // Snap to a way (not an area fill)
-            } else if (d.type === 'way' && !d3_select(d3_event.sourceEvent.target).classed('fill')) {
-
-                // var childNodes = context.childNodes(d);
-                // var childIDs = childNodes.map(function(node) { return node.id; });
+            // Snap to a way
+            } else if (d.type === 'way') {
                 var choice = geoChooseEdge(context.childNodes(d), context.mouse(), context.projection);
                 // (not along a segment adjacent to self)
                 if (entity.id !== d.nodes[choice.index - 1] && entity.id !== d.nodes[choice.index]) {
@@ -274,6 +271,7 @@ export function modeDragNode(context) {
         context.map()
             .on('drawn.drag-node', null);
 
+        _activeIDs = [];
         context.surface()
             .selectAll('.active')
             .classed('active', false);
@@ -289,6 +287,13 @@ export function modeDragNode(context) {
     };
 
 
+    mode.activeIDs = function() {
+        if (!arguments.length) return _activeIDs;
+        // no assign
+        return mode;
+    };
+
+
     mode.restoreSelectedIDs = function(_) {
         if (!arguments.length) return _restoreSelectedIDs;
         _restoreSelectedIDs = _;
diff --git a/modules/modes/draw_area.js b/modules/modes/draw_area.js
index e5478d339..23bd196b5 100644
--- a/modules/modes/draw_area.js
+++ b/modules/modes/draw_area.js
@@ -20,8 +20,8 @@ export function modeDrawArea(context, wayId, startGraph) {
         var addNode = behavior.addNode;
 
         behavior.addNode = function(node) {
-            var length = way.nodes.length,
-                penultimate = length > 2 ? way.nodes[length - 2] : null;
+            var length = way.nodes.length;
+            var penultimate = length > 2 ? way.nodes[length - 2] : null;
 
             if (node.id === way.first() || node.id === penultimate) {
                 behavior.finish();
@@ -43,6 +43,10 @@ export function modeDrawArea(context, wayId, startGraph) {
         return [wayId];
     };
 
+    mode.activeIDs = function() {
+        return (behavior && behavior.activeIDs()) || [];
+    };
+
 
     return mode;
 }
diff --git a/modules/modes/draw_line.js b/modules/modes/draw_line.js
index 5198fd444..e6099b9bd 100644
--- a/modules/modes/draw_line.js
+++ b/modules/modes/draw_line.js
@@ -12,15 +12,14 @@ export function modeDrawLine(context, wayId, startGraph, affix) {
 
 
     mode.enter = function() {
-        var way = context.entity(wayId),
-            index = (affix === 'prefix') ? 0 : undefined,
-            headId = (affix === 'prefix') ? way.first() : way.last();
+        var way = context.entity(wayId);
+        var index = (affix === 'prefix') ? 0 : undefined;
+        var headId = (affix === 'prefix') ? way.first() : way.last();
 
         behavior = behaviorDrawWay(context, wayId, index, mode, startGraph)
             .tail(t('modes.draw_line.tail'));
 
         var addNode = behavior.addNode;
-
         behavior.addNode = function(node) {
             if (node.id === headId) {
                 behavior.finish();
@@ -42,6 +41,9 @@ export function modeDrawLine(context, wayId, startGraph, affix) {
         return [wayId];
     };
 
+    mode.activeIDs = function() {
+        return (behavior && behavior.activeIDs()) || [];
+    };
 
     return mode;
 }
diff --git a/modules/renderer/map.js b/modules/renderer/map.js
index 39996d04f..5987d7f2b 100644
--- a/modules/renderer/map.js
+++ b/modules/renderer/map.js
@@ -199,8 +199,9 @@ export function rendererMap(context) {
         supersurface
             .call(context.background());
 
-        context.on('enter.map', function() {
+        context.on('enter.map',  function() {
             if (map.editable() && !transformed) {
+                // redraw immediately the objects that are affected by a chnage in selectedIDs.
                 var all = context.intersects(map.extent());
                 var filter = utilFunctor(true);
                 var graph = context.graph();
@@ -210,6 +211,9 @@ export function rendererMap(context) {
                     .call(drawVertices.drawSelected, graph, all, map.extent())
                     .call(drawMidpoints, graph, all, filter, map.trimmedExtent());
                 dispatch.call('drawn', this, { full: false });
+
+                // redraw everything else later
+                scheduleRedraw();
             }
         });
 
diff --git a/modules/svg/areas.js b/modules/svg/areas.js
index d540a08a0..b017d531c 100644
--- a/modules/svg/areas.js
+++ b/modules/svg/areas.js
@@ -44,10 +44,13 @@ export function svgAreas(projection, context) {
     function drawTargets(selection, graph, entities, filter) {
         var fillClass = context.getDebug('target') ? 'pink ' : 'nocolor ';
         var getPath = svgPath(projection, graph);
+        var passive = entities.filter(function(d) {
+            return context.activeIDs().indexOf(d.id) === -1;
+        });
 
         var targets = selection.selectAll('.area.target')
             .filter(filter)
-            .data(entities, function key(d) { return d.id; });
+            .data(passive, function key(d) { return d.id; });
 
         // exit
         targets.exit()
diff --git a/modules/svg/lines.js b/modules/svg/lines.js
index caf11c294..d5b1010bb 100644
--- a/modules/svg/lines.js
+++ b/modules/svg/lines.js
@@ -39,10 +39,13 @@ export function svgLines(projection, context) {
     function drawTargets(selection, graph, entities, filter) {
         var fillClass = context.getDebug('target') ? 'pink ' : 'nocolor ';
         var getPath = svgPath(projection, graph);
+        var passive = entities.filter(function(d) {
+            return context.activeIDs().indexOf(d.id) === -1;
+        });
 
         var targets = selection.selectAll('.line.target')
             .filter(filter)
-            .data(entities, function key(d) { return d.id; });
+            .data(passive, function key(d) { return d.id; });
 
         // exit
         targets.exit()
diff --git a/modules/svg/points.js b/modules/svg/points.js
index 8c41f7a1a..40898e2aa 100644
--- a/modules/svg/points.js
+++ b/modules/svg/points.js
@@ -29,9 +29,13 @@ 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 context.activeIDs().indexOf(d.id) === -1;
+        });
+
         var targets = selection.selectAll('.point.target')
             .filter(filter)
-            .data(entities, function key(d) { return d.id; });
+            .data(passive, function key(d) { return d.id; });
 
         // exit
         targets.exit()
diff --git a/modules/svg/vertices.js b/modules/svg/vertices.js
index 4b066cb59..4ed0ea17b 100644
--- a/modules/svg/vertices.js
+++ b/modules/svg/vertices.js
@@ -183,9 +183,13 @@ export function svgVertices(projection, context) {
 
     function drawTargets(selection, graph, entities, filter) {
         var fillClass = context.getDebug('target') ? 'pink ' : 'nocolor ';
+        var passive = entities.filter(function(d) {
+            return context.activeIDs().indexOf(d.id) === -1;
+        });
+
         var targets = selection.selectAll('.vertex.target')
             .filter(filter)
-            .data(entities, function key(d) { return d.id; });
+            .data(passive, function key(d) { return d.id; });
 
         // exit
         targets.exit()