From 0a4a7c413515ae96f352bfa9071d176669d9c845 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Mon, 2 Dec 2024 12:34:55 +0100 Subject: [PATCH] perf: Optimize DashboardPage and SyncDashboardState --- .../DashboardBuilder/DashboardBuilder.tsx | 8 +-- .../components/SyncDashboardState/index.tsx | 68 ++++++++++--------- .../dashboard/containers/DashboardPage.tsx | 46 ++++++++----- .../src/types/DashboardContextForExplore.ts | 1 + 4 files changed, 68 insertions(+), 55 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index 30c61f0af6553..e5caed6968642 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -18,7 +18,7 @@ */ /* eslint-env browser */ import cx from 'classnames'; -import { FC, useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { addAlpha, css, @@ -82,8 +82,6 @@ import DashboardContainer from './DashboardContainer'; import { useNativeFilters } from './state'; import DashboardWrapper from './DashboardWrapper'; -type DashboardBuilderProps = {}; - // @z-index-above-dashboard-charts + 1 = 11 const FiltersPanel = styled.div<{ width: number; hidden: boolean }>` grid-column: 1; @@ -374,7 +372,7 @@ const ELEMENT_ON_SCREEN_OPTIONS = { threshold: [1], }; -const DashboardBuilder: FC = () => { +const DashboardBuilder = () => { const dispatch = useDispatch(); const uiConfig = useUiConfig(); const theme = useTheme(); @@ -737,4 +735,4 @@ const DashboardBuilder: FC = () => { ); }; -export default DashboardBuilder; +export default memo(DashboardBuilder); 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/containers/DashboardPage.tsx b/superset-frontend/src/dashboard/containers/DashboardPage.tsx index d8ddf0f19e73a..fb8bb6c05d824 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 @@ -228,17 +232,23 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { if (error) throw error; // caught in error boundary + 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 +257,7 @@ export const DashboardPage: FC = ({ idOrSlug }: PageProps) => { activeFilters={activeFilters} ownDataCharts={relevantDataMask} > - + {DashboardBuilderComponent} 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; }