chore(native-filters): Grid units, type guard, feature flag guard (#2…

…2307)
apache · Dec 2, 2022 · 997950e · 997950e
1 parent 5c77f1a
commit 997950e
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 35 deletions.
diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts
@@ -91,7 +91,7 @@ export type Filter = {
   description: string;
 };
 
-export type FilterWithDataMask = Filter & { dataMask?: DataMaskWithId };
+export type FilterWithDataMask = Filter & { dataMask: DataMaskWithId };
 
 export type Divider = Partial<Omit<Filter, 'id' | 'type'>> & {
   id: string;
@@ -106,6 +106,15 @@ export function isNativeFilter(
   return filterElement.type === NativeFilterType.NATIVE_FILTER;
 }
 
+export function isNativeFilterWithDataMask(
+  filterElement: Filter | Divider,
+): filterElement is FilterWithDataMask {
+  return (
+    isNativeFilter(filterElement) &&
+    (filterElement as FilterWithDataMask).dataMask?.filterState?.value
+  );
+}
+
 export function isFilterDivider(
   filterElement: Filter | Divider,
 ): filterElement is Divider {

diff --git a/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts b/superset-frontend/packages/superset-ui-core/test/query/types/Dashboard.test.ts
@@ -21,27 +21,50 @@ import {
   isFilterDivider,
   Filter,
   NativeFilterType,
+  FilterWithDataMask,
+  Divider,
+  isNativeFilterWithDataMask,
 } from '@superset-ui/core';
 
-test('should do native filter type guard', () => {
-  const dummyFilter: Filter = {
-    cascadeParentIds: [],
-    defaultDataMask: {},
-    id: 'dummyID',
-    name: 'dummyName',
-    scope: { rootPath: [], excluded: [] },
-    filterType: 'dummyType',
-    targets: [{}],
-    controlValues: {},
-    type: NativeFilterType.NATIVE_FILTER,
-    description: 'dummyDesc',
-  };
-  expect(isNativeFilter(dummyFilter)).toBeTruthy();
-  expect(
-    isFilterDivider({
-      ...dummyFilter,
-      type: NativeFilterType.DIVIDER,
-      title: 'dummyTitle',
-    }),
-  ).toBeTruthy();
+const filter: Filter = {
+  cascadeParentIds: [],
+  defaultDataMask: {},
+  id: 'filter_id',
+  name: 'Filter Name',
+  scope: { rootPath: [], excluded: [] },
+  filterType: 'filter_type',
+  targets: [{}],
+  controlValues: {},
+  type: NativeFilterType.NATIVE_FILTER,
+  description: 'Filter description.',
+};
+
+const filterWithDataMask: FilterWithDataMask = {
+  ...filter,
+  dataMask: { id: 'data_mask_id', filterState: { value: 'Filter value' } },
+};
+
+const filterDivider: Divider = {
+  id: 'divider_id',
+  type: NativeFilterType.DIVIDER,
+  title: 'Divider title',
+  description: 'Divider description.',
+};
+
+test('filter type guard', () => {
+  expect(isNativeFilter(filter)).toBeTruthy();
+  expect(isNativeFilter(filterWithDataMask)).toBeTruthy();
+  expect(isNativeFilter(filterDivider)).toBeFalsy();
+});
+
+test('filter with dataMask type guard', () => {
+  expect(isNativeFilterWithDataMask(filter)).toBeFalsy();
+  expect(isNativeFilterWithDataMask(filterWithDataMask)).toBeTruthy();
+  expect(isNativeFilterWithDataMask(filterDivider)).toBeFalsy();
+});
+
+test('filter divider type guard', () => {
+  expect(isFilterDivider(filter)).toBeFalsy();
+  expect(isFilterDivider(filterWithDataMask)).toBeFalsy();
+  expect(isFilterDivider(filterDivider)).toBeTruthy();
 });
diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js
@@ -430,7 +430,9 @@ export const hydrateDashboard =
             conf: common?.conf,
           },
           filterBarOrientation:
-            metadata.filter_bar_orientation ?? FilterBarOrientation.VERTICAL,
+            (isFeatureEnabled(FeatureFlag.HORIZONTAL_FILTER_BAR) &&
+              metadata.filter_bar_orientation) ||
+            FilterBarOrientation.VERTICAL,
         },
         dataMask,
         dashboardFilters,

diff --git a/...ontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx b/...ontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx
@@ -32,9 +32,9 @@ import {
   css,
   SupersetTheme,
   t,
-  isNativeFilter,
   isFeatureEnabled,
   FeatureFlag,
+  isNativeFilterWithDataMask,
 } from '@superset-ui/core';
 import {
   createHtmlPortalNode,
@@ -150,8 +150,8 @@ const FilterControls: FC<FilterControlsProps> = ({
 
   const activeOverflowedFiltersInScope = useMemo(
     () =>
-      overflowedFiltersInScope.filter(
-        filter => isNativeFilter(filter) && filter.dataMask?.filterState?.value,
+      overflowedFiltersInScope.filter(filter =>
+        isNativeFilterWithDataMask(filter),
       ).length,
     [overflowedFiltersInScope],
   );

diff --git a/...rontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterDivider.tsx b/...rontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterDivider.tsx
@@ -130,7 +130,7 @@ const HorizontalOverflowDivider = ({
               display: block;
               font-size: ${theme.typography.sizes.s}px;
               color: ${theme.colors.grayscale.base};
-              margin: ${theme.gridUnit * 2.5}px 0 0 0;
+              margin: ${theme.gridUnit * 2}px 0 0 0;
               line-height: 1;
             `}
           >

diff --git a/...set-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx b/...set-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx
@@ -23,7 +23,6 @@ import {
   DataMaskStateWithId,
   Divider,
   Filter,
-  FilterWithDataMask,
   isFilterDivider,
 } from '@superset-ui/core';
 import { FilterBarOrientation } from 'src/dashboard/types';
@@ -38,7 +37,7 @@ export const useFilterControlFactory = (
 ) => {
   const filters = useFilters();
   const filterValues = useMemo(() => Object.values(filters), [filters]);
-  const filtersWithValues: (FilterWithDataMask | Divider)[] = useMemo(
+  const filtersWithValues: (Filter | Divider)[] = useMemo(
     () =>
       filterValues.map(filter => ({
         ...filter,

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.ts
@@ -23,7 +23,6 @@ import {
   FilterConfiguration,
   Divider,
   isFilterDivider,
-  FilterWithDataMask,
 } from '@superset-ui/core';
 import { ActiveTabs, DashboardLayout, RootState } from '../../types';
 import { TAB_TYPE } from '../../util/componentTypes';
@@ -110,15 +109,13 @@ function useIsFilterInScope() {
       }));
 }
 
-export function useSelectFiltersInScope(
-  filters: (FilterWithDataMask | Divider)[],
-) {
+export function useSelectFiltersInScope(filters: (Filter | Divider)[]) {
   const dashboardHasTabs = useDashboardHasTabs();
   const isFilterInScope = useIsFilterInScope();
 
   return useMemo(() => {
-    let filtersInScope: (FilterWithDataMask | Divider)[] = [];
-    const filtersOutOfScope: (FilterWithDataMask | Divider)[] = [];
+    let filtersInScope: (Filter | Divider)[] = [];
+    const filtersOutOfScope: (Filter | Divider)[] = [];
 
     // we check native filters scopes only on dashboards with tabs
     if (!dashboardHasTabs) {