feat: scroll to bottom when adding a new native filter and the page i…

…s filled (#19053)

* feat: Scroll to bottom when adding a new native filter and the page is filled

* Add test

(cherry picked from commit cfb967f)

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx

@@ -57,7 +57,7 @@ const DragIcon = styled(Icons.Drag, {
 interface FilterTabTitleProps {
   index: number;
   filterIds: string[];
-  onRearrage: (dragItemIndex: number, targetIndex: number) => void;
+  onRearrange: (dragItemIndex: number, targetIndex: number) => void;
 }
 
 interface DragItem {
@@ -68,7 +68,7 @@ interface DragItem {
 
 export const DraggableFilter: React.FC<FilterTabTitleProps> = ({
   index,
-  onRearrage,
+  onRearrange,
   filterIds,
   children,
 }) => {
@@ -120,7 +120,7 @@ export const DraggableFilter: React.FC<FilterTabTitleProps> = ({
         return;
       }
 
-      onRearrage(dragIndex, hoverIndex);
+      onRearrange(dragIndex, hoverIndex);
       // Note: we're mutating the monitor item here.
       // Generally it's better to avoid mutations,
       // but it's good here for the sake of performance

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx

@@ -22,6 +22,9 @@ import { buildNativeFilter } from 'spec/fixtures/mockNativeFilters';
 import { act, fireEvent, render, screen } from 'spec/helpers/testing-library';
 import FilterConfigPane from './FilterConfigurePane';
 
+const scrollMock = jest.fn();
+Element.prototype.scroll = scrollMock;
+
 const defaultProps = {
   children: jest.fn(),
   getFilterTitle: (id: string) => id,
@@ -56,6 +59,10 @@ function defaultRender(initialState: any = defaultState, props = defaultProps) {
   });
 }
 
+beforeEach(() => {
+  scrollMock.mockClear();
+});
+
 test('renders form', async () => {
   await act(async () => {
     defaultRender();
@@ -65,7 +72,7 @@ test('renders form', async () => {
 
 test('drag and drop', async () => {
   defaultRender();
-  // Drag the state and contry filter above the product filter
+  // Drag the state and country filter above the product filter
   const [countryStateFilter, productFilter] = document.querySelectorAll(
     'div[draggable=true]',
   );
@@ -132,3 +139,41 @@ test('add divider', async () => {
   });
   expect(defaultProps.onAdd).toHaveBeenCalledWith('DIVIDER');
 });
+
+test('filter container should scroll to bottom when adding items', async () => {
+  const state = {
+    dashboardInfo: {
+      metadata: {
+        native_filter_configuration: new Array(35)
+          .fill(0)
+          .map((_, index) =>
+            buildNativeFilter(`NATIVE_FILTER-${index}`, `filter-${index}`, []),
+          ),
+      },
+    },
+    dashboardLayout,
+  };
+  const props = {
+    ...defaultProps,
+    filters: new Array(35).fill(0).map((_, index) => `NATIVE_FILTER-${index}`),
+  };
+
+  defaultRender(state, props);
+
+  const addButton = screen.getByText('Add filters and dividers')!;
+  fireEvent.mouseOver(addButton);
+
+  const addFilterButton = await screen.findByText('Filter');
+  await act(async () => {
+    fireEvent(
+      addFilterButton,
+      new MouseEvent('click', {
+        bubbles: true,
+        cancelable: true,
+      }),
+    );
+  });
+
+  const containerElement = screen.getByTestId('filter-title-container');
+  expect(containerElement.scroll).toHaveBeenCalled();
+});

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx

@@ -50,7 +50,7 @@ const TitlesContainer = styled.div`
   border-right: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
 `;
 
-const FiltureConfigurePane: React.FC<Props> = ({
+const FilterConfigurePane: React.FC<Props> = ({
   getFilterTitle,
   onChange,
   onRemove,
@@ -75,7 +75,7 @@ const FiltureConfigurePane: React.FC<Props> = ({
           getFilterTitle={getFilterTitle}
           onChange={onChange}
           onAdd={(type: NativeFilterType) => onAdd(type)}
-          onRearrage={onRearrange}
+          onRearrange={onRearrange}
           onRemove={(id: string) => onRemove(id)}
           restoreFilter={restoreFilter}
         />
@@ -98,4 +98,4 @@ const FiltureConfigurePane: React.FC<Props> = ({
   );
 };
 
-export default FiltureConfigurePane;
+export default FilterConfigurePane;

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React from 'react';
+import React, { forwardRef } from 'react';
 import { styled, t } from '@superset-ui/core';
 import Icons from 'src/components/Icons';
 import { FilterRemoval } from './types';
@@ -72,124 +72,134 @@ interface Props {
   removedFilters: Record<string, FilterRemoval>;
   onRemove: (id: string) => void;
   restoreFilter: (id: string) => void;
-  onRearrage: (dragIndex: number, targetIndex: number) => void;
+  onRearrange: (dragIndex: number, targetIndex: number) => void;
   filters: string[];
   erroredFilters: string[];
 }
 
-const FilterTitleContainer: React.FC<Props> = ({
-  getFilterTitle,
-  onChange,
-  onRemove,
-  restoreFilter,
-  onRearrage,
-  currentFilterId,
-  removedFilters,
-  filters,
-  erroredFilters = [],
-}) => {
-  const renderComponent = (id: string) => {
-    const isRemoved = !!removedFilters[id];
-    const isErrored = erroredFilters.includes(id);
-    const isActive = currentFilterId === id;
-    const classNames = [];
-    if (isErrored) {
-      classNames.push('errored');
-    }
-    if (isActive) {
-      classNames.push('active');
-    }
-    return (
-      <FilterTitle
-        role="tab"
-        key={`filter-title-tab-${id}`}
-        onClick={() => onChange(id)}
-        className={classNames.join(' ')}
-      >
-        <div css={{ display: 'flex', width: '100%' }}>
-          <div css={{ alignItems: 'center', display: 'flex' }}>
-            {isRemoved ? t('(Removed)') : getFilterTitle(id)}
+const FilterTitleContainer = forwardRef<HTMLDivElement, Props>(
+  (
+    {
+      getFilterTitle,
+      onChange,
+      onRemove,
+      restoreFilter,
+      onRearrange,
+      currentFilterId,
+      removedFilters,
+      filters,
+      erroredFilters = [],
+    },
+    ref,
+  ) => {
+    const renderComponent = (id: string) => {
+      const isRemoved = !!removedFilters[id];
+      const isErrored = erroredFilters.includes(id);
+      const isActive = currentFilterId === id;
+      const classNames = [];
+      if (isErrored) {
+        classNames.push('errored');
+      }
+      if (isActive) {
+        classNames.push('active');
+      }
+      return (
+        <FilterTitle
+          role="tab"
+          key={`filter-title-tab-${id}`}
+          onClick={() => onChange(id)}
+          className={classNames.join(' ')}
+        >
+          <div css={{ display: 'flex', width: '100%' }}>
+            <div css={{ alignItems: 'center', display: 'flex' }}>
+              {isRemoved ? t('(Removed)') : getFilterTitle(id)}
+            </div>
+            {!removedFilters[id] && isErrored && (
+              <StyledWarning className="warning" />
+            )}
+            {isRemoved && (
+              <span
+                css={{ alignSelf: 'flex-end', marginLeft: 'auto' }}
+                role="button"
+                data-test="undo-button"
+                tabIndex={0}
+                onClick={e => {
+                  e.preventDefault();
+                  restoreFilter(id);
+                }}
+              >
+                {t('Undo?')}
+              </span>
+            )}
           </div>
-          {!removedFilters[id] && isErrored && (
-            <StyledWarning className="warning" />
-          )}
-          {isRemoved && (
-            <span
-              css={{ alignSelf: 'flex-end', marginLeft: 'auto' }}
-              role="button"
-              data-test="undo-button"
-              tabIndex={0}
-              onClick={e => {
-                e.preventDefault();
-                restoreFilter(id);
-              }}
-            >
-              {t('Undo?')}
-            </span>
-          )}
-        </div>
-        <div css={{ alignSelf: 'flex-start', marginLeft: 'auto' }}>
-          {isRemoved ? null : (
-            <StyledTrashIcon
-              onClick={event => {
-                event.stopPropagation();
-                onRemove(id);
-              }}
-              alt="RemoveFilter"
-            />
-          )}
-        </div>
-      </FilterTitle>
-    );
-  };
-  const recursivelyRender = (
-    elementId: string,
-    nodeList: Array<{ id: string; parentId: string | null }>,
-    rendered: Array<string>,
-  ): React.ReactNode => {
-    const didAlreadyRender = rendered.indexOf(elementId) >= 0;
-    if (didAlreadyRender) {
-      return null;
-    }
-    let parent = null;
-    const element = nodeList.filter(el => el.id === elementId)[0];
-    if (!element) {
-      return null;
-    }
+          <div css={{ alignSelf: 'flex-start', marginLeft: 'auto' }}>
+            {isRemoved ? null : (
+              <StyledTrashIcon
+                onClick={event => {
+                  event.stopPropagation();
+                  onRemove(id);
+                }}
+                alt="RemoveFilter"
+              />
+            )}
+          </div>
+        </FilterTitle>
+      );
+    };
+    const recursivelyRender = (
+      elementId: string,
+      nodeList: Array<{ id: string; parentId: string | null }>,
+      rendered: Array<string>,
+    ): React.ReactNode => {
+      const didAlreadyRender = rendered.indexOf(elementId) >= 0;
+      if (didAlreadyRender) {
+        return null;
+      }
+      let parent = null;
+      const element = nodeList.filter(el => el.id === elementId)[0];
+      if (!element) {
+        return null;
+      }
+
+      rendered.push(elementId);
+      if (element.parentId) {
+        parent = recursivelyRender(element.parentId, nodeList, rendered);
+      }
+      const children = nodeList
+        .filter(item => item.parentId === elementId)
+        .map(item => recursivelyRender(item.id, nodeList, rendered));
+      return (
+        <>
+          {parent}
+          {renderComponent(elementId)}
+          {children}
+        </>
+      );
+    };
+
+    const renderFilterGroups = () => {
+      const items: React.ReactNode[] = [];
+      filters.forEach((item, index) => {
+        items.push(
+          <DraggableFilter
+            key={index}
+            onRearrange={onRearrange}
+            index={index}
+            filterIds={[item]}
+          >
+            {renderComponent(item)}
+          </DraggableFilter>,
+        );
+      });
+      return items;
+    };
 
-    rendered.push(elementId);
-    if (element.parentId) {
-      parent = recursivelyRender(element.parentId, nodeList, rendered);
-    }
-    const children = nodeList
-      .filter(item => item.parentId === elementId)
-      .map(item => recursivelyRender(item.id, nodeList, rendered));
     return (
-      <>
-        {parent}
-        {renderComponent(elementId)}
-        {children}
-      </>
+      <Container data-test="filter-title-container" ref={ref}>
+        {renderFilterGroups()}
+      </Container>
     );
-  };
-
-  const renderFilterGroups = () => {
-    const items: React.ReactNode[] = [];
-    filters.forEach((item, index) => {
-      items.push(
-        <DraggableFilter
-          key={index}
-          onRearrage={onRearrage}
-          index={index}
-          filterIds={[item]}
-        >
-          {renderComponent(item)}
-        </DraggableFilter>,
-      );
-    });
-    return items;
-  };
-  return <Container>{renderFilterGroups()}</Container>;
-};
+  },
+);
 
 export default FilterTitleContainer;