fix(native-filters): default value checkbox in config modal (#15257)

* fix(native-filters): default value checkbox in config modal * disable checkbox if required * simplify styling logic * make default value mandatory if checked * address comments
2021-06-21 15:58:30 +03:00 · 2021-06-21 15:58:30 +03:00 · b28d7eace3
parent 2792ddc9b5
commit b28d7eace3
6 changed files with 86 additions and 34 deletions
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/CollapsibleControl.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/CollapsibleControl.tsx
@ -19,11 +19,14 @@
 import React, { ReactNode, useEffect, useState } from 'react';
 import { styled } from '@superset-ui/core';
 import { Checkbox } from 'src/common/components';
+import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';

 interface CollapsibleControlProps {
  initialValue?: boolean;
+  disabled?: boolean;
  checked?: boolean;
  title: string;
+  tooltip?: string;
  children: ReactNode;
  onChange?: (checked: boolean) => void;
 }
@ -48,7 +51,9 @@ const StyledContainer = styled.div<{ checked: boolean }>`
 const CollapsibleControl = (props: CollapsibleControlProps) => {
  const {
    checked,
+    disabled,
    title,
+    tooltip,
    children,
    onChange = () => {},
    initialValue = false,
@ -68,6 +73,7 @@ const CollapsibleControl = (props: CollapsibleControlProps) => {
      <Checkbox
        className="checkbox"
        checked={isChecked}
+        disabled={disabled}
        onChange={e => {
          const value = e.target.checked;
          // external `checked` value has more priority then local state
@ -78,7 +84,12 @@ const CollapsibleControl = (props: CollapsibleControlProps) => {
          onChange(value);
        }}
      >
-        {title}
+        <>
+          {title}&nbsp;
+          {tooltip && (
+            <InfoTooltipWithTrigger placement="top" tooltip={tooltip} />
+          )}
+        </>
      </Checkbox>
      {isChecked && children}
    </StyledContainer>
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@ -129,6 +129,20 @@ export const StyledRowFormItem = styled(FormItem)`
  }
 `;

+export const StyledRowSubFormItem = styled(FormItem)`
+  & .ant-form-item-label {
+    padding-bottom: 0;
+  }
+
+  .ant-form-item-control-input-content > div > div {
+    height: auto;
+  }
+
+  & .ant-form-item-control-input {
+    height: auto;
+  }
+`;
+
 export const StyledLabel = styled.span`
  color: ${({ theme }) => theme.colors.grayscale.base};
  font-size: ${({ theme }) => theme.typography.sizes.s}px;
@ -454,10 +468,12 @@ const FiltersConfigForm = (
    ...formFilter,
  });

-  const [hasDefaultValue, setHasDefaultValue] = useDefaultValue(
-    formFilter,
-    filterToEdit,
-  );
+  const [
+    hasDefaultValue,
+    isRequired,
+    defaultValueTooltip,
+    setHasDefaultValue,
+  ] = useDefaultValue(formFilter, filterToEdit);

  useEffect(() => {
    if (hasDataset && hasFilledDataset && hasDefaultValue && isDataDirty) {
@ -540,6 +556,8 @@ const FiltersConfigForm = (

  const hasAdhoc = formFilter?.adhoc_filters?.length > 0;

+  const defaultToFirstItem = formFilter?.controlValues?.defaultToFirstItem;
+
  const preFilterValidator = () => {
    if (hasTimeRange || hasAdhoc) {
      return Promise.resolve();
@ -713,26 +731,22 @@ const FiltersConfigForm = (
            <CollapsibleControl
              title={t('Filter has default value')}
              initialValue={hasDefaultValue}
+              disabled={isRequired || defaultToFirstItem}
+              tooltip={defaultValueTooltip}
              checked={hasDefaultValue}
              onChange={value => setHasDefaultValue(value)}
            >
-              <StyledRowFormItem
+              <StyledRowSubFormItem
                name={['filters', filterId, 'defaultDataMask']}
                initialValue={filterToEdit?.defaultDataMask}
                data-test="default-input"
                label={<StyledLabel>{t('Default Value')}</StyledLabel>}
-                required={formFilter?.controlValues?.enableEmptyFilter}
+                required={hasDefaultValue}
                rules={[
                  {
                    validator: (rule, value) => {
                      const hasValue = !!value?.filterState?.value;
-                      if (
-                        hasValue ||
-                        // TODO: do more generic
-                        formFilter.controlValues?.defaultToFirstItem ||
-                        // Not marked as required
-                        !formFilter.controlValues?.enableEmptyFilter
-                      ) {
+                      if (hasValue) {
                        return Promise.resolve();
                      }
                      return Promise.reject(
@ -775,7 +789,7 @@ const FiltersConfigForm = (
                ) : (
                  t('Fill all required fields to enable "Default Value"')
                )}
-              </StyledRowFormItem>
+              </StyledRowSubFormItem>
            </CollapsibleControl>
            {Object.keys(controlItems)
              .filter(key => BASIC_CONTROL_ITEMS.includes(key))
@ -814,7 +828,7 @@ const FiltersConfigForm = (
                    }
                  }}
                >
-                  <StyledRowFormItem
+                  <StyledRowSubFormItem
                    name={['filters', filterId, 'parentFilter']}
                    label={<StyledLabel>{t('Parent filter')}</StyledLabel>}
                    initialValue={parentFilter}
@ -832,7 +846,7 @@ const FiltersConfigForm = (
                      options={parentFilterOptions}
                      isClearable
                    />
-                  </StyledRowFormItem>
+                  </StyledRowSubFormItem>
                </CollapsibleControl>
              )}
              {Object.keys(controlItems)
@ -848,7 +862,7 @@ const FiltersConfigForm = (
                    }
                  }}
                >
-                  <StyledRowFormItem
+                  <StyledRowSubFormItem
                    name={['filters', filterId, 'adhoc_filters']}
                    initialValue={filterToEdit?.adhoc_filters}
                    required
@ -880,7 +894,7 @@ const FiltersConfigForm = (
                        </span>
                      }
                    />
-                  </StyledRowFormItem>
+                  </StyledRowSubFormItem>
                  {showTimeRangePicker && (
                    <StyledRowFormItem
                      name={['filters', filterId, 'time_range']}
@ -976,7 +990,7 @@ const FiltersConfigForm = (
                    />
                  </StyledFormItem>
                  {hasMetrics && (
-                    <StyledRowFormItem
+                    <StyledRowSubFormItem
                      name={['filters', filterId, 'sortMetric']}
                      initialValue={filterToEdit?.sortMetric}
                      label={
@ -1009,7 +1023,7 @@ const FiltersConfigForm = (
                          }
                        }}
                      />
-                    </StyledRowFormItem>
+                    </StyledRowSubFormItem>
                  )}
                </CollapsibleControl>
              )}
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx
@ -116,7 +116,7 @@ export default function getControlItemsMap({
                  forceUpdate();
                }}
              >
-                {controlItem.config.label}{' '}
+                {controlItem.config.label}&nbsp;
                {controlItem.config.description && (
                  <InfoTooltipWithTrigger
                    placement="top"
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts
@ -18,6 +18,7 @@
 */
 import { useCallback, useEffect, useState } from 'react';
 import { FormInstance } from 'antd/lib/form';
+import { t } from '@superset-ui/core';
 import { NativeFiltersForm, NativeFiltersFormItem } from '../types';
 import { setNativeFilterFieldValues, useForceUpdate } from './utils';
 import { Filter } from '../../types';
@ -56,20 +57,47 @@ export const useDefaultValue = (
    !!filterToEdit?.defaultDataMask?.filterState?.value ||
      formFilter?.controlValues?.enableEmptyFilter,
  );
+  const [isRequired, setisRequired] = useState(
+    formFilter?.controlValues?.enableEmptyFilter,
+  );
+
+  const [defaultValueTooltip, setDefaultValueTooltip] = useState('');
+
+  const defaultToFirstItem = formFilter?.controlValues?.defaultToFirstItem;
+
  const setHasDefaultValue = useCallback(
    (value?) => {
-      setHasPartialDefaultValue(
-        value || formFilter?.controlValues?.enableEmptyFilter
-          ? true
-          : undefined,
-      );
+      const required =
+        !!formFilter?.controlValues?.enableEmptyFilter && !defaultToFirstItem;
+      setisRequired(required);
+      setHasPartialDefaultValue(required ? true : value);
    },
-    [formFilter?.controlValues?.enableEmptyFilter],
+    [formFilter?.controlValues?.enableEmptyFilter, defaultToFirstItem],
  );

  useEffect(() => {
-    setHasDefaultValue();
-  }, [setHasDefaultValue]);
+    setHasDefaultValue(
+      defaultToFirstItem
+        ? false
+        : !!formFilter?.defaultDataMask?.filterState?.value,
+    );
+  }, [setHasDefaultValue, defaultToFirstItem]);

-  return [hasDefaultValue, setHasDefaultValue];
+  useEffect(() => {
+    let tooltip = '';
+    if (defaultToFirstItem) {
+      tooltip = t(
+        'Default value set automatically when "Default to first item" is checked',
+      );
+    } else if (isRequired) {
+      tooltip = t('Default value must be set when "Required" is checked');
+    } else if (hasDefaultValue) {
+      tooltip = t(
+        'Default value must be set when "Filter has default value" is checked',
+      );
+    }
+    setDefaultValueTooltip(tooltip);
+  }, [hasDefaultValue, isRequired, defaultToFirstItem]);
+
+  return [hasDefaultValue, isRequired, defaultValueTooltip, setHasDefaultValue];
 };
--- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
@ -284,7 +284,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
        extra={<Error>{filterState.validateMessage}</Error>}
      >
        <StyledSelect
-          allowClear={!enableEmptyFilter}
+          allowClear
          // @ts-ignore
          value={filterState.value || []}
          disabled={isDisabled}
--- a/superset-frontend/src/filters/components/Select/controlPanel.ts
+++ b/superset-frontend/src/filters/components/Select/controlPanel.ts
@ -77,8 +77,7 @@ const config: ControlPanelConfig = {
              default: enableEmptyFilter,
              renderTrigger: true,
              description: t(
-                'User must select a value for this filter when filter is in single select mode. ' +
-                  'If selection is empty, an always false filter is emitted.',
+                'User must select a value before applying the filter',
              ),
            },
          },