fix(core): fix menu bugs (#8074)

.eslintrc.js

@@ -248,7 +248,7 @@ const config = {
           'warn',
           {
             additionalHooks:
-              '(useAsyncCallback|useCatchEventCallback|useDraggable|useDropTarget)',
+              '(useAsyncCallback|useCatchEventCallback|useDraggable|useDropTarget|useRefEffect)',
           },
         ],
       },

packages/frontend/component/src/hooks/focus-and-select.ts

@@ -25,7 +25,9 @@ export const useAutoSelect = <T extends HTMLInputElement = HTMLInputElement>(
 
   useLayoutEffect(() => {
     if (ref.current && autoSelect) {
-      ref.current?.select();
+      setTimeout(() => {
+        ref.current?.select();
+      }, 0);
     }
   }, [autoSelect, ref]);
 

packages/frontend/component/src/hooks/index.ts

@@ -1 +1,2 @@
 export { useAutoFocus, useAutoSelect } from './focus-and-select';
+export { useRefEffect } from './use-ref-effect';

packages/frontend/component/src/hooks/use-ref-effect.ts

@@ -0,0 +1,92 @@
+/**
+ * modified version of useRefEffect from https://github.com/jantimon/react-use-ref-effect/blob/master/src/index.tsx
+ */
+import { useDebugValue, useEffect, useState } from 'react';
+
+// internalRef is used as a reference and therefore save to be used inside an effect
+/* eslint-disable react-hooks/exhaustive-deps */
+
+// the `process.env.NODE_ENV !== 'production'` condition is resolved by the build tool
+/* eslint-disable react-hooks/rules-of-hooks */
+
+const noop: (...args: any[]) => any = () => {};
+
+/**
+ * `useRefEffect` returns a mutable ref object to be connected with a DOM Node.
+ *
+ * The returned object will persist for the full lifetime of the component.
+ * Accepts a function that contains imperative, possibly effectful code.
+ *
+ * @param effect Imperative function that can return a cleanup function
+ * @param deps If present, effect will only activate if the ref or the values in the list change.
+ */
+export const useRefEffect = <T>(
+  effect: (element: T) => void | (() => void),
+  dependencies: any[] = []
+): React.RefCallback<T> & React.MutableRefObject<T | null> => {
+  // Use the initial state as mutable reference
+  const internalRef = useState(() => {
+    let currentValue = null as T | null;
+    let cleanupPreviousEffect = noop as () => void;
+    let currentDeps: any[] | undefined;
+    /**
+     * React.RefCallback
+     */
+    const setRefValue = (newElement: T | null) => {
+      // Only execute if dependencies or element changed:
+      if (
+        internalRef.dependencies_ !== currentDeps ||
+        currentValue !== newElement
+      ) {
+        currentValue = newElement;
+        currentDeps = internalRef.dependencies_;
+        internalRef.cleanup_();
+        if (newElement) {
+          cleanupPreviousEffect = internalRef.effect_(newElement) || noop;
+        }
+      }
+    };
+    return {
+      /** Execute the effects cleanup function */
+      cleanup_: () => {
+        cleanupPreviousEffect();
+        cleanupPreviousEffect = noop;
+      },
+      ref_: Object.defineProperty(setRefValue, 'current', {
+        get: () => currentValue,
+        set: setRefValue,
+      }),
+    } as {
+      cleanup_: () => void;
+      ref_: React.RefCallback<T> & React.MutableRefObject<T | null>;
+      // Those two properties will be set immediately after initialisation
+      effect_: typeof effect;
+      dependencies_: typeof dependencies;
+    };
+  })[0];
+
+  // Show the current ref value in development
+  // in react dev tools
+  if (process.env.NODE_ENV !== 'production') {
+    useDebugValue(internalRef.ref_.current);
+  }
+
+  // Keep a ref to the latest callback
+  internalRef.effect_ = effect;
+
+  useEffect(
+    () => {
+      // Run effect if dependencies change
+      internalRef.ref_(internalRef.ref_.current);
+      return () => {
+        if (internalRef.dependencies_ === dependencies) {
+          internalRef.cleanup_();
+          internalRef.dependencies_ = [];
+        }
+      };
+    }, // Keep a ref to the latest dependencies
+    (internalRef.dependencies_ = dependencies)
+  );
+
+  return internalRef.ref_;
+};

packages/frontend/component/src/ui/editable/inline-edit.stories.tsx

@@ -57,7 +57,6 @@ Basic.args = {
   editable: true,
   placeholder: 'Untitled',
   trigger: 'doubleClick',
-  autoSelect: true,
 };
 
 export const CustomizeText: StoryFn<typeof InlineEdit> =
@@ -104,7 +103,6 @@ export const TriggerEdit: StoryFn<typeof InlineEdit> = args => {
 TriggerEdit.args = {
   value: 'Trigger edit mode in parent component by `handleRef`',
   editable: true,
-  autoSelect: true,
 };
 
 export const UpdateValue: StoryFn<typeof InlineEdit> = args => {
@@ -137,5 +135,4 @@ export const UpdateValue: StoryFn<typeof InlineEdit> = args => {
 UpdateValue.args = {
   value: 'Update value in parent component by `value`',
   editable: true,
-  autoSelect: true,
 };

packages/frontend/component/src/ui/editable/inline-edit.tsx

@@ -46,11 +46,6 @@ export interface InlineEditProps
    */
   trigger?: 'click' | 'doubleClick';
 
-  /**
-   * whether to auto select all text when trigger edit
-   */
-  autoSelect?: boolean;
-
   /**
    * Placeholder when value is empty
    */
@@ -79,7 +74,6 @@ export const InlineEdit = ({
   className,
   style,
   trigger = 'doubleClick',
-  autoSelect,
 
   onInput,
   onChange,
@@ -104,11 +98,7 @@ export const InlineEdit = ({
   const triggerEdit = useCallback(() => {
     if (!editable) return;
     setEditing(true);
-    setTimeout(() => {
-      inputRef.current?.focus();
-      autoSelect && inputRef.current?.select();
-    }, 0);
-  }, [autoSelect, editable]);
+  }, [editable]);
 
   const onDoubleClick = useCallback(() => {
     if (trigger !== 'doubleClick') return;
@@ -208,7 +198,7 @@ export const InlineEdit = ({
       </div>
 
       {/* actual input */}
-      {
+      {editing && (
         <Input
           ref={inputRef}
           className={styles.inlineEditInput}
@@ -220,9 +210,11 @@ export const InlineEdit = ({
           style={inputWrapperInheritsStyles}
           inputStyle={inputInheritsStyles}
           onBlur={onBlur}
+          autoFocus
+          autoSelect
           {...inputAttrs}
         />
-      }
+      )}
     </div>
   );
 };

packages/frontend/component/src/ui/menu/desktop/controller.ts

@@ -1,20 +1,24 @@
-import { useCallback, useEffect, useRef, useState } from 'react';
+import { useCallback, useState } from 'react';
+
+import { useRefEffect } from '../../../hooks';
 
 export const useMenuContentController = ({
   onOpenChange,
   side,
   defaultOpen,
   sideOffset,
+  open: controlledOpen,
 }: {
   defaultOpen?: boolean;
   side?: 'top' | 'bottom' | 'left' | 'right';
   onOpenChange?: (open: boolean) => void;
+  open?: boolean;
   sideOffset?: number;
 } = {}) => {
   const [open, setOpen] = useState(defaultOpen ?? false);
+  const actualOpen = controlledOpen ?? open;
   const contentSide = side ?? 'bottom';
   const [contentOffset, setContentOffset] = useState<number>(0);
-  const contentRef = useRef<HTMLDivElement>(null);
 
   const handleOpenChange = useCallback(
     (open: boolean) => {
@@ -23,65 +27,69 @@ export const useMenuContentController = ({
     },
     [onOpenChange]
   );
-  useEffect(() => {
-    if (!open || !contentRef.current) return;
+  const contentRef = useRefEffect<HTMLDivElement>(
+    contentElement => {
+      if (!actualOpen) return;
 
-    const wrapperElement = contentRef.current.parentNode as HTMLDivElement;
+      const wrapperElement = contentElement.parentNode as HTMLDivElement;
 
-    const updateContentOffset = () => {
-      if (!contentRef.current) return;
-      const contentRect = wrapperElement.getBoundingClientRect();
-      if (contentSide === 'bottom') {
-        setContentOffset(prev => {
-          const viewportHeight = window.innerHeight;
-          const newOffset = Math.min(
-            viewportHeight - (contentRect.bottom - prev),
-            0
-          );
-          return newOffset;
-        });
-      } else if (contentSide === 'top') {
-        setContentOffset(prev => {
-          const newOffset = Math.max(contentRect.top - prev, 0);
-          return newOffset;
-        });
-      } else if (contentSide === 'left') {
-        setContentOffset(prev => {
-          const newOffset = Math.max(contentRect.left - prev, 0);
-          return newOffset;
-        });
-      } else if (contentSide === 'right') {
-        setContentOffset(prev => {
-          const viewportWidth = window.innerWidth;
-          const newOffset = Math.min(
-            viewportWidth - (contentRect.right - prev),
-            0
-          );
-          return newOffset;
-        });
-      }
-    };
-    let animationFrame: number = 0;
-    const requestUpdateContentOffset = () => {
-      cancelAnimationFrame(animationFrame);
-      animationFrame = requestAnimationFrame(updateContentOffset);
-    };
+      const updateContentOffset = () => {
+        if (!contentElement) return;
+        const contentRect = wrapperElement.getBoundingClientRect();
+        if (contentSide === 'bottom') {
+          setContentOffset(prev => {
+            const viewportHeight = window.innerHeight;
+            const newOffset = Math.min(
+              viewportHeight - (contentRect.bottom - prev),
+              0
+            );
+            return newOffset;
+          });
+        } else if (contentSide === 'top') {
+          setContentOffset(prev => {
+            const newOffset = Math.min(contentRect.top + prev, 0);
+            return newOffset;
+          });
+        } else if (contentSide === 'left') {
+          setContentOffset(prev => {
+            const newOffset = Math.min(contentRect.left + prev, 0);
+            return newOffset;
+          });
+        } else if (contentSide === 'right') {
+          setContentOffset(prev => {
+            const viewportWidth = window.innerWidth;
+            const newOffset = Math.min(
+              viewportWidth - (contentRect.right - prev),
+              0
+            );
+            return newOffset;
+          });
+        }
+      };
+      let animationFrame: number = 0;
+      const requestUpdateContentOffset = () => {
+        cancelAnimationFrame(animationFrame);
+        animationFrame = requestAnimationFrame(updateContentOffset);
+      };
 
-    const observer = new ResizeObserver(requestUpdateContentOffset);
-    observer.observe(wrapperElement);
-    window.addEventListener('resize', requestUpdateContentOffset);
-    requestUpdateContentOffset();
-    return () => {
-      observer.disconnect();
-      window.removeEventListener('resize', requestUpdateContentOffset);
-      cancelAnimationFrame(animationFrame);
-    };
-  }, [contentSide, open]);
+      const observer = new ResizeObserver(requestUpdateContentOffset);
+      observer.observe(wrapperElement);
+      window.addEventListener('resize', requestUpdateContentOffset);
+      requestUpdateContentOffset();
+      return () => {
+        observer.disconnect();
+        window.removeEventListener('resize', requestUpdateContentOffset);
+        cancelAnimationFrame(animationFrame);
+      };
+    },
+    [actualOpen, contentSide]
+  );
 
   return {
     handleOpenChange,
     contentSide,
     contentOffset: (sideOffset ?? 0) + contentOffset,
     contentRef,
+    open: actualOpen,
   };
 };

packages/frontend/component/src/ui/menu/desktop/root.tsx

@@ -10,7 +10,13 @@ export const DesktopMenu = ({
   children,
   items,
   portalOptions,
-  rootOptions: { onOpenChange, defaultOpen, modal, ...rootOptions } = {},
+  rootOptions: {
+    onOpenChange,
+    defaultOpen,
+    modal,
+    open: rootOpen,
+    ...rootOptions
+  } = {},
   contentOptions: {
     className = '',
     style: contentStyle = {},
@@ -19,8 +25,9 @@ export const DesktopMenu = ({
     ...otherContentOptions
   } = {},
 }: MenuProps) => {
-  const { handleOpenChange, contentSide, contentOffset, contentRef } =
+  const { handleOpenChange, contentSide, contentOffset, contentRef, open } =
     useMenuContentController({
+      open: rootOpen,
       defaultOpen,
       onOpenChange,
       side,
@@ -31,6 +38,7 @@ export const DesktopMenu = ({
       onOpenChange={handleOpenChange}
       defaultOpen={defaultOpen}
       modal={modal ?? false}
+      open={open}
       {...rootOptions}
     >
       <DropdownMenu.Trigger

packages/frontend/component/src/ui/menu/desktop/sub.tsx

@@ -12,7 +12,12 @@ export const DesktopMenuSub = ({
   children: propsChildren,
   items,
   portalOptions,
-  subOptions: { defaultOpen, onOpenChange, ...otherSubOptions } = {},
+  subOptions: {
+    defaultOpen,
+    onOpenChange,
+    open: rootOpen,
+    ...otherSubOptions
+  } = {},
   triggerOptions,
   subContentOptions: {
     className: subContentClassName = '',
@@ -27,9 +32,10 @@ export const DesktopMenuSub = ({
     suffixIcon: <ArrowRightSmallIcon />,
   });
 
-  const { handleOpenChange, contentOffset, contentRef } =
+  const { handleOpenChange, contentOffset, contentRef, open } =
     useMenuContentController({
       defaultOpen,
+      open: rootOpen,
       onOpenChange,
       side: 'right',
       sideOffset: (sideOffset ?? 0) + 12,
@@ -39,6 +45,7 @@ export const DesktopMenuSub = ({
     <DropdownMenu.Sub
       defaultOpen={defaultOpen}
       onOpenChange={handleOpenChange}
+      open={open}
       {...otherSubOptions}
     >
       <DropdownMenu.SubTrigger className={className} {...otherProps}>

packages/frontend/core/src/components/blocksuite/block-suite-header/title/index.tsx

@@ -43,7 +43,6 @@ export const BlocksuiteHeaderTitle = (props: BlockSuiteHeaderTitleProps) => {
   return (
     <InlineEdit
       className={clsx(styles.title, props.className)}
-      autoSelect
       value={docTitle}
       onChange={onChange}
       editable={!isSharedMode}

packages/frontend/core/src/modules/explorer/views/tree/node.tsx

@@ -376,9 +376,9 @@ export const ExplorerTreeNode = ({
         </div>
       </div>
 
-      {renameable && renaming && (
+      {renameable && (
         <RenameModal
-          open
+          open={!!renaming}
           width={sidebarWidth - 32}
           onOpenChange={setRenaming}
           onRename={handleRename}

packages/frontend/core/src/pages/workspace/detail-page/detail-page-header.tsx

@@ -116,7 +116,10 @@ export function NormalPageHeader({ page, workspace }: PageHeaderProps) {
     useDetailPageHeaderResponsive(containerWidth);
 
   const onRename = useCallback(() => {
-    setTimeout(() => titleInputHandleRef.current?.triggerEdit());
+    setTimeout(
+      () => titleInputHandleRef.current?.triggerEdit(),
+      500 /* wait for menu animation end */
+    );
   }, []);
 
   const title = useDocCollectionPageTitle(workspace.docCollection, page?.id);