chore: Improves the native filters UI/UX - iteration 6 (#14932)

2021-06-02 03:03:13 -03:00 · 2021-06-02 03:03:13 -03:00 · b6f00e69e1
parent 06945ccbcf
commit b6f00e69e1
5 changed files with 137 additions and 84 deletions
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTabs.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTabs.tsx
@ -24,7 +24,7 @@ import Icon from 'src/components/Icon';
 import { FilterRemoval } from './types';
 import { REMOVAL_DELAY_SECS } from './utils';

-export const FILTER_WIDTH = 200;
+export const FILTER_WIDTH = 180;

 export const StyledSpan = styled.span`
  cursor: pointer;
@ -115,6 +115,7 @@ const FilterTabsContainer = styled(LineEditableTabs)`
      padding-right: ${theme.gridUnit}px;
      padding-bottom: ${theme.gridUnit * 3}px;
      padding-left: ${theme.gridUnit * 3}px;
+      width: 270px;
    }

    // extra selector specificity:
@ -185,6 +186,8 @@ const FilterTabs: FC<FilterTabsProps> = ({
  children,
 }) => (
  <FilterTabsContainer
+    id="native-filters-tabs"
+    type="editable-card"
    tabPosition="left"
    onChange={onChange}
    activeKey={currentFilterId}
@ -193,7 +196,20 @@ const FilterTabs: FC<FilterTabsProps> = ({
    tabBarExtraContent={{
      left: <StyledHeader>{t('Filters')}</StyledHeader>,
      right: (
-        <StyledAddFilterBox onClick={() => onEdit('', 'add')}>
+        <StyledAddFilterBox
+          onClick={() => {
+            onEdit('', 'add');
+            setTimeout(() => {
+              const element = document.getElementById('native-filters-tabs');
+              if (element) {
+                const navList = element.getElementsByClassName(
+                  'ant-tabs-nav-list',
+                )[0];
+                navList.scrollTop = navList.scrollHeight;
+              }
+            }, 0);
+          }}
+        >
          <PlusOutlined />{' '}
          <span data-test="add-filter-button" aria-label="Add filter">
            {t('Add filter')}
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@ -229,6 +229,7 @@ const FILTERS_WITH_ADHOC_FILTERS = ['filter_select', 'filter_range'];

 const BASIC_CONTROL_ITEMS = ['enableEmptyFilter', 'multiSelect'];

+// TODO: Rename the filter plugins and remove this mapping
 const FILTER_TYPE_NAME_MAPPING = {
  [t('Select filter')]: t('Value'),
  [t('Range filter')]: t('Numerical range'),
@ -254,7 +255,12 @@ const FiltersConfigForm = (
  ref: React.RefObject<any>,
 ) => {
  const [metrics, setMetrics] = useState<Metric[]>([]);
-  const [activeTabKey, setActiveTabKey] = useState<string | undefined>();
+  const [activeTabKey, setActiveTabKey] = useState<string>(
+    FilterTabs.configuration.key,
+  );
+  const [activeFilterPanelKey, setActiveFilterPanelKey] = useState<
+    string | string[]
+  >(FilterPanels.basic.key);
  const [hasDefaultValue, setHasDefaultValue] = useState(
    !!filterToEdit?.defaultDataMask?.filterState?.value,
  );
@ -410,10 +416,6 @@ const FiltersConfigForm = (
    [],
  );

-  if (removed) {
-    return <RemovedFilter onClick={() => restoreFilter(filterId)} />;
-  }
-
  const parentFilterOptions = parentFilters.map(filter => ({
    value: filter.id,
    label: filter.title,
@ -423,6 +425,14 @@ const FiltersConfigForm = (
    ({ value }) => value === filterToEdit?.cascadeParentIds[0],
  );

+  const hasParentFilter = !!parentFilter;
+
+  const hasPreFilter =
+    !!filterToEdit?.adhoc_filters || !!filterToEdit?.time_range;
+
+  const hasSorting =
+    typeof filterToEdit?.controlValues?.sortAscending === 'boolean';
+
  const showDefaultValue = !hasDataset || (!isDataDirty && hasFilledDataset);

  const controlItems = formFilter
@ -447,9 +457,27 @@ const FiltersConfigForm = (
    forceUpdate();
  };

+  let hasCheckedAdvancedControl = hasParentFilter || hasPreFilter || hasSorting;
+  if (!hasCheckedAdvancedControl) {
+    hasCheckedAdvancedControl = Object.keys(controlItems)
+      .filter(key => !BASIC_CONTROL_ITEMS.includes(key))
+      .some(key => controlItems[key].checked);
+  }
+
+  useEffect(() => {
+    const activeFilterPanelKey = [FilterPanels.basic.key];
+    if (hasCheckedAdvancedControl) {
+      activeFilterPanelKey.push(FilterPanels.advanced.key);
+    }
+    setActiveFilterPanelKey(activeFilterPanelKey);
+  }, [hasCheckedAdvancedControl]);
+
+  if (removed) {
+    return <RemovedFilter onClick={() => restoreFilter(filterId)} />;
+  }
+
  return (
    <StyledTabs
-      defaultActiveKey={FilterTabs.configuration.key}
      activeKey={activeTabKey}
      onChange={activeKey => setActiveTabKey(activeKey)}
      centered
@ -559,7 +587,8 @@ const FiltersConfigForm = (
          </StyledRowContainer>
        )}
        <StyledCollapse
-          defaultActiveKey={FilterPanels.basic.key}
+          activeKey={activeFilterPanelKey}
+          onChange={key => setActiveFilterPanelKey(key)}
          expandIconPosition="right"
        >
          <Collapse.Panel
@ -630,7 +659,7 @@ const FiltersConfigForm = (
            </CollapsibleControl>
            {Object.keys(controlItems)
              .filter(key => BASIC_CONTROL_ITEMS.includes(key))
-              .map(key => controlItems[key])}
+              .map(key => controlItems[key].element)}
            <StyledRowFormItem
              name={['filters', filterId, 'isInstant']}
              initialValue={filterToEdit?.isInstant || false}
@ -650,7 +679,7 @@ const FiltersConfigForm = (
              {isCascadingFilter && (
                <CollapsibleControl
                  title={t('Filter is hierarchical')}
-                  checked={!!parentFilter}
+                  checked={hasParentFilter}
                  onChange={checked => {
                    if (checked) {
                      // execute after render
@ -687,13 +716,11 @@ const FiltersConfigForm = (
              )}
              {Object.keys(controlItems)
                .filter(key => !BASIC_CONTROL_ITEMS.includes(key))
-                .map(key => controlItems[key])}
+                .map(key => controlItems[key].element)}
              {hasDataset && hasAdditionalFilters && (
                <CollapsibleControl
                  title={t('Pre-filter available values')}
-                  checked={
-                    !!filterToEdit?.adhoc_filters || !!filterToEdit?.time_range
-                  }
+                  checked={hasPreFilter}
                  onChange={checked => {
                    if (checked) {
                      // execute after render
@ -757,72 +784,71 @@ const FiltersConfigForm = (
                  </StyledRowFormItem>
                </CollapsibleControl>
              )}
-              <CollapsibleControl
-                title={t('Sort filter values')}
-                onChange={checked => onSortChanged(checked || undefined)}
-                checked={
-                  typeof filterToEdit?.controlValues?.sortAscending ===
-                  'boolean'
-                }
-              >
-                <StyledRowContainer>
-                  <StyledFormItem
-                    name={[
-                      'filters',
-                      filterId,
-                      'controlValues',
-                      'sortAscending',
-                    ]}
-                    initialValue={filterToEdit?.controlValues?.sortAscending}
-                    label={<StyledLabel>{t('Sort type')}</StyledLabel>}
-                  >
-                    <Select
-                      form={form}
-                      filterId={filterId}
-                      name="sortAscending"
-                      options={[
-                        {
-                          value: true,
-                          label: t('Sort ascending'),
-                        },
-                        {
-                          value: false,
-                          label: t('Sort descending'),
-                        },
-                      ]}
-                      onChange={({ value }: { value: boolean }) =>
-                        onSortChanged(value)
-                      }
-                    />
-                  </StyledFormItem>
-                  {hasMetrics && (
+              {formFilter?.filterType !== 'filter_range' && (
+                <CollapsibleControl
+                  title={t('Sort filter values')}
+                  onChange={checked => onSortChanged(checked || undefined)}
+                  checked={hasSorting}
+                >
+                  <StyledRowContainer>
                    <StyledFormItem
-                      name={['filters', filterId, 'sortMetric']}
-                      initialValue={filterToEdit?.sortMetric}
-                      label={<StyledLabel>{t('Sort Metric')}</StyledLabel>}
-                      data-test="field-input"
+                      name={[
+                        'filters',
+                        filterId,
+                        'controlValues',
+                        'sortAscending',
+                      ]}
+                      initialValue={filterToEdit?.controlValues?.sortAscending}
+                      label={<StyledLabel>{t('Sort type')}</StyledLabel>}
                    >
-                      <SelectControl
+                      <Select
                        form={form}
                        filterId={filterId}
-                        name="sortMetric"
-                        options={metrics.map((metric: Metric) => ({
-                          value: metric.metric_name,
-                          label: metric.verbose_name ?? metric.metric_name,
-                        }))}
-                        onChange={(value: string | null): void => {
-                          if (value !== undefined) {
-                            setNativeFilterFieldValues(form, filterId, {
-                              sortMetric: value,
-                            });
-                            forceUpdate();
-                          }
-                        }}
+                        name="sortAscending"
+                        options={[
+                          {
+                            value: true,
+                            label: t('Sort ascending'),
+                          },
+                          {
+                            value: false,
+                            label: t('Sort descending'),
+                          },
+                        ]}
+                        onChange={({ value }: { value: boolean }) =>
+                          onSortChanged(value)
+                        }
                      />
                    </StyledFormItem>
-                  )}
-                </StyledRowContainer>
-              </CollapsibleControl>
+                    {hasMetrics && (
+                      <StyledFormItem
+                        name={['filters', filterId, 'sortMetric']}
+                        initialValue={filterToEdit?.sortMetric}
+                        label={<StyledLabel>{t('Sort Metric')}</StyledLabel>}
+                        data-test="field-input"
+                      >
+                        <SelectControl
+                          form={form}
+                          filterId={filterId}
+                          name="sortMetric"
+                          options={metrics.map((metric: Metric) => ({
+                            value: metric.metric_name,
+                            label: metric.verbose_name ?? metric.metric_name,
+                          }))}
+                          onChange={(value: string | null): void => {
+                            if (value !== undefined) {
+                              setNativeFilterFieldValues(form, filterId, {
+                                sortMetric: value,
+                              });
+                              forceUpdate();
+                            }
+                          }}
+                        />
+                      </StyledFormItem>
+                    )}
+                  </StyledRowContainer>
+                </CollapsibleControl>
+              )}
            </Collapse.Panel>
          )}
        </StyledCollapse>
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx
@ -83,8 +83,12 @@ beforeEach(() => {
  jest.clearAllMocks();
 });

-function renderControlItems(controlItemsMap: {}): any {
-  return render(<>{Object.values(controlItemsMap).map(value => value)}</>);
+function renderControlItems(
+  controlItemsMap: ReturnType<typeof getControlItemsMap>,
+) {
+  return render(
+    <>{Object.values(controlItemsMap).map(value => value.element)}</>,
+  );
 }

 test('Should render null when has no "formFilter"', () => {
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx
@ -50,7 +50,10 @@ export default function getControlItemsMap({
  const controlPanelRegistry = getChartControlPanelRegistry();
  const controlItems =
    getControlItems(controlPanelRegistry.get(filterType)) ?? [];
-  const map = {};
+  const map: Record<
+    string,
+    { element: React.ReactNode; checked: boolean }
+  > = {};

  controlItems
    .filter(
@ -59,6 +62,9 @@ export default function getControlItemsMap({
        controlItem.name !== 'sortAscending',
    )
    .forEach(controlItem => {
+      const initialValue =
+        filterToEdit?.controlValues?.[controlItem.name] ??
+        controlItem?.config?.default;
      const element = (
        <Tooltip
          key={controlItem.name}
@ -72,10 +78,7 @@ export default function getControlItemsMap({
          <StyledRowFormItem
            key={controlItem.name}
            name={['filters', filterId, 'controlValues', controlItem.name]}
-            initialValue={
-              filterToEdit?.controlValues?.[controlItem.name] ??
-              controlItem?.config?.default
-            }
+            initialValue={initialValue}
            valuePropName="checked"
            colon={false}
          >
@ -104,7 +107,7 @@ export default function getControlItemsMap({
          </StyledRowFormItem>
        </Tooltip>
      );
-      map[controlItem.name] = element;
+      map[controlItem.name] = { element, checked: initialValue };
    });
  return map;
 }
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
@ -183,7 +183,11 @@ export function FiltersConfigModal({
    filterIds
      .filter(filterId => filterId !== id && !removedFilters[filterId])
      .filter(filterId =>
-        CASCADING_FILTERS.includes(formValues.filters[filterId]?.filterType),
+        CASCADING_FILTERS.includes(
+          formValues.filters[filterId]
+            ? formValues.filters[filterId].filterType
+            : filterConfigMap[filterId]?.filterType,
+        ),
      )
      .map(id => ({
        id,