Skip to content

Commit

Permalink
perf: Optimize DashboardPage and SyncDashboardState (#31244)
Browse files Browse the repository at this point in the history
  • Loading branch information
kgabryje authored Dec 2, 2024
1 parent ce0e06a commit 06fb330
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -374,7 +372,7 @@ const ELEMENT_ON_SCREEN_OPTIONS = {
threshold: [1],
};

const DashboardBuilder: FC<DashboardBuilderProps> = () => {
const DashboardBuilder = () => {
const dispatch = useDispatch();
const uiConfig = useUiConfig();
const theme = useTheme();
Expand Down Expand Up @@ -737,4 +735,4 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
);
};

export default DashboardBuilder;
export default memo(DashboardBuilder);
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 = (
Expand All @@ -56,38 +53,45 @@ const updateDashboardTabLocalStorage = (
const dashboardsContexts = getDashboardContextLocalStorage();
setItem(LocalStorageKeys.DashboardExploreContext, {
...dashboardsContexts,
[dashboardPageId]: dashboardContext,
[dashboardPageId]: { ...dashboardContext, dashboardPageId },
});
};

const SyncDashboardState: FC<Props> = ({ 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<Props> = ({ dashboardPageId }) => {
const dashboardContextForExplore = useSelector<
RootState,
DashboardContextForExplore
>(selectDashboardContextForExplore);

useEffect(() => {
updateDashboardTabLocalStorage(dashboardPageId, dashboardContextForExplore);
Expand Down
46 changes: 28 additions & 18 deletions superset-frontend/src/dashboard/containers/DashboardPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -228,17 +232,23 @@ export const DashboardPage: FC<PageProps> = ({ 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(() => <DashboardBuilder />, []);
return (
<>
<Global
styles={[
filterCardPopoverStyle(theme),
headerStyles(theme),
chartContextMenuStyles(theme),
focusStyle(theme),
chartHeaderStyles(theme),
]}
/>
<Global styles={globalStyles} />
{readyToRender && hasDashboardInfoInitiated ? (
<>
<SyncDashboardState dashboardPageId={dashboardPageId} />
Expand All @@ -247,7 +257,7 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
activeFilters={activeFilters}
ownDataCharts={relevantDataMask}
>
<DashboardBuilder />
{DashboardBuilderComponent}
</DashboardContainer>
</DashboardPageIdContext.Provider>
</>
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/types/DashboardContextForExplore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,5 @@ export interface DashboardContextForExplore {
}
| {};
isRedundant?: boolean;
dashboardPageId?: string;
}

0 comments on commit 06fb330

Please sign in to comment.