You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ju...@apache.org on 2021/02/15 16:06:42 UTC

[superset] branch master updated: fix: Fix switching viz type to and from Filter box (#13094)

This is an automated email from the ASF dual-hosted git repository.

junlin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new d0b00bc  fix: Fix switching viz type to and from Filter box (#13094)
d0b00bc is described below

commit d0b00bc08f273ef7f9d5728de2867aa1bb0ee346
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Mon Feb 15 17:05:40 2021 +0100

    fix: Fix switching viz type to and from Filter box (#13094)
    
    * Fix switching viz_type to and from filter_box
    
    * Remove RESET_FIELDS action
    
    * Restore removed log
    
    * Refactor reducer logic
---
 .../src/explore/actions/exploreActions.ts          |  6 ----
 .../src/explore/components/ExploreChartPanel.jsx   | 10 ++-----
 .../explore/components/ExploreViewContainer.jsx    | 34 +++++++++++++---------
 .../src/explore/reducers/exploreReducer.js         | 24 ++++++++-------
 4 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/superset-frontend/src/explore/actions/exploreActions.ts b/superset-frontend/src/explore/actions/exploreActions.ts
index 9d53f75..4923ae3 100644
--- a/superset-frontend/src/explore/actions/exploreActions.ts
+++ b/superset-frontend/src/explore/actions/exploreActions.ts
@@ -61,11 +61,6 @@ export function fetchDatasourcesSucceeded() {
   return { type: FETCH_DATASOURCES_SUCCEEDED };
 }
 
-export const RESET_FIELDS = 'RESET_FIELDS';
-export function resetControls() {
-  return { type: RESET_FIELDS };
-}
-
 export const TOGGLE_FAVE_STAR = 'TOGGLE_FAVE_STAR';
 export function toggleFaveStar(isStarred: boolean) {
   return { type: TOGGLE_FAVE_STAR, isStarred };
@@ -154,7 +149,6 @@ export const exploreActions = {
   setDatasources,
   fetchDatasourcesStarted,
   fetchDatasourcesSucceeded,
-  resetControls,
   toggleFaveStar,
   fetchFaveStar,
   saveFaveStar,
diff --git a/superset-frontend/src/explore/components/ExploreChartPanel.jsx b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
index 6b86b30..6962dfa 100644
--- a/superset-frontend/src/explore/components/ExploreChartPanel.jsx
+++ b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
@@ -211,12 +211,8 @@ const ExploreChartPanel = props => {
   }, [calcSectionHeight, chartWidth, props, splitSizes]);
 
   const panelBody = useMemo(
-    () => (
-      <div className="panel-body" ref={chartRef}>
-        {renderChart()}
-      </div>
-    ),
-    [chartRef, renderChart],
+    () => <div className="panel-body">{renderChart()}</div>,
+    [renderChart],
   );
 
   const standaloneChartBody = useMemo(
@@ -256,7 +252,7 @@ const ExploreChartPanel = props => {
   });
 
   return (
-    <Styles className="panel panel-default chart-container">
+    <Styles className="panel panel-default chart-container" ref={chartRef}>
       <div className="panel-heading" ref={headerRef}>
         {header}
       </div>
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer.jsx b/superset-frontend/src/explore/components/ExploreViewContainer.jsx
index 9c605d4..bb50812 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer.jsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer.jsx
@@ -17,7 +17,7 @@
  * under the License.
  */
 /* eslint camelcase: 0 */
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 import PropTypes from 'prop-types';
 import { bindActionCreators } from 'redux';
 import { connect } from 'react-redux';
@@ -165,7 +165,6 @@ function ExploreViewContainer(props) {
   const windowSize = useWindowSize();
 
   const [showingModal, setShowingModal] = useState(false);
-  const [chartIsStale, setChartIsStale] = useState(false);
   const [isCollapsed, setIsCollapsed] = useState(false);
 
   const width = `${windowSize.width}px`;
@@ -226,7 +225,6 @@ function ExploreViewContainer(props) {
     props.actions.removeControlPanelAlert();
     props.actions.triggerQuery(true, props.chart.id);
 
-    setChartIsStale(false);
     addHistory();
   }
 
@@ -301,9 +299,6 @@ function ExploreViewContainer(props) {
   // effect to run when controls change
   useEffect(() => {
     if (previousControls) {
-      if (props.controls.viz_type.value !== previousControls.viz_type.value) {
-        props.actions.resetControls();
-      }
       if (
         props.controls.datasource &&
         (previousControls.datasource == null ||
@@ -334,19 +329,32 @@ function ExploreViewContainer(props) {
         props.actions.renderTriggered(new Date().getTime(), props.chart.id);
         addHistory();
       }
+    }
+  }, [props.controls]);
 
-      // this should be handled inside actions too
-      const hasQueryControlChanged = changedControlKeys.some(
+  const chartIsStale = useMemo(() => {
+    if (previousControls) {
+      const changedControlKeys = Object.keys(props.controls).filter(
+        key =>
+          typeof previousControls[key] !== 'undefined' &&
+          !areObjectsEqual(
+            props.controls[key].value,
+            previousControls[key].value,
+          ),
+      );
+
+      return changedControlKeys.some(
         key =>
           !props.controls[key].renderTrigger &&
           !props.controls[key].dontRefreshOnChange,
       );
-      if (hasQueryControlChanged) {
-        props.actions.logEvent(LOG_ACTIONS_CHANGE_EXPLORE_CONTROLS);
-        setChartIsStale(true);
-      }
     }
-  }, [props.controls]);
+    return false;
+  }, [previousControls, props.controls]);
+
+  if (chartIsStale) {
+    props.actions.logEvent(LOG_ACTIONS_CHANGE_EXPLORE_CONTROLS);
+  }
 
   function renderErrorMessage() {
     // Returns an error message as a node if any errors are in the store
diff --git a/superset-frontend/src/explore/reducers/exploreReducer.js b/superset-frontend/src/explore/reducers/exploreReducer.js
index e8167f9..3076727 100644
--- a/superset-frontend/src/explore/reducers/exploreReducer.js
+++ b/superset-frontend/src/explore/reducers/exploreReducer.js
@@ -121,12 +121,25 @@ export default function exploreReducer(state = {}, action) {
       });
       const hasErrors = errors && errors.length > 0;
 
+      const currentControlsState =
+        action.controlName === 'viz_type' &&
+        action.value !== state.controls.viz_type.value
+          ? // rebuild the full control state if switching viz type
+            getControlsState(
+              state,
+              getFormDataFromControls({
+                ...state.controls,
+                viz_type: control,
+              }),
+            )
+          : state.controls;
+
       return {
         ...state,
         form_data: new_form_data,
         triggerRender: control.renderTrigger && !hasErrors,
         controls: {
-          ...state.controls,
+          ...currentControlsState,
           [action.controlName]: {
             ...control,
             validationErrors: errors,
@@ -146,15 +159,6 @@ export default function exploreReducer(state = {}, action) {
         sliceName: action.sliceName,
       };
     },
-    [actions.RESET_FIELDS]() {
-      return {
-        ...state,
-        controls: getControlsState(
-          state,
-          getFormDataFromControls(state.controls),
-        ),
-      };
-    },
     [actions.CREATE_NEW_SLICE]() {
       return {
         ...state,