[bugfix] mapbox pan on filter (#5858)

* Pan mapbox map on query - Modify the backend to send back the bounds of the points to cluster - Clean up of mapbox component to derive lat, lon and zoom from the mercator that fitsbounds based on the bounds passed from the backend. This and the computation of clusters are now only done on initial load b/c we are not making queries to the backend on pan/zoom - A minor clean up of unused props and also making the visual properties panel open by default * Remove default latitude, longitude and zoom props which are unnecessary for computing the viewport now that it is driven by the bounds from the backend. * a few small tweaks (rename vars to be sneak case and avoid two extra hash lookups)
apache · Sep 11, 2018 · 6df720d · 6df720d
1 parent 6ae285b
commit 6df720d
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 44 deletions.
diff --git a/superset/assets/src/explore/visTypes.jsx b/superset/assets/src/explore/visTypes.jsx
@@ -1743,6 +1743,7 @@ export const visTypes = {
       },
       {
         label: t('Viewport'),
+        expanded: true,
         controlSetRows: [
           ['viewport_longitude', 'viewport_latitude'],
           ['viewport_zoom', null],

diff --git a/superset/assets/src/utils/common.js b/superset/assets/src/utils/common.js
@@ -9,9 +9,6 @@ export const LUMINANCE_RED_WEIGHT = 0.2126;
 export const LUMINANCE_GREEN_WEIGHT = 0.7152;
 export const LUMINANCE_BLUE_WEIGHT = 0.0722;
 export const MILES_PER_KM = 1.60934;
-export const DEFAULT_LONGITUDE = -122.405293;
-export const DEFAULT_LATITUDE = 37.772123;
-export const DEFAULT_ZOOM = 11;
 
 // Regexp for the label added to time shifted series (1 hour offset, 2 days offset, etc.)
 export const TIME_SHIFT_PATTERN = /\d+ \w+ offset/;

diff --git a/superset/assets/src/visualizations/MapBox/MapBox.jsx b/superset/assets/src/visualizations/MapBox/MapBox.jsx
@@ -6,12 +6,6 @@ import Immutable from 'immutable';
 import supercluster from 'supercluster';
 import ViewportMercator from 'viewport-mercator-project';
 import ScatterPlotGlowOverlay from './ScatterPlotGlowOverlay';
-
-import {
-  DEFAULT_LONGITUDE,
-  DEFAULT_LATITUDE,
-  DEFAULT_ZOOM,
-} from '../../utils/common';
 import './MapBox.css';
 
 const NOOP = () => {};
@@ -31,37 +25,39 @@ const propTypes = {
   pointRadiusUnit: PropTypes.string,
   renderWhileDragging: PropTypes.bool,
   rgb: PropTypes.array,
-  viewportLatitude: PropTypes.number,
-  viewportLongitude: PropTypes.number,
-  viewportZoom: PropTypes.number,
+  bounds: PropTypes.array,
 };
 
 const defaultProps = {
   globalOpacity: 1,
   onViewportChange: NOOP,
   pointRadius: DEFAULT_POINT_RADIUS,
   pointRadiusUnit: 'Pixels',
-  viewportLatitude: DEFAULT_LATITUDE,
-  viewportLongitude: DEFAULT_LONGITUDE,
-  viewportZoom: DEFAULT_ZOOM,
 };
 
 class MapBox extends React.Component {
   constructor(props) {
     super(props);
 
-    const {
-      viewportLatitude: latitude,
-      viewportLongitude: longitude,
-      viewportZoom: zoom,
-    } = this.props;
+    const { width, height, bounds } = this.props;
+    // Get a viewport that fits the given bounds, which all marks to be clustered.
+    // Derive lat, lon and zoom from this viewport. This is only done on initial
+    // render as the bounds don't update as we pan/zoom in the current design.
+    const mercator = new ViewportMercator({
+      width,
+      height,
+    }).fitBounds(bounds);
+    const { latitude, longitude, zoom } = mercator;
+    // Compute the clusters based on the bounds. Again, this is only done once because
+    // we don't update the clusters as we pan/zoom in the current design.
+    const bbox = [bounds[0][0], bounds[0][1], bounds[1][0], bounds[1][1]];
+    this.clusters = this.props.clusterer.getClusters(bbox, Math.round(zoom));
 
     this.state = {
       viewport: {
         longitude,
         latitude,
         zoom,
-        startDragLngLat: [longitude, latitude],
       },
     };
     this.onViewportChange = this.onViewportChange.bind(this);
@@ -86,23 +82,11 @@ class MapBox extends React.Component {
       rgb,
     } = this.props;
     const { viewport } = this.state;
-    const { latitude, longitude, zoom } = viewport;
-    const mercator = new ViewportMercator({
-      width,
-      height,
-      longitude,
-      latitude,
-      zoom,
-    });
-    const topLeft = mercator.unproject([0, 0]);
-    const bottomRight = mercator.unproject([width, height]);
-    const bbox = [topLeft[0], bottomRight[1], bottomRight[0], topLeft[1]];
-    const clusters = this.props.clusterer.getClusters(bbox, Math.round(zoom));
     const isDragging = viewport.isDragging === undefined ? false :
                        viewport.isDragging;
     return (
       <MapGL
-        {...this.state.viewport}
+        {...viewport}
         mapStyle={mapStyle}
         width={width}
         height={height}
@@ -114,7 +98,7 @@ class MapBox extends React.Component {
           isDragging={isDragging}
           width={width}
           height={height}
-          locations={Immutable.fromJS(clusters)}
+          locations={Immutable.fromJS(this.clusters)}
           dotRadius={pointRadius}
           pointRadiusUnit={pointRadiusUnit}
           rgb={rgb}
@@ -164,6 +148,7 @@ function mapbox(slice, payload, setControlValue) {
   const {
     customMetric,
     geoJSON,
+    bounds,
     mapboxApiKey,
   } = payload.data;
   const {
@@ -175,9 +160,6 @@ function mapbox(slice, payload, setControlValue) {
     point_radius: pointRadius,
     point_radius_unit: pointRadiusUnit,
     render_while_dragging: renderWhileDragging,
-    viewport_latitude: viewportLatitude,
-    viewport_longitude: viewportLongitude,
-    viewport_zoom: viewportZoom,
   } = formData;
 
   // Validate mapbox color
@@ -214,9 +196,7 @@ function mapbox(slice, payload, setControlValue) {
       pointRadiusUnit={pointRadiusUnit}
       renderWhileDragging={renderWhileDragging}
       rgb={rgb}
-      viewportLatitude={viewportLatitude}
-      viewportLongitude={viewportLongitude}
-      viewportZoom={viewportZoom}
+      bounds={bounds}
     />,
     document.querySelector(selector),
   );

diff --git a/superset/viz.py b/superset/viz.py
@@ -2015,6 +2015,10 @@ def get_data(self, df):
             ],
         }
 
+        x_series, y_series = df[fd.get('all_columns_x')], df[fd.get('all_columns_y')]
+        south_west = [x_series.min(), y_series.min()]
+        north_east = [x_series.max(), y_series.max()]
+
         return {
             'geoJSON': geo_json,
             'customMetric': custom_metric,
@@ -2024,9 +2028,7 @@ def get_data(self, df):
             'clusteringRadius': fd.get('clustering_radius'),
             'pointRadiusUnit': fd.get('point_radius_unit'),
             'globalOpacity': fd.get('global_opacity'),
-            'viewportLongitude': fd.get('viewport_longitude'),
-            'viewportLatitude': fd.get('viewport_latitude'),
-            'viewportZoom': fd.get('viewport_zoom'),
+            'bounds': [south_west, north_east],
             'renderWhileDragging': fd.get('render_while_dragging'),
             'tooltip': fd.get('rich_tooltip'),
             'color': fd.get('mapbox_color'),