This is an automated email from the ASF dual-hosted git repository. villebro pushed a commit to branch 0.37 in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
commit 820d07d6cf0c934a1d6bf6aa2653ac600d8206eb Author: Jesse Yang <[email protected]> AuthorDate: Tue Aug 11 00:57:50 2020 -0700 fix(dashboard): add animation state to fix tab switch re-renders (#10475) --- .../components/gridComponents/Chart_spec.jsx | 10 +++++++- superset-frontend/src/chart/Chart.jsx | 2 ++ superset-frontend/src/chart/ChartRenderer.jsx | 7 ++---- .../src/dashboard/actions/dashboardState.js | 8 +++++++ .../src/dashboard/components/DashboardBuilder.jsx | 7 ++++++ .../dashboard/components/gridComponents/Chart.jsx | 27 ++++++++++++++++++++-- .../dashboard/components/gridComponents/Tabs.jsx | 11 ++++++++- .../src/dashboard/containers/DashboardBuilder.jsx | 2 ++ .../dashboard/containers/DashboardComponent.jsx | 3 ++- .../src/dashboard/reducers/dashboardState.js | 10 ++++++++ 10 files changed, 77 insertions(+), 10 deletions(-) diff --git a/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx index d75b5fa..e7f3e42 100644 --- 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, diff --git a/superset-frontend/src/chart/Chart.jsx b/superset-frontend/src/chart/Chart.jsx index 9562304..5991a00 100644 --- a/superset-frontend/src/chart/Chart.jsx +++ b/superset-frontend/src/chart/Chart.jsx @@ -62,6 +62,8 @@ const propTypes = { onQuery: PropTypes.func, onFilterMenuOpen: PropTypes.func, onFilterMenuClose: PropTypes.func, + // id of the last mounted parent tab + mountedParent: PropTypes.string, }; const BLANK = {}; diff --git a/superset-frontend/src/chart/ChartRenderer.jsx b/superset-frontend/src/chart/ChartRenderer.jsx index e45130a..984e1b3 100644 --- a/superset-frontend/src/chart/ChartRenderer.jsx +++ b/superset-frontend/src/chart/ChartRenderer.jsx @@ -87,8 +87,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 || @@ -96,9 +95,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; } diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 33737f8..5ecc335 100644 --- 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 }; diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder.jsx b/superset-frontend/src/dashboard/components/DashboardBuilder.jsx index de9a94e..da3217e 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder.jsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder.jsx @@ -62,6 +62,7 @@ const propTypes = { handleComponentDrop: PropTypes.func.isRequired, directPathToChild: PropTypes.arrayOf(PropTypes.string), setDirectPathToChild: PropTypes.func.isRequired, + setMountedTab: PropTypes.func.isRequired, }; const defaultProps = { @@ -250,6 +251,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]} diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index 4a443f1..de0e6e6 100644 --- 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; } @@ -346,4 +353,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); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx index 8c812c4..0ac44fb 100644 --- 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 diff --git a/superset-frontend/src/dashboard/containers/DashboardBuilder.jsx b/superset-frontend/src/dashboard/containers/DashboardBuilder.jsx index 9887295..a508b5a 100644 --- 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, @@ -49,6 +50,7 @@ function mapDispatchToProps(dispatch) { showBuilderPane, setColorSchemeAndUnsavedChanges, setDirectPathToChild, + setMountedTab, }, dispatch, ); diff --git a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx index e0eb38a..d4a403f 100644 --- 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, diff --git a/superset-frontend/src/dashboard/reducers/dashboardState.js b/superset-frontend/src/dashboard/reducers/dashboardState.js index fc7e079..32ce82d 100644 --- 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'; import { BUILDER_PANE_TYPE } from '../util/constants'; @@ -127,10 +128,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) {
