From 9ab108f2b43149210fe74b1a6bdbf19bac521f49 Mon Sep 17 00:00:00 2001 From: Sean Parsons <217400+seanparsons@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:50:45 +0100 Subject: [PATCH] performance(editor) Switch Event Handlers 2.0 (#6469) - Switched lots of event handlers from onClick to onMouseDown, along with updating the name of props if those use that naming. - Added `onMouseDown` handler that stops propagation to the react-contextify menu, so that the mouse down doesn't pass through it and activate things underneath the menu. - Added `stopPropagation` call to the `onMouseDown` of the navigator so that the event doesn't pass through to the canvas. - Removed `stopPropagation` and `preventDefault` calls in `NavigatorRowClickableWrapper` so that the event can still trigger dragging. --- .../components/canvas/design-panel-root.tsx | 34 ++++++++--------- .../src/components/context-menu-wrapper.tsx | 12 +++++- ...spector-end-to-end-tests.spec.browser2.tsx | 8 ++-- .../components/inspector/section-header.tsx | 2 +- .../component-section/component-section.tsx | 4 +- .../header-section/target-selector.tsx | 4 +- .../layout-section/list-source-cartouche.tsx | 13 ++++--- .../components/navigator/left-pane/index.tsx | 34 ++++++++--------- .../navigator-condensed-entry.tsx | 12 +++--- .../navigator-item-clickable-wrapper.tsx | 17 ++++----- .../navigator-item-components.tsx | 37 +++++++++---------- .../navigator-item/navigator-item.tsx | 29 ++++++++------- ...nent-picker-context-menu.spec.browser2.tsx | 9 +++-- .../navigator/navigator.spec.browser2.tsx | 9 +++-- editor/src/components/navigator/navigator.tsx | 5 ++- 15 files changed, 122 insertions(+), 107 deletions(-) diff --git a/editor/src/components/canvas/design-panel-root.tsx b/editor/src/components/canvas/design-panel-root.tsx index 13d381968d01..d9f877e33fc3 100644 --- a/editor/src/components/canvas/design-panel-root.tsx +++ b/editor/src/components/canvas/design-panel-root.tsx @@ -134,7 +134,7 @@ export const RightPane = React.memo((props) => { ) const dispatch = useDispatch() - const onClickTab = React.useCallback( + const onMouseDownTab = React.useCallback( (menuTab: RightMenuTab) => { const actions: Array = [EditorActions.setRightMenuTab(menuTab)] if (isCommentMode(editorModeRef.current) && menuTab !== RightMenuTab.Comments) { @@ -145,21 +145,21 @@ export const RightPane = React.memo((props) => { [dispatch, editorModeRef], ) - const onClickInsertTab = React.useCallback(() => { - onClickTab(RightMenuTab.Insert) - }, [onClickTab]) + const onMouseDownInsertTab = React.useCallback(() => { + onMouseDownTab(RightMenuTab.Insert) + }, [onMouseDownTab]) - const onClickCommentsTab = React.useCallback(() => { - onClickTab(RightMenuTab.Comments) - }, [onClickTab]) + const onMouseDownCommentsTab = React.useCallback(() => { + onMouseDownTab(RightMenuTab.Comments) + }, [onMouseDownTab]) - const onClickInspectorTab = React.useCallback(() => { - onClickTab(RightMenuTab.Inspector) - }, [onClickTab]) + const onMouseDownInspectorTab = React.useCallback(() => { + onMouseDownTab(RightMenuTab.Inspector) + }, [onMouseDownTab]) - const onClickSettingsTab = React.useCallback(() => { - onClickTab(RightMenuTab.Settings) - }, [onClickTab]) + const onMouseDownSettingsTab = React.useCallback(() => { + onMouseDownTab(RightMenuTab.Settings) + }, [onMouseDownTab]) const canComment = useCanComment() @@ -200,7 +200,7 @@ export const RightPane = React.memo((props) => { {when( allowedToEdit, @@ -210,7 +210,7 @@ export const RightPane = React.memo((props) => { , )} , @@ -221,13 +221,13 @@ export const RightPane = React.memo((props) => { testId='comments-tab' label={'Comments'} selected={selectedTab === RightMenuTab.Comments} - onClick={onClickCommentsTab} + onMouseDown={onMouseDownCommentsTab} />, )} ({ dispatch, getData, id, items }: ContextMenuPro [getData, dispatch, isDisabled, isHidden], ) + const onMouseDown = React.useCallback((event: React.MouseEvent) => { + event.stopPropagation() + }, []) + return ( - + {splitItems.map((item, index) => { if (item?.type === 'submenu') { return ( diff --git a/editor/src/components/inspector/common/inspector-end-to-end-tests.spec.browser2.tsx b/editor/src/components/inspector/common/inspector-end-to-end-tests.spec.browser2.tsx index 3d9e79541755..bfbeb21cdc95 100644 --- a/editor/src/components/inspector/common/inspector-end-to-end-tests.spec.browser2.tsx +++ b/editor/src/components/inspector/common/inspector-end-to-end-tests.spec.browser2.tsx @@ -1479,12 +1479,12 @@ describe('inspector tests with real metadata', () => { await act(async () => { await screen.findByTestId('section-header-Advanced') - fireEvent.click(screen.getByTestId('section-header-Advanced')) + fireEvent.mouseDown(screen.getByTestId('section-header-Advanced')) }) await act(async () => { await screen.findByTestId('target-selector-style') - fireEvent.click(screen.getByTestId('target-selector')) + fireEvent.mouseDown(screen.getByTestId('target-selector')) }) await act(async () => { await screen.findByTestId('target-list-item-css') @@ -1586,12 +1586,12 @@ describe('inspector tests with real metadata', () => { await act(async () => { await screen.findByTestId('section-header-Advanced') - fireEvent.click(screen.getByTestId('section-header-Advanced')) + fireEvent.mouseDown(screen.getByTestId('section-header-Advanced')) }) await act(async () => { await screen.findByTestId('target-selector-style') - fireEvent.click(screen.getByTestId('target-selector')) + fireEvent.mouseDown(screen.getByTestId('target-selector')) }) await act(async () => { await screen.findByTestId('target-list-item-css') diff --git a/editor/src/components/inspector/section-header.tsx b/editor/src/components/inspector/section-header.tsx index 1887359b8cd5..77719edac898 100644 --- a/editor/src/components/inspector/section-header.tsx +++ b/editor/src/components/inspector/section-header.tsx @@ -19,7 +19,7 @@ export function InspectorSectionHeader({ padding: 8, cursor: 'pointer', }} - onClick={toggle} + onMouseDown={toggle} data-testid={`section-header-${title}`} >
{ return (
Component )} - + { Target - + - + { - if (openOn === 'single-click') { - openPopup() - } - }, [openOn, openPopup]) + const onClick = React.useCallback( + (e: React.MouseEvent) => { + if (openOn === 'single-click') { + openPopup() + } + }, + [openOn, openPopup], + ) const onDoubleClick = React.useCallback(() => { if (openOn === 'double-click') { diff --git a/editor/src/components/navigator/left-pane/index.tsx b/editor/src/components/navigator/left-pane/index.tsx index 286bf18cbbe5..8babed005899 100644 --- a/editor/src/components/navigator/left-pane/index.tsx +++ b/editor/src/components/navigator/left-pane/index.tsx @@ -48,7 +48,7 @@ export const LeftPaneComponent = React.memo((props) => { const dispatch = useDispatch() - const onClickTab = React.useCallback( + const onMouseDownTab = React.useCallback( (menuTab: LeftMenuTab) => { let actions: Array = [] actions.push(setLeftMenuTab(menuTab)) @@ -57,21 +57,21 @@ export const LeftPaneComponent = React.memo((props) => { [dispatch], ) - const onClickPagesTab = React.useCallback(() => { - onClickTab(LeftMenuTab.Pages) - }, [onClickTab]) + const onMouseDownPagesTab = React.useCallback(() => { + onMouseDownTab(LeftMenuTab.Pages) + }, [onMouseDownTab]) - const onClickProjectTab = React.useCallback(() => { - onClickTab(LeftMenuTab.Project) - }, [onClickTab]) + const onMouseDownProjectTab = React.useCallback(() => { + onMouseDownTab(LeftMenuTab.Project) + }, [onMouseDownTab]) - const onClickNavigatorTab = React.useCallback(() => { - onClickTab(LeftMenuTab.Navigator) - }, [onClickTab]) + const onMouseDownNavigatorTab = React.useCallback(() => { + onMouseDownTab(LeftMenuTab.Navigator) + }, [onMouseDownTab]) - const onClickGithubTab = React.useCallback(() => { - onClickTab(LeftMenuTab.Github) - }, [onClickTab]) + const onMouseDownGithubTab = React.useCallback(() => { + onMouseDownTab(LeftMenuTab.Github) + }, [onMouseDownTab]) const isMyProject = useIsMyProject() @@ -126,23 +126,23 @@ export const LeftPaneComponent = React.memo((props) => { , )} , )} diff --git a/editor/src/components/navigator/navigator-item/navigator-condensed-entry.tsx b/editor/src/components/navigator/navigator-item/navigator-condensed-entry.tsx index 60f9e0afce4e..6f208b183679 100644 --- a/editor/src/components/navigator/navigator-item/navigator-condensed-entry.tsx +++ b/editor/src/components/navigator/navigator-item/navigator-condensed-entry.tsx @@ -20,7 +20,7 @@ import { LayoutIcon } from './layout-icon' import { DataReferenceCartoucheControl } from '../../inspector/sections/component-section/data-reference-cartouche' import { NavigatorRowClickableWrapper, - useGetNavigatorClickActions, + useGetNavigatorMouseDownActions, } from './navigator-item-clickable-wrapper' import type { ThemeObject } from '../../../uuiui/styles/theme/theme-helpers' import { useNavigatorSelectionBoundsForEntry } from './use-navigator-selection-bounds-for-entry' @@ -341,18 +341,18 @@ const CondensedEntryItemContent = React.memo( ]) }, [props.entry, dispatch, highlightedViews]) - const getClickActions = useGetNavigatorClickActions( + const getMouseDownActions = useGetNavigatorMouseDownActions( props.entry.elementPath, props.selected, condensedNavigatorRow([props.entry], 'leaf', props.indentation), ) - const onClick = React.useCallback( + const onMouseDown = React.useCallback( (e: React.MouseEvent) => { e.stopPropagation() - dispatch(getClickActions(e)) + dispatch(getMouseDownActions(e)) }, - [dispatch, getClickActions], + [dispatch, getMouseDownActions], ) return ( @@ -374,7 +374,7 @@ const CondensedEntryItemContent = React.memo( }} onMouseOver={onMouseOver} onMouseOut={onMouseOut} - onClick={onClick} + onMouseDown={onMouseDown} >
EP.pathsEqual(targetPath, view)) }, [selectedViews, targetPath]) - const getActions = useGetNavigatorClickActions(targetPath, selected, props.row) + const getMouseDownActions = useGetNavigatorMouseDownActions(targetPath, selected, props.row) - const onClick = React.useCallback( + const onMouseDown = React.useCallback( (e: React.MouseEvent) => { - e.stopPropagation() - e.preventDefault() - - const actions = getActions(e) + const actions = getMouseDownActions(e) dispatch(actions) }, - [dispatch, getActions], + [dispatch, getMouseDownActions], ) return ( -
+
{props.children}
) @@ -68,11 +65,11 @@ export const NavigatorRowClickableWrapper = React.memo( ) NavigatorRowClickableWrapper.displayName = 'NavigatorRowClickableWrapper' -export function useGetNavigatorClickActions( +export function useGetNavigatorMouseDownActions( targetPath: ElementPath, selected: boolean, row: NavigatorRow, -) { +): (event: React.MouseEvent) => Array { const navigatorTargets = useRefEditorState(navigatorTargetsSelector) const selectedViews = useRefEditorState((store) => store.editor.selectedViews) const collapsedViews = useRefEditorState((store) => store.editor.navigator.collapsedViews) diff --git a/editor/src/components/navigator/navigator-item/navigator-item-components.tsx b/editor/src/components/navigator/navigator-item/navigator-item-components.tsx index 3fe002f4ea84..1686b791019d 100644 --- a/editor/src/components/navigator/navigator-item/navigator-item-components.tsx +++ b/editor/src/components/navigator/navigator-item/navigator-item-components.tsx @@ -180,7 +180,7 @@ interface VisiblityIndicatorProps { visibilityEnabled: boolean selected: boolean iconColor: IcnProps['color'] - onClick: () => void + onMouseDown: () => void } export const VisibilityIndicator: React.FunctionComponent< @@ -190,7 +190,7 @@ export const VisibilityIndicator: React.FunctionComponent< return (