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.

Sign up for GitHub

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

Jump to bottom

perf: Improve dashboard performance by decreasing rerenders #30958

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

perf: Improve dashboard performance by decreasing rerenders #30958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
18bbed5
Optimize SyncDashboardState
kgabryje Nov 13, 2024
db4ee31
Rewrite Tabs and DashboardComponent to func to decrease rerenders
kgabryje Nov 14, 2024
3e16d47
Rewrite Tab to func
kgabryje Nov 14, 2024
c21bde0
ChartHolder and Dashboard.jsx improvements
kgabryje Nov 15, 2024
e762638
Chart.jsx changes
kgabryje Nov 15, 2024
09027b5
Optimize FilterBar
kgabryje Nov 15, 2024
0cad307
Fix some bugs
kgabryje Nov 15, 2024
3829797
Fix resizing
kgabryje Nov 15, 2024
0eeb637
Fix lint
kgabryje Nov 15, 2024
b6ce3b2
fux lint
kgabryje Nov 18, 2024
8622fa3
Fix tests and lint
kgabryje Nov 18, 2024
aba57a6
restore gitkeep
kgabryje Nov 18, 2024
be4b6dc
bugfix
kgabryje Nov 18, 2024
8e8b21b
Add nullchecks
kgabryje Nov 18, 2024
b9654c1
bugfix
kgabryje Nov 19, 2024
b5e05cf
name change
kgabryje Nov 19, 2024
c8d5eca
Fix native filters
kgabryje Nov 19, 2024
6be601f
Optimize DashboardContainer
kgabryje Nov 20, 2024
68125b3
refactor Row.jsx and getChartIdsInFilterScope
kgabryje Nov 20, 2024
555df32
refactor Column
kgabryje Nov 20, 2024
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
30 changes: 16 additions & 14 deletions superset-frontend/src/dashboard/components/Dashboard.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,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);
});
Expand Down Expand Up @@ -239,7 +241,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();
Expand Down
75 changes: 45 additions & 30 deletions superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,10 @@ const StyledDashboardContent = styled.div<{
`}
`;

const ELEMENT_ON_SCREEN_OPTIONS = {
threshold: [1],
};

const DashboardBuilder: FC<DashboardBuilderProps> = () => {
const dispatch = useDispatch();
const uiConfig = useUiConfig();
Expand Down Expand Up @@ -469,9 +473,9 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
nativeFiltersEnabled,
} = useNativeFilters();

const [containerRef, isSticky] = useElementOnScreen<HTMLDivElement>({
threshold: [1],
});
const [containerRef, isSticky] = useElementOnScreen<HTMLDivElement>(
ELEMENT_ON_SCREEN_OPTIONS,
);

const showFilterBar =
(crossFiltersEnabled || nativeFiltersEnabled) && !editMode;
Expand Down Expand Up @@ -581,6 +585,43 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
? 0
: theme.gridUnit * 8;

const renderChild = useCallback(
adjustedWidth => {
const filterBarWidth = dashboardFiltersOpen
? adjustedWidth
: CLOSED_FILTER_BAR_WIDTH;
return (
<FiltersPanel
width={filterBarWidth}
hidden={isReport}
data-test="dashboard-filters-panel"
>
<StickyPanel ref={containerRef} width={filterBarWidth}>
<ErrorBoundary>
<FilterBar
orientation={FilterBarOrientation.Vertical}
verticalConfig={{
filtersOpen: dashboardFiltersOpen,
toggleFiltersBar: toggleDashboardFiltersOpen,
width: filterBarWidth,
height: filterBarHeight,
offset: filterBarOffset,
}}
/>
</ErrorBoundary>
</StickyPanel>
</FiltersPanel>
);
},
[
dashboardFiltersOpen,
toggleDashboardFiltersOpen,
filterBarHeight,
filterBarOffset,
isReport,
],
);

return (
<DashboardWrapper>
{showFilterBar &&
Expand All @@ -593,33 +634,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
maxWidth={OPEN_FILTER_BAR_MAX_WIDTH}
initialWidth={OPEN_FILTER_BAR_WIDTH}
>
{adjustedWidth => {
const filterBarWidth = dashboardFiltersOpen
? adjustedWidth
: CLOSED_FILTER_BAR_WIDTH;
return (
<FiltersPanel
width={filterBarWidth}
hidden={isReport}
data-test="dashboard-filters-panel"
>
<StickyPanel ref={containerRef} width={filterBarWidth}>
<ErrorBoundary>
<FilterBar
orientation={FilterBarOrientation.Vertical}
verticalConfig={{
filtersOpen: dashboardFiltersOpen,
toggleFiltersBar: toggleDashboardFiltersOpen,
width: filterBarWidth,
height: filterBarHeight,
offset: filterBarOffset,
}}
/>
</ErrorBoundary>
</StickyPanel>
</FiltersPanel>
);
}}
{renderChild}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

</ResizableSidebar>
</>
)}
Expand Down
134 changes: 74 additions & 60 deletions superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
// 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, useEffect, useMemo, useRef } from 'react';
import { FC, memo, useCallback, useEffect, useMemo, useRef } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import {
Filter,
Expand All @@ -44,6 +44,7 @@ import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilter
import findTabIndexByComponentId from 'src/dashboard/util/findTabIndexByComponentId';
import { setInScopeStatusOfFilters } from 'src/dashboard/actions/nativeFilters';
import { updateDashboardLabelsColor } from 'src/dashboard/actions/dashboardState';
import { useChartIds } from 'src/dashboard/util/charts/useChartIds';
import {
applyColors,
getColorNamespace,
Expand All @@ -52,6 +53,7 @@ import {
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;
Expand All @@ -68,10 +70,12 @@ const useNativeFilterScopes = () => {
pick(filter, ['id', 'scope', 'type']),
)
: [],
[JSON.stringify(nativeFilters)],
[nativeFilters],
);
};

const TOP_OF_PAGE_RANGE = 220;

const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
const nativeFilterScopes = useNativeFilterScopes();
const dispatch = useDispatch();
Expand All @@ -85,9 +89,7 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
const directPathToChild = useSelector<RootState, string[]>(
state => state.dashboardState.directPathToChild,
);
const chartIds = useSelector<RootState, number[]>(state =>
Object.values(state.charts).map(chart => chart.id),
);
const chartIds = useChartIds();

const prevTabIndexRef = useRef();
const tabIndex = useMemo(() => {
Expand Down Expand Up @@ -115,13 +117,19 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ 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 {
Expand All @@ -131,14 +139,14 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ 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(() => {
// verify freshness of color map on tab change
Expand All @@ -163,57 +171,63 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [dashboardInfo.id, dispatch]);

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
*/
<Tabs
id={DASHBOARD_GRID_ID}
activeKey={activeKey}
renderTabBar={renderTabBar}
fullWidth={false}
animated={false}
allowOverflow
onFocus={handleFocus}
>
{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.
<Tabs.TabPane
key={index === 0 ? DASHBOARD_GRID_ID : index.toString()}
>
<DashboardGrid
gridComponent={dashboardLayout[id]}
// see isValidChild for why tabs do not increment the depth of their children
depth={DASHBOARD_ROOT_DEPTH + 1} // (topLevelTabs ? 0 : 1)}
width={width}
isComponentVisible={index === tabIndex}
/>
</Tabs.TabPane>
))}
</Tabs>
),
[activeKey, childIds, dashboardLayout, handleFocus, renderTabBar, tabIndex],
);

return (
<div className="grid-container" data-test="grid-container">
<ParentSize>
{({ 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
*/
<Tabs
id={DASHBOARD_GRID_ID}
activeKey={activeKey}
renderTabBar={() => <></>}
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.
<Tabs.TabPane
key={index === 0 ? DASHBOARD_GRID_ID : index.toString()}
>
<DashboardGrid
gridComponent={dashboardLayout[id]}
// see isValidChild for why tabs do not increment the depth of their children
depth={DASHBOARD_ROOT_DEPTH + 1} // (topLevelTabs ? 0 : 1)}
width={width}
isComponentVisible={index === tabIndex}
/>
</Tabs.TabPane>
))}
</Tabs>
)}
</ParentSize>
<ParentSize>{renderParentSizeChildren}</ParentSize>
</div>
);
};

export default DashboardContainer;
export default memo(DashboardContainer);
Loading
Loading