Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Optimize DashboardPage and SyncDashboardState #31244

Merged
merged 1 commit into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
Loading