Geoviz state management fix (#6260)

* Fix deckgl getPoints

* Fix CSS

* Fix zoom

* Fix CategoricalDeckGLContainer

* Fix cypress

.flaskenv

@@ -1 +1,2 @@
 FLASK_APP=superset:app
+FLASK_ENV=development

superset/assets/cypress_build.sh

@@ -5,7 +5,7 @@ superset/bin/superset db upgrade
 superset/bin/superset load_test_users
 superset/bin/superset load_examples
 superset/bin/superset init
-superset/bin/superset runserver &
+flask run -p 8081 --with-threads --reload --debugger &
 
 cd "$(dirname "$0")"
 

superset/assets/spec/javascripts/visualizations/deckgl/utils_spec.js

@@ -11,7 +11,7 @@ describe('getBreakPoints', () => {
 
   it('returns sorted break points', () => {
     const fd = { break_points: ['0', '10', '100', '50', '1000'] };
-    const result = getBreakPoints(fd);
+    const result = getBreakPoints(fd, []);
     const expected = ['0', '10', '50', '100', '1000'];
     expect(result).toEqual(expected);
   });

superset/assets/src/visualizations/PlaySlider.css

@@ -1,10 +1,19 @@
 .play-slider {
-    position: relative;
+    display: flex;
-    height: 20px;
+    height: 40px;
     width: 100%;
     margin: 0;
 }
 
+.play-slider-controls {
+    flex: 0 0 80px;
+    text-align: middle;
+}
+
+.play-slider-scrobbler {
+    flex: 1;
+}
+
 .slider.slider-horizontal {
     width: 100% !important;
 }

superset/assets/src/visualizations/PlaySlider.jsx

@@ -1,7 +1,6 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 import Mousetrap from 'mousetrap';
-import { Row, Col } from 'react-bootstrap';
 import { t } from '@superset-ui/translation';
 import BootrapSliderWrapper from '../components/BootstrapSliderWrapper';
 import './PlaySlider.css';
@@ -124,13 +123,13 @@ export default class PlaySlider extends React.PureComponent {
   render() {
     const { start, end, step, orientation, reversed, disabled, range, values } = this.props;
     return (
-      <Row className="play-slider">
+      <div className="play-slider">
-        <Col md={1} className="padded">
+        <div className="play-slider-controls padded">
           <i className="fa fa-step-backward fa-lg slider-button " onClick={this.stepBackward} />
           <i className={this.getPlayClass()} onClick={this.play} />
           <i className="fa fa-step-forward fa-lg slider-button " onClick={this.stepForward} />
-        </Col>
+        </div>
-        <Col md={11} className="padded">
+        <div className="play-slider-scrobbler padded">
           <BootrapSliderWrapper
             value={range ? values : values[0]}
             range={range}
@@ -143,8 +142,8 @@ export default class PlaySlider extends React.PureComponent {
             reversed={reversed}
             disabled={disabled ? 'disabled' : 'enabled'}
           />
-        </Col>
+        </div>
-      </Row>
+      </div>
     );
   }
 }

superset/assets/src/visualizations/deckgl/AnimatableDeckGLContainer.jsx

@@ -4,6 +4,8 @@ import PropTypes from 'prop-types';
 import DeckGLContainer from './DeckGLContainer';
 import PlaySlider from '../PlaySlider';
 
+const PLAYSLIDER_HEIGHT = 20;  // px
+
 const propTypes = {
   getLayers: PropTypes.func.isRequired,
   start: PropTypes.number.isRequired,
@@ -30,6 +32,14 @@ export default class AnimatableDeckGLContainer extends React.Component {
     super(props);
     const { getLayers, start, end, getStep, values, disabled, viewport, ...other } = props;
     this.other = other;
+
+    this.onViewportChange = this.onViewportChange.bind(this);
+  }
+  onViewportChange(viewport) {
+    const originalViewport = this.props.disabled
+      ? { ...viewport }
+      : { ...viewport, height: viewport.height + PLAYSLIDER_HEIGHT };
+    this.props.onViewportChange(originalViewport);
   }
   render() {
     const {
@@ -41,18 +51,24 @@ export default class AnimatableDeckGLContainer extends React.Component {
       children,
       getLayers,
       values,
-      viewport,
-      onViewportChange,
       onValuesChange,
+      viewport,
     } = this.props;
     const layers = getLayers(values);
+
+    // leave space for the play slider
+    const modifiedViewport = {
+      ...viewport,
+      height: disabled ? viewport.height : viewport.height - PLAYSLIDER_HEIGHT,
+    };
+
     return (
       <div>
         <DeckGLContainer
           {...this.other}
-          viewport={viewport}
+          viewport={modifiedViewport}
           layers={layers}
-          onViewportChange={onViewportChange}
+          onViewportChange={this.onViewportChange}
         />
         {!disabled &&
         <PlaySlider

superset/assets/src/visualizations/deckgl/CategoricalDeckGLContainer.jsx

@@ -8,6 +8,7 @@ import { getScale } from '../../modules/colors/CategoricalColorNamespace';
 import { hexToRGB } from '../../modules/colors';
 import { getPlaySliderParams } from '../../modules/time';
 import sandboxedEval from '../../modules/sandbox';
+import { fitViewport } from './layers/common';
 
 function getCategories(fd, data) {
   const c = fd.color_picker || { r: 0, g: 0, b: 0, a: 1 };
@@ -34,6 +35,7 @@ const propTypes = {
   setControlValue: PropTypes.func.isRequired,
   viewport: PropTypes.object.isRequired,
   getLayer: PropTypes.func.isRequired,
+  getPoints: PropTypes.func.isRequired,
   payload: PropTypes.object.isRequired,
   onAddFilter: PropTypes.func,
   setTooltip: PropTypes.func,
@@ -49,12 +51,7 @@ export default class CategoricalDeckGLContainer extends React.PureComponent {
   constructor(props) {
     super(props);
 
-    const fd = props.formData;
+    this.state = CategoricalDeckGLContainer.getDerivedStateFromProps(props);
-    const timeGrain = fd.time_grain_sqla || fd.granularity || 'PT1M';
-    const timestamps = props.payload.data.features.map(f => f.__timestamp);
-    const { start, end, getStep, values, disabled } = getPlaySliderParams(timestamps, timeGrain);
-    const categories = getCategories(fd, props.payload.data.features);
-    this.state = { start, end, getStep, values, disabled, categories, viewport: props.viewport };
 
     this.getLayers = this.getLayers.bind(this);
     this.onValuesChange = this.onValuesChange.bind(this);
@@ -62,6 +59,51 @@ export default class CategoricalDeckGLContainer extends React.PureComponent {
     this.toggleCategory = this.toggleCategory.bind(this);
     this.showSingleCategory = this.showSingleCategory.bind(this);
   }
+  static getDerivedStateFromProps(props, state) {
+    const features = props.payload.data.features || [];
+    const timestamps = features.map(f => f.__timestamp);
+    const categories = getCategories(props.formData, features);
+
+    // the state is computed only from the payload; if it hasn't changed, do
+    // not recompute state since this would reset selections and/or the play
+    // slider position due to changes in form controls
+    if (state && props.payload.form_data === state.formData) {
+      return { ...state, categories };
+    }
+
+    // the granularity has to be read from the payload form_data, not the
+    // props formData which comes from the instantaneous controls state
+    const granularity = (
+      props.payload.form_data.time_grain_sqla ||
+      props.payload.form_data.granularity ||
+      'P1D'
+    );
+
+    const {
+      start,
+      end,
+      getStep,
+      values,
+      disabled,
+    } = getPlaySliderParams(timestamps, granularity);
+
+    const viewport = props.formData.autozoom
+      ? fitViewport(props.viewport, props.getPoints(features))
+      : props.viewport;
+
+    return {
+      start,
+      end,
+      getStep,
+      values,
+      disabled,
+      viewport,
+      selected: [],
+      lastClick: 0,
+      formData: props.payload.form_data,
+      categories,
+    };
+  }
   onValuesChange(values) {
     this.setState({
       values: Array.isArray(values)
@@ -80,7 +122,9 @@ export default class CategoricalDeckGLContainer extends React.PureComponent {
       onAddFilter,
       setTooltip,
     } = this.props;
-    let features = [...payload.data.features];
+    let features = payload.data.features
+      ? [...payload.data.features]
+      : [];
 
     // Add colors from categories or fixed color
     features = this.addColor(features, fd);

superset/assets/src/visualizations/deckgl/DeckGLContainer.jsx

@@ -4,6 +4,8 @@ import MapGL from 'react-map-gl';
 import DeckGL from 'deck.gl';
 import 'mapbox-gl/dist/mapbox-gl.css';
 
+const TICK = 1000;  // milliseconds
+
 const propTypes = {
   viewport: PropTypes.object.isRequired,
   layers: PropTypes.array.isRequired,
@@ -22,42 +24,42 @@ export default class DeckGLContainer extends React.Component {
   constructor(props) {
     super(props);
     this.state = {
-      viewport: props.viewport,
+      previousViewport: props.viewport,
+      timer: setInterval(this.tick, TICK),
     };
     this.tick = this.tick.bind(this);
     this.onViewportChange = this.onViewportChange.bind(this);
   }
-  componentWillMount() {
+  static getDerivedStateFromProps(nextProps, prevState) {
-    const timer = setInterval(this.tick, 1000);
+    if (nextProps.viewport !== prevState.viewport) {
-    this.setState(() => ({ timer }));
+      return {
-  }
+        viewport: { ...nextProps.viewport },
-  componentWillReceiveProps(nextProps) {
+        previousViewport: prevState.viewport,
-    this.setState(() => ({
+      };
-      viewport: { ...nextProps.viewport },
+    }
-      previousViewport: this.state.viewport,
+    return null;
-    }));
   }
   componentWillUnmount() {
     clearInterval(this.state.timer);
   }
   onViewportChange(viewport) {
     const vp = Object.assign({}, viewport);
-    delete vp.width;
+    // delete vp.width;
-    delete vp.height;
+    // delete vp.height;
-    const newVp = { ...this.state.viewport, ...vp };
+    const newVp = { ...this.state.previousViewport, ...vp };
 
-    this.setState(() => ({ viewport: newVp }));
+    // this.setState(() => ({ viewport: newVp }));
     this.props.onViewportChange(newVp);
   }
   tick() {
     // Limiting updating viewport controls through Redux at most 1*sec
-    if (this.state.previousViewport !== this.state.viewport) {
+    if (this.state && this.state.previousViewport !== this.props.viewport) {
       const setCV = this.props.setControlValue;
-      const vp = this.state.viewport;
+      const vp = this.props.viewport;
       if (setCV) {
         setCV('viewport', vp);
       }
-      this.setState(() => ({ previousViewport: this.state.viewport }));
+      this.setState(() => ({ previousViewport: this.props.viewport }));
     }
   }
   layers() {
@@ -68,7 +70,7 @@ export default class DeckGLContainer extends React.Component {
     return this.props.layers;
   }
   render() {
-    const { viewport } = this.state;
+    const { viewport } = this.props;
     return (
       <MapGL
         {...viewport}

superset/assets/src/visualizations/deckgl/factory.jsx

@@ -59,13 +59,9 @@ export function createCategoricalDeckGLComponent(getLayer, getPoints) {
       setControlValue,
       onAddFilter,
       setTooltip,
-      viewport: originalViewport,
+      viewport,
     } = props;
 
-    const viewport = formData.autozoom
-      ? fitViewport(originalViewport, getPoints(payload.data.features))
-      : originalViewport;
-
     return (
       <CategoricalDeckGLContainer
         formData={formData}
@@ -76,6 +72,7 @@ export function createCategoricalDeckGLComponent(getLayer, getPoints) {
         payload={payload}
         onAddFilter={onAddFilter}
         setTooltip={setTooltip}
+        getPoints={getPoints}
       />
     );
   }

superset/assets/src/visualizations/deckgl/layers/Polygon/Polygon.jsx

@@ -9,12 +9,16 @@ import AnimatableDeckGLContainer from '../../AnimatableDeckGLContainer';
 import Legend from '../../../Legend';
 import { getBuckets, getBreakPointColorScaler } from '../../utils';
 
-import { commonLayerProps } from '../common';
+import { commonLayerProps, fitViewport } from '../common';
 import { getPlaySliderParams } from '../../../../modules/time';
 import sandboxedEval from '../../../../modules/sandbox';
 
 const DOUBLE_CLICK_TRESHOLD = 250;  // milliseconds
 
+function getPoints(features) {
+  return features.map(d => d.polygon).flat();
+}
+
 function getElevation(d, colorScaler) {
   /* in deck.gl 5.3.4 (used in Superset as of 2018-10-24), if a polygon has
    * opacity zero it will make everything behind it have opacity zero,
@@ -90,30 +94,60 @@ const defaultProps = {
   setTooltip() {},
 };
 
-class DeckGLPolygon extends React.PureComponent {
+class DeckGLPolygon extends React.Component {
   constructor(props) {
     super(props);
 
-    const fd = props.formData;
+    this.state = DeckGLPolygon.getDerivedStateFromProps(props);
-    const timeGrain = fd.time_grain_sqla || fd.granularity || 'PT1M';
-    const timestamps = props.payload.data.features.map(f => f.__timestamp);
-    const { start, end, getStep, values, disabled } = getPlaySliderParams(timestamps, timeGrain);
-    this.state = {
-      start,
-      end,
-      getStep,
-      values,
-      disabled,
-      viewport: props.viewport,
-      selected: [],
-      lastClick: 0,
-    };
 
     this.getLayers = this.getLayers.bind(this);
     this.onSelect = this.onSelect.bind(this);
     this.onValuesChange = this.onValuesChange.bind(this);
     this.onViewportChange = this.onViewportChange.bind(this);
   }
+  static getDerivedStateFromProps(props, state) {
+    // the state is computed only from the payload; if it hasn't changed, do
+    // not recompute state since this would reset selections and/or the play
+    // slider position due to changes in form controls
+    if (state && props.payload.form_data === state.formData) {
+      return null;
+    }
+
+    const features = props.payload.data.features || [];
+    const timestamps = features.map(f => f.__timestamp);
+
+    // the granularity has to be read from the payload form_data, not the
+    // props formData which comes from the instantaneous controls state
+    const granularity = (
+      props.payload.form_data.time_grain_sqla ||
+      props.payload.form_data.granularity ||
+      'P1D'
+    );
+
+    const {
+      start,
+      end,
+      getStep,
+      values,
+      disabled,
+    } = getPlaySliderParams(timestamps, granularity);
+
+    const viewport = props.formData.autozoom
+      ? fitViewport(props.viewport, getPoints(features))
+      : props.viewport;
+
+    return {
+      start,
+      end,
+      getStep,
+      values,
+      disabled,
+      viewport,
+      selected: [],
+      lastClick: 0,
+      formData: props.payload.form_data,
+    };
+  }
   onSelect(polygon) {
     const { formData, onAddFilter } = this.props;
 

superset/assets/src/visualizations/deckgl/layers/Screengrid/Screengrid.jsx

@@ -64,19 +64,55 @@ class DeckGLScreenGrid extends React.PureComponent {
   constructor(props) {
     super(props);
 
-    const fd = props.formData;
+    this.state = DeckGLScreenGrid.getDerivedStateFromProps(props);
-    const timeGrain = fd.time_grain_sqla || fd.granularity || 'PT1M';
-    const timestamps = props.payload.data.features.map(f => f.__timestamp);
-    const { start, end, getStep, values, disabled } = getPlaySliderParams(timestamps, timeGrain);
-    const viewport = fd.autozoom
-      ? fitViewport(props.viewport, getPoints(props.payload.data.features))
-      : props.viewport;
-    this.state = { start, end, getStep, values, disabled, viewport };
 
     this.getLayers = this.getLayers.bind(this);
     this.onValuesChange = this.onValuesChange.bind(this);
     this.onViewportChange = this.onViewportChange.bind(this);
   }
+  static getDerivedStateFromProps(props, state) {
+    // the state is computed only from the payload; if it hasn't changed, do
+    // not recompute state since this would reset selections and/or the play
+    // slider position due to changes in form controls
+    if (state && props.payload.form_data === state.formData) {
+      return null;
+    }
+
+    const features = props.payload.data.features || [];
+    const timestamps = features.map(f => f.__timestamp);
+
+    // the granularity has to be read from the payload form_data, not the
+    // props formData which comes from the instantaneous controls state
+    const granularity = (
+      props.payload.form_data.time_grain_sqla ||
+      props.payload.form_data.granularity ||
+      'P1D'
+    );
+
+    const {
+      start,
+      end,
+      getStep,
+      values,
+      disabled,
+    } = getPlaySliderParams(timestamps, granularity);
+
+    const viewport = props.formData.autozoom
+      ? fitViewport(props.viewport, getPoints(features))
+      : props.viewport;
+
+    return {
+      start,
+      end,
+      getStep,
+      values,
+      disabled,
+      viewport,
+      selected: [],
+      lastClick: 0,
+      formData: props.payload.form_data,
+    };
+  }
   onValuesChange(values) {
     this.setState({
       values: Array.isArray(values)

superset/assets/src/visualizations/deckgl/utils.js

@@ -7,6 +7,9 @@ export function getBreakPoints({
     num_buckets: formDataNumBuckets,
     metric,
   }, features) {
+  if (!features) {
+    return [];
+  }
   if (formDataBreakPoints === undefined || formDataBreakPoints.length === 0) {
     // compute evenly distributed break points based on number of buckets
     const numBuckets = formDataNumBuckets