[Filters] only show the "More filters" button if necessary (#2856)

Co-authored-by: Tim Layton <tmlayton@users.noreply.github.com>

Author: rickitan

UNRELEASED.md

@@ -6,6 +6,8 @@
 
 ### Enhancements
 
+- Updated `Filters` to only show the "More filters" button if necessary ([#2856](https://github.com/Shopify/polaris-react/pull/2856)).
+
 ### Bug fixes
 
 ### Documentation

src/components/Filters/Filters.tsx

@@ -243,12 +243,15 @@ class FiltersInner extends React.Component<ComposedProps, State> {
       plural: intl.translate('Polaris.ResourceList.defaultItemPlural'),
     };
 
+    const transformedFilters = this.transformFilters(filters);
+
     const filtersControlMarkup = (
       <ConnectedFilterControl
-        rightPopoverableActions={this.transformFilters(filters)}
+        rightPopoverableActions={transformedFilters}
         rightAction={rightActionMarkup}
         auxiliary={children}
         disabled={disabled}
+        forceShowMorefiltersButton={filters.length > transformedFilters.length}
       >
         <TextField
           placeholder={
@@ -513,10 +516,10 @@ class FiltersInner extends React.Component<ComposedProps, State> {
     }
   }
 
-  private transformFilters(
-    filters: FilterInterface[],
-  ): ConnectedFilterControlProps['rightPopoverableActions'] | null {
-    const transformedActions: ConnectedFilterControlProps['rightPopoverableActions'] = [];
+  private transformFilters(filters: FilterInterface[]) {
+    const transformedActions: Required<
+      ConnectedFilterControlProps['rightPopoverableActions']
+    > = [];
 
     getShortcutFilters(filters).forEach((filter) => {
       const {key, label, disabled} = filter;

src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.scss

@@ -76,6 +76,19 @@ $stacking-order: (
   }
 }
 
+.RightContainerWithoutMoreFilters {
+  .Item:last-child > * > * {
+    border-top-right-radius: var(
+      --p-border-radius-base,
+      border-radius()
+    ) !important;
+    border-bottom-right-radius: var(
+      --p-border-radius-base,
+      border-radius()
+    ) !important;
+  }
+}
+
 .newDesignLanguage .RightContainer {
   .Item:first-of-type > * > * {
     border-top-left-radius: var(--p-border-radius-base) !important;

src/components/Filters/components/ConnectedFilterControl/ConnectedFilterControl.tsx

@@ -25,6 +25,7 @@ export interface ConnectedFilterControlProps {
   rightAction?: React.ReactNode;
   auxiliary?: React.ReactNode;
   disabled?: boolean;
+  forceShowMorefiltersButton?: boolean;
 }
 
 interface ComputedProperty {
@@ -74,6 +75,7 @@ export class ConnectedFilterControl extends React.Component<
       rightPopoverableActions,
       rightAction,
       auxiliary,
+      forceShowMorefiltersButton = true,
     } = this.props;
 
     const actionsToRender =
@@ -87,11 +89,25 @@ export class ConnectedFilterControl extends React.Component<
       newDesignLanguage && styles.newDesignLanguage,
     );
 
-    const rightMarkup = rightPopoverableActions ? (
-      <div className={styles.RightContainer} testID="FilterShortcutContainer">
-        {this.popoverFrom(actionsToRender)}
-      </div>
-    ) : null;
+    const shouldRenderMoreFiltersButton =
+      forceShowMorefiltersButton ||
+      (rightPopoverableActions &&
+        rightPopoverableActions.length !== actionsToRender.length);
+
+    const RightContainerClassName = classNames(
+      styles.RightContainer,
+      !shouldRenderMoreFiltersButton && styles.RightContainerWithoutMoreFilters,
+    );
+
+    const rightMarkup =
+      actionsToRender.length > 0 ? (
+        <div
+          className={RightContainerClassName}
+          testID="FilterShortcutContainer"
+        >
+          {this.popoverFrom(actionsToRender)}
+        </div>
+      ) : null;
 
     const moreFiltersButtonContainerClassname = classNames(
       styles.MoreFiltersButtonContainer,
@@ -105,7 +121,7 @@ export class ConnectedFilterControl extends React.Component<
         ref={this.moreFiltersButtonContainer}
         className={moreFiltersButtonContainerClassname}
       >
-        <Item>{rightAction}</Item>
+        {shouldRenderMoreFiltersButton && <Item>{rightAction}</Item>}
       </div>
     ) : null;
 
@@ -197,6 +213,10 @@ export class ConnectedFilterControl extends React.Component<
       if (actionWidth <= remainingWidth) {
         actionsToReturn.push(action);
         remainingWidth -= actionWidth;
+      } else {
+        // When we can't fit an action, we break the loop.
+        // The ones that didn't fit will be accessible through the "More filters" button
+        break;
       }
     }
     return actionsToReturn;

src/components/Filters/components/ConnectedFilterControl/tests/ConnectedFilterControl.test.tsx

@@ -66,16 +66,46 @@ describe('<ConnectedFilterControl />', () => {
     expect(connectedFilterControl.find(Popover).exists()).toBe(false);
   });
 
-  it('does render a button with a right action', () => {
+  it('always render a RightAction if forceShowMorefiltersButton is true', () => {
     const connectedFilterControl = mountWithAppProvider(
-      <ConnectedFilterControl rightAction={mockRightAction}>
+      <ConnectedFilterControl
+        rightAction={mockRightAction}
+        forceShowMorefiltersButton
+      >
+        <MockChild />
+      </ConnectedFilterControl>,
+    );
+
+    expect(connectedFilterControl.find(Button).exists()).toBe(true);
+  });
+
+  it('renders a RightAction if forceShowMorefiltersButton is false and rightPopoverableActions do not fit on the "right action" container', () => {
+    const connectedFilterControl = mountWithAppProvider(
+      <ConnectedFilterControl
+        rightAction={mockRightAction}
+        rightPopoverableActions={[mockRightOpenPopoverableAction]}
+        forceShowMorefiltersButton={false}
+      >
         <MockChild />
       </ConnectedFilterControl>,
     );
 
     expect(connectedFilterControl.find(Button).exists()).toBe(true);
   });
 
+  it('does not render a RightAction there are no actions hidden', () => {
+    const connectedFilterControl = mountWithAppProvider(
+      <ConnectedFilterControl
+        rightAction={mockRightAction}
+        forceShowMorefiltersButton={false}
+      >
+        <MockChild />
+      </ConnectedFilterControl>,
+    );
+
+    expect(connectedFilterControl.find(Button).exists()).toBe(false);
+  });
+
   it('does render a button with a popoverable action', () => {
     const connectedFilterControl = mountWithAppProvider(
       <ConnectedFilterControl
@@ -104,6 +134,24 @@ describe('<ConnectedFilterControl />', () => {
     expect(connectedFilterControl.find(Button)).toHaveLength(3);
   });
 
+  it('hides an action if it does not fit', () => {
+    const connectedFilterControl = mountWithAppProvider(
+      <ConnectedFilterControl
+        rightPopoverableActions={[
+          mockRightOpenPopoverableAction,
+          mockRightClosedPopoverableAction,
+        ]}
+        rightAction={mockRightAction}
+      >
+        <MockChild />
+      </ConnectedFilterControl>,
+    );
+
+    connectedFilterControl.setState({availableWidth: 100});
+
+    expect(findActions(connectedFilterControl)).toHaveLength(2);
+  });
+
   it('renders auxiliary content', () => {
     const connectedFilterControl = mountWithAppProvider(
       <ConnectedFilterControl auxiliary={<MockAux />}>
@@ -171,3 +219,8 @@ function noop() {}
 function findById(wrapper: ReactWrapper, id: string) {
   return wrapper.find(`#${id}`).first();
 }
+
+function findActions(wrapper: ReactWrapper) {
+  // this omits the invisible proxy actions used for measuring width
+  return wrapper.find('.Wrapper Button');
+}

src/components/Filters/tests/Filters.test.tsx

@@ -218,6 +218,46 @@ describe('<Filters />', () => {
       ).toHaveLength(2);
     });
 
+    it('forces showing the "More Filters" button if there are filters without shortcuts', () => {
+      const resourceFilters = mountWithAppProvider(
+        <Filters {...mockPropsWithShortcuts} />,
+      );
+
+      expect(
+        resourceFilters.find(ConnectedFilterControl).props()
+          .forceShowMorefiltersButton,
+      ).toBe(true);
+    });
+
+    it('does not force showing the "More Filters" button if all the filters have shorcuts', () => {
+      const mockPropsWithShortcuts: FiltersProps = {
+        onQueryChange: noop,
+        onQueryClear: noop,
+        onClearAll: noop,
+        filters: [
+          {
+            key: 'filterOne',
+            label: 'Filter One',
+            filter: <MockFilter id="filterOne" />,
+            shortcut: true,
+          },
+          {
+            key: 'filterTwo',
+            label: 'Filter Two',
+            filter: <MockFilter id="filterTwo" />,
+            shortcut: true,
+          },
+        ],
+      };
+      const resourceFilters = mountWithAppProvider(
+        <Filters {...mockPropsWithShortcuts} />,
+      );
+      expect(
+        resourceFilters.find(ConnectedFilterControl).props()
+          .forceShowMorefiltersButton,
+      ).toBe(false);
+    });
+
     it('receives shortcut filters with popoverOpen set to false on mount', () => {
       const resourceFilters = mountWithAppProvider(
         <Filters {...mockPropsWithShortcuts} />,

src/components/MediaCard/MediaCard.tsx

@@ -4,7 +4,7 @@ import {HorizontalDotsMinor} from '@shopify/polaris-icons';
 import {useToggle} from '../../utilities/use-toggle';
 import {classNames} from '../../utilities/css';
 import {useI18n} from '../../utilities/i18n';
-import {Action, ActionListItemDescriptor} from '../../types';
+import type {Action, ActionListItemDescriptor} from '../../types';
 import {Card} from '../Card';
 import {Button, buttonFrom} from '../Button';
 import {Heading} from '../Heading';