fix(dashboard): Add correct icon, label and badge to horizontal native filters dropdown button (#22211)

Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>

superset-frontend/packages/superset-ui-core/src/query/types/Dashboard.ts

@@ -91,6 +91,8 @@ export type Filter = {
   description: string;
 };
 
+export type FilterWithDataMask = Filter & { dataMask: DataMaskWithId };
+
 export type Divider = Partial<Omit<Filter, 'id' | 'type'>> & {
   id: string;
   title: string;

superset-frontend/src/components/DropdownContainer/index.tsx

@@ -104,7 +104,7 @@ const DropdownContainer = forwardRef(
     {
       items,
       onOverflowingStateChange,
-      dropdownContent: popoverContent,
+      dropdownContent: getPopoverContent,
       dropdownRef: popoverRef,
       dropdownStyle: popoverStyle = {},
       dropdownTriggerCount: popoverTriggerCount,
@@ -209,26 +209,31 @@ const DropdownContainer = forwardRef(
       }
     }, [notOverflowedIds, onOverflowingStateChange, overflowedIds]);
 
-    const content = useMemo(
-      () => (
-        <div
-          css={css`
-            display: flex;
-            flex-direction: column;
-            gap: ${theme.gridUnit * 4}px;
-          `}
-          data-test="dropdown-content"
-          style={popoverStyle}
-          ref={popoverRef}
-        >
-          {popoverContent
-            ? popoverContent(overflowedItems)
-            : overflowedItems.map(item => item.element)}
-        </div>
-      ),
+    const overflowingCount =
+      overflowingIndex !== -1 ? items.length - overflowingIndex : 0;
+
+    const popoverContent = useMemo(
+      () =>
+        getPopoverContent || overflowingCount ? (
+          <div
+            css={css`
+              display: flex;
+              flex-direction: column;
+              gap: ${theme.gridUnit * 4}px;
+            `}
+            data-test="dropdown-content"
+            style={popoverStyle}
+            ref={popoverRef}
+          >
+            {getPopoverContent
+              ? getPopoverContent(overflowedItems)
+              : overflowedItems.map(item => item.element)}
+          </div>
+        ) : null,
       [
+        getPopoverContent,
         overflowedItems,
-        popoverContent,
+        overflowingCount,
         popoverRef,
         popoverStyle,
         theme.gridUnit,
@@ -244,9 +249,6 @@ const DropdownContainer = forwardRef(
       [ref],
     );
 
-    const overflowingCount =
-      overflowingIndex !== -1 ? items.length - overflowingIndex : 0;
-
     return (
       <div
         ref={ref}
@@ -268,20 +270,33 @@ const DropdownContainer = forwardRef(
         >
           {notOverflowedItems.map(item => item.element)}
         </div>
-        {overflowingCount > 0 && (
+        {popoverContent && (
           <Popover
-            content={content}
+            content={popoverContent}
             trigger="click"
             visible={popoverVisible}
             onVisibleChange={visible => setPopoverVisible(visible)}
+            placement="bottom"
           >
             <Button buttonStyle="secondary">
               {popoverTriggerIcon}
               {popoverTriggerText}
-              <Badge count={popoverTriggerCount || overflowingCount} />
+              <Badge
+                count={popoverTriggerCount ?? overflowingCount}
+                css={css`
+                  margin-left: ${popoverTriggerCount ?? overflowingCount
+                    ? '8px'
+                    : '0'};
+                `}
+              />
               <Icons.DownOutlined
                 iconSize="m"
-                iconColor={theme.colors.grayscale.base}
+                iconColor={theme.colors.grayscale.light1}
+                css={css`
+                  .anticon {
+                    display: flex;
+                  }
+                `}
               />
             </Button>
           </Popover>

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx

@@ -24,6 +24,8 @@ import {
   Divider,
   css,
   SupersetTheme,
+  t,
+  isNativeFilter,
 } from '@superset-ui/core';
 import {
   createHtmlPortalNode,
@@ -37,6 +39,7 @@ import {
 } from 'src/dashboard/components/nativeFilters/state';
 import { FilterBarOrientation, RootState } from 'src/dashboard/types';
 import DropdownContainer from 'src/components/DropdownContainer';
+import Icons from 'src/components/Icons';
 import { FiltersOutOfScopeCollapsible } from '../FiltersOutOfScopeCollapsible';
 import { useFilterControlFactory } from '../useFilterControlFactory';
 import { FiltersDropdownContent } from '../FiltersDropdownContent';
@@ -56,7 +59,7 @@ const FilterControls: FC<FilterControlsProps> = ({
     state => state.dashboardInfo.filterBarOrientation,
   );
 
-  const [overflowIndex, setOverflowIndex] = useState(0);
+  const [overflowedIds, setOverflowedIds] = useState<string[]>([]);
 
   const { filterControlFactory, filtersWithValues } = useFilterControlFactory(
     dataMaskSelected,
@@ -100,52 +103,99 @@ const FilterControls: FC<FilterControlsProps> = ({
     </>
   );
 
-  const renderHorizontalContent = () => {
-    const items = filtersInScope.map(filter => ({
-      id: filter.id,
-      element: (
-        <div
-          css={css`
-            flex-shrink: 0;
-          `}
-        >
-          {renderer(filter)}
-        </div>
-      ),
-    }));
-    return (
-      <div
-        css={(theme: SupersetTheme) =>
-          css`
-            padding-left: ${theme.gridUnit * 4}px;
-            min-width: 0;
-          `
+  const items = useMemo(
+    () =>
+      filtersInScope.map(filter => ({
+        id: filter.id,
+        element: (
+          <div
+            css={css`
+              flex-shrink: 0;
+            `}
+          >
+            {renderer(filter)}
+          </div>
+        ),
+      })),
+    [filtersInScope, renderer],
+  );
+
+  const overflowedFiltersInScope = useMemo(
+    () => filtersInScope.filter(({ id }) => overflowedIds?.includes(id)),
+    [filtersInScope, overflowedIds],
+  );
+
+  const activeOverflowedFiltersInScope = useMemo(
+    () =>
+      overflowedFiltersInScope.filter(
+        filter => isNativeFilter(filter) && filter.dataMask.filterState?.value,
+      ).length,
+    [overflowedFiltersInScope],
+  );
+
+  const renderHorizontalContent = () => (
+    <div
+      css={(theme: SupersetTheme) =>
+        css`
+          padding-left: ${theme.gridUnit * 4}px;
+          min-width: 0;
+        `
+      }
+    >
+      <DropdownContainer
+        items={items}
+        dropdownTriggerIcon={
+          <Icons.FilterSmall
+            css={css`
+              && {
+                margin-right: -4px;
+                display: flex;
+              }
+            `}
+          />
         }
-      >
-        <DropdownContainer
-          items={items}
-          dropdownContent={overflowedItems => {
-            const overflowedItemIds = new Set(
-              overflowedItems.map(({ id }) => id),
-            );
-            return (
-              <FiltersDropdownContent
-                filtersInScope={filtersInScope.filter(({ id }) =>
-                  overflowedItemIds.has(id),
-                )}
-                filtersOutOfScope={filtersOutOfScope}
-                renderer={renderer}
-                showCollapsePanel={showCollapsePanel}
-              />
-            );
-          }}
-          onOverflowingStateChange={overflowingState =>
-            setOverflowIndex(overflowingState.notOverflowed.length)
+        dropdownTriggerText={t('More filters')}
+        dropdownTriggerCount={activeOverflowedFiltersInScope}
+        dropdownContent={
+          overflowedFiltersInScope.length ||
+          (filtersOutOfScope.length && showCollapsePanel)
+            ? () => (
+                <FiltersDropdownContent
+                  filtersInScope={overflowedFiltersInScope}
+                  filtersOutOfScope={filtersOutOfScope}
+                  renderer={renderer}
+                  showCollapsePanel={showCollapsePanel}
+                />
+              )
+            : undefined
+        }
+        onOverflowingStateChange={({ overflowed: nextOverflowedIds }) => {
+          if (
+            nextOverflowedIds.length !== overflowedIds.length ||
+            overflowedIds.reduce(
+              (a, b, i) => a || b !== nextOverflowedIds[i],
+              false,
+            )
+          ) {
+            setOverflowedIds(nextOverflowedIds);
           }
-        />
-      </div>
+        }}
+      />
+    </div>
+  );
+
+  const overflowedByIndex = useMemo(() => {
+    const filtersOutOfScopeIds = new Set(filtersOutOfScope.map(({ id }) => id));
+    const overflowedFiltersInScopeIds = new Set(
+      overflowedFiltersInScope.map(({ id }) => id),
     );
-  };
+
+    return filtersWithValues.map(
+      filter =>
+        filtersOutOfScopeIds.has(filter.id) ||
+        overflowedFiltersInScopeIds.has(filter.id),
+    );
+  }, [filtersOutOfScope, filtersWithValues, overflowedFiltersInScope]);
 
   return (
     <>
@@ -153,7 +203,11 @@ const FilterControls: FC<FilterControlsProps> = ({
         .filter((node, index) => filterIds.has(filtersWithValues[index].id))
         .map((node, index) => (
           <InPortal node={node}>
-            {filterControlFactory(index, filterBarOrientation, overflowIndex)}
+            {filterControlFactory(
+              index,
+              filterBarOrientation,
+              overflowedByIndex[index],
+            )}
           </InPortal>
         ))}
       {filterBarOrientation === FilterBarOrientation.VERTICAL &&

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx

@@ -74,9 +74,6 @@ const FiltersLinkContainer = styled.div<{ hasFilters: boolean }>`
     button {
       display: flex;
       align-items: center;
-      text-transform: capitalize;
-      font-weight: ${theme.typography.weights.normal};
-      color: ${theme.colors.primary.base};
       > .anticon {
         height: 24px;
         padding-right: ${theme.gridUnit}px;

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx

@@ -23,6 +23,7 @@ import {
   DataMaskStateWithId,
   Divider,
   Filter,
+  FilterWithDataMask,
   isFilterDivider,
 } from '@superset-ui/core';
 import { FilterBarOrientation } from 'src/dashboard/types';
@@ -37,7 +38,7 @@ export const useFilterControlFactory = (
 ) => {
   const filters = useFilters();
   const filterValues = useMemo(() => Object.values(filters), [filters]);
-  const filtersWithValues: (Filter | Divider)[] = useMemo(
+  const filtersWithValues: (FilterWithDataMask | Divider)[] = useMemo(
     () =>
       filterValues.map(filter => ({
         ...filter,
@@ -50,7 +51,7 @@ export const useFilterControlFactory = (
     (
       index: number,
       filterBarOrientation: FilterBarOrientation,
-      overflowIndex: number,
+      overflow: boolean,
     ) => {
       const filter = filtersWithValues[index];
       if (isFilterDivider(filter)) {
@@ -59,7 +60,7 @@ export const useFilterControlFactory = (
             title={filter.title}
             description={filter.description}
             orientation={filterBarOrientation}
-            overflow={index >= overflowIndex}
+            overflow={overflow}
           />
         );
       }
@@ -71,7 +72,7 @@ export const useFilterControlFactory = (
           onFilterSelectionChange={onFilterSelectionChange}
           inView={false}
           orientation={filterBarOrientation}
-          overflow={index >= overflowIndex}
+          overflow={overflow}
         />
       );
     },

superset-frontend/src/dashboard/components/nativeFilters/state.ts

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