From ea534e23866915498a1c40fd4acfd16e4a475748 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Fri, 4 Mar 2022 13:06:10 -0300 Subject: [PATCH] feat: Adds support to multiple dependencies to the native filters (#18793) * chore(native-filters): Remove cascading popovers from filter bar Co-authored-by: Kamil Gabryjelski --- .../dashboard/nativeFilters.test.ts | 115 +------- .../src/chart/components/SuperChart.tsx | 2 + .../src/chart/models/ChartProps.ts | 10 + .../src/dashboard/types/Base.ts | 20 +- .../spec/fixtures/mockNativeFilters.ts | 4 +- .../src/components/Select/Select.tsx | 63 +++-- .../DashboardBuilder/DashboardContainer.tsx | 20 +- .../components/DashboardBuilder/state.ts | 3 +- .../components/FiltersBadge/selectors.ts | 2 +- .../CascadeFilterControl.test.tsx | 95 ------- .../CascadeFilterControl/index.tsx | 77 ------ .../CascadeFilters/CascadePopover/index.tsx | 247 ------------------ .../FilterBar/CascadeFilters/types.ts | 24 -- .../FilterControls/FilterControl.tsx | 43 +-- .../FilterControls/FilterControls.tsx | 58 ++-- .../FilterBar/FilterControls/FilterValue.tsx | 27 +- .../FilterBar/FilterControls/state.ts | 22 +- .../FilterBar/FilterControls/types.ts | 1 + .../FilterBar/FilterControls/utils.ts | 26 -- .../FilterBar/FilterSets/FiltersHeader.tsx | 10 +- .../nativeFilters/FilterBar/Header/index.tsx | 4 +- .../nativeFilters/FilterBar/index.tsx | 236 ++++++++++------- .../nativeFilters/FilterBar/state.ts | 2 +- .../nativeFilters/FilterBar/utils.ts | 24 +- .../FilterCard/DependenciesRow.tsx | 23 +- .../FilterCard/FilterCard.test.tsx | 1 - .../nativeFilters/FilterCard/ScopeRow.tsx | 46 +++- .../nativeFilters/FilterCard/Styles.ts | 4 + .../nativeFilters/FilterCard/index.tsx | 9 +- .../nativeFilters/FilterCard/types.ts | 2 +- .../FilterCard/useFilterDependencies.ts | 2 +- .../FilterCard/useFilterScope.ts | 41 +-- .../FiltersConfigModal/DraggableFilter.tsx | 4 +- .../FilterConfigPane.test.tsx | 4 +- .../FilterConfigurePane.tsx | 10 +- .../FilterTitleContainer.tsx | 13 +- .../FiltersConfigModal/FilterTitlePane.tsx | 6 +- .../FiltersConfigForm/CollapsibleControl.tsx | 6 +- .../FiltersConfigForm/DependencyList.tsx | 213 +++++++++++++++ .../FilterScope/FilterScope.test.tsx | 6 +- .../FiltersConfigForm/FiltersConfigForm.tsx | 129 +++------ .../FiltersConfigModal.test.tsx | 18 +- .../FiltersConfigModal/FiltersConfigModal.tsx | 225 +++++++++------- .../nativeFilters/FiltersConfigModal/state.ts | 2 +- .../nativeFilters/FiltersConfigModal/types.ts | 9 +- .../nativeFilters/FiltersConfigModal/utils.ts | 170 ++---------- .../components/nativeFilters/state.ts | 41 ++- .../components/nativeFilters/utils.ts | 10 +- superset-frontend/src/dashboard/styles.ts | 3 + .../DateFilterControl/DateFilterLabel.tsx | 22 +- .../index.tsx | 6 +- .../GroupBy/GroupByFilterPlugin.tsx | 5 +- .../components/GroupBy/transformProps.ts | 11 +- .../src/filters/components/GroupBy/types.ts | 1 + .../components/Range/RangeFilterPlugin.tsx | 6 +- .../components/Range/transformProps.ts | 11 +- .../components/Select/SelectFilterPlugin.tsx | 4 +- .../components/Select/transformProps.ts | 11 +- .../src/filters/components/Select/types.ts | 2 +- .../components/Time/TimeFilterPlugin.tsx | 5 +- .../filters/components/Time/transformProps.ts | 11 +- .../src/filters/components/Time/types.ts | 2 + .../TimeColumn/TimeColumnFilterPlugin.tsx | 5 +- .../components/TimeColumn/transformProps.ts | 11 +- .../filters/components/TimeColumn/types.ts | 1 + .../TimeGrain/TimeGrainFilterPlugin.tsx | 5 +- .../components/TimeGrain/transformProps.ts | 12 +- .../src/filters/components/TimeGrain/types.ts | 1 + .../src/filters/components/types.ts | 1 + 69 files changed, 940 insertions(+), 1325 deletions(-) delete mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/CascadeFilterControl.test.tsx delete mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/index.tsx delete mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx delete mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types.ts create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DependencyList.tsx diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts index f4a6f9df4a4cf..e11775d8bd1ad 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts @@ -22,11 +22,7 @@ import { nativeFilters, exploreView, } from 'cypress/support/directories'; -import { - testItems, - WORLD_HEALTH_CHARTS, - waitForChartLoad, -} from './dashboard.helper'; +import { testItems } from './dashboard.helper'; import { DASHBOARD_LIST } from '../dashboard_list/dashboard_list.helper'; import { CHART_LIST } from '../chart_list/chart_list.helper'; import { FORM_DATA_DEFAULTS } from '../explore/visualizations/shared.helper'; @@ -263,7 +259,6 @@ describe('Nativefilters Sanity test', () => { 'Filter has default value', 'Can select multiple values', 'Filter value is required', - 'Filter is hierarchical', 'Select first filter value by default', 'Inverse selection', 'Dynamically search all filter values', @@ -535,114 +530,6 @@ describe('Nativefilters Sanity test', () => { .contains('year') .should('be.visible'); }); - it('User can create parent filters using "Filter is hierarchical"', () => { - cy.get(nativeFilters.filterFromDashboardView.expand).click({ force: true }); - // Create region filter - cy.get(nativeFilters.filterFromDashboardView.createFilterButton) - .should('be.visible') - .click(); - cy.get(nativeFilters.filtersPanel.filterTypeInput) - .find(nativeFilters.filtersPanel.filterTypeItem) - .click({ force: true }); - cy.get('[label="Value"]').click(); - cy.get(nativeFilters.modal.container) - .find(nativeFilters.filtersPanel.datasetName) - .click({ force: true }) - .within(() => - cy - .get('input') - .type('wb_health_population{enter}', { delay: 50, force: true }), - ); - cy.get(nativeFilters.modal.container) - .find(nativeFilters.filtersPanel.filterName) - .click({ force: true }) - .clear() - .type('region', { scrollBehavior: false, force: true }); - cy.wait(3000); - cy.get(nativeFilters.silentLoading).should('not.exist'); - cy.get(nativeFilters.filtersPanel.filterInfoInput) - .last() - .should('be.visible') - .click({ force: true }); - cy.get(nativeFilters.filtersPanel.filterInfoInput).last().type('region'); - cy.get(nativeFilters.filtersPanel.inputDropdown) - .last() - .should('be.visible', { timeout: 20000 }) - .click({ force: true }); - // Create country filter - cy.get(nativeFilters.addFilterButton.button) - .first() - .click({ force: true }) - .then(() => { - cy.get(nativeFilters.addFilterButton.dropdownItem) - .contains('Filter') - .click({ force: true }); - }); - cy.get(nativeFilters.filtersPanel.filterTypeInput) - .find(nativeFilters.filtersPanel.filterTypeItem) - .last() - .click({ force: true }); - cy.get('[label="Value"]').last().click({ force: true }); - cy.get(nativeFilters.modal.container) - .find(nativeFilters.filtersPanel.datasetName) - .last() - .click({ scrollBehavior: false }) - .within(() => - cy - .get('input') - .clear({ force: true }) - .type('wb_health_population{enter}', { - delay: 50, - force: true, - scrollBehavior: false, - }), - ); - cy.get(nativeFilters.filtersPanel.filterInfoInput) - .last() - .should('be.visible') - .click({ force: true }); - cy.get(nativeFilters.filtersPanel.inputDropdown) - .should('be.visible', { timeout: 20000 }) - .last() - .click(); - cy.get(nativeFilters.modal.container) - .find(nativeFilters.filtersPanel.filterName) - .last() - .click({ force: true }) - .type('country', { scrollBehavior: false, force: true }); - cy.get(nativeFilters.silentLoading).should('not.exist'); - cy.get(nativeFilters.filtersPanel.filterInfoInput) - .last() - .click({ force: true }); - cy.get(nativeFilters.filtersPanel.filterInfoInput) - .last() - .type('country_name', { delay: 50, scrollBehavior: false, force: true }); - cy.get(nativeFilters.filtersPanel.inputDropdown) - .last() - .should('be.visible', { timeout: 20000 }) - .click({ force: true }); - // Setup parent filter - cy.get(nativeFilters.filterConfigurationSections.displayedSection).within( - () => { - cy.contains('Filter is hierarchical').should('be.visible').click(); - cy.wait(1000); - cy.get(nativeFilters.filterConfigurationSections.parentFilterInput) - .click() - .type('region{enter}', { delay: 30 }); - }, - ); - cy.get(nativeFilters.modal.footer) - .contains('Save') - .should('be.visible') - .click(); - WORLD_HEALTH_CHARTS.forEach(waitForChartLoad); - // assert that native filter is created - cy.get(nativeFilters.filterFromDashboardView.filterName) - .should('be.visible') - .contains('region'); - cy.get(nativeFilters.filterIcon).click(); - cy.contains('Select parent filters (2)').should('be.visible'); - }); }); xdescribe('Nativefilters', () => { diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx index a8e559244462d..4a9a29f206ef6 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx +++ b/superset-frontend/packages/superset-ui-core/src/chart/components/SuperChart.tsx @@ -63,6 +63,8 @@ export type Props = Omit & showOverflow?: boolean; /** Prop for popovercontainer ref */ parentRef?: RefObject; + /** Prop for chart ref */ + inputRef?: RefObject; /** Chart width */ height?: number | string; /** Chart height */ diff --git a/superset-frontend/packages/superset-ui-core/src/chart/models/ChartProps.ts b/superset-frontend/packages/superset-ui-core/src/chart/models/ChartProps.ts index bb0ab5d6f45e2..54dda97bb30ec 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/models/ChartProps.ts +++ b/superset-frontend/packages/superset-ui-core/src/chart/models/ChartProps.ts @@ -20,6 +20,7 @@ /** Type checking is disabled for this file due to reselect only supporting * TS declarations for selectors with up to 12 arguments. */ // @ts-nocheck +import { RefObject } from 'react'; import { createSelector } from 'reselect'; import { AppSection, @@ -90,6 +91,8 @@ export interface ChartPropsConfig { appSection?: AppSection; /** is the chart refreshing its contents */ isRefreshing?: boolean; + /** chart ref */ + inputRef?: RefObject; } const DEFAULT_WIDTH = 800; @@ -128,6 +131,8 @@ export default class ChartProps { isRefreshing?: boolean; + inputRef?: RefObject; + constructor(config: ChartPropsConfig & { formData?: FormData } = {}) { const { annotationData = {}, @@ -143,6 +148,7 @@ export default class ChartProps { height = DEFAULT_HEIGHT, appSection, isRefreshing, + inputRef, } = config; this.width = width; this.height = height; @@ -159,6 +165,7 @@ export default class ChartProps { this.behaviors = behaviors; this.appSection = appSection; this.isRefreshing = isRefreshing; + this.inputRef = inputRef; } } @@ -178,6 +185,7 @@ ChartProps.createSelector = function create(): ChartPropsSelector { input => input.behaviors, input => input.appSection, input => input.isRefreshing, + input => input.inputRef, ( annotationData, datasource, @@ -192,6 +200,7 @@ ChartProps.createSelector = function create(): ChartPropsSelector { behaviors, appSection, isRefreshing, + inputRef, ) => new ChartProps({ annotationData, @@ -207,6 +216,7 @@ ChartProps.createSelector = function create(): ChartPropsSelector { behaviors, appSection, isRefreshing, + inputRef, }), ); }; diff --git a/superset-frontend/packages/superset-ui-core/src/dashboard/types/Base.ts b/superset-frontend/packages/superset-ui-core/src/dashboard/types/Base.ts index fbd631d259145..af7ca34dde1ec 100644 --- a/superset-frontend/packages/superset-ui-core/src/dashboard/types/Base.ts +++ b/superset-frontend/packages/superset-ui-core/src/dashboard/types/Base.ts @@ -65,7 +65,7 @@ export type FilterSets = { [filtersSetId: string]: FilterSet; }; -export interface Filter { +export type Filter = { cascadeParentIds: string[]; defaultDataMask: DataMask; id: string; // randomly generated at filter creation @@ -90,19 +90,31 @@ export interface Filter { chartsInScope?: number[]; type: typeof NativeFilterType.NATIVE_FILTER; description: string; -} +}; -export interface Divider { +export type Divider = Partial> & { id: string; title: string; description: string; type: typeof NativeFilterType.DIVIDER; +}; + +export function isNativeFilter( + filterElement: Filter | Divider, +): filterElement is Filter { + return filterElement.type === NativeFilterType.NATIVE_FILTER; +} + +export function isFilterDivider( + filterElement: Filter | Divider, +): filterElement is Divider { + return filterElement.type === NativeFilterType.DIVIDER; } export type FilterConfiguration = Array; export type Filters = { - [filterId: string]: Filter; + [filterId: string]: Filter | Divider; }; export type NativeFiltersState = { diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts index 516a2c5aa3b14..32aeaa9290a1a 100644 --- a/superset-frontend/spec/fixtures/mockNativeFilters.ts +++ b/superset-frontend/spec/fixtures/mockNativeFilters.ts @@ -456,7 +456,7 @@ export const mockQueryDataForCountries = [ export const buildNativeFilter = ( id: string, name: string, - parents: string[], + dependencies: string[], ) => ({ id, controlValues: { @@ -481,7 +481,7 @@ export const buildNativeFilter = ( filterState: {}, ownState: {}, }, - cascadeParentIds: parents, + cascadeParentIds: dependencies, scope: { rootPath: ['ROOT_ID'], excluded: [], diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index 12b481bcc0f78..ec13791cd7869 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -17,6 +17,7 @@ * under the License. */ import React, { + forwardRef, ReactElement, ReactNode, RefObject, @@ -59,6 +60,7 @@ type PickedSelectProps = Pick< | 'onClear' | 'onFocus' | 'onBlur' + | 'onDropdownVisibleChange' | 'placeholder' | 'showSearch' | 'value' @@ -264,31 +266,35 @@ const getQueryCacheKey = (value: string, page: number, pageSize: number) => * Each of the categories come with different abilities. For a comprehensive guide please refer to * the storybook in src/components/Select/Select.stories.tsx. */ -const Select = ({ - allowNewOptions = false, - ariaLabel, - fetchOnlyOnSearch, - filterOption = true, - header = null, - invertSelection = false, - labelInValue = false, - lazyLoading = true, - loading, - mode = 'single', - name, - notFoundContent, - onError, - onChange, - onClear, - optionFilterProps = ['label', 'value'], - options, - pageSize = DEFAULT_PAGE_SIZE, - placeholder = t('Select ...'), - showSearch = true, - sortComparator = defaultSortComparator, - value, - ...props -}: SelectProps) => { +const Select = ( + { + allowNewOptions = false, + ariaLabel, + fetchOnlyOnSearch, + filterOption = true, + header = null, + invertSelection = false, + labelInValue = false, + lazyLoading = true, + loading, + mode = 'single', + name, + notFoundContent, + onError, + onChange, + onClear, + onDropdownVisibleChange, + optionFilterProps = ['label', 'value'], + options, + pageSize = DEFAULT_PAGE_SIZE, + placeholder = t('Select ...'), + showSearch = true, + sortComparator = defaultSortComparator, + value, + ...props + }: SelectProps, + ref: RefObject, +) => { const isAsync = typeof options === 'function'; const isSingleMode = mode === 'single'; const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch; @@ -616,6 +622,10 @@ const Select = ({ if (!isSingleMode && isDropdownVisible) { handleTopOptions(selectValue); } + + if (onDropdownVisibleChange) { + onDropdownVisibleChange(isDropdownVisible); + } }; const dropdownRender = ( @@ -759,6 +769,7 @@ const Select = ({ ) } + ref={ref} {...props} > {shouldUseChildrenOptions && @@ -779,4 +790,4 @@ const Select = ({ ); }; -export default Select; +export default forwardRef(Select); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index aebb2a14071fd..50e53b9c7fc79 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -20,7 +20,12 @@ // when its container size changes, due to e.g., builder side panel opening import React, { FC, useEffect, useState } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { FeatureFlag, Filters, isFeatureEnabled } from '@superset-ui/core'; +import { + FeatureFlag, + Filter, + Filters, + isFeatureEnabled, +} from '@superset-ui/core'; import { ParentSize } from '@vx/responsive'; import Tabs from 'src/components/Tabs'; import DashboardGrid from 'src/dashboard/containers/DashboardGrid'; @@ -68,11 +73,13 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { }, [getLeafComponentIdFromPath(directPathToChild)]); // recalculate charts and tabs in scopes of native filters only when a scope or dashboard layout changes - const filterScopes = Object.values(nativeFilters ?? {}).map(filter => ({ - id: filter.id, - scope: filter.scope, - type: filter.type, - })); + const filterScopes = Object.values(nativeFilters ?? {}).map( + (filter: Filter) => ({ + id: filter.id, + scope: filter.scope, + type: filter.type, + }), + ); useEffect(() => { if ( !isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) || @@ -92,7 +99,6 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { const chartsInScope: number[] = getChartIdsInFilterScope({ filterScope: { scope: scope.rootPath, - // @ts-ignore immune: scope.excluded, }, }); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts b/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts index 51d255db10442..4c56ab87eb95a 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts @@ -17,7 +17,6 @@ * under the License. */ import { useSelector } from 'react-redux'; -import { Filter } from '@superset-ui/core'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import { useCallback, useEffect, useState, useContext } from 'react'; import { URL_PARAMS } from 'src/constants'; @@ -44,7 +43,7 @@ export const useNativeFilters = () => { ); const filters = useFilters(); - const filterValues = Object.values(filters); + const filterValues = Object.values(filters); const nativeFiltersEnabled = showNativeFilters && diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts index 3eafe248a15ba..2c6022a8b2352 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts +++ b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts @@ -280,7 +280,7 @@ export const selectNativeIndicatorsForChart = ( ), ) .map(nativeFilter => { - const column = nativeFilter.targets[0]?.column?.name; + const column = nativeFilter.targets?.[0]?.column?.name; const filterState = dataMask[nativeFilter.id]?.filterState; const label = extractLabel(filterState); return { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/CascadeFilterControl.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/CascadeFilterControl.test.tsx deleted file mode 100644 index 4d7d92cdff9ce..0000000000000 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/CascadeFilterControl.test.tsx +++ /dev/null @@ -1,95 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import React from 'react'; -import { fireEvent, render, screen } from 'spec/helpers/testing-library'; -import { mockStore } from 'spec/fixtures/mockStore'; -import { Provider } from 'react-redux'; -import { nativeFiltersInfo } from 'src/dashboard/fixtures/mockNativeFilters'; -import CascadeFilterControl, { CascadeFilterControlProps } from '.'; - -const createProps = (defaultsId = nativeFiltersInfo.filters.DefaultsID) => ({ - filter: { - ...defaultsId, - cascadeChildren: [ - { - ...defaultsId, - name: 'test child filter 1', - cascadeChildren: [], - }, - { - ...defaultsId, - name: 'test child filter 2', - cascadeChildren: [ - { - ...defaultsId, - name: 'test child of a child filter', - cascadeChildren: [], - }, - ], - }, - ], - }, - onFilterSelectionChange: jest.fn(), -}); - -const setup = (props: CascadeFilterControlProps) => ( - - - -); - -test('should render', () => { - const { container } = render(setup(createProps())); - expect(container).toBeInTheDocument(); -}); - -test('should render the filter name', () => { - render(setup(createProps())); - expect(screen.getByText('test')).toBeInTheDocument(); -}); - -test('should render the children filter names', () => { - render(setup(createProps())); - expect(screen.getByText('test child filter 1')).toBeInTheDocument(); - expect(screen.getByText('test child filter 2')).toBeInTheDocument(); -}); - -test('should render the child of a child filter name', () => { - render(setup(createProps())); - expect(screen.getByText('test child of a child filter')).toBeInTheDocument(); -}); - -test('should render tooltip if description is not empty', async () => { - render(setup(createProps())); - expect(screen.getByText('test')).toBeInTheDocument(); - const toolTip = screen.getByText('test')?.parentElement?.querySelector('i'); - expect(toolTip).not.toBe(null); - fireEvent.mouseOver(toolTip as HTMLElement); - expect(await screen.findByText('test description')).toBeInTheDocument(); -}); - -test('should not render tooltip if description is empty', () => { - render( - setup( - createProps({ ...nativeFiltersInfo.filters.DefaultsID, description: '' }), - ), - ); - const toolTip = screen.getByText('test')?.parentElement?.querySelector('i'); - expect(toolTip).toBe(null); -}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/index.tsx deleted file mode 100644 index f19b6b01ac899..0000000000000 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/index.tsx +++ /dev/null @@ -1,77 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import React, { RefObject } from 'react'; -import { - DataMaskStateWithId, - Filter, - styled, - DataMask, -} from '@superset-ui/core'; -import FilterControl from 'src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl'; -import { CascadeFilter } from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types'; - -export interface CascadeFilterControlProps { - dataMaskSelected?: DataMaskStateWithId; - filter: CascadeFilter; - directPathToChild?: string[]; - onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void; - parentRef?: RefObject; -} - -const StyledDiv = styled.div` - display: flex; - width: 100%; - flex-direction: column; - align-items: center; - .ant-form-item { - margin-bottom: ${({ theme }) => theme.gridUnit * 4}px; - } -`; - -const CascadeFilterControl: React.FC = ({ - dataMaskSelected, - filter, - directPathToChild, - onFilterSelectionChange, - parentRef, -}) => ( - <> - - - {filter.cascadeChildren?.map(childFilter => ( - - ))} - - -); - -export default CascadeFilterControl; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx deleted file mode 100644 index 0bb177b9b9c10..0000000000000 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx +++ /dev/null @@ -1,247 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import React, { - useCallback, - useEffect, - useMemo, - useState, - useRef, -} from 'react'; -import { - css, - DataMask, - DataMaskStateWithId, - Filter, - styled, - SupersetTheme, - t, -} from '@superset-ui/core'; -import Popover from 'src/components/Popover'; -import Icons from 'src/components/Icons'; -import { Pill } from 'src/dashboard/components/FiltersBadge/Styles'; -import FilterControl from 'src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl'; -import CascadeFilterControl from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl'; -import { CascadeFilter } from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types'; - -interface CascadePopoverProps { - dataMaskSelected: DataMaskStateWithId; - filter: CascadeFilter; - visible: boolean; - directPathToChild?: string[]; - inView?: boolean; - onVisibleChange: (visible: boolean) => void; - onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void; -} - -const StyledTitleBox = styled.div` - display: flex; - flex-direction: row; - justify-content: space-between; - align-items: center; - background-color: ${({ theme }) => theme.colors.grayscale.light4}; - margin: ${({ theme }) => theme.gridUnit * -1}px - ${({ theme }) => theme.gridUnit * -4}px; // to override default antd padding - padding: ${({ theme }) => theme.gridUnit * 2}px - ${({ theme }) => theme.gridUnit * 4}px; - - & > *:last-child { - cursor: pointer; - } -`; - -const StyledTitle = styled.h4` - display: flex; - align-items: center; - color: ${({ theme }) => theme.colors.grayscale.dark1}; - margin: 0; - padding: 0; -`; - -const IconStyles = (theme: SupersetTheme) => css` - margin-right: ${theme.gridUnit}px; - color: ${theme.colors.grayscale.dark1}; - width: ${theme.gridUnit * 4}px; -`; - -const StyledPill = styled(Pill)` - padding: ${({ theme }) => theme.gridUnit}px - ${({ theme }) => theme.gridUnit * 2}px; - font-size: ${({ theme }) => theme.typography.sizes.s}px; - background: ${({ theme }) => theme.colors.grayscale.light1}; -`; - -const ContentStyles = styled.div` - max-height: 700px; - overflow: auto; -`; - -const CascadePopover: React.FC = ({ - dataMaskSelected, - filter, - visible, - onVisibleChange, - onFilterSelectionChange, - directPathToChild, - inView, -}) => { - const [currentPathToChild, setCurrentPathToChild] = useState(); - const dataMask = dataMaskSelected[filter.id]; - const parent = useRef(); - - useEffect(() => { - setCurrentPathToChild(directPathToChild); - // clear local copy of directPathToChild after 500ms - // to prevent triggering multiple focus - const timeout = setTimeout(() => setCurrentPathToChild(undefined), 500); - return () => clearTimeout(timeout); - }, [directPathToChild, setCurrentPathToChild]); - - const getActiveChildren = useCallback( - (filter: CascadeFilter): CascadeFilter[] | null => { - const children = filter.cascadeChildren || []; - const currentValue = dataMask?.filterState?.value; - - const activeChildren = children.flatMap( - childFilter => getActiveChildren(childFilter) || [], - ); - - if (activeChildren.length > 0) { - return activeChildren; - } - - if (currentValue !== undefined && currentValue !== null) { - return [filter]; - } - - return null; - }, - [dataMask], - ); - - const getAllFilters = (filter: CascadeFilter): CascadeFilter[] => { - const children = filter.cascadeChildren || []; - const allChildren = children.flatMap(getAllFilters); - return [filter, ...allChildren]; - }; - - const allFilters = getAllFilters(filter); - const activeFilters = useMemo( - () => getActiveChildren(filter) || [filter], - [filter, getActiveChildren], - ); - - useEffect(() => { - const focusedFilterId = currentPathToChild?.[0]; - // filters not directly displayed in the Filter Bar - const inactiveFilters = allFilters.filter( - filterEl => !activeFilters.includes(filterEl), - ); - const focusedInactiveFilter = inactiveFilters.some( - cascadeChild => cascadeChild.id === focusedFilterId, - ); - - if (focusedInactiveFilter) { - onVisibleChange(true); - } - }, [currentPathToChild]); - - if (!filter.cascadeChildren?.length) { - return ( - - ); - } - - const title = ( - - - IconStyles(theme)} - /> - {t('Select parent filters')} ({allFilters.length}) - - IconStyles(theme)} - onClick={() => onVisibleChange(false)} - /> - - ); - - const content = ( - - - - ); - - return ( - -
- {activeFilters.map(activeFilter => ( - - {filter.cascadeChildren.length !== 0 && ( - onVisibleChange(true)}> - {allFilters.length} - - )} - - } - /> - ))} -
-
- ); -}; -export default React.memo(CascadePopover); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types.ts deleted file mode 100644 index 6d319f6d29452..0000000000000 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types.ts +++ /dev/null @@ -1,24 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { DataMask, Filter } from '@superset-ui/core'; - -export type CascadeFilter = Filter & { dataMask?: DataMask } & { - cascadeChildren: CascadeFilter[]; -}; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx index eec3c78fa3e75..ff5b08781e4e4 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx @@ -16,13 +16,15 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useMemo } from 'react'; +import React, { useContext, useMemo, useState } from 'react'; import { styled, SupersetTheme } from '@superset-ui/core'; import { FormItem as StyledFormItem, Form } from 'src/components/Form'; import { Tooltip } from 'src/components/Tooltip'; import { checkIsMissingRequiredValue } from '../utils'; import FilterValue from './FilterValue'; import { FilterProps } from './types'; +import { FilterCard } from '../../FilterCard'; +import { FilterBarScrollContext } from '../index'; const StyledIcon = styled.div` position: absolute; @@ -117,6 +119,8 @@ const FilterControl: React.FC = ({ showOverflow, parentRef, }) => { + const [isFilterActive, setIsFilterActive] = useState(false); + const { name = '' } = filter; const isMissingRequiredValue = checkIsMissingRequiredValue( @@ -141,23 +145,30 @@ const FilterControl: React.FC = ({ [name, isRequired, filter.description, icon], ); + const isScrolling = useContext(FilterBarScrollContext); + return ( - - - + +
+ + + +
+
); }; 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 d0591dfa72d61..55a14e06f6a75 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx @@ -16,13 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import React, { FC, useCallback, useMemo, useState } from 'react'; +import React, { FC, useCallback, useMemo } from 'react'; import { css } from '@emotion/react'; import { DataMask, DataMaskStateWithId, Filter, - NativeFilterType, + isFilterDivider, styled, t, } from '@superset-ui/core'; @@ -36,9 +36,8 @@ import { useDashboardHasTabs, useSelectFiltersInScope, } from 'src/dashboard/components/nativeFilters/state'; -import CascadePopover from '../CascadeFilters/CascadePopover'; import { useFilters } from '../state'; -import { buildCascadeFiltersTree } from './utils'; +import FilterControl from './FilterControl'; const Wrapper = styled.div` padding: ${({ theme }) => theme.gridUnit * 4}px; @@ -57,9 +56,8 @@ const FilterControls: FC = ({ dataMaskSelected, onFilterSelectionChange, }) => { - const [visiblePopoverId, setVisiblePopoverId] = useState(null); const filters = useFilters(); - const filterValues = useMemo(() => Object.values(filters), [filters]); + const filterValues = useMemo(() => Object.values(filters), [filters]); const portalNodes = useMemo(() => { const nodes = new Array(filterValues.length); for (let i = 0; i < filterValues.length; i += 1) { @@ -68,24 +66,25 @@ const FilterControls: FC = ({ return nodes; }, [filterValues.length]); - const cascadeFilters = useMemo(() => { - const filtersWithValue = filterValues.map(filter => ({ - ...filter, - dataMask: dataMaskSelected[filter.id], - })); - return buildCascadeFiltersTree(filtersWithValue); - }, [filterValues, dataMaskSelected]); - const cascadeFilterIds = new Set(cascadeFilters.map(item => item.id)); + const filtersWithValues = useMemo( + () => + filterValues.map(filter => ({ + ...filter, + dataMask: dataMaskSelected[filter.id], + })), + [filterValues, dataMaskSelected], + ); + const filterIds = new Set(filtersWithValues.map(item => item.id)); const [filtersInScope, filtersOutOfScope] = - useSelectFiltersInScope(cascadeFilters); + useSelectFiltersInScope(filtersWithValues); const dashboardHasTabs = useDashboardHasTabs(); - const showCollapsePanel = dashboardHasTabs && cascadeFilters.length > 0; + const showCollapsePanel = dashboardHasTabs && filtersWithValues.length > 0; - const cascadePopoverFactory = useCallback( + const filterControlFactory = useCallback( index => { - const filter = cascadeFilters[index]; - if (filter.type === NativeFilterType.DIVIDER) { + const filter = filtersWithValues[index]; + if (isFilterDivider(filter)) { return (

{filter.title}

@@ -94,35 +93,28 @@ const FilterControls: FC = ({ ); } return ( - - setVisiblePopoverId(visible ? filter.id : null) - } filter={filter} - onFilterSelectionChange={onFilterSelectionChange} directPathToChild={directPathToChild} + onFilterSelectionChange={onFilterSelectionChange} inView={false} /> ); }, [ - cascadeFilters, + filtersWithValues, JSON.stringify(dataMaskSelected), directPathToChild, onFilterSelectionChange, - visiblePopoverId, ], ); return ( {portalNodes - .filter((node, index) => cascadeFilterIds.has(filterValues[index].id)) + .filter((node, index) => filterIds.has(filterValues[index].id)) .map((node, index) => ( - {cascadePopoverFactory(index)} + {filterControlFactory(index)} ))} {filtersInScope.map(filter => { const index = filterValues.findIndex(f => f.id === filter.id); @@ -161,7 +153,9 @@ const FilterControls: FC = ({ key="1" > {filtersOutOfScope.map(filter => { - const index = cascadeFilters.findIndex(f => f.id === filter.id); + const index = filtersWithValues.findIndex( + f => f.id === filter.id, + ); return ; })} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx index 4d6bdc23f022f..caf00521a8e1d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -46,7 +46,7 @@ import { RootState } from 'src/dashboard/types'; import { dispatchFocusAction } from './utils'; import { FilterProps } from './types'; import { getFormData } from '../../utils'; -import { useCascadingFilters } from './state'; +import { useFilterDependencies } from './state'; import { checkIsMissingRequiredValue } from '../utils'; const HEIGHT = 32; @@ -70,10 +70,11 @@ const FilterValue: React.FC = ({ inView = true, showOverflow, parentRef, + setFilterActive, }) => { const { id, targets, filterType, adhoc_filters, time_range } = filter; const metadata = getChartMetadataRegistry().get(filterType); - const cascadingFilters = useCascadingFilters(id, dataMaskSelected); + const dependencies = useFilterDependencies(id, dataMaskSelected); const isDashboardRefreshing = useSelector( state => state.dashboardState.isRefreshing, ); @@ -109,9 +110,8 @@ const FilterValue: React.FC = ({ const newFormData = getFormData({ ...filter, datasetId, - cascadingFilters, + dependencies, groupby, - inputRef, adhoc_filters, time_range, }); @@ -184,7 +184,7 @@ const FilterValue: React.FC = ({ } }, [ inViewFirstTime, - cascadingFilters, + dependencies, datasetId, groupby, JSON.stringify(filter), @@ -195,13 +195,8 @@ const FilterValue: React.FC = ({ useEffect(() => { if (directPathToChild?.[0] === filter.id) { - // wait for Cascade Popover to open - const timeout = setTimeout(() => { - inputRef?.current?.focus(); - }, 200); - return () => clearTimeout(timeout); + inputRef?.current?.focus(); } - return undefined; }, [inputRef, directPathToChild, filter.id]); const setDataMask = useCallback( @@ -219,8 +214,13 @@ const FilterValue: React.FC = ({ ); const hooks = useMemo( - () => ({ setDataMask, setFocusedFilter, unsetFocusedFilter }), - [setDataMask, setFocusedFilter, unsetFocusedFilter], + () => ({ + setDataMask, + setFocusedFilter, + unsetFocusedFilter, + setFilterActive, + }), + [setDataMask, setFilterActive, setFocusedFilter, unsetFocusedFilter], ); const isMissingRequiredValue = checkIsMissingRequiredValue( @@ -257,6 +257,7 @@ const FilterValue: React.FC = ({ showOverflow={showOverflow} formData={formData} parentRef={parentRef} + inputRef={inputRef} // For charts that don't have datasource we need workaround for empty placeholder queriesData={hasDataSource ? state : queriesDataPlaceholder} chartType={filterType} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts index 80ce41b5aef44..a71028145969b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts @@ -20,30 +20,28 @@ import { useMemo } from 'react'; import { useSelector } from 'react-redux'; import { DataMaskStateWithId, + ensureIsArray, ExtraFormData, - NativeFiltersState, } from '@superset-ui/core'; import { mergeExtraFormData } from '../../utils'; // eslint-disable-next-line import/prefer-default-export -export function useCascadingFilters( +export function useFilterDependencies( id: string, dataMaskSelected?: DataMaskStateWithId, ): ExtraFormData { - const { filters } = useSelector( - state => state.nativeFilters, + const dependencyIds = useSelector( + state => state.nativeFilters.filters[id]?.cascadeParentIds, ); - const filter = filters[id]; return useMemo(() => { - const cascadeParentIds: string[] = filter?.cascadeParentIds ?? []; - let cascadedFilters = {}; - cascadeParentIds.forEach(parentId => { + let dependencies = {}; + ensureIsArray(dependencyIds).forEach(parentId => { const parentState = dataMaskSelected?.[parentId]; - cascadedFilters = mergeExtraFormData( - cascadedFilters, + dependencies = mergeExtraFormData( + dependencies, parentState?.extraFormData, ); }); - return cascadedFilters; - }, [dataMaskSelected, filter?.cascadeParentIds]); + return dependencies; + }, [dataMaskSelected, dependencyIds]); } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/types.ts index 60a7b2d526f27..e5b553712634e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/types.ts @@ -30,4 +30,5 @@ export interface FilterProps { inView?: boolean; showOverflow?: boolean; parentRef?: RefObject; + setFilterActive?: (isActive: boolean) => void; } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/utils.ts index 87f11a76f9ad8..3077c42768e84 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/utils.ts @@ -18,36 +18,10 @@ */ import { debounce } from 'lodash'; import { Dispatch } from 'react'; -import { Filter, NativeFilterType, Divider } from '@superset-ui/core'; import { setFocusedNativeFilter, unsetFocusedNativeFilter, } from 'src/dashboard/actions/nativeFilters'; -import { CascadeFilter } from '../CascadeFilters/types'; -import { mapParentFiltersToChildren } from '../utils'; - -// eslint-disable-next-line import/prefer-default-export -export function buildCascadeFiltersTree( - filters: Array, -): Array { - const cascadeChildren = mapParentFiltersToChildren(filters); - - const getCascadeFilter = (filter: Filter): CascadeFilter => { - const children = cascadeChildren[filter.id] || []; - return { - ...filter, - cascadeChildren: children.map(getCascadeFilter), - }; - }; - - return filters - .filter( - filter => - filter.type === NativeFilterType.DIVIDER || - !(filter as Filter).cascadeParentIds?.length, - ) - .map(getCascadeFilter); -} export const dispatchFocusAction = debounce( (dispatch: Dispatch, id?: string) => { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FiltersHeader.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FiltersHeader.tsx index 8f5f4a1a93c23..6185c573f398a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FiltersHeader.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FiltersHeader.tsx @@ -20,7 +20,7 @@ import React, { FC } from 'react'; import { DataMaskState, FilterSet, - NativeFilterType, + isNativeFilter, styled, t, useTheme, @@ -73,13 +73,13 @@ export type FiltersHeaderProps = { const FiltersHeader: FC = ({ dataMask, filterSet }) => { const theme = useTheme(); const filters = useFilters(); - const filterValues = Object.values(filters).filter( - nativeFilter => nativeFilter.type === NativeFilterType.NATIVE_FILTER, - ); + const filterValues = Object.values(filters).filter(isNativeFilter); let resultFilters = filterValues ?? []; if (filterSet?.nativeFilters) { - resultFilters = Object.values(filterSet?.nativeFilters); + resultFilters = Object.values(filterSet?.nativeFilters).filter( + isNativeFilter, + ); } const getFiltersHeader = () => ( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx index 86895b6a4f8fe..45019f58cdfc7 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx @@ -17,7 +17,7 @@ * under the License. */ /* eslint-disable no-param-reassign */ -import { css, Filter, styled, t, useTheme } from '@superset-ui/core'; +import { css, styled, t, useTheme } from '@superset-ui/core'; import React, { FC } from 'react'; import Icons from 'src/components/Icons'; import Button from 'src/components/Button'; @@ -74,7 +74,7 @@ const AddFiltersButtonContainer = styled.div` const Header: FC = ({ toggleFiltersBar }) => { const theme = useTheme(); const filters = useFilters(); - const filterValues = Object.values(filters); + const filterValues = Object.values(filters); const canEdit = useSelector( ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, ); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index cb5e3e80a7ae1..867b6752f4f2f 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -18,20 +18,28 @@ */ /* eslint-disable no-param-reassign */ +import throttle from 'lodash/throttle'; +import React, { + useEffect, + useState, + useCallback, + useMemo, + useRef, + createContext, +} from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import cx from 'classnames'; import { DataMaskStateWithId, DataMaskWithId, Filter, - NativeFilterType, DataMask, HandlerFunction, styled, t, SLOW_DEBOUNCE, + isNativeFilter, } from '@superset-ui/core'; -import React, { useEffect, useState, useCallback, useMemo } from 'react'; -import { useDispatch, useSelector } from 'react-redux'; -import cx from 'classnames'; import Icons from 'src/components/Icons'; import { Tabs } from 'src/common/components'; import { useHistory } from 'react-router-dom'; @@ -47,6 +55,7 @@ import { URL_PARAMS } from 'src/constants'; import { getUrlParam } from 'src/utils/urlUtils'; import { EmptyStateSmall } from 'src/components/EmptyState'; import { useTabId } from 'src/hooks/useTabId'; +import { RootState } from 'src/dashboard/types'; import { checkIsApplyDisabled, TabIds } from './utils'; import FilterSets from './FilterSets'; import { @@ -60,7 +69,6 @@ import { createFilterKey, updateFilterKey } from './keyValue'; import EditSection from './FilterSets/EditSection'; import Header from './Header'; import FilterControls from './FilterControls/FilterControls'; -import { RootState } from '../../../types'; import { ActionButtons } from './ActionButtons'; export const FILTER_BAR_TEST_ID = 'filter-bar'; @@ -204,6 +212,7 @@ const publishDataMask = debounce( SLOW_DEBOUNCE, ); +export const FilterBarScrollContext = createContext(false); const FilterBar: React.FC = ({ filtersOpen, toggleFiltersBar, @@ -225,7 +234,8 @@ const FilterBar: React.FC = ({ const [tab, setTab] = useState(TabIds.AllFilters); const filters = useFilters(); const previousFilters = usePrevious(filters); - const filterValues = Object.values(filters); + const filterValues = Object.values(filters); + const nativeFilterValues = filterValues.filter(isNativeFilter); const dashboardId = useSelector( ({ dashboardInfo }) => dashboardInfo?.id, ); @@ -233,6 +243,9 @@ const FilterBar: React.FC = ({ ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, ); + const [isScrolling, setIsScrolling] = useState(false); + const timeout = useRef(); + const handleFilterSelectionChange = useCallback( ( filter: Pick & Partial, @@ -328,11 +341,29 @@ const FilterBar: React.FC = ({ [toggleFiltersBar], ); + const onScroll = useCallback( + throttle(() => { + clearTimeout(timeout.current); + setIsScrolling(true); + timeout.current = setTimeout(() => { + setIsScrolling(false); + }, 300); + }, 200), + [], + ); + + useEffect(() => { + document.onscroll = onScroll; + return () => { + document.onscroll = null; + }; + }, [onScroll]); + useFilterUpdates(dataMaskSelected, setDataMaskSelected); const isApplyDisabled = checkIsApplyDisabled( dataMaskSelected, dataMaskApplied, - filterValues, + nativeFilterValues, ); const isInitialized = useInitialization(); const tabPaneStyle = useMemo( @@ -340,56 +371,98 @@ const FilterBar: React.FC = ({ [height], ); - const numberOfFilters = filterValues.filter( - filterValue => filterValue.type === NativeFilterType.NATIVE_FILTER, - ).length; + const numberOfFilters = nativeFilterValues.length; return ( - - + - - - - -
- {!isInitialized ? ( -
- -
- ) : isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) ? ( - - + + + + +
+ {!isInitialized ? ( +
+ +
+ ) : isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) ? ( + - {editFilterSetId && ( - + {editFilterSetId && ( + setEditFilterSetId(null)} + filterSetId={editFilterSetId} + /> + )} + {filterValues.length === 0 ? ( + + + + ) : ( + + )} + + + setEditFilterSetId(null)} - filterSetId={editFilterSetId} + dataMaskSelected={dataMaskSelected} + tab={tab} + onFilterSelectionChange={handleFilterSelectionChange} /> - )} + + + ) : ( +
{filterValues.length === 0 ? ( = ({ onFilterSelectionChange={handleFilterSelectionChange} /> )} - - - - - - ) : ( -
- {filterValues.length === 0 ? ( - - - - ) : ( - - )} -
- )} - - - +
+ )} + + + + ); }; export default React.memo(FilterBar); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts index 054f440a74f46..4e1b2eda1271e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts @@ -42,7 +42,7 @@ export const useFilters = () => { const preselectedNativeFilters = useSelector( state => state.dashboardState?.preselectNativeFilters, ); - const nativeFilters = useSelector( + const nativeFilters = useSelector( state => state.nativeFilters.filters, ); return useMemo( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts index 926abbf069563..842bb440542c3 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts @@ -18,35 +18,13 @@ */ import { areObjectsEqual } from 'src/reduxUtils'; -import { - DataMaskStateWithId, - Filter, - FilterState, - Divider, -} from '@superset-ui/core'; +import { DataMaskStateWithId, Filter, FilterState } from '@superset-ui/core'; export enum TabIds { AllFilters = 'allFilters', FilterSets = 'filterSets', } -export function mapParentFiltersToChildren(filters: Array): { - [id: string]: Filter[]; -} { - const cascadeChildren = {}; - filters.forEach(filter => { - const [parentId] = - ('cascadeParentIds' in filter && filter.cascadeParentIds) || []; - if (parentId) { - if (!cascadeChildren[parentId]) { - cascadeChildren[parentId] = []; - } - cascadeChildren[parentId].push(filter); - } - }); - return cascadeChildren; -} - export const getOnlyExtraFormData = (data: DataMaskStateWithId) => Object.values(data).reduce( (prev, next) => ({ ...prev, [next.id]: next.extraFormData }), diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx index c585fc40e4119..18a1c257b4ba3 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx @@ -34,15 +34,21 @@ import { useTruncation } from './useTruncation'; import { DependencyValueProps, FilterCardRowProps } from './types'; import { TooltipWithTruncation } from './TooltipWithTruncation'; -const DependencyValue = ({ dependency, label }: DependencyValueProps) => { +const DependencyValue = ({ + dependency, + hasSeparator, +}: DependencyValueProps) => { const dispatch = useDispatch(); const handleClick = useCallback(() => { dispatch(setDirectPathToChild([dependency.id])); }, [dependency.id, dispatch]); return ( - - {label} - + + {hasSeparator && , } + + {dependency.name} + + ); }; @@ -58,10 +64,7 @@ export const DependenciesRow = React.memo(({ filter }: FilterCardRowProps) => { {dependencies.map(dependency => (
  • - +
  • ))}
    @@ -87,7 +90,7 @@ export const DependenciesRow = React.memo(({ filter }: FilterCardRowProps) => { )} > { {dependencies.map((dependency, index) => ( ))} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx index 5a0663032d3cf..93f1ccfca438d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx @@ -246,7 +246,6 @@ describe('Filter Card', () => { }; renderContent(filter); expect(screen.getByText('Scope')).toBeVisible(); - expect(screen.getByText('Test chart 2')).toBeVisible(); expect( screen.getByText(getTextInHTMLTags('Test chart 2, Test chart 3')), ).toBeVisible(); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx index f7ede30692bd4..66656f0ba514d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/ScopeRow.tsx @@ -25,27 +25,43 @@ import { RowTruncationCount, RowValue, TooltipList, + TooltipSectionLabel, } from './Styles'; import { useTruncation } from './useTruncation'; import { FilterCardRowProps } from './types'; import { TooltipWithTruncation } from './TooltipWithTruncation'; +const getTooltipSection = (items: string[] | undefined, label: string) => + Array.isArray(items) && items.length > 0 ? ( + <> + {label}: + + {items.map(item => ( +
  • {item}
  • + ))} +
    + + ) : null; + export const ScopeRow = React.memo(({ filter }: FilterCardRowProps) => { const scope = useFilterScope(filter); const scopeRef = useRef(null); const [elementsTruncated, hasHiddenElements] = useTruncation(scopeRef); - const tooltipText = useMemo( - () => - elementsTruncated > 0 && scope ? ( - - {scope.map(val => ( -
  • {val}
  • - ))} -
    - ) : null, - [elementsTruncated, scope], - ); + const tooltipText = useMemo(() => { + if (elementsTruncated === 0 || !scope) { + return null; + } + if (scope.all) { + return {t('All charts')}; + } + return ( +
    + {getTooltipSection(scope.tabs, t('Tabs'))} + {getTooltipSection(scope.charts, t('Charts'))} +
    + ); + }, [elementsTruncated, scope]); return ( @@ -53,9 +69,11 @@ export const ScopeRow = React.memo(({ filter }: FilterCardRowProps) => { {scope - ? scope.map((element, index) => ( - {index === 0 ? element : `, ${element}`} - )) + ? Object.values(scope) + .flat() + .map((element, index) => ( + {index === 0 ? element : `, ${element}`} + )) : t('None')} {hasHiddenElements > 0 && ( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/Styles.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/Styles.ts index 9799a0d333bd4..bda865063d751 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/Styles.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/Styles.ts @@ -83,6 +83,10 @@ export const TooltipList = styled.ul` `}; `; +export const TooltipSectionLabel = styled.span` + font-weight: ${({ theme }) => theme.typography.weights.bold}; +`; + export const TooltipTrigger = styled.div` min-width: 0; display: inline-flex; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx index 6f544fc95e8ac..bc1f7b2ea3712 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/index.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { useState } from 'react'; +import React, { useEffect, useState } from 'react'; import Popover from 'src/components/Popover'; import { FilterCardContent } from './FilterCardContent'; import { FilterCardProps } from './types'; @@ -30,6 +30,11 @@ export const FilterCard = ({ }: FilterCardProps) => { const [internalIsVisible, setInternalIsVisible] = useState(false); + useEffect(() => { + if (!externalIsVisible) { + setInternalIsVisible(false); + } + }, [externalIsVisible]); return ( { - setInternalIsVisible(visible); + setInternalIsVisible(externalIsVisible && visible); }} visible={externalIsVisible && internalIsVisible} content={} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/types.ts index dd5bdf477bece..6e65a4eecc634 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/types.ts @@ -32,5 +32,5 @@ export interface FilterCardRowProps { export interface DependencyValueProps { dependency: Filter; - label: string; + hasSeparator?: boolean; } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterDependencies.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterDependencies.ts index 60966658d6127..ee2cc9c6d2c38 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterDependencies.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterDependencies.ts @@ -27,7 +27,7 @@ export const useFilterDependencies = (filter: Filter) => { return []; } return filterDependencyIds.reduce((acc: Filter[], filterDependencyId) => { - acc.push(state.nativeFilters.filters[filterDependencyId]); + acc.push(state.nativeFilters.filters[filterDependencyId] as Filter); return acc; }, []); }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterScope.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterScope.ts index f713076168ce2..12a578c35a140 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterScope.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/useFilterScope.ts @@ -29,9 +29,9 @@ import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; import { useMemo } from 'react'; const extractTabLabel = (tab?: LayoutItem) => - tab?.meta?.text || tab?.meta?.defaultText; + tab?.meta?.text || tab?.meta?.defaultText || ''; const extractChartLabel = (chart?: LayoutItem) => - chart?.meta?.sliceNameOverride || chart?.meta?.sliceName || chart?.id; + chart?.meta?.sliceNameOverride || chart?.meta?.sliceName || chart?.id || ''; const useCharts = () => { const charts = useSelector(state => state.charts); @@ -67,13 +67,17 @@ export const useFilterScope = (filter: Filter) => { filter.scope.rootPath.includes(topLevelTab), ))) ) { - return [t('All charts')]; + return { all: [t('All charts')] }; } // no charts excluded and not every top level tab in scope // returns "TAB1, TAB2" if (filter.scope.excluded.length === 0 && topLevelTabs) { - return filter.scope.rootPath.map(tabId => extractTabLabel(layout[tabId])); + return { + tabs: filter.scope.rootPath + .map(tabId => extractTabLabel(layout[tabId])) + .filter(Boolean), + }; } const layoutCharts = Object.values(layout).filter( @@ -83,15 +87,17 @@ export const useFilterScope = (filter: Filter) => { // no top level tabs, charts excluded // returns "CHART1, CHART2" if (filter.scope.rootPath[0] === DASHBOARD_ROOT_ID) { - return charts - .filter(chart => !filter.scope.excluded.includes(chart.id)) - .map(chart => { - const layoutElement = layoutCharts.find( - layoutChart => layoutChart.meta.chartId === chart.id, - ); - return extractChartLabel(layoutElement); - }) - .filter(Boolean); + return { + charts: charts + .filter(chart => !filter.scope.excluded.includes(chart.id)) + .map(chart => { + const layoutElement = layoutCharts.find( + layoutChart => layoutChart.meta.chartId === chart.id, + ); + return extractChartLabel(layoutElement); + }) + .filter(Boolean), + }; } // top level tabs, charts excluded @@ -133,9 +139,12 @@ export const useFilterScope = (filter: Filter) => { return acc; }, []); // Join tab names and chart names - return topLevelTabsInFullScope - .map(tabId => extractTabLabel(layout[tabId])) - .concat(chartsInExcludedTabs.map(extractChartLabel)); + return { + tabs: topLevelTabsInFullScope + .map(tabId => extractTabLabel(layout[tabId])) + .filter(Boolean), + charts: chartsInExcludedTabs.map(extractChartLabel).filter(Boolean), + }; } return undefined; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx index dbaba821c84b8..188655355ca45 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/DraggableFilter.tsx @@ -39,7 +39,7 @@ const Container = styled.div` cursor: ${isDragging ? 'grabbing' : 'pointer'}; width: 100%; display: flex; - padding: ${theme.gridUnit}px + padding: ${theme.gridUnit}px; `} `; @@ -133,7 +133,7 @@ export const DraggableFilter: React.FC = ({ return ( -
    {children}
    +
    {children}
    ); }; 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 3188bbe771e33..78c4d77da1918 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx @@ -31,7 +31,7 @@ const defaultProps = { onRearrange: jest.fn(), restoreFilter: jest.fn(), currentFilterId: 'NATIVE_FILTER-1', - filterGroups: [['NATIVE_FILTER-2', 'NATIVE_FILTER-1'], ['NATIVE_FILTER-3']], + filters: ['NATIVE_FILTER-1', 'NATIVE_FILTER-2', 'NATIVE_FILTER-3'], removedFilters: {}, erroredFilters: [], }; @@ -94,7 +94,7 @@ test('remove filter', async () => { }), ); }); - expect(defaultProps.onRemove).toHaveBeenCalledWith('NATIVE_FILTER-2'); + expect(defaultProps.onRemove).toHaveBeenCalledWith('NATIVE_FILTER-1'); }); test('add filter', async () => { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx index 744bcf8fe4a84..a65e167fdf39b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx @@ -31,7 +31,7 @@ interface Props { erroredFilters: string[]; restoreFilter: (id: string) => void; currentFilterId: string; - filterGroups: string[][]; + filters: string[]; removedFilters: Record; } @@ -60,16 +60,16 @@ const FiltureConfigurePane: React.FC = ({ erroredFilters, children, currentFilterId, - filterGroups, + filters, removedFilters, }) => { - const active = filterGroups.flat().filter(id => id === currentFilterId)[0]; + const active = filters.filter(id => id === currentFilterId)[0]; return ( = ({ /> - {filterGroups.flat().map(id => ( + {filters.map(id => (
    ` display: flex; + align-items: center; padding: ${theme.gridUnit * 2}px; width: 100%; border-radius: ${theme.borderRadius}px; @@ -72,7 +73,7 @@ interface Props { onRemove: (id: string) => void; restoreFilter: (id: string) => void; onRearrage: (dragIndex: number, targetIndex: number) => void; - filterGroups: string[][]; + filters: string[]; erroredFilters: string[]; } @@ -84,7 +85,7 @@ const FilterTitleContainer: React.FC = ({ onRearrage, currentFilterId, removedFilters, - filterGroups, + filters, erroredFilters = [], }) => { const renderComponent = (id: string) => { @@ -127,7 +128,7 @@ const FilterTitleContainer: React.FC = ({ )}
    -
    +
    {isRemoved ? null : ( { @@ -174,15 +175,15 @@ const FilterTitleContainer: React.FC = ({ const renderFilterGroups = () => { const items: React.ReactNode[] = []; - filterGroups.forEach((item, index) => { + filters.forEach((item, index) => { items.push( - {item.map(filter => renderComponent(filter))} + {renderComponent(item)} , ); }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx index d9981cbe6f04c..0a87035936a03 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx @@ -32,7 +32,7 @@ interface Props { onAdd: (type: NativeFilterType) => void; removedFilters: Record; currentFilterId: string; - filterGroups: string[][]; + filters: string[]; erroredFilters: string[]; } @@ -60,7 +60,7 @@ const FilterTitlePane: React.FC = ({ onRearrage, restoreFilter, currentFilterId, - filterGroups, + filters, removedFilters, erroredFilters, }) => { @@ -104,7 +104,7 @@ const FilterTitlePane: React.FC = ({ }} > ` } `; +const ChildrenContainer = styled.div` + margin-left: ${({ theme }) => theme.gridUnit * 6}px; +`; + const CollapsibleControl = (props: CollapsibleControlProps) => { const { checked, @@ -91,7 +95,7 @@ const CollapsibleControl = (props: CollapsibleControlProps) => { )} - {isChecked && children} + {isChecked && {children}} ); }; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DependencyList.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DependencyList.tsx new file mode 100644 index 0000000000000..3ba2765a30c93 --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DependencyList.tsx @@ -0,0 +1,213 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React, { useState } from 'react'; +import { styled, t } from '@superset-ui/core'; +import Icons from 'src/components/Icons'; +import { Select } from 'src/components'; +import { CollapsibleControl } from './CollapsibleControl'; + +interface DependencyListProps { + availableFilters: { label: string; value: string }[]; + dependencies: string[]; + onDependenciesChange: (dependencies: string[]) => void; + getDependencySuggestion: () => string; +} + +const MainPanel = styled.div` + display: flex; + flex-direction: column; +`; + +const AddFilter = styled.div` + ${({ theme }) => ` + display: inline-flex; + flex-direction: row; + align-items: center; + cursor: pointer; + color: ${theme.colors.primary.base}; + &:hover { + color: ${theme.colors.primary.dark1}; + } + `} +`; + +const DeleteFilter = styled(Icons.Trash)` + ${({ theme }) => ` + cursor: pointer; + margin-left: ${theme.gridUnit * 2}px; + color: ${theme.colors.grayscale.base}; + &:hover { + color: ${theme.colors.grayscale.dark1}; + } + `} +`; + +const RowPanel = styled.div` + ${({ theme }) => ` + display: flex; + width: 220px; + flex-direction: row; + align-items: center; + margin-bottom: ${theme.gridUnit}px; + `} +`; + +const Label = styled.div` + text-transform: uppercase; + font-size: ${({ theme }) => theme.typography.sizes.s}px; + color: ${({ theme }) => theme.colors.grayscale.base}; + margin-bottom: ${({ theme }) => theme.gridUnit}px; +`; + +const Row = ({ + availableFilters, + selection, + onChange, + onDelete, +}: { + availableFilters: { label: string; value: string }[]; + selection: string; + onChange: (id: string, value: string) => void; + onDelete: (id: string) => void; +}) => { + let value = availableFilters.find(e => e.value === selection); + let options = availableFilters; + if (!value) { + value = { label: t('(deleted or invalid type)'), value: selection }; + options = [value, ...options]; + } + return ( + + - ); - }; + const availableFilters = getAvailableFilters(filterId); + const hasAvailableFilters = availableFilters.length > 0; useEffect(() => { if (datasetId) { @@ -862,51 +825,27 @@ const FiltersConfigForm = ( header={FilterPanels.configuration.name} key={`${filterId}-${FilterPanels.configuration.key}`} > - {isCascadingFilter && ( - - { + {canDependOnOtherFilters && hasAvailableFilters && ( + + { + setNativeFilterFieldValues(form, filterId, { + dependencies, + }); + forceUpdate(); + validateDependencies(); formChanged(); - // execute after render - setTimeout(() => { - if (checked) { - form.validateFields([ - ['filters', filterId, 'parentFilter'], - ]); - } else { - setNativeFilterFieldValues(form, filterId, { - parentFilter: undefined, - }); - } - onFilterHierarchyChange( - filterId, - checked - ? form.getFieldValue('filters')[filterId].parentFilter - : undefined, - ); - }, 0); }} - > - {t('Parent filter')}} - initialValue={parentFilter} - normalize={value => (value ? { value } : undefined)} - data-test="parent-filter-input" - required - rules={[ - { - required: true, - message: t('Parent filter is required'), - }, - ]} - > - - - - + getDependencySuggestion={() => + getDependencySuggestion(filterId) + } + /> + )} {hasDataset && hasAdditionalFilters && ( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx index 259a9c3de6364..80126fe6a9256 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx @@ -124,7 +124,7 @@ const FILTER_SETTINGS_REGEX = /^filter settings$/i; const DEFAULT_VALUE_REGEX = /^filter has default value$/i; const MULTIPLE_REGEX = /^can select multiple values$/i; const REQUIRED_REGEX = /^filter value is required$/i; -const HIERARCHICAL_REGEX = /^filter is hierarchical$/i; +const DEPENDENCIES_REGEX = /^values are dependent on other filters$/i; const FIRST_VALUE_REGEX = /^select first filter value by default$/i; const INVERSE_SELECTION_REGEX = /^inverse selection$/i; const SEARCH_ALL_REGEX = /^dynamically search all filter values$/i; @@ -134,7 +134,6 @@ const SAVE_REGEX = /^save$/i; const NAME_REQUIRED_REGEX = /^name is required$/i; const COLUMN_REQUIRED_REGEX = /^column is required$/i; const DEFAULT_VALUE_REQUIRED_REGEX = /^default value is required$/i; -const PARENT_REQUIRED_REGEX = /^parent filter is required$/i; const PRE_FILTER_REQUIRED_REGEX = /^pre-filter is required$/i; const FILL_REQUIRED_FIELDS_REGEX = /fill all required fields to enable/; const TIME_RANGE_PREFILTER_REGEX = /^time range$/i; @@ -178,7 +177,7 @@ test('renders a value filter type', () => { expect(getCheckbox(DEFAULT_VALUE_REGEX)).not.toBeChecked(); expect(getCheckbox(REQUIRED_REGEX)).not.toBeChecked(); - expect(getCheckbox(HIERARCHICAL_REGEX)).not.toBeChecked(); + expect(queryCheckbox(DEPENDENCIES_REGEX)).not.toBeInTheDocument(); expect(getCheckbox(FIRST_VALUE_REGEX)).not.toBeChecked(); expect(getCheckbox(INVERSE_SELECTION_REGEX)).not.toBeChecked(); expect(getCheckbox(SEARCH_ALL_REGEX)).not.toBeChecked(); @@ -207,7 +206,7 @@ test('renders a numerical range filter type', async () => { expect(getCheckbox(PRE_FILTER_REGEX)).not.toBeChecked(); expect(queryCheckbox(MULTIPLE_REGEX)).not.toBeInTheDocument(); - expect(queryCheckbox(HIERARCHICAL_REGEX)).not.toBeInTheDocument(); + expect(queryCheckbox(DEPENDENCIES_REGEX)).not.toBeInTheDocument(); expect(queryCheckbox(FIRST_VALUE_REGEX)).not.toBeInTheDocument(); expect(queryCheckbox(INVERSE_SELECTION_REGEX)).not.toBeInTheDocument(); expect(queryCheckbox(SEARCH_ALL_REGEX)).not.toBeInTheDocument(); @@ -302,13 +301,6 @@ test.skip('validates the default value', async () => { ).toBeInTheDocument(); }); -test('validates the hierarchical value', async () => { - defaultRender(); - userEvent.click(screen.getByText(FILTER_SETTINGS_REGEX)); - userEvent.click(getCheckbox(HIERARCHICAL_REGEX)); - expect(await screen.findByText(PARENT_REQUIRED_REGEX)).toBeInTheDocument(); -}); - test('validates the pre-filter value', async () => { defaultRender(); userEvent.click(screen.getByText(FILTER_SETTINGS_REGEX)); @@ -335,7 +327,7 @@ test.skip("doesn't render time range pre-filter if there are no temporal columns ); }); -test('filter title groups are draggable', async () => { +test('filters are draggable', async () => { const nativeFilterState = [ buildNativeFilter('NATIVE_FILTER-1', 'state', ['NATIVE_FILTER-2']), buildNativeFilter('NATIVE_FILTER-2', 'country', []), @@ -350,7 +342,7 @@ test('filter title groups are draggable', async () => { }; defaultRender(state, { ...props, createNewOnOpen: false }); const draggables = document.querySelectorAll('div[draggable=true]'); - expect(draggables.length).toBe(2); + expect(draggables.length).toBe(3); }); /* diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx index d288bba05e0a8..67d249c793b29 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx @@ -44,15 +44,15 @@ import FiltersConfigForm, { } from './FiltersConfigForm/FiltersConfigForm'; import Footer from './Footer/Footer'; import { useOpenModal, useRemoveCurrentFilter } from './state'; -import { FilterRemoval, NativeFiltersForm, FilterHierarchy } from './types'; +import { FilterRemoval, NativeFiltersForm } from './types'; import { createHandleSave, createHandleRemoveItem, generateFilterId, getFilterIds, - buildFilterGroup, validateForm, NATIVE_FILTER_DIVIDER_PREFIX, + hasCircularDependency, } from './utils'; import DividerConfigForm from './DividerConfigForm'; @@ -89,7 +89,7 @@ export interface FiltersConfigModalProps { onSave: (filterConfig: FilterConfiguration) => Promise; onCancel: () => void; } -export const CASCADING_FILTERS = ['filter_select']; +export const ALLOW_DEPENDENCIES = ['filter_select']; /** * This is the modal to configure all the dashboard-native filters. @@ -159,22 +159,11 @@ export function FiltersConfigModal({ if (removal?.isPending) clearTimeout(removal.timerId); setRemovedFilters(current => ({ ...current, [id]: null })); }; - const getInitialFilterHierarchy = () => - filterConfig.map(filter => ({ - id: filter.id, - parentId: - filter.type === NativeFilterType.NATIVE_FILTER - ? filter.cascadeParentIds[0] || null - : null, - })); - - const [filterHierarchy, setFilterHierarchy] = useState(() => - getInitialFilterHierarchy(), - ); + const getInitialFilterOrder = () => Object.keys(filterConfigMap); // State for tracking the re-ordering of filters - const [orderedFilters, setOrderedFilters] = useState(() => - buildFilterGroup(filterHierarchy), + const [orderedFilters, setOrderedFilters] = useState( + getInitialFilterOrder(), ); const getActiveFilterPanelKey = (filterId: string) => [ @@ -198,18 +187,13 @@ export function FiltersConfigModal({ setNewFilterIds([...newFilterIds, newFilterId]); setCurrentFilterId(newFilterId); setSaveAlertVisible(false); - setFilterHierarchy(previousState => [ - ...previousState, - { id: newFilterId, parentId: null }, - ]); - setOrderedFilters([...orderedFilters, [newFilterId]]); + setOrderedFilters([...orderedFilters, newFilterId]); setActiveFilterPanelKey(getActiveFilterPanelKey(newFilterId)); }, [ newFilterIds, orderedFilters, setCurrentFilterId, - setFilterHierarchy, setOrderedFilters, setNewFilterIds, ], @@ -226,10 +210,8 @@ export function FiltersConfigModal({ const handleRemoveItem = createHandleRemoveItem( setRemovedFilters, - setSaveAlertVisible, setOrderedFilters, - setFilterHierarchy, - filterHierarchy, + setSaveAlertVisible, ); // After this, it should be as if the modal was just opened fresh. @@ -245,41 +227,54 @@ export function FiltersConfigModal({ setActiveFilterPanelKey(getActiveFilterPanelKey(filterIds[0])); } if (!isSaving) { - const initialFilterHierarchy = getInitialFilterHierarchy(); - setFilterHierarchy(initialFilterHierarchy); - setOrderedFilters(buildFilterGroup(initialFilterHierarchy)); - form.resetFields(['filters']); + setOrderedFilters(getInitialFilterOrder()); } + form.resetFields(['filters']); form.setFieldsValue({ changed: false }); }; - const getFilterTitle = (id: string) => { - const formValue = formValues.filters[id]; - const config = filterConfigMap[id]; - return ( - (formValue && 'name' in formValue && formValue.name) || - (formValue && 'title' in formValue && formValue.title) || - (config && 'name' in config && config.name) || - (config && 'title' in config && config.title) || - '[untitled]' - ); - }; - const getParentFilters = (id: string) => - filterIds - .filter(filterId => filterId !== id && !removedFilters[filterId]) - .filter(filterId => { - const component = - formValues.filters[filterId] || filterConfigMap[filterId]; - return ( - component && - 'filterType' in component && - CASCADING_FILTERS.includes(component.filterType) - ); - }) - .map(id => ({ - id, - title: getFilterTitle(id), - })); + const getFilterTitle = useCallback( + (id: string) => { + const formValue = formValues.filters[id]; + const config = filterConfigMap[id]; + return ( + (formValue && 'name' in formValue && formValue.name) || + (formValue && 'title' in formValue && formValue.title) || + (config && 'name' in config && config.name) || + (config && 'title' in config && config.title) || + t('[untitled]') + ); + }, + [filterConfigMap, formValues.filters], + ); + + const canBeUsedAsDependency = useCallback( + (filterId: string) => { + if (removedFilters[filterId]) { + return false; + } + const component = + form.getFieldValue('filters')?.[filterId] || filterConfigMap[filterId]; + return ( + component && + 'filterType' in component && + ALLOW_DEPENDENCIES.includes(component.filterType) + ); + }, + [filterConfigMap, form, removedFilters], + ); + + const getAvailableFilters = useCallback( + (filterId: string) => + filterIds + .filter(key => key !== filterId) + .filter(filterId => canBeUsedAsDependency(filterId)) + .map(key => ({ + label: getFilterTitle(key), + value: key, + })), + [canBeUsedAsDependency, filterIds, getFilterTitle], + ); const cleanDeletedParents = (values: NativeFiltersForm | null) => { Object.keys(filterConfigMap).forEach(key => { @@ -287,9 +282,11 @@ export function FiltersConfigModal({ if (!('cascadeParentIds' in filter)) { return; } - const parentId = filter.cascadeParentIds?.[0]; - if (parentId && removedFilters[parentId]) { - filter.cascadeParentIds = []; + const { cascadeParentIds } = filter; + if (cascadeParentIds) { + filter.cascadeParentIds = cascadeParentIds.filter(id => + canBeUsedAsDependency(id), + ); } }); @@ -297,12 +294,14 @@ export function FiltersConfigModal({ if (filters) { Object.keys(filters).forEach(key => { const filter = filters[key]; - if (!('parentFilter' in filter)) { + if (!('dependencies' in filter)) { return; } - const parentId = filter.parentFilter?.value; - if (parentId && removedFilters[parentId]) { - filter.parentFilter = undefined; + const { dependencies } = filter; + if (dependencies) { + filter.dependencies = dependencies.filter(id => + canBeUsedAsDependency(id), + ); } }); } @@ -338,9 +337,6 @@ export function FiltersConfigModal({ const values: NativeFiltersForm | null = await validateForm( form, currentFilterId, - filterConfigMap, - filterIds, - removedFilters, setCurrentFilterId, ); @@ -350,7 +346,7 @@ export function FiltersConfigModal({ cleanDeletedParents(values); createHandleSave( filterConfigMap, - orderedFilters.flat(), + orderedFilters, removedFilters, onSave, values, @@ -368,10 +364,10 @@ export function FiltersConfigModal({ const handleCancel = () => { const changed = form.getFieldValue('changed'); - const initialOrder = buildFilterGroup(getInitialFilterHierarchy()).flat(); + const initialOrder = getInitialFilterOrder(); const didChangeOrder = - orderedFilters.flat().length !== initialOrder.length || - orderedFilters.flat().some((val, index) => val !== initialOrder[index]); + orderedFilters.length !== initialOrder.length || + orderedFilters.some((val, index) => val !== initialOrder[index]); if ( unsavedFiltersIds.length > 0 || form.isFieldsTouched() || @@ -384,24 +380,62 @@ export function FiltersConfigModal({ } }; const onRearrage = (dragIndex: number, targetIndex: number) => { - const newOrderedFilter = orderedFilters.map(group => [...group]); + const newOrderedFilter = [...orderedFilters]; const removed = newOrderedFilter.splice(dragIndex, 1)[0]; newOrderedFilter.splice(targetIndex, 0, removed); setOrderedFilters(newOrderedFilter); }; - const handleFilterHierarchyChange = useCallback( - (filterId: string, parentFilter?: { value: string; label: string }) => { - const index = filterHierarchy.findIndex(item => item.id === filterId); - const newState = [...filterHierarchy]; - newState.splice(index, 1, { - id: filterId, - parentId: parentFilter ? parentFilter.value : null, + + const buildDependencyMap = useCallback(() => { + const dependencyMap = new Map(); + const filters = form.getFieldValue('filters'); + if (filters) { + Object.keys(filters).forEach(key => { + const formItem = filters[key]; + const configItem = filterConfigMap[key]; + let array: string[] = []; + if (formItem && 'dependencies' in formItem) { + array = [...formItem.dependencies]; + } else if (configItem && configItem.cascadeParentIds) { + array = [...configItem.cascadeParentIds]; + } + dependencyMap.set(key, array); }); - setFilterHierarchy(newState); - setOrderedFilters(buildFilterGroup(newState)); - }, - [setFilterHierarchy, setOrderedFilters, filterHierarchy], - ); + } + return dependencyMap; + }, [filterConfigMap, form]); + + const validateDependencies = () => { + const dependencyMap = buildDependencyMap(); + filterIds + .filter(id => !removedFilters[id]) + .forEach(filterId => { + const result = hasCircularDependency(dependencyMap, filterId); + const field = { + name: ['filters', filterId, 'dependencies'], + errors: result ? [t('Cyclic dependency detected')] : [], + }; + form.setFields([field]); + }); + handleErroredFilters(); + }; + + const getDependencySuggestion = (filterId: string) => { + const dependencyMap = buildDependencyMap(); + const possibleDependencies = orderedFilters.filter( + key => key !== filterId && canBeUsedAsDependency(key), + ); + const found = possibleDependencies.find(filter => { + const dependencies = dependencyMap.get(filterId) || []; + dependencies.push(filter); + if (hasCircularDependency(dependencyMap, filterId)) { + dependencies.pop(); + return false; + } + return true; + }); + return found || possibleDependencies[0]; + }; const onValuesChange = useMemo( () => @@ -420,23 +454,10 @@ export function FiltersConfigModal({ // we only need to set this if a name/title changed setFormValues(values); } - const changedFilterHierarchies = Object.keys(changes.filters) - .filter(key => changes.filters[key].parentFilter) - .map(key => ({ - id: key, - parentFilter: changes.filters[key].parentFilter, - })); - if (changedFilterHierarchies.length > 0) { - const changedFilterId = changedFilterHierarchies[0]; - handleFilterHierarchyChange( - changedFilterId.id, - changedFilterId.parentFilter, - ); - } setSaveAlertVisible(false); handleErroredFilters(); }, SLOW_DEBOUNCE), - [handleFilterHierarchyChange, handleErroredFilters], + [handleErroredFilters], ); useEffect(() => { @@ -444,6 +465,7 @@ export function FiltersConfigModal({ prevErroredFilters.filter(f => !removedFilters[f]), ); }, [removedFilters]); + const getForm = (id: string) => { const isDivider = id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX); return isDivider ? ( @@ -459,13 +481,14 @@ export function FiltersConfigModal({ filterToEdit={filterConfigMap[id] as Filter} removedFilters={removedFilters} restoreFilter={restoreFilter} - parentFilters={getParentFilters(id)} - onFilterHierarchyChange={handleFilterHierarchyChange} + getAvailableFilters={getAvailableFilters} key={id} activeFilterPanelKeys={activeFilterPanelKey} handleActiveFilterPanelChange={key => setActiveFilterPanelKey(key)} isActive={currentFilterId === id} setErroredFilters={setErroredFilters} + validateDependencies={validateDependencies} + getDependencySuggestion={getDependencySuggestion} /> ); }; @@ -509,7 +532,7 @@ export function FiltersConfigModal({ removedFilters={removedFilters} restoreFilter={restoreFilter} onRearrange={onRearrage} - filterGroups={orderedFilters} + filters={orderedFilters} > {(id: string) => getForm(id)} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/state.ts index 285c30e5c03bc..e9b9ec17c6730 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/state.ts @@ -25,7 +25,7 @@ import { FilterRemoval } from './types'; export const useRemoveCurrentFilter = ( removedFilters: Record, currentFilterId: string, - orderedFilters: string[][], + orderedFilters: string[], setCurrentFilterId: Function, ) => { useEffect(() => { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts index 42daba882214c..ea4916b53f189 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts @@ -40,17 +40,13 @@ export interface NativeFiltersFormItem { }; defaultValue: any; defaultDataMask: DataMask; - parentFilter?: { - value: string; - label: string; - }; + dependencies?: string[]; sortMetric: string | null; adhoc_filters?: AdhocFilter[]; time_range?: string; granularity_sqla?: string; type: typeof NativeFilterType.NATIVE_FILTER; description: string; - hierarchicalFilter?: boolean; } export interface NativeFilterDivider { id: string; @@ -71,6 +67,3 @@ export type FilterRemoval = timerId: number; // id of the timer that finally removes the filter } | { isPending: false }; - -export type FilterHierarchyNode = { id: string; parentId: string | null }; -export type FilterHierarchy = FilterHierarchyNode[]; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts index b6edd9fd024aa..5b434bb7c902a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts @@ -19,46 +19,41 @@ import { FormInstance } from 'antd/lib/form'; import shortid from 'shortid'; import { getInitialDataMask } from 'src/dataMask/reducer'; - import { Filter, FilterConfiguration, NativeFilterType, Divider, - t, NativeFilterTarget, + logging, } from '@superset-ui/core'; import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants'; -import { - FilterRemoval, - NativeFiltersForm, - FilterHierarchy, - FilterHierarchyNode, -} from './types'; +import { FilterRemoval, NativeFiltersForm } from './types'; export const REMOVAL_DELAY_SECS = 5; +export const hasCircularDependency = ( + dependencyMap: Map, + filterId: string, + trace: string[] = [], +): boolean => { + if (trace.includes(filterId)) { + return true; + } + const dependencies = dependencyMap.get(filterId); + if (dependencies) { + return dependencies.some(dependency => + hasCircularDependency(dependencyMap, dependency, [...trace, filterId]), + ); + } + return false; +}; + export const validateForm = async ( form: FormInstance, currentFilterId: string, - filterConfigMap: Record, - filterIds: string[], - removedFilters: Record, setCurrentFilterId: Function, ) => { - const addValidationError = ( - filterId: string, - field: string, - error: string, - ) => { - const fieldError = { - name: ['filters', filterId, field], - errors: [error], - }; - form.setFields([fieldError]); - setCurrentFilterId(filterId); - }; - try { let formValues: NativeFiltersForm; try { @@ -71,42 +66,9 @@ export const validateForm = async ( throw error; } } - - const validateCycles = ( - filterId: string, - trace: string[] = [], - ): boolean => { - if (trace.includes(filterId)) { - addValidationError( - filterId, - 'parentFilter', - t('Cannot create cyclic hierarchy'), - ); - return false; - } - const formItem = formValues.filters?.[filterId]; - const configItem = filterConfigMap[filterId]; - const parentId = formItem - ? 'parentFilter' in formItem && formItem.parentFilter?.value - : 'cascadeParentIds' in configItem && configItem?.cascadeParentIds?.[0]; - - if (parentId) { - return validateCycles(parentId, [...trace, filterId]); - } - return true; - }; - - const invalid = filterIds - .filter(id => !removedFilters[id]) - .some(filterId => !validateCycles(filterId)); - - if (invalid) { - return null; - } - return formValues; } catch (error) { - console.warn('Filter configuration failed:', error); + logging.warn('Filter configuration failed:', error); if (!error.errorFields || !error.errorFields.length) return null; // not a validation error @@ -177,9 +139,7 @@ export const createHandleSave = // for now there will only ever be one target targets: [target], defaultDataMask: formInputs.defaultDataMask ?? getInitialDataMask(), - cascadeParentIds: formInputs.parentFilter - ? [formInputs.parentFilter.value] - : [], + cascadeParentIds: formInputs.dependencies || [], scope: formInputs.scope, sortMetric: formInputs.sortMetric, type: formInputs.type, @@ -189,49 +149,7 @@ export const createHandleSave = await saveForm(newFilterConfig); }; -export function buildFilterGroup(nodes: FilterHierarchyNode[]) { - const buildGroup = ( - elementId: string, - nodeList: FilterHierarchyNode[], - found: string[], - ): string[] | null => { - const element = nodeList.find(el => el.id === elementId); - const didFind = found.includes(elementId); - let parent: string[] = []; - let children: string[] = []; - - if (!element || didFind) { - return null; - } - found.push(elementId); - const { parentId } = element; - - if (parentId) { - const parentArray = buildGroup(parentId, nodeList, found); - parent = parentArray ? parentArray.flat() : []; - } - const childrenArray = nodeList - .filter(el => el.parentId === elementId) - .map(el => buildGroup(el.id, nodeList, found)); - if (childrenArray) { - children = childrenArray - ? (childrenArray.flat().filter(id => id) as string[]) - : []; - } - return [...parent, elementId, ...children]; - }; - const rendered: string[] = []; - const group: string[][] = []; - for (let index = 0; index < nodes.length; index += 1) { - const element = nodes[index]; - const subGroup = buildGroup(element.id, nodes, rendered); - if (subGroup) { - group.push(subGroup); - } - } - return group; -} export const createHandleRemoveItem = ( setRemovedFilters: ( @@ -241,28 +159,13 @@ export const createHandleRemoveItem = ) => Record) | Record, ) => void, - setSaveAlertVisible: Function, setOrderedFilters: ( - val: string[][] | ((prevState: string[][]) => string[][]), + val: string[] | ((prevState: string[]) => string[]), ) => void, - setFilterHierarchy: ( - state: - | FilterHierarchy - | ((prevState: FilterHierarchy) => FilterHierarchy), - ) => void, - filterHierarchy: FilterHierarchy, + setSaveAlertVisible: Function, ) => (filterId: string) => { const completeFilterRemoval = (filterId: string) => { - const buildNewFilterHierarchy = (hierarchy: FilterHierarchy) => - hierarchy - .filter(nativeFilter => nativeFilter.id !== filterId) - .map(nativeFilter => { - const didRemoveParent = nativeFilter.parentId === filterId; - return didRemoveParent - ? { ...nativeFilter, parentId: null } - : nativeFilter; - }); // the filter state will actually stick around in the form, // and the filterConfig/newFilterIds, but we use removedFilters // to mark it as removed. @@ -270,32 +173,9 @@ export const createHandleRemoveItem = ...removedFilters, [filterId]: { isPending: false }, })); - // Remove the filter from the side tab and de-associate children - // in case we removed a parent. - setFilterHierarchy(prevFilterHierarchy => - buildNewFilterHierarchy(prevFilterHierarchy), + setOrderedFilters((orderedFilters: string[]) => + orderedFilters.filter(filter => filter !== filterId), ); - setOrderedFilters((orderedFilters: string[][]) => { - const newOrder = []; - for (let index = 0; index < orderedFilters.length; index += 1) { - const doesGroupContainDeletedFilter = - orderedFilters[index].findIndex(id => id === filterId) >= 0; - // Rebuild just the group that contains deleted filter ID. - if (doesGroupContainDeletedFilter) { - const newGroups = buildFilterGroup( - buildNewFilterHierarchy( - filterHierarchy.filter(filter => - orderedFilters[index].includes(filter.id), - ), - ), - ); - newGroups.forEach(group => newOrder.push(group)); - } else { - newOrder.push(orderedFilters[index]); - } - } - return newOrder; - }); }; // first set up the timer to completely remove it diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.ts index be2089c6be08a..030b65859b3ea 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.ts @@ -21,12 +21,11 @@ import { useMemo } from 'react'; import { Filter, FilterConfiguration, - NativeFilterType, Divider, + isFilterDivider, } from '@superset-ui/core'; import { ActiveTabs, DashboardLayout, RootState } from '../../types'; import { TAB_TYPE } from '../../util/componentTypes'; -import { CascadeFilter } from './FilterBar/CascadeFilters/types'; const defaultFilterConfiguration: Filter[] = []; @@ -98,37 +97,31 @@ function useIsFilterInScope() { // Chart is visible if it's placed in an active tab tree or if it's not attached to any tab. // Chart is in an active tab tree if all of it's ancestors of type TAB are active // Dividers are always in scope - return (filter: CascadeFilter | Divider) => { - const isDivider = filter.type === NativeFilterType.DIVIDER; - return ( - isDivider || - ('chartsInScope' in filter && - filter.chartsInScope?.some((chartId: number) => { - const tabParents = selectChartTabParents(chartId); - return ( - tabParents?.length === 0 || - tabParents?.every(tab => activeTabs.includes(tab)) - ); - })) - ); - }; + return (filter: Filter | Divider) => + isFilterDivider(filter) || + ('chartsInScope' in filter && + filter.chartsInScope?.some((chartId: number) => { + const tabParents = selectChartTabParents(chartId); + return ( + tabParents?.length === 0 || + tabParents?.every(tab => activeTabs.includes(tab)) + ); + })); } -export function useSelectFiltersInScope( - cascadeFilters: (CascadeFilter | Divider)[], -) { +export function useSelectFiltersInScope(filters: (Filter | Divider)[]) { const dashboardHasTabs = useDashboardHasTabs(); const isFilterInScope = useIsFilterInScope(); return useMemo(() => { - let filtersInScope: (CascadeFilter | Divider)[] = []; - const filtersOutOfScope: (CascadeFilter | Divider)[] = []; + let filtersInScope: (Filter | Divider)[] = []; + const filtersOutOfScope: (Filter | Divider)[] = []; // we check native filters scopes only on dashboards with tabs if (!dashboardHasTabs) { - filtersInScope = cascadeFilters; + filtersInScope = filters; } else { - cascadeFilters.forEach(filter => { + filters.forEach(filter => { const filterInScope = isFilterInScope(filter); if (filterInScope) { @@ -139,5 +132,5 @@ export function useSelectFiltersInScope( }); } return [filtersInScope, filtersOutOfScope]; - }, [cascadeFilters, dashboardHasTabs, isFilterInScope]); + }, [filters, dashboardHasTabs, isFilterInScope]); } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts index 1ebba4619e35e..9d71fb6fbb72a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts @@ -29,7 +29,6 @@ import { QueryFormData, } from '@superset-ui/core'; import { Charts, DashboardLayout } from 'src/dashboard/types'; -import { RefObject } from 'react'; import extractUrlParams from 'src/dashboard/util/extractUrlParams'; import { isFeatureEnabled } from 'src/featureFlags'; import { CHART_TYPE, TAB_TYPE } from '../../util/componentTypes'; @@ -37,9 +36,8 @@ import { DASHBOARD_GRID_ID, DASHBOARD_ROOT_ID } from '../../util/constants'; export const getFormData = ({ datasetId, - cascadingFilters = {}, + dependencies = {}, groupby, - inputRef, defaultDataMask, controlValues, filterType, @@ -50,8 +48,7 @@ export const getFormData = ({ type, }: Partial & { datasetId?: number; - inputRef?: RefObject; - cascadingFilters?: object; + dependencies?: object; groupby?: string; adhoc_filters?: AdhocFilter[]; time_range?: string; @@ -75,7 +72,7 @@ export const getFormData = ({ ...otherProps, adhoc_filters: adhoc_filters ?? [], extra_filters: [], - extra_form_data: cascadingFilters, + extra_form_data: dependencies, granularity_sqla, metrics: ['count'], row_limit: 1000, @@ -86,7 +83,6 @@ export const getFormData = ({ url_params: extractUrlParams('regular'), inView: true, viz_type: filterType, - inputRef, type, }; }; diff --git a/superset-frontend/src/dashboard/styles.ts b/superset-frontend/src/dashboard/styles.ts index 3df91ec7aa419..5947fd6f26145 100644 --- a/superset-frontend/src/dashboard/styles.ts +++ b/superset-frontend/src/dashboard/styles.ts @@ -24,6 +24,9 @@ export const filterCardPopoverStyle = (theme: SupersetTheme) => css` padding: 0; border-radius: 4px; + .ant-popover-inner { + box-shadow: 0 0 8px rgb(0 0 0 / 10%); + } .ant-popover-inner-content { padding: ${theme.gridUnit * 4}px; } diff --git a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx index 80f287295989f..47916beed7a14 100644 --- a/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx +++ b/superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx @@ -46,6 +46,7 @@ import { DEFAULT_TIME_RANGE } from 'src/explore/constants'; import { useDebouncedEffect } from 'src/explore/exploreUtils'; import { SLOW_DEBOUNCE } from 'src/constants'; import { testWithId } from 'src/utils/testUtils'; +import { noOp } from 'src/utils/common'; import { FrameType } from './types'; import { @@ -173,6 +174,8 @@ interface DateFilterControlProps { value?: string; endpoints?: TimeRangeEndpoints; type?: Type; + onOpenPopover?: () => void; + onClosePopover?: () => void; } export const DATE_FILTER_CONTROL_TEST_ID = 'date-filter-control'; @@ -181,7 +184,14 @@ export const getDateFilterControlTestId = testWithId( ); export default function DateFilterLabel(props: DateFilterControlProps) { - const { value = DEFAULT_TIME_RANGE, endpoints, onChange, type } = props; + const { + value = DEFAULT_TIME_RANGE, + endpoints, + onChange, + type, + onOpenPopover = noOp, + onClosePopover = noOp, + } = props; const [actualTimeRange, setActualTimeRange] = useState(value); const [show, setShow] = useState(false); @@ -273,8 +283,10 @@ export default function DateFilterLabel(props: DateFilterControlProps) { const togglePopover = () => { if (show) { onHide(); + onClosePopover(); } else { - setShow(true); + onOpen(); + onOpenPopover(); } }; @@ -372,11 +384,7 @@ export default function DateFilterLabel(props: DateFilterControlProps) { overlayStyle={overlayStyle} > - diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx index e86ed587c8e40..828dd36d5fa8e 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx @@ -18,7 +18,7 @@ */ import React, { useEffect, useState } from 'react'; import { Select } from 'src/components'; -import { t, SupersetClient, styled } from '@superset-ui/core'; +import { t, SupersetClient, SupersetTheme, styled } from '@superset-ui/core'; import { Operators, OPERATORS_OPTIONS, @@ -373,7 +373,7 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC = props => { return ( <> ({ marginBottom: theme.gridUnit * 4 })} + css={(theme: SupersetTheme) => ({ marginBottom: theme.gridUnit * 4 })} options={(props.operators ?? OPERATORS_OPTIONS) .filter(op => isOperatorRelevant(op, subject)) .map((option, index) => ({ diff --git a/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx b/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx index e3c202c5df23c..9742e27e5908f 100644 --- a/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx @@ -38,9 +38,11 @@ export default function PluginFilterGroupBy(props: PluginFilterGroupByProps) { setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, filterState, + inputRef, } = props; - const { defaultValue, inputRef, multiSelect } = formData; + const { defaultValue, multiSelect } = formData; const [value, setValue] = useState(defaultValue ?? []); @@ -116,6 +118,7 @@ export default function PluginFilterGroupBy(props: PluginFilterGroupByProps) { onFocus={setFocusedFilter} ref={inputRef} options={options} + onDropdownVisibleChange={setFilterActive} /> diff --git a/superset-frontend/src/filters/components/GroupBy/transformProps.ts b/superset-frontend/src/filters/components/GroupBy/transformProps.ts index 0353feea1d100..9ca0ec047bfa8 100644 --- a/superset-frontend/src/filters/components/GroupBy/transformProps.ts +++ b/superset-frontend/src/filters/components/GroupBy/transformProps.ts @@ -17,6 +17,7 @@ * under the License. */ import { ChartProps } from '@superset-ui/core'; +import { noOp } from 'src/utils/common'; import { DEFAULT_FORM_DATA } from './types'; export default function transformProps(chartProps: ChartProps) { @@ -28,11 +29,13 @@ export default function transformProps(chartProps: ChartProps) { queriesData, width, filterState, + inputRef, } = chartProps; const { - setDataMask = () => {}, - setFocusedFilter = () => {}, - unsetFocusedFilter = () => {}, + setDataMask = noOp, + setFocusedFilter = noOp, + unsetFocusedFilter = noOp, + setFilterActive = noOp, } = hooks; const { data } = queriesData[0]; @@ -47,5 +50,7 @@ export default function transformProps(chartProps: ChartProps) { setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, + inputRef, }; } diff --git a/superset-frontend/src/filters/components/GroupBy/types.ts b/superset-frontend/src/filters/components/GroupBy/types.ts index fd50389784d8b..5775edaa2f332 100644 --- a/superset-frontend/src/filters/components/GroupBy/types.ts +++ b/superset-frontend/src/filters/components/GroupBy/types.ts @@ -40,6 +40,7 @@ export type PluginFilterGroupByProps = PluginFilterStylesProps & { data: DataRecord[]; filterState: FilterState; formData: PluginFilterGroupByQueryFormData; + inputRef: RefObject; } & PluginFilterHooks; export const DEFAULT_FORM_DATA: PluginFilterGroupByCustomizeProps = { diff --git a/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx b/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx index b768511baa31d..05b797a07ab47 100644 --- a/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx @@ -152,12 +152,14 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) { setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, filterState, + inputRef, } = props; const [row] = data; // @ts-ignore const { min, max }: { min: number; max: number } = row; - const { groupby, defaultValue, inputRef, enableSingleValue } = formData; + const { groupby, defaultValue, enableSingleValue } = formData; const enableSingleMinValue = enableSingleValue === SingleValueType.Minimum; const enableSingleMaxValue = enableSingleValue === SingleValueType.Maximum; @@ -289,6 +291,8 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) { onBlur={unsetFocusedFilter} onMouseEnter={setFocusedFilter} onMouseLeave={unsetFocusedFilter} + onMouseDown={() => setFilterActive(true)} + onMouseUp={() => setFilterActive(false)} > {enableSingleMaxValue && ( {}, - setFocusedFilter = () => {}, - unsetFocusedFilter = () => {}, + setDataMask = noOp, + setFocusedFilter = noOp, + unsetFocusedFilter = noOp, + setFilterActive = noOp, } = hooks; const { data } = queriesData[0]; @@ -45,5 +48,7 @@ export default function transformProps(chartProps: ChartProps) { width, setFocusedFilter, unsetFocusedFilter, + setFilterActive, + inputRef, }; } diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx index a34891e98cc97..16c748a174b32 100644 --- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx @@ -84,16 +84,17 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, appSection, showOverflow, parentRef, + inputRef, } = props; const { enableEmptyFilter, multiSelect, showSearch, inverseSelection, - inputRef, defaultToFirstItem, searchAllOptions, } = formData; @@ -323,6 +324,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { // @ts-ignore options={options} sortComparator={sortComparator} + onDropdownVisibleChange={setFilterActive} /> diff --git a/superset-frontend/src/filters/components/Select/transformProps.ts b/superset-frontend/src/filters/components/Select/transformProps.ts index 59e58d352ef00..59d9adc57e930 100644 --- a/superset-frontend/src/filters/components/Select/transformProps.ts +++ b/superset-frontend/src/filters/components/Select/transformProps.ts @@ -17,6 +17,7 @@ * under the License. */ import { GenericDataType } from '@superset-ui/core'; +import { noOp } from 'src/utils/common'; import { DEFAULT_FORM_DATA, PluginFilterSelectChartProps } from './types'; export default function transformProps( @@ -32,12 +33,14 @@ export default function transformProps( appSection, filterState, isRefreshing, + inputRef, } = chartProps; const newFormData = { ...DEFAULT_FORM_DATA, ...formData }; const { - setDataMask = () => {}, - setFocusedFilter = () => {}, - unsetFocusedFilter = () => {}, + setDataMask = noOp, + setFocusedFilter = noOp, + unsetFocusedFilter = noOp, + setFilterActive = noOp, } = hooks; const [queryData] = queriesData; const { colnames = [], coltypes = [], data = [] } = queryData || {}; @@ -59,5 +62,7 @@ export default function transformProps( setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, + inputRef, }; } diff --git a/superset-frontend/src/filters/components/Select/types.ts b/superset-frontend/src/filters/components/Select/types.ts index 82dba5b9296cc..2d4f0c958669f 100644 --- a/superset-frontend/src/filters/components/Select/types.ts +++ b/superset-frontend/src/filters/components/Select/types.ts @@ -37,7 +37,6 @@ export interface PluginFilterSelectCustomizeProps { inverseSelection: boolean; multiSelect: boolean; defaultToFirstItem: boolean; - inputRef?: RefObject; searchAllOptions: boolean; sortAscending?: boolean; sortMetric?: string; @@ -61,6 +60,7 @@ export type PluginFilterSelectProps = PluginFilterStylesProps & { isRefreshing: boolean; showOverflow: boolean; parentRef?: RefObject; + inputRef?: RefObject; } & PluginFilterHooks; export const DEFAULT_FORM_DATA: PluginFilterSelectCustomizeProps = { diff --git a/superset-frontend/src/filters/components/Time/TimeFilterPlugin.tsx b/superset-frontend/src/filters/components/Time/TimeFilterPlugin.tsx index 5dda91670168f..3d00f346635d3 100644 --- a/superset-frontend/src/filters/components/Time/TimeFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/Time/TimeFilterPlugin.tsx @@ -65,10 +65,11 @@ export default function TimeFilterPlugin(props: PluginFilterTimeProps) { setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, width, height, filterState, - formData: { inputRef }, + inputRef, } = props; const handleTimeRangeChange = useCallback( @@ -110,6 +111,8 @@ export default function TimeFilterPlugin(props: PluginFilterTimeProps) { name="time_range" onChange={handleTimeRangeChange} type={filterState.validateStatus} + onOpenPopover={() => setFilterActive(true)} + onClosePopover={() => setFilterActive(false)} /> diff --git a/superset-frontend/src/filters/components/Time/transformProps.ts b/superset-frontend/src/filters/components/Time/transformProps.ts index c2fe3c0d1c6b8..883b6002d8920 100644 --- a/superset-frontend/src/filters/components/Time/transformProps.ts +++ b/superset-frontend/src/filters/components/Time/transformProps.ts @@ -17,6 +17,7 @@ * under the License. */ import { ChartProps } from '@superset-ui/core'; +import { noOp } from 'src/utils/common'; import { DEFAULT_FORM_DATA } from './types'; export default function transformProps(chartProps: ChartProps) { @@ -28,11 +29,13 @@ export default function transformProps(chartProps: ChartProps) { width, behaviors, filterState, + inputRef, } = chartProps; const { - setDataMask = () => {}, - setFocusedFilter = () => {}, - unsetFocusedFilter = () => {}, + setDataMask = noOp, + setFocusedFilter = noOp, + unsetFocusedFilter = noOp, + setFilterActive = noOp, } = hooks; const { data } = queriesData[0]; @@ -48,6 +51,8 @@ export default function transformProps(chartProps: ChartProps) { setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, width, + inputRef, }; } diff --git a/superset-frontend/src/filters/components/Time/types.ts b/superset-frontend/src/filters/components/Time/types.ts index 6adab6b97e0f0..4219c7cbb351e 100644 --- a/superset-frontend/src/filters/components/Time/types.ts +++ b/superset-frontend/src/filters/components/Time/types.ts @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import { RefObject } from 'react'; import { Behavior, DataRecord, @@ -37,6 +38,7 @@ export type PluginFilterTimeProps = PluginFilterStylesProps & { data: DataRecord[]; formData: PluginFilterSelectQueryFormData; filterState: FilterState; + inputRef: RefObject; } & PluginFilterHooks; export const DEFAULT_FORM_DATA: PluginFilterTimeCustomizeProps = { diff --git a/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx b/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx index 0f616a6a709ba..fd5f21caad914 100644 --- a/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx @@ -40,9 +40,11 @@ export default function PluginFilterTimeColumn( setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, filterState, + inputRef, } = props; - const { defaultValue, inputRef } = formData; + const { defaultValue } = formData; const [value, setValue] = useState(defaultValue ?? []); @@ -116,6 +118,7 @@ export default function PluginFilterTimeColumn( onMouseLeave={unsetFocusedFilter} ref={inputRef} options={options} + onDropdownVisibleChange={setFilterActive} /> diff --git a/superset-frontend/src/filters/components/TimeColumn/transformProps.ts b/superset-frontend/src/filters/components/TimeColumn/transformProps.ts index 0353feea1d100..9ca0ec047bfa8 100644 --- a/superset-frontend/src/filters/components/TimeColumn/transformProps.ts +++ b/superset-frontend/src/filters/components/TimeColumn/transformProps.ts @@ -17,6 +17,7 @@ * under the License. */ import { ChartProps } from '@superset-ui/core'; +import { noOp } from 'src/utils/common'; import { DEFAULT_FORM_DATA } from './types'; export default function transformProps(chartProps: ChartProps) { @@ -28,11 +29,13 @@ export default function transformProps(chartProps: ChartProps) { queriesData, width, filterState, + inputRef, } = chartProps; const { - setDataMask = () => {}, - setFocusedFilter = () => {}, - unsetFocusedFilter = () => {}, + setDataMask = noOp, + setFocusedFilter = noOp, + unsetFocusedFilter = noOp, + setFilterActive = noOp, } = hooks; const { data } = queriesData[0]; @@ -47,5 +50,7 @@ export default function transformProps(chartProps: ChartProps) { setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, + inputRef, }; } diff --git a/superset-frontend/src/filters/components/TimeColumn/types.ts b/superset-frontend/src/filters/components/TimeColumn/types.ts index 19ad0a8377d90..95b1e5edfdc93 100644 --- a/superset-frontend/src/filters/components/TimeColumn/types.ts +++ b/superset-frontend/src/filters/components/TimeColumn/types.ts @@ -39,6 +39,7 @@ export type PluginFilterTimeColumnProps = PluginFilterStylesProps & { data: DataRecord[]; filterState: FilterState; formData: PluginFilterTimeColumnQueryFormData; + inputRef: RefObject; } & PluginFilterHooks; export const DEFAULT_FORM_DATA: PluginFilterTimeColumnCustomizeProps = { diff --git a/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx b/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx index 4bac5c47f71bf..383272c7bbd52 100644 --- a/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx @@ -40,9 +40,11 @@ export default function PluginFilterTimegrain( setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, filterState, + inputRef, } = props; - const { defaultValue, inputRef } = formData; + const { defaultValue } = formData; const [value, setValue] = useState(defaultValue ?? []); const durationMap = useMemo( @@ -126,6 +128,7 @@ export default function PluginFilterTimegrain( onMouseLeave={unsetFocusedFilter} ref={inputRef} options={options} + onDropdownVisibleChange={setFilterActive} /> diff --git a/superset-frontend/src/filters/components/TimeGrain/transformProps.ts b/superset-frontend/src/filters/components/TimeGrain/transformProps.ts index c49d5e0edd6c3..797a2d1d80e50 100644 --- a/superset-frontend/src/filters/components/TimeGrain/transformProps.ts +++ b/superset-frontend/src/filters/components/TimeGrain/transformProps.ts @@ -17,15 +17,17 @@ * under the License. */ import { ChartProps } from '@superset-ui/core'; +import { noOp } from 'src/utils/common'; import { DEFAULT_FORM_DATA } from './types'; export default function transformProps(chartProps: ChartProps) { - const { formData, height, hooks, queriesData, width, filterState } = + const { formData, height, hooks, queriesData, width, filterState, inputRef } = chartProps; const { - setDataMask = () => {}, - setFocusedFilter = () => {}, - unsetFocusedFilter = () => {}, + setDataMask = noOp, + setFocusedFilter = noOp, + unsetFocusedFilter = noOp, + setFilterActive = noOp, } = hooks; const { data } = queriesData[0]; @@ -39,5 +41,7 @@ export default function transformProps(chartProps: ChartProps) { setDataMask, setFocusedFilter, unsetFocusedFilter, + setFilterActive, + inputRef, }; } diff --git a/superset-frontend/src/filters/components/TimeGrain/types.ts b/superset-frontend/src/filters/components/TimeGrain/types.ts index 353538849fbb0..3dfa9a99d8dc5 100644 --- a/superset-frontend/src/filters/components/TimeGrain/types.ts +++ b/superset-frontend/src/filters/components/TimeGrain/types.ts @@ -33,6 +33,7 @@ export type PluginFilterTimeGrainProps = PluginFilterStylesProps & { data: DataRecord[]; filterState: FilterState; formData: PluginFilterTimeGrainQueryFormData; + inputRef: RefObject; } & PluginFilterHooks; export const DEFAULT_FORM_DATA: PluginFilterTimeGrainCustomizeProps = { diff --git a/superset-frontend/src/filters/components/types.ts b/superset-frontend/src/filters/components/types.ts index a831bcfdedcdb..2a403fe61bb73 100644 --- a/superset-frontend/src/filters/components/types.ts +++ b/superset-frontend/src/filters/components/types.ts @@ -27,4 +27,5 @@ export interface PluginFilterHooks { setDataMask: SetDataMaskHook; setFocusedFilter: () => void; unsetFocusedFilter: () => void; + setFilterActive: (isActive: boolean) => void; }