From bf5ad6c769e727130fb964b5a56ca7cabb94f7ab Mon Sep 17 00:00:00 2001 From: Zheng Song Date: Tue, 30 Jul 2024 23:44:38 +1000 Subject: [PATCH 1/4] Fix infinite repositioning when overflow causes menu to resize --- dist/es/components/MenuList.js | 7 ++----- dist/index.js | 7 ++----- src/components/MenuList.js | 17 ++--------------- 3 files changed, 6 insertions(+), 25 deletions(-) diff --git a/dist/es/components/MenuList.js b/dist/es/components/MenuList.js index 23e59a04..b6019b8e 100644 --- a/dist/es/components/MenuList.js +++ b/dist/es/components/MenuList.js @@ -174,14 +174,13 @@ const MenuList = ({ getBottomOverflow } = positionHelpers; let height, overflowAmt; - const prevHeight = latestMenuSize.current.height; const bottomOverflow = getBottomOverflow(y); - if (bottomOverflow > 0 || floatEqual(bottomOverflow, 0) && floatEqual(menuHeight, prevHeight)) { + if (bottomOverflow > 0) { height = menuHeight - bottomOverflow; overflowAmt = bottomOverflow; } else { const topOverflow = getTopOverflow(y); - if (topOverflow < 0 || floatEqual(topOverflow, 0) && floatEqual(menuHeight, prevHeight)) { + if (topOverflow < 0) { height = menuHeight + topOverflow; overflowAmt = 0 - topOverflow; if (height >= 0) y -= topOverflow; @@ -193,8 +192,6 @@ const MenuList = ({ height, overflowAmt }); - } else { - setOverflowData(); } } if (arrow) setArrowPosition({ diff --git a/dist/index.js b/dist/index.js index 472349f6..6e955cd5 100644 --- a/dist/index.js +++ b/dist/index.js @@ -1037,14 +1037,13 @@ const MenuList = ({ getBottomOverflow } = positionHelpers; let height, overflowAmt; - const prevHeight = latestMenuSize.current.height; const bottomOverflow = getBottomOverflow(y); - if (bottomOverflow > 0 || floatEqual(bottomOverflow, 0) && floatEqual(menuHeight, prevHeight)) { + if (bottomOverflow > 0) { height = menuHeight - bottomOverflow; overflowAmt = bottomOverflow; } else { const topOverflow = getTopOverflow(y); - if (topOverflow < 0 || floatEqual(topOverflow, 0) && floatEqual(menuHeight, prevHeight)) { + if (topOverflow < 0) { height = menuHeight + topOverflow; overflowAmt = 0 - topOverflow; if (height >= 0) y -= topOverflow; @@ -1056,8 +1055,6 @@ const MenuList = ({ height, overflowAmt }); - } else { - setOverflowData(); } } if (arrow) setArrowPosition({ diff --git a/src/components/MenuList.js b/src/components/MenuList.js index 430a72ab..d842730e 100644 --- a/src/components/MenuList.js +++ b/src/components/MenuList.js @@ -203,24 +203,13 @@ export const MenuList = ({ const { getTopOverflow, getBottomOverflow } = positionHelpers; let height, overflowAmt; - const prevHeight = latestMenuSize.current.height; const bottomOverflow = getBottomOverflow(y); - // When bottomOverflow is 0, menu is on the bottom edge of viewport - // This might be the result of a previous maxHeight set on the menu. - // In this situation, we need to still apply a new maxHeight. - // Same reason for the top side - if ( - bottomOverflow > 0 || - (floatEqual(bottomOverflow, 0) && floatEqual(menuHeight, prevHeight)) - ) { + if (bottomOverflow > 0) { height = menuHeight - bottomOverflow; overflowAmt = bottomOverflow; } else { const topOverflow = getTopOverflow(y); - if ( - topOverflow < 0 || - (floatEqual(topOverflow, 0) && floatEqual(menuHeight, prevHeight)) - ) { + if (topOverflow < 0) { height = menuHeight + topOverflow; overflowAmt = 0 - topOverflow; // avoid getting -0 if (height >= 0) y -= topOverflow; @@ -231,8 +220,6 @@ export const MenuList = ({ // To avoid triggering reposition in the next ResizeObserver callback menuHeight = height; setOverflowData({ height, overflowAmt }); - } else { - setOverflowData(); } } From a386d7b51daac72861f35286d0c7ce745db3ccdd Mon Sep 17 00:00:00 2001 From: Zheng Song Date: Wed, 31 Jul 2024 23:35:06 +1000 Subject: [PATCH 2/4] Reposition menu after anchor size has changed --- example/src/data/documentation.js | 4 +- src/components/MenuList.js | 66 ++++++++++++------------------- types/index.d.ts | 2 +- 3 files changed, 29 insertions(+), 43 deletions(-) diff --git a/example/src/data/documentation.js b/example/src/data/documentation.js index c7758728..2a28fce3 100644 --- a/example/src/data/documentation.js +++ b/example/src/data/documentation.js @@ -466,8 +466,8 @@ const rootMenuProps = [ want to explicitly reposition menu using the repositionFlag prop.
  • - 'auto' Reposition menu whenever its size has changed, using the{' '} - ResizeObserver API. + 'auto' Reposition menu whenever itself or the anchor has changed in size, + using the ResizeObserver API.
  • diff --git a/src/components/MenuList.js b/src/components/MenuList.js index d842730e..21c542f5 100644 --- a/src/components/MenuList.js +++ b/src/components/MenuList.js @@ -2,13 +2,12 @@ import { useState, useReducer, useEffect, useRef, useMemo, useCallback, useConte import { flushSync } from 'react-dom'; import { MenuContainer } from './MenuContainer'; import { useBEM, useCombinedRef, useLayoutEffect, useItems } from '../hooks'; -import { getNormalizedClientRect, getPositionHelpers, positionMenu } from '../positionUtils'; +import { getPositionHelpers, positionMenu } from '../positionUtils'; import { mergeProps, batchedUpdates, commonProps, createSubmenuCtx, - floatEqual, getScrollAncestor, getTransition, positionAbsolute, @@ -84,8 +83,6 @@ export const MenuList = ({ const focusRef = useRef(); const arrowRef = useRef(); const prevOpen = useRef(false); - const latestMenuSize = useRef({ width: 0, height: 0 }); - const latestHandlePosition = useRef(() => {}); const { hoverItem, dispatch, updateItems } = useItems(menuRef, focusRef); const isOpen = isMenuOpen(state); @@ -197,7 +194,7 @@ export const MenuList = ({ }); const { menuRect } = positionHelpers; - let menuHeight = menuRect.height; + const menuHeight = menuRect.height; if (!noOverflowCheck && overflow !== 'visible') { const { getTopOverflow, getBottomOverflow } = positionHelpers; @@ -217,8 +214,6 @@ export const MenuList = ({ } if (height >= 0) { - // To avoid triggering reposition in the next ResizeObserver callback - menuHeight = height; setOverflowData({ height, overflowAmt }); } } @@ -226,7 +221,6 @@ export const MenuList = ({ if (arrow) setArrowPosition({ x: arrowX, y: arrowY }); setMenuPosition({ x, y }); setExpandedDirection(computedDirection); - latestMenuSize.current = { width: menuRect.width, height: menuHeight }; }, [ arrow, @@ -253,7 +247,6 @@ export const MenuList = ({ if (prevOpen.current) forceReposSubmenu(); } prevOpen.current = isOpen; - latestHandlePosition.current = handlePosition; }, [isOpen, handlePosition, /* effect dep */ reposFlag]); useLayoutEffect(() => { @@ -304,37 +297,30 @@ export const MenuList = ({ }, [isOpen, hasOverflow, parentScrollingRef, handlePosition]); useEffect(() => { - if (typeof ResizeObserver !== 'function' || reposition === 'initial') return; - - const resizeObserver = new ResizeObserver(([entry]) => { - const { borderBoxSize, target } = entry; - let width, height; - if (borderBoxSize) { - const { inlineSize, blockSize } = borderBoxSize[0] || borderBoxSize; - width = inlineSize; - height = blockSize; - } else { - const borderRect = getNormalizedClientRect(target); - width = borderRect.width; - height = borderRect.height; - } - - if (width === 0 || height === 0) return; - if ( - floatEqual(width, latestMenuSize.current.width, 1) && - floatEqual(height, latestMenuSize.current.height, 1) - ) - return; - flushSync(() => { - latestHandlePosition.current(); - forceReposSubmenu(); - }); - }); - - const observeTarget = menuRef.current; - resizeObserver.observe(observeTarget, { box: 'border-box' }); - return () => resizeObserver.unobserve(observeTarget); - }, [reposition]); + if (!isOpen || typeof ResizeObserver !== 'function' || reposition === 'initial') return; + + const targetList = []; + const resizeObserver = new ResizeObserver((entries) => + entries.forEach(({ target }) => { + if (targetList.indexOf(target) < 0) { + // ResizeObserver callback fires on initial observe call, + // use this workaround to skip the first callback + targetList.push(target); + } else { + flushSync(() => { + handlePosition(); + forceReposSubmenu(); + }); + } + }) + ); + + const resizeObserverOptions = { box: 'border-box' }; + resizeObserver.observe(menuRef.current, resizeObserverOptions); + const anchor = anchorRef?.current; + anchor && resizeObserver.observe(anchor, resizeObserverOptions); + return () => resizeObserver.disconnect(); + }, [isOpen, reposition, anchorRef, handlePosition]); useEffect(() => { if (!isOpen) { diff --git a/types/index.d.ts b/types/index.d.ts index 80f9adb3..4c0b284c 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -306,7 +306,7 @@ export interface RootMenuProps extends BaseMenuProps, Omit Date: Thu, 1 Aug 2024 00:13:03 +1000 Subject: [PATCH 3/4] Fix minor docs issue --- example/src/components/Example.js | 2 +- example/src/data/codeExamples.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/example/src/components/Example.js b/example/src/components/Example.js index d0cb7a7e..af590b71 100644 --- a/example/src/components/Example.js +++ b/example/src/components/Example.js @@ -123,7 +123,7 @@ export const Example = React.memo( {sourceCode && ( -
    +          
                 {sourceCode}
               
    )} diff --git a/example/src/data/codeExamples.js b/example/src/data/codeExamples.js index 503cd827..8ba3500d 100644 --- a/example/src/data/codeExamples.js +++ b/example/src/data/codeExamples.js @@ -22,7 +22,7 @@ export const basicMenu = { fullSource: `import { Menu, MenuItem, MenuButton } from '@szhsin/react-menu'; import '@szhsin/react-menu/dist/index.css'; -import '@szhsin/react-menu/dist/transitions/slide.css'; +import '@szhsin/react-menu/dist/transitions/zoom.css'; export default function Example() { return ( @@ -955,7 +955,7 @@ export const menuStateHook = { fullSource: `import { useRef } from 'react'; import { ControlledMenu, MenuItem, useClick, useMenuState } from '@szhsin/react-menu'; import '@szhsin/react-menu/dist/index.css'; -import '@szhsin/react-menu/dist/transitions/slide.css'; +import '@szhsin/react-menu/dist/transitions/zoom.css'; export default function () { const ref = useRef(null); @@ -1005,7 +1005,7 @@ export const hoverMenu = { fullSource: `import { useRef, useState } from 'react'; import { ControlledMenu, MenuItem, useHover, useMenuState } from '@szhsin/react-menu'; import '@szhsin/react-menu/dist/index.css'; -import '@szhsin/react-menu/dist/transitions/slide.css'; +import '@szhsin/react-menu/dist/transitions/zoom.css'; const HoverMenu = () => { const ref = useRef(null); From 0e2bbd6b687de479ec787e1e5ffe97b21deed047 Mon Sep 17 00:00:00 2001 From: Zheng Song Date: Thu, 1 Aug 2024 20:08:05 +1000 Subject: [PATCH 4/4] 4.2.1 --- dist/es/components/MenuList.js | 64 +++++++++++----------------------- dist/es/utils/utils.js | 3 +- dist/index.js | 62 +++++++++++--------------------- example/package-lock.json | 2 +- example/src/utils/index.js | 4 +-- package-lock.json | 4 +-- package.json | 2 +- 7 files changed, 48 insertions(+), 93 deletions(-) diff --git a/dist/es/components/MenuList.js b/dist/es/components/MenuList.js index b6019b8e..82e771ee 100644 --- a/dist/es/components/MenuList.js +++ b/dist/es/components/MenuList.js @@ -5,11 +5,10 @@ import { jsxs, jsx } from 'react/jsx-runtime'; import { createSubmenuCtx } from '../utils/submenuCtx.js'; import { SettingsContext, MenuListContext, HoverActionTypes, menuClass, menuArrowClass, positionAbsolute, dummyItemProps, MenuListItemContext, HoverItemContext, Keys, CloseReason, FocusPositions } from '../utils/constants.js'; import { useItems } from '../hooks/useItems.js'; -import { getScrollAncestor, floatEqual, commonProps, mergeProps, safeCall, isMenuOpen, getTransition, batchedUpdates } from '../utils/utils.js'; +import { getScrollAncestor, commonProps, mergeProps, safeCall, isMenuOpen, getTransition, batchedUpdates } from '../utils/utils.js'; import { getPositionHelpers } from '../positionUtils/getPositionHelpers.js'; import { positionMenu } from '../positionUtils/positionMenu.js'; import { useLayoutEffect as useIsomorphicLayoutEffect } from '../hooks/useIsomorphicLayoutEffect.js'; -import { getNormalizedClientRect } from '../positionUtils/getNormalizedClientRect.js'; import { useBEM } from '../hooks/useBEM.js'; import { useCombinedRef } from '../hooks/useCombinedRef.js'; @@ -72,11 +71,6 @@ const MenuList = ({ const focusRef = useRef(); const arrowRef = useRef(); const prevOpen = useRef(false); - const latestMenuSize = useRef({ - width: 0, - height: 0 - }); - const latestHandlePosition = useRef(() => {}); const { hoverItem, dispatch, @@ -167,7 +161,7 @@ const MenuList = ({ const { menuRect } = positionHelpers; - let menuHeight = menuRect.height; + const menuHeight = menuRect.height; if (!noOverflowCheck && overflow !== 'visible') { const { getTopOverflow, @@ -187,7 +181,6 @@ const MenuList = ({ } } if (height >= 0) { - menuHeight = height; setOverflowData({ height, overflowAmt @@ -203,10 +196,6 @@ const MenuList = ({ y }); setExpandedDirection(computedDirection); - latestMenuSize.current = { - width: menuRect.width, - height: menuHeight - }; }, [arrow, align, boundingBoxPadding, direction, gap, shift, position, overflow, anchorPoint, anchorRef, containerRef, boundingBoxRef, rootMenuRef, scrollNodes]); useIsomorphicLayoutEffect(() => { if (isOpen) { @@ -214,7 +203,6 @@ const MenuList = ({ if (prevOpen.current) forceReposSubmenu(); } prevOpen.current = isOpen; - latestHandlePosition.current = handlePosition; }, [isOpen, handlePosition, reposFlag]); useIsomorphicLayoutEffect(() => { if (overflowData && !setDownOverflow) menuRef.current.scrollTop = 0; @@ -259,38 +247,28 @@ const MenuList = ({ return () => parentScroll.removeEventListener('scroll', handleScroll); }, [isOpen, hasOverflow, parentScrollingRef, handlePosition]); useEffect(() => { - if (typeof ResizeObserver !== 'function' || reposition === 'initial') return; - const resizeObserver = new ResizeObserver(([entry]) => { - const { - borderBoxSize, - target - } = entry; - let width, height; - if (borderBoxSize) { - const { - inlineSize, - blockSize - } = borderBoxSize[0] || borderBoxSize; - width = inlineSize; - height = blockSize; + if (!isOpen || typeof ResizeObserver !== 'function' || reposition === 'initial') return; + const targetList = []; + const resizeObserver = new ResizeObserver(entries => entries.forEach(({ + target + }) => { + if (targetList.indexOf(target) < 0) { + targetList.push(target); } else { - const borderRect = getNormalizedClientRect(target); - width = borderRect.width; - height = borderRect.height; + flushSync(() => { + handlePosition(); + forceReposSubmenu(); + }); } - if (width === 0 || height === 0) return; - if (floatEqual(width, latestMenuSize.current.width, 1) && floatEqual(height, latestMenuSize.current.height, 1)) return; - flushSync(() => { - latestHandlePosition.current(); - forceReposSubmenu(); - }); - }); - const observeTarget = menuRef.current; - resizeObserver.observe(observeTarget, { + })); + const resizeObserverOptions = { box: 'border-box' - }); - return () => resizeObserver.unobserve(observeTarget); - }, [reposition]); + }; + resizeObserver.observe(menuRef.current, resizeObserverOptions); + const anchor = anchorRef == null ? void 0 : anchorRef.current; + anchor && resizeObserver.observe(anchor, resizeObserverOptions); + return () => resizeObserver.disconnect(); + }, [isOpen, reposition, anchorRef, handlePosition]); useEffect(() => { if (!isOpen) { dispatch(HoverActionTypes.RESET); diff --git a/dist/es/utils/utils.js b/dist/es/utils/utils.js index 46dc2a85..b1a506dc 100644 --- a/dist/es/utils/utils.js +++ b/dist/es/utils/utils.js @@ -3,7 +3,6 @@ import { unstable_batchedUpdates } from 'react-dom'; const isMenuOpen = state => !!state && state[0] === 'o'; const batchedUpdates = unstable_batchedUpdates || (callback => callback()); const values = Object.values || (obj => Object.keys(obj).map(key => obj[key])); -const floatEqual = (a, b, diff = 0.0001) => Math.abs(a - b) < diff; const getTransition = (transition, name) => transition === true || !!(transition && transition[name]); const safeCall = (fn, arg) => typeof fn === 'function' ? fn(arg) : fn; const internalKey = '_szhsinMenu'; @@ -68,4 +67,4 @@ function indexOfNode(nodeList, node) { return -1; } -export { batchedUpdates, commonProps, defineName, floatEqual, getName, getScrollAncestor, getTransition, indexOfNode, isMenuOpen, mergeProps, parsePadding, safeCall, values }; +export { batchedUpdates, commonProps, defineName, getName, getScrollAncestor, getTransition, indexOfNode, isMenuOpen, mergeProps, parsePadding, safeCall, values }; diff --git a/dist/index.js b/dist/index.js index 6e955cd5..bc0af00f 100644 --- a/dist/index.js +++ b/dist/index.js @@ -143,7 +143,6 @@ const dummyItemProps = { const isMenuOpen = state => !!state && state[0] === 'o'; const batchedUpdates = reactDom.unstable_batchedUpdates || (callback => callback()); const values = Object.values || (obj => Object.keys(obj).map(key => obj[key])); -const floatEqual = (a, b, diff = 0.0001) => Math.abs(a - b) < diff; const getTransition = (transition, name) => transition === true || !!(transition && transition[name]); const safeCall = (fn, arg) => typeof fn === 'function' ? fn(arg) : fn; const internalKey = '_szhsinMenu'; @@ -935,11 +934,6 @@ const MenuList = ({ const focusRef = react.useRef(); const arrowRef = react.useRef(); const prevOpen = react.useRef(false); - const latestMenuSize = react.useRef({ - width: 0, - height: 0 - }); - const latestHandlePosition = react.useRef(() => {}); const { hoverItem, dispatch, @@ -1030,7 +1024,7 @@ const MenuList = ({ const { menuRect } = positionHelpers; - let menuHeight = menuRect.height; + const menuHeight = menuRect.height; if (!noOverflowCheck && overflow !== 'visible') { const { getTopOverflow, @@ -1050,7 +1044,6 @@ const MenuList = ({ } } if (height >= 0) { - menuHeight = height; setOverflowData({ height, overflowAmt @@ -1066,10 +1059,6 @@ const MenuList = ({ y }); setExpandedDirection(computedDirection); - latestMenuSize.current = { - width: menuRect.width, - height: menuHeight - }; }, [arrow, align, boundingBoxPadding, direction, gap, shift, position, overflow, anchorPoint, anchorRef, containerRef, boundingBoxRef, rootMenuRef, scrollNodes]); useIsomorphicLayoutEffect(() => { if (isOpen) { @@ -1077,7 +1066,6 @@ const MenuList = ({ if (prevOpen.current) forceReposSubmenu(); } prevOpen.current = isOpen; - latestHandlePosition.current = handlePosition; }, [isOpen, handlePosition, reposFlag]); useIsomorphicLayoutEffect(() => { if (overflowData && !setDownOverflow) menuRef.current.scrollTop = 0; @@ -1122,38 +1110,28 @@ const MenuList = ({ return () => parentScroll.removeEventListener('scroll', handleScroll); }, [isOpen, hasOverflow, parentScrollingRef, handlePosition]); react.useEffect(() => { - if (typeof ResizeObserver !== 'function' || reposition === 'initial') return; - const resizeObserver = new ResizeObserver(([entry]) => { - const { - borderBoxSize, - target - } = entry; - let width, height; - if (borderBoxSize) { - const { - inlineSize, - blockSize - } = borderBoxSize[0] || borderBoxSize; - width = inlineSize; - height = blockSize; + if (!isOpen || typeof ResizeObserver !== 'function' || reposition === 'initial') return; + const targetList = []; + const resizeObserver = new ResizeObserver(entries => entries.forEach(({ + target + }) => { + if (targetList.indexOf(target) < 0) { + targetList.push(target); } else { - const borderRect = getNormalizedClientRect(target); - width = borderRect.width; - height = borderRect.height; + reactDom.flushSync(() => { + handlePosition(); + forceReposSubmenu(); + }); } - if (width === 0 || height === 0) return; - if (floatEqual(width, latestMenuSize.current.width, 1) && floatEqual(height, latestMenuSize.current.height, 1)) return; - reactDom.flushSync(() => { - latestHandlePosition.current(); - forceReposSubmenu(); - }); - }); - const observeTarget = menuRef.current; - resizeObserver.observe(observeTarget, { + })); + const resizeObserverOptions = { box: 'border-box' - }); - return () => resizeObserver.unobserve(observeTarget); - }, [reposition]); + }; + resizeObserver.observe(menuRef.current, resizeObserverOptions); + const anchor = anchorRef == null ? void 0 : anchorRef.current; + anchor && resizeObserver.observe(anchor, resizeObserverOptions); + return () => resizeObserver.disconnect(); + }, [isOpen, reposition, anchorRef, handlePosition]); react.useEffect(() => { if (!isOpen) { dispatch(HoverActionTypes.RESET); diff --git a/example/package-lock.json b/example/package-lock.json index e154a935..65d5a0c3 100644 --- a/example/package-lock.json +++ b/example/package-lock.json @@ -21,7 +21,7 @@ }, "..": { "name": "@szhsin/react-menu", - "version": "4.2.0", + "version": "4.2.1", "license": "MIT", "dependencies": { "prop-types": "^15.7.2", diff --git a/example/src/utils/index.js b/example/src/utils/index.js index 94f08165..d26aeda7 100644 --- a/example/src/utils/index.js +++ b/example/src/utils/index.js @@ -1,8 +1,8 @@ import { useEffect, useLayoutEffect } from 'react'; import { useTheme } from '../store'; -export const version = '4.2.0'; -export const build = '135'; +export const version = '4.2.1'; +export const build = '136'; export const bem = (block, element, modifiers = {}) => { let blockElement = element ? `${block}__${element}` : block; diff --git a/package-lock.json b/package-lock.json index e9d7629f..b33f675b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@szhsin/react-menu", - "version": "4.2.0", + "version": "4.2.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@szhsin/react-menu", - "version": "4.2.0", + "version": "4.2.1", "license": "MIT", "dependencies": { "prop-types": "^15.7.2", diff --git a/package.json b/package.json index 06e97205..825fded4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@szhsin/react-menu", - "version": "4.2.0", + "version": "4.2.1", "description": "React component for building accessible menu, dropdown, submenu, context menu and more.", "author": "Zheng Song", "license": "MIT",