From 1fc08523af97f8a36a0b18d643a3fe60df815ca4 Mon Sep 17 00:00:00 2001 From: simcha90 <56388545+simcha90@users.noreply.github.com> Date: Mon, 7 Jun 2021 13:41:19 +0300 Subject: [PATCH] feat(native-filters): Support default to first value in select filter (#14869) * fix:fix get permission function * feat: add async filters support * revert: revert ff * feat: add async filters support * fix: merge with master * fix: remove tests * lint: fix lint * fix: fix CR notes * fix: fix with master * test: fix tests * refactor: update logic for default first value * fix: get requiredFirst * fix: support instant * docs: update text * docs: fix comments * docs: update texts --- .../DashboardBuilder/DashboardBuilder.tsx | 50 ++----- .../components/DashboardBuilder/state.ts | 93 +++++++++++++ .../FilterBar/FilterControls/FilterValue.tsx | 2 +- .../nativeFilters/FilterBar/index.tsx | 21 ++- .../nativeFilters/FilterBar/state.ts | 6 + .../FiltersConfigForm/FiltersConfigForm.tsx | 9 +- .../FiltersConfigForm/getControlItemsMap.tsx | 100 ++++++++------ .../nativeFilters/FiltersConfigModal/types.ts | 3 + .../nativeFilters/FiltersConfigModal/utils.ts | 3 + .../components/nativeFilters/types.ts | 1 + superset-frontend/src/dataMask/reducer.ts | 2 +- .../Select/SelectFilterPlugin.test.tsx | 16 +++ .../components/Select/SelectFilterPlugin.tsx | 127 ++++++++++-------- .../filters/components/Select/controlPanel.ts | 5 +- .../src/filters/components/Select/types.ts | 2 +- 15 files changed, 296 insertions(+), 144 deletions(-) create mode 100644 superset-frontend/src/dashboard/components/DashboardBuilder/state.ts diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 807fe46b75..9873dfbd4f 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -18,7 +18,7 @@ */ /* eslint-env browser */ import cx from 'classnames'; -import React, { FC, useEffect, useState } from 'react'; +import React, { FC } from 'react'; import { Sticky, StickyContainer } from 'react-sticky'; import { JsonObject, styled } from '@superset-ui/core'; import ErrorBoundary from 'src/components/ErrorBoundary'; @@ -30,7 +30,6 @@ import DashboardComponent from 'src/dashboard/containers/DashboardComponent'; import ToastPresenter from 'src/messageToasts/containers/ToastPresenter'; import WithPopoverMenu from 'src/dashboard/components/menu/WithPopoverMenu'; import getDirectPathToTabIndex from 'src/dashboard/util/getDirectPathToTabIndex'; -import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import { URL_PARAMS } from 'src/constants'; import { useDispatch, useSelector } from 'react-redux'; import { getUrlParam } from 'src/utils/urlUtils'; @@ -47,11 +46,11 @@ import { DashboardStandaloneMode, } from 'src/dashboard/util/constants'; import FilterBar from 'src/dashboard/components/nativeFilters/FilterBar'; +import Loading from 'src/components/Loading'; import { StickyVerticalBar } from '../StickyVerticalBar'; import { shouldFocusTabs, getRootLevelTabsComponent } from './utils'; -import { useFilters } from '../nativeFilters/FilterBar/state'; -import { Filter } from '../nativeFilters/types'; import DashboardContainer from './DashboardContainer'; +import { useNativeFilters } from './state'; const TABS_HEIGHT = 47; const HEADER_HEIGHT = 67; @@ -99,12 +98,6 @@ const DashboardBuilder: FC = () => { const dashboardLayout = useSelector( state => state.dashboardLayout.present, ); - const showNativeFilters = useSelector( - state => state.dashboardInfo.metadata?.show_native_filters, - ); - const canEdit = useSelector( - ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, - ); const editMode = useSelector( state => state.dashboardState.editMode, ); @@ -112,22 +105,6 @@ const DashboardBuilder: FC = () => { state => state.dashboardState.directPathToChild, ); - const filters = useFilters(); - const filterValues = Object.values(filters); - - const nativeFiltersEnabled = - showNativeFilters && - isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && - (canEdit || (!canEdit && filterValues.length !== 0)); - - const [dashboardFiltersOpen, setDashboardFiltersOpen] = useState( - getUrlParam(URL_PARAMS.showFilters) ?? true, - ); - - const toggleDashboardFiltersOpen = (visible?: boolean) => { - setDashboardFiltersOpen(visible ?? !dashboardFiltersOpen); - }; - const handleChangeTab = ({ pathToTabIndex, }: { @@ -161,15 +138,12 @@ const DashboardBuilder: FC = () => { (hideDashboardHeader ? 0 : HEADER_HEIGHT) + (topLevelTabs ? TABS_HEIGHT : 0); - useEffect(() => { - if ( - filterValues.length === 0 && - dashboardFiltersOpen && - nativeFiltersEnabled - ) { - toggleDashboardFiltersOpen(false); - } - }, [filterValues.length]); + const { + showDashboard, + dashboardFiltersOpen, + toggleDashboardFiltersOpen, + nativeFiltersEnabled, + } = useNativeFilters(); return ( = () => { )} - + {showDashboard ? ( + + ) : ( + + )} {editMode && } diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts b/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts new file mode 100644 index 0000000000..874525d93d --- /dev/null +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts @@ -0,0 +1,93 @@ +/** + * 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 { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; +import { useEffect, useState } from 'react'; +import { URL_PARAMS } from 'src/constants'; +import { getUrlParam } from 'src/utils/urlUtils'; +import { RootState } from 'src/dashboard/types'; +import { + useFilters, + useNativeFiltersDataMask, +} from '../nativeFilters/FilterBar/state'; +import { Filter } from '../nativeFilters/types'; + +// eslint-disable-next-line import/prefer-default-export +export const useNativeFilters = () => { + const [isInitialized, setIsInitialized] = useState(false); + const [dashboardFiltersOpen, setDashboardFiltersOpen] = useState( + getUrlParam(URL_PARAMS.showFilters) ?? true, + ); + const showNativeFilters = useSelector( + state => state.dashboardInfo.metadata?.show_native_filters, + ); + const canEdit = useSelector( + ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, + ); + + const filters = useFilters(); + const filterValues = Object.values(filters); + + const nativeFiltersEnabled = + showNativeFilters && + isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && + (canEdit || (!canEdit && filterValues.length !== 0)); + + const requiredFirstFilter = filterValues.filter( + ({ requiredFirst }) => requiredFirst, + ); + const dataMask = useNativeFiltersDataMask(); + const showDashboard = + isInitialized || + !nativeFiltersEnabled || + !( + nativeFiltersEnabled && + requiredFirstFilter.length && + requiredFirstFilter.find( + ({ id }) => dataMask[id]?.filterState?.value === undefined, + ) + ); + + const toggleDashboardFiltersOpen = (visible?: boolean) => { + setDashboardFiltersOpen(visible ?? !dashboardFiltersOpen); + }; + + useEffect(() => { + if ( + filterValues.length === 0 && + dashboardFiltersOpen && + nativeFiltersEnabled + ) { + toggleDashboardFiltersOpen(false); + } + }, [filterValues.length]); + + useEffect(() => { + if (showDashboard) { + setIsInitialized(true); + } + }, [showDashboard]); + + return { + showDashboard, + dashboardFiltersOpen, + toggleDashboardFiltersOpen, + nativeFiltersEnabled, + }; +}; 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 495f50f437..bb071315c6 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -74,7 +74,7 @@ const FilterValue: React.FC = ({ const { name: groupby } = column; const hasDataSource = !!datasetId; const [isLoading, setIsLoading] = useState(hasDataSource); - const [isRefreshing, setIsRefreshing] = useState(false); + const [isRefreshing, setIsRefreshing] = useState(true); const dispatch = useDispatch(); useEffect(() => { const newFormData = getFormData({ diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 44a4f82aed..631179b898 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -18,7 +18,7 @@ */ /* eslint-disable no-param-reassign */ -import { HandlerFunction, styled, t } from '@superset-ui/core'; +import { DataMask, HandlerFunction, styled, t } from '@superset-ui/core'; import React, { useEffect, useState } from 'react'; import { useDispatch } from 'react-redux'; import cx from 'classnames'; @@ -26,11 +26,7 @@ import Icon from 'src/components/Icon'; import { Tabs } from 'src/common/components'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import { updateDataMask } from 'src/dataMask/actions'; -import { - DataMaskState, - DataMaskStateWithId, - DataMaskWithId, -} from 'src/dataMask/types'; +import { DataMaskStateWithId, DataMaskWithId } from 'src/dataMask/types'; import { useImmer } from 'use-immer'; import { areObjectsEqual } from 'src/reduxUtils'; import { testWithId } from 'src/utils/testUtils'; @@ -178,10 +174,21 @@ const FilterBar: React.FC = ({ const handleFilterSelectionChange = ( filter: Pick & Partial, - dataMask: Partial, + dataMask: Partial, ) => { setIsFilterSetChanged(tab !== TabIds.AllFilters); setDataMaskSelected(draft => { + // force instant updating on initialization for filters with `requiredFirst` is true or instant filters + if ( + (dataMaskSelected[filter.id] && filter.isInstant) || + // filterState.value === undefined - means that value not initialized + (dataMask.filterState?.value !== undefined && + dataMaskSelected[filter.id]?.filterState?.value === undefined && + filter.requiredFirst) + ) { + dispatch(updateDataMask(filter.id, dataMask)); + } + draft[filter.id] = { ...(getInitialDataMask(filter.id) as DataMaskWithId), ...dataMask, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts index 8edf714500..aa4894fd9e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts @@ -76,6 +76,7 @@ export const useFilterUpdates = ( // Load filters after charts loaded export const useInitialization = () => { const [isInitialized, setIsInitialized] = useState(false); + const filters = useFilters(); const charts = useSelector(state => state.charts); // We need to know how much charts now shown on dashboard to know how many of all charts should be loaded @@ -90,6 +91,11 @@ export const useInitialization = () => { return; } + if (Object.values(filters).find(({ requiredFirst }) => requiredFirst)) { + setIsInitialized(true); + return; + } + // For some dashboards may be there are no charts on first page, // so we check up to 1 sec if there is at least on chart to load let filterTimeout: NodeJS.Timeout; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx index 76fa33bdc7..296ee8e6ad 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -443,6 +443,7 @@ const FiltersConfigForm = ( filterId, filterType: formFilter.filterType, filterToEdit, + formFilter, }) : {}; @@ -592,6 +593,7 @@ const FiltersConfigForm = ( expandIconPosition="right" > @@ -625,7 +627,11 @@ const FiltersConfigForm = ( { validator: (rule, value) => { const hasValue = !!value.filterState?.value; - if (hasValue) { + if ( + hasValue || + // TODO: do more generic + formFilter.controlValues?.defaultToFirstItem + ) { return Promise.resolve(); } return Promise.reject( @@ -673,6 +679,7 @@ const FiltersConfigForm = ( {((hasDataset && hasAdditionalFilters) || hasMetrics) && ( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx index 15e877ee57..097d80b8d8 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx @@ -23,10 +23,11 @@ import { import React from 'react'; import { Checkbox } from 'src/common/components'; import { FormInstance } from 'antd/lib/form'; -import { getChartControlPanelRegistry, t } from '@superset-ui/core'; +import { getChartControlPanelRegistry, styled, t } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; +import { FormItem } from 'src/components/Form'; import { getControlItems, setNativeFilterFieldValues } from './utils'; -import { NativeFiltersForm } from '../types'; +import { NativeFiltersForm, NativeFiltersFormItem } from '../types'; import { StyledRowFormItem } from './FiltersConfigForm'; import { Filter } from '../../types'; @@ -37,8 +38,13 @@ export interface ControlItemsProps { filterId: string; filterType: string; filterToEdit?: Filter; + formFilter?: NativeFiltersFormItem; } +const CleanFormItem = styled(FormItem)` + margin-bottom: 0; +`; + export default function getControlItemsMap({ disabled, forceUpdate, @@ -46,6 +52,7 @@ export default function getControlItemsMap({ filterId, filterType, filterToEdit, + formFilter, }: ControlItemsProps) { const controlPanelRegistry = getChartControlPanelRegistry(); const controlItems = @@ -66,46 +73,61 @@ export default function getControlItemsMap({ filterToEdit?.controlValues?.[controlItem.name] ?? controlItem?.config?.default; const element = ( - - + - + { + if (controlItem.config.requiredFirst) { + setNativeFilterFieldValues(form, filterId, { + requiredFirst: { + ...formFilter?.requiredFirst, + [controlItem.name]: checked, + }, + }); + } + if (controlItem.config.resetConfig) { + setNativeFilterFieldValues(form, filterId, { + defaultDataMask: null, + }); + } + forceUpdate(); + }} + > + {controlItem.config.label}{' '} + {controlItem.config.description && ( + + )} + + + + ); map[controlItem.name] = { element, checked: initialValue }; }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts index 60051e73f0..0ba091f4ee 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts @@ -31,6 +31,9 @@ export interface NativeFiltersFormItem { controlValues: { [key: string]: any; }; + requiredFirst: { + [key: string]: boolean; + }; defaultValue: any; defaultDataMask: DataMask; parentFilter: { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts index 852d4e15f6..b218ad4944 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts @@ -140,6 +140,9 @@ export const createHandleSave = ( adhoc_filters: formInputs.adhoc_filters, time_range: formInputs.time_range, controlValues: formInputs.controlValues ?? {}, + requiredFirst: Object.values(formInputs.requiredFirst ?? {}).find( + rf => rf, + ), name: formInputs.name, filterType: formInputs.filterType, // for now there will only ever be one target diff --git a/superset-frontend/src/dashboard/components/nativeFilters/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/types.ts index ac772dcd73..a1e206bab0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/types.ts @@ -56,6 +56,7 @@ export interface Filter { sortMetric?: string | null; adhoc_filters?: AdhocFilter[]; time_range?: string; + requiredFirst?: boolean; tabsInScope?: string[]; chartsInScope?: number[]; } diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index a0b0e38609..275787e0db 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -49,7 +49,7 @@ export function getInitialDataMask(id: string): DataMaskWithId { ...otherProps, extraFormData: {}, filterState: { - value: null, + value: undefined, }, ownState: {}, } as DataMaskWithId; diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx index c42c1ce3e1..70cb4346fa 100644 --- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx +++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx @@ -88,6 +88,7 @@ describe('SelectFilterPlugin', () => { it('Add multiple values with first render', () => { getWrapper(); expect(setDataMask).toHaveBeenCalledWith({ + extraFormData: {}, filterState: { value: ['boy'], }, @@ -98,6 +99,9 @@ describe('SelectFilterPlugin', () => { }, }); expect(setDataMask).toHaveBeenCalledWith({ + __cache: { + value: ['boy'], + }, extraFormData: { filters: [ { @@ -120,6 +124,9 @@ describe('SelectFilterPlugin', () => { userEvent.click(screen.getByRole('combobox')); userEvent.click(screen.getByTitle('girl')); expect(setDataMask).toHaveBeenCalledWith({ + __cache: { + value: ['boy'], + }, extraFormData: { filters: [ { @@ -146,6 +153,9 @@ describe('SelectFilterPlugin', () => { getWrapper(); userEvent.click(document.querySelector('[data-icon="close"]')!); expect(setDataMask).toHaveBeenCalledWith({ + __cache: { + value: ['boy'], + }, extraFormData: { adhoc_filters: [ { @@ -171,6 +181,9 @@ describe('SelectFilterPlugin', () => { getWrapper({ enableEmptyFilter: false }); userEvent.click(document.querySelector('[data-icon="close"]')!); expect(setDataMask).toHaveBeenCalledWith({ + __cache: { + value: ['boy'], + }, extraFormData: {}, filterState: { label: '', @@ -189,6 +202,9 @@ describe('SelectFilterPlugin', () => { userEvent.click(screen.getByRole('combobox')); userEvent.click(screen.getByTitle('girl')); expect(setDataMask).toHaveBeenCalledWith({ + __cache: { + value: ['boy'], + }, extraFormData: { filters: [ { diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx index 2f7d5651c0..4afdc1d2c8 100644 --- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +/* eslint-disable no-param-reassign */ import { AppSection, DataMask, @@ -28,16 +29,11 @@ import { t, tn, } from '@superset-ui/core'; -import React, { - useCallback, - useEffect, - useMemo, - useReducer, - useState, -} from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import { Select } from 'src/common/components'; import debounce from 'lodash/debounce'; import { SLOW_DEBOUNCE } from 'src/constants'; +import { useImmerReducer } from 'use-immer'; import Icons from 'src/components/Icons'; import { PluginFilterSelectProps, SelectValue } from './types'; import { StyledSelect, Styles } from '../common'; @@ -49,41 +45,33 @@ type DataMaskAction = | { type: 'ownState'; ownState: JsonObject } | { type: 'filterState'; + __cache: JsonObject; extraFormData: ExtraFormData; filterState: { value: SelectValue; label?: string }; }; -function reducer(state: DataMask, action: DataMaskAction): DataMask { +function reducer( + draft: Required & { __cache?: JsonObject }, + action: DataMaskAction, +) { switch (action.type) { case 'ownState': - return { - ...state, - ownState: { - ...(state.ownState || {}), - ...action.ownState, - }, + draft.ownState = { + ...draft.ownState, + ...action.ownState, }; + return draft; case 'filterState': - return { - ...state, - extraFormData: action.extraFormData, - filterState: { - ...(state.filterState || {}), - ...action.filterState, - }, - }; + draft.extraFormData = action.extraFormData; + // eslint-disable-next-line no-underscore-dangle + draft.__cache = action.__cache; + draft.filterState = { ...draft.filterState, ...action.filterState }; + return draft; default: - return { - ...state, - }; + return draft; } } -type DataMaskReducer = ( - prevState: DataMask, - action: DataMaskAction, -) => DataMask; - export default function PluginFilterSelect(props: PluginFilterSelectProps) { const { coltypeMap, @@ -127,32 +115,49 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { }, [col, selectedValues, data]); const [isDropdownVisible, setIsDropdownVisible] = useState(false); const [currentSuggestionSearch, setCurrentSuggestionSearch] = useState(''); - const [dataMask, dispatchDataMask] = useReducer(reducer, { + const [dataMask, dispatchDataMask] = useImmerReducer(reducer, { + extraFormData: {}, filterState, ownState: { coltypeMap, }, }); - const updateDataMask = (values: SelectValue) => { - const emptyFilter = - enableEmptyFilter && !inverseSelection && !values?.length; - const suffix = - inverseSelection && values?.length ? ` (${t('excluded')})` : ''; + const updateDataMask = useCallback( + (values: SelectValue) => { + const emptyFilter = + enableEmptyFilter && !inverseSelection && !values?.length; - dispatchDataMask({ - type: 'filterState', - extraFormData: getSelectExtraFormData( - col, - values, - emptyFilter, - inverseSelection, - ), - filterState: { - value: values, - label: `${(values || []).join(', ')}${suffix}`, - }, - }); - }; + const suffix = + inverseSelection && values?.length ? ` (${t('excluded')})` : ''; + + dispatchDataMask({ + type: 'filterState', + __cache: filterState, + extraFormData: getSelectExtraFormData( + col, + values, + emptyFilter, + inverseSelection, + ), + filterState: { + label: `${(values || []).join(', ')}${suffix}`, + value: + appSection === AppSection.FILTER_CONFIG_MODAL && defaultToFirstItem + ? undefined + : values, + }, + }); + }, + [ + appSection, + col, + defaultToFirstItem, + dispatchDataMask, + enableEmptyFilter, + inverseSelection, + JSON.stringify(filterState), + ], + ); useEffect(() => { if (!isDropdownVisible) { @@ -216,15 +221,19 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { }; useEffect(() => { - const firstItem: SelectValue = data[0] - ? (groupby.map(col => data[0][col]) as string[]) - : null; - if (isDisabled) { + if (defaultToFirstItem && filterState.value === undefined) { + // initialize to first value if set to default to first item + const firstItem: SelectValue = data[0] + ? (groupby.map(col => data[0][col]) as string[]) + : null; + // firstItem[0] !== undefined for a case when groupby changed but new data still not fetched + // TODO: still need repopulate default value in config modal when column changed + if (firstItem && firstItem[0] !== undefined) { + updateDataMask(firstItem); + } + } else if (isDisabled) { // empty selection if filter is disabled updateDataMask(null); - } else if (!isDisabled && defaultToFirstItem && firstItem) { - // initialize to first value if set to default to first item - updateDataMask(firstItem); } else { // reset data mask based on filter state updateDataMask(filterState.value); @@ -235,6 +244,10 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { defaultToFirstItem, enableEmptyFilter, inverseSelection, + updateDataMask, + data, + groupby, + JSON.stringify(filterState), ]); useEffect(() => { diff --git a/superset-frontend/src/filters/components/Select/controlPanel.ts b/superset-frontend/src/filters/components/Select/controlPanel.ts index 335e44483f..74891b0ed9 100644 --- a/superset-frontend/src/filters/components/Select/controlPanel.ts +++ b/superset-frontend/src/filters/components/Select/controlPanel.ts @@ -93,7 +93,10 @@ const config: ControlPanelConfig = { resetConfig: true, affectsDataMask: true, renderTrigger: true, - description: t('Select first item by default'), + requiredFirst: true, + description: t( + 'Select first item by default (when using this option, default value can’t be set)', + ), }, }, ], diff --git a/superset-frontend/src/filters/components/Select/types.ts b/superset-frontend/src/filters/components/Select/types.ts index aac5aa905a..36052e8696 100644 --- a/superset-frontend/src/filters/components/Select/types.ts +++ b/superset-frontend/src/filters/components/Select/types.ts @@ -29,7 +29,7 @@ import { import { RefObject } from 'react'; import { PluginFilterHooks, PluginFilterStylesProps } from '../types'; -export type SelectValue = (number | string)[] | null; +export type SelectValue = (number | string)[] | null | undefined; interface PluginFilterSelectCustomizeProps { defaultValue?: SelectValue;