From 9fa52b50edaa2e42b7d064139ddb41d329aa1344 Mon Sep 17 00:00:00 2001
From: Ville Brofeldt <33317356+villebro@users.noreply.github.com>
Date: Mon, 29 Mar 2021 10:23:01 +0300
Subject: [PATCH] fix(native-filters): improve loading styles for filter
 component (#13794)

* fix(native-filters): improve loading styles for filter component

* round up fractional grid unit

* remove LoadingBox
---
 .../src/components/Loading/index.tsx          | 11 +++-
 .../FilterBar/FilterControls/FilterValue.tsx  | 41 +++++-------
 .../FiltersConfigForm/DefaultValue.tsx        | 64 +++++++++++--------
 .../FiltersConfigForm/state.ts                |  5 ++
 4 files changed, 70 insertions(+), 51 deletions(-)

diff --git a/superset-frontend/src/components/Loading/index.tsx b/superset-frontend/src/components/Loading/index.tsx
index d62535602..6ba6fb45c 100644
--- a/superset-frontend/src/components/Loading/index.tsx
+++ b/superset-frontend/src/components/Loading/index.tsx
@@ -21,7 +21,11 @@ import React from 'react';
 import { styled } from '@superset-ui/core';
 import cls from 'classnames';
 
-export type PositionOption = 'normal' | 'inline' | 'floating';
+export type PositionOption =
+  | 'floating'
+  | 'inline'
+  | 'inline-centered'
+  | 'normal';
 export interface Props {
   position?: PositionOption;
   className?: string;
@@ -37,6 +41,11 @@ const LoaderImg = styled.img`
     margin: 0px;
     width: 30px;
   }
+  &.inline-centered {
+    margin: 0 auto;
+    width: 30px;
+    display: block;
+  }
   &.floating {
     padding: 0;
     margin: 0;
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
index 6e7a4eaca..ee19655ce 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
@@ -33,14 +33,9 @@ import { FilterProps } from './types';
 import { getFormData } from '../../utils';
 import { useCascadingFilters } from './state';
 
-const StyledLoadingBox = styled.div`
-  position: relative;
-  height: ${({ theme }) => theme.gridUnit * 8}px;
-  margin-bottom: ${({ theme }) => theme.gridUnit * 6}px;
-`;
-
 const FilterItem = styled.div`
-  padding-bottom: 10px;
+  min-height: ${({ theme }) => theme.gridUnit * 11}px;
+  padding-bottom: ${({ theme }) => theme.gridUnit * 3}px;
 `;
 
 const FilterValue: React.FC<FilterProps> = ({
@@ -112,14 +107,6 @@ const FilterValue: React.FC<FilterProps> = ({
   const setDataMask = (dataMask: DataMask) =>
     onFilterSelectionChange(filter, dataMask);
 
-  if (loading) {
-    return (
-      <StyledLoadingBox>
-        <Loading />
-      </StyledLoadingBox>
-    );
-  }
-
   if (error) {
     return (
       <BasicErrorAlert
@@ -132,16 +119,20 @@ const FilterValue: React.FC<FilterProps> = ({
 
   return (
     <FilterItem data-test="form-item-value">
-      <SuperChart
-        height={20}
-        width={220}
-        formData={formData}
-        // For charts that don't have datasource we need workaround for empty placeholder
-        queriesData={hasDataSource ? state : [{ data: [{}] }]}
-        chartType={filterType}
-        behaviors={[Behavior.NATIVE_FILTER]}
-        hooks={{ setDataMask }}
-      />
+      {loading ? (
+        <Loading position="inline-centered" />
+      ) : (
+        <SuperChart
+          height={20}
+          width={220}
+          formData={formData}
+          // For charts that don't have datasource we need workaround for empty placeholder
+          queriesData={hasDataSource ? state : [{ data: [{}] }]}
+          chartType={filterType}
+          behaviors={[Behavior.NATIVE_FILTER]}
+          hooks={{ setDataMask }}
+        />
+      )}
     </FilterItem>
   );
 };
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx
index 4d0dcdd4f..e9067166b 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx
@@ -16,9 +16,10 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React, { FC } from 'react';
+import React, { FC, useEffect, useState } from 'react';
 import { t, SuperChart, Behavior } from '@superset-ui/core';
 import { FormInstance } from 'antd/lib/form';
+import Loading from 'src/components/Loading';
 import { setNativeFilterFieldValues } from './utils';
 import { StyledFormItem, StyledLabel } from './FiltersConfigForm';
 import { Filter } from '../../types';
@@ -44,7 +45,18 @@ const DefaultValue: FC<DefaultValueProps> = ({
   forceUpdate,
   formData,
 }) => {
+  const [loading, setLoading] = useState(hasDatasource);
   const formFilter = (form.getFieldValue('filters') || {})[filterId];
+  const queriesData = formFilter?.defaultValueQueriesData;
+
+  useEffect(() => {
+    if (!hasDatasource || queriesData !== null) {
+      setLoading(false);
+    } else {
+      setLoading(true);
+    }
+  }, [hasDatasource, queriesData]);
+
   return (
     <StyledFormItem
       name={['filters', filterId, 'defaultValue']}
@@ -52,30 +64,32 @@ const DefaultValue: FC<DefaultValueProps> = ({
       data-test="default-input"
       label={<StyledLabel>{t('Default Value')}</StyledLabel>}
     >
-      {((hasFilledDatasource && formFilter?.defaultValueQueriesData) ||
-        !hasDatasource) && (
-        <SuperChart
-          height={25}
-          width={250}
-          behaviors={[Behavior.NATIVE_FILTER]}
-          formData={formData}
-          // For charts that don't have datasource we need workaround for empty placeholder
-          queriesData={
-            hasDatasource
-              ? formFilter?.defaultValueQueriesData
-              : [{ data: [null] }]
-          }
-          chartType={formFilter?.filterType}
-          hooks={{
-            setDataMask: ({ nativeFilters }) => {
-              setNativeFilterFieldValues(form, filterId, {
-                defaultValue: nativeFilters?.currentState?.value,
-              });
-              forceUpdate();
-            },
-          }}
-        />
-      )}
+      {(hasFilledDatasource || !hasDatasource) &&
+        (loading ? (
+          <Loading position="inline-centered" />
+        ) : (
+          <SuperChart
+            height={25}
+            width={250}
+            behaviors={[Behavior.NATIVE_FILTER]}
+            formData={formData}
+            // For charts that don't have datasource we need workaround for empty placeholder
+            queriesData={
+              hasDatasource
+                ? formFilter?.defaultValueQueriesData
+                : [{ data: [{}] }]
+            }
+            chartType={formFilter?.filterType}
+            hooks={{
+              setDataMask: ({ nativeFilters }) => {
+                setNativeFilterFieldValues(form, filterId, {
+                  defaultValue: nativeFilters?.currentState?.value,
+                });
+                forceUpdate();
+              },
+            }}
+          />
+        ))}
     </StyledFormItem>
   );
 };
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts
index d30442226..e9affd859 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts
@@ -60,6 +60,11 @@ export const useBackendFormUpdate = (
       defaultValue: formFilter?.defaultValue,
       ...formFilter,
     });
+    setNativeFilterFieldValues(form, filterId, {
+      defaultValueQueriesData: null,
+      defaultValue: resolvedDefaultValue,
+    });
+    forceUpdate();
     getChartDataRequest({
       formData,
       force: false,