diff --git a/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts b/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts index 1c4d278f6cc46..b3884a8488013 100644 --- a/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts +++ b/superset-frontend/packages/superset-ui-core/src/chart/types/Base.ts @@ -58,7 +58,6 @@ export enum AppSection { export type FilterState = { value?: any; [key: string]: any }; export type DataMask = { - __cache?: FilterState; extraFormData?: ExtraFormData; filterState?: FilterState; ownState?: JsonObject; diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx index 827f0f455d3d6..6e909f3b1527f 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.jsx @@ -25,9 +25,8 @@ import Loading from 'src/components/Loading'; import getBootstrapData from 'src/utils/getBootstrapData'; import getChartIdsFromLayout from '../util/getChartIdsFromLayout'; import getLayoutComponentFromChartId from '../util/getLayoutComponentFromChartId'; -import DashboardBuilder from './DashboardBuilder/DashboardBuilder'; + import { - chartPropShape, slicePropShape, dashboardInfoPropShape, dashboardStatePropShape, @@ -53,7 +52,6 @@ const propTypes = { }).isRequired, dashboardInfo: dashboardInfoPropShape.isRequired, dashboardState: dashboardStatePropShape.isRequired, - charts: PropTypes.objectOf(chartPropShape).isRequired, slices: PropTypes.objectOf(slicePropShape).isRequired, activeFilters: PropTypes.object.isRequired, chartConfiguration: PropTypes.object, @@ -213,11 +211,6 @@ class Dashboard extends React.PureComponent { } } - // return charts in array - getAllCharts() { - return Object.values(this.props.charts); - } - applyFilters() { const { appliedFilters } = this; const { activeFilters, ownDataCharts } = this.props; @@ -288,11 +281,7 @@ class Dashboard extends React.PureComponent { if (this.context.loading) { return ; } - return ( - <> - - - ); + return this.props.children; } } diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx b/superset-frontend/src/dashboard/components/Dashboard.test.jsx index 56a696f913140..a66eab37e37d7 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.test.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.test.jsx @@ -21,7 +21,6 @@ import { shallow } from 'enzyme'; import sinon from 'sinon'; import Dashboard from 'src/dashboard/components/Dashboard'; -import DashboardBuilder from 'src/dashboard/components/DashboardBuilder/DashboardBuilder'; import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; import newComponentFactory from 'src/dashboard/util/newComponentFactory'; @@ -63,8 +62,14 @@ describe('Dashboard', () => { loadStats: {}, }; + const ChildrenComponent = () =>
Test
; + function setup(overrideProps) { - const wrapper = shallow(); + const wrapper = shallow( + + + , + ); return wrapper; } @@ -76,9 +81,9 @@ describe('Dashboard', () => { '3_country_name': { values: ['USA'], scope: [] }, }; - it('should render a DashboardBuilder', () => { + it('should render the children component', () => { const wrapper = setup(); - expect(wrapper.find(DashboardBuilder)).toExist(); + expect(wrapper.find(ChildrenComponent)).toExist(); }); describe('UNSAFE_componentWillReceiveProps', () => { diff --git a/superset-frontend/src/dashboard/components/SyncDashboardState/SyncDashboardState.test.tsx b/superset-frontend/src/dashboard/components/SyncDashboardState/SyncDashboardState.test.tsx new file mode 100644 index 0000000000000..1565a43e19657 --- /dev/null +++ b/superset-frontend/src/dashboard/components/SyncDashboardState/SyncDashboardState.test.tsx @@ -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 React from 'react'; +import { render } from 'spec/helpers/testing-library'; +import { getItem, LocalStorageKeys } from 'src/utils/localStorageHelpers'; +import SyncDashboardState from '.'; + +test('stores the dashboard info with local storages', () => { + const testDashboardPageId = 'dashboardPageId'; + render(, { + useRedux: true, + }); + expect(getItem(LocalStorageKeys.dashboard__explore_context, {})).toEqual({ + [testDashboardPageId]: expect.objectContaining({ + dashboardPageId: testDashboardPageId, + }), + }); +}); diff --git a/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx b/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx new file mode 100644 index 0000000000000..b25d243292254 --- /dev/null +++ b/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx @@ -0,0 +1,103 @@ +/** + * 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, { useEffect } from 'react'; +import pick from 'lodash/pick'; +import { shallowEqual, useSelector } from 'react-redux'; +import { DashboardContextForExplore } from 'src/types/DashboardContextForExplore'; +import { + getItem, + LocalStorageKeys, + setItem, +} from 'src/utils/localStorageHelpers'; +import { RootState } from 'src/dashboard/types'; +import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters'; + +type Props = { dashboardPageId: string }; + +const EMPTY_OBJECT = {}; + +export const getDashboardContextLocalStorage = () => { + const dashboardsContexts = getItem( + LocalStorageKeys.dashboard__explore_context, + {}, + ); + // A new dashboard tab id is generated on each dashboard page opening. + // We mark ids as redundant when user leaves the dashboard, because they won't be reused. + // Then we remove redundant dashboard contexts from local storage in order not to clutter it + return Object.fromEntries( + Object.entries(dashboardsContexts).filter( + ([, value]) => !value.isRedundant, + ), + ); +}; + +const updateDashboardTabLocalStorage = ( + dashboardPageId: string, + dashboardContext: DashboardContextForExplore, +) => { + const dashboardsContexts = getDashboardContextLocalStorage(); + setItem(LocalStorageKeys.dashboard__explore_context, { + ...dashboardsContexts, + [dashboardPageId]: dashboardContext, + }); +}; + +const SyncDashboardState: React.FC = ({ dashboardPageId }) => { + const dashboardContextForExplore = useSelector< + RootState, + DashboardContextForExplore + >( + ({ dashboardInfo, dashboardState, nativeFilters, dataMask }) => ({ + labelColors: dashboardInfo.metadata?.label_colors || EMPTY_OBJECT, + sharedLabelColors: + dashboardInfo.metadata?.shared_label_colors || EMPTY_OBJECT, + colorScheme: dashboardState?.colorScheme, + chartConfiguration: + dashboardInfo.metadata?.chart_configuration || EMPTY_OBJECT, + nativeFilters: Object.entries(nativeFilters.filters).reduce( + (acc, [key, filterValue]) => ({ + ...acc, + [key]: pick(filterValue, ['chartsInScope']), + }), + {}, + ), + dataMask, + dashboardId: dashboardInfo.id, + filterBoxFilters: getActiveFilters(), + dashboardPageId, + }), + shallowEqual, + ); + + useEffect(() => { + updateDashboardTabLocalStorage(dashboardPageId, dashboardContextForExplore); + return () => { + // mark tab id as redundant when dashboard unmounts - case when user opens + // Explore in the same tab + updateDashboardTabLocalStorage(dashboardPageId, { + ...dashboardContextForExplore, + isRedundant: true, + }); + }; + }, [dashboardContextForExplore, dashboardPageId]); + + return null; +}; + +export default SyncDashboardState; 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 5235edcdc353d..f44a1a1df6878 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -52,6 +52,7 @@ import { onFiltersRefreshSuccess, setDirectPathToChild, } from 'src/dashboard/actions/dashboardState'; +import { RESPONSIVE_WIDTH } from 'src/filters/components/common'; import { FAST_DEBOUNCE } from 'src/constants'; import { dispatchHoverAction, dispatchFocusAction } from './utils'; import { FilterControlProps } from './types'; @@ -322,7 +323,7 @@ const FilterValue: React.FC = ({ ) : ( import( /* webpackChunkName: "DashboardContainer" */ /* webpackPreload: true */ - 'src/dashboard/containers/Dashboard' + 'src/dashboard/components/DashboardBuilder/DashboardBuilder' ), ); @@ -83,74 +81,15 @@ type PageProps = { idOrSlug: string; }; -const getDashboardContextLocalStorage = () => { - const dashboardsContexts = getItem( - LocalStorageKeys.dashboard__explore_context, - {}, - ); - // A new dashboard tab id is generated on each dashboard page opening. - // We mark ids as redundant when user leaves the dashboard, because they won't be reused. - // Then we remove redundant dashboard contexts from local storage in order not to clutter it - return Object.fromEntries( - Object.entries(dashboardsContexts).filter( - ([, value]) => !value.isRedundant, - ), - ); -}; - -const updateDashboardTabLocalStorage = ( - dashboardPageId: string, - dashboardContext: DashboardContextForExplore, -) => { - const dashboardsContexts = getDashboardContextLocalStorage(); - setItem(LocalStorageKeys.dashboard__explore_context, { - ...dashboardsContexts, - [dashboardPageId]: dashboardContext, - }); -}; - -const useSyncDashboardStateWithLocalStorage = () => { - const dashboardPageId = useMemo(() => shortid.generate(), []); - const dashboardContextForExplore = useSelector< - RootState, - DashboardContextForExplore - >(({ dashboardInfo, dashboardState, nativeFilters, dataMask }) => ({ - labelColors: dashboardInfo.metadata?.label_colors || {}, - sharedLabelColors: dashboardInfo.metadata?.shared_label_colors || {}, - colorScheme: dashboardState?.colorScheme, - chartConfiguration: dashboardInfo.metadata?.chart_configuration || {}, - nativeFilters: Object.entries(nativeFilters.filters).reduce( - (acc, [key, filterValue]) => ({ - ...acc, - [key]: pick(filterValue, ['chartsInScope']), - }), - {}, - ), - dataMask, - dashboardId: dashboardInfo.id, - filterBoxFilters: getActiveFilters(), - dashboardPageId, - })); - - useEffect(() => { - updateDashboardTabLocalStorage(dashboardPageId, dashboardContextForExplore); - return () => { - // mark tab id as redundant when dashboard unmounts - case when user opens - // Explore in the same tab - updateDashboardTabLocalStorage(dashboardPageId, { - ...dashboardContextForExplore, - isRedundant: true, - }); - }; - }, [dashboardContextForExplore, dashboardPageId]); - return dashboardPageId; -}; - export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { const theme = useTheme(); const dispatch = useDispatch(); const history = useHistory(); - const dashboardPageId = useSyncDashboardStateWithLocalStorage(); + const dashboardPageId = useMemo(() => shortid.generate(), []); + const hasDashboardInfoInitiated = useSelector( + ({ dashboardInfo }) => + dashboardInfo && Object.keys(dashboardInfo).length > 0, + ); const { addDangerToast } = useToasts(); const { result: dashboard, error: dashboardApiError } = useDashboard(idOrSlug); @@ -284,7 +223,7 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { }, [addDangerToast, datasets, datasetsApiError, dispatch]); if (error) throw error; // caught in error boundary - if (!readyToRender || !isDashboardHydrated.current) return ; + if (!readyToRender || !hasDashboardInfoInitiated) return ; return ( <> @@ -295,8 +234,11 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { chartContextMenuStyles(theme), ]} /> + - + + + ); diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index 6e9a5fae5404a..f2163a54a44a0 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -56,7 +56,6 @@ export function getInitialDataMask( } return { ...otherProps, - __cache: {}, extraFormData: {}, filterState: {}, ownState: {}, diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx index c035f81c01b89..99e6259871430 100644 --- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx +++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.test.tsx @@ -91,15 +91,6 @@ describe('SelectFilterPlugin', () => { test('Add multiple values with first render', async () => { getWrapper(); expect(setDataMask).toHaveBeenCalledWith({ - extraFormData: {}, - filterState: { - value: ['boy'], - }, - }); - expect(setDataMask).toHaveBeenCalledWith({ - __cache: { - value: ['boy'], - }, extraFormData: { filters: [ { @@ -118,9 +109,6 @@ describe('SelectFilterPlugin', () => { userEvent.click(screen.getByTitle('girl')); expect(await screen.findByTitle(/girl/i)).toBeInTheDocument(); expect(setDataMask).toHaveBeenCalledWith({ - __cache: { - value: ['boy'], - }, extraFormData: { filters: [ { @@ -146,9 +134,6 @@ describe('SelectFilterPlugin', () => { }), ); expect(setDataMask).toHaveBeenCalledWith({ - __cache: { - value: ['boy'], - }, extraFormData: { adhoc_filters: [ { @@ -174,9 +159,6 @@ describe('SelectFilterPlugin', () => { }), ); expect(setDataMask).toHaveBeenCalledWith({ - __cache: { - value: ['boy'], - }, extraFormData: {}, filterState: { label: undefined, @@ -191,9 +173,6 @@ describe('SelectFilterPlugin', () => { expect(await screen.findByTitle('girl')).toBeInTheDocument(); userEvent.click(screen.getByTitle('girl')); expect(setDataMask).toHaveBeenCalledWith({ - __cache: { - value: ['boy'], - }, extraFormData: { filters: [ { @@ -216,9 +195,6 @@ describe('SelectFilterPlugin', () => { expect(await screen.findByRole('combobox')).toBeInTheDocument(); userEvent.click(screen.getByTitle(NULL_STRING)); expect(setDataMask).toHaveBeenLastCalledWith({ - __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 7d8ab55fb5571..a4b9f5b05efaf 100644 --- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx @@ -37,7 +37,6 @@ import { Select } from 'src/components'; import { SLOW_DEBOUNCE } from 'src/constants'; import { hasOption, propertyComparator } from 'src/components/Select/utils'; import { FilterBarOrientation } from 'src/dashboard/types'; -import { uniqWith, isEqual } from 'lodash'; import { PluginFilterSelectProps, SelectValue } from './types'; import { FilterPluginStyle, StatusMessage, StyledFormItem } from '../common'; import { getDataRecordFormatter, getSelectExtraFormData } from '../../utils'; @@ -46,15 +45,11 @@ type DataMaskAction = | { type: 'ownState'; ownState: JsonObject } | { type: 'filterState'; - __cache: JsonObject; extraFormData: ExtraFormData; filterState: { value: SelectValue; label?: string }; }; -function reducer( - draft: DataMask & { __cache?: JsonObject }, - action: DataMaskAction, -) { +function reducer(draft: DataMask, action: DataMaskAction) { switch (action.type) { case 'ownState': draft.ownState = { @@ -63,10 +58,18 @@ function reducer( }; return draft; case 'filterState': - draft.extraFormData = action.extraFormData; - // eslint-disable-next-line no-underscore-dangle - draft.__cache = action.__cache; - draft.filterState = { ...draft.filterState, ...action.filterState }; + if ( + JSON.stringify(draft.extraFormData) !== + JSON.stringify(action.extraFormData) + ) { + draft.extraFormData = action.extraFormData; + } + if ( + JSON.stringify(draft.filterState) !== JSON.stringify(action.filterState) + ) { + draft.filterState = { ...draft.filterState, ...action.filterState }; + } + return draft; default: return draft; @@ -130,7 +133,6 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { const suffix = inverseSelection && values?.length ? t(' (excluded)') : ''; dispatchDataMask({ type: 'filterState', - __cache: filterState, extraFormData: getSelectExtraFormData( col, values, @@ -219,16 +221,13 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { }, [filterState.validateMessage, filterState.validateStatus]); const uniqueOptions = useMemo(() => { - const allOptions = [...data]; - return uniqWith(allOptions, isEqual).map(row => { - const [value] = groupby.map(col => row[col]); - return { - label: labelFormatter(value, datatype), - value, - isNewOption: false, - }; - }); - }, [data, datatype, groupby, labelFormatter]); + const allOptions = new Set([...data.map(el => el[col])]); + return [...allOptions].map((value: string) => ({ + label: labelFormatter(value, datatype), + value, + isNewOption: false, + })); + }, [data, datatype, col, labelFormatter]); const options = useMemo(() => { if (search && !multiSelect && !hasOption(search, uniqueOptions, true)) { diff --git a/superset-frontend/src/filters/components/common.ts b/superset-frontend/src/filters/components/common.ts index af1fe9c791761..cb6d7f22f14be 100644 --- a/superset-frontend/src/filters/components/common.ts +++ b/superset-frontend/src/filters/components/common.ts @@ -20,9 +20,11 @@ import { styled } from '@superset-ui/core'; import { PluginFilterStylesProps } from './types'; import FormItem from '../../components/Form/FormItem'; +export const RESPONSIVE_WIDTH = 0; + export const FilterPluginStyle = styled.div` min-height: ${({ height }) => height}px; - width: ${({ width }) => width}px; + width: ${({ width }) => (width === RESPONSIVE_WIDTH ? '100%' : `${width}px`)}; `; export const StyledFormItem = styled(FormItem)`