squash

mapbox · Feb 4, 2019 · 6c7c5cf · 6c7c5cf
2 parents 0e5b982 + 4cc2a7d
commit 6c7c5cf
Show file tree

Hide file tree

Showing 18 changed files with 125 additions and 99 deletions.
diff --git a/debug/cluster.html b/debug/cluster.html
@@ -33,9 +33,9 @@
         "cluster": true,
         "clusterRadius": 50,
         "clusterProperties": {
-            "max": ["max", 0, ["get", "scalerank"]],
-            "sum": ["+", 0, ["get", "scalerank"]],
-            "has_island": ["any", false, ["==", ["get", "featureclass"], "island"]]
+            "max": ["max", ["get", "scalerank"]],
+            "sum": ["+", ["get", "scalerank"]],
+            "has_island": ["any", ["==", ["get", "featureclass"], "island"]]
         }
     });
     map.addLayer({

diff --git a/debug/popup.html b/debug/popup.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>Mapbox GL JS debug page</title>
+    <meta charset='utf-8'>
+    <meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=no">
+    <link rel='stylesheet' href='/dist/mapbox-gl.css' />
+    <style>
+        body { margin: 0; padding: 0; }
+        html, body, #map { height: 100%; }
+    </style>
+</head>
+
+<body>
+<div id='map'></div>
+
+<script src='/dist/mapbox-gl-dev.js'></script>
+<script src='/debug/access_token_generated.js'></script>
+<script>
+
+var map = window.map = new mapboxgl.Map({
+    container: 'map',
+    zoom: 12.5,
+    center: [-77.01866, 38.888],
+    style: 'mapbox://styles/mapbox/streets-v10',
+    hash: true
+});
+
+var popup = new mapboxgl.Popup({closeButton: false})
+    .trackPointer()
+    .setHTML('<h1>Hello World!</h1>')
+    .addTo(map);
+</script>
+</body>
+</html>
diff --git a/package.json b/package.json
@@ -34,7 +34,7 @@
     "potpack": "^1.0.1",
     "quickselect": "^1.0.0",
     "rw": "^1.3.3",
-    "supercluster": "^5.0.0",
+    "supercluster": "^6.0.0",
     "tinyqueue": "^1.1.0",
     "vt-pbf": "^3.0.1"
   },

diff --git a/src/css/mapbox-gl.css b/src/css/mapbox-gl.css
@@ -25,6 +25,10 @@
     user-select: none;
 }
 
+.mapboxgl-canvas-container.mapboxgl-interactive.mapboxgl-track-pointer {
+    cursor: pointer;
+}
+
 .mapboxgl-canvas-container.mapboxgl-interactive:active,
 .mapboxgl-ctrl-group > button.mapboxgl-ctrl-compass:active {
     cursor: -webkit-grabbing;
@@ -446,6 +450,23 @@ a.mapboxgl-ctrl-logo.mapboxgl-compact {
     border-bottom-right-radius: 0;
 }
 
+.mapboxgl-popup-track-pointer {
+    display: none;
+}
+
+.mapboxgl-popup-track-pointer * {
+    pointer-events: none;
+    user-select: none;
+}
+
+.mapboxgl-map:hover .mapboxgl-popup-track-pointer {
+    display: flex;
+}
+
+.mapboxgl-map:active .mapboxgl-popup-track-pointer {
+    display: none;
+}
+
 .mapboxgl-marker {
     position: absolute;
     top: 0;

diff --git a/src/source/geojson_worker_source.js b/src/source/geojson_worker_source.js
@@ -298,37 +298,26 @@ class GeoJSONWorkerSource extends VectorTileWorkerSource {
 function getSuperclusterOptions({superclusterOptions, clusterProperties}) {
     if (!clusterProperties || !superclusterOptions) return superclusterOptions;
 
-    const initialValues = {};
     const mapExpressions = {};
     const reduceExpressions = {};
     const globals = {accumulated: null, zoom: 0};
     const feature = {properties: null};
     const propertyNames = Object.keys(clusterProperties);
 
     for (const key of propertyNames) {
-        const [operator, initialExpression, mapExpression] = clusterProperties[key];
+        const [operator, mapExpression] = clusterProperties[key];
 
-        const initialExpressionParsed = createExpression(initialExpression);
         const mapExpressionParsed = createExpression(mapExpression);
         const reduceExpressionParsed = createExpression(
             typeof operator === 'string' ? [operator, ['accumulated'], ['get', key]] : operator);
 
-        assert(initialExpressionParsed.result === 'success');
         assert(mapExpressionParsed.result === 'success');
         assert(reduceExpressionParsed.result === 'success');
 
-        initialValues[key] = (initialExpressionParsed.value: any).evaluate(globals);
         mapExpressions[key] = mapExpressionParsed.value;
         reduceExpressions[key] = reduceExpressionParsed.value;
     }
 
-    superclusterOptions.initial = () => {
-        const properties = {};
-        for (const key of propertyNames) {
-            properties[key] = initialValues[key];
-        }
-        return properties;
-    };
     superclusterOptions.map = (pointProperties) => {
         feature.properties = pointProperties;
         const properties = {};

diff --git a/src/style-spec/reference/v8.json b/src/style-spec/reference/v8.json
@@ -375,7 +375,7 @@
     },
     "clusterProperties": {
       "type": "*",
-      "doc": "An object defining custom properties on the generated clusters if clustering is enabled, aggregating values from clustered points. Has the form `{\"property_name\": [operator, initial_expression, map_expression]}`. `operator` is any expression function that accepts at least 2 operands (e.g. `\"+\"` or `\"max\"`) — it accumulates the property value from clusters/points the cluster contains; `initial_expression` evaluates the initial value of the property before accummulating other points/clusters; `map_expression` produces the value of a single point.\n\nExample: `{\"sum\": [\"+\", 0, [\"get\", \"scalerank\"]]}`.\n\nFor more advanced use cases, in place of `operator`, you can use a custom reduce expression that references a special `[\"accumulated\"]` value, e.g.:\n`{\"sum\": [[\"+\", [\"accumulated\"], [\"get\", \"sum\"]], 0, [\"get\", \"scalerank\"]]}`"
+      "doc": "An object defining custom properties on the generated clusters if clustering is enabled, aggregating values from clustered points. Has the form `{\"property_name\": [operator, map_expression]}`. `operator` is any expression function that accepts at least 2 operands (e.g. `\"+\"` or `\"max\"`) — it accumulates the property value from clusters/points the cluster contains; `map_expression` produces the value of a single point.\n\nExample: `{\"sum\": [\"+\", [\"get\", \"scalerank\"]]}`.\n\nFor more advanced use cases, in place of `operator`, you can use a custom reduce expression that references a special `[\"accumulated\"]` value, e.g.:\n`{\"sum\": [[\"+\", [\"accumulated\"], [\"get\", \"sum\"]], [\"get\", \"scalerank\"]]}`"
     },
     "lineMetrics": {
       "type": "boolean",

diff --git a/src/style-spec/validate/validate_source.js b/src/style-spec/validate/validate_source.js
@@ -48,14 +48,9 @@ export default function validateSource(options) {
         });
         if (value.cluster) {
             for (const prop in value.clusterProperties) {
-                const [operator, initialExpr, mapExpr] = value.clusterProperties[prop];
+                const [operator, mapExpr] = value.clusterProperties[prop];
                 const reduceExpr = typeof operator === 'string' ? [operator, ['accumulated'], ['get', prop]] : operator;
 
-                errors.push(...validateExpression({
-                    key: `${key}.${prop}.initial`,
-                    value: initialExpr,
-                    expressionContext: 'cluster-initial'
-                }));
                 errors.push(...validateExpression({
                     key: `${key}.${prop}.map`,
                     value: mapExpr,

diff --git a/src/style/style_layer/line_style_layer.js b/src/style/style_layer/line_style_layer.js
@@ -108,6 +108,7 @@ class LineStyleLayer extends StyleLayer {
         if (lineOffset) {
             geometry = offsetLine(geometry, lineOffset * pixelsToTileUnits);
         }
+
         return polygonIntersectsBufferedMultiLine(translatedPolygon, geometry, halfWidth);
     }
 }

diff --git a/src/ui/control/attribution_control.js b/src/ui/control/attribution_control.js
@@ -103,7 +103,7 @@ class AttributionControl {
                 }
                 return acc;
             }, `?`);
-            editLink.href = `https://www.mapbox.com/feedback/${paramString}${this._map._hash ? this._map._hash.getHashString(true) : ''}`;
+            editLink.href = `${config.FEEDBACK_URL}/${paramString}${this._map._hash ? this._map._hash.getHashString(true) : ''}`;
         }
     }
 

diff --git a/src/ui/popup.js b/src/ui/popup.js
@@ -79,7 +79,7 @@ export default class Popup extends Evented {
     _closeButton: HTMLElement;
     _tip: HTMLElement;
     _lngLat: LngLat;
-    _trackCursor: Boolean;
+    _trackPointer: boolean;
     _pos: ?Point;
 
     constructor(options: PopupOptions) {
@@ -103,14 +103,12 @@ export default class Popup extends Evented {
         this._map.on('remove', this.remove);
         this._update();
 
-        if (this._trackCursor) {
-            this._map.on('mousemove', (e) => {this._update(e.point)});
-            this._map.on('touchmove', (e) => {this._update(e.point)});
-            this._map.on('mouseout', () => {DOM.setDisplay(this._container, `none`)});
-            this._map.on('mouseover', () => {DOM.setDisplay(this._container, ``)});
-        }
-
-        else this._map.on('move', this._update);
+        if (this._trackPointer) {
+            this._map.on('mousemove', (e) => { this._update(e.point); });
+            this._map.on('mouseup', (e) => { this._update(e.point); });
+            this._container.classList.add('mapboxgl-popup-track-pointer');
+            this._map._canvasContainer.classList.add('mapboxgl-track-pointer');
+        } else { this._map.on('move', this._update); }
 
         /**
          * Fired when the popup is opened manually or programatically.
@@ -156,7 +154,6 @@ export default class Popup extends Evented {
             this._map.off('click', this._onClickClose);
             this._map.off('remove', this.remove);
             this._map.off('mousemove');
-            this._map.off('touchmove');
             delete this._map;
         }
 
@@ -188,7 +185,7 @@ export default class Popup extends Evented {
     }
 
     /**
-     * Sets the geographical location of the popup's anchor, and moves the popup to it. Replaces trackCursor() behavior
+     * Sets the geographical location of the popup's anchor, and moves the popup to it. Replaces trackPointer() behavior
      *
      * @param lnglat The geographical location to set as the popup's anchor.
      * @returns {Popup} `this`
@@ -199,30 +196,32 @@ export default class Popup extends Evented {
 
         if (this._map) {
             this._map.on('move', this._update);
-            this._map.off('mousemove')
-            this._map.off('touchmove')
+            this._map.off('mousemove');
+            this._container.classList.remove('mapboxgl-popup-track-pointer');
+            this._map._canvasContainer.classList.remove('mapboxgl-track-pointer');
         }
 
-        this._trackCursor = false;
+        this._trackPointer = false;
 
         this._update();
         return this;
     }
 
     /**
-     * Tracks the popup anchor to the cursor position. Replaces the setLngLat behavior
+     * Tracks the popup anchor to the cursor position, on screens with a pointer device (will be hidden on touchscreens). Replaces the setLngLat behavior.
      *
      * @returns {Popup} `this`
      */
-    trackCursor() {
-        this._trackCursor = true;
+    trackPointer() {
+        this._trackPointer = true;
         this._pos = null;
+
         if (this._map) {
             this._map.off('move', this._update);
-            this._map.on('mousemove', (e) =>{this._update(e.point)});
-            this._map.on('touchmove', (e) =>{this._update(e.point)});
-            this._map.on('mouseout', this.remove);
-
+            this._map.on('mousemove', (e) => { this._update(e.point); });
+            this._map.on('drag', (e) => { this._update(e.point); });
+            this._container.classList.add('mapboxgl-popup-track-pointer');
+            this._map._canvasContainer.classList.add('mapboxgl-track-pointer');
         }
 
         return this;
@@ -309,7 +308,7 @@ export default class Popup extends Evented {
     }
 
     _update(cursor: PointLike) {
-        if (!this._map || !this._lngLat && !this._trackCursor || !this._content) { return; }
+        if (!this._map || !this._lngLat && !this._trackPointer || !this._content) { return; }
 
         if (!this._container) {
             this._container = DOM.create('div', 'mapboxgl-popup', this._map.getContainer());
@@ -320,15 +319,16 @@ export default class Popup extends Evented {
                 this.options.className.split(' ').forEach(name =>
                     this._container.classList.add(name));
             }
+
         }
 
-        if (this._map.transform.renderWorldCopies && !this._trackCursor) {
+        if (this._map.transform.renderWorldCopies && !this._trackPointer) {
             this._lngLat = smartWrap(this._lngLat, this._pos, this._map.transform);
         }
 
-        if (this._trackCursor && !cursor) return;
+        if (this._trackPointer && !cursor) return;
 
-        const pos = this._pos = this._trackCursor && cursor ? cursor : this._map.project(this._lngLat);
+        const pos = this._pos = this._trackPointer && cursor ? cursor : this._map.project(this._lngLat);
 
         let anchor: ?Anchor = this.options.anchor;
         const offset = normalizeOffset(this.options.offset);

diff --git a/src/util/config.js b/src/util/config.js
@@ -3,6 +3,7 @@
 type Config = {|
   API_URL: string,
   EVENTS_URL: string,
+  FEEDBACK_URL: string,
   REQUIRE_ACCESS_TOKEN: boolean,
   ACCESS_TOKEN: ?string,
   MAX_PARALLEL_IMAGE_REQUESTS: number
@@ -17,6 +18,7 @@ const config: Config = {
             return 'https://events.mapbox.com/events/v2';
         }
     },
+    FEEDBACK_URL: 'https://apps.mapbox.com/feedback',
     REQUIRE_ACCESS_TOKEN: true,
     ACCESS_TOKEN: null,
     MAX_PARALLEL_IMAGE_REQUESTS: 16

diff --git a/src/util/dom.js b/src/util/dom.js
@@ -56,12 +56,6 @@ DOM.setTransform = function(el: HTMLElement, value: string) {
     (el.style: any)[transformProp] = value;
 };
 
-const displayProp = testProp(['display']);
-
-DOM.setDisplay = function(el: HTMLElement, value: string) {
-    (el.style: any)[displayProp] = value;
-};
-
 // Feature detection for {passive: false} support in add/removeEventListener.
 let passiveSupported = false;
 

diff --git a/test/integration/render-tests/geojson/clustered-properties/style.json b/test/integration/render-tests/geojson/clustered-properties/style.json
@@ -18,9 +18,9 @@
       "cluster": true,
       "clusterRadius": 50,
       "clusterProperties": {
-        "max": ["max", 0, ["get", "scalerank"]],
-        "sum": ["+", 0, ["get", "scalerank"]],
-        "has_island": ["any", false, ["==", ["get", "featureclass"], "island"]]
+        "max": ["max", ["get", "scalerank"]],
+        "sum": ["+", ["get", "scalerank"]],
+        "has_island": ["any", ["==", ["get", "featureclass"], "island"]]
       }
     }
   },

diff --git a/test/unit/style-spec/fixture/sources.input.json b/test/unit/style-spec/fixture/sources.input.json
@@ -46,9 +46,8 @@
       "data": "/test/integration/data/places.geojson",
       "cluster": true,
       "clusterProperties": {
-        "initial": ["+", ["get", "scalerank"], 0],
-        "zoom": ["+", 0, ["zoom"]],
-        "state": ["+", 0, ["feature-state", "foo"]]
+        "zoom": ["+", ["zoom"]],
+        "state": ["+", ["feature-state", "foo"]]
       }
     }
   },

diff --git a/test/unit/style-spec/fixture/sources.output.json b/test/unit/style-spec/fixture/sources.output.json
@@ -39,16 +39,12 @@
     "message": "sources.canvas: Please use runtime APIs to add canvas sources, rather than including them in stylesheets.",
     "identifier": "source.canvas"
   },
-  {
-    "message": "sources.cluster-properties.initial.initial: Feature data expressions are not supported with initial expression part of cluster properties.",
-    "line": 49
-  },
   {
     "message": "sources.cluster-properties.zoom.map: \"zoom\" and \"feature-state\" expressions are not supported with cluster properties.",
-    "line": 50
+    "line": 49
   },
   {
     "message": "sources.cluster-properties.state.map: \"zoom\" and \"feature-state\" expressions are not supported with cluster properties.",
-    "line": 51
+    "line": 50
   }
 ]
diff --git a/test/unit/ui/control/attribution.test.js b/test/unit/ui/control/attribution.test.js
@@ -109,17 +109,18 @@ test('AttributionControl dedupes attributions that are substrings of others', (t
 });
 
 test('AttributionControl has the correct edit map link', (t) => {
+    config.FEEDBACK_URL = "https://feedback.com";
     const map = createMap(t);
     const attribution = new AttributionControl();
     map.addControl(attribution);
     map.on('load', () => {
-        map.addSource('1', { type: 'geojson', data: { type: 'FeatureCollection', features: [] }, attribution: '<a class="mapbox-improve-map" href="https://www.mapbox.com/feedback/" target="_blank">Improve this map</a>'});
+        map.addSource('1', { type: 'geojson', data: { type: 'FeatureCollection', features: [] }, attribution: '<a class="mapbox-improve-map" href="https://feedback.com" target="_blank">Improve this map</a>'});
         map.addLayer({ id: '1', type: 'fill', source: '1' });
         map.on('data', (e) => {
             if (e.dataType === 'source' && e.sourceDataType === 'metadata') {
-                t.equal(attribution._editLink.href, 'https://www.mapbox.com/feedback/?owner=mapbox&id=streets-v10&access_token=pk.123#/0/0/0', 'edit link contains map location data');
+                t.equal(attribution._editLink.href, 'https://feedback.com/?owner=mapbox&id=streets-v10&access_token=pk.123#/0/0/0', 'edit link contains map location data');
                 map.setZoom(2);
-                t.equal(attribution._editLink.href, 'https://www.mapbox.com/feedback/?owner=mapbox&id=streets-v10&access_token=pk.123#/0/0/2', 'edit link updates on mapmove');
+                t.equal(attribution._editLink.href, 'https://feedback.com/?owner=mapbox&id=streets-v10&access_token=pk.123#/0/0/2', 'edit link updates on mapmove');
                 t.end();
             }
         });