Skip to content

Commit

Permalink
perf(dashboard): reduce rerenders of DragDroppable (#16525)
Browse files Browse the repository at this point in the history
* perf(dashboard): reduce rerenders of DragDroppable

* lint fix
  • Loading branch information
kgabryje authored Sep 7, 2021
1 parent 3fe2e6e commit 7faa5c6
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 72 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 React, { FC } from 'react';
import React, { FC, useCallback, useMemo } from 'react';
import { JsonObject, styled, css } from '@superset-ui/core';
import ErrorBoundary from 'src/components/ErrorBoundary';
import BuilderComponentPane from 'src/dashboard/components/BuilderComponentPane';
Expand Down Expand Up @@ -157,23 +157,27 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
state => state.dashboardState.fullSizeChartId,
);

const handleChangeTab = ({
pathToTabIndex,
}: {
pathToTabIndex: string[];
}) => {
dispatch(setDirectPathToChild(pathToTabIndex));
};
const handleChangeTab = useCallback(
({ pathToTabIndex }: { pathToTabIndex: string[] }) => {
dispatch(setDirectPathToChild(pathToTabIndex));
},
[dispatch],
);

const handleDeleteTopLevelTabs = () => {
const handleDeleteTopLevelTabs = useCallback(() => {
dispatch(deleteTopLevelTabs());

const firstTab = getDirectPathToTabIndex(
getRootLevelTabsComponent(dashboardLayout),
0,
);
dispatch(setDirectPathToChild(firstTab));
};
}, [dashboardLayout, dispatch]);

const handleDrop = useCallback(
dropResult => dispatch(handleComponentDrop(dropResult)),
[dispatch],
);

const dashboardRoot = dashboardLayout[DASHBOARD_ROOT_ID];
const rootChildId = dashboardRoot.children[0];
Expand Down Expand Up @@ -217,6 +221,54 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
const filterBarHeight = `calc(100vh - ${offset}px)`;
const filterBarOffset = dashboardFiltersOpen ? 0 : barTopOffset + 20;

const draggableStyle = useMemo(
() => ({
marginLeft: dashboardFiltersOpen || editMode ? 0 : -32,
}),
[dashboardFiltersOpen, editMode],
);

const renderDraggableContent = useCallback(
({ dropIndicatorProps }: { dropIndicatorProps: JsonObject }) => (
<div>
{!hideDashboardHeader && <DashboardHeader />}
{dropIndicatorProps && <div {...dropIndicatorProps} />}
{!isReport && topLevelTabs && (
<WithPopoverMenu
shouldFocus={shouldFocusTabs}
menuItems={[
<IconButton
icon={<Icons.FallOutlined iconSize="xl" />}
label="Collapse tab content"
onClick={handleDeleteTopLevelTabs}
/>,
]}
editMode={editMode}
>
{/* @ts-ignore */}
<DashboardComponent
id={topLevelTabs?.id}
parentId={DASHBOARD_ROOT_ID}
depth={DASHBOARD_ROOT_DEPTH + 1}
index={0}
renderTabContent={false}
renderHoverMenu={false}
onChangeTab={handleChangeTab}
/>
</WithPopoverMenu>
)}
</div>
),
[
editMode,
handleChangeTab,
handleDeleteTopLevelTabs,
hideDashboardHeader,
isReport,
topLevelTabs,
],
);

return (
<StyledDiv>
{nativeFiltersEnabled && !editMode && (
Expand Down Expand Up @@ -244,45 +296,13 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
depth={DASHBOARD_ROOT_DEPTH}
index={0}
orientation="column"
onDrop={dropResult => dispatch(handleComponentDrop(dropResult))}
onDrop={handleDrop}
editMode={editMode}
// you cannot drop on/displace tabs if they already exist
disableDragDrop={!!topLevelTabs}
style={{
marginLeft: dashboardFiltersOpen || editMode ? 0 : -32,
}}
style={draggableStyle}
>
{({ dropIndicatorProps }: { dropIndicatorProps: JsonObject }) => (
<div>
{!hideDashboardHeader && <DashboardHeader />}
{dropIndicatorProps && <div {...dropIndicatorProps} />}
{!isReport && topLevelTabs && (
<WithPopoverMenu
shouldFocus={shouldFocusTabs}
menuItems={[
<IconButton
icon={<Icons.FallOutlined iconSize="xl" />}
label="Collapse tab content"
onClick={handleDeleteTopLevelTabs}
/>,
]}
editMode={editMode}
>
{/*
// @ts-ignore */}
<DashboardComponent
id={topLevelTabs?.id}
parentId={DASHBOARD_ROOT_ID}
depth={DASHBOARD_ROOT_DEPTH + 1}
index={0}
renderTabContent={false}
renderHoverMenu={false}
onChangeTab={handleChangeTab}
/>
</WithPopoverMenu>
)}
</div>
)}
{renderDraggableContent}
</DragDroppable>
</StyledHeader>
<StyledContent fullSizeChartId={fullSizeChartId}>
Expand Down
22 changes: 12 additions & 10 deletions superset-frontend/src/dashboard/components/DashboardGrid.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ const propTypes = {

const defaultProps = {};

const renderDraggableContentBottom = dropProps =>
dropProps.dropIndicatorProps && (
<div className="drop-indicator drop-indicator--bottom" />
);

const renderDraggableContentTop = dropProps =>
dropProps.dropIndicatorProps && (
<div className="drop-indicator drop-indicator--top" />
);

class DashboardGrid extends React.PureComponent {
constructor(props) {
super(props);
Expand Down Expand Up @@ -144,11 +154,7 @@ class DashboardGrid extends React.PureComponent {
className="empty-droptarget"
editMode
>
{({ dropIndicatorProps }) =>
dropIndicatorProps && (
<div className="drop-indicator drop-indicator--bottom" />
)
}
{renderDraggableContentBottom}
</DragDroppable>
)}

Expand Down Expand Up @@ -181,11 +187,7 @@ class DashboardGrid extends React.PureComponent {
className="empty-droptarget"
editMode
>
{({ dropIndicatorProps }) =>
dropIndicatorProps && (
<div className="drop-indicator drop-indicator--top" />
)
}
{renderDraggableContentTop}
</DragDroppable>
)}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const defaultProps = {
};

// export unwrapped component for testing
export class UnwrappedDragDroppable extends React.Component {
export class UnwrappedDragDroppable extends React.PureComponent {
constructor(props) {
super(props);
this.state = {
Expand Down
22 changes: 12 additions & 10 deletions superset-frontend/src/dashboard/components/gridComponents/Tab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ const TabTitleContainer = styled.div`
`}
`;

const renderDraggableContentBottom = dropProps =>
dropProps.dropIndicatorProps && (
<div className="drop-indicator drop-indicator--bottom" />
);

const renderDraggableContentTop = dropProps =>
dropProps.dropIndicatorProps && (
<div className="drop-indicator drop-indicator--top" />
);

export default class Tab extends React.PureComponent {
constructor(props) {
super(props);
Expand Down Expand Up @@ -148,11 +158,7 @@ export default class Tab extends React.PureComponent {
editMode
className="empty-droptarget"
>
{({ dropIndicatorProps }) =>
dropIndicatorProps && (
<div className="drop-indicator drop-indicator--top" />
)
}
{renderDraggableContentTop}
</DragDroppable>
)}
{tabComponent.children.map((componentId, componentIndex) => (
Expand Down Expand Up @@ -184,11 +190,7 @@ export default class Tab extends React.PureComponent {
editMode
className="empty-droptarget"
>
{({ dropIndicatorProps }) =>
dropIndicatorProps && (
<div className="drop-indicator drop-indicator--bottom" />
)
}
{renderDraggableContentBottom}
</DragDroppable>
)}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export class Tabs extends React.PureComponent {
this.handleDeleteComponent = this.handleDeleteComponent.bind(this);
this.handleDeleteTab = this.handleDeleteTab.bind(this);
this.handleDropOnTab = this.handleDropOnTab.bind(this);
this.handleDrop = this.handleDrop.bind(this);
}

componentDidMount() {
Expand Down Expand Up @@ -281,6 +282,12 @@ export class Tabs extends React.PureComponent {
}
}

handleDrop(dropResult) {
if (dropResult.dragging.type !== TABS_TYPE) {
this.props.handleComponentDrop(dropResult);
}
}

render() {
const {
depth,
Expand All @@ -292,7 +299,6 @@ export class Tabs extends React.PureComponent {
onResizeStart,
onResize,
onResizeStop,
handleComponentDrop,
renderTabContent,
renderHoverMenu,
isComponentVisible: isCurrentTabVisible,
Expand All @@ -315,11 +321,7 @@ export class Tabs extends React.PureComponent {
orientation="row"
index={index}
depth={depth}
onDrop={dropResult => {
if (dropResult.dragging.type !== TABS_TYPE) {
handleComponentDrop(dropResult);
}
}}
onDrop={this.handleDrop}
editMode={editMode}
>
{({
Expand Down

0 comments on commit 7faa5c6

Please sign in to comment.