diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx index 5bdce6d72ecfa..1f01edfe77585 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.jsx @@ -26,11 +26,7 @@ import getBootstrapData from 'src/utils/getBootstrapData'; import getChartIdsFromLayout from '../util/getChartIdsFromLayout'; import getLayoutComponentFromChartId from '../util/getLayoutComponentFromChartId'; -import { - slicePropShape, - dashboardInfoPropShape, - dashboardStatePropShape, -} from '../util/propShapes'; +import { slicePropShape } from '../util/propShapes'; import { LOG_ACTIONS_HIDE_BROWSER_TAB, LOG_ACTIONS_MOUNT_DASHBOARD, @@ -51,8 +47,10 @@ const propTypes = { logEvent: PropTypes.func.isRequired, clearDataMaskState: PropTypes.func.isRequired, }).isRequired, - dashboardInfo: dashboardInfoPropShape.isRequired, - dashboardState: dashboardStatePropShape.isRequired, + dashboardId: PropTypes.number.isRequired, + editMode: PropTypes.bool, + isPublished: PropTypes.bool, + hasUnsavedChanges: PropTypes.bool, slices: PropTypes.objectOf(slicePropShape).isRequired, activeFilters: PropTypes.object.isRequired, chartConfiguration: PropTypes.object, @@ -96,13 +94,13 @@ class Dashboard extends PureComponent { componentDidMount() { const bootstrapData = getBootstrapData(); - const { dashboardState, layout } = this.props; + const { editMode, isPublished, layout } = this.props; const eventData = { is_soft_navigation: Logger.timeOriginOffset > 0, - is_edit_mode: dashboardState.editMode, + is_edit_mode: editMode, mount_duration: Logger.getTimestamp(), is_empty: isDashboardEmpty(layout), - is_published: dashboardState.isPublished, + is_published: isPublished, bootstrap_data_length: bootstrapData.length, }; const directLinkComponentId = getLocationHash(); @@ -130,7 +128,7 @@ class Dashboard extends PureComponent { const currentChartIds = getChartIdsFromLayout(this.props.layout); const nextChartIds = getChartIdsFromLayout(nextProps.layout); - if (this.props.dashboardInfo.id !== nextProps.dashboardInfo.id) { + if (this.props.dashboardId !== nextProps.dashboardId) { // single-page-app navigation check return; } @@ -157,10 +155,14 @@ class Dashboard extends PureComponent { } applyCharts() { - const { hasUnsavedChanges, editMode } = this.props.dashboardState; - const { appliedFilters, appliedOwnDataCharts } = this; - const { activeFilters, ownDataCharts, chartConfiguration } = this.props; + const { + activeFilters, + ownDataCharts, + chartConfiguration, + hasUnsavedChanges, + editMode, + } = this.props; if ( isFeatureEnabled(FeatureFlag.DashboardCrossFilters) && !chartConfiguration diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx index 7da2064f96305..43e207d8c8821 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx @@ -208,7 +208,9 @@ describe('DashboardBuilder', () => { }); it('should render a BuilderComponentPane if editMode=true and user selects "Insert Components" pane', () => { - const { queryAllByTestId } = setup({ dashboardState: { editMode: true } }); + const { queryAllByTestId } = setup({ + dashboardState: { ...mockState.dashboardState, editMode: true }, + }); const builderComponents = queryAllByTestId('mock-builder-component-pane'); expect(builderComponents.length).toBeGreaterThanOrEqual(1); }); @@ -241,7 +243,7 @@ describe('DashboardBuilder', () => { it('should display a loading spinner when saving is in progress', async () => { const { findByAltText } = setup({ - dashboardState: { dashboardIsSaving: true }, + dashboardState: { ...mockState.dashboardState, dashboardIsSaving: true }, }); expect(await findByAltText('Loading...')).toBeVisible(); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 347858f039eaf..30c61f0af6553 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -370,6 +370,10 @@ const StyledDashboardContent = styled.div<{ `} `; +const ELEMENT_ON_SCREEN_OPTIONS = { + threshold: [1], +}; + const DashboardBuilder: FC = () => { const dispatch = useDispatch(); const uiConfig = useUiConfig(); @@ -469,9 +473,9 @@ const DashboardBuilder: FC = () => { nativeFiltersEnabled, } = useNativeFilters(); - const [containerRef, isSticky] = useElementOnScreen({ - threshold: [1], - }); + const [containerRef, isSticky] = useElementOnScreen( + ELEMENT_ON_SCREEN_OPTIONS, + ); const showFilterBar = (crossFiltersEnabled || nativeFiltersEnabled) && !editMode; @@ -581,6 +585,43 @@ const DashboardBuilder: FC = () => { ? 0 : theme.gridUnit * 8; + const renderChild = useCallback( + adjustedWidth => { + const filterBarWidth = dashboardFiltersOpen + ? adjustedWidth + : CLOSED_FILTER_BAR_WIDTH; + return ( + + ); + }, + [ + dashboardFiltersOpen, + toggleDashboardFiltersOpen, + filterBarHeight, + filterBarOffset, + isReport, + ], + ); + return ( {showFilterBar && @@ -593,33 +634,7 @@ const DashboardBuilder: FC = () => { maxWidth={OPEN_FILTER_BAR_MAX_WIDTH} initialWidth={OPEN_FILTER_BAR_WIDTH} > - {adjustedWidth => { - const filterBarWidth = dashboardFiltersOpen - ? adjustedWidth - : CLOSED_FILTER_BAR_WIDTH; - return ( - - ); - }} + {renderChild} )} diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index f36520ba317f5..db7ef52838b3c 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -18,8 +18,17 @@ */ // ParentSize uses resize observer so the dashboard will update size // when its container size changes, due to e.g., builder side panel opening -import { FC, useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { + FC, + memo, + useCallback, + useEffect, + useMemo, + useRef, + useState, +} from 'react'; import { useDispatch, useSelector } from 'react-redux'; +import { createSelector } from '@reduxjs/toolkit'; import { Filter, Filters, @@ -43,6 +52,7 @@ import { import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope'; import findTabIndexByComponentId from 'src/dashboard/util/findTabIndexByComponentId'; import { setInScopeStatusOfFilters } from 'src/dashboard/actions/nativeFilters'; +import { useChartIds } from 'src/dashboard/util/charts/useChartIds'; import { applyDashboardLabelsColorOnLoad, updateDashboardLabelsColor, @@ -54,11 +64,20 @@ import { getColorNamespace, resetColors } from 'src/utils/colorScheme'; import { NATIVE_FILTER_DIVIDER_PREFIX } from '../nativeFilters/FiltersConfigModal/utils'; import { findTabsWithChartsInScope } from '../nativeFilters/utils'; import { getRootLevelTabsComponent } from './utils'; +import { CHART_TYPE } from '../../util/componentTypes'; type DashboardContainerProps = { topLevelTabs?: LayoutItem; }; +export const renderedChartIdsSelector = createSelector( + [(state: RootState) => state.charts], + charts => + Object.values(charts) + .filter(chart => chart.chartStatus === 'rendered') + .map(chart => chart.id), +); + const useNativeFilterScopes = () => { const nativeFilters = useSelector( state => state.nativeFilters?.filters, @@ -70,10 +89,12 @@ const useNativeFilterScopes = () => { pick(filter, ['id', 'scope', 'type']), ) : [], - [JSON.stringify(nativeFilters)], + [nativeFilters], ); }; +const TOP_OF_PAGE_RANGE = 220; + const DashboardContainer: FC = ({ topLevelTabs }) => { const nativeFilterScopes = useNativeFilterScopes(); const dispatch = useDispatch(); @@ -87,13 +108,10 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { const directPathToChild = useSelector( state => state.dashboardState.directPathToChild, ); - const chartIds = useSelector(state => - Object.values(state.charts).map(chart => chart.id), - ); - const renderedChartIds = useSelector(state => - Object.values(state.charts) - .filter(chart => chart.chartStatus === 'rendered') - .map(chart => chart.id), + const chartIds = useChartIds(); + + const renderedChartIds = useSelector( + renderedChartIdsSelector, ); const [dashboardLabelsColorInitiated, setDashboardLabelsColorInitiated] = useState(false); @@ -136,13 +154,19 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { chartsInScope: [], }; } + + const chartLayoutItems = Object.values(dashboardLayout).filter( + item => item?.type === CHART_TYPE, + ); + const chartsInScope: number[] = getChartIdsInFilterScope( filterScope.scope, chartIds, - dashboardLayout, + chartLayoutItems, ); + const tabsInScope = findTabsWithChartsInScope( - dashboardLayout, + chartLayoutItems, chartsInScope, ); return { @@ -152,14 +176,14 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { }; }); dispatch(setInScopeStatusOfFilters(scopes)); - }, [nativeFilterScopes, dashboardLayout, dispatch]); + }, [chartIds, JSON.stringify(nativeFilterScopes), dashboardLayout, dispatch]); - const childIds: string[] = topLevelTabs - ? topLevelTabs.children - : [DASHBOARD_GRID_ID]; + const childIds: string[] = useMemo( + () => (topLevelTabs ? topLevelTabs.children : [DASHBOARD_GRID_ID]), + [topLevelTabs], + ); const min = Math.min(tabIndex, childIds.length - 1); const activeKey = min === 0 ? DASHBOARD_GRID_ID : min.toString(); - const TOP_OF_PAGE_RANGE = 220; useEffect(() => { if (shouldForceFreshSharedLabelsColors) { @@ -229,57 +253,63 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { }; }, [onBeforeUnload]); + const renderTabBar = useCallback(() => <>, []); + const handleFocus = useCallback(e => { + if ( + // prevent scrolling when tabbing to the tab pane + e.target.classList.contains('ant-tabs-tabpane') && + window.scrollY < TOP_OF_PAGE_RANGE + ) { + // prevent window from jumping down when tabbing + // if already at the top of the page + // to help with accessibility when using keyboard navigation + window.scrollTo(window.scrollX, 0); + } + }, []); + + const renderParentSizeChildren = useCallback( + ({ width }) => ( + /* + We use a TabContainer irrespective of whether top-level tabs exist to maintain + a consistent React component tree. This avoids expensive mounts/unmounts of + the entire dashboard upon adding/removing top-level tabs, which would otherwise + happen because of React's diffing algorithm + */ + + {childIds.map((id, index) => ( + // Matching the key of the first TabPane irrespective of topLevelTabs + // lets us keep the same React component tree when !!topLevelTabs changes. + // This avoids expensive mounts/unmounts of the entire dashboard. + + + + ))} + + ), + [activeKey, childIds, dashboardLayout, handleFocus, renderTabBar, tabIndex], + ); + return (
- - {({ width }) => ( - /* - We use a TabContainer irrespective of whether top-level tabs exist to maintain - a consistent React component tree. This avoids expensive mounts/unmounts of - the entire dashboard upon adding/removing top-level tabs, which would otherwise - happen because of React's diffing algorithm - */ - <>} - fullWidth={false} - animated={false} - allowOverflow - onFocus={e => { - if ( - // prevent scrolling when tabbing to the tab pane - e.target.classList.contains('ant-tabs-tabpane') && - window.scrollY < TOP_OF_PAGE_RANGE - ) { - // prevent window from jumping down when tabbing - // if already at the top of the page - // to help with accessibility when using keyboard navigation - window.scrollTo(window.scrollX, 0); - } - }} - > - {childIds.map((id, index) => ( - // Matching the key of the first TabPane irrespective of topLevelTabs - // lets us keep the same React component tree when !!topLevelTabs changes. - // This avoids expensive mounts/unmounts of the entire dashboard. - - - - ))} - - )} - + {renderParentSizeChildren}
); }; -export default DashboardContainer; +export default memo(DashboardContainer); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts b/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts index b7e6c266de537..ec1cc0bc1f0f8 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/state.ts @@ -17,7 +17,7 @@ * under the License. */ import { useSelector } from 'react-redux'; -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; import { URL_PARAMS } from 'src/constants'; import { getUrlParam } from 'src/utils/urlUtils'; import { RootState } from 'src/dashboard/types'; @@ -34,7 +34,7 @@ export const useNativeFilters = () => { ); const filters = useFilters(); - const filterValues = Object.values(filters); + const filterValues = useMemo(() => Object.values(filters), [filters]); const expandFilters = getUrlParam(URL_PARAMS.expandFilters); const [dashboardFiltersOpen, setDashboardFiltersOpen] = useState( expandFilters ?? !!filterValues.length, @@ -43,24 +43,28 @@ export const useNativeFilters = () => { const nativeFiltersEnabled = canEdit || (!canEdit && filterValues.length !== 0); - const requiredFirstFilter = filterValues.filter( - filter => filter.requiredFirst, + const requiredFirstFilter = useMemo( + () => filterValues.filter(filter => filter.requiredFirst), + [filterValues], ); const dataMask = useNativeFiltersDataMask(); - const missingInitialFilters = requiredFirstFilter - .filter(({ id }) => dataMask[id]?.filterState?.value === undefined) - .map(({ name }) => name); + const missingInitialFilters = useMemo( + () => + requiredFirstFilter + .filter(({ id }) => dataMask[id]?.filterState?.value === undefined) + .map(({ name }) => name), + [requiredFirstFilter, dataMask], + ); + const showDashboard = isInitialized || !nativeFiltersEnabled || missingInitialFilters.length === 0; - const toggleDashboardFiltersOpen = useCallback( - (visible?: boolean) => { - setDashboardFiltersOpen(visible ?? !dashboardFiltersOpen); - }, - [dashboardFiltersOpen], - ); + + const toggleDashboardFiltersOpen = useCallback((visible?: boolean) => { + setDashboardFiltersOpen(prevState => visible ?? !prevState); + }, []); useEffect(() => { if ( diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx index 485879e959543..57413ffd23a80 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx +++ b/superset-frontend/src/dashboard/components/FiltersBadge/index.tsx @@ -39,6 +39,7 @@ import { } from '@superset-ui/core'; import Icons from 'src/components/Icons'; import { setDirectPathToChild } from 'src/dashboard/actions/dashboardState'; +import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems'; import Badge from 'src/components/Badge'; import DetailsPanelPopover from './DetailsPanel'; import { @@ -47,7 +48,7 @@ import { selectIndicatorsForChart, selectNativeIndicatorsForChart, } from '../nativeFilters/selectors'; -import { Chart, DashboardLayout, RootState } from '../../types'; +import { Chart, RootState } from '../../types'; export interface FiltersBadgeProps { chartId: number; @@ -126,9 +127,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { state => state.dashboardInfo.metadata?.chart_configuration, ); const chart = useSelector(state => state.charts[chartId]); - const present = useSelector( - state => state.dashboardLayout.present, - ); + const chartLayoutItems = useChartLayoutItems(); const dataMask = useSelector( state => state.dataMask, ); @@ -207,7 +206,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { ]); const prevNativeFilters = usePrevious(nativeFilters); - const prevDashboardLayout = usePrevious(present); + const prevChartLayoutItems = usePrevious(chartLayoutItems); const prevDataMask = usePrevious(dataMask); const prevChartConfig = usePrevious(chartConfiguration); @@ -221,7 +220,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { chart?.queriesResponse?.[0]?.applied_filters !== prevChart?.queriesResponse?.[0]?.applied_filters || nativeFilters !== prevNativeFilters || - present !== prevDashboardLayout || + chartLayoutItems !== prevChartLayoutItems || dataMask !== prevDataMask || prevChartConfig !== chartConfiguration ) { @@ -231,7 +230,7 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { dataMask, chartId, chart, - present, + chartLayoutItems, chartConfiguration, ), ); @@ -244,14 +243,14 @@ export const FiltersBadge = ({ chartId }: FiltersBadgeProps) => { dataMask, nativeFilters, nativeIndicators.length, - present, prevChart?.queriesResponse, prevChartConfig, prevChartStatus, - prevDashboardLayout, prevDataMask, prevNativeFilters, showIndicators, + chartLayoutItems, + prevChartLayoutItems, ]); const indicators = useMemo( diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx index b02c6eee3e059..952f2d56cec5b 100644 --- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx +++ b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx @@ -16,7 +16,14 @@ * specific language governing permissions and limitations * under the License. */ -import { FC, ReactNode, useContext, useEffect, useRef, useState } from 'react'; +import { + forwardRef, + ReactNode, + useContext, + useEffect, + useRef, + useState, +} from 'react'; import { css, getExtensionsRegistry, styled, t } from '@superset-ui/core'; import { useUiConfig } from 'src/components/UiConfigContext'; import { Tooltip } from 'src/components/Tooltip'; @@ -34,7 +41,6 @@ import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage'; const extensionsRegistry = getExtensionsRegistry(); type SliceHeaderProps = SliceHeaderControlsProps & { - innerRef?: string; updateSliceName?: (arg0: string) => void; editMode?: boolean; annotationQuery?: object; @@ -122,176 +128,182 @@ const ChartHeaderStyles = styled.div` `} `; -const SliceHeader: FC = ({ - innerRef = null, - forceRefresh = () => ({}), - updateSliceName = () => ({}), - toggleExpandSlice = () => ({}), - logExploreChart = () => ({}), - logEvent, - exportCSV = () => ({}), - exportXLSX = () => ({}), - editMode = false, - annotationQuery = {}, - annotationError = {}, - cachedDttm = null, - updatedDttm = null, - isCached = [], - isExpanded = false, - sliceName = '', - supersetCanExplore = false, - supersetCanShare = false, - supersetCanCSV = false, - exportPivotCSV, - exportFullCSV, - exportFullXLSX, - slice, - componentId, - dashboardId, - addSuccessToast, - addDangerToast, - handleToggleFullSize, - isFullSize, - chartStatus, - formData, - width, - height, -}) => { - const SliceHeaderExtension = extensionsRegistry.get('dashboard.slice.header'); - const uiConfig = useUiConfig(); - const dashboardPageId = useContext(DashboardPageIdContext); - const [headerTooltip, setHeaderTooltip] = useState(null); - const headerRef = useRef(null); - // TODO: change to indicator field after it will be implemented - const crossFilterValue = useSelector( - state => state.dataMask[slice?.slice_id]?.filterState?.value, - ); - const isCrossFiltersEnabled = useSelector( - ({ dashboardInfo }) => dashboardInfo.crossFiltersEnabled, - ); +const SliceHeader = forwardRef( + ( + { + forceRefresh = () => ({}), + updateSliceName = () => ({}), + toggleExpandSlice = () => ({}), + logExploreChart = () => ({}), + logEvent, + exportCSV = () => ({}), + exportXLSX = () => ({}), + editMode = false, + annotationQuery = {}, + annotationError = {}, + cachedDttm = null, + updatedDttm = null, + isCached = [], + isExpanded = false, + sliceName = '', + supersetCanExplore = false, + supersetCanShare = false, + supersetCanCSV = false, + exportPivotCSV, + exportFullCSV, + exportFullXLSX, + slice, + componentId, + dashboardId, + addSuccessToast, + addDangerToast, + handleToggleFullSize, + isFullSize, + chartStatus, + formData, + width, + height, + }, + ref, + ) => { + const SliceHeaderExtension = extensionsRegistry.get( + 'dashboard.slice.header', + ); + const uiConfig = useUiConfig(); + const dashboardPageId = useContext(DashboardPageIdContext); + const [headerTooltip, setHeaderTooltip] = useState(null); + const headerRef = useRef(null); + // TODO: change to indicator field after it will be implemented + const crossFilterValue = useSelector( + state => state.dataMask[slice?.slice_id]?.filterState?.value, + ); + const isCrossFiltersEnabled = useSelector( + ({ dashboardInfo }) => dashboardInfo.crossFiltersEnabled, + ); - const canExplore = !editMode && supersetCanExplore; + const canExplore = !editMode && supersetCanExplore; - useEffect(() => { - const headerElement = headerRef.current; - if (canExplore) { - setHeaderTooltip(getSliceHeaderTooltip(sliceName)); - } else if ( - headerElement && - (headerElement.scrollWidth > headerElement.offsetWidth || - headerElement.scrollHeight > headerElement.offsetHeight) - ) { - setHeaderTooltip(sliceName ?? null); - } else { - setHeaderTooltip(null); - } - }, [sliceName, width, height, canExplore]); + useEffect(() => { + const headerElement = headerRef.current; + if (canExplore) { + setHeaderTooltip(getSliceHeaderTooltip(sliceName)); + } else if ( + headerElement && + (headerElement.scrollWidth > headerElement.offsetWidth || + headerElement.scrollHeight > headerElement.offsetHeight) + ) { + setHeaderTooltip(sliceName ?? null); + } else { + setHeaderTooltip(null); + } + }, [sliceName, width, height, canExplore]); - const exploreUrl = `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${slice.slice_id}`; + const exploreUrl = `/explore/?dashboard_page_id=${dashboardPageId}&slice_id=${slice.slice_id}`; - return ( - -
- - - - {!!Object.values(annotationQuery).length && ( - - +
+ + - )} - {!!Object.values(annotationError).length && ( - - - - )} -
-
- {!editMode && ( - <> - {SliceHeaderExtension && ( - + - )} - {crossFilterValue && ( - - - - )} - {!uiConfig.hideChartControls && ( - - )} - {!uiConfig.hideChartControls && ( - + )} + {!!Object.values(annotationError).length && ( + + - )} - - )} -
- - ); -}; + + )} +
+
+ {!editMode && ( + <> + {SliceHeaderExtension && ( + + )} + {crossFilterValue && ( + + + + )} + {!uiConfig.hideChartControls && ( + + )} + {!uiConfig.hideChartControls && ( + + )} + + )} +
+ + ); + }, +); export default SliceHeader; diff --git a/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx b/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx index fab9b9672d2c8..59199ec278a9d 100644 --- a/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx +++ b/superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx @@ -18,8 +18,9 @@ */ import { FC, useEffect } from 'react'; -import { pick } from 'lodash'; -import { shallowEqual, useSelector } from 'react-redux'; +import { pick, pickBy } from 'lodash'; +import { useSelector } from 'react-redux'; +import { createSelector } from '@reduxjs/toolkit'; import { DashboardContextForExplore } from 'src/types/DashboardContextForExplore'; import { getItem, @@ -42,11 +43,7 @@ export const getDashboardContextLocalStorage = () => { // 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, - ), - ); + return pickBy(dashboardsContexts, value => !value.isRedundant); }; const updateDashboardTabLocalStorage = ( @@ -56,38 +53,45 @@ const updateDashboardTabLocalStorage = ( const dashboardsContexts = getDashboardContextLocalStorage(); setItem(LocalStorageKeys.DashboardExploreContext, { ...dashboardsContexts, - [dashboardPageId]: dashboardContext, + [dashboardPageId]: { ...dashboardContext, dashboardPageId }, }); }; -const SyncDashboardState: FC = ({ dashboardPageId }) => { - const dashboardContextForExplore = useSelector< - RootState, - DashboardContextForExplore - >( - ({ dashboardInfo, dashboardState, nativeFilters, dataMask }) => ({ - labelsColor: dashboardInfo.metadata?.label_colors || EMPTY_OBJECT, - labelsColorMap: dashboardInfo.metadata?.map_label_colors || EMPTY_OBJECT, +const selectDashboardContextForExplore = createSelector( + [ + (state: RootState) => state.dashboardInfo.metadata, + (state: RootState) => state.dashboardInfo.id, + (state: RootState) => state.dashboardState?.colorScheme, + (state: RootState) => state.nativeFilters?.filters, + (state: RootState) => state.dataMask, + ], + (metadata, dashboardId, colorScheme, filters, dataMask) => { + const nativeFilters = Object.keys(filters).reduce((acc, key) => { + acc[key] = pick(filters[key], ['chartsInScope']); + return acc; + }, {}); + + return { + labelsColor: metadata?.label_colors || EMPTY_OBJECT, + labelsColorMap: metadata?.map_label_colors || EMPTY_OBJECT, sharedLabelsColors: enforceSharedLabelsColorsArray( - dashboardInfo.metadata?.shared_label_colors, - ), - colorScheme: dashboardState?.colorScheme, - chartConfiguration: - dashboardInfo.metadata?.chart_configuration || EMPTY_OBJECT, - nativeFilters: Object.entries(nativeFilters.filters).reduce( - (acc, [key, filterValue]) => ({ - ...acc, - [key]: pick(filterValue, ['chartsInScope']), - }), - {}, + metadata?.shared_label_colors, ), + colorScheme, + chartConfiguration: metadata?.chart_configuration || EMPTY_OBJECT, + nativeFilters, dataMask, - dashboardId: dashboardInfo.id, + dashboardId, filterBoxFilters: getActiveFilters(), - dashboardPageId, - }), - shallowEqual, - ); + }; + }, +); + +const SyncDashboardState: FC = ({ dashboardPageId }) => { + const dashboardContextForExplore = useSelector< + RootState, + DashboardContextForExplore + >(selectDashboardContextForExplore); useEffect(() => { updateDashboardTabLocalStorage(dashboardPageId, dashboardContextForExplore); diff --git a/superset-frontend/src/dashboard/components/URLShortLinkButton/index.tsx b/superset-frontend/src/dashboard/components/URLShortLinkButton/index.tsx index 849b1cd44478e..f227a9e845b23 100644 --- a/superset-frontend/src/dashboard/components/URLShortLinkButton/index.tsx +++ b/superset-frontend/src/dashboard/components/URLShortLinkButton/index.tsx @@ -22,7 +22,7 @@ import Popover, { PopoverProps } from 'src/components/Popover'; import CopyToClipboard from 'src/components/CopyToClipboard'; import { getDashboardPermalink } from 'src/utils/urlUtils'; import { useToasts } from 'src/components/MessageToasts/withToasts'; -import { useSelector } from 'react-redux'; +import { shallowEqual, useSelector } from 'react-redux'; import { RootState } from 'src/dashboard/types'; export type URLShortLinkButtonProps = { @@ -42,10 +42,13 @@ export default function URLShortLinkButton({ }: URLShortLinkButtonProps) { const [shortUrl, setShortUrl] = useState(''); const { addDangerToast } = useToasts(); - const { dataMask, activeTabs } = useSelector((state: RootState) => ({ - dataMask: state.dataMask, - activeTabs: state.dashboardState.activeTabs, - })); + const { dataMask, activeTabs } = useSelector( + (state: RootState) => ({ + dataMask: state.dataMask, + activeTabs: state.dashboardState.activeTabs, + }), + shallowEqual, + ); const getCopyUrl = async () => { try { diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index d125ccb626de3..5cc8ba6d37f61 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx @@ -17,11 +17,13 @@ * under the License. */ import cx from 'classnames'; -import { Component } from 'react'; +import { useCallback, useEffect, useRef, useMemo, useState, memo } from 'react'; import PropTypes from 'prop-types'; import { styled, t, logging } from '@superset-ui/core'; -import { debounce, isEqual } from 'lodash'; -import { withRouter } from 'react-router-dom'; +import { debounce } from 'lodash'; +import { useHistory } from 'react-router-dom'; +import { bindActionCreators } from 'redux'; +import { useDispatch, useSelector } from 'react-redux'; import { exportChart, mountExploreUrl } from 'src/explore/exploreUtils'; import ChartContainer from 'src/components/Chart/ChartContainer'; @@ -32,13 +34,30 @@ import { LOG_ACTIONS_EXPORT_XLSX_DASHBOARD_CHART, LOG_ACTIONS_FORCE_REFRESH_CHART, } from 'src/logger/LogUtils'; -import { areObjectsEqual } from 'src/reduxUtils'; import { postFormData } from 'src/explore/exploreUtils/formData'; import { URL_PARAMS } from 'src/constants'; +import { enforceSharedLabelsColorsArray } from 'src/utils/colorScheme'; import SliceHeader from '../SliceHeader'; import MissingChart from '../MissingChart'; -import { slicePropShape, chartPropShape } from '../../util/propShapes'; +import { + addDangerToast, + addSuccessToast, +} from '../../../components/MessageToasts/actions'; +import { + setFocusedFilterField, + toggleExpandSlice, + unsetFocusedFilterField, +} from '../../actions/dashboardState'; +import { changeFilter } from '../../actions/dashboardFilters'; +import { refreshChart } from '../../../components/Chart/chartAction'; +import { logEvent } from '../../../logger/actions'; +import { + getActiveFilters, + getAppliedFilterValues, +} from '../../util/activeDashboardFilters'; +import getFormDataWithExtraFilters from '../../util/charts/getFormDataWithExtraFilters'; +import { PLACEHOLDER_DATASOURCE } from '../../constants'; const propTypes = { id: PropTypes.number.isRequired, @@ -50,53 +69,15 @@ const propTypes = { isComponentVisible: PropTypes.bool, handleToggleFullSize: PropTypes.func.isRequired, setControlValue: PropTypes.func, - - // from redux - chart: chartPropShape.isRequired, - formData: PropTypes.object.isRequired, - labelsColor: PropTypes.object, - labelsColorMap: PropTypes.object, - datasource: PropTypes.object, - slice: slicePropShape.isRequired, sliceName: PropTypes.string.isRequired, - timeout: PropTypes.number.isRequired, - maxRows: PropTypes.number.isRequired, - // all active filter fields in dashboard - filters: PropTypes.object.isRequired, - refreshChart: PropTypes.func.isRequired, - logEvent: PropTypes.func.isRequired, - toggleExpandSlice: PropTypes.func.isRequired, - changeFilter: PropTypes.func.isRequired, - setFocusedFilterField: PropTypes.func.isRequired, - unsetFocusedFilterField: PropTypes.func.isRequired, - editMode: PropTypes.bool.isRequired, - isExpanded: PropTypes.bool.isRequired, - isCached: PropTypes.bool, - supersetCanExplore: PropTypes.bool.isRequired, - supersetCanShare: PropTypes.bool.isRequired, - supersetCanCSV: PropTypes.bool.isRequired, - addSuccessToast: PropTypes.func.isRequired, - addDangerToast: PropTypes.func.isRequired, - ownState: PropTypes.object, - filterState: PropTypes.object, - postTransformProps: PropTypes.func, - datasetsStatus: PropTypes.oneOf(['loading', 'error', 'complete']), + isFullSize: PropTypes.bool, + extraControls: PropTypes.object, isInView: PropTypes.bool, - emitCrossFilters: PropTypes.bool, -}; - -const defaultProps = { - isCached: false, - isComponentVisible: true, }; // we use state + shouldComponentUpdate() logic to prevent perf-wrecking // resizing across all slices on a dashboard on every update const RESIZE_TIMEOUT = 500; -const SHOULD_UPDATE_ON_PROP_CHANGES = Object.keys(propTypes).filter( - prop => - prop !== 'width' && prop !== 'height' && prop !== 'isComponentVisible', -); const DEFAULT_HEADER_HEIGHT = 22; const ChartWrapper = styled.div` @@ -121,429 +102,457 @@ const SliceContainer = styled.div` max-height: 100%; `; -class Chart extends Component { - constructor(props) { - super(props); - this.state = { - width: props.width, - height: props.height, - descriptionHeight: 0, - }; - - this.changeFilter = this.changeFilter.bind(this); - this.handleFilterMenuOpen = this.handleFilterMenuOpen.bind(this); - this.handleFilterMenuClose = this.handleFilterMenuClose.bind(this); - this.exportCSV = this.exportCSV.bind(this); - this.exportPivotCSV = this.exportPivotCSV.bind(this); - this.exportFullCSV = this.exportFullCSV.bind(this); - this.exportXLSX = this.exportXLSX.bind(this); - this.exportFullXLSX = this.exportFullXLSX.bind(this); - this.forceRefresh = this.forceRefresh.bind(this); - this.resize = debounce(this.resize.bind(this), RESIZE_TIMEOUT); - this.setDescriptionRef = this.setDescriptionRef.bind(this); - this.setHeaderRef = this.setHeaderRef.bind(this); - this.getChartHeight = this.getChartHeight.bind(this); - this.getDescriptionHeight = this.getDescriptionHeight.bind(this); - } - - shouldComponentUpdate(nextProps, nextState) { - // this logic mostly pertains to chart resizing. we keep a copy of the dimensions in - // state so that we can buffer component size updates and only update on the final call - // which improves performance significantly - if ( - nextState.width !== this.state.width || - nextState.height !== this.state.height || - nextState.descriptionHeight !== this.state.descriptionHeight || - !isEqual(nextProps.datasource, this.props.datasource) - ) { - return true; - } - - // allow chart to update if the status changed and the previous status was loading. - if ( - this.props?.chart?.chartStatus !== nextProps?.chart?.chartStatus && - this.props?.chart?.chartStatus === 'loading' - ) { - return true; - } - - // allow chart update/re-render only if visible: - // under selected tab or no tab layout - if (nextProps.isComponentVisible) { - if (nextProps.chart.triggerQuery) { - return true; - } - - if (nextProps.isFullSize !== this.props.isFullSize) { - this.resize(); - return false; - } - - if ( - nextProps.width !== this.props.width || - nextProps.height !== this.props.height || - nextProps.width !== this.state.width || - nextProps.height !== this.state.height - ) { - this.resize(); - } - - for (let i = 0; i < SHOULD_UPDATE_ON_PROP_CHANGES.length; i += 1) { - const prop = SHOULD_UPDATE_ON_PROP_CHANGES[i]; - // use deep objects equality comparison to prevent - // unnecessary updates when objects references change - if (!areObjectsEqual(nextProps[prop], this.props[prop])) { - return true; - } - } - } else if ( - // chart should re-render if color scheme or label colors were changed - nextProps.formData?.color_scheme !== this.props.formData?.color_scheme || - !areObjectsEqual( - nextProps.formData?.label_colors || {}, - this.props.formData?.label_colors || {}, - ) || - !areObjectsEqual( - nextProps.formData?.map_label_colors || {}, - this.props.formData?.map_label_colors || {}, - ) || - !isEqual( - nextProps.formData?.shared_label_colors || [], - this.props.formData?.shared_label_colors || [], - ) - ) { - return true; - } - - // `cacheBusterProp` is injected by react-hot-loader - return this.props.cacheBusterProp !== nextProps.cacheBusterProp; - } - - componentDidMount() { - if (this.props.isExpanded) { - const descriptionHeight = this.getDescriptionHeight(); - this.setState({ descriptionHeight }); - } - } - - componentWillUnmount() { - this.resize.cancel(); - } - - componentDidUpdate(prevProps) { - if (this.props.isExpanded !== prevProps.isExpanded) { - const descriptionHeight = this.getDescriptionHeight(); - // eslint-disable-next-line react/no-did-update-set-state - this.setState({ descriptionHeight }); +const EMPTY_OBJECT = {}; + +const Chart = props => { + const dispatch = useDispatch(); + const descriptionRef = useRef(null); + const headerRef = useRef(null); + + const boundActionCreators = useMemo( + () => + bindActionCreators( + { + addSuccessToast, + addDangerToast, + toggleExpandSlice, + changeFilter, + setFocusedFilterField, + unsetFocusedFilterField, + refreshChart, + logEvent, + }, + dispatch, + ), + [dispatch], + ); + + const chart = useSelector(state => state.charts[props.id] || EMPTY_OBJECT); + const slice = useSelector( + state => state.sliceEntities.slices[props.id] || EMPTY_OBJECT, + ); + const editMode = useSelector(state => state.dashboardState.editMode); + const isExpanded = useSelector( + state => !!state.dashboardState.expandedSlices[props.id], + ); + const supersetCanExplore = useSelector( + state => !!state.dashboardInfo.superset_can_explore, + ); + const supersetCanShare = useSelector( + state => !!state.dashboardInfo.superset_can_share, + ); + const supersetCanCSV = useSelector( + state => !!state.dashboardInfo.superset_can_csv, + ); + const timeout = useSelector( + state => state.dashboardInfo.common.conf.SUPERSET_WEBSERVER_TIMEOUT, + ); + const emitCrossFilters = useSelector( + state => !!state.dashboardInfo.crossFiltersEnabled, + ); + const datasource = useSelector( + state => + (chart && + chart.form_data && + state.datasources[chart.form_data.datasource]) || + PLACEHOLDER_DATASOURCE, + ); + + const [descriptionHeight, setDescriptionHeight] = useState(0); + const [height, setHeight] = useState(props.height); + const [width, setWidth] = useState(props.width); + const history = useHistory(); + const resize = useCallback( + debounce(() => { + const { width, height } = props; + setHeight(height); + setWidth(width); + }, RESIZE_TIMEOUT), + [props.width, props.height], + ); + + const ownColorScheme = chart.form_data?.color_scheme; + + const addFilter = useCallback( + (newSelectedValues = {}) => { + boundActionCreators.logEvent(LOG_ACTIONS_CHANGE_DASHBOARD_FILTER, { + id: chart.id, + columns: Object.keys(newSelectedValues).filter( + key => newSelectedValues[key] !== null, + ), + }); + boundActionCreators.changeFilter(chart.id, newSelectedValues); + }, + [boundActionCreators.logEvent, boundActionCreators.changeFilter, chart.id], + ); + + useEffect(() => { + if (isExpanded) { + const descriptionHeight = + isExpanded && descriptionRef.current + ? descriptionRef.current?.offsetHeight + : 0; + setDescriptionHeight(descriptionHeight); } - } - - getDescriptionHeight() { - return this.props.isExpanded && this.descriptionRef - ? this.descriptionRef.offsetHeight - : 0; - } - - getChartHeight() { - const headerHeight = this.getHeaderHeight(); - return Math.max( - this.state.height - headerHeight - this.state.descriptionHeight, - 20, - ); - } - - getHeaderHeight() { - if (this.headerRef) { - const computedStyle = getComputedStyle(this.headerRef).getPropertyValue( - 'margin-bottom', - ); + }, [isExpanded]); + + useEffect( + () => () => { + resize.cancel(); + }, + [resize], + ); + + useEffect(() => { + resize(); + }, [resize, props.isFullSize]); + + const getHeaderHeight = useCallback(() => { + if (headerRef.current) { + const computedStyle = getComputedStyle( + headerRef.current, + ).getPropertyValue('margin-bottom'); const marginBottom = parseInt(computedStyle, 10) || 0; - return this.headerRef.offsetHeight + marginBottom; + return headerRef.current.offsetHeight + marginBottom; } return DEFAULT_HEADER_HEIGHT; - } - - setDescriptionRef(ref) { - this.descriptionRef = ref; - } - - setHeaderRef(ref) { - this.headerRef = ref; - } - - resize() { - const { width, height } = this.props; - this.setState(() => ({ width, height })); - } - - changeFilter(newSelectedValues = {}) { - this.props.logEvent(LOG_ACTIONS_CHANGE_DASHBOARD_FILTER, { - id: this.props.chart.id, - columns: Object.keys(newSelectedValues).filter( - key => newSelectedValues[key] !== null, - ), + }, [headerRef]); + + const getChartHeight = useCallback(() => { + const headerHeight = getHeaderHeight(); + return Math.max(height - headerHeight - descriptionHeight, 20); + }, [getHeaderHeight, height, descriptionHeight]); + + const handleFilterMenuOpen = useCallback( + (chartId, column) => { + boundActionCreators.setFocusedFilterField(chartId, column); + }, + [boundActionCreators.setFocusedFilterField], + ); + + const handleFilterMenuClose = useCallback( + (chartId, column) => { + boundActionCreators.unsetFocusedFilterField(chartId, column); + }, + [boundActionCreators.unsetFocusedFilterField], + ); + + const logExploreChart = useCallback(() => { + boundActionCreators.logEvent(LOG_ACTIONS_EXPLORE_DASHBOARD_CHART, { + slice_id: slice.slice_id, + is_cached: props.isCached, }); - this.props.changeFilter(this.props.chart.id, newSelectedValues); - } - - handleFilterMenuOpen(chartId, column) { - this.props.setFocusedFilterField(chartId, column); - } - - handleFilterMenuClose(chartId, column) { - this.props.unsetFocusedFilterField(chartId, column); - } - - logExploreChart = () => { - this.props.logEvent(LOG_ACTIONS_EXPLORE_DASHBOARD_CHART, { - slice_id: this.props.slice.slice_id, - is_cached: this.props.isCached, - }); - }; - - onExploreChart = async clickEvent => { - const isOpenInNewTab = - clickEvent.shiftKey || clickEvent.ctrlKey || clickEvent.metaKey; - try { - const lastTabId = window.localStorage.getItem('last_tab_id'); - const nextTabId = lastTabId - ? String(Number.parseInt(lastTabId, 10) + 1) - : undefined; - const key = await postFormData( - this.props.datasource.id, - this.props.datasource.type, - this.props.formData, - this.props.slice.slice_id, - nextTabId, - ); - const url = mountExploreUrl(null, { - [URL_PARAMS.formDataKey.name]: key, - [URL_PARAMS.sliceId.name]: this.props.slice.slice_id, - }); - if (isOpenInNewTab) { - window.open(url, '_blank', 'noreferrer'); - } else { - this.props.history.push(url); - } - } catch (error) { - logging.error(error); - this.props.addDangerToast(t('An error occurred while opening Explore')); - } - }; - - exportFullCSV() { - this.exportCSV(true); - } - - exportCSV(isFullCSV = false) { - this.exportTable('csv', isFullCSV); - } - - exportPivotCSV() { - this.exportTable('csv', false, true); - } - - exportXLSX() { - this.exportTable('xlsx', false); - } - - exportFullXLSX() { - this.exportTable('xlsx', true); - } - - exportTable(format, isFullCSV, isPivot = false) { - const logAction = - format === 'csv' - ? LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART - : LOG_ACTIONS_EXPORT_XLSX_DASHBOARD_CHART; - this.props.logEvent(logAction, { - slice_id: this.props.slice.slice_id, - is_cached: this.props.isCached, - }); - exportChart({ - formData: isFullCSV - ? { ...this.props.formData, row_limit: this.props.maxRows } - : this.props.formData, - resultType: isPivot ? 'post_processed' : 'full', - resultFormat: format, - force: true, - ownState: this.props.ownState, - }); - } - - forceRefresh() { - this.props.logEvent(LOG_ACTIONS_FORCE_REFRESH_CHART, { - slice_id: this.props.slice.slice_id, - is_cached: this.props.isCached, - }); - return this.props.refreshChart( - this.props.chart.id, - true, - this.props.dashboardId, - ); - } - - render() { - const { - id, - componentId, - dashboardId, + }, [boundActionCreators.logEvent, slice.slice_id, props.isCached]); + + const chartConfiguration = useSelector( + state => state.dashboardInfo.metadata?.chart_configuration, + ); + const colorScheme = useSelector(state => state.dashboardState.colorScheme); + const colorNamespace = useSelector( + state => state.dashboardState.colorNamespace, + ); + const datasetsStatus = useSelector( + state => state.dashboardState.datasetsStatus, + ); + const allSliceIds = useSelector(state => state.dashboardState.sliceIds); + const nativeFilters = useSelector(state => state.nativeFilters?.filters); + const dataMask = useSelector(state => state.dataMask); + const labelsColor = useSelector( + state => state.dashboardInfo?.metadata?.label_colors || EMPTY_OBJECT, + ); + const labelsColorMap = useSelector( + state => state.dashboardInfo?.metadata?.map_label_colors || EMPTY_OBJECT, + ); + const sharedLabelsColors = useSelector(state => + enforceSharedLabelsColorsArray( + state.dashboardInfo?.metadata?.shared_label_colors, + ), + ); + + const formData = useMemo( + () => + getFormDataWithExtraFilters({ + chart, + chartConfiguration, + filters: getAppliedFilterValues(props.id), + colorScheme, + colorNamespace, + sliceId: props.id, + nativeFilters, + allSliceIds, + dataMask, + extraControls: props.extraControls, + labelsColor, + labelsColorMap, + sharedLabelsColors, + ownColorScheme, + }), + [ chart, - slice, - datasource, - isExpanded, - editMode, - filters, - formData, + chartConfiguration, + props.id, + props.extraControls, + colorScheme, + colorNamespace, + nativeFilters, + allSliceIds, + dataMask, labelsColor, labelsColorMap, - updateSliceName, - sliceName, - toggleExpandSlice, - timeout, - supersetCanExplore, - supersetCanShare, - supersetCanCSV, - addSuccessToast, - addDangerToast, - ownState, - filterState, - handleToggleFullSize, - isFullSize, - setControlValue, - postTransformProps, - datasetsStatus, - isInView, - emitCrossFilters, - logEvent, - } = this.props; - - const { width } = this.state; - // this prevents throwing in the case that a gridComponent - // references a chart that is not associated with the dashboard - if (!chart || !slice) { - return ; - } + sharedLabelsColors, + ownColorScheme, + ], + ); + + const onExploreChart = useCallback( + async clickEvent => { + const isOpenInNewTab = + clickEvent.shiftKey || clickEvent.ctrlKey || clickEvent.metaKey; + try { + const lastTabId = window.localStorage.getItem('last_tab_id'); + const nextTabId = lastTabId + ? String(Number.parseInt(lastTabId, 10) + 1) + : undefined; + const key = await postFormData( + datasource.id, + datasource.type, + formData, + slice.slice_id, + nextTabId, + ); + const url = mountExploreUrl(null, { + [URL_PARAMS.formDataKey.name]: key, + [URL_PARAMS.sliceId.name]: slice.slice_id, + }); + if (isOpenInNewTab) { + window.open(url, '_blank', 'noreferrer'); + } else { + history.push(url); + } + } catch (error) { + logging.error(error); + boundActionCreators.addDangerToast( + t('An error occurred while opening Explore'), + ); + } + }, + [ + datasource.id, + datasource.type, + formData, + slice.slice_id, + boundActionCreators.addDangerToast, + history, + ], + ); + + const exportTable = useCallback( + (format, isFullCSV, isPivot = false) => { + const logAction = + format === 'csv' + ? LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART + : LOG_ACTIONS_EXPORT_XLSX_DASHBOARD_CHART; + boundActionCreators.logEvent(logAction, { + slice_id: slice.slice_id, + is_cached: props.isCached, + }); + exportChart({ + formData: isFullCSV + ? { ...formData, row_limit: props.maxRows } + : formData, + resultType: isPivot ? 'post_processed' : 'full', + resultFormat: format, + force: true, + ownState: props.ownState, + }); + }, + [ + slice.slice_id, + props.isCached, + formData, + props.maxRows, + props.ownState, + boundActionCreators.logEvent, + ], + ); + + const exportCSV = useCallback( + (isFullCSV = false) => { + exportTable('csv', isFullCSV); + }, + [exportTable], + ); + + const exportFullCSV = useCallback(() => { + exportCSV(true); + }, [exportCSV]); + + const exportPivotCSV = useCallback(() => { + exportTable('csv', false, true); + }, [exportTable]); + + const exportXLSX = useCallback(() => { + exportTable('xlsx', false); + }, [exportTable]); + + const exportFullXLSX = useCallback(() => { + exportTable('xlsx', true); + }, [exportTable]); + + const forceRefresh = useCallback(() => { + boundActionCreators.logEvent(LOG_ACTIONS_FORCE_REFRESH_CHART, { + slice_id: slice.slice_id, + is_cached: props.isCached, + }); + return boundActionCreators.refreshChart(chart.id, true, props.dashboardId); + }, [ + boundActionCreators.refreshChart, + chart.id, + props.dashboardId, + slice.slice_id, + props.isCached, + boundActionCreators.logEvent, + ]); + + if (chart === EMPTY_OBJECT || slice === EMPTY_OBJECT) { + return ; + } - const { queriesResponse, chartUpdateEndTime, chartStatus } = chart; - const isLoading = chartStatus === 'loading'; + const { queriesResponse, chartUpdateEndTime, chartStatus, annotationQuery } = + chart; + const isLoading = chartStatus === 'loading'; + // eslint-disable-next-line camelcase + const isCached = queriesResponse?.map(({ is_cached }) => is_cached) || []; + const cachedDttm = // eslint-disable-next-line camelcase - const isCached = queriesResponse?.map(({ is_cached }) => is_cached) || []; - const cachedDttm = - // eslint-disable-next-line camelcase - queriesResponse?.map(({ cached_dttm }) => cached_dttm) || []; - const initialValues = {}; - - return ( - - - - {/* + queriesResponse?.map(({ cached_dttm }) => cached_dttm) || []; + + return ( + + + + {/* This usage of dangerouslySetInnerHTML is safe since it is being used to render markdown that is sanitized with nh3. See: https://github.com/apache/superset/pull/4390 and https://github.com/apache/superset/pull/23862 */} - {isExpanded && slice.description_markeddown && ( -
+ )} + + + {isLoading && ( + )} - - {isLoading && ( - - )} - - - - - ); - } -} + + + + ); +}; Chart.propTypes = propTypes; -Chart.defaultProps = defaultProps; -export default withRouter(Chart); +export default memo(Chart, (prevProps, nextProps) => { + if (prevProps.cacheBusterProp !== nextProps.cacheBusterProp) { + return false; + } + return ( + !nextProps.isComponentVisible || + (prevProps.isInView === nextProps.isInView && + prevProps.componentId === nextProps.componentId && + prevProps.id === nextProps.id && + prevProps.dashboardId === nextProps.dashboardId && + prevProps.extraControls === nextProps.extraControls && + prevProps.handleToggleFullSize === nextProps.handleToggleFullSize && + prevProps.isFullSize === nextProps.isFullSize && + prevProps.setControlValue === nextProps.setControlValue && + prevProps.sliceName === nextProps.sliceName && + prevProps.updateSliceName === nextProps.updateSliceName && + prevProps.width === nextProps.width && + prevProps.height === nextProps.height) + ); +}); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx index 9efbb8ae3d08d..1b38f0e75fd3a 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.test.jsx @@ -18,6 +18,7 @@ */ import { fireEvent, render } from 'spec/helpers/testing-library'; import { FeatureFlag } from '@superset-ui/core'; +import * as redux from 'redux'; import Chart from 'src/dashboard/components/gridComponents/Chart'; import * as exploreUtils from 'src/explore/exploreUtils'; @@ -32,18 +33,10 @@ const props = { width: 100, height: 100, updateSliceName() {}, - // from redux maxRows: 666, - chart: chartQueries[queryId], formData: chartQueries[queryId].form_data, datasource: mockDatasource[sliceEntities.slices[queryId].datasource], - slice: { - ...sliceEntities.slices[queryId], - description_markeddown: 'markdown', - owners: [], - viz_type: 'table', - }, sliceName: sliceEntities.slices[queryId].slice_name, timeout: 60, filters: {}, @@ -63,20 +56,60 @@ const props = { exportFullXLSX() {}, componentId: 'test', dashboardId: 111, - editMode: false, - isExpanded: false, - supersetCanExplore: false, - supersetCanCSV: false, - supersetCanShare: false, }; -function setup(overrideProps) { - return render(, { +const defaultState = { + charts: chartQueries, + sliceEntities: { + ...sliceEntities, + slices: { + [queryId]: { + ...sliceEntities.slices[queryId], + description_markeddown: 'markdown', + owners: [], + viz_type: 'table', + }, + }, + }, + datasources: mockDatasource, + dashboardState: { editMode: false, expandedSlices: {} }, + dashboardInfo: { + superset_can_explore: false, + superset_can_share: false, + superset_can_csv: false, + common: { conf: { SUPERSET_WEBSERVER_TIMEOUT: 0 } }, + }, +}; + +function setup(overrideProps, overrideState) { + return render(, { useRedux: true, useRouter: true, + initialState: { ...defaultState, ...overrideState }, }); } +const refreshChart = jest.fn(); +const logEvent = jest.fn(); +const changeFilter = jest.fn(); +const addSuccessToast = jest.fn(); +const addDangerToast = jest.fn(); +const toggleExpandSlice = jest.fn(); +const setFocusedFilterField = jest.fn(); +const unsetFocusedFilterField = jest.fn(); +beforeAll(() => { + jest.spyOn(redux, 'bindActionCreators').mockImplementation(() => ({ + refreshChart, + logEvent, + changeFilter, + addSuccessToast, + addDangerToast, + toggleExpandSlice, + setFocusedFilterField, + unsetFocusedFilterField, + })); +}); + test('should render a SliceHeader', () => { const { getByTestId, container } = setup(); expect(getByTestId('slice-header')).toBeInTheDocument(); @@ -89,23 +122,20 @@ test('should render a ChartContainer', () => { }); test('should render a description if it has one and isExpanded=true', () => { - const { container } = setup({ isExpanded: true }); - expect(container.querySelector('.slice_description')).toBeInTheDocument(); -}); - -test('should calculate the description height if it has one and isExpanded=true', () => { - const spy = jest.spyOn( - Chart.WrappedComponent.prototype, - 'getDescriptionHeight', + const { container } = setup( + {}, + { + dashboardState: { + ...defaultState.dashboardState, + expandedSlices: { [props.id]: true }, + }, + }, ); - const { container } = setup({ isExpanded: true }); expect(container.querySelector('.slice_description')).toBeInTheDocument(); - expect(spy).toHaveBeenCalled(); }); test('should call refreshChart when SliceHeader calls forceRefresh', () => { - const refreshChart = jest.fn(); - const { getByText, getByRole } = setup({ refreshChart }); + const { getByText, getByRole } = setup({}); fireEvent.click(getByRole('button', { name: 'More Options' })); fireEvent.click(getByText('Force refresh')); expect(refreshChart).toHaveBeenCalled(); @@ -122,7 +152,12 @@ test('should call exportChart when exportCSV is clicked', async () => { const stubbedExportCSV = jest .spyOn(exploreUtils, 'exportChart') .mockImplementation(() => {}); - const { findByText, getByRole } = setup({ supersetCanCSV: true }); + const { findByText, getByRole } = setup( + {}, + { + dashboardInfo: { ...defaultState.dashboardInfo, superset_can_csv: true }, + }, + ); fireEvent.click(getByRole('button', { name: 'More Options' })); fireEvent.mouseOver(getByRole('button', { name: 'Download right' })); const exportAction = await findByText('Export to .CSV'); @@ -145,7 +180,12 @@ test('should call exportChart with row_limit props.maxRows when exportFullCSV is const stubbedExportCSV = jest .spyOn(exploreUtils, 'exportChart') .mockImplementation(() => {}); - const { findByText, getByRole } = setup({ supersetCanCSV: true }); + const { findByText, getByRole } = setup( + {}, + { + dashboardInfo: { ...defaultState.dashboardInfo, superset_can_csv: true }, + }, + ); fireEvent.click(getByRole('button', { name: 'More Options' })); fireEvent.mouseOver(getByRole('button', { name: 'Download right' })); const exportAction = await findByText('Export to full .CSV'); @@ -167,7 +207,12 @@ test('should call exportChart when exportXLSX is clicked', async () => { const stubbedExportXLSX = jest .spyOn(exploreUtils, 'exportChart') .mockImplementation(() => {}); - const { findByText, getByRole } = setup({ supersetCanCSV: true }); + const { findByText, getByRole } = setup( + {}, + { + dashboardInfo: { ...defaultState.dashboardInfo, superset_can_csv: true }, + }, + ); fireEvent.click(getByRole('button', { name: 'More Options' })); fireEvent.mouseOver(getByRole('button', { name: 'Download right' })); const exportAction = await findByText('Export to Excel'); @@ -189,7 +234,12 @@ test('should call exportChart with row_limit props.maxRows when exportFullXLSX i const stubbedExportXLSX = jest .spyOn(exploreUtils, 'exportChart') .mockImplementation(() => {}); - const { findByText, getByRole } = setup({ supersetCanCSV: true }); + const { findByText, getByRole } = setup( + {}, + { + dashboardInfo: { ...defaultState.dashboardInfo, superset_can_csv: true }, + }, + ); fireEvent.click(getByRole('button', { name: 'More Options' })); fireEvent.mouseOver(getByRole('button', { name: 'Download right' })); const exportAction = await findByText('Export to full Excel'); diff --git a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx index d31e240e4f70d..887f1baa73f58 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useState, useMemo, useCallback, useEffect } from 'react'; +import { useState, useMemo, useCallback, useEffect, memo } from 'react'; import { ResizeCallback, ResizeStartCallback } from 're-resizable'; import cx from 'classnames'; @@ -24,7 +24,7 @@ import { useSelector } from 'react-redux'; import { css, useTheme } from '@superset-ui/core'; import { LayoutItem, RootState } from 'src/dashboard/types'; import AnchorLink from 'src/dashboard/components/AnchorLink'; -import Chart from 'src/dashboard/containers/Chart'; +import Chart from 'src/dashboard/components/gridComponents/Chart'; import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton'; import { Draggable } from 'src/dashboard/components/dnd/DragDroppable'; import HoverMenu from 'src/dashboard/components/menu/HoverMenu'; @@ -70,7 +70,7 @@ interface ChartHolderProps { isInView: boolean; } -const ChartHolder: React.FC = ({ +const ChartHolder = ({ id, parentId, component, @@ -92,7 +92,7 @@ const ChartHolder: React.FC = ({ handleComponentDrop, setFullSizeChartId, isInView, -}) => { +}: ChartHolderProps) => { const theme = useTheme(); const fullSizeStyle = css` && { @@ -107,9 +107,13 @@ const ChartHolder: React.FC = ({ const isFullSize = fullSizeChartId === chartId; const focusHighlightStyles = useFilterFocusHighlightStyles(chartId); - const dashboardState = useSelector( - (state: RootState) => state.dashboardState, + const directPathToChild = useSelector( + (state: RootState) => state.dashboardState.directPathToChild, ); + const directPathLastUpdated = useSelector( + (state: RootState) => state.dashboardState.directPathLastUpdated ?? 0, + ); + const [extraControls, setExtraControls] = useState>( {}, ); @@ -118,18 +122,8 @@ const ChartHolder: React.FC = ({ const [currentDirectPathLastUpdated, setCurrentDirectPathLastUpdated] = useState(0); - const directPathToChild = useMemo( - () => dashboardState?.directPathToChild ?? [], - [dashboardState], - ); - - const directPathLastUpdated = useMemo( - () => dashboardState?.directPathLastUpdated ?? 0, - [dashboardState], - ); - const infoFromPath = useMemo( - () => getChartAndLabelComponentIdFromPath(directPathToChild) as any, + () => getChartAndLabelComponentIdFromPath(directPathToChild ?? []) as any, [directPathToChild], ); @@ -191,26 +185,26 @@ const ChartHolder: React.FC = ({ ]); const { chartWidth, chartHeight } = useMemo(() => { - let chartWidth = 0; - let chartHeight = 0; + let width = 0; + let height = 0; if (isFullSize) { - chartWidth = window.innerWidth - CHART_MARGIN; - chartHeight = window.innerHeight - CHART_MARGIN; + width = window.innerWidth - CHART_MARGIN; + height = window.innerHeight - CHART_MARGIN; } else { - chartWidth = Math.floor( + width = Math.floor( widthMultiple * columnWidth + (widthMultiple - 1) * GRID_GUTTER_SIZE - CHART_MARGIN, ); - chartHeight = Math.floor( + height = Math.floor( component.meta.height * GRID_BASE_UNIT - CHART_MARGIN, ); } return { - chartWidth, - chartHeight, + chartWidth: width, + chartHeight: height, }; }, [columnWidth, component, isFullSize, widthMultiple]); @@ -244,6 +238,111 @@ const ChartHolder: React.FC = ({ })); }, []); + const renderChild = useCallback( + ({ dragSourceRef }) => ( + +
+ {!editMode && ( + + )} + {!!outlinedComponentId && ( + + )} + + {editMode && ( + +
+ +
+ + )} +
+ + ), + [ + component.id, + component.meta.height, + component.meta.chartId, + component.meta.sliceNameOverride, + component.meta.sliceName, + parentComponent.type, + columnWidth, + widthMultiple, + availableColumnCount, + onResizeStart, + onResize, + onResizeStop, + editMode, + focusHighlightStyles, + isFullSize, + fullSizeStyle, + chartId, + outlinedComponentId, + outlinedColumnName, + dashboardId, + chartWidth, + chartHeight, + handleUpdateSliceName, + isComponentVisible, + handleToggleFullSize, + handleExtraControl, + extraControls, + isInView, + handleDeleteComponent, + ], + ); + return ( = ({ disableDragDrop={false} editMode={editMode} > - {({ dragSourceRef }) => ( - -
- {!editMode && ( - - )} - {!!outlinedComponentId && ( - - )} - - {editMode && ( - -
- -
- - )} -
- - )} + {renderChild} ); }; -export default ChartHolder; +export default memo(ChartHolder); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Column.jsx b/superset-frontend/src/dashboard/components/gridComponents/Column.jsx index d11937ab176ab..71c1892f3b887 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Column.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Column.jsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { PureComponent, Fragment } from 'react'; +import { Fragment, useCallback, useState, useMemo, memo } from 'react'; import PropTypes from 'prop-types'; import cx from 'classnames'; import { css, styled, t } from '@superset-ui/core'; @@ -119,203 +119,219 @@ const emptyColumnContentStyles = theme => css` color: ${theme.colors.text.label}; `; -class Column extends PureComponent { - constructor(props) { - super(props); - this.state = { - isFocused: false, - }; - this.handleChangeBackground = this.handleUpdateMeta.bind( - this, - 'background', - ); - this.handleChangeFocus = this.handleChangeFocus.bind(this); - this.handleDeleteComponent = this.handleDeleteComponent.bind(this); - } +const Column = props => { + const { + component: columnComponent, + parentComponent, + index, + availableColumnCount, + columnWidth, + minColumnWidth, + depth, + onResizeStart, + onResize, + onResizeStop, + handleComponentDrop, + editMode, + onChangeTab, + isComponentVisible, + deleteComponent, + id, + parentId, + updateComponents, + } = props; - handleDeleteComponent() { - const { deleteComponent, id, parentId } = this.props; + const [isFocused, setIsFocused] = useState(false); + + const handleDeleteComponent = useCallback(() => { deleteComponent(id, parentId); - } + }, [deleteComponent, id, parentId]); - handleChangeFocus(nextFocus) { - this.setState(() => ({ isFocused: Boolean(nextFocus) })); - } + const handleChangeFocus = useCallback(nextFocus => { + setIsFocused(Boolean(nextFocus)); + }, []); - handleUpdateMeta(metaKey, nextValue) { - const { updateComponents, component } = this.props; - if (nextValue && component.meta[metaKey] !== nextValue) { - updateComponents({ - [component.id]: { - ...component, - meta: { - ...component.meta, - [metaKey]: nextValue, + const handleChangeBackground = useCallback( + nextValue => { + const metaKey = 'background'; + if (nextValue && columnComponent.meta[metaKey] !== nextValue) { + updateComponents({ + [columnComponent.id]: { + ...columnComponent, + meta: { + ...columnComponent.meta, + [metaKey]: nextValue, + }, }, - }, - }); - } - } + }); + } + }, + [columnComponent, updateComponents], + ); - render() { - const { - component: columnComponent, - parentComponent, - index, - availableColumnCount, - columnWidth, - minColumnWidth, - depth, - onResizeStart, - onResize, - onResizeStop, - handleComponentDrop, - editMode, - onChangeTab, - isComponentVisible, - } = this.props; + const columnItems = useMemo( + () => columnComponent.children || [], + [columnComponent.children], + ); - const columnItems = columnComponent.children || []; - const backgroundStyle = backgroundStyleOptions.find( - opt => - opt.value === - (columnComponent.meta.background || BACKGROUND_TRANSPARENT), - ); + const backgroundStyle = backgroundStyleOptions.find( + opt => + opt.value === (columnComponent.meta.background || BACKGROUND_TRANSPARENT), + ); - return ( - ( + - {({ dragSourceRef }) => ( - , + ]} + editMode={editMode} + > + {editMode && ( + + + + } + /> + + )} + - , - ]} - editMode={editMode} - > - {editMode && ( - - - - } - /> - - )} - - {editMode && ( - 0 && 'droptarget-edge', - )} - editMode - > - {({ dropIndicatorProps }) => - dropIndicatorProps &&
+ {editMode && ( + + : { + component: columnItems[0], + })} + depth={depth} + index={0} + orientation="column" + onDrop={handleComponentDrop} + className={cx( + 'empty-droptarget', + columnItems.length > 0 && 'droptarget-edge', )} - {columnItems.length === 0 ? ( -
{t('Empty column')}
- ) : ( - columnItems.map((componentId, itemIndex) => ( - - - {editMode && ( - - {({ dropIndicatorProps }) => - dropIndicatorProps && ( -
- ) - } - + editMode + > + {({ dropIndicatorProps }) => + dropIndicatorProps &&
+ } + + )} + {columnItems.length === 0 ? ( +
{t('Empty column')}
+ ) : ( + columnItems.map((componentId, itemIndex) => ( + + + {editMode && ( + - )) - )} - - - - )} - - ); - } -} + editMode + > + {({ dropIndicatorProps }) => + dropIndicatorProps &&
+ } + + )} + + )) + )} + + + + ), + [ + availableColumnCount, + backgroundStyle.className, + columnComponent, + columnItems, + columnWidth, + depth, + editMode, + handleChangeBackground, + handleChangeFocus, + handleComponentDrop, + handleDeleteComponent, + isComponentVisible, + isFocused, + minColumnWidth, + onChangeTab, + onResize, + onResizeStart, + onResizeStop, + ], + ); + + return ( + + {renderChild} + + ); +}; Column.propTypes = propTypes; Column.defaultProps = defaultProps; -export default Column; +export default memo(Column); diff --git a/superset-frontend/src/dashboard/components/gridComponents/DynamicComponent.tsx b/superset-frontend/src/dashboard/components/gridComponents/DynamicComponent.tsx index e30b4a5455126..3b97c277d9c72 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/DynamicComponent.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/DynamicComponent.tsx @@ -20,7 +20,7 @@ import { FC, Suspense } from 'react'; import { DashboardComponentMetadata, JsonObject, t } from '@superset-ui/core'; import backgroundStyleOptions from 'src/dashboard/util/backgroundStyleOptions'; import cx from 'classnames'; -import { useSelector } from 'react-redux'; +import { shallowEqual, useSelector } from 'react-redux'; import { Draggable } from '../dnd/DragDroppable'; import { COLUMN_TYPE, ROW_TYPE } from '../../util/componentTypes'; import WithPopoverMenu from '../menu/WithPopoverMenu'; @@ -103,6 +103,7 @@ const DynamicComponent: FC = ({ nativeFilters, dataMask, }), + shallowEqual, ); return ( diff --git a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx index a22361e1421a9..bd59306ca97fa 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx @@ -16,10 +16,17 @@ * specific language governing permissions and limitations * under the License. */ -import { createRef, PureComponent, Fragment } from 'react'; +import { + Fragment, + useState, + useCallback, + useRef, + useEffect, + useMemo, + memo, +} from 'react'; import PropTypes from 'prop-types'; import cx from 'classnames'; -import { debounce } from 'lodash'; import { css, FAST_DEBOUNCE, @@ -46,6 +53,7 @@ import backgroundStyleOptions from 'src/dashboard/util/backgroundStyleOptions'; import { BACKGROUND_TRANSPARENT } from 'src/dashboard/util/constants'; import { EMPTY_CONTAINER_Z_INDEX } from 'src/dashboard/constants'; import { isCurrentUserBot } from 'src/utils/isBot'; +import { useDebouncedEffect } from '../../../explore/exploreUtils'; const propTypes = { id: PropTypes.string.isRequired, @@ -126,285 +134,301 @@ const emptyRowContentStyles = theme => css` color: ${theme.colors.text.label}; `; -class Row extends PureComponent { - constructor(props) { - super(props); - this.state = { - isFocused: false, - isInView: false, - hoverMenuHovered: false, - }; - this.handleDeleteComponent = this.handleDeleteComponent.bind(this); - this.handleUpdateMeta = this.handleUpdateMeta.bind(this); - this.handleChangeBackground = this.handleUpdateMeta.bind( - this, - 'background', - ); - this.handleChangeFocus = this.handleChangeFocus.bind(this); - this.handleMenuHover = this.handleMenuHover.bind(this); - this.setVerticalEmptyContainerHeight = debounce( - this.setVerticalEmptyContainerHeight.bind(this), - FAST_DEBOUNCE, - ); +const Row = props => { + const { + component: rowComponent, + parentComponent, + index, + availableColumnCount, + columnWidth, + occupiedColumnCount, + depth, + onResizeStart, + onResize, + onResizeStop, + handleComponentDrop, + editMode, + onChangeTab, + isComponentVisible, + updateComponents, + deleteComponent, + parentId, + } = props; + + const [isFocused, setIsFocused] = useState(false); + const [isInView, setIsInView] = useState(false); + const [hoverMenuHovered, setHoverMenuHovered] = useState(false); + const [containerHeight, setContainerHeight] = useState(null); + const containerRef = useRef(); + const isComponentVisibleRef = useRef(isComponentVisible); - this.containerRef = createRef(); - this.observerEnabler = null; - this.observerDisabler = null; - } + useEffect(() => { + isComponentVisibleRef.current = isComponentVisible; + }, [isComponentVisible]); // if chart not rendered - render it if it's less than 1 view height away from current viewport // if chart rendered - remove it if it's more than 4 view heights away from current viewport - componentDidMount() { + useEffect(() => { + let observerEnabler; + let observerDisabler; if ( isFeatureEnabled(FeatureFlag.DashboardVirtualization) && !isCurrentUserBot() ) { - this.observerEnabler = new IntersectionObserver( + observerEnabler = new IntersectionObserver( ([entry]) => { - if (entry.isIntersecting && !this.state.isInView) { - this.setState({ isInView: true }); + if (entry.isIntersecting && isComponentVisibleRef.current) { + setIsInView(true); } }, { rootMargin: '100% 0px', }, ); - this.observerDisabler = new IntersectionObserver( + observerDisabler = new IntersectionObserver( ([entry]) => { - if (!entry.isIntersecting && this.state.isInView) { - this.setState({ isInView: false }); + if (!entry.isIntersecting && isComponentVisibleRef.current) { + setIsInView(false); } }, { rootMargin: '400% 0px', }, ); - const element = this.containerRef.current; + const element = containerRef.current; if (element) { - this.observerEnabler.observe(element); - this.observerDisabler.observe(element); - this.setVerticalEmptyContainerHeight(); + observerEnabler.observe(element); + observerDisabler.observe(element); } } - } - - componentDidUpdate() { - this.setVerticalEmptyContainerHeight(); - } - - setVerticalEmptyContainerHeight() { - const { containerHeight } = this.state; - const { editMode } = this.props; - const updatedHeight = this.containerRef.current?.clientHeight; - if ( - editMode && - this.containerRef.current && - updatedHeight !== containerHeight - ) { - this.setState({ containerHeight: updatedHeight }); - } - } + return () => { + observerEnabler?.disconnect(); + observerDisabler?.disconnect(); + }; + }, []); - componentWillUnmount() { - this.observerEnabler?.disconnect(); - this.observerDisabler?.disconnect(); - } + useDebouncedEffect( + () => { + const updatedHeight = containerRef.current?.clientHeight; + if ( + editMode && + containerRef.current && + updatedHeight !== containerHeight + ) { + setContainerHeight(updatedHeight); + } + }, + FAST_DEBOUNCE, + [editMode, containerHeight], + ); - handleChangeFocus(nextFocus) { - this.setState(() => ({ isFocused: Boolean(nextFocus) })); - } + const handleChangeFocus = useCallback(nextFocus => { + setIsFocused(Boolean(nextFocus)); + }, []); - handleUpdateMeta(metaKey, nextValue) { - const { updateComponents, component } = this.props; - if (nextValue && component.meta[metaKey] !== nextValue) { - updateComponents({ - [component.id]: { - ...component, - meta: { - ...component.meta, - [metaKey]: nextValue, + const handleChangeBackground = useCallback( + nextValue => { + const metaKey = 'background'; + if (nextValue && rowComponent.meta[metaKey] !== nextValue) { + updateComponents({ + [rowComponent.id]: { + ...rowComponent, + meta: { + ...rowComponent.meta, + [metaKey]: nextValue, + }, }, - }, - }); - } - } + }); + } + }, + [updateComponents, rowComponent], + ); - handleDeleteComponent() { - const { deleteComponent, component, parentId } = this.props; - deleteComponent(component.id, parentId); - } + const handleDeleteComponent = useCallback(() => { + deleteComponent(rowComponent.id, parentId); + }, [deleteComponent, rowComponent, parentId]); - handleMenuHover = hovered => { + const handleMenuHover = useCallback(hovered => { const { isHovered } = hovered; - this.setState(() => ({ hoverMenuHovered: isHovered })); - }; + setHoverMenuHovered(isHovered); + }, []); - render() { - const { - component: rowComponent, - parentComponent, - index, - availableColumnCount, - columnWidth, - occupiedColumnCount, - depth, - onResizeStart, - onResize, - onResizeStop, - handleComponentDrop, - editMode, - onChangeTab, - isComponentVisible, - } = this.props; - const { containerHeight, hoverMenuHovered } = this.state; + const rowItems = useMemo( + () => rowComponent.children || [], + [rowComponent.children], + ); - const rowItems = rowComponent.children || []; - - const backgroundStyle = backgroundStyleOptions.find( - opt => - opt.value === (rowComponent.meta.background || BACKGROUND_TRANSPARENT), - ); - const remainColumnCount = availableColumnCount - occupiedColumnCount; - - return ( - + opt.value === (rowComponent.meta.background || BACKGROUND_TRANSPARENT), + ); + const remainColumnCount = availableColumnCount - occupiedColumnCount; + const renderChild = useCallback( + ({ dragSourceRef }) => ( + , + ]} editMode={editMode} > - {({ dragSourceRef }) => ( - , - ]} - editMode={editMode} + {editMode && ( + - {editMode && ( - - - - } - /> - - )} - + + } + /> + + )} + + {editMode && ( + 0 && 'droptarget-side', )} - data-test={`grid-row-${backgroundStyle.className}`} - ref={this.containerRef} - editMode={editMode} + editMode + style={{ + height: rowItems.length > 0 ? containerHeight : '100%', + ...(rowItems.length > 0 && { width: 16 }), + }} > - {editMode && ( - 0 && 'droptarget-side', - )} - editMode - style={{ - height: rowItems.length > 0 ? containerHeight : '100%', - ...(rowItems.length > 0 && { width: 16 }), - }} - > - {({ dropIndicatorProps }) => - dropIndicatorProps &&
- } - - )} - {rowItems.length === 0 && ( -
{t('Empty row')}
- )} - {rowItems.length > 0 && - rowItems.map((componentId, itemIndex) => ( - - - {editMode && ( - - {({ dropIndicatorProps }) => - dropIndicatorProps &&
- } - + {({ dropIndicatorProps }) => + dropIndicatorProps &&
+ } + + )} + {rowItems.length === 0 && ( +
{t('Empty row')}
+ )} + {rowItems.length > 0 && + rowItems.map((componentId, itemIndex) => ( + + + {editMode && ( + - ))} - - - )} - - ); - } -} + editMode + style={{ + height: containerHeight, + ...(remainColumnCount === 0 && + itemIndex === rowItems.length - 1 && { width: 16 }), + }} + > + {({ dropIndicatorProps }) => + dropIndicatorProps &&
+ } + + )} + + ))} + + + ), + [ + backgroundStyle.className, + backgroundStyle.value, + columnWidth, + containerHeight, + depth, + editMode, + handleChangeBackground, + handleChangeFocus, + handleComponentDrop, + handleDeleteComponent, + handleMenuHover, + hoverMenuHovered, + isComponentVisible, + isFocused, + isInView, + onChangeTab, + onResize, + onResizeStart, + onResizeStop, + remainColumnCount, + rowComponent, + rowItems, + ], + ); + + return ( + + {renderChild} + + ); +}; Row.propTypes = propTypes; -export default Row; +export default memo(Row); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx index 7b9bc8d483cdc..16493732df2c2 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab.jsx @@ -16,11 +16,10 @@ * specific language governing permissions and limitations * under the License. */ -import { PureComponent, Fragment } from 'react'; +import { Fragment, useCallback, memo } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; -import { bindActionCreators } from 'redux'; -import { connect } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; import { styled, t } from '@superset-ui/core'; import { EmptyStateMedium } from 'src/components/EmptyState'; @@ -51,7 +50,6 @@ const propTypes = { onDragTab: PropTypes.func, onHoverTab: PropTypes.func, editMode: PropTypes.bool.isRequired, - canEdit: PropTypes.bool.isRequired, // grid related availableColumnCount: PropTypes.number, @@ -64,7 +62,6 @@ const propTypes = { handleComponentDrop: PropTypes.func.isRequired, updateComponents: PropTypes.func.isRequired, setDirectPathToChild: PropTypes.func.isRequired, - setEditMode: PropTypes.func.isRequired, }; const defaultProps = { @@ -101,62 +98,65 @@ const TitleDropIndicator = styled.div` const renderDraggableContent = dropProps => dropProps.dropIndicatorProps &&
; -class Tab extends PureComponent { - constructor(props) { - super(props); - this.handleChangeText = this.handleChangeText.bind(this); - this.handleDrop = this.handleDrop.bind(this); - this.handleOnHover = this.handleOnHover.bind(this); - this.handleTopDropTargetDrop = this.handleTopDropTargetDrop.bind(this); - this.handleChangeTab = this.handleChangeTab.bind(this); - } - - handleChangeTab({ pathToTabIndex }) { - this.props.setDirectPathToChild(pathToTabIndex); - } +const Tab = props => { + const dispatch = useDispatch(); + const canEdit = useSelector(state => state.dashboardInfo.dash_edit_perm); + const handleChangeTab = useCallback( + ({ pathToTabIndex }) => { + props.setDirectPathToChild(pathToTabIndex); + }, + [props.setDirectPathToChild], + ); - handleChangeText(nextTabText) { - const { updateComponents, component } = this.props; - if (nextTabText && nextTabText !== component.meta.text) { - updateComponents({ - [component.id]: { - ...component, - meta: { - ...component.meta, - text: nextTabText, + const handleChangeText = useCallback( + nextTabText => { + const { updateComponents, component } = props; + if (nextTabText && nextTabText !== component.meta.text) { + updateComponents({ + [component.id]: { + ...component, + meta: { + ...component.meta, + text: nextTabText, + }, }, - }, - }); - } - } + }); + } + }, + [props.updateComponents, props.component], + ); - handleDrop(dropResult) { - this.props.handleComponentDrop(dropResult); - this.props.onDropOnTab(dropResult); - } + const handleDrop = useCallback( + dropResult => { + props.handleComponentDrop(dropResult); + props.onDropOnTab(dropResult); + }, + [props.handleComponentDrop, props.onDropOnTab], + ); - handleOnHover() { - this.props.onHoverTab(); - } + const handleHoverTab = useCallback(() => { + props.onHoverTab?.(); + }, [props.onHoverTab]); - handleTopDropTargetDrop(dropResult) { - if (dropResult) { - this.props.handleComponentDrop({ - ...dropResult, - destination: { - ...dropResult.destination, - // force appending as the first child if top drop target - index: 0, - }, - }); - } - } + const handleTopDropTargetDrop = useCallback( + dropResult => { + if (dropResult) { + props.handleComponentDrop({ + ...dropResult, + destination: { + ...dropResult.destination, + // force appending as the first child if top drop target + index: 0, + }, + }); + } + }, + [props.handleComponentDrop], + ); - shouldDropToChild(item) { - return item.type !== TAB_TYPE; - } + const shouldDropToChild = useCallback(item => item.type !== TAB_TYPE, []); - renderTabContent() { + const renderTabContent = useCallback(() => { const { component: tabComponent, depth, @@ -167,10 +167,8 @@ class Tab extends PureComponent { onResizeStop, editMode, isComponentVisible, - canEdit, - setEditMode, dashboardId, - } = this.props; + } = props; const shouldDisplayEmptyState = tabComponent.children.length === 0; return ( @@ -184,8 +182,8 @@ class Tab extends PureComponent { depth={depth} onDrop={ tabComponent.children.length === 0 - ? this.handleTopDropTargetDrop - : this.handleDrop + ? handleTopDropTargetDrop + : handleDrop } editMode className={classNames({ @@ -224,7 +222,7 @@ class Tab extends PureComponent { setEditMode(true)} + onClick={() => dispatch(setEditMode(true))} > {t('edit mode')} @@ -241,15 +239,15 @@ class Tab extends PureComponent { parentId={tabComponent.id} depth={depth} // see isValidChild.js for why tabs don't increment child depth index={componentIndex} - onDrop={this.handleDrop} - onHover={this.handleOnHover} + onDrop={handleDrop} + onHover={handleHoverTab} availableColumnCount={availableColumnCount} columnWidth={columnWidth} onResizeStart={onResizeStart} onResize={onResize} onResizeStop={onResizeStop} isComponentVisible={isComponentVisible} - onChangeTab={this.handleChangeTab} + onChangeTab={handleChangeTab} /> {/* Make bottom of tab droppable */} {editMode && ( @@ -258,7 +256,7 @@ class Tab extends PureComponent { orientation="column" index={componentIndex + 1} depth={depth} - onDrop={this.handleDrop} + onDrop={handleDrop} editMode className="empty-droptarget" > @@ -269,20 +267,94 @@ class Tab extends PureComponent { ))}
); - } + }, [ + dispatch, + props.component, + props.depth, + props.availableColumnCount, + props.columnWidth, + props.onResizeStart, + props.onResize, + props.onResizeStop, + props.editMode, + props.isComponentVisible, + props.dashboardId, + props.handleComponentDrop, + props.onDropOnTab, + props.setDirectPathToChild, + props.updateComponents, + handleHoverTab, + canEdit, + handleChangeTab, + handleChangeText, + handleDrop, + handleTopDropTargetDrop, + shouldDropToChild, + ]); - renderTab() { + const renderTabChild = useCallback( + ({ dropIndicatorProps, dragSourceRef, draggingTabOnTab }) => { + const { + component, + index, + editMode, + isFocused, + isHighlighted, + dashboardId, + } = props; + return ( + + + {!editMode && ( + = 5 ? 'left' : 'right'} + /> + )} + + {dropIndicatorProps && !draggingTabOnTab && ( + + )} + + ); + }, + [ + props.component, + props.index, + props.editMode, + props.isFocused, + props.isHighlighted, + props.dashboardId, + handleChangeText, + ], + ); + + const renderTab = useCallback(() => { const { component, parentComponent, index, depth, editMode, - isFocused, - isHighlighted, onDropPositionChange, onDragTab, - } = this.props; + } = props; return ( - {({ dropIndicatorProps, dragSourceRef, draggingTabOnTab }) => ( - - - {!editMode && ( - = 5 ? 'left' : 'right'} - /> - )} - {dropIndicatorProps && !draggingTabOnTab && ( - - )} - - )} + {renderTabChild} ); - } + }, [ + props.component, + props.parentComponent, + props.index, + props.depth, + props.editMode, + handleDrop, + handleHoverTab, + shouldDropToChild, + renderTabChild, + ]); - render() { - const { renderType } = this.props; - return renderType === RENDER_TAB - ? this.renderTab() - : this.renderTabContent(); - } -} + return props.renderType === RENDER_TAB ? renderTab() : renderTabContent(); +}; Tab.propTypes = propTypes; Tab.defaultProps = defaultProps; -function mapStateToProps(state) { - return { - canEdit: state.dashboardInfo.dash_edit_perm, - }; -} - -function mapDispatchToProps(dispatch) { - return bindActionCreators( - { - setEditMode, - }, - dispatch, - ); -} - -export default connect(mapStateToProps, mapDispatchToProps)(Tab); +export default memo(Tab); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx index 901d8729b1158..19d49254da09f 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs.jsx @@ -16,10 +16,10 @@ * specific language governing permissions and limitations * under the License. */ -import { PureComponent } from 'react'; +import { useCallback, useEffect, useMemo, useState, memo } from 'react'; import PropTypes from 'prop-types'; -import { styled, t } from '@superset-ui/core'; -import { connect } from 'react-redux'; +import { styled, t, usePrevious } from '@superset-ui/core'; +import { useSelector } from 'react-redux'; import { LineEditableTabs } from 'src/components/Tabs'; import Icons from 'src/components/Icons'; import { LOG_ACTIONS_SELECT_DASHBOARD_TAB } from 'src/logger/LogUtils'; @@ -48,7 +48,6 @@ const propTypes = { renderTabContent: PropTypes.bool, // whether to render tabs + content or just tabs editMode: PropTypes.bool.isRequired, renderHoverMenu: PropTypes.bool, - directPathToChild: PropTypes.arrayOf(PropTypes.string), activeTabs: PropTypes.arrayOf(PropTypes.string), // actions (from DashboardComponent.jsx) @@ -71,12 +70,6 @@ const propTypes = { }; const defaultProps = { - renderTabContent: true, - renderHoverMenu: true, - availableColumnCount: 0, - columnWidth: 0, - activeTabs: [], - directPathToChild: [], setActiveTab() {}, onResizeStart() {}, onResize() {}, @@ -133,58 +126,76 @@ const CloseIconWithDropIndicator = props => ( ); -export class Tabs extends PureComponent { - constructor(props) { - super(props); - const { tabIndex, activeKey } = this.getTabInfo(props); +const Tabs = props => { + const nativeFilters = useSelector(state => state.nativeFilters); + const activeTabs = useSelector(state => state.dashboardState.activeTabs); + const directPathToChild = useSelector( + state => state.dashboardState.directPathToChild, + ); - this.state = { + const { tabIndex: initTabIndex, activeKey: initActiveKey } = useMemo(() => { + let tabIndex = Math.max( + 0, + findTabIndexByComponentId({ + currentComponent: props.component, + directPathToChild, + }), + ); + if (tabIndex === 0 && activeTabs?.length) { + props.component.children.forEach((tabId, index) => { + if (tabIndex === 0 && activeTabs?.includes(tabId)) { + tabIndex = index; + } + }); + } + const { children: tabIds } = props.component; + const activeKey = tabIds[tabIndex]; + + return { tabIndex, activeKey, }; - this.handleClickTab = this.handleClickTab.bind(this); - this.handleDeleteComponent = this.handleDeleteComponent.bind(this); - this.handleDeleteTab = this.handleDeleteTab.bind(this); - this.handleDropOnTab = this.handleDropOnTab.bind(this); - this.handleDrop = this.handleDrop.bind(this); - this.handleGetDropPosition = this.handleGetDropPosition.bind(this); - this.handleDragggingTab = this.handleDragggingTab.bind(this); - } - - componentDidMount() { - this.props.setActiveTab(this.state.activeKey); - } - - componentDidUpdate(prevProps, prevState) { - if (prevState.activeKey !== this.state.activeKey) { - this.props.setActiveTab(this.state.activeKey, prevState.activeKey); + }, [activeTabs, props.component, directPathToChild]); + + const [activeKey, setActiveKey] = useState(initActiveKey); + const [selectedTabIndex, setSelectedTabIndex] = useState(initTabIndex); + const [dropPosition, setDropPosition] = useState(null); + const [dragOverTabIndex, setDragOverTabIndex] = useState(null); + const [draggingTabId, setDraggingTabId] = useState(null); + const prevActiveKey = usePrevious(activeKey); + const prevDashboardId = usePrevious(props.dashboardId); + const prevDirectPathToChild = usePrevious(directPathToChild); + const prevTabIds = usePrevious(props.component.children); + + useEffect(() => { + if (prevActiveKey) { + props.setActiveTab(activeKey, prevActiveKey); + } else { + props.setActiveTab(activeKey); } - } + }, [props.setActiveTab, prevActiveKey, activeKey]); - UNSAFE_componentWillReceiveProps(nextProps) { - const maxIndex = Math.max(0, nextProps.component.children.length - 1); - const currTabsIds = this.props.component.children; - const nextTabsIds = nextProps.component.children; - - if (this.state.tabIndex > maxIndex) { - this.setState(() => ({ tabIndex: maxIndex })); + useEffect(() => { + if (prevDashboardId && props.dashboardId !== prevDashboardId) { + setSelectedTabIndex(initTabIndex); + setActiveKey(initActiveKey); } + }, [props.dashboardId, prevDashboardId, initTabIndex, initActiveKey]); - // reset tab index if dashboard was changed - if (nextProps.dashboardId !== this.props.dashboardId) { - const { tabIndex, activeKey } = this.getTabInfo(nextProps); - this.setState(() => ({ - tabIndex, - activeKey, - })); + useEffect(() => { + const maxIndex = Math.max(0, props.component.children.length - 1); + if (selectedTabIndex > maxIndex) { + setSelectedTabIndex(maxIndex); } + }, [selectedTabIndex, props.component.children.length, setSelectedTabIndex]); - if (nextProps.isComponentVisible) { - const nextFocusComponent = getLeafComponentIdFromPath( - nextProps.directPathToChild, - ); + useEffect(() => { + const currTabsIds = props.component.children; + + if (props.isComponentVisible) { + const nextFocusComponent = getLeafComponentIdFromPath(directPathToChild); const currentFocusComponent = getLeafComponentIdFromPath( - this.props.directPathToChild, + prevDirectPathToChild, ); // If the currently selected component is different than the new one, @@ -193,328 +204,349 @@ export class Tabs extends PureComponent { if ( nextFocusComponent !== currentFocusComponent || (nextFocusComponent === currentFocusComponent && - currTabsIds !== nextTabsIds) + currTabsIds !== prevTabIds) ) { const nextTabIndex = findTabIndexByComponentId({ - currentComponent: nextProps.component, - directPathToChild: nextProps.directPathToChild, + currentComponent: props.component, + directPathToChild, }); // make sure nextFocusComponent is under this tabs component - if (nextTabIndex > -1 && nextTabIndex !== this.state.tabIndex) { - this.setState(() => ({ - tabIndex: nextTabIndex, - activeKey: nextTabsIds[nextTabIndex], - })); + if (nextTabIndex > -1 && nextTabIndex !== selectedTabIndex) { + setSelectedTabIndex(nextTabIndex); + setActiveKey(currTabsIds[nextTabIndex]); } } } - } + }, [ + props.component, + directPathToChild, + props.isComponentVisible, + selectedTabIndex, + prevDirectPathToChild, + prevTabIds, + ]); + + const handleClickTab = useCallback( + tabIndex => { + const { component } = props; + const { children: tabIds } = component; + + if (tabIndex !== selectedTabIndex) { + const pathToTabIndex = getDirectPathToTabIndex(component, tabIndex); + const targetTabId = pathToTabIndex[pathToTabIndex.length - 1]; + props.logEvent(LOG_ACTIONS_SELECT_DASHBOARD_TAB, { + target_id: targetTabId, + index: tabIndex, + }); - getTabInfo = props => { - let tabIndex = Math.max( - 0, - findTabIndexByComponentId({ - currentComponent: props.component, - directPathToChild: props.directPathToChild, - }), - ); - if (tabIndex === 0 && props.activeTabs?.length) { - props.component.children.forEach((tabId, index) => { - if (tabIndex === 0 && props.activeTabs.includes(tabId)) { - tabIndex = index; + props.onChangeTab({ pathToTabIndex }); + } + setActiveKey(tabIds[tabIndex]); + }, + [ + props.component, + props.logEvent, + props.onChangeTab, + selectedTabIndex, + setActiveKey, + ], + ); + + const handleDropOnTab = useCallback( + dropResult => { + const { component } = props; + + // Ensure dropped tab is visible + const { destination } = dropResult; + if (destination) { + const dropTabIndex = + destination.id === component.id + ? destination.index // dropped ON tabs + : component.children.indexOf(destination.id); // dropped IN tab + + if (dropTabIndex > -1) { + setTimeout(() => { + handleClickTab(dropTabIndex); + }, 30); } - }); - } - const { children: tabIds } = props.component; - const activeKey = tabIds[tabIndex]; - - return { - tabIndex, - activeKey, - }; - }; - - showDeleteConfirmModal = key => { - const { component, deleteComponent } = this.props; - AntdModal.confirm({ - title: t('Delete dashboard tab?'), - content: ( - - {t( - 'Deleting a tab will remove all content within it and will deactivate any related alerts or reports. You may still ' + - 'reverse this action with the', - )}{' '} - {t('undo')}{' '} - {t('button (cmd + z) until you save your changes.')} - - ), - onOk: () => { - deleteComponent(key, component.id); - const tabIndex = component.children.indexOf(key); - this.handleDeleteTab(tabIndex); - }, - okType: 'danger', - okText: t('DELETE'), - cancelText: t('CANCEL'), - icon: null, - }); - }; - - handleEdit = (event, action) => { - const { component, createComponent } = this.props; - if (action === 'add') { - // Prevent the tab container to be selected - event?.stopPropagation?.(); - - createComponent({ - destination: { - id: component.id, - type: component.type, - index: component.children.length, - }, - dragging: { - id: NEW_TAB_ID, - type: TAB_TYPE, + } + }, + [props.component, handleClickTab], + ); + + const handleDrop = useCallback( + dropResult => { + if (dropResult.dragging.type !== TABS_TYPE) { + props.handleComponentDrop(dropResult); + } + }, + [props.handleComponentDrop], + ); + + const handleDeleteTab = useCallback( + tabIndex => { + // If we're removing the currently selected tab, + // select the previous one (if any) + if (selectedTabIndex === tabIndex) { + handleClickTab(Math.max(0, tabIndex - 1)); + } + }, + [selectedTabIndex, handleClickTab], + ); + + const showDeleteConfirmModal = useCallback( + key => { + const { component, deleteComponent } = props; + AntdModal.confirm({ + title: t('Delete dashboard tab?'), + content: ( + + {t( + 'Deleting a tab will remove all content within it and will deactivate any related alerts or reports. You may still ' + + 'reverse this action with the', + )}{' '} + {t('undo')}{' '} + {t('button (cmd + z) until you save your changes.')} + + ), + onOk: () => { + deleteComponent(key, component.id); + const tabIndex = component.children.indexOf(key); + handleDeleteTab(tabIndex); }, + okType: 'danger', + okText: t('DELETE'), + cancelText: t('CANCEL'), + icon: null, }); - } else if (action === 'remove') { - this.showDeleteConfirmModal(event); - } - }; - - handleClickTab(tabIndex) { - const { component } = this.props; - const { children: tabIds } = component; - - if (tabIndex !== this.state.tabIndex) { - const pathToTabIndex = getDirectPathToTabIndex(component, tabIndex); - const targetTabId = pathToTabIndex[pathToTabIndex.length - 1]; - this.props.logEvent(LOG_ACTIONS_SELECT_DASHBOARD_TAB, { - target_id: targetTabId, - index: tabIndex, - }); - - this.props.onChangeTab({ pathToTabIndex }); - } - this.setState(() => ({ activeKey: tabIds[tabIndex] })); - } + }, + [props.component, props.deleteComponent, handleDeleteTab], + ); + + const handleEdit = useCallback( + (event, action) => { + const { component, createComponent } = props; + if (action === 'add') { + // Prevent the tab container to be selected + event?.stopPropagation?.(); + + createComponent({ + destination: { + id: component.id, + type: component.type, + index: component.children.length, + }, + dragging: { + id: NEW_TAB_ID, + type: TAB_TYPE, + }, + }); + } else if (action === 'remove') { + showDeleteConfirmModal(event); + } + }, + [props.component, props.createComponent, showDeleteConfirmModal], + ); - handleDeleteComponent() { - const { deleteComponent, id, parentId } = this.props; + const handleDeleteComponent = useCallback(() => { + const { deleteComponent, id, parentId } = props; deleteComponent(id, parentId); - } + }, [props.deleteComponent, props.id, props.parentId]); - handleDeleteTab(tabIndex) { - // If we're removing the currently selected tab, - // select the previous one (if any) - if (this.state.tabIndex === tabIndex) { - this.handleClickTab(Math.max(0, tabIndex - 1)); - } - } - - handleGetDropPosition(dragObject) { + const handleGetDropPosition = useCallback(dragObject => { const { dropIndicator, isDraggingOver, index } = dragObject; if (isDraggingOver) { - this.setState(() => ({ - dropPosition: dropIndicator, - dragOverTabIndex: index, - })); + setDropPosition(dropIndicator); + setDragOverTabIndex(index); } else { - this.setState(() => ({ dropPosition: null })); - } - } - - handleDropOnTab(dropResult) { - const { component } = this.props; - - // Ensure dropped tab is visible - const { destination } = dropResult; - if (destination) { - const dropTabIndex = - destination.id === component.id - ? destination.index // dropped ON tabs - : component.children.indexOf(destination.id); // dropped IN tab - - if (dropTabIndex > -1) { - setTimeout(() => { - this.handleClickTab(dropTabIndex); - }, 30); - } + setDropPosition(null); } - } - - handleDrop(dropResult) { - if (dropResult.dragging.type !== TABS_TYPE) { - this.props.handleComponentDrop(dropResult); - } - } + }, []); - handleDragggingTab(tabId) { + const handleDragggingTab = useCallback(tabId => { if (tabId) { - this.setState(() => ({ draggingTabId: tabId })); + setDraggingTabId(tabId); } else { - this.setState(() => ({ draggingTabId: null })); + setDraggingTabId(null); } - } - - render() { - const { - depth, - component: tabsComponent, - parentComponent, - index, - availableColumnCount, - columnWidth, - onResizeStart, - onResize, - onResizeStop, - renderTabContent, - renderHoverMenu, - isComponentVisible: isCurrentTabVisible, - editMode, - nativeFilters, - } = this.props; - - const { children: tabIds } = tabsComponent; - const { - tabIndex: selectedTabIndex, - activeKey, - dropPosition, - dragOverTabIndex, - } = this.state; - - const showDropIndicators = currentDropTabIndex => + }, []); + + const { + depth, + component: tabsComponent, + parentComponent, + index, + availableColumnCount = 0, + columnWidth = 0, + onResizeStart, + onResize, + onResizeStop, + renderTabContent = true, + renderHoverMenu = true, + isComponentVisible: isCurrentTabVisible, + editMode, + } = props; + + const { children: tabIds } = tabsComponent; + + const showDropIndicators = useCallback( + currentDropTabIndex => currentDropTabIndex === dragOverTabIndex && { left: editMode && dropPosition === DROP_LEFT, right: editMode && dropPosition === DROP_RIGHT, - }; - - const removeDraggedTab = tabID => this.state.draggingTabId === tabID; + }, + [dragOverTabIndex, dropPosition, editMode], + ); + + const removeDraggedTab = useCallback( + tabID => draggingTabId === tabID, + [draggingTabId], + ); + + let tabsToHighlight; + const highlightedFilterId = + nativeFilters?.focusedFilterId || nativeFilters?.hoveredFilterId; + if (highlightedFilterId) { + tabsToHighlight = nativeFilters.filters[highlightedFilterId]?.tabsInScope; + } - let tabsToHighlight; - const highlightedFilterId = - nativeFilters?.focusedFilterId || nativeFilters?.hoveredFilterId; - if (highlightedFilterId) { - tabsToHighlight = nativeFilters.filters[highlightedFilterId]?.tabsInScope; - } - return ( - ( + - {({ dragSourceRef: tabsDragSourceRef }) => ( - - {editMode && renderHoverMenu && ( - - - - - )} - - { - this.handleClickTab(tabIds.indexOf(key)); - }} - onEdit={this.handleEdit} - data-test="nav-list" - type={editMode ? 'editable-card' : 'card'} - > - {tabIds.map((tabId, tabIndex) => ( - - ) : ( - <> - {showDropIndicators(tabIndex).left && ( - - )} - this.handleClickTab(tabIndex)} - isFocused={activeKey === tabId} - isHighlighted={ - activeKey !== tabId && - tabsToHighlight?.includes(tabId) - } - /> - - ) - } - closeIcon={ - removeDraggedTab(tabId) ? ( - <> - ) : ( - + + + + )} + + { + handleClickTab(tabIds.indexOf(key)); + }} + onEdit={handleEdit} + data-test="nav-list" + type={editMode ? 'editable-card' : 'card'} + > + {tabIds.map((tabId, tabIndex) => ( + + ) : ( + <> + {showDropIndicators(tabIndex).left && ( + - ) - } - > - {renderTabContent && ( + )} handleClickTab(tabIndex)} + isFocused={activeKey === tabId} + isHighlighted={ + activeKey !== tabId && tabsToHighlight?.includes(tabId) } /> - )} - - ))} - - - )} - - ); - } -} + + ) + } + closeIcon={ + removeDraggedTab(tabId) ? ( + <> + ) : ( + + ) + } + > + {renderTabContent && ( + + )} + + ))} + + + ), + [ + editMode, + renderHoverMenu, + handleDeleteComponent, + tabsComponent.id, + activeKey, + handleEdit, + tabIds, + handleClickTab, + removeDraggedTab, + showDropIndicators, + depth, + availableColumnCount, + columnWidth, + handleDropOnTab, + handleGetDropPosition, + handleDragggingTab, + tabsToHighlight, + renderTabContent, + onResizeStart, + onResize, + onResizeStop, + selectedTabIndex, + isCurrentTabVisible, + ], + ); + + return ( + + {renderChild} + + ); +}; Tabs.propTypes = propTypes; Tabs.defaultProps = defaultProps; -function mapStateToProps(state) { - return { - nativeFilters: state.nativeFilters, - activeTabs: state.dashboardState.activeTabs, - directPathToChild: state.dashboardState.directPathToChild, - }; -} - -export default connect(mapStateToProps)(Tabs); +export default memo(Tabs); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.jsx index 6d33091d8e40f..a477420249455 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.jsx @@ -20,11 +20,10 @@ import { fireEvent, render } from 'spec/helpers/testing-library'; import { AntdModal } from 'src/components'; import fetchMock from 'fetch-mock'; -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'; -import { getMockStore } from 'spec/fixtures/mockStore'; import { nativeFilters } from 'spec/fixtures/mockNativeFilters'; import { initialState } from 'src/SqlLab/fixtures'; @@ -81,17 +80,17 @@ const props = { nativeFilters: nativeFilters.filters, }; -const mockStore = getMockStore({ - ...initialState, - dashboardLayout: dashboardLayoutWithTabs, - dashboardFilters: {}, -}); - -function setup(overrideProps) { +function setup(overrideProps, overrideState = {}) { return render(, { useDnd: true, useRouter: true, - store: mockStore, + useRedux: true, + initialState: { + ...initialState, + dashboardLayout: dashboardLayoutWithTabs, + dashboardFilters: {}, + ...overrideState, + }, }); } @@ -174,11 +173,7 @@ test('should direct display direct-link tab', () => { // display child in directPathToChild list const directPathToChild = dashboardLayoutWithTabs.present.ROW_ID2.parents.slice(); - const directLinkProps = { - ...props, - directPathToChild, - }; - const { getByRole } = setup(directLinkProps); + const { getByRole } = setup({}, { dashboardState: { directPathToChild } }); expect(getByRole('tab', { selected: true })).toHaveTextContent('TAB_ID2'); }); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx index 98ae968fd697b..90a23c03583ef 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tabs.test.tsx @@ -25,7 +25,7 @@ import { Draggable } 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 95c524f5f7a64..38f3558864923 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/index.js +++ b/superset-frontend/src/dashboard/components/gridComponents/index.js @@ -35,7 +35,7 @@ import Divider from './Divider'; import Header from './Header'; import Row from './Row'; import Tab from './Tab'; -import TabsConnected from './Tabs'; +import Tabs from './Tabs'; import DynamicComponent from './DynamicComponent'; export { default as ChartHolder } from './ChartHolder'; @@ -56,6 +56,6 @@ export const componentLookup = { [HEADER_TYPE]: Header, [ROW_TYPE]: Row, [TAB_TYPE]: Tab, - [TABS_TYPE]: TabsConnected, + [TABS_TYPE]: Tabs, [DYNAMIC_TYPE]: DynamicComponent, }; diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx index 96a00dd24cfaf..0d7211ba8ebe1 100644 --- a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx +++ b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx @@ -22,7 +22,7 @@ import { t, logging } from '@superset-ui/core'; import { Menu } from 'src/components/Menu'; import { getDashboardPermalink } from 'src/utils/urlUtils'; import { MenuKeys, RootState } from 'src/dashboard/types'; -import { useSelector } from 'react-redux'; +import { shallowEqual, useSelector } from 'react-redux'; interface ShareMenuItemProps { url?: string; @@ -54,10 +54,13 @@ const ShareMenuItems = (props: ShareMenuItemProps) => { selectedKeys, ...rest } = props; - const { dataMask, activeTabs } = useSelector((state: RootState) => ({ - dataMask: state.dataMask, - activeTabs: state.dashboardState.activeTabs, - })); + const { dataMask, activeTabs } = useSelector( + (state: RootState) => ({ + dataMask: state.dataMask, + activeTabs: state.dashboardState.activeTabs, + }), + shallowEqual, + ); async function generateUrl() { return getDashboardPermalink({ diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx index 392db3ae09d82..8914398c54a2d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.test.tsx @@ -39,6 +39,9 @@ const INITIAL_STATE = { 3: { id: 3 }, 4: { id: 4 }, }, + dashboardState: { + sliceIds: [1, 2, 3, 4], + }, dashboardInfo: { id: 1, metadata: { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx index 61375f9ceb35d..e4530eacec639 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx @@ -22,7 +22,6 @@ import { isDefined, NativeFilterScope, t } from '@superset-ui/core'; import Modal from 'src/components/Modal'; import { ChartConfiguration, - Layout, RootState, isCrossFilterScopeGlobal, GlobalChartCrossFilterConfig, @@ -32,6 +31,7 @@ import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilter import { useChartIds } from 'src/dashboard/util/charts/useChartIds'; import { saveChartConfiguration } from 'src/dashboard/actions/dashboardInfo'; import { DEFAULT_CROSS_FILTER_SCOPING } from 'src/dashboard/constants'; +import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems'; import { ScopingModalContent } from './ScopingModalContent'; import { NEW_CHART_SCOPING_ID } from './constants'; @@ -76,9 +76,7 @@ export const ScopingModal = ({ closeModal, }: ScopingModalProps) => { const dispatch = useDispatch(); - const layout = useSelector( - state => state.dashboardLayout.present, - ); + const chartLayoutItems = useChartLayoutItems(); const chartIds = useChartIds(); const [currentChartId, setCurrentChartId] = useState(initialChartId); const initialChartConfig = useSelector( @@ -154,7 +152,11 @@ export const ScopingModal = ({ id: currentChartId, crossFilters: { scope, - chartsInScope: getChartIdsInFilterScope(scope, chartIds, layout), + chartsInScope: getChartIdsInFilterScope( + scope, + chartIds, + chartLayoutItems, + ), }, }, })); @@ -162,7 +164,7 @@ export const ScopingModal = ({ const globalChartsInScope = getChartIdsInFilterScope( scope, chartIds, - layout, + chartLayoutItems, ); setGlobalChartConfig({ scope, @@ -176,7 +178,7 @@ export const ScopingModal = ({ ); } }, - [currentChartId, chartIds, layout], + [currentChartId, chartIds, chartLayoutItems], ); const removeCustomScope = useCallback( @@ -241,7 +243,11 @@ export const ScopingModal = ({ id: newChartId, crossFilters: { scope: newScope, - chartsInScope: getChartIdsInFilterScope(newScope, chartIds, layout), + chartsInScope: getChartIdsInFilterScope( + newScope, + chartIds, + chartLayoutItems, + ), }, }; @@ -275,7 +281,7 @@ export const ScopingModal = ({ currentChartId, globalChartConfig.chartsInScope, globalChartConfig.scope, - layout, + chartLayoutItems, ], ); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/Vertical.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/Vertical.tsx index 2064d0814f037..5deff0a98872b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/Vertical.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/Vertical.tsx @@ -17,9 +17,11 @@ * under the License. */ -import { DataMaskStateWithId, JsonObject } from '@superset-ui/core'; +import { DataMaskStateWithId } from '@superset-ui/core'; import { useSelector } from 'react-redux'; -import { DashboardLayout, RootState } from 'src/dashboard/types'; +import { RootState } from 'src/dashboard/types'; +import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems'; +import { useChartIds } from 'src/dashboard/util/charts/useChartIds'; import crossFiltersSelector from './selectors'; import VerticalCollapse from './VerticalCollapse'; import { useChartsVerboseMaps } from '../utils'; @@ -28,17 +30,13 @@ const CrossFiltersVertical = () => { const dataMask = useSelector( state => state.dataMask, ); - const chartConfiguration = useSelector( - state => state.dashboardInfo.metadata?.chart_configuration, - ); - const dashboardLayout = useSelector( - state => state.dashboardLayout.present, - ); + const chartIds = useChartIds(); + const chartLayoutItems = useChartLayoutItems(); const verboseMaps = useChartsVerboseMaps(); const selectedCrossFilters = crossFiltersSelector({ dataMask, - chartConfiguration, - dashboardLayout, + chartIds, + chartLayoutItems, verboseMaps, }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/selectors.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/selectors.ts index c8fd8e2841c72..2c2b3ec7531ca 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/selectors.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/selectors.ts @@ -21,36 +21,37 @@ import { DataMaskStateWithId, getColumnLabel, isDefined, - JsonObject, } from '@superset-ui/core'; -import { DashboardLayout } from 'src/dashboard/types'; +import { LayoutItem } from 'src/dashboard/types'; import { CrossFilterIndicator, getCrossFilterIndicator } from '../../selectors'; export const crossFiltersSelector = (props: { dataMask: DataMaskStateWithId; - chartConfiguration: JsonObject; - dashboardLayout: DashboardLayout; + chartIds: number[]; + chartLayoutItems: LayoutItem[]; verboseMaps: { [key: string]: Record }; }): CrossFilterIndicator[] => { - const { dataMask, chartConfiguration, dashboardLayout, verboseMaps } = props; - const chartsIds = Object.keys(chartConfiguration || {}); + const { dataMask, chartIds, chartLayoutItems, verboseMaps } = props; - return chartsIds + return chartIds .map(chartId => { - const id = Number(chartId); const filterIndicator = getCrossFilterIndicator( - id, - dataMask[id], - dashboardLayout, + chartId, + dataMask[chartId], + chartLayoutItems, ); if ( isDefined(filterIndicator.column) && isDefined(filterIndicator.value) ) { const verboseColName = - verboseMaps[id]?.[getColumnLabel(filterIndicator.column)] || + verboseMaps[chartId]?.[getColumnLabel(filterIndicator.column)] || filterIndicator.column; - return { ...filterIndicator, column: verboseColName, emitterId: id }; + return { + ...filterIndicator, + column: verboseColName, + emitterId: chartId, + }; } return null; }) 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 09415047b11b8..d959026792a65 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx @@ -37,7 +37,6 @@ import { isFeatureEnabled, FeatureFlag, isNativeFilterWithDataMask, - JsonObject, } from '@superset-ui/core'; import { createHtmlPortalNode, @@ -49,15 +48,13 @@ import { useDashboardHasTabs, useSelectFiltersInScope, } from 'src/dashboard/components/nativeFilters/state'; -import { - DashboardLayout, - FilterBarOrientation, - RootState, -} from 'src/dashboard/types'; +import { FilterBarOrientation, RootState } from 'src/dashboard/types'; import DropdownContainer, { Ref as DropdownContainerRef, } from 'src/components/DropdownContainer'; import Icons from 'src/components/Icons'; +import { useChartIds } from 'src/dashboard/util/charts/useChartIds'; +import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems'; import { FiltersOutOfScopeCollapsible } from '../FiltersOutOfScopeCollapsible'; import { useFilterControlFactory } from '../useFilterControlFactory'; import { FiltersDropdownContent } from '../FiltersDropdownContent'; @@ -65,12 +62,15 @@ import crossFiltersSelector from '../CrossFilters/selectors'; import CrossFilter from '../CrossFilters/CrossFilter'; import { useFilterOutlined } from '../useFilterOutlined'; import { useChartsVerboseMaps } from '../utils'; +import { CrossFilterIndicator } from '../../selectors'; type FilterControlsProps = { dataMaskSelected: DataMaskStateWithId; onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void; }; +const EMPTY_ARRAY: CrossFilterIndicator[] = []; + const FilterControls: FC = ({ dataMaskSelected, onFilterSelectionChange, @@ -90,12 +90,8 @@ const FilterControls: FC = ({ const dataMask = useSelector( state => state.dataMask, ); - const chartConfiguration = useSelector( - state => state.dashboardInfo.metadata?.chart_configuration, - ); - const dashboardLayout = useSelector( - state => state.dashboardLayout.present, - ); + const chartIds = useChartIds(); + const chartLayoutItems = useChartLayoutItems(); const verboseMaps = useChartsVerboseMaps(); const isCrossFiltersEnabled = isFeatureEnabled( @@ -106,12 +102,12 @@ const FilterControls: FC = ({ isCrossFiltersEnabled ? crossFiltersSelector({ dataMask, - chartConfiguration, - dashboardLayout, + chartIds, + chartLayoutItems, verboseMaps, }) - : [], - [chartConfiguration, dashboardLayout, dataMask, isCrossFiltersEnabled], + : EMPTY_ARRAY, + [chartIds, chartLayoutItems, dataMask, isCrossFiltersEnabled, verboseMaps], ); const { filterControlFactory, filtersWithValues } = useFilterControlFactory( dataMaskSelected, @@ -154,18 +150,27 @@ const FilterControls: FC = ({ [filtersWithValues, portalNodes], ); - const renderVerticalContent = () => ( - <> - {filtersInScope.map(renderer)} - {showCollapsePanel && ( - 0} - renderer={renderer} - /> - )} - + const renderVerticalContent = useCallback( + () => ( + <> + {filtersInScope.map(renderer)} + {showCollapsePanel && ( + 0} + renderer={renderer} + /> + )} + + ), + [ + filtersInScope, + renderer, + showCollapsePanel, + filtersOutOfScope, + hasRequiredFirst, + ], ); const overflowedFiltersInScope = useMemo( @@ -230,70 +235,84 @@ const FilterControls: FC = ({ return [...crossFilters, ...nativeFiltersInScope]; }, [filtersInScope, renderer, rendererCrossFilter, selectedCrossFilters]); - const renderHorizontalContent = () => ( -
css` - padding: 0 ${theme.gridUnit * 4}px; - min-width: 0; - flex: 1; - `} - > - - } - dropdownTriggerText={t('More filters')} - dropdownTriggerCount={activeOverflowedFiltersInScope.length} - dropdownTriggerTooltip={ - activeOverflowedFiltersInScope.length === 0 - ? t('No applied filters') - : t( - 'Applied filters: %s', - activeOverflowedFiltersInScope - .map(filter => filter.name) - .join(', '), - ) - } - dropdownContent={ - overflowedFiltersInScope.length || - overflowedCrossFilters.length || - (filtersOutOfScope.length && showCollapsePanel) - ? () => ( - - ) - : undefined - } - forceRender={hasRequiredFirst} - ref={popoverRef} - onOverflowingStateChange={({ overflowed: nextOverflowedIds }) => { - if ( - nextOverflowedIds.length !== overflowedIds.length || - overflowedIds.reduce( - (a, b, i) => a || b !== nextOverflowedIds[i], - false, - ) - ) { - setOverflowedIds(nextOverflowedIds); + const renderHorizontalContent = useCallback( + () => ( +
css` + padding: 0 ${theme.gridUnit * 4}px; + min-width: 0; + flex: 1; + `} + > + } - }} - /> -
+ dropdownTriggerText={t('More filters')} + dropdownTriggerCount={activeOverflowedFiltersInScope.length} + dropdownTriggerTooltip={ + activeOverflowedFiltersInScope.length === 0 + ? t('No applied filters') + : t( + 'Applied filters: %s', + activeOverflowedFiltersInScope + .map(filter => filter.name) + .join(', '), + ) + } + dropdownContent={ + overflowedFiltersInScope.length || + overflowedCrossFilters.length || + (filtersOutOfScope.length && showCollapsePanel) + ? () => ( + + ) + : undefined + } + forceRender={hasRequiredFirst} + ref={popoverRef} + onOverflowingStateChange={({ overflowed: nextOverflowedIds }) => { + if ( + nextOverflowedIds.length !== overflowedIds.length || + overflowedIds.reduce( + (a, b, i) => a || b !== nextOverflowedIds[i], + false, + ) + ) { + setOverflowedIds(nextOverflowedIds); + } + }} + /> +
+ ), + [ + items, + activeOverflowedFiltersInScope, + overflowedFiltersInScope, + overflowedCrossFilters, + filtersOutOfScope, + showCollapsePanel, + renderer, + rendererCrossFilter, + hasRequiredFirst, + overflowedIds, + ], ); const overflowedByIndex = useMemo(() => { 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 45ccd4dd4119a..454d9060321e9 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -221,7 +221,7 @@ const FilterValue: FC = ({ datasetId, groupby, handleFilterLoadFinish, - JSON.stringify(filter), + filter, hasDataSource, isRefreshing, shouldRefresh, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx index 2b96d9963fc45..e97a8768e063b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Horizontal.tsx @@ -17,18 +17,19 @@ * under the License. */ -import { FC, memo } from 'react'; +import { FC, memo, useMemo } from 'react'; import { DataMaskStateWithId, FeatureFlag, isFeatureEnabled, - JsonObject, styled, t, } from '@superset-ui/core'; import Icons from 'src/components/Icons'; import Loading from 'src/components/Loading'; -import { DashboardLayout, RootState } from 'src/dashboard/types'; +import { RootState } from 'src/dashboard/types'; +import { useChartLayoutItems } from 'src/dashboard/util/useChartLayoutItems'; +import { useChartIds } from 'src/dashboard/util/charts/useChartIds'; import { useSelector } from 'react-redux'; import FilterControls from './FilterControls/FilterControls'; import { useChartsVerboseMaps, getFilterBarTestId } from './utils'; @@ -36,6 +37,7 @@ import { HorizontalBarProps } from './types'; import FilterBarSettings from './FilterBarSettings'; import FilterConfigurationLink from './FilterConfigurationLink'; import crossFiltersSelector from './CrossFilters/selectors'; +import { CrossFilterIndicator } from '../selectors'; const HorizontalBar = styled.div` ${({ theme }) => ` @@ -96,6 +98,7 @@ const FiltersLinkContainer = styled.div<{ hasFilters: boolean }>` `} `; +const EMPTY_ARRAY: CrossFilterIndicator[] = []; const HorizontalFilterBar: FC = ({ actions, canEdit, @@ -108,25 +111,26 @@ const HorizontalFilterBar: FC = ({ const dataMask = useSelector( state => state.dataMask, ); - const chartConfiguration = useSelector( - state => state.dashboardInfo.metadata?.chart_configuration, - ); - const dashboardLayout = useSelector( - state => state.dashboardLayout.present, - ); + const chartIds = useChartIds(); + const chartLayoutItems = useChartLayoutItems(); const isCrossFiltersEnabled = isFeatureEnabled( FeatureFlag.DashboardCrossFilters, ); const verboseMaps = useChartsVerboseMaps(); - const selectedCrossFilters = isCrossFiltersEnabled - ? crossFiltersSelector({ - dataMask, - chartConfiguration, - dashboardLayout, - verboseMaps, - }) - : []; + const selectedCrossFilters = useMemo( + () => + isCrossFiltersEnabled + ? crossFiltersSelector({ + dataMask, + chartIds, + chartLayoutItems, + verboseMaps, + }) + : EMPTY_ARRAY, + [chartIds, chartLayoutItems, dataMask, isCrossFiltersEnabled, verboseMaps], + ); + const hasFilters = filterValues.length > 0 || selectedCrossFilters.length > 0; return ( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index c25134715d83e..0b64d37a25169 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -24,8 +24,8 @@ import { useEffect, useState, useCallback, - createContext, useRef, + useMemo, } from 'react'; import { useDispatch, useSelector } from 'react-redux'; @@ -129,7 +129,6 @@ const publishDataMask = debounce( SLOW_DEBOUNCE, ); -export const FilterBarScrollContext = createContext(false); const FilterBar: FC = ({ orientation = FilterBarOrientation.Vertical, verticalConfig, @@ -144,8 +143,11 @@ const FilterBar: FC = ({ const tabId = useTabId(); const filters = useFilters(); const previousFilters = usePrevious(filters); - const filterValues = Object.values(filters); - const nativeFilterValues = filterValues.filter(isNativeFilter); + const filterValues = useMemo(() => Object.values(filters), [filters]); + const nativeFilterValues = useMemo( + () => filterValues.filter(isNativeFilter), + [filterValues], + ); const dashboardId = useSelector( ({ dashboardInfo }) => dashboardInfo?.id, ); @@ -212,14 +214,9 @@ const FilterBar: FC = ({ if (!isEmpty(updates)) { setDataMaskSelected(draft => ({ ...draft, ...updates })); - Object.keys(updates).forEach(key => dispatch(clearDataMask(key))); } } - }, [ - JSON.stringify(filters), - JSON.stringify(previousFilters), - previousDashboardId, - ]); + }, [dashboardId, filters, previousDashboardId, setDataMaskSelected]); const dataMaskAppliedText = JSON.stringify(dataMaskApplied); @@ -276,16 +273,27 @@ const FilterBar: FC = ({ ); const isInitialized = useInitialization(); - const actions = ( - + const actions = useMemo( + () => ( + + ), + [ + orientation, + verticalConfig?.width, + handleApply, + handleClearAll, + dataMaskSelected, + dataMaskAppliedText, + isApplyDisabled, + ], ); const filterBarComponent = diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterOutlined.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterOutlined.ts index 05c9b671f8dad..b2448fcce7b57 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterOutlined.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/useFilterOutlined.ts @@ -18,17 +18,26 @@ */ import { useSelector } from 'react-redux'; +import { createSelector } from '@reduxjs/toolkit'; import { RootState } from 'src/dashboard/types'; import getChartAndLabelComponentIdFromPath from 'src/dashboard/util/getChartAndLabelComponentIdFromPath'; +const filterOutlinedSelector = createSelector( + [ + (state: RootState) => state.dashboardState.directPathToChild, + (state: RootState) => state.dashboardState.directPathLastUpdated, + ], + (directPathToChild, directPathLastUpdated) => ({ + outlinedFilterId: ( + getChartAndLabelComponentIdFromPath(directPathToChild || []) as Record< + string, + string + > + )?.native_filter, + lastUpdated: directPathLastUpdated, + }), +); export const useFilterOutlined = () => useSelector( - state => ({ - outlinedFilterId: ( - getChartAndLabelComponentIdFromPath( - state.dashboardState.directPathToChild || [], - ) as Record - )?.native_filter, - lastUpdated: state.dashboardState.directPathLastUpdated, - }), + filterOutlinedSelector, ); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts index cdf488bc3dda8..6a6ca3a3e3a5d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts @@ -22,6 +22,7 @@ import { DataMaskStateWithId, Filter, FilterState } from '@superset-ui/core'; import { testWithId } from 'src/utils/testUtils'; import { RootState } from 'src/dashboard/types'; import { useSelector } from 'react-redux'; +import { createSelector } from '@reduxjs/toolkit'; export const getOnlyExtraFormData = (data: DataMaskStateWithId) => Object.values(data).reduce( @@ -64,20 +65,26 @@ export const checkIsApplyDisabled = ( ); }; +const chartsVerboseMapSelector = createSelector( + [ + (state: RootState) => state.sliceEntities.slices, + (state: RootState) => state.datasources, + ], + (slices, datasources) => + Object.keys(slices).reduce((chartsVerboseMaps, chartId) => { + const chartDatasource = slices[chartId]?.datasource + ? datasources[slices[chartId].datasource] + : undefined; + return { + ...chartsVerboseMaps, + [chartId]: chartDatasource ? chartDatasource.verbose_map : {}, + }; + }, {}), +); + export const useChartsVerboseMaps = () => useSelector }>( - state => { - const { charts, datasources } = state; - - return Object.keys(state.charts).reduce((chartsVerboseMaps, chartId) => { - const chartDatasource = - datasources[charts[chartId]?.form_data?.datasource]; - return { - ...chartsVerboseMaps, - [chartId]: chartDatasource ? chartDatasource.verbose_map : {}, - }; - }, {}); - }, + chartsVerboseMapSelector, ); export const FILTER_BAR_TEST_ID = 'filter-bar'; 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 319fecec0a423..2f2f4e4525e42 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterCard/FilterCard.test.tsx @@ -86,6 +86,9 @@ const baseInitialState = { id: 3, }, }, + dashboardState: { + sliceIds: [1, 2, 3], + }, dashboardLayout: { past: [], future: [], diff --git a/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts b/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts index 36f0fd1925dd9..232ff57de2ffe 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/selectors.ts @@ -32,11 +32,7 @@ import { } from '@superset-ui/core'; import { TIME_FILTER_MAP } from 'src/explore/constants'; import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters'; -import { - ChartConfiguration, - DashboardLayout, - Layout, -} from 'src/dashboard/types'; +import { ChartConfiguration, LayoutItem } from 'src/dashboard/types'; import { areObjectsEqual } from 'src/reduxUtils'; export enum IndicatorStatus { @@ -170,7 +166,7 @@ export type CrossFilterIndicator = Indicator & { emitterId: number }; export const getCrossFilterIndicator = ( chartId: number, dataMask: DataMask, - dashboardLayout: DashboardLayout, + chartLayoutItems: LayoutItem[], ) => { const filterState = dataMask?.filterState; const filters = dataMask?.extraFormData?.filters; @@ -179,19 +175,17 @@ export const getCrossFilterIndicator = ( const column = filters?.[0]?.col || (filtersState && Object.keys(filtersState)[0]); - const dashboardLayoutItem = Object.values(dashboardLayout).find( + const chartLayoutItem = chartLayoutItems.find( layoutItem => layoutItem?.meta?.chartId === chartId, ); + const filterObject: Indicator = { column, name: - dashboardLayoutItem?.meta?.sliceNameOverride || - dashboardLayoutItem?.meta?.sliceName || + chartLayoutItem?.meta?.sliceNameOverride || + chartLayoutItem?.meta?.sliceName || '', - path: [ - ...(dashboardLayoutItem?.parents ?? []), - dashboardLayoutItem?.id || '', - ], + path: [...(chartLayoutItem?.parents ?? []), chartLayoutItem?.id || ''], value: label, }; return filterObject; @@ -288,7 +282,7 @@ const defaultChartConfig = {}; export const selectChartCrossFilters = ( dataMask: DataMaskStateWithId, chartId: number, - dashboardLayout: Layout, + chartLayoutItems: LayoutItem[], chartConfiguration: ChartConfiguration = defaultChartConfig, appliedColumns: Set, rejectedColumns: Set, @@ -312,7 +306,7 @@ export const selectChartCrossFilters = ( const filterIndicator = getCrossFilterIndicator( Number(chartConfig.id), dataMask[chartConfig.id], - dashboardLayout, + chartLayoutItems, ); const filterStatus = getStatus({ label: filterIndicator.value, @@ -339,7 +333,7 @@ export const selectNativeIndicatorsForChart = ( dataMask: DataMaskStateWithId, chartId: number, chart: any, - dashboardLayout: Layout, + chartLayoutItems: LayoutItem[], chartConfiguration: ChartConfiguration = defaultChartConfig, ): Indicator[] => { const appliedColumns = getAppliedColumns(chart); @@ -351,7 +345,7 @@ export const selectNativeIndicatorsForChart = ( areObjectsEqual(cachedFilterData?.appliedColumns, appliedColumns) && areObjectsEqual(cachedFilterData?.rejectedColumns, rejectedColumns) && cachedFilterData?.nativeFilters === nativeFilters && - cachedFilterData?.dashboardLayout === dashboardLayout && + cachedFilterData?.chartLayoutItems === chartLayoutItems && cachedFilterData?.chartConfiguration === chartConfiguration && cachedFilterData?.dataMask === dataMask ) { @@ -389,7 +383,7 @@ export const selectNativeIndicatorsForChart = ( crossFilterIndicators = selectChartCrossFilters( dataMask, chartId, - dashboardLayout, + chartLayoutItems, chartConfiguration, appliedColumns, rejectedColumns, @@ -399,7 +393,7 @@ export const selectNativeIndicatorsForChart = ( cachedNativeIndicatorsForChart[chartId] = indicators; cachedNativeFilterDataForChart[chartId] = { nativeFilters, - dashboardLayout, + chartLayoutItems, chartConfiguration, dataMask, appliedColumns, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.ts index 5bf71116c358e..06774b87c7cc0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.ts @@ -17,7 +17,7 @@ * under the License. */ import { useSelector } from 'react-redux'; -import { useMemo } from 'react'; +import { useCallback, useMemo } from 'react'; import { Filter, FilterConfiguration, @@ -25,7 +25,7 @@ import { isFilterDivider, } from '@superset-ui/core'; import { ActiveTabs, DashboardLayout, RootState } from '../../types'; -import { TAB_TYPE } from '../../util/componentTypes'; +import { CHART_TYPE, TAB_TYPE } from '../../util/componentTypes'; const defaultFilterConfiguration: Filter[] = []; @@ -79,14 +79,22 @@ function useActiveDashboardTabs() { function useSelectChartTabParents() { const dashboardLayout = useDashboardLayout(); - return (chartId: number) => { - const chartLayoutItem = Object.values(dashboardLayout).find( - layoutItem => layoutItem.meta?.chartId === chartId, - ); - return chartLayoutItem?.parents?.filter( - (parent: string) => dashboardLayout[parent]?.type === TAB_TYPE, - ); - }; + const layoutChartItems = useMemo( + () => + Object.values(dashboardLayout).filter(item => item.type === CHART_TYPE), + [dashboardLayout], + ); + return useCallback( + (chartId: number) => { + const chartLayoutItem = layoutChartItems.find( + layoutItem => layoutItem.meta?.chartId === chartId, + ); + return chartLayoutItem?.parents?.filter( + (parent: string) => dashboardLayout[parent]?.type === TAB_TYPE, + ); + }, + [dashboardLayout, layoutChartItems], + ); } export function useIsFilterInScope() { @@ -97,16 +105,19 @@ export function useIsFilterInScope() { // Chart is visible if it's placed in an active tab tree or if it's not attached to any tab. // Chart is in an active tab tree if all of it's ancestors of type TAB are active // Dividers are always in scope - return (filter: Filter | Divider) => - isFilterDivider(filter) || - ('chartsInScope' in filter && - filter.chartsInScope?.some((chartId: number) => { - const tabParents = selectChartTabParents(chartId); - return ( - tabParents?.length === 0 || - tabParents?.every(tab => activeTabs.includes(tab)) - ); - })); + return useCallback( + (filter: Filter | Divider) => + isFilterDivider(filter) || + ('chartsInScope' in filter && + filter.chartsInScope?.some((chartId: number) => { + const tabParents = selectChartTabParents(chartId); + return ( + tabParents?.length === 0 || + tabParents?.every(tab => activeTabs.includes(tab)) + ); + })), + [selectChartTabParents, activeTabs], + ); } export function useSelectFiltersInScope(filters: (Filter | Divider)[]) { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts index 095a9edaf3de8..db21db547d597 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts @@ -19,6 +19,7 @@ import { Behavior, FeatureFlag } from '@superset-ui/core'; import * as uiCore from '@superset-ui/core'; import { DashboardLayout } from 'src/dashboard/types'; +import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; import { nativeFilterGate, findTabsWithChartsInScope } from './utils'; let isFeatureEnabledMock: jest.MockInstance; @@ -119,7 +120,10 @@ test('findTabsWithChartsInScope should handle a recursive layout structure', () }, } as any as DashboardLayout; - expect(Array.from(findTabsWithChartsInScope(dashboardLayout, []))).toEqual( + const chartLayoutItems = Object.values(dashboardLayout).filter( + item => item.type === CHART_TYPE, + ); + expect(Array.from(findTabsWithChartsInScope(chartLayoutItems, []))).toEqual( [], ); }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts index 0ba28d0a3a3f9..734e0ce91fe9d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts @@ -30,10 +30,9 @@ import { QueryFormData, t, } from '@superset-ui/core'; -import { DashboardLayout } from 'src/dashboard/types'; +import { LayoutItem } from 'src/dashboard/types'; import extractUrlParams from 'src/dashboard/util/extractUrlParams'; -import { CHART_TYPE, TAB_TYPE } from '../../util/componentTypes'; -import { DASHBOARD_GRID_ID, DASHBOARD_ROOT_ID } from '../../util/constants'; +import { TAB_TYPE } from '../../util/componentTypes'; import getBootstrapData from '../../../utils/getBootstrapData'; const getDefaultRowLimit = (): number => { @@ -156,84 +155,20 @@ export function nativeFilterGate(behaviors: Behavior[]): boolean { ); } -const isComponentATab = ( - dashboardLayout: DashboardLayout, - componentId: string, -) => dashboardLayout?.[componentId]?.type === TAB_TYPE; - -const findTabsWithChartsInScopeHelper = ( - dashboardLayout: DashboardLayout, - chartsInScope: number[], - componentId: string, - tabIds: string[], - tabsToHighlight: Set, - visited: Set, -) => { - if (visited.has(componentId)) { - return; - } - visited.add(componentId); - if ( - dashboardLayout?.[componentId]?.type === CHART_TYPE && - chartsInScope.includes(dashboardLayout[componentId]?.meta?.chartId) - ) { - tabIds.forEach(tabsToHighlight.add, tabsToHighlight); - } - if ( - dashboardLayout?.[componentId]?.children?.length === 0 || - (isComponentATab(dashboardLayout, componentId) && - tabsToHighlight.has(componentId)) - ) { - return; - } - dashboardLayout[componentId]?.children.forEach(childId => - findTabsWithChartsInScopeHelper( - dashboardLayout, - chartsInScope, - childId, - isComponentATab(dashboardLayout, childId) ? [...tabIds, childId] : tabIds, - tabsToHighlight, - visited, - ), - ); -}; - export const findTabsWithChartsInScope = ( - dashboardLayout: DashboardLayout, + chartLayoutItems: LayoutItem[], chartsInScope: number[], -) => { - const dashboardRoot = dashboardLayout[DASHBOARD_ROOT_ID]; - const rootChildId = dashboardRoot.children[0]; - const hasTopLevelTabs = rootChildId !== DASHBOARD_GRID_ID; - const tabsInScope = new Set(); - const visited = new Set(); - if (hasTopLevelTabs) { - dashboardLayout[rootChildId]?.children?.forEach(tabId => - findTabsWithChartsInScopeHelper( - dashboardLayout, - chartsInScope, - tabId, - [tabId], - tabsInScope, - visited, - ), - ); - } else { - Object.values(dashboardLayout) - .filter(element => element?.type === TAB_TYPE) - .forEach(element => - findTabsWithChartsInScopeHelper( - dashboardLayout, - chartsInScope, - element.id, - [element.id], - tabsInScope, - visited, - ), - ); - } - return tabsInScope; -}; +) => + new Set( + chartsInScope + .map(chartId => + chartLayoutItems + .find(item => item?.meta?.chartId === chartId) + ?.parents?.filter(parent => parent.startsWith(`${TAB_TYPE}-`)), + ) + .filter(id => id !== undefined) + .flat() as string[], + ); export const getFilterValueForDisplay = ( value?: string[] | null | string | number | object, diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx deleted file mode 100644 index 9f00bd0bf7096..0000000000000 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ /dev/null @@ -1,135 +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 { bindActionCreators } from 'redux'; -import { connect } from 'react-redux'; -import { - toggleExpandSlice, - setFocusedFilterField, - unsetFocusedFilterField, -} from 'src/dashboard/actions/dashboardState'; -import { updateComponents } from 'src/dashboard/actions/dashboardLayout'; -import { changeFilter } from 'src/dashboard/actions/dashboardFilters'; -import { - addSuccessToast, - addDangerToast, -} from 'src/components/MessageToasts/actions'; -import { refreshChart } from 'src/components/Chart/chartAction'; -import { logEvent } from 'src/logger/actions'; -import { - getActiveFilters, - getAppliedFilterValues, -} from 'src/dashboard/util/activeDashboardFilters'; -import getFormDataWithExtraFilters from 'src/dashboard/util/charts/getFormDataWithExtraFilters'; -import Chart from 'src/dashboard/components/gridComponents/Chart'; -import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants'; -import { enforceSharedLabelsColorsArray } from 'src/utils/colorScheme'; - -const EMPTY_OBJECT = {}; - -function mapStateToProps( - { - charts: chartQueries, - dashboardInfo, - dashboardState, - dataMask, - datasources, - sliceEntities, - nativeFilters, - common, - }, - ownProps, -) { - const { id, extraControls, setControlValue } = ownProps; - const chart = chartQueries[id] || EMPTY_OBJECT; - const datasource = - (chart && chart.form_data && datasources[chart.form_data.datasource]) || - PLACEHOLDER_DATASOURCE; - const { - colorScheme: appliedColorScheme, - colorNamespace, - datasetsStatus, - } = dashboardState; - const labelsColor = dashboardInfo?.metadata?.label_colors || {}; - const labelsColorMap = dashboardInfo?.metadata?.map_label_colors || {}; - const sharedLabelsColors = enforceSharedLabelsColorsArray( - dashboardInfo?.metadata?.shared_label_colors, - ); - const ownColorScheme = chart.form_data?.color_scheme; - // note: this method caches filters if possible to prevent render cascades - const formData = getFormDataWithExtraFilters({ - chart, - chartConfiguration: dashboardInfo.metadata?.chart_configuration, - charts: chartQueries, - filters: getAppliedFilterValues(id), - colorNamespace, - colorScheme: appliedColorScheme, - ownColorScheme, - sliceId: id, - nativeFilters: nativeFilters?.filters, - allSliceIds: dashboardState.sliceIds, - dataMask, - extraControls, - labelsColor, - labelsColorMap, - sharedLabelsColors, - }); - - formData.dashboardId = dashboardInfo.id; - - return { - chart, - datasource, - labelsColor, - labelsColorMap, - slice: sliceEntities.slices[id], - timeout: dashboardInfo.common.conf.SUPERSET_WEBSERVER_TIMEOUT, - filters: getActiveFilters() || EMPTY_OBJECT, - formData, - editMode: dashboardState.editMode, - isExpanded: !!dashboardState.expandedSlices[id], - supersetCanExplore: !!dashboardInfo.superset_can_explore, - supersetCanShare: !!dashboardInfo.superset_can_share, - supersetCanCSV: !!dashboardInfo.superset_can_csv, - ownState: dataMask[id]?.ownState, - filterState: dataMask[id]?.filterState, - maxRows: common.conf.SQL_MAX_ROW, - setControlValue, - datasetsStatus, - emitCrossFilters: !!dashboardInfo.crossFiltersEnabled, - }; -} - -function mapDispatchToProps(dispatch) { - return bindActionCreators( - { - updateComponents, - addSuccessToast, - addDangerToast, - toggleExpandSlice, - changeFilter, - setFocusedFilterField, - unsetFocusedFilterField, - refreshChart, - logEvent, - }, - dispatch, - ); -} - -export default connect(mapStateToProps, mapDispatchToProps)(Chart); diff --git a/superset-frontend/src/dashboard/containers/Dashboard.ts b/superset-frontend/src/dashboard/containers/Dashboard.ts index ab1f7c46fa40a..a59767c004a2e 100644 --- a/superset-frontend/src/dashboard/containers/Dashboard.ts +++ b/superset-frontend/src/dashboard/containers/Dashboard.ts @@ -43,8 +43,10 @@ function mapStateToProps(state: RootState) { return { timeout: dashboardInfo.common?.conf?.SUPERSET_WEBSERVER_TIMEOUT, userId: dashboardInfo.userId, - dashboardInfo, - dashboardState, + dashboardId: dashboardInfo.id, + editMode: dashboardState.editMode, + isPublished: dashboardState.isPublished, + hasUnsavedChanges: dashboardState.hasUnsavedChanges, datasources, chartConfiguration: dashboardInfo.metadata?.chart_configuration, slices: sliceEntities.slices, diff --git a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx index bf92c5dcecaeb..5f3b7750a732d 100644 --- a/superset-frontend/src/dashboard/containers/DashboardComponent.jsx +++ b/superset-frontend/src/dashboard/containers/DashboardComponent.jsx @@ -16,16 +16,15 @@ * specific language governing permissions and limitations * under the License. */ -import { PureComponent } from 'react'; +import { useCallback, memo, useMemo } from 'react'; import PropTypes from 'prop-types'; import { bindActionCreators } from 'redux'; -import { connect } from 'react-redux'; +import { useSelector, useDispatch } from 'react-redux'; import { logEvent } from 'src/logger/actions'; import { addDangerToast } from 'src/components/MessageToasts/actions'; import { componentLookup } from 'src/dashboard/components/gridComponents'; import getDetailedComponentWidth from 'src/dashboard/util/getDetailedComponentWidth'; import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters'; -import { componentShape } from 'src/dashboard/util/propShapes'; import { COLUMN_TYPE, ROW_TYPE } from 'src/dashboard/util/componentTypes'; import { createComponent, @@ -47,85 +46,91 @@ const propTypes = { renderHoverMenu: PropTypes.bool, renderTabContent: PropTypes.bool, onChangeTab: PropTypes.func, - component: componentShape.isRequired, - parentComponent: componentShape.isRequired, - createComponent: PropTypes.func.isRequired, - deleteComponent: PropTypes.func.isRequired, - updateComponents: PropTypes.func.isRequired, - handleComponentDrop: PropTypes.func.isRequired, - logEvent: PropTypes.func.isRequired, directPathToChild: PropTypes.arrayOf(PropTypes.string), directPathLastUpdated: PropTypes.number, - dashboardId: PropTypes.number.isRequired, isComponentVisible: PropTypes.bool, }; -const defaultProps = { - isComponentVisible: true, -}; +const DashboardComponent = props => { + const dispatch = useDispatch(); + const dashboardLayout = useSelector(state => state.dashboardLayout.present); + const dashboardInfo = useSelector(state => state.dashboardInfo); + const editMode = useSelector(state => state.dashboardState.editMode); + const fullSizeChartId = useSelector( + state => state.dashboardState.fullSizeChartId, + ); + const dashboardId = dashboardInfo.id; + const component = dashboardLayout[props.id]; + const parentComponent = dashboardLayout[props.parentId]; + const getComponentById = useCallback( + id => dashboardLayout[id], + [dashboardLayout], + ); + const { isComponentVisible = true } = props; + const filters = getActiveFilters(); -function mapStateToProps( - { dashboardLayout: undoableLayout, dashboardState, dashboardInfo }, - ownProps, -) { - const dashboardLayout = undoableLayout.present; - const { id, parentId } = ownProps; - const component = dashboardLayout[id]; - const props = { - component, - getComponentById: id => dashboardLayout[id], - parentComponent: dashboardLayout[parentId], - editMode: dashboardState.editMode, - filters: getActiveFilters(), - dashboardId: dashboardInfo.id, - dashboardInfo, - fullSizeChartId: dashboardState.fullSizeChartId, - }; + const boundActionCreators = useMemo( + () => + bindActionCreators( + { + addDangerToast, + createComponent, + deleteComponent, + updateComponents, + handleComponentDrop, + setDirectPathToChild, + setFullSizeChartId, + setActiveTab, + logEvent, + }, + dispatch, + ), + [dispatch], + ); // rows and columns need more data about their child dimensions // doing this allows us to not pass the entire component lookup to all Components - if (component) { - const componentType = component.type; - if (componentType === ROW_TYPE || componentType === COLUMN_TYPE) { - const { occupiedWidth, minimumWidth } = getDetailedComponentWidth({ - id, - components: dashboardLayout, - }); + const { occupiedColumnCount, minColumnWidth } = useMemo(() => { + if (component) { + const componentType = component.type; + if (componentType === ROW_TYPE || componentType === COLUMN_TYPE) { + const { occupiedWidth, minimumWidth } = getDetailedComponentWidth({ + id: props.id, + components: dashboardLayout, + }); - if (componentType === ROW_TYPE) props.occupiedColumnCount = occupiedWidth; - if (componentType === COLUMN_TYPE) props.minColumnWidth = minimumWidth; + if (componentType === ROW_TYPE) { + return { occupiedColumnCount: occupiedWidth }; + } + if (componentType === COLUMN_TYPE) { + return { minColumnWidth: minimumWidth }; + } + } + return {}; } - } + return {}; + }, [component, dashboardLayout, props.id]); - return props; -} - -function mapDispatchToProps(dispatch) { - return bindActionCreators( - { - addDangerToast, - createComponent, - deleteComponent, - updateComponents, - handleComponentDrop, - setDirectPathToChild, - setFullSizeChartId, - setActiveTab, - logEvent, - }, - dispatch, - ); -} - -class DashboardComponent extends PureComponent { - render() { - const { component } = this.props; - const Component = component ? componentLookup[component.type] : null; - return Component ? : null; - } -} + const Component = component ? componentLookup[component.type] : null; + return Component ? ( + + ) : null; +}; DashboardComponent.propTypes = propTypes; -DashboardComponent.defaultProps = defaultProps; -export default connect(mapStateToProps, mapDispatchToProps)(DashboardComponent); +export default memo(DashboardComponent); diff --git a/superset-frontend/src/dashboard/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index d8ddf0f19e73a..384caddc04fd3 100644 --- a/superset-frontend/src/dashboard/containers/DashboardPage.tsx +++ b/superset-frontend/src/dashboard/containers/DashboardPage.tsx @@ -83,16 +83,20 @@ const selectRelevantDatamask = createSelector( dataMask => getRelevantDataMask(dataMask, 'ownState'), // the second parameter conducts the transformation ); +const selectChartConfiguration = (state: RootState) => + state.dashboardInfo.metadata?.chart_configuration; +const selectNativeFilters = (state: RootState) => state.nativeFilters.filters; +const selectDataMask = (state: RootState) => state.dataMask; +const selectAllSliceIds = (state: RootState) => state.dashboardState.sliceIds; // TODO: move to Dashboard.jsx when it's refactored to functional component const selectActiveFilters = createSelector( - (state: RootState) => ({ - // eslint-disable-next-line camelcase - chartConfiguration: state.dashboardInfo.metadata?.chart_configuration, - nativeFilters: state.nativeFilters.filters, - dataMask: state.dataMask, - allSliceIds: state.dashboardState.sliceIds, - }), - ({ chartConfiguration, nativeFilters, dataMask, allSliceIds }) => ({ + [ + selectChartConfiguration, + selectNativeFilters, + selectDataMask, + selectAllSliceIds, + ], + (chartConfiguration, nativeFilters, dataMask, allSliceIds) => ({ ...getActiveFilters(), ...getAllActiveFilters({ // eslint-disable-next-line camelcase @@ -226,19 +230,23 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { const relevantDataMask = useSelector(selectRelevantDatamask); const activeFilters = useSelector(selectActiveFilters); + const globalStyles = useMemo( + () => [ + filterCardPopoverStyle(theme), + headerStyles(theme), + chartContextMenuStyles(theme), + focusStyle(theme), + chartHeaderStyles(theme), + ], + [theme], + ); + if (error) throw error; // caught in error boundary + const DashboardBuilderComponent = useMemo(() => , []); return ( <> - + {readyToRender && hasDashboardInfoInitiated ? ( <> @@ -247,7 +255,7 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { activeFilters={activeFilters} ownDataCharts={relevantDataMask} > - + {DashboardBuilderComponent} diff --git a/superset-frontend/src/dashboard/util/charts/useChartIds.ts b/superset-frontend/src/dashboard/util/charts/useChartIds.ts index c95327a94a559..9afc16d9f4e75 100644 --- a/superset-frontend/src/dashboard/util/charts/useChartIds.ts +++ b/superset-frontend/src/dashboard/util/charts/useChartIds.ts @@ -17,20 +17,7 @@ * under the License. */ import { useSelector } from 'react-redux'; -import { isEqual } from 'lodash'; -import { createSelector } from '@reduxjs/toolkit'; import { RootState } from 'src/dashboard/types'; -import { useMemoCompare } from 'src/hooks/useMemoCompare'; -const chartIdsSelector = createSelector( - (state: RootState) => state.charts, - charts => Object.values(charts).map(chart => chart.id), -); - -export const useChartIds = () => { - const chartIds = useSelector(chartIdsSelector); - return useMemoCompare( - chartIds, - (prev, next) => prev === next || isEqual(prev, next), - ); -}; +export const useChartIds = () => + useSelector(state => state.dashboardState.sliceIds); diff --git a/superset-frontend/src/dashboard/util/crossFilters.ts b/superset-frontend/src/dashboard/util/crossFilters.ts index 903a1f74fbe55..123435a430a1f 100644 --- a/superset-frontend/src/dashboard/util/crossFilters.ts +++ b/superset-frontend/src/dashboard/util/crossFilters.ts @@ -33,6 +33,7 @@ import { isCrossFilterScopeGlobal, } from '../types'; import { DEFAULT_CROSS_FILTER_SCOPING } from '../constants'; +import { CHART_TYPE } from './componentTypes'; export const isCrossFiltersEnabled = ( metadataCrossFiltersEnabled: boolean | undefined, @@ -52,13 +53,17 @@ export const getCrossFiltersConfiguration = ( return undefined; } + const chartLayoutItems = Object.values(dashboardLayout).filter( + item => item?.type === CHART_TYPE, + ); + const globalChartConfiguration = metadata.global_chart_configuration?.scope ? { scope: metadata.global_chart_configuration.scope, chartsInScope: getChartIdsInFilterScope( metadata.global_chart_configuration.scope, Object.values(charts).map(chart => chart.id), - dashboardLayout, + chartLayoutItems, ), } : { @@ -69,7 +74,7 @@ export const getCrossFiltersConfiguration = ( // If user just added cross filter to dashboard it's not saving its scope on server, // so we tweak it until user will update scope and will save it in server const chartConfiguration = {}; - Object.values(dashboardLayout).forEach(layoutItem => { + chartLayoutItems.forEach(layoutItem => { const chartId = layoutItem.meta?.chartId; if (!isDefined(chartId)) { @@ -105,7 +110,7 @@ export const getCrossFiltersConfiguration = ( : getChartIdsInFilterScope( chartConfiguration[chartId].crossFilters.scope, Object.values(charts).map(chart => chart.id), - dashboardLayout, + chartLayoutItems, ); } }); diff --git a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts index 7e4b7b1273121..5bd37f26bcbd6 100644 --- a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts +++ b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts @@ -18,14 +18,13 @@ */ import { NativeFilterScope } from '@superset-ui/core'; import { CHART_TYPE } from './componentTypes'; -import { Layout } from '../types'; +import { LayoutItem } from '../types'; export function getChartIdsInFilterScope( filterScope: NativeFilterScope, chartIds: number[], - layout: Layout, + layoutItems: LayoutItem[], ) { - const layoutItems = Object.values(layout); return chartIds.filter( chartId => !filterScope.excluded.includes(chartId) && diff --git a/superset-frontend/src/dashboard/util/useChartLayoutItems.ts b/superset-frontend/src/dashboard/util/useChartLayoutItems.ts new file mode 100644 index 0000000000000..10635fc05b41c --- /dev/null +++ b/superset-frontend/src/dashboard/util/useChartLayoutItems.ts @@ -0,0 +1,29 @@ +/** + * 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 { createSelector } from '@reduxjs/toolkit'; +import { useSelector } from 'react-redux'; +import { RootState } from '../types'; +import { CHART_TYPE } from './componentTypes'; + +const chartLayoutItemsSelector = createSelector( + (state: RootState) => state.dashboardLayout.present, + layout => Object.values(layout).filter(item => item?.type === CHART_TYPE), +); + +export const useChartLayoutItems = () => useSelector(chartLayoutItemsSelector); diff --git a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx index 80e6038c0726a..a2047bf2aa618 100644 --- a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx +++ b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx @@ -141,54 +141,4 @@ describe('useFilterFocusHighlightStyles', () => { const styles = getComputedStyle(container); expect(parseFloat(styles.opacity)).toBe(1); }); - - it('should return unfocused styles if focusedFilterField is targeting a different chart', async () => { - const chartId = 18; - mockGetRelatedCharts.mockReturnValue([]); - const store = createMockStore({ - dashboardState: { - focusedFilterField: { - chartId: 10, - column: 'test', - }, - }, - dashboardFilters: { - 10: { - scopes: {}, - }, - }, - }); - renderWrapper(chartId, store); - - const container = screen.getByTestId('test-component'); - - const styles = getComputedStyle(container); - expect(parseFloat(styles.opacity)).toBe(0.3); - }); - - it('should return focused styles if focusedFilterField chart equals our own', async () => { - const chartId = 18; - mockGetRelatedCharts.mockReturnValue([chartId]); - const store = createMockStore({ - dashboardState: { - focusedFilterField: { - chartId, - column: 'test', - }, - }, - dashboardFilters: { - [chartId]: { - scopes: { - otherColumn: {}, - }, - }, - }, - }); - renderWrapper(chartId, store); - - const container = screen.getByTestId('test-component'); - - const styles = getComputedStyle(container); - expect(parseFloat(styles.opacity)).toBe(1); - }); }); diff --git a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts index aa636cb1ee55d..3dea0d54ac138 100644 --- a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts +++ b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts @@ -16,40 +16,30 @@ * specific language governing permissions and limitations * under the License. */ +import { useMemo } from 'react'; import { Filter, useTheme } from '@superset-ui/core'; import { useSelector } from 'react-redux'; -import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters'; -import { DashboardState, RootState } from 'src/dashboard/types'; +import { RootState } from 'src/dashboard/types'; import { getRelatedCharts } from './getRelatedCharts'; -const selectFocusedFilterScope = ( - dashboardState: DashboardState, - dashboardFilters: any, -) => { - if (!dashboardState.focusedFilterField) return null; - const { chartId, column } = dashboardState.focusedFilterField; - return { - chartId, - scope: dashboardFilters[chartId].scopes[column], - }; -}; +const unfocusedChartStyles = { opacity: 0.3, pointerEvents: 'none' }; +const EMPTY = {}; const useFilterFocusHighlightStyles = (chartId: number) => { const theme = useTheme(); - const nativeFilters = useSelector((state: RootState) => state.nativeFilters); - const dashboardState = useSelector( - (state: RootState) => state.dashboardState, + const focusedChartStyles = useMemo( + () => ({ + borderColor: theme.colors.primary.light2, + opacity: 1, + boxShadow: `0px 0px ${theme.gridUnit * 2}px ${theme.colors.primary.base}`, + pointerEvents: 'auto', + }), + [theme], ); - const dashboardFilters = useSelector( - (state: RootState) => state.dashboardFilters, - ); - const focusedFilterScope = selectFocusedFilterScope( - dashboardState, - dashboardFilters, - ); + const nativeFilters = useSelector((state: RootState) => state.nativeFilters); const slices = useSelector((state: RootState) => state.sliceEntities.slices) || {}; @@ -57,8 +47,8 @@ const useFilterFocusHighlightStyles = (chartId: number) => { const highlightedFilterId = nativeFilters?.focusedFilterId || nativeFilters?.hoveredFilterId; - if (!(focusedFilterScope || highlightedFilterId)) { - return {}; + if (!highlightedFilterId) { + return EMPTY; } const relatedCharts = getRelatedCharts( @@ -67,29 +57,7 @@ const useFilterFocusHighlightStyles = (chartId: number) => { slices, ); - // we use local styles here instead of a conditionally-applied class, - // because adding any conditional class to this container - // causes performance issues in Chrome. - - // default to the "de-emphasized" state - const unfocusedChartStyles = { opacity: 0.3, pointerEvents: 'none' }; - const focusedChartStyles = { - borderColor: theme.colors.primary.light2, - opacity: 1, - boxShadow: `0px 0px ${theme.gridUnit * 2}px ${theme.colors.primary.base}`, - pointerEvents: 'auto', - }; - - if (highlightedFilterId) { - if (relatedCharts.includes(chartId)) { - return focusedChartStyles; - } - } else if ( - chartId === focusedFilterScope?.chartId || - getChartIdsInFilterScope({ - filterScope: focusedFilterScope?.scope, - }).includes(chartId) - ) { + if (highlightedFilterId && relatedCharts.includes(chartId)) { return focusedChartStyles; } diff --git a/superset-frontend/src/types/DashboardContextForExplore.ts b/superset-frontend/src/types/DashboardContextForExplore.ts index 117a182606cec..e9e964a1a3c05 100644 --- a/superset-frontend/src/types/DashboardContextForExplore.ts +++ b/superset-frontend/src/types/DashboardContextForExplore.ts @@ -41,4 +41,5 @@ export interface DashboardContextForExplore { } | {}; isRedundant?: boolean; + dashboardPageId?: string; } diff --git a/superset-frontend/src/utils/colorScheme.ts b/superset-frontend/src/utils/colorScheme.ts index 1b9f2d12ecef2..35f23d29f4d4d 100644 --- a/superset-frontend/src/utils/colorScheme.ts +++ b/superset-frontend/src/utils/colorScheme.ts @@ -19,10 +19,13 @@ import { CategoricalColorNamespace, + ensureIsArray, getCategoricalSchemeRegistry, getLabelsColorMap, } from '@superset-ui/core'; +const EMPTY_ARRAY: string[] = []; + /** * Force falsy namespace values to undefined to default to GLOBAL * @@ -41,7 +44,7 @@ export const getColorNamespace = (namespace?: string) => namespace || undefined; */ export const enforceSharedLabelsColorsArray = ( sharedLabelsColors: string[] | Record | undefined, -) => (Array.isArray(sharedLabelsColors) ? sharedLabelsColors : []); +) => (Array.isArray(sharedLabelsColors) ? sharedLabelsColors : EMPTY_ARRAY); /** * Get labels shared across all charts in a dashboard. @@ -67,7 +70,9 @@ export const getFreshSharedLabels = ( .filter(([, count]) => count > 1) .map(([label]) => label); - return Array.from(new Set([...currentSharedLabels, ...duplicates])); + return Array.from( + new Set([...ensureIsArray(currentSharedLabels), ...duplicates]), + ); }; export const getSharedLabelsColorMapEntries = (