Merge pull request mapbox#414 from mapbox/studio-inspired-fixes

Studio inspired fixes

API.md

@@ -136,9 +136,9 @@ console.log(Draw.getAll());
 ```
 ---
 
-###`.delete(String: id) -> Draw`
+###`.delete(String | Array<String> : id) -> Draw`
 
-This method takes an id removes the coorisponding feature from Draw.
+This method takes an id or an array of ids and removes the corresponding features from Draw.
 
 In `direct_select` mode, deleting the active feature will stop the mode and revert to the `simple_select` mode.
 
@@ -197,6 +197,10 @@ This is different from `delete` or `deleteAlll` in that it follows rules describ
 
 ---
 
+### `.getMode() -> Draw`
+
+Returns Draw's current mode. For more about the modes, see below.
+
 ### `.changeMode(String: mode, ?Object: options) -> Draw`
 
 `changeMode` triggers the mode switching process inside Draw. `mode` must be one of the below strings. Each mode takes its own options. They are described in detail below.

src/api.js

@@ -2,7 +2,9 @@ var isEqual = require('lodash.isequal');
 var normalize = require('geojson-normalize');
 var hat = require('hat');
 var featuresAt = require('./lib/features_at');
+var stringSetsAreEqual = require('./lib/string_sets_are_equal');
 var geojsonhint = require('geojsonhint');
+var Constants = require('./constants');
 
 var featureTypes = {
   'Polygon': require('./feature_types/polygon'),
@@ -38,7 +40,8 @@ module.exports = function(ctx) {
       toDelete = toDelete.filter(id => {
         return newIdsForFasterLookUp[id] === undefined;
       });
-      ctx.store.delete(toDelete);
+      this.delete(toDelete);
+
       return newIds;
     },
     add: function (geojson) {
@@ -82,17 +85,45 @@ module.exports = function(ctx) {
         features: ctx.store.getAll().map(feature => feature.toGeoJSON())
       };
     },
-    delete: function(id) {
-      ctx.store.delete([id], { silent: true });
-      ctx.store.render();
+    delete: function(featureIds) {
+      ctx.store.delete(featureIds, { silent: true });
+      // If we were in direct select mode and our selected feature no longer exists
+      // (because it was deleted), we need to get out of that mode.
+      if (this.getMode() === Constants.modes.DIRECT_SELECT && !ctx.store.getSelectedIds().length) {
+        ctx.events.changeMode(Constants.modes.SIMPLE_SELECT, undefined, { silent: true });
+      } else {
+        ctx.store.render();
+      }
     },
     deleteAll: function() {
       ctx.store.delete(ctx.store.getAllIds(), { silent: true });
-      ctx.store.render();
+      // If we were in direct select mode, now our selected feature no longer exists,
+      // so escape that mode.
+      if (this.getMode() === Constants.modes.DIRECT_SELECT) {
+        ctx.events.changeMode(Constants.modes.SIMPLE_SELECT, undefined, { silent: true });
+      } else {
+        ctx.store.render();
+      }
     },
-    changeMode: function(mode, modeOptions) {
+    changeMode: function(mode, modeOptions = {}) {
+      // Avoid changing modes just to re-select what's already selected
+      if (mode === Constants.modes.SIMPLE_SELECT && this.getMode() === Constants.modes.SIMPLE_SELECT) {
+        if (stringSetsAreEqual((modeOptions.featureIds || []), ctx.store.getSelectedIds())) return;
+        // And if we are changing the selection within simple_select mode, just change the selection,
+        // instead of stopping and re-starting the mode
+        return ctx.store.setSelected(modeOptions.featureIds, { silent: true });
+      }
+
+      if (mode === Constants.modes.DIRECT_SELECT && this.getMode() === Constants.modes.DIRECT_SELECT
+        && modeOptions.featureId === ctx.store.getSelectedIds()[0]) {
+        return;
+      }
+
       ctx.events.changeMode(mode, modeOptions, { silent: true });
     },
+    getMode: function() {
+      return ctx.events.getMode();
+    },
     trash: function() {
       ctx.events.trash({ silent: true });
     }

src/constants.js

@@ -5,10 +5,15 @@ module.exports = {
   CONTROL_BUTTON_POINT_CLASS: 'mapbox-gl-draw_point',
   CONTROL_BUTTON_TRASH_CLASS: 'mapbox-gl-draw_trash',
   CONTROL_GROUP_CLASS: 'mapboxgl-ctrl-group',
-  MOUSE_ADD_CLASS_FRAGMENT: 'add',
-  MOUSE_MOVE_CLASS_FRAGMENT: 'move',
   ATTRIBUTION_CLASS: 'mapboxgl-ctrl-attrib',
   ACTIVE_BUTTON_CLASS: 'active',
+  cursors: {
+    ADD: 'add',
+    MOVE: 'move',
+    DRAG: 'drag',
+    POINTER: 'pointer',
+    NONE: 'none'
+  },
   types: {
     POLYGON: 'polygon',
     LINE: 'line_string',

src/events.js

@@ -1,5 +1,5 @@
 var ModeHandler = require('./lib/mode_handler');
-var getFeatureAtAndSetCursors = require('./lib/get_features_and_set_cursor');
+var getFeaturesAndSetCursor = require('./lib/get_features_and_set_cursor');
 var isClick = require('./lib/is_click');
 var Constants = require('./constants');
 
@@ -29,20 +29,18 @@ module.exports = function(ctx) {
       event.originalEvent.stopPropagation();
     }
     else {
-      ctx.ui.queueMapClasses({mouse: 'drag'});
+      ctx.ui.queueMapClasses({ mouse: Constants.cursors.DRAG });
       currentMode.drag(event);
     }
   };
 
   events.mousemove = function(event) {
     if (mouseDownInfo.isDown) {
-      events.drag(event);
-    }
-    else {
-      var target = getFeatureAtAndSetCursors(event, ctx);
-      event.featureTarget = target;
-      currentMode.mousemove(event);
+      return events.drag(event);
     }
+    var target = getFeaturesAndSetCursor(event, ctx);
+    event.featureTarget = target;
+    currentMode.mousemove(event);
   };
 
   events.mousedown = function(event) {
@@ -51,14 +49,14 @@ module.exports = function(ctx) {
       time: new Date().getTime(),
       point: event.point
     };
-    var target = getFeatureAtAndSetCursors(event, ctx);
+    var target = getFeaturesAndSetCursor(event, ctx);
     event.featureTarget = target;
     currentMode.mousedown(event);
   };
 
   events.mouseup = function(event) {
     mouseDownInfo.isDown = false;
-    var target = getFeatureAtAndSetCursors(event, ctx);
+    var target = getFeaturesAndSetCursor(event, ctx);
     event.featureTarget = target;
 
     if (isClick(mouseDownInfo, {
@@ -161,6 +159,9 @@ module.exports = function(ctx) {
     },
     trash: function(options) {
       currentMode.trash(options);
+    },
+    getMode: function() {
+      return currentModeName;
     }
   };
 

src/lib/common_selectors.js

@@ -7,13 +7,9 @@ module.exports = {
       return featureTarget.properties.meta === type;
     };
   },
-  isBoxSelecting(e) {
-    if (!e.originalEvent) {
-      return false;
-    }
-    if (!e.originalEvent.shiftKey) {
-      return false;
-    }
+  isShiftMousedown(e) {
+    if (!e.originalEvent) return false;
+    if (!e.originalEvent.shiftKey) return false;
     return e.originalEvent.button === 0;
   },
   isActiveFeature: function(e) {

src/lib/double_click_zoom.js

@@ -0,0 +1,14 @@
+module.exports = {
+  enable(ctx) {
+    setTimeout(() => {
+      if (!ctx.map || !ctx.map.doubleClickZoom) return;
+      ctx.map.doubleClickZoom.enable();
+    }, 0);
+  },
+  disable(ctx) {
+    setTimeout(() => {
+      if (!ctx.map || !ctx.map.doubleClickZoom) return;
+      ctx.map.doubleClickZoom.disable();
+    }, 0);
+  }
+};

src/lib/get_features_and_set_cursor.js

@@ -1,19 +1,23 @@
 var featuresAt = require('./features_at');
+var Constants = require('../constants');
 
 module.exports = function getFeatureAtAndSetCursors(event, ctx) {
   var features = featuresAt(event, null, ctx);
-  var classes = { mouse: 'none' };
+  var classes = { mouse: Constants.cursors.NONE };
 
   if (features[0]) {
-    classes.mouse = features[0].properties.active === 'true' ? 'move' : 'pointer';
+    classes.mouse = features[0].properties.active === 'true'
+      ? Constants.cursors.MOVE
+      : Constants.cursors.POINTER;
     classes.feature = features[0].properties.meta;
   }
 
-  if (ctx.events.currentModeName().includes('draw')) {
-    classes.mouse = 'add';
+  if (ctx.events.currentModeName().indexOf('draw') !== -1) {
+    classes.mouse = Constants.cursors.ADD;
   }
 
   ctx.ui.queueMapClasses(classes);
+  ctx.ui.updateMapClasses();
 
   return features[0];
 };

src/lib/mode_handler.js

@@ -37,15 +37,18 @@ var ModeHandler = function(mode, DrawContext) {
   var delegate = function (eventName, event) {
     var handles = handlers[eventName];
     var iHandle = handles.length;
+    var handlerCalled = false;
     while (iHandle--) {
       var handle = handles[iHandle];
       if (handle.selector(event)) {
         handle.fn.call(ctx, event);
-        DrawContext.store.render();
-        break;
+        handlerCalled = true;
       }
     }
-    DrawContext.ui.updateMapClasses();
+    if (handlerCalled) {
+      DrawContext.store.render();
+      DrawContext.ui.updateMapClasses();
+    }
   };
 
   mode.start.call(ctx);

src/lib/string_sets_are_equal.js

@@ -0,0 +1,3 @@
+module.exports = function(a, b) {
+  return JSON.stringify(a.sort()) === JSON.stringify(b.sort());
+};

src/modes/direct_select.js

@@ -1,5 +1,6 @@
 var {noFeature, isOfMetaType, isInactiveFeature, isShiftDown} = require('../lib/common_selectors');
 var createSupplementaryPoints = require('../lib/create_supplementary_points');
+const doubleClickZoom = require('../lib/double_click_zoom');
 const Constants = require('../constants');
 
 module.exports = function(ctx, opts) {
@@ -62,7 +63,7 @@ module.exports = function(ctx, opts) {
       dragging = false;
       canDragMove = false;
       ctx.store.setSelected(featureId);
-      ctx.map.doubleClickZoom.disable();
+      doubleClickZoom.disable(ctx);
       this.on('mousedown', isOfMetaType('vertex'), onVertex);
       this.on('mousedown', isOfMetaType('midpoint'), onMidpoint);
       this.on('drag', () => canDragMove, function(e) {
@@ -96,14 +97,14 @@ module.exports = function(ctx, opts) {
         stopDragging();
       });
       this.on('click', noFeature, function() {
-        ctx.events.changeMode('simple_select');
+        ctx.events.changeMode(Constants.modes.SIMPLE_SELECT);
       });
       this.on('click', isInactiveFeature, function() {
-        ctx.events.changeMode('simple_select');
+        ctx.events.changeMode(Constants.modes.SIMPLE_SELECT);
       });
     },
     stop: function() {
-      ctx.map.doubleClickZoom.enable();
+      doubleClickZoom.enable(ctx);
     },
     render: function(geojson, push) {
       if (featureId === geojson.properties.id) {
@@ -122,7 +123,7 @@ module.exports = function(ctx, opts) {
     },
     trash: function() {
       if (selectedCoordPaths.length === 0) {
-        return ctx.events.changeMode('simple_select', { features: [feature] });
+        return ctx.events.changeMode(Constants.modes.SIMPLE_SELECT, { features: [feature] });
       }
 
       selectedCoordPaths.sort().reverse().forEach(id => feature.removeCoordinate(id));
@@ -133,7 +134,7 @@ module.exports = function(ctx, opts) {
       selectedCoordPaths = [];
       if (feature.isValid() === false) {
         ctx.store.delete([featureId]);
-        ctx.events.changeMode('simple_select', null);
+        ctx.events.changeMode(Constants.modes.SIMPLE_SELECT, null);
       }
     }
   };

src/modes/draw_line_string.js

@@ -1,6 +1,7 @@
 const CommonSelectors = require('../lib/common_selectors');
 const LineString = require('../feature_types/line_string');
 const isEventAtCoordinates = require('../lib/is_event_at_coordinates');
+const doubleClickZoom = require('../lib/double_click_zoom');
 const Constants = require('../constants');
 
 module.exports = function(ctx) {
@@ -19,8 +20,8 @@ module.exports = function(ctx) {
   ctx.store.add(line);
 
   function stopDrawingAndRemove() {
-    ctx.events.changeMode(Constants.modes.SIMPLE_SELECT);
     ctx.store.delete([line.id], { silent: true });
+    ctx.events.changeMode(Constants.modes.SIMPLE_SELECT);
   }
 
   function handleMouseMove(e) {
@@ -29,6 +30,7 @@ module.exports = function(ctx) {
   }
 
   function handleClick(e) {
+    ctx.ui.queueMapClasses({ mouse: Constants.cursors.ADD });
     // Finish if we clicked on the first or last point
     if (currentVertexPosition > 0 &&
       (isEventAtCoordinates(e, line.coordinates[0]) || isEventAtCoordinates(e, line.coordinates[currentVertexPosition - 1]))
@@ -41,25 +43,20 @@ module.exports = function(ctx) {
   }
 
   function finish() {
+    if (!line.isValid()) return stopDrawingAndRemove();
     line.removeCoordinate(`${currentVertexPosition}`);
     currentVertexPosition--;
-    if (line.isValid()) {
-      ctx.map.fire(Constants.events.CREATE, {
-        features: [line.toGeoJSON()]
-      });
-    }
+    ctx.map.fire(Constants.events.CREATE, {
+      features: [line.toGeoJSON()]
+    });
     ctx.events.changeMode(Constants.modes.SIMPLE_SELECT, { featureIds: [line.id] });
   }
 
   return {
     start: function() {
       ctx.store.clearSelected();
-      setTimeout(() => {
-        if (ctx.map && ctx.map.doubleClickZoom) {
-          ctx.map.doubleClickZoom.disable();
-        }
-      });
-      ctx.ui.queueMapClasses({ mouse: Constants.MOUSE_ADD_CLASS_FRAGMENT });
+      doubleClickZoom.disable(ctx);
+      ctx.ui.queueMapClasses({ mouse: Constants.cursors.ADD });
       ctx.ui.setActiveButton(Constants.types.LINE);
       this.on('mousemove', CommonSelectors.true, handleMouseMove);
       this.on('click', CommonSelectors.true, handleClick);
@@ -68,11 +65,7 @@ module.exports = function(ctx) {
     },
 
     stop() {
-      setTimeout(() => {
-        if (ctx.map && ctx.map.doubleClickZoom) {
-          ctx.map.doubleClickZoom.enable();
-        }
-      }, 0);
+      doubleClickZoom.enable(ctx);
       ctx.ui.setActiveButton();
 
       // If it's invalid, just destroy the thing