From 51f3566bec3114ae34444248f91d934f1217e19e Mon Sep 17 00:00:00 2001 From: EYHN Date: Wed, 4 Sep 2024 07:19:08 +0000 Subject: [PATCH] fix(core): fix menu bugs (#8074) --- .eslintrc.js | 2 +- .../component/src/hooks/focus-and-select.ts | 4 +- .../frontend/component/src/hooks/index.ts | 1 + .../component/src/hooks/use-ref-effect.ts | 92 ++++++++++++++ .../src/ui/editable/inline-edit.stories.tsx | 3 - .../component/src/ui/editable/inline-edit.tsx | 18 +-- .../src/ui/menu/desktop/controller.ts | 114 ++++++++++-------- .../component/src/ui/menu/desktop/root.tsx | 12 +- .../component/src/ui/menu/desktop/sub.tsx | 11 +- .../block-suite-header/title/index.tsx | 1 - .../src/modules/explorer/views/tree/node.tsx | 4 +- .../detail-page/detail-page-header.tsx | 5 +- 12 files changed, 188 insertions(+), 79 deletions(-) create mode 100644 packages/frontend/component/src/hooks/use-ref-effect.ts diff --git a/.eslintrc.js b/.eslintrc.js index c96058a4ed..7832c9da5a 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -248,7 +248,7 @@ const config = { 'warn', { additionalHooks: - '(useAsyncCallback|useCatchEventCallback|useDraggable|useDropTarget)', + '(useAsyncCallback|useCatchEventCallback|useDraggable|useDropTarget|useRefEffect)', }, ], }, diff --git a/packages/frontend/component/src/hooks/focus-and-select.ts b/packages/frontend/component/src/hooks/focus-and-select.ts index edd92c5eaa..6f3242e5c6 100644 --- a/packages/frontend/component/src/hooks/focus-and-select.ts +++ b/packages/frontend/component/src/hooks/focus-and-select.ts @@ -25,7 +25,9 @@ export const useAutoSelect = ( useLayoutEffect(() => { if (ref.current && autoSelect) { - ref.current?.select(); + setTimeout(() => { + ref.current?.select(); + }, 0); } }, [autoSelect, ref]); diff --git a/packages/frontend/component/src/hooks/index.ts b/packages/frontend/component/src/hooks/index.ts index 2c69d81da9..93ae56c400 100644 --- a/packages/frontend/component/src/hooks/index.ts +++ b/packages/frontend/component/src/hooks/index.ts @@ -1 +1,2 @@ export { useAutoFocus, useAutoSelect } from './focus-and-select'; +export { useRefEffect } from './use-ref-effect'; diff --git a/packages/frontend/component/src/hooks/use-ref-effect.ts b/packages/frontend/component/src/hooks/use-ref-effect.ts new file mode 100644 index 0000000000..4530a83c28 --- /dev/null +++ b/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 = ( + effect: (element: T) => void | (() => void), + dependencies: any[] = [] +): React.RefCallback & React.MutableRefObject => { + // 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 & React.MutableRefObject; + // 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_; +}; diff --git a/packages/frontend/component/src/ui/editable/inline-edit.stories.tsx b/packages/frontend/component/src/ui/editable/inline-edit.stories.tsx index 694ab14558..0424ed2c49 100644 --- a/packages/frontend/component/src/ui/editable/inline-edit.stories.tsx +++ b/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 = @@ -104,7 +103,6 @@ export const TriggerEdit: StoryFn = args => { TriggerEdit.args = { value: 'Trigger edit mode in parent component by `handleRef`', editable: true, - autoSelect: true, }; export const UpdateValue: StoryFn = args => { @@ -137,5 +135,4 @@ export const UpdateValue: StoryFn = args => { UpdateValue.args = { value: 'Update value in parent component by `value`', editable: true, - autoSelect: true, }; diff --git a/packages/frontend/component/src/ui/editable/inline-edit.tsx b/packages/frontend/component/src/ui/editable/inline-edit.tsx index a6229ae58f..b67aa47cd1 100644 --- a/packages/frontend/component/src/ui/editable/inline-edit.tsx +++ b/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 = ({ {/* actual input */} - { + {editing && ( - } + )} ); }; diff --git a/packages/frontend/component/src/ui/menu/desktop/controller.ts b/packages/frontend/component/src/ui/menu/desktop/controller.ts index 0a5e94f997..d96f615595 100644 --- a/packages/frontend/component/src/ui/menu/desktop/controller.ts +++ b/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(0); - const contentRef = useRef(null); const handleOpenChange = useCallback( (open: boolean) => { @@ -23,65 +27,69 @@ export const useMenuContentController = ({ }, [onOpenChange] ); - useEffect(() => { - if (!open || !contentRef.current) return; + const contentRef = useRefEffect( + 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, }; }; diff --git a/packages/frontend/component/src/ui/menu/desktop/root.tsx b/packages/frontend/component/src/ui/menu/desktop/root.tsx index 9bcfc26c5d..cad1a0f0ea 100644 --- a/packages/frontend/component/src/ui/menu/desktop/root.tsx +++ b/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} > , }); - 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 = ({ diff --git a/packages/frontend/core/src/components/blocksuite/block-suite-header/title/index.tsx b/packages/frontend/core/src/components/blocksuite/block-suite-header/title/index.tsx index 0ee3660860..45d597fdd9 100644 --- a/packages/frontend/core/src/components/blocksuite/block-suite-header/title/index.tsx +++ b/packages/frontend/core/src/components/blocksuite/block-suite-header/title/index.tsx @@ -43,7 +43,6 @@ export const BlocksuiteHeaderTitle = (props: BlockSuiteHeaderTitleProps) => { return ( - {renameable && renaming && ( + {renameable && ( { - setTimeout(() => titleInputHandleRef.current?.triggerEdit()); + setTimeout( + () => titleInputHandleRef.current?.triggerEdit(), + 500 /* wait for menu animation end */ + ); }, []); const title = useDocCollectionPageTitle(workspace.docCollection, page?.id);