diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index e6bba25be6d8cd..3d0002b5554054 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -9,6 +9,7 @@ ### Bug Fix - `Popover`: fix and improve opening animation ([#43186](https://github.com/WordPress/gutenberg/pull/43186)). +- `Popover`: fix incorrect deps in hooks resulting in incorrect positioning after calling `update` ([#43267](https://github.com/WordPress/gutenberg/pull/43267/)). ### Enhancements diff --git a/packages/components/src/popover/index.js b/packages/components/src/popover/index.js index b530f2e51d4c61..6716e4cd92dfc5 100644 --- a/packages/components/src/popover/index.js +++ b/packages/components/src/popover/index.js @@ -42,7 +42,11 @@ import { Path, SVG } from '@wordpress/primitives'; import Button from '../button'; import ScrollLock from '../scroll-lock'; import { Slot, Fill, useSlot } from '../slot-fill'; -import { positionToPlacement, placementToMotionAnimationProps } from './utils'; +import { + getFrameOffset, + positionToPlacement, + placementToMotionAnimationProps, +} from './utils'; /** * Name of slot in which popover should fill. @@ -183,6 +187,10 @@ const Popover = ( } return documentToReturn ?? document; + + // 'reference' and 'refs.floating' are refs and don't need to be listed + // as dependencies (see https://github.com/WordPress/gutenberg/pull/41612) + // eslint-disable-next-line react-hooks/exhaustive-deps }, [ anchorRef, anchorRect, getAnchorRect ] ); /** @@ -191,7 +199,7 @@ const Popover = ( * Store the offset in a ref, due to constraints with floating-ui: * https://floating-ui.com/docs/react-dom#variables-inside-middleware-functions. */ - const frameOffset = useRef(); + const frameOffset = useRef( getFrameOffset( ownerDocument ) ); const middleware = [ frameOffset.current || offset @@ -364,7 +372,7 @@ const Popover = ( refs.floating.current, update ); - }, [ anchorRef, anchorRect, getAnchorRect ] ); + }, [ anchorRef, anchorRect, getAnchorRect, update ] ); // This is only needed for a smooth transition when moving blocks. useLayoutEffect( () => { @@ -377,7 +385,7 @@ const Popover = ( return () => { observer.disconnect(); }; - }, [ __unstableObserveElement ] ); + }, [ __unstableObserveElement, update ] ); // If the reference element is in a different ownerDocument (e.g. iFrame), // we need to manually update the floating's position as the reference's owner @@ -388,15 +396,15 @@ const Popover = ( } const { defaultView } = ownerDocument; - const { frameElement } = defaultView; ownerDocument.addEventListener( 'scroll', update ); let updateFrameOffset; - if ( frameElement ) { + const hasFrameElement = !! ownerDocument?.defaultView?.frameElement; + if ( hasFrameElement ) { updateFrameOffset = () => { - const iframeRect = frameElement.getBoundingClientRect(); - frameOffset.current = { x: iframeRect.left, y: iframeRect.top }; + frameOffset.current = getFrameOffset( ownerDocument ); + update(); }; updateFrameOffset(); defaultView.addEventListener( 'resize', updateFrameOffset ); @@ -409,7 +417,7 @@ const Popover = ( defaultView.removeEventListener( 'resize', updateFrameOffset ); } }; - }, [ ownerDocument ] ); + }, [ ownerDocument, update ] ); const mergedFloatingRef = useMergeRefs( [ floating, diff --git a/packages/components/src/popover/utils.js b/packages/components/src/popover/utils.js index 05bf818aa969d1..90cd3d84d047cd 100644 --- a/packages/components/src/popover/utils.js +++ b/packages/components/src/popover/utils.js @@ -81,3 +81,27 @@ export const placementToMotionAnimationProps = ( placement ) => { transition: { duration: 0.1, ease: [ 0, 0, 0.2, 1 ] }, }; }; + +/** + * @typedef FrameOffset + * @type {Object} + * @property {number} x A numerical value representing the horizontal offset of the frame. + * @property {number} y A numerical value representing the vertical offset of the frame. + */ + +/** + * Returns the offset of a document's frame element. + * + * @param {Document} document A document. This will usually be the document within an iframe. + * + * @return {FrameOffset|undefined} The offset of the document's frame element, + * or undefined if the document has no frame element. + */ +export const getFrameOffset = ( document ) => { + const frameElement = document?.defaultView?.frameElement; + if ( ! frameElement ) { + return; + } + const iframeRect = frameElement.getBoundingClientRect(); + return { x: iframeRect.left, y: iframeRect.top }; +};