fix(dashboard): add animation state to fix tab switch re-renders (#10475)

2020-08-11 00:57:50 -07:00 · 2020-08-11 00:57:50 -07:00 · a37b635674
parent 613dd12fbf
commit a37b635674
10 changed files with 77 additions and 10 deletions
--- a/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
+++ b/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
@ -20,7 +20,7 @@ import React from 'react';
 import { shallow } from 'enzyme';
 import sinon from 'sinon';

-import Chart from 'src/dashboard/components/gridComponents/Chart';
+import { ChartUnconnected as Chart } from 'src/dashboard/components/gridComponents/Chart';
 import SliceHeader from 'src/dashboard/components/SliceHeader';
 import ChartContainer from 'src/chart/ChartContainer';

@ -44,6 +44,7 @@ describe('Chart', () => {
    slice: {
      ...sliceEntities.slices[queryId],
      description_markeddown: 'markdown',
+      owners: [],
    },
    sliceName: sliceEntities.slices[queryId].slice_name,
    timeout: 60,
@ -52,6 +53,13 @@ describe('Chart', () => {
    toggleExpandSlice() {},
    addFilter() {},
    logEvent() {},
+    handleToggleFullSize() {},
+    changeFilter() {},
+    setFocusedFilterField() {},
+    unsetFocusedFilterField() {},
+    addDangerToast() {},
+    componentId: 'test',
+    dashboardId: 111,
    editMode: false,
    isExpanded: false,
    supersetCanExplore: false,
--- a/superset-frontend/src/chart/Chart.jsx
+++ b/superset-frontend/src/chart/Chart.jsx
@ -63,6 +63,8 @@ const propTypes = {
  onQuery: PropTypes.func,
  onFilterMenuOpen: PropTypes.func,
  onFilterMenuClose: PropTypes.func,
+  // id of the last mounted parent tab
+  mountedParent: PropTypes.string,
 };

 const BLANK = {};
--- a/superset-frontend/src/chart/ChartRenderer.jsx
+++ b/superset-frontend/src/chart/ChartRenderer.jsx
@ -86,8 +86,7 @@ class ChartRenderer extends React.Component {
    if (resultsReady) {
      this.hasQueryResponseChange =
        nextProps.queryResponse !== this.props.queryResponse;
-
-      if (
+      return (
        this.hasQueryResponseChange ||
        nextProps.annotationData !== this.props.annotationData ||
        nextProps.height !== this.props.height ||
@ -95,9 +94,7 @@ class ChartRenderer extends React.Component {
        nextProps.triggerRender ||
        nextProps.formData.color_scheme !== this.props.formData.color_scheme ||
        nextProps.cacheBusterProp !== this.props.cacheBusterProp
-      ) {
-        return true;
-      }
+      );
    }
    return false;
  }
--- a/superset-frontend/src/dashboard/actions/dashboardState.js
+++ b/superset-frontend/src/dashboard/actions/dashboardState.js
@ -321,6 +321,14 @@ export function setDirectPathToChild(path) {
  return { type: SET_DIRECT_PATH, path };
 }

+export const SET_MOUNTED_TAB = 'SET_MOUNTED_TAB';
+/**
+ * Set if tab switch animation is in progress
+ */
+export function setMountedTab(mountedTab) {
+  return { type: SET_MOUNTED_TAB, mountedTab };
+}
+
 export const SET_FOCUSED_FILTER_FIELD = 'SET_FOCUSED_FILTER_FIELD';
 export function setFocusedFilterField(chartId, column) {
  return { type: SET_FOCUSED_FILTER_FIELD, chartId, column };
--- a/superset-frontend/src/dashboard/components/DashboardBuilder.jsx
+++ b/superset-frontend/src/dashboard/components/DashboardBuilder.jsx
@ -60,6 +60,7 @@ const propTypes = {
  handleComponentDrop: PropTypes.func.isRequired,
  directPathToChild: PropTypes.arrayOf(PropTypes.string),
  setDirectPathToChild: PropTypes.func.isRequired,
+  setMountedTab: PropTypes.func.isRequired,
 };

 const defaultProps = {
@ -247,6 +248,12 @@ class DashboardBuilder extends React.Component {
                      <TabPane
                        key={index === 0 ? DASHBOARD_GRID_ID : id}
                        eventKey={index}
+                        mountOnEnter
+                        unmountOnExit={false}
+                        onEntering={() => {
+                          // Entering current tab, DOM is visible and has dimension
+                          this.props.setMountedTab(id);
+                        }}
                      >
                        <DashboardGrid
                          gridComponent={dashboardLayout[id]}
--- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
@ -18,6 +18,7 @@
 */
 import cx from 'classnames';
 import React from 'react';
+import { connect } from 'react-redux';
 import PropTypes from 'prop-types';
 import { exploreChart, exportChart } from '../../../explore/exploreUtils';
 import SliceHeader from '../SliceHeader';
@ -41,6 +42,8 @@ const propTypes = {
  height: PropTypes.number.isRequired,
  updateSliceName: PropTypes.func.isRequired,
  isComponentVisible: PropTypes.bool,
+  // last switched tab
+  mountedParent: PropTypes.string,
  handleToggleFullSize: PropTypes.func.isRequired,

  // from redux
@ -70,6 +73,7 @@ const propTypes = {
 const defaultProps = {
  isCached: false,
  isComponentVisible: true,
+  mountedParent: 'ROOT',
 };

 // we use state + shouldComponentUpdate() logic to prevent perf-wrecking
@ -114,6 +118,9 @@ class Chart extends React.Component {
    // allow chart update/re-render only if visible:
    // under selected tab or no tab layout
    if (nextProps.isComponentVisible) {
+      if (nextProps.mountedParent === null) {
+        return false;
+      }
      if (nextProps.chart.triggerQuery) {
        return true;
      }
@ -140,7 +147,7 @@ class Chart extends React.Component {
      }
    }

-    // `cacheBusterProp` is nnjected by react-hot-loader
+    // `cacheBusterProp` is jected by react-hot-loader
    return this.props.cacheBusterProp !== nextProps.cacheBusterProp;
  }

@ -347,4 +354,20 @@ class Chart extends React.Component {
 Chart.propTypes = propTypes;
 Chart.defaultProps = defaultProps;

-export default Chart;
+function mapStateToProps({ dashboardState }) {
+  return {
+    // needed to prevent chart from rendering while tab switch animation in progress
+    // when undefined, default to have mounted the root tab
+    mountedParent: dashboardState?.mountedTab,
+  };
+}
+
+/**
+ * The original Chart component not connected to state.
+ */
+export const ChartUnconnected = Chart;
+
+/**
+ * Redux connected Chart component.
+ */
+export default connect(mapStateToProps, null)(Chart);
--- a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx
@ -47,9 +47,12 @@ const propTypes = {
  renderTabContent: PropTypes.bool, // whether to render tabs + content or just tabs
  editMode: PropTypes.bool.isRequired,
  renderHoverMenu: PropTypes.bool,
-  logEvent: PropTypes.func.isRequired,
  directPathToChild: PropTypes.arrayOf(PropTypes.string),

+  // actions (from DashboardComponent.jsx)
+  logEvent: PropTypes.func.isRequired,
+  setMountedTab: PropTypes.func.isRequired,
+
  // grid related
  availableColumnCount: PropTypes.number,
  columnWidth: PropTypes.number,
@ -260,6 +263,12 @@ class Tabs extends React.PureComponent {
                      onDeleteTab={this.handleDeleteTab}
                    />
                  }
+                  onEntering={() => {
+                    // Entering current tab, DOM is visible and has dimension
+                    if (renderTabContent) {
+                      this.props.setMountedTab(tabId);
+                    }
+                  }}
                >
                  {renderTabContent && (
                    <DashboardComponent
--- a/superset-frontend/src/dashboard/containers/DashboardBuilder.jsx
+++ b/superset-frontend/src/dashboard/containers/DashboardBuilder.jsx
@ -24,6 +24,7 @@ import {
  setColorSchemeAndUnsavedChanges,
  showBuilderPane,
  setDirectPathToChild,
+  setMountedTab,
 } from '../actions/dashboardState';
 import {
  deleteTopLevelTabs,
@ -48,6 +49,7 @@ function mapDispatchToProps(dispatch) {
      showBuilderPane,
      setColorSchemeAndUnsavedChanges,
      setDirectPathToChild,
+      setMountedTab,
    },
    dispatch,
  );
--- a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx
+++ b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx
@ -33,7 +33,7 @@ import {
  updateComponents,
  handleComponentDrop,
 } from '../actions/dashboardLayout';
-import { setDirectPathToChild } from '../actions/dashboardState';
+import { setDirectPathToChild, setMountedTab } from '../actions/dashboardState';
 import { logEvent } from '../../logger/actions';
 import { addDangerToast } from '../../messageToasts/actions';

@ -106,6 +106,7 @@ function mapDispatchToProps(dispatch) {
      updateComponents,
      handleComponentDrop,
      setDirectPathToChild,
+      setMountedTab,
      logEvent,
    },
    dispatch,
--- a/superset-frontend/src/dashboard/reducers/dashboardState.js
+++ b/superset-frontend/src/dashboard/reducers/dashboardState.js
@ -33,6 +33,7 @@ import {
  UPDATE_CSS,
  SET_REFRESH_FREQUENCY,
  SET_DIRECT_PATH,
+  SET_MOUNTED_TAB,
  SET_FOCUSED_FILTER_FIELD,
 } from '../actions/dashboardState';

@ -122,10 +123,19 @@ export default function dashboardStateReducer(state = {}, action) {
    [SET_DIRECT_PATH]() {
      return {
        ...state,
+        // change of direct path (tabs) will reset current mounted tab
+        mountedTab: null,
        directPathToChild: action.path,
        directPathLastUpdated: Date.now(),
      };
    },
+    [SET_MOUNTED_TAB]() {
+      // set current mounted tab after tab is really mounted to DOM
+      return {
+        ...state,
+        mountedTab: action.mountedTab,
+      };
+    },
    [SET_FOCUSED_FILTER_FIELD]() {
      const { focusedFilterField } = state;
      if (action.chartId && action.column) {