From 2a61e157318d4bab871edec70c90b3a82b592c57 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Sat, 25 Feb 2023 15:56:20 +0100 Subject: [PATCH] fix(dashboard): Focusing charts and native filters from filters badge (#23190) (cherry picked from commit 7d4aee956e61eee600ae5ab298b4c1fd9d3925dd) --- .../DashboardBuilder/DashboardBuilder.tsx | 9 +--- .../components/FiltersBadge/index.tsx | 16 +++---- .../FilterControls/FilterControl.tsx | 2 - .../FilterControls/FilterControls.tsx | 10 ++--- .../FilterBar/FilterControls/FilterValue.tsx | 43 ++++++++++++------- .../nativeFilters/FilterBar/Horizontal.tsx | 2 - .../nativeFilters/FilterBar/Vertical.tsx | 3 -- .../nativeFilters/FilterBar/index.tsx | 3 -- .../nativeFilters/FilterBar/types.ts | 8 ++-- .../FilterBar/useFilterControlFactory.tsx | 9 +--- .../FilterBar/useFilterOutlined.ts | 34 +++++++++++++++ .../FilterCard/DependenciesRow.tsx | 4 +- .../FilterCard/FilterCard.test.tsx | 6 +-- 13 files changed, 86 insertions(+), 63 deletions(-) create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterOutlined.ts diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index f07fdb9c066c4..9d6d3a51eb55f 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -442,8 +442,6 @@ const DashboardBuilder: FC = () => { const dashboardIsSaving = useSelector( ({ dashboardState }) => dashboardState.dashboardIsSaving, ); - const nativeFilters = useSelector((state: RootState) => state.nativeFilters); - const focusedFilterId = nativeFilters?.focusedFilterId; const fullSizeChartId = useSelector( state => state.dashboardState.fullSizeChartId, ); @@ -580,10 +578,7 @@ const DashboardBuilder: FC = () => { {!hideDashboardHeader && } {showFilterBar && filterBarOrientation === FilterBarOrientation.HORIZONTAL && ( - + )} {dropIndicatorProps &&
} {!isReport && topLevelTabs && !uiConfig.hideNav && ( @@ -613,7 +608,6 @@ const DashboardBuilder: FC = () => {
), [ - focusedFilterId, nativeFiltersEnabled, filterBarOrientation, editMode, @@ -658,7 +652,6 @@ const DashboardBuilder: FC = () => { {!isReport && ( { const onHighlightFilterSource = useCallback( (path: string[]) => { - dispatch(setFocusedNativeFilter(path[0])); + dispatch(setDirectPathToChild(path)); }, [dispatch], ); 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 fa54036dbbcaf..218603f74d5d5 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx @@ -222,7 +222,6 @@ const FilterControl = ({ filter, icon, onFilterSelectionChange, - focusedFilterId, inView, showOverflow, parentRef, @@ -288,7 +287,6 @@ const FilterControl = ({ dataMaskSelected={dataMaskSelected} filter={filter} showOverflow={showOverflow} - focusedFilterId={focusedFilterId} onFilterSelectionChange={onFilterSelectionChange} inView={inView} parentRef={parentRef} 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 a621052d51438..fa0bfeac6f22a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx @@ -54,15 +54,14 @@ import Icons from 'src/components/Icons'; import { FiltersOutOfScopeCollapsible } from '../FiltersOutOfScopeCollapsible'; import { useFilterControlFactory } from '../useFilterControlFactory'; import { FiltersDropdownContent } from '../FiltersDropdownContent'; +import { useFilterOutlined } from '../useFilterOutlined'; type FilterControlsProps = { - focusedFilterId?: string; dataMaskSelected: DataMaskStateWithId; onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void; }; const FilterControls: FC = ({ - focusedFilterId, dataMaskSelected, onFilterSelectionChange, }) => { @@ -73,12 +72,13 @@ const FilterControls: FC = ({ : FilterBarOrientation.VERTICAL, ); + const { outlinedFilterId, lastUpdated } = useFilterOutlined(); + const [overflowedIds, setOverflowedIds] = useState([]); const popoverRef = useRef(null); const { filterControlFactory, filtersWithValues } = useFilterControlFactory( dataMaskSelected, - focusedFilterId, onFilterSelectionChange, ); const portalNodes = useMemo(() => { @@ -234,10 +234,10 @@ const FilterControls: FC = ({ }, [filtersOutOfScope, filtersWithValues, overflowedFiltersInScope]); useEffect(() => { - if (focusedFilterId && overflowedIds.includes(focusedFilterId)) { + if (outlinedFilterId && overflowedIds.includes(outlinedFilterId)) { popoverRef?.current?.open(); } - }, [focusedFilterId, popoverRef, overflowedIds]); + }, [outlinedFilterId, lastUpdated, popoverRef, overflowedIds]); 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 308361622e16a..0347d2095e336 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -43,13 +43,17 @@ import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import { waitForAsyncData } from 'src/middleware/asyncEvent'; import { ClientErrorObject } from 'src/utils/getClientErrorObject'; import { FilterBarOrientation, RootState } from 'src/dashboard/types'; -import { onFiltersRefreshSuccess } from 'src/dashboard/actions/dashboardState'; +import { + onFiltersRefreshSuccess, + setDirectPathToChild, +} from 'src/dashboard/actions/dashboardState'; import { FAST_DEBOUNCE } from 'src/constants'; import { dispatchHoverAction, dispatchFocusAction } from './utils'; import { FilterControlProps } from './types'; import { getFormData } from '../../utils'; import { useFilterDependencies } from './state'; import { checkIsMissingRequiredValue } from '../utils'; +import { useFilterOutlined } from '../useFilterOutlined'; const HEIGHT = 32; @@ -79,7 +83,6 @@ const useShouldFilterRefresh = () => { const FilterValue: React.FC = ({ dataMaskSelected, filter, - focusedFilterId, onFilterSelectionChange, inView = true, showOverflow, @@ -111,6 +114,8 @@ const FilterValue: React.FC = ({ const [isRefreshing, setIsRefreshing] = useState(false); const dispatch = useDispatch(); + const { outlinedFilterId, lastUpdated } = useFilterOutlined(); + const handleFilterLoadFinish = useCallback(() => { setIsRefreshing(false); setIsLoading(false); @@ -212,26 +217,34 @@ const FilterValue: React.FC = ({ ]); useEffect(() => { - if (focusedFilterId && focusedFilterId === filter.id) { - setTimeout(() => { - inputRef?.current?.focus(); - }, FAST_DEBOUNCE); + if (outlinedFilterId && outlinedFilterId === filter.id) { + setTimeout( + () => { + inputRef?.current?.focus(); + }, + overflow ? FAST_DEBOUNCE : 0, + ); } - }, [inputRef, focusedFilterId, filter.id]); + }, [inputRef, outlinedFilterId, lastUpdated, filter.id, overflow]); const setDataMask = useCallback( (dataMask: DataMask) => onFilterSelectionChange(filter, dataMask), [filter, onFilterSelectionChange], ); - const setFocusedFilter = useCallback( - () => dispatchFocusAction(dispatch, id), - [dispatch, id], - ); - const unsetFocusedFilter = useCallback( - () => dispatchFocusAction(dispatch), - [dispatch], - ); + const setFocusedFilter = useCallback(() => { + // don't highlight charts in scope if filter was focused programmatically + if (outlinedFilterId !== id) { + dispatchFocusAction(dispatch, id); + } + }, [dispatch, id, outlinedFilterId]); + + const unsetFocusedFilter = useCallback(() => { + dispatchFocusAction(dispatch); + if (outlinedFilterId === id) { + dispatch(setDirectPathToChild([])); + } + }, [dispatch, id, outlinedFilterId]); const setHoveredFilter = useCallback( () => dispatchHoverAction(dispatch, id), diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx index 7e786985e890f..9dc2d62d44f0f 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx @@ -93,7 +93,6 @@ const HorizontalFilterBar: React.FC = ({ dataMaskSelected, filterValues, isInitialized, - focusedFilterId, onSelectionChange, }) => { const hasFilters = filterValues.length > 0; @@ -124,7 +123,6 @@ const HorizontalFilterBar: React.FC = ({ {hasFilters && ( )} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx index 2a6b7178362d1..3150ed5e90652 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx @@ -141,7 +141,6 @@ const VerticalFilterBar: React.FC = ({ actions, canEdit, dataMaskSelected, - focusedFilterId, filtersOpen, filterValues, height, @@ -258,7 +257,6 @@ const VerticalFilterBar: React.FC = ({ @@ -300,7 +298,6 @@ const VerticalFilterBar: React.FC = ({ diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 6eeee2ae708f9..d82c128bf8664 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -111,7 +111,6 @@ const publishDataMask = debounce( export const FilterBarScrollContext = createContext(false); const FilterBar: React.FC = ({ - focusedFilterId, orientation = FilterBarOrientation.VERTICAL, verticalConfig, }) => { @@ -254,7 +253,6 @@ const FilterBar: React.FC = ({ canEdit={canEdit} dashboardId={dashboardId} dataMaskSelected={dataMaskSelected} - focusedFilterId={focusedFilterId} filterValues={filterValues} isInitialized={isInitialized} onSelectionChange={handleFilterSelectionChange} @@ -264,7 +262,6 @@ const FilterBar: React.FC = ({ actions={actions} canEdit={canEdit} dataMaskSelected={dataMaskSelected} - focusedFilterId={focusedFilterId} filtersOpen={verticalConfig.filtersOpen} filterValues={filterValues} isInitialized={isInitialized} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/types.ts index 034a639f589e0..ac7ed704560f8 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/types.ts @@ -16,6 +16,8 @@ * specific language governing permissions and limitations * under the License. */ + +import { ReactNode } from 'react'; import { DataMask, DataMaskStateWithId, @@ -25,10 +27,9 @@ import { import { FilterBarOrientation } from 'src/dashboard/types'; interface CommonFiltersBarProps { - actions: React.ReactNode; + actions: ReactNode; canEdit: boolean; dataMaskSelected: DataMaskStateWithId; - focusedFilterId?: string; filterValues: (Filter | Divider)[]; isInitialized: boolean; onSelectionChange: ( @@ -45,8 +46,7 @@ interface VerticalBarConfig { width: number; } -export interface FiltersBarProps - extends Pick { +export interface FiltersBarProps { orientation: FilterBarOrientation; verticalConfig?: VerticalBarConfig; } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx index 29e539630d15e..92f9ca58e7a9f 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterControlFactory.tsx @@ -32,7 +32,6 @@ import FilterDivider from './FilterControls/FilterDivider'; export const useFilterControlFactory = ( dataMaskSelected: DataMaskStateWithId, - focusedFilterId: string | undefined, onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void, ) => { const filters = useFilters(); @@ -67,7 +66,6 @@ export const useFilterControlFactory = ( ); }, - [ - filtersWithValues, - dataMaskSelected, - focusedFilterId, - onFilterSelectionChange, - ], + [filtersWithValues, dataMaskSelected, onFilterSelectionChange], ); return { filterControlFactory, filtersWithValues }; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterOutlined.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterOutlined.ts new file mode 100644 index 0000000000000..05c9b671f8dad --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterOutlined.ts @@ -0,0 +1,34 @@ +/** + * 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 { useSelector } from 'react-redux'; +import { RootState } from 'src/dashboard/types'; +import getChartAndLabelComponentIdFromPath from 'src/dashboard/util/getChartAndLabelComponentIdFromPath'; + +export const useFilterOutlined = () => + useSelector( + state => ({ + outlinedFilterId: ( + getChartAndLabelComponentIdFromPath( + state.dashboardState.directPathToChild || [], + ) as Record + )?.native_filter, + lastUpdated: state.dashboardState.directPathLastUpdated, + }), + ); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx index 6a846c012f6d8..0d0907a6eb0a6 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/DependenciesRow.tsx @@ -21,7 +21,7 @@ import { useDispatch } from 'react-redux'; import { css, t, useTheme } from '@superset-ui/core'; import Icons from 'src/components/Icons'; import { useTruncation } from 'src/hooks/useTruncation'; -import { setFocusedNativeFilter } from 'src/dashboard/actions/nativeFilters'; +import { setDirectPathToChild } from 'src/dashboard/actions/dashboardState'; import { DependencyItem, Row, @@ -40,7 +40,7 @@ const DependencyValue = ({ }: DependencyValueProps) => { const dispatch = useDispatch(); const handleClick = useCallback(() => { - dispatch(setFocusedNativeFilter(dependency.id)); + dispatch(setDirectPathToChild([dependency.id])); }, [dependency.id, dispatch]); return ( 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 560fa1faf0080..93c12a53daf53 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx @@ -23,7 +23,7 @@ import { Filter, NativeFilterType } from '@superset-ui/core'; import userEvent from '@testing-library/user-event'; import { render, screen } from 'spec/helpers/testing-library'; import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants'; -import { SET_FOCUSED_NATIVE_FILTER } from 'src/dashboard/actions/nativeFilters'; +import { SET_DIRECT_PATH } from 'src/dashboard/actions/dashboardState'; import { FilterCardContent } from './FilterCardContent'; const baseInitialState = { @@ -304,8 +304,8 @@ test('focus filter on filter card dependency click', () => { userEvent.click(screen.getByText('Native filter 2')); expect(dummyDispatch).toHaveBeenCalledWith({ - type: SET_FOCUSED_NATIVE_FILTER, - id: 'NATIVE_FILTER-2', + type: SET_DIRECT_PATH, + path: ['NATIVE_FILTER-2'], }); });