diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx index 188655355ca45..7a4827c80bcf0 100644 --- 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 = ({ index, - onRearrage, + onRearrange, filterIds, children, }) => { @@ -120,7 +120,7 @@ export const DraggableFilter: React.FC = ({ 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 diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx index 78c4d77da1918..3742d536326fb 100644 --- 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(); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx index a65e167fdf39b..dba7e6bb30250 100644 --- 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 = ({ +const FilterConfigurePane: React.FC = ({ getFilterTitle, onChange, onRemove, @@ -75,7 +75,7 @@ const FiltureConfigurePane: React.FC = ({ 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 = ({ ); }; -export default FiltureConfigurePane; +export default FilterConfigurePane; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitleContainer.tsx index 6ef40d8303b57..f5fe459e4b260 100644 --- 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; 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 = ({ - 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 ( - onChange(id)} - className={classNames.join(' ')} - > -
-
- {isRemoved ? t('(Removed)') : getFilterTitle(id)} +const FilterTitleContainer = forwardRef( + ( + { + 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 ( + onChange(id)} + className={classNames.join(' ')} + > +
+
+ {isRemoved ? t('(Removed)') : getFilterTitle(id)} +
+ {!removedFilters[id] && isErrored && ( + + )} + {isRemoved && ( + { + e.preventDefault(); + restoreFilter(id); + }} + > + {t('Undo?')} + + )}
- {!removedFilters[id] && isErrored && ( - - )} - {isRemoved && ( - { - e.preventDefault(); - restoreFilter(id); - }} - > - {t('Undo?')} - - )} -
-
- {isRemoved ? null : ( - { - event.stopPropagation(); - onRemove(id); - }} - alt="RemoveFilter" - /> - )} -
- - ); - }; - const recursivelyRender = ( - elementId: string, - nodeList: Array<{ id: string; parentId: string | null }>, - rendered: Array, - ): 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; - } +
+ {isRemoved ? null : ( + { + event.stopPropagation(); + onRemove(id); + }} + alt="RemoveFilter" + /> + )} +
+ + ); + }; + const recursivelyRender = ( + elementId: string, + nodeList: Array<{ id: string; parentId: string | null }>, + rendered: Array, + ): 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( + + {renderComponent(item)} + , + ); + }); + 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} - + + {renderFilterGroups()} + ); - }; - - const renderFilterGroups = () => { - const items: React.ReactNode[] = []; - filters.forEach((item, index) => { - items.push( - - {renderComponent(item)} - , - ); - }); - return items; - }; - return {renderFilterGroups()}; -}; + }, +); export default FilterTitleContainer; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx index 5681a41717666..79dc4148349aa 100644 --- 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 = ({ getFilterTitle, onChange, onAdd, onRemove, - onRearrage, + onRearrange, restoreFilter, currentFilterId, filters, removedFilters, erroredFilters, }) => { + const filtersContainerRef = useRef(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 = ({ 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 = ({ }} > = ({ erroredFilters={erroredFilters} onChange={onChange} onRemove={onRemove} - onRearrage={onRearrage} + onRearrange={onRearrange} restoreFilter={restoreFilter} />
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx index fd3ac06ea6db9..d258b34fa7489 100644 --- 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" > - {(id: string) => getForm(id)} - +