feat: scroll to bottom when adding a new native filter and the page is filled (#19053)

* feat: Scroll to bottom when adding a new native filter and the page is filled * Add test
2024-09-06 22:07:34 -04:00 · 2022-03-16 22:47:15 -04:00 · 2022-03-16 22:47:15 -04:00 · cfb967f430
commit cfb967f430
parent 0277ebc225
6 changed files with 198 additions and 134 deletions
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx
+++ b/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
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx
+++ b/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();
+});
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx
+++ b/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;
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx
+++ b/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)}
-          </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;
-    }
-
-    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}
-          onRearrage={onRearrage}
-          index={index}
-          filterIds={[item]}
+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(' ')}
        >
-          {renderComponent(item)}
-        </DraggableFilter>,
+          <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>
+          <div css={{ alignSelf: 'flex-start', marginLeft: 'auto' }}>
+            {isRemoved ? null : (
+              <StyledTrashIcon
+                onClick={event => {
+                  event.stopPropagation();
+                  onRemove(id);
+                }}
+                alt="RemoveFilter"
+              />
+            )}
+          </div>
+        </FilterTitle>
      );
-    });
-    return items;
-  };
-  return <Container>{renderFilterGroups()}</Container>;
-};
+    };
+    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;
+    };
+
+    return (
+      <Container data-test="filter-title-container" ref={ref}>
+        {renderFilterGroups()}
+      </Container>
+    );
+  },
+);

 export default FilterTitleContainer;
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx
@ -16,8 +16,8 @@
 * specific language governing permissions and limitations
 * under the License.
 */
+import React, { useRef } from 'react';
 import { NativeFilterType, styled, t, useTheme } from '@superset-ui/core';
-import React from 'react';
 import { AntdDropdown } from 'src/components';
 import { MainNav as Menu } from 'src/components/Menu';
 import FilterTitleContainer from './FilterTitleContainer';
@ -26,7 +26,7 @@ import { FilterRemoval } from './types';
 interface Props {
  restoreFilter: (id: string) => void;
  getFilterTitle: (id: string) => string;
-  onRearrage: (dragIndex: number, targetIndex: number) => void;
+  onRearrange: (dragIndex: number, targetIndex: number) => void;
  onRemove: (id: string) => void;
  onChange: (id: string) => void;
  onAdd: (type: NativeFilterType) => void;
@ -52,23 +52,26 @@ const TabsContainer = styled.div`
  flex-direction: column;
 `;

+const options = [
+  { label: 'Filter', type: NativeFilterType.NATIVE_FILTER },
+  { label: 'Divider', type: NativeFilterType.DIVIDER },
+];
+
 const FilterTitlePane: React.FC<Props> = ({
  getFilterTitle,
  onChange,
  onAdd,
  onRemove,
-  onRearrage,
+  onRearrange,
  restoreFilter,
  currentFilterId,
  filters,
  removedFilters,
  erroredFilters,
 }) => {
+  const filtersContainerRef = useRef<HTMLDivElement>(null);
  const theme = useTheme();
-  const options = [
-    { label: 'Filter', type: NativeFilterType.NATIVE_FILTER },
-    { label: 'Divider', type: NativeFilterType.DIVIDER },
-  ];
+
  const handleOnAdd = (type: NativeFilterType) => {
    onAdd(type);
    setTimeout(() => {
@ -77,6 +80,11 @@ const FilterTitlePane: React.FC<Props> = ({
        const navList = element.getElementsByClassName('ant-tabs-nav-list')[0];
        navList.scrollTop = navList.scrollHeight;
      }
+
+      filtersContainerRef?.current?.scroll?.({
+        top: filtersContainerRef.current.scrollHeight,
+        behavior: 'smooth',
+      });
    }, 0);
  };
  const menu = (
@ -109,6 +117,7 @@ const FilterTitlePane: React.FC<Props> = ({
        }}
      >
        <FilterTitleContainer
+          ref={filtersContainerRef}
          filters={filters}
          currentFilterId={currentFilterId}
          removedFilters={removedFilters}
@ -116,7 +125,7 @@ const FilterTitlePane: React.FC<Props> = ({
          erroredFilters={erroredFilters}
          onChange={onChange}
          onRemove={onRemove}
-          onRearrage={onRearrage}
+          onRearrange={onRearrange}
          restoreFilter={restoreFilter}
        />
      </div>
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
@ -38,7 +38,7 @@ import ErrorBoundary from 'src/components/ErrorBoundary';
 import { StyledModal } from 'src/components/Modal';
 import { testWithId } from 'src/utils/testUtils';
 import { useFilterConfigMap, useFilterConfiguration } from '../state';
-import FiltureConfigurePane from './FilterConfigurePane';
+import FilterConfigurePane from './FilterConfigurePane';
 import FiltersConfigForm, {
  FilterPanels,
 } from './FiltersConfigForm/FiltersConfigForm';
@ -379,7 +379,7 @@ export function FiltersConfigModal({
      handleConfirmCancel();
    }
  };
-  const onRearrage = (dragIndex: number, targetIndex: number) => {
+  const onRearrange = (dragIndex: number, targetIndex: number) => {
    const newOrderedFilter = [...orderedFilters];
    const removed = newOrderedFilter.splice(dragIndex, 1)[0];
    newOrderedFilter.splice(targetIndex, 0, removed);
@ -522,7 +522,7 @@ export function FiltersConfigModal({
            onValuesChange={onValuesChange}
            layout="vertical"
          >
-            <FiltureConfigurePane
+            <FilterConfigurePane
              erroredFilters={erroredFilters}
              onRemove={handleRemoveItem}
              onAdd={addFilter}
@ -531,11 +531,11 @@ export function FiltersConfigModal({
              currentFilterId={currentFilterId}
              removedFilters={removedFilters}
              restoreFilter={restoreFilter}
-              onRearrange={onRearrage}
+              onRearrange={onRearrange}
              filters={orderedFilters}
            >
              {(id: string) => getForm(id)}
-            </FiltureConfigurePane>
+            </FilterConfigurePane>
          </StyledForm>
        </StyledModalBody>
      </ErrorBoundary>