Improve sidebar menu and navigation tree performance (#12873)

* Improve sidebar menu and navigation tree performance

* remote some test and rollback useOnScreen

* feat: introduce ActionFactory type and update context menu components to support it

- Added ActionFactory type to allow functions returning ActionWithChildren.
- Updated ContextMenu and DropdownMenu components to handle ActionFactory.
- Modified DocumentRow, SidebarLink, and other components to accept ActionFactory for context actions.
- Refactored useDocumentMenuAction to utilize createRootMenuAction for better action management.

---------

Co-authored-by: Tom Moor <tom@getoutline.com>

app/components/Menu/ContextMenu.tsx

@@ -2,7 +2,7 @@ import * as React from "react";
 import { actionToMenuItem } from "~/actions";
 import useActionContext from "~/hooks/useActionContext";
 import useMobile from "~/hooks/useMobile";
-import type { ActionVariant, ActionWithChildren } from "~/types";
+import type { ActionFactory, ActionVariant, ActionWithChildren } from "~/types";
 import { preventDefault } from "~/utils/events";
 import { toMenuItems } from "./transformer";
 import { observer } from "mobx-react";
@@ -12,7 +12,7 @@ import { MenuProvider } from "~/components/primitives/Menu/MenuContext";
 
 type Props = {
   /** Root action with children representing the menu items */
-  action?: ActionWithChildren;
+  action?: ActionWithChildren | ActionFactory;
   /** Trigger for the menu */
   children: React.ReactNode;
   /** ARIA label for the menu */
@@ -25,22 +25,28 @@ type Props = {
 
 export const ContextMenu = observer(
   ({ action, children, ariaLabel, onOpen, onClose }: Props) => {
+    const [open, setOpen] = React.useState(false);
     const isMobile = useMobile();
     const contentRef = React.useRef<React.ElementRef<typeof MenuContent>>(null);
     const actionContext = useActionContext({
       isMenu: true,
     });
 
-    const menuItems = useComputed(
-      () =>
-        ((action?.children as ActionVariant[]) ?? []).map((childAction) =>
-          actionToMenuItem(childAction, actionContext)
-        ),
-      [action?.children, actionContext]
-    );
+    const menuItems = useComputed(() => {
+      if (!open) {
+        return [];
+      }
+
+      const resolvedAction = typeof action === "function" ? action() : action;
+
+      return ((resolvedAction?.children as ActionVariant[]) ?? []).map(
+        (childAction) => actionToMenuItem(childAction, actionContext)
+      );
+    }, [open, action, actionContext]);
 
     const handleOpenChange = React.useCallback(
       (open: boolean) => {
+        setOpen(open);
         if (open) {
           onOpen?.();
         } else {
@@ -62,15 +68,15 @@ export const ContextMenu = observer(
       }
     }, []);
 
-    if (isMobile || !action || menuItems.length === 0) {
+    if (isMobile || !action) {
       return <>{children}</>;
     }
 
-    const content = toMenuItems(menuItems);
+    const content = open ? toMenuItems(menuItems) : null;
 
     return (
       <MenuProvider variant="context">
-        <Menu onOpenChange={handleOpenChange}>
+        <Menu open={open} onOpenChange={handleOpenChange}>
           <MenuTrigger aria-label={ariaLabel}>{children}</MenuTrigger>
           <MenuContent
             aria-label={ariaLabel}

app/components/Menu/DropdownMenu.tsx

@@ -15,6 +15,7 @@ import useActionContext from "~/hooks/useActionContext";
 import useMobile from "~/hooks/useMobile";
 import { preventDefault } from "~/utils/events";
 import type {
+  ActionFactory,
   ActionVariant,
   ActionWithChildren,
   MenuItem,
@@ -26,7 +27,7 @@ import { useComputed } from "~/hooks/useComputed";
 
 type Props = {
   /** Root action with children representing the menu items */
-  action: ActionWithChildren;
+  action: ActionWithChildren | ActionFactory;
   /** Trigger for the menu */
   children: React.ReactNode;
   /** Alignment w.r.t trigger - defaults to start */
@@ -77,10 +78,12 @@ export const DropdownMenu = observer(
           return [];
         }
 
-        return (action.children as ActionVariant[]).map((childAction) =>
+        const resolvedAction = typeof action === "function" ? action() : action;
+
+        return (resolvedAction.children as ActionVariant[]).map((childAction) =>
           actionToMenuItem(childAction, actionContext)
         );
-      }, [open, action.children, actionContext]);
+      }, [open, action, actionContext]);
 
       const handleOpenChange = React.useCallback(
         (open: boolean) => {

app/components/Sidebar/components/DocumentRow.tsx

@@ -19,7 +19,7 @@ import Relative from "./Relative";
 import SidebarLink from "./SidebarLink";
 import type { SidebarContextType } from "./SidebarContext";
 import { useSidebarContext } from "./SidebarContext";
-import type { ActionWithChildren } from "~/types";
+import type { ActionFactory, ActionWithChildren } from "~/types";
 
 export type DocumentRowProps = {
   /** Document identifier for policy, prefetch and import. */
@@ -98,7 +98,7 @@ export type DocumentRowProps = {
   newChildDepth?: number;
 
   /** Context menu action for the row. */
-  contextAction?: ActionWithChildren;
+  contextAction?: ActionWithChildren | ActionFactory;
 
   /** Optional override for the active-match function. */
   isActiveOverride?: (

app/components/Sidebar/components/SidebarLink.tsx

@@ -12,7 +12,7 @@ import { undraggableOnDesktop } from "~/styles";
 import Disclosure from "./Disclosure";
 import type { Props as NavLinkProps } from "./NavLink";
 import NavLink from "./NavLink";
-import type { ActionWithChildren } from "~/types";
+import type { ActionFactory, ActionWithChildren } from "~/types";
 import { ContextMenu } from "~/components/Menu/ContextMenu";
 import { useTranslation } from "react-i18next";
 
@@ -58,7 +58,7 @@ type Props = Omit<NavLinkProps, "to"> & {
   /** Whether to automatically scroll this link into view if needed */
   scrollIntoViewIfNeeded?: boolean;
   /** Optional context menu action to display */
-  contextAction?: ActionWithChildren;
+  contextAction?: ActionWithChildren | ActionFactory;
 };
 
 const activeDropStyle = {

app/components/Sidebar/components/StarredLink.tsx

@@ -5,7 +5,7 @@ import * as React from "react";
 import { useEffect, useState } from "react";
 import { useHistory } from "react-router-dom";
 import styled from "styled-components";
-import { UserPreference } from "@shared/types";
+import { type NavigationNode, UserPreference } from "@shared/types";
 import { ProsemirrorDataHelper } from "@shared/utils/ProsemirrorDataHelper";
 import type Collection from "~/models/Collection";
 import type Document from "~/models/Document";
@@ -75,6 +75,8 @@ type StarredCollectionLinkProps = {
   isDraggingAnyStar: boolean;
 };
 
+const emptyChildDocuments: NavigationNode[] = [];
+
 const StarredDocumentLink = observer(function StarredDocumentLink({
   star,
   document,
@@ -102,7 +104,7 @@ const StarredDocumentLink = observer(function StarredDocumentLink({
     : undefined;
   const childDocuments = documentCollection
     ? documentCollection.getChildrenForDocument(document.id)
-    : [];
+    : emptyChildDocuments;
   const hasChildDocuments = childDocuments.length > 0;
   const displayChildDocuments = expanded && !isDragging;
   const expansion = useSidebarExpansionState(

app/hooks/useDocumentMenuAction.tsx

@@ -1,7 +1,7 @@
-import { useMemo } from "react";
+import { useCallback } from "react";
 import { useTranslation } from "react-i18next";
 import { InputIcon, SearchIcon } from "outline-icons";
-import { ActionSeparator, createAction } from "~/actions";
+import { ActionSeparator, createAction, createRootMenuAction } from "~/actions";
 import {
   restoreDocument,
   unsubscribeDocument,
@@ -39,9 +39,7 @@ import { ActiveDocumentSection } from "~/actions/sections";
 import useMobile from "./useMobile";
 import type Template from "~/models/Template";
 import usePolicy from "./usePolicy";
-import useCurrentUser from "./useCurrentUser";
 import { useTemplateMenuActions } from "./useTemplateMenuActions";
-import { useMenuAction } from "./useMenuAction";
 
 type Props = {
   /** Document ID for which the actions are generated */
@@ -62,7 +60,6 @@ export function useDocumentMenuAction({
 }: Props) {
   const { t } = useTranslation();
   const isMobile = useMobile();
-  const user = useCurrentUser();
   const can = usePolicy(documentId);
 
   const templateMenuActions = useTemplateMenuActions({
@@ -70,67 +67,58 @@ export function useDocumentMenuAction({
     onSelectTemplate,
   });
 
-  const actions = useMemo(
-    () => [
-      restoreDocument,
-      restoreDocumentToCollection,
-      starDocument,
-      unstarDocument,
-      subscribeDocument,
-      unsubscribeDocument,
-      createAction({
-        name: `${t("Find and replace")}…`,
-        section: ActiveDocumentSection,
-        icon: <SearchIcon />,
-        visible: !!onFindAndReplace && isMobile,
-        perform: () => onFindAndReplace?.(),
-      }),
-      ActionSeparator,
-      editDocument,
-      createAction({
-        name: `${t("Rename")}…`,
-        section: ActiveDocumentSection,
-        icon: <InputIcon />,
-        visible: !!can.update && !!onRename,
-        perform: () => requestAnimationFrame(() => onRename?.()),
-      }),
-      shareDocument,
-      createTemplateFromDocument,
-      duplicateDocument,
-      publishDocument,
-      unpublishDocument,
-      archiveDocument,
-      moveDocument,
-      applyTemplateFactory({ actions: templateMenuActions }),
-      importDocument,
-      createNewDocument,
-      createNewDocumentInAlphabeticalCollection,
-      pinDocument,
-      ActionSeparator,
-      openDocumentComments,
-      openDocumentHistory,
-      openDocumentInsights,
-      openDocumentInDesktop,
-      presentDocument,
-      downloadDocument,
-      copyDocument,
-      printDocument,
-      searchInDocument,
-      ActionSeparator,
-      deleteDocument,
-      permanentlyDeleteDocument,
-      leaveDocument,
-    ],
-    [
-      t,
-      isMobile,
-      templateMenuActions,
-      can.update,
-      user.separateEditMode,
-      onFindAndReplace,
-      onRename,
-    ]
+  return useCallback(
+    () =>
+      createRootMenuAction([
+        restoreDocument,
+        restoreDocumentToCollection,
+        starDocument,
+        unstarDocument,
+        subscribeDocument,
+        unsubscribeDocument,
+        createAction({
+          name: `${t("Find and replace")}…`,
+          section: ActiveDocumentSection,
+          icon: <SearchIcon />,
+          visible: !!onFindAndReplace && isMobile,
+          perform: () => onFindAndReplace?.(),
+        }),
+        ActionSeparator,
+        editDocument,
+        createAction({
+          name: `${t("Rename")}…`,
+          section: ActiveDocumentSection,
+          icon: <InputIcon />,
+          visible: !!can.update && !!onRename,
+          perform: () => requestAnimationFrame(() => onRename?.()),
+        }),
+        shareDocument,
+        createTemplateFromDocument,
+        duplicateDocument,
+        publishDocument,
+        unpublishDocument,
+        archiveDocument,
+        moveDocument,
+        applyTemplateFactory({ actions: templateMenuActions }),
+        importDocument,
+        createNewDocument,
+        createNewDocumentInAlphabeticalCollection,
+        pinDocument,
+        ActionSeparator,
+        openDocumentComments,
+        openDocumentHistory,
+        openDocumentInsights,
+        openDocumentInDesktop,
+        presentDocument,
+        downloadDocument,
+        copyDocument,
+        printDocument,
+        searchInDocument,
+        ActionSeparator,
+        deleteDocument,
+        permanentlyDeleteDocument,
+        leaveDocument,
+      ]),
+    [t, isMobile, templateMenuActions, can.update, onFindAndReplace, onRename]
   );
-
-  return useMenuAction(actions);
 }

app/hooks/useMenuAction.ts

@@ -12,11 +12,14 @@ import usePrevious from "./usePrevious";
 type Actions = (ActionVariant | ActionGroup | ActionSeparator)[];
 
 export function useMenuAction(actions: Actions) {
-  const rootActionRef = useRef<ActionWithChildren>(
-    createRootMenuAction(actions)
-  );
+  const rootActionRef = useRef<ActionWithChildren>();
   const prevActions = usePrevious(actions);
 
+  if (!rootActionRef.current) {
+    rootActionRef.current = createRootMenuAction(actions);
+    return rootActionRef.current;
+  }
+
   if (!prevActions || isEqual(actions, prevActions)) {
     return rootActionRef.current;
   }

app/models/Collection.ts

@@ -162,6 +162,29 @@ export default class Collection extends ParanoidModel {
     return sortNavigationNodes(this.documents, this.sort);
   }
 
+  /**
+   * Returns a lookup from document id to child documents.
+   *
+   * @returns a map of document id to child document nodes.
+   */
+  @computed({ keepAlive: true })
+  get childrenByDocumentId(): Map<string, NavigationNode[]> {
+    const childrenByDocumentId = new Map<string, NavigationNode[]>();
+
+    const travelNodes = (nodes: NavigationNode[]) => {
+      for (const node of nodes) {
+        childrenByDocumentId.set(node.id, node.children);
+        travelNodes(node.children);
+      }
+    };
+
+    if (this.sortedDocuments) {
+      travelNodes(this.sortedDocuments);
+    }
+
+    return childrenByDocumentId;
+  }
+
   /** The initial letter of the collection name as a string. */
   @computed
   get initial() {
@@ -319,24 +342,7 @@ export default class Collection extends ParanoidModel {
   }
 
   getChildrenForDocument(documentId: string) {
-    let result: NavigationNode[] = [];
-
-    const travelNodes = (nodes: NavigationNode[]) => {
-      nodes.forEach((node) => {
-        if (node.id === documentId) {
-          result = node.children;
-          return;
-        }
-
-        return travelNodes(node.children);
-      });
-    };
-
-    if (this.sortedDocuments) {
-      travelNodes(this.sortedDocuments);
-    }
-
-    return result;
+    return this.childrenByDocumentId.get(documentId) ?? [];
   }
 
   @computed

app/models/base/NavigableModel.ts

@@ -49,6 +49,29 @@ export default abstract class NavigableModel extends Model {
     return this.node?.children;
   }
 
+  /**
+   * Returns a lookup from document id to child documents.
+   *
+   * @returns a map of document id to child document nodes.
+   */
+  @computed({ keepAlive: true })
+  get childrenByDocumentId(): Map<string, NavigationNode[]> {
+    const childrenByDocumentId = new Map<string, NavigationNode[]>();
+
+    const travelNodes = (nodes: NavigationNode[]) => {
+      for (const node of nodes) {
+        childrenByDocumentId.set(node.id, node.children);
+        travelNodes(node.children);
+      }
+    };
+
+    if (this.node) {
+      travelNodes([this.node]);
+    }
+
+    return childrenByDocumentId;
+  }
+
   @action
   setDocuments(value: NavigationNode[] | undefined) {
     if (this.node && value) {
@@ -101,24 +124,7 @@ export default abstract class NavigableModel extends Model {
    * Returns the child documents structure for the document.
    */
   getChildrenForDocument(documentId: string) {
-    let result: NavigationNode[] = [];
-
-    const travelNodes = (nodes: NavigationNode[]) => {
-      nodes.forEach((node) => {
-        if (node.id === documentId) {
-          result = node.children;
-          return;
-        }
-
-        return travelNodes(node.children);
-      });
-    };
-
-    if (this.node) {
-      travelNodes([this.node]);
-    }
-
-    return result;
+    return this.childrenByDocumentId.get(documentId) ?? [];
   }
 
   /**

app/types.ts

@@ -183,6 +183,8 @@ export type ActionWithChildren = BaseAction & {
     | (ActionVariant | ActionGroup | ActionSeparator)[];
 };
 
+export type ActionFactory = () => ActionWithChildren;
+
 export type ActionVariant =
   | Action
   | InternalLinkAction