This is an automated email from the ASF dual-hosted git repository. amitmiran pushed a commit to branch 1.3 in repository https://gitbox.apache.org/repos/asf/superset.git
commit c891ed8de8d19ce6a48dcf1fe3c6197b871de49f Author: Kamil Gabryjelski <[email protected]> AuthorDate: Tue Jun 29 12:55:53 2021 +0200 perf(dashboard): Improve perf of highlighting charts in scope of active filter (#15424) * perf(dashboard): Improve perf of highlighting charts in scope of active filter * More optimizations * fix tests * Fix lint * bug fix (cherry picked from commit f109da479d9c8e7f38e9012099836eab9835e68d) --- .../dashboard/components/FiltersBadge_spec.tsx | 99 ++++++------- .../components/gridComponents/Tabs_spec.jsx | 13 +- .../dashboard/components/FiltersBadge/index.tsx | 160 +++++++++++++++++++-- .../dashboard/components/FiltersBadge/selectors.ts | 66 ++++----- .../components/SliceHeader/SliceHeader.test.tsx | 2 +- .../src/dashboard/components/SliceHeader/index.tsx | 2 +- .../components/gridComponents/ChartHolder.jsx | 36 +++-- .../dashboard/components/gridComponents/Tabs.jsx | 12 +- .../components/gridComponents/Tabs.test.tsx | 2 +- .../dashboard/components/gridComponents/index.js | 4 +- .../dashboard/containers/DashboardComponent.jsx | 28 +--- .../src/dashboard/containers/FiltersBadge.tsx | 115 --------------- 12 files changed, 275 insertions(+), 264 deletions(-) diff --git a/superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx b/superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx index d6bd497..f24b973 100644 --- a/superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx +++ b/superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx @@ -20,10 +20,15 @@ import React from 'react'; import { shallow } from 'enzyme'; import { supersetTheme } from '@superset-ui/core'; import { Provider } from 'react-redux'; +import { Store } from 'redux'; import * as SupersetUI from '@superset-ui/core'; -import { CHART_UPDATE_SUCCEEDED } from 'src/chart/chartAction'; +import { styledMount as mount } from 'spec/helpers/theming'; +import { + CHART_RENDERING_SUCCEEDED, + CHART_UPDATE_SUCCEEDED, +} from 'src/chart/chartAction'; import { buildActiveFilters } from 'src/dashboard/util/activeDashboardFilters'; -import FiltersBadge from 'src/dashboard/containers/FiltersBadge'; +import { FiltersBadge } from 'src/dashboard/components/FiltersBadge'; import { getMockStoreWithFilters, getMockStoreWithNativeFilters, @@ -34,6 +39,15 @@ import { dashboardWithFilter } from 'spec/fixtures/mockDashboardLayout'; import Icons from 'src/components/Icons'; import { FeatureFlag } from 'src/featureFlags'; +const defaultStore = getMockStoreWithFilters(); +function setup(store: Store = defaultStore) { + return mount( + <Provider store={store}> + <FiltersBadge chartId={sliceId} /> + </Provider>, + ); +} + describe('FiltersBadge', () => { // there's this bizarre "active filters" thing // that doesn't actually use any kind of state management. @@ -71,9 +85,7 @@ describe('FiltersBadge', () => { <FiltersBadge chartId={sliceId} />, </Provider>, ); - expect( - wrapper.dive().find('[data-test="applied-filter-count"]'), - ).not.toExist(); + expect(wrapper.find('[data-test="applied-filter-count"]')).not.toExist(); }); it('shows the indicator when filters have been applied', () => { @@ -91,14 +103,13 @@ describe('FiltersBadge', () => { ], dashboardFilters, }); - const wrapper = shallow( - <FiltersBadge {...{ store }} chartId={sliceId} />, - ).dive(); - expect(wrapper.dive().find('DetailsPanelPopover')).toExist(); - expect( - wrapper.dive().find('[data-test="applied-filter-count"]'), - ).toHaveText('1'); - expect(wrapper.dive().find('WarningFilled')).not.toExist(); + store.dispatch({ type: CHART_RENDERING_SUCCEEDED, key: sliceId }); + const wrapper = setup(store); + expect(wrapper.find('DetailsPanelPopover')).toExist(); + expect(wrapper.find('[data-test="applied-filter-count"]')).toHaveText( + '1', + ); + expect(wrapper.find('WarningFilled')).not.toExist(); }); it("shows a warning when there's a rejected filter", () => { @@ -118,19 +129,18 @@ describe('FiltersBadge', () => { ], dashboardFilters, }); - const wrapper = shallow( - <FiltersBadge {...{ store }} chartId={sliceId} />, - ).dive(); - expect(wrapper.dive().find('DetailsPanelPopover')).toExist(); - expect( - wrapper.dive().find('[data-test="applied-filter-count"]'), - ).toHaveText('0'); + store.dispatch({ type: CHART_RENDERING_SUCCEEDED, key: sliceId }); + const wrapper = setup(store); + expect(wrapper.find('DetailsPanelPopover')).toExist(); + expect(wrapper.find('[data-test="applied-filter-count"]')).toHaveText( + '0', + ); expect( - wrapper.dive().find('[data-test="incompatible-filter-count"]'), + wrapper.find('[data-test="incompatible-filter-count"]'), ).toHaveText('1'); // to look at the shape of the wrapper use: - // console.log(wrapper.dive().debug()) - expect(wrapper.dive().find(Icons.AlertSolid)).toExist(); + // console.log(wrapper.debug()) + expect(wrapper.find(Icons.AlertSolid)).toExist(); }); }); @@ -149,14 +159,9 @@ describe('FiltersBadge', () => { }, ], }); - const wrapper = shallow( - <Provider store={store}> - <FiltersBadge chartId={sliceId} />, - </Provider>, - ); - expect( - wrapper.dive().find('[data-test="applied-filter-count"]'), - ).not.toExist(); + store.dispatch({ type: CHART_RENDERING_SUCCEEDED, key: sliceId }); + const wrapper = setup(store); + expect(wrapper.find('[data-test="applied-filter-count"]')).not.toExist(); }); it('shows the indicator when filters have been applied', () => { @@ -177,14 +182,13 @@ describe('FiltersBadge', () => { }, ], }); - const wrapper = shallow( - <FiltersBadge {...{ store }} chartId={sliceId} />, - ).dive(); - expect(wrapper.dive().find('DetailsPanelPopover')).toExist(); - expect( - wrapper.dive().find('[data-test="applied-filter-count"]'), - ).toHaveText('1'); - expect(wrapper.dive().find('WarningFilled')).not.toExist(); + store.dispatch({ type: CHART_RENDERING_SUCCEEDED, key: sliceId }); + const wrapper = setup(store); + expect(wrapper.find('DetailsPanelPopover')).toExist(); + expect(wrapper.find('[data-test="applied-filter-count"]')).toHaveText( + '1', + ); + expect(wrapper.find('WarningFilled')).not.toExist(); }); it("shows a warning when there's a rejected filter", () => { @@ -207,17 +211,16 @@ describe('FiltersBadge', () => { }, ], }); - const wrapper = shallow( - <FiltersBadge {...{ store }} chartId={sliceId} />, - ).dive(); - expect(wrapper.dive().find('DetailsPanelPopover')).toExist(); - expect( - wrapper.dive().find('[data-test="applied-filter-count"]'), - ).toHaveText('0'); + store.dispatch({ type: CHART_RENDERING_SUCCEEDED, key: sliceId }); + const wrapper = setup(store); + expect(wrapper.find('DetailsPanelPopover')).toExist(); + expect(wrapper.find('[data-test="applied-filter-count"]')).toHaveText( + '0', + ); expect( - wrapper.dive().find('[data-test="incompatible-filter-count"]'), + wrapper.find('[data-test="incompatible-filter-count"]'), ).toHaveText('1'); - expect(wrapper.dive().find(Icons.AlertSolid)).toExist(); + expect(wrapper.find(Icons.AlertSolid)).toExist(); }); }); }); diff --git a/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Tabs_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Tabs_spec.jsx index 517ac30..0985fd6 100644 --- a/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Tabs_spec.jsx +++ b/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Tabs_spec.jsx @@ -31,7 +31,7 @@ import DashboardComponent from 'src/dashboard/containers/DashboardComponent'; import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton'; import HoverMenu from 'src/dashboard/components/menu/HoverMenu'; import DragDroppable from 'src/dashboard/components/dnd/DragDroppable'; -import Tabs from 'src/dashboard/components/gridComponents/Tabs'; +import { Tabs } from 'src/dashboard/components/gridComponents/Tabs'; import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants'; import emptyDashboardLayout from 'src/dashboard/fixtures/emptyDashboardLayout'; import { dashboardLayoutWithTabs } from 'spec/fixtures/mockDashboardLayout'; @@ -66,14 +66,15 @@ describe('Tabs', () => { nativeFilters: nativeFilters.filters, }; + const mockStore = getMockStore({ + ...initialState, + dashboardLayout: dashboardLayoutWithTabs, + dashboardFilters: {}, + }); + function setup(overrideProps) { // We have to wrap provide DragDropContext for the underlying DragDroppable // otherwise we cannot assert on DragDroppable children - const mockStore = getMockStore({ - ...initialState, - dashboardLayout: dashboardLayoutWithTabs, - dashboardFilters: {}, - }); const wrapper = mount( <Provider store={mockStore}> <DndProvider backend={HTML5Backend}> diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx index 506dc52..5cc86e1 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx +++ b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx @@ -16,29 +16,159 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { useDispatch, useSelector } from 'react-redux'; +import { uniqWith } from 'lodash'; import cx from 'classnames'; import Icon from 'src/components/Icon'; import Icons from 'src/components/Icons'; +import { usePrevious } from 'src/common/hooks/usePrevious'; +import { DataMaskStateWithId } from 'src/dataMask/types'; import DetailsPanelPopover from './DetailsPanel'; import { Pill } from './Styles'; -import { Indicator } from './selectors'; +import { + Indicator, + IndicatorStatus, + selectIndicatorsForChart, + selectNativeIndicatorsForChart, +} from './selectors'; +import { setDirectPathToChild } from '../../actions/dashboardState'; +import { + ChartsState, + DashboardInfo, + DashboardLayout, + RootState, +} from '../../types'; +import { Filters } from '../../reducers/types'; export interface FiltersBadgeProps { - appliedCrossFilterIndicators: Indicator[]; - appliedIndicators: Indicator[]; - unsetIndicators: Indicator[]; - incompatibleIndicators: Indicator[]; - onHighlightFilterSource: (path: string[]) => void; + chartId: number; } -const FiltersBadge = ({ - appliedCrossFilterIndicators, - appliedIndicators, - unsetIndicators, - incompatibleIndicators, - onHighlightFilterSource, -}: FiltersBadgeProps) => { +const sortByStatus = (indicators: Indicator[]): Indicator[] => { + const statuses = [ + IndicatorStatus.Applied, + IndicatorStatus.Unset, + IndicatorStatus.Incompatible, + ]; + return indicators.sort( + (a, b) => + statuses.indexOf(a.status as IndicatorStatus) - + statuses.indexOf(b.status as IndicatorStatus), + ); +}; + +export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { + const dispatch = useDispatch(); + const datasources = useSelector<RootState, any>(state => state.datasources); + const dashboardFilters = useSelector<RootState, any>( + state => state.dashboardFilters, + ); + const nativeFilters = useSelector<RootState, Filters>( + state => state.nativeFilters?.filters, + ); + const dashboardInfo = useSelector<RootState, DashboardInfo>( + state => state.dashboardInfo, + ); + const charts = useSelector<RootState, ChartsState>(state => state.charts); + const present = useSelector<RootState, DashboardLayout>( + state => state.dashboardLayout.present, + ); + const dataMask = useSelector<RootState, DataMaskStateWithId>( + state => state.dataMask, + ); + + const [nativeIndicators, setNativeIndicators] = useState<Indicator[]>([]); + const [dashboardIndicators, setDashboardIndicators] = useState<Indicator[]>( + [], + ); + + const onHighlightFilterSource = useCallback( + (path: string[]) => { + dispatch(setDirectPathToChild(path)); + }, + [dispatch], + ); + + const chart = charts[chartId]; + const prevChartStatus = usePrevious(chart?.chartStatus); + + const showIndicators = useCallback( + () => + chart?.chartStatus && ['rendered', 'success'].includes(chart.chartStatus), + [chart.chartStatus], + ); + useEffect(() => { + if (!showIndicators) { + setDashboardIndicators([]); + } + if (prevChartStatus !== 'success') { + setDashboardIndicators( + selectIndicatorsForChart(chartId, dashboardFilters, datasources, chart), + ); + } + }, [ + chart, + chartId, + dashboardFilters, + datasources, + prevChartStatus, + showIndicators, + ]); + + useEffect(() => { + if (!showIndicators) { + setNativeIndicators([]); + } + if (prevChartStatus !== 'success') { + setNativeIndicators( + selectNativeIndicatorsForChart( + nativeFilters, + dataMask, + chartId, + chart, + present, + dashboardInfo.metadata?.chart_configuration, + ), + ); + } + }, [ + chart, + chartId, + dashboardInfo.metadata?.chart_configuration, + dataMask, + nativeFilters, + present, + prevChartStatus, + showIndicators, + ]); + + const indicators = useMemo( + () => + uniqWith( + sortByStatus([...dashboardIndicators, ...nativeIndicators]), + (ind1, ind2) => + ind1.column === ind2.column && + ind1.name === ind2.name && + (ind1.status !== IndicatorStatus.Applied || + ind2.status !== IndicatorStatus.Applied), + ), + [dashboardIndicators, nativeIndicators], + ); + + const appliedCrossFilterIndicators = indicators.filter( + indicator => indicator.status === IndicatorStatus.CrossFilterApplied, + ); + const appliedIndicators = indicators.filter( + indicator => indicator.status === IndicatorStatus.Applied, + ); + const unsetIndicators = indicators.filter( + indicator => indicator.status === IndicatorStatus.Unset, + ); + const incompatibleIndicators = indicators.filter( + indicator => indicator.status === IndicatorStatus.Incompatible, + ); + if ( !appliedCrossFilterIndicators.length && !appliedIndicators.length && @@ -89,4 +219,4 @@ const FiltersBadge = ({ ); }; -export default FiltersBadge; +export default React.memo(FiltersBadge); diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts index 8b5854d..48d706f 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts +++ b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts @@ -18,10 +18,7 @@ */ import { NO_TIME_RANGE, TIME_FILTER_MAP } from 'src/explore/constants'; import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters'; -import { - ChartConfiguration, - NativeFiltersState, -} from 'src/dashboard/reducers/types'; +import { ChartConfiguration, Filters } from 'src/dashboard/reducers/types'; import { DataMaskStateWithId, DataMaskType } from 'src/dataMask/types'; import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; import { Layout } from '../../types'; @@ -147,12 +144,8 @@ export const selectIndicatorsForChart = ( chartId: number, filters: { [key: number]: Filter }, datasources: { [key: string]: Datasource }, - charts: any, + chart: any, ): Indicator[] => { - const chart = charts[chartId]; - // no indicators if chart is loading - if (chart.chartStatus === 'loading') return []; - // for now we only need to know which columns are compatible/incompatible, // so grab the columns from the applied/rejected filters const appliedColumns = getAppliedColumns(chart); @@ -178,15 +171,13 @@ export const selectIndicatorsForChart = ( }; export const selectNativeIndicatorsForChart = ( - nativeFilters: NativeFiltersState, + nativeFilters: Filters, dataMask: DataMaskStateWithId, chartId: number, - charts: any, + chart: any, dashboardLayout: Layout, chartConfiguration: ChartConfiguration = {}, ): Indicator[] => { - const chart = charts[chartId]; - const appliedColumns = getAppliedColumns(chart); const rejectedColumns = getRejectedColumns(chart); @@ -218,29 +209,32 @@ export const selectNativeIndicatorsForChart = ( let nativeFilterIndicators: any = []; if (isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS)) { - nativeFilterIndicators = Object.values(nativeFilters.filters) - .filter(nativeFilter => - getTreeCheckedItems(nativeFilter.scope, dashboardLayout).some( - layoutItem => dashboardLayout[layoutItem]?.meta?.chartId === chartId, - ), - ) - .map(nativeFilter => { - const column = nativeFilter.targets[0]?.column?.name; - let value = - dataMask[nativeFilter.id]?.filterState?.label ?? - dataMask[nativeFilter.id]?.filterState?.value ?? - null; - if (!Array.isArray(value) && value !== null) { - value = [value]; - } - return { - column, - name: nativeFilter.name, - path: [nativeFilter.id], - status: getStatus({ value, column }), - value, - }; - }); + nativeFilterIndicators = + nativeFilters && + Object.values(nativeFilters) + .filter(nativeFilter => + getTreeCheckedItems(nativeFilter.scope, dashboardLayout).some( + layoutItem => + dashboardLayout[layoutItem]?.meta?.chartId === chartId, + ), + ) + .map(nativeFilter => { + const column = nativeFilter.targets[0]?.column?.name; + let value = + dataMask[nativeFilter.id]?.filterState?.label ?? + dataMask[nativeFilter.id]?.filterState?.value ?? + null; + if (!Array.isArray(value) && value !== null) { + value = [value]; + } + return { + column, + name: nativeFilter.name, + path: [nativeFilter.id], + status: getStatus({ value, column }), + value, + }; + }); } let crossFilterIndicators: any = []; diff --git a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx index fc67bb5..3416fef 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx @@ -93,7 +93,7 @@ jest.mock('src/dashboard/components/SliceHeaderControls', () => ({ ), })); -jest.mock('src/dashboard/containers/FiltersBadge', () => ({ +jest.mock('src/dashboard/components/FiltersBadge', () => ({ __esModule: true, default: (props: any) => ( <div data-test="FiltersBadge" data-chart-id={props.chartId} /> diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx index d6f4110..2e0e234 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx @@ -22,7 +22,7 @@ import { Tooltip } from 'src/components/Tooltip'; import { useSelector } from 'react-redux'; import EditableTitle from 'src/components/EditableTitle'; import SliceHeaderControls from 'src/dashboard/components/SliceHeaderControls'; -import FiltersBadge from 'src/dashboard/containers/FiltersBadge'; +import FiltersBadge from 'src/dashboard/components/FiltersBadge'; import Icon from 'src/components/Icon'; import { RootState } from 'src/dashboard/types'; import FilterIndicator from 'src/dashboard/components/FiltersBadge/FilterIndicator'; diff --git a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx index 01be44f..32b0795 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx @@ -20,6 +20,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import cx from 'classnames'; import { useTheme } from '@superset-ui/core'; +import { useSelector } from 'react-redux'; import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters'; import Chart from '../../containers/Chart'; @@ -30,13 +31,13 @@ import HoverMenu from '../menu/HoverMenu'; import ResizableContainer from '../resizable/ResizableContainer'; import getChartAndLabelComponentIdFromPath from '../../util/getChartAndLabelComponentIdFromPath'; import { componentShape } from '../../util/propShapes'; -import { ROW_TYPE, COLUMN_TYPE } from '../../util/componentTypes'; +import { COLUMN_TYPE, ROW_TYPE } from '../../util/componentTypes'; import { - GRID_MIN_COLUMN_COUNT, - GRID_MIN_ROW_UNITS, GRID_BASE_UNIT, GRID_GUTTER_SIZE, + GRID_MIN_COLUMN_COUNT, + GRID_MIN_ROW_UNITS, } from '../../util/constants'; const CHART_MARGIN = 32; @@ -73,6 +74,21 @@ const defaultProps = { }; /** + * Selects the chart scope of the filter input that has focus. + * + * @returns {{chartId: number, scope: { scope: string[], immune: string[] }} | null } + * the scope of the currently focused filter, if any + */ +function selectFocusedFilterScope(dashboardState, dashboardFilters) { + if (!dashboardState.focusedFilterField) return null; + const { chartId, column } = dashboardState.focusedFilterField; + return { + chartId, + scope: dashboardFilters[chartId].scopes[column], + }; +} + +/** * Renders any styles necessary to highlight the chart's relationship to the focused filter. * * If there is no focused filter scope (i.e. most of the time), this will be just a pass-through. @@ -85,8 +101,16 @@ const defaultProps = { * If ChartHolder were a function component, this could be implemented as a hook instead. */ const FilterFocusHighlight = React.forwardRef( - ({ chartId, focusedFilterScope, nativeFilters, ...otherProps }, ref) => { + ({ chartId, ...otherProps }, ref) => { const theme = useTheme(); + + const nativeFilters = useSelector(state => state.nativeFilters); + const dashboardState = useSelector(state => state.dashboardState); + const dashboardFilters = useSelector(state => state.dashboardFilters); + const focusedFilterScope = selectFocusedFilterScope( + dashboardState, + dashboardFilters, + ); const focusedNativeFilterId = nativeFilters.focusedFilterId; if (!(focusedFilterScope || focusedNativeFilterId)) return <div ref={ref} {...otherProps} />; @@ -239,8 +263,6 @@ class ChartHolder extends React.Component { editMode, isComponentVisible, dashboardId, - focusedFilterScope, - nativeFilters, } = this.props; // inherit the size of parent columns @@ -298,8 +320,6 @@ class ChartHolder extends React.Component { > <FilterFocusHighlight chartId={chartId} - focusedFilterScope={focusedFilterScope} - nativeFilters={nativeFilters} ref={dragSourceRef} data-test="dashboard-component-chart-holder" className={cx( diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx index 3c68ffa..83d75bc 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx @@ -18,10 +18,11 @@ */ import React from 'react'; import PropTypes from 'prop-types'; +import { styled, t } from '@superset-ui/core'; +import { connect } from 'react-redux'; import { LineEditableTabs } from 'src/components/Tabs'; import { LOG_ACTIONS_SELECT_DASHBOARD_TAB } from 'src/logger/LogUtils'; import { Modal } from 'src/common/components'; -import { styled, t } from '@superset-ui/core'; import DragDroppable from '../dnd/DragDroppable'; import DragHandle from '../dnd/DragHandle'; import DashboardComponent from '../../containers/DashboardComponent'; @@ -107,7 +108,7 @@ const StyledTabsContainer = styled.div` } `; -class Tabs extends React.PureComponent { +export class Tabs extends React.PureComponent { constructor(props) { super(props); const tabIndex = Math.max( @@ -300,7 +301,7 @@ class Tabs extends React.PureComponent { const { tabIndex: selectedTabIndex, activeKey } = this.state; let tabsToHighlight; - if (nativeFilters.focusedFilterId) { + if (nativeFilters?.focusedFilterId) { tabsToHighlight = nativeFilters.filters[nativeFilters.focusedFilterId].tabsInScope; } @@ -396,4 +397,7 @@ class Tabs extends React.PureComponent { Tabs.propTypes = propTypes; Tabs.defaultProps = defaultProps; -export default Tabs; +function mapStateToProps(state) { + return { nativeFilters: state.nativeFilters }; +} +export default connect(mapStateToProps)(Tabs); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx index 49ec405..d37d793 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx @@ -26,7 +26,7 @@ import DragDroppable from 'src/dashboard/components/dnd/DragDroppable'; import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton'; import getLeafComponentIdFromPath from 'src/dashboard/util/getLeafComponentIdFromPath'; import emptyDashboardLayout from 'src/dashboard/fixtures/emptyDashboardLayout'; -import Tabs from './Tabs'; +import { Tabs } from './Tabs'; jest.mock('src/dashboard/containers/DashboardComponent', () => jest.fn(props => ( diff --git a/superset-frontend/src/dashboard/components/gridComponents/index.js b/superset-frontend/src/dashboard/components/gridComponents/index.js index 27f419c..ce014f8 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/index.js +++ b/superset-frontend/src/dashboard/components/gridComponents/index.js @@ -34,7 +34,7 @@ import Divider from './Divider'; import Header from './Header'; import Row from './Row'; import Tab from './Tab'; -import Tabs from './Tabs'; +import TabsConnected from './Tabs'; export { default as ChartHolder } from './ChartHolder'; export { default as Markdown } from './Markdown'; @@ -53,5 +53,5 @@ export const componentLookup = { [HEADER_TYPE]: Header, [ROW_TYPE]: Row, [TAB_TYPE]: Tab, - [TABS_TYPE]: Tabs, + [TABS_TYPE]: TabsConnected, }; diff --git a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx index 296b3bd..eb31a72 100644 --- a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx +++ b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx @@ -64,29 +64,8 @@ const defaultProps = { isComponentVisible: true, }; -/** - * Selects the chart scope of the filter input that has focus. - * - * @returns {{chartId: number, scope: { scope: string[], immune: string[] }} | null } - * the scope of the currently focused filter, if any - */ -function selectFocusedFilterScope(dashboardState, dashboardFilters) { - if (!dashboardState.focusedFilterField) return null; - const { chartId, column } = dashboardState.focusedFilterField; - return { - chartId, - scope: dashboardFilters[chartId].scopes[column], - }; -} - function mapStateToProps( - { - dashboardLayout: undoableLayout, - dashboardState, - dashboardInfo, - dashboardFilters, - nativeFilters, - }, + { dashboardLayout: undoableLayout, dashboardState, dashboardInfo }, ownProps, ) { const dashboardLayout = undoableLayout.present; @@ -104,11 +83,6 @@ function mapStateToProps( activeTabs: dashboardState.activeTabs, directPathLastUpdated: dashboardState.directPathLastUpdated, dashboardId: dashboardInfo.id, - nativeFilters, - focusedFilterScope: selectFocusedFilterScope( - dashboardState, - dashboardFilters, - ), }; // rows and columns need more data about their child dimensions diff --git a/superset-frontend/src/dashboard/containers/FiltersBadge.tsx b/superset-frontend/src/dashboard/containers/FiltersBadge.tsx deleted file mode 100644 index 546b963..0000000 --- a/superset-frontend/src/dashboard/containers/FiltersBadge.tsx +++ /dev/null @@ -1,115 +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 { connect } from 'react-redux'; -import { bindActionCreators, Dispatch } from 'redux'; -import { uniqWith } from 'lodash'; -import { setDirectPathToChild } from 'src/dashboard/actions/dashboardState'; -import { - selectIndicatorsForChart, - Indicator, - IndicatorStatus, - selectNativeIndicatorsForChart, -} from 'src/dashboard/components/FiltersBadge/selectors'; -import FiltersBadge from 'src/dashboard/components/FiltersBadge'; - -export interface FiltersBadgeProps { - chartId: number; -} - -const mapDispatchToProps = (dispatch: Dispatch) => - bindActionCreators( - { - onHighlightFilterSource: setDirectPathToChild, - }, - dispatch, - ); - -const sortByStatus = (indicators: Indicator[]): Indicator[] => { - const statuses = [ - IndicatorStatus.Applied, - IndicatorStatus.Unset, - IndicatorStatus.Incompatible, - ]; - return indicators.sort( - (a, b) => - statuses.indexOf(a.status as IndicatorStatus) - - statuses.indexOf(b.status as IndicatorStatus), - ); -}; - -const mapStateToProps = ( - { - datasources, - dashboardFilters, - nativeFilters, - dashboardInfo, - charts, - dataMask, - dashboardLayout: { present }, - }: any, - { chartId }: FiltersBadgeProps, -) => { - const dashboardIndicators = selectIndicatorsForChart( - chartId, - dashboardFilters, - datasources, - charts, - ); - - const nativeIndicators = selectNativeIndicatorsForChart( - nativeFilters, - dataMask, - chartId, - charts, - present, - dashboardInfo.metadata?.chart_configuration, - ); - - const indicators = uniqWith( - sortByStatus([...dashboardIndicators, ...nativeIndicators]), - (ind1, ind2) => - ind1.column === ind2.column && - ind1.name === ind2.name && - (ind1.status !== IndicatorStatus.Applied || - ind2.status !== IndicatorStatus.Applied), - ); - - const appliedCrossFilterIndicators = indicators.filter( - indicator => indicator.status === IndicatorStatus.CrossFilterApplied, - ); - const appliedIndicators = indicators.filter( - indicator => indicator.status === IndicatorStatus.Applied, - ); - const unsetIndicators = indicators.filter( - indicator => indicator.status === IndicatorStatus.Unset, - ); - const incompatibleIndicators = indicators.filter( - indicator => indicator.status === IndicatorStatus.Incompatible, - ); - - return { - chartId, - appliedIndicators, - appliedCrossFilterIndicators, - unsetIndicators, - incompatibleIndicators, - }; -}; - -export default connect(mapStateToProps, mapDispatchToProps)(FiltersBadge);
