diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.test.tsx index 4527347e1271..7612499013b8 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.test.tsx @@ -17,6 +17,10 @@ * under the License. */ import { render, screen } from 'spec/helpers/testing-library'; +import { Provider } from 'react-redux'; +import { Store } from 'redux'; +import configureStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; import { stateWithoutNativeFilters } from 'spec/fixtures/mockStore'; import { ChartCustomizationType, @@ -25,7 +29,9 @@ import { import { FilterBarOrientation } from 'src/dashboard/types'; import FilterControls from './FilterControls'; -const mockStore = { +const mockStore = configureStore([thunk]); + +const mockStoreForCustomization = { ...stateWithoutNativeFilters, dashboardInfo: { ...stateWithoutNativeFilters.dashboardInfo, @@ -35,7 +41,7 @@ const mockStore = { jest.mock('react-redux', () => ({ ...jest.requireActual('react-redux'), - useSelector: jest.fn(selector => selector(mockStore)), + useSelector: jest.fn(selector => selector(mockStoreForCustomization)), useDispatch: () => jest.fn(), })); @@ -96,9 +102,9 @@ test('renders multiple chart customization dividers in vertical mode', () => { test('renders chart customization divider in horizontal mode', () => { const horizontalStore = { - ...mockStore, + ...mockStoreForCustomization, dashboardInfo: { - ...mockStore.dashboardInfo, + ...mockStoreForCustomization.dashboardInfo, filterBarOrientation: FilterBarOrientation.Horizontal, }, }; @@ -184,3 +190,222 @@ test('renders empty state when no chart customizations provided', () => { container.querySelector('.chart-customization-item-wrapper'), ).not.toBeInTheDocument(); }); + +const createMockFilter = (id: string, name: string) => ({ + id, + name, + filterType: 'filter_select', + targets: [{ datasetId: 1, column: { name: 'country' } }], + defaultDataMask: {}, + controlValues: {}, + cascadeParentIds: [], + scope: { + rootPath: ['ROOT_ID'], + excluded: [] as string[], + }, + isInstant: true, + allowsMultipleValues: true, + isRequired: false, +}); + +const getDefaultState = (orientation: FilterBarOrientation) => ({ + dashboardInfo: { + id: 1, + filterBarOrientation: orientation, + }, + dashboardLayout: { + present: { + ROOT_ID: { + type: 'ROOT', + id: 'ROOT_ID', + children: ['TABS-1'], + }, + 'TABS-1': { + type: 'TABS', + id: 'TABS-1', + children: ['TAB-1', 'TAB-2'], + }, + 'TAB-1': { + type: 'TAB', + id: 'TAB-1', + children: ['CHART-1'], + }, + 'TAB-2': { + type: 'TAB', + id: 'TAB-2', + children: ['CHART-2'], + }, + 'CHART-1': { + type: 'CHART', + id: 'CHART-1', + meta: { chartId: 1 }, + }, + 'CHART-2': { + type: 'CHART', + id: 'CHART-2', + meta: { chartId: 2 }, + }, + }, + }, + charts: { + 1: { id: 1, formData: {} }, + 2: { id: 2, formData: {} }, + }, + dataMask: {}, + nativeFilters: { + filters: { + 'filter-1': createMockFilter('filter-1', 'Country Filter'), + 'filter-2': createMockFilter('filter-2', 'Region Filter'), + 'filter-3': createMockFilter('filter-3', 'City Filter'), + }, + filterSets: {}, + }, + dashboardState: { + directPathToChild: [], + activeTabs: ['TAB-1'], + chartCustomizationItems: [], + }, + sliceEntities: { + slices: { + 1: { + slice_id: 1, + slice_name: 'Chart 1', + form_data: {}, + }, + 2: { + slice_id: 2, + slice_name: 'Chart 2', + form_data: {}, + }, + }, + }, + datasources: {}, +}); + +function setupWithFilters(overrideState: any = {}, props: any = {}) { + const state = { + ...getDefaultState(FilterBarOrientation.Vertical), + ...overrideState, + }; + const store = mockStore(state) as Store; + + return render( + + + , + ); +} + +test('FilterControls should mark out-of-scope filters as not overflowed in vertical mode', () => { + const stateWithVertical = getDefaultState(FilterBarOrientation.Vertical); + + stateWithVertical.nativeFilters.filters['filter-3'].scope = { + rootPath: ['ROOT_ID'], + excluded: ['TAB-1'], + }; + + const { container } = setupWithFilters(stateWithVertical); + + expect(container).toBeInTheDocument(); +}); + +test('FilterControls should mark out-of-scope filters as overflowed in horizontal mode', () => { + const stateWithHorizontal = getDefaultState(FilterBarOrientation.Horizontal); + + stateWithHorizontal.nativeFilters.filters['filter-3'].scope = { + rootPath: ['ROOT_ID'], + excluded: ['TAB-1'], + }; + + const { container } = setupWithFilters(stateWithHorizontal); + + expect(container).toBeInTheDocument(); +}); + +test('FilterControls overflowedByIndex calculation respects filter bar orientation', () => { + const verticalState = getDefaultState(FilterBarOrientation.Vertical); + verticalState.nativeFilters.filters['filter-2'].scope = { + rootPath: ['ROOT_ID'], + excluded: ['TAB-1'], + }; + + const { rerender, container } = setupWithFilters(verticalState); + expect(container).toBeInTheDocument(); + + const horizontalState = getDefaultState(FilterBarOrientation.Horizontal); + horizontalState.nativeFilters.filters['filter-2'].scope = { + rootPath: ['ROOT_ID'], + excluded: ['TAB-1'], + }; + + rerender( + + + , + ); + + expect(container).toBeInTheDocument(); +}); + +test('FilterControls should correctly pass isOverflowing prop to filter controls', () => { + const state = getDefaultState(FilterBarOrientation.Vertical); + + state.nativeFilters.filters['filter-1'].scope = { + rootPath: ['ROOT_ID'], + excluded: [], + }; + + state.nativeFilters.filters['filter-2'].scope = { + rootPath: ['ROOT_ID'], + excluded: ['TAB-1'], + }; + + const { container } = setupWithFilters(state); + + expect(container).toBeInTheDocument(); +}); + +test('FilterControls should handle empty filters list', () => { + const state = getDefaultState(FilterBarOrientation.Vertical); + state.nativeFilters.filters = {} as any; + + const { container } = setupWithFilters(state); + expect(container).toBeInTheDocument(); +}); + +test('FilterControls overflowedByIndex updates when filters change scope', () => { + const state = getDefaultState(FilterBarOrientation.Vertical); + + const { container, rerender } = setupWithFilters(state); + expect(container).toBeInTheDocument(); + + const updatedState = getDefaultState(FilterBarOrientation.Vertical); + updatedState.nativeFilters.filters['filter-1'].scope = { + rootPath: ['ROOT_ID'], + excluded: ['TAB-1'], + }; + + rerender( + + + , + ); + + expect(container).toBeInTheDocument(); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx index fd9cf7d6f77c..c94537da8519 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx @@ -659,12 +659,19 @@ const FilterControls: FC = ({ overflowedFiltersInScope.map(({ id }) => id), ); - return filtersWithValues.map( - filter => - filtersOutOfScopeIds.has(filter.id) || - overflowedFiltersInScopeIds.has(filter.id), - ); - }, [filtersOutOfScope, filtersWithValues, overflowedFiltersInScope]); + return filtersWithValues.map(filter => { + // Out-of-scope filters in vertical mode are in a Collapse panel, not overflowed + if (filtersOutOfScopeIds.has(filter.id)) { + return filterBarOrientation === FilterBarOrientation.Horizontal; + } + return overflowedFiltersInScopeIds.has(filter.id); + }); + }, [ + filtersOutOfScope, + filtersWithValues, + overflowedFiltersInScope, + filterBarOrientation, + ]); useEffect(() => { if (outlinedFilterId && overflowedIds.includes(outlinedFilterId)) { diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/tests/DateFilterLabel.test.tsx b/superset-frontend/src/explore/components/controls/DateFilterControl/tests/DateFilterLabel.test.tsx index f4c36bfa1749..1a88d1da8436 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/tests/DateFilterLabel.test.tsx +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/tests/DateFilterLabel.test.tsx @@ -93,3 +93,46 @@ test('Open and close popover', () => { expect(defaultProps.onClosePopover).toHaveBeenCalled(); expect(screen.queryByText('Edit time range')).not.toBeInTheDocument(); }); + +test('DateFilter popover should attach to document.body when not overflowing', () => { + render(setup({ ...defaultProps, isOverflowingFilterBar: false })); + + userEvent.click(screen.getByText(NO_TIME_RANGE)); + + const popover = document.querySelector('.time-range-popover'); + expect(popover?.parentElement).toBe(document.body); +}); + +test('DateFilter popover should attach to parent node when overflowing in filter bar', () => { + render(setup({ ...defaultProps, isOverflowingFilterBar: true })); + + userEvent.click(screen.getByText(NO_TIME_RANGE)); + + const popover = document.querySelector('.time-range-popover'); + const trigger = screen.getByTestId(DateFilterTestKey.PopoverOverlay); + + expect(popover?.parentElement).toBe(trigger.parentElement); +}); + +test('DateFilter should properly handle isOverflowingFilterBar prop changes', () => { + const { rerender } = render( + setup({ ...defaultProps, isOverflowingFilterBar: false }), + ); + + // When not overflowing, popover should attach to document.body + userEvent.click(screen.getByText(NO_TIME_RANGE)); + const popover = document.querySelector('.time-range-popover'); + expect(popover?.parentElement).toBe(document.body); + + userEvent.click(screen.getByText('CANCEL')); + + // When overflowing, popover should attach to parent node + rerender(setup({ ...defaultProps, isOverflowingFilterBar: true })); + userEvent.click(screen.getByText(NO_TIME_RANGE)); + + const popoverAfterRerender = document.querySelector('.time-range-popover'); + const trigger = screen.getByTestId(DateFilterTestKey.PopoverOverlay); + + expect(popoverAfterRerender?.parentElement).toBe(trigger.parentElement); + expect(popoverAfterRerender?.parentElement).not.toBe(document.body); +});