From 636d0d11bfdebdb78ab3cc9a7f97a7ebc2349a72 Mon Sep 17 00:00:00 2001 From: somebody1234 Date: Thu, 1 Aug 2024 17:58:15 +1000 Subject: [PATCH] Move selected rows state of Data Catalog to zustand store (#10637) - Eliminates lag when using drag-to-select (the `SelectionBrush`) by moving the state into a zustand store. - This avoids the lag because now the entire Data Catalog no longer has to rerender, because the state is no longer stored in the `AssetsTable` component that contains all the rows (and would therefore rerender all the rows when its state changes) # Important Notes - The lag is present on Chromium, but any lag in general is generally more visible on Firefox, so it's highly recommended to test on Firefox as well as Electron - On current develop, *any* drag selection should be enough to trigger the lag (typically 200ms JS + 200ms rendering). If it's not reproducible, then you may need to create more assets. --- app/dashboard/src/App.tsx | 4 + .../src/components/dashboard/AssetRow.tsx | 83 ++++--- .../dashboard/AssetRow/assetRowUtils.tsx | 4 +- .../dashboard/DirectoryNameColumn.tsx | 6 +- .../dashboard/ProjectNameColumn.tsx | 6 +- app/dashboard/src/layouts/AssetsTable.tsx | 223 +++++++++--------- .../src/layouts/AssetsTableContextMenu.tsx | 12 +- app/dashboard/src/layouts/Drive.tsx | 3 - app/dashboard/src/layouts/DriveBar.tsx | 5 +- app/dashboard/src/providers/DriveProvider.tsx | 134 +++++++++++ app/dashboard/src/utilities/AssetTreeNode.ts | 6 - app/dashboard/src/utilities/set.ts | 2 +- 12 files changed, 315 insertions(+), 173 deletions(-) create mode 100644 app/dashboard/src/providers/DriveProvider.tsx diff --git a/app/dashboard/src/App.tsx b/app/dashboard/src/App.tsx index b0bb4bbc654..0dee36c5c7d 100644 --- a/app/dashboard/src/App.tsx +++ b/app/dashboard/src/App.tsx @@ -49,6 +49,7 @@ import * as backendHooks from '#/hooks/backendHooks' import AuthProvider, * as authProvider from '#/providers/AuthProvider' import BackendProvider, { useLocalBackend, useRemoteBackend } from '#/providers/BackendProvider' +import DriveProvider from '#/providers/DriveProvider' import DevtoolsProvider from '#/providers/EnsoDevtoolsProvider' import * as httpClientProvider from '#/providers/HttpClientProvider' import InputBindingsProvider from '#/providers/InputBindingsProvider' @@ -549,6 +550,9 @@ function AppRouter(props: AppRouterProps) { {result} ) + // Ideally this would be in `Drive.tsx`, but it currently must be all the way out here + // due to modals being in `TheModal`. + result = {result} result = ( {result} diff --git a/app/dashboard/src/components/dashboard/AssetRow.tsx b/app/dashboard/src/components/dashboard/AssetRow.tsx index af3d8b29dcf..5cda40c8b7c 100644 --- a/app/dashboard/src/components/dashboard/AssetRow.tsx +++ b/app/dashboard/src/components/dashboard/AssetRow.tsx @@ -1,14 +1,18 @@ /** @file A table row for an arbitrary asset. */ import * as React from 'react' +import { useStore } from 'zustand' + import BlankIcon from '#/assets/blank.svg' import * as backendHooks from '#/hooks/backendHooks' import * as dragAndDropHooks from '#/hooks/dragAndDropHooks' +import { useEventCallback } from '#/hooks/eventCallbackHooks' import * as setAssetHooks from '#/hooks/setAssetHooks' import * as toastAndLogHooks from '#/hooks/toastAndLogHooks' import * as authProvider from '#/providers/AuthProvider' +import { useDriveStore, useSetSelectedKeys } from '#/providers/DriveProvider' import * as modalProvider from '#/providers/ModalProvider' import * as textProvider from '#/providers/TextProvider' @@ -34,7 +38,6 @@ import * as localBackend from '#/services/LocalBackend' import * as projectManager from '#/services/ProjectManager' import type * as assetTreeNode from '#/utilities/AssetTreeNode' -import AssetTreeNode from '#/utilities/AssetTreeNode' import * as dateTime from '#/utilities/dateTime' import * as download from '#/utilities/download' import * as drag from '#/utilities/drag' @@ -79,29 +82,35 @@ export interface AssetRowProps readonly state: assetsTable.AssetsTableState readonly hidden: boolean readonly columns: columnUtils.Column[] - readonly selected: boolean - readonly setSelected: (selected: boolean) => void - readonly isSoleSelected: boolean readonly isKeyboardSelected: boolean readonly grabKeyboardFocus: () => void - readonly allowContextMenu: boolean readonly onClick: (props: AssetRowInnerProps, event: React.MouseEvent) => void - readonly onContextMenu?: ( - props: AssetRowInnerProps, - event: React.MouseEvent, - ) => void + readonly select: () => void readonly updateAssetRef: React.Ref<(asset: backendModule.AnyAsset) => void> } /** A row containing an {@link backendModule.AnyAsset}. */ export default function AssetRow(props: AssetRowProps) { - const { selected, isSoleSelected, isKeyboardSelected, isOpened } = props - const { setSelected, allowContextMenu, onContextMenu, state, columns, onClick } = props + const { isKeyboardSelected, isOpened, select, state, columns, onClick } = props const { item: rawItem, hidden: hiddenRaw, updateAssetRef, grabKeyboardFocus } = props const { nodeMap, setAssetPanelProps, doToggleDirectoryExpansion, doCopy, doCut, doPaste } = state const { setIsAssetPanelTemporarilyVisible, scrollContainerRef, rootDirectoryId, backend } = state const { visibilities } = state + const [item, setItem] = React.useState(rawItem) + const driveStore = useDriveStore() + const setSelectedKeys = useSetSelectedKeys() + const selected = useStore(driveStore, ({ visuallySelectedKeys, selectedKeys }) => + (visuallySelectedKeys ?? selectedKeys).has(item.key), + ) + const isSoleSelected = useStore( + driveStore, + ({ selectedKeys }) => selected && selectedKeys.size === 1, + ) + const allowContextMenu = useStore( + driveStore, + ({ selectedKeys }) => selectedKeys.size === 0 || !selected || isSoleSelected, + ) const draggableProps = dragAndDropHooks.useDraggable() const { user } = authProvider.useNonPartialUserSession() const { setModal, unsetModal } = modalProvider.useSetModal() @@ -110,7 +119,6 @@ export default function AssetRow(props: AssetRowProps) { const dispatchAssetEvent = eventListProvider.useDispatchAssetEvent() const dispatchAssetListEvent = eventListProvider.useDispatchAssetListEvent() const [isDraggedOver, setIsDraggedOver] = React.useState(false) - const [item, setItem] = React.useState(rawItem) const rootRef = React.useRef(null) const dragOverTimeoutHandle = React.useRef(null) const grabKeyboardFocusRef = React.useRef(grabKeyboardFocus) @@ -120,9 +128,8 @@ export default function AssetRow(props: AssetRowProps) { const [rowState, setRowState] = React.useState(() => object.merge(assetRowUtils.INITIAL_ROW_STATE, { setVisibility: setInsertionVisibility }), ) - const key = AssetTreeNode.getKey(item) const isCloud = backend.type === backendModule.BackendType.remote - const outerVisibility = visibilities.get(key) + const outerVisibility = visibilities.get(item.key) const visibility = outerVisibility == null || outerVisibility === Visibility.visible ? insertionVisibility @@ -147,6 +154,11 @@ export default function AssetRow(props: AssetRowProps) { const openProjectMutate = openProjectMutation.mutateAsync const closeProjectMutate = closeProjectMutation.mutateAsync + const setSelected = useEventCallback((newSelected: boolean) => { + const { selectedKeys } = driveStore.getState() + setSelectedKeys(set.withPresence(selectedKeys, item.key, newSelected)) + }) + React.useEffect(() => { setItem(rawItem) }, [rawItem]) @@ -570,30 +582,30 @@ export default function AssetRow(props: AssetRowProps) { break } case AssetEventType.temporarilyAddLabels: { - const labels = event.ids.has(item.key) ? event.labelNames : set.EMPTY + const labels = event.ids.has(item.key) ? event.labelNames : set.EMPTY_SET setRowState((oldRowState) => ( oldRowState.temporarilyAddedLabels === labels && - oldRowState.temporarilyRemovedLabels === set.EMPTY + oldRowState.temporarilyRemovedLabels === set.EMPTY_SET ) ? oldRowState : object.merge(oldRowState, { temporarilyAddedLabels: labels, - temporarilyRemovedLabels: set.EMPTY, + temporarilyRemovedLabels: set.EMPTY_SET, }), ) break } case AssetEventType.temporarilyRemoveLabels: { - const labels = event.ids.has(item.key) ? event.labelNames : set.EMPTY + const labels = event.ids.has(item.key) ? event.labelNames : set.EMPTY_SET setRowState((oldRowState) => ( - oldRowState.temporarilyAddedLabels === set.EMPTY && + oldRowState.temporarilyAddedLabels === set.EMPTY_SET && oldRowState.temporarilyRemovedLabels === labels ) ? oldRowState : object.merge(oldRowState, { - temporarilyAddedLabels: set.EMPTY, + temporarilyAddedLabels: set.EMPTY_SET, temporarilyRemovedLabels: labels, }), ) @@ -601,9 +613,9 @@ export default function AssetRow(props: AssetRowProps) { } case AssetEventType.addLabels: { setRowState((oldRowState) => - oldRowState.temporarilyAddedLabels === set.EMPTY ? + oldRowState.temporarilyAddedLabels === set.EMPTY_SET ? oldRowState - : object.merge(oldRowState, { temporarilyAddedLabels: set.EMPTY }), + : object.merge(oldRowState, { temporarilyAddedLabels: set.EMPTY_SET }), ) const labels = asset.labels if ( @@ -626,9 +638,9 @@ export default function AssetRow(props: AssetRowProps) { } case AssetEventType.removeLabels: { setRowState((oldRowState) => - oldRowState.temporarilyAddedLabels === set.EMPTY ? + oldRowState.temporarilyAddedLabels === set.EMPTY_SET ? oldRowState - : object.merge(oldRowState, { temporarilyAddedLabels: set.EMPTY }), + : object.merge(oldRowState, { temporarilyAddedLabels: set.EMPTY_SET }), ) const labels = asset.labels if ( @@ -677,9 +689,9 @@ export default function AssetRow(props: AssetRowProps) { const clearDragState = React.useCallback(() => { setIsDraggedOver(false) setRowState((oldRowState) => - oldRowState.temporarilyAddedLabels === set.EMPTY ? + oldRowState.temporarilyAddedLabels === set.EMPTY_SET ? oldRowState - : object.merge(oldRowState, { temporarilyAddedLabels: set.EMPTY }), + : object.merge(oldRowState, { temporarilyAddedLabels: set.EMPTY_SET }), ) }, []) @@ -707,7 +719,14 @@ export default function AssetRow(props: AssetRowProps) { case backendModule.AssetType.file: case backendModule.AssetType.datalink: case backendModule.AssetType.secret: { - const innerProps: AssetRowInnerProps = { key, item, setItem, state, rowState, setRowState } + const innerProps: AssetRowInnerProps = { + key: item.key, + item, + setItem, + state, + rowState, + setRowState, + } return ( <> {!hidden && ( @@ -758,7 +777,9 @@ export default function AssetRow(props: AssetRowProps) { if (allowContextMenu) { event.preventDefault() event.stopPropagation() - onContextMenu?.(innerProps, event) + if (!selected) { + select() + } setModal( , ) - } else { - onContextMenu?.(innerProps, event) } }} onDragStart={(event) => { @@ -872,7 +891,7 @@ export default function AssetRow(props: AssetRowProps) { return (