Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popover: update @floating-ui to latest version, remove custom fix for iframe positioning and scaling #46845

Merged
merged 9 commits into from
Aug 30, 2023
44 changes: 22 additions & 22 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ function BlockPopoverInbetween( {
return undefined;
}

const { ownerDocument } = previousElement || nextElement;
const contextElement = previousElement || nextElement;

return {
ownerDocument,
contextElement,
getBoundingClientRect() {
const previousRect = previousElement
? previousElement.getBoundingClientRect()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function BlockPopover(

return new window.DOMRect( left, top, width, height );
},
ownerDocument: selectedElement.ownerDocument,
contextElement: selectedElement,
};
}, [
bottomClientId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,8 @@ export default function ListViewDropIndicator( {
return undefined;
}

const ownerDocument = targetElement.ownerDocument;

return {
ownerDocument,
contextElement: targetElement,
getBoundingClientRect() {
const rect = targetElement.getBoundingClientRect();
const indent = getDropIndicatorIndent( rect );
Expand All @@ -189,9 +187,10 @@ export default function ListViewDropIndicator( {
'horizontal'
);

const doc = targetElement.ownerDocument;
const windowScroll =
scrollContainer === ownerDocument.body ||
scrollContainer === ownerDocument.documentElement;
scrollContainer === doc.body ||
scrollContainer === doc.documentElement;

// If the scroll container is not the window, offset the left position, if need be.
if ( scrollContainer && ! windowScroll ) {
Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

- `Shortcut`: Add Storybook stories ([#53627](https://github.com/WordPress/gutenberg/pull/53627)).
- `SlotFill`: Do not render children when using `<Slot bubblesVirtually />`. ([#53272](https://github.com/WordPress/gutenberg/pull/53272))
- Update `@floating-ui/react-dom` to the latest version ([#46845](https://github.com/WordPress/gutenberg/pull/46845)).

## 25.6.0 (2023-08-16)

Expand Down
2 changes: 1 addition & 1 deletion packages/components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"@emotion/serialize": "^1.0.2",
"@emotion/styled": "^11.6.0",
"@emotion/utils": "^1.0.0",
"@floating-ui/react-dom": "1.0.0",
"@floating-ui/react-dom": "^2.0.1",
"@radix-ui/react-dropdown-menu": "2.0.4",
"@use-gesture/react": "^10.2.24",
"@wordpress/a11y": "file:../a11y",
Expand Down
140 changes: 40 additions & 100 deletions packages/components/src/popover/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
*/
import type { ForwardedRef, SyntheticEvent, RefCallback } from 'react';
import classnames from 'classnames';
import type { Middleware, MiddlewareArguments } from '@floating-ui/react-dom';
import {
useFloating,
flip as flipMiddleware,
shift as shiftMiddleware,
limitShift,
autoUpdate,
arrow,
offset as offsetMiddleware,
Expand Down Expand Up @@ -50,8 +50,6 @@ import ScrollLock from '../scroll-lock';
import { Slot, Fill, useSlot } from '../slot-fill';
import {
computePopoverPosition,
getFrameOffset,
getFrameScale,
positionToPlacement,
placementToMotionAnimationProps,
getReferenceOwnerDocument,
Expand All @@ -64,7 +62,6 @@ import type {
PopoverAnchorRefReference,
PopoverAnchorRefTopBottom,
} from './types';
import { limitShift as customLimitShift } from './limit-shift';
import { overlayMiddlewares } from './overlay-middlewares';

/**
Expand Down Expand Up @@ -262,69 +259,34 @@ const UnforwardedPopover = (
? positionToPlacement( position )
: placementProp;

/**
* Offsets the position of the popover when the anchor is inside an iframe.
*
* Store the offset in a ref, due to constraints with floating-ui:
* https://floating-ui.com/docs/react-dom#variables-inside-middleware-functions.
*/
const frameOffsetRef = useRef( getFrameOffset( referenceOwnerDocument ) );

const middleware = [
...( placementProp === 'overlay' ? overlayMiddlewares() : [] ),
// Custom middleware which adjusts the popover's position by taking into
// account the offset of the anchor's iframe (if any) compared to the page.
{
name: 'frameOffset',
fn( { x, y }: MiddlewareArguments ) {
if ( ! frameOffsetRef.current ) {
return {
x,
y,
};
}

return {
x: x + frameOffsetRef.current.x,
y: y + frameOffsetRef.current.y,
data: {
// This will be used in the customLimitShift() function.
amount: frameOffsetRef.current,
},
};
},
},
offsetMiddleware( offsetProp ),
computedFlipProp ? flipMiddleware() : undefined,
computedResizeProp
? size( {
apply( sizeProps ) {
const { firstElementChild } =
refs.floating.current ?? {};

// Only HTMLElement instances have the `style` property.
if ( ! ( firstElementChild instanceof HTMLElement ) )
return;

// Reduce the height of the popover to the available space.
Object.assign( firstElementChild.style, {
maxHeight: `${ sizeProps.availableHeight }px`,
overflow: 'auto',
} );
},
} )
: undefined,
shift
? shiftMiddleware( {
crossAxis: true,
limiter: customLimitShift(),
padding: 1, // Necessary to avoid flickering at the edge of the viewport.
} )
: undefined,
computedFlipProp && flipMiddleware(),
computedResizeProp &&
size( {
apply( sizeProps ) {
const { firstElementChild } = refs.floating.current ?? {};

// Only HTMLElement instances have the `style` property.
if ( ! ( firstElementChild instanceof HTMLElement ) )
return;

// Reduce the height of the popover to the available space.
Object.assign( firstElementChild.style, {
maxHeight: `${ sizeProps.availableHeight }px`,
overflow: 'auto',
} );
},
} ),
shift &&
shiftMiddleware( {
crossAxis: true,
limiter: limitShift(),
padding: 1, // Necessary to avoid flickering at the edge of the viewport.
} ),
arrow( { element: arrowRef } ),
].filter(
( m: Middleware | undefined ): m is Middleware => m !== undefined
);
];
const slotName = useContext( slotNameContext ) || __unstableSlotName;
const slot = useSlot( slotName );

Expand Down Expand Up @@ -353,10 +315,6 @@ const UnforwardedPopover = (
// Positioning coordinates
x,
y,
// Callback refs (not regular refs). This allows the position to be updated.
// when either elements change.
reference: referenceCallbackRef,
floating,
// Object with "regular" refs to both "reference" and "floating"
refs,
// Type of CSS position property to use (absolute or fixed)
Expand All @@ -372,6 +330,7 @@ const UnforwardedPopover = (
middleware,
whileElementsMounted: ( referenceParam, floatingParam, updateParam ) =>
autoUpdate( referenceParam, floatingParam, updateParam, {
layoutShift: false,
animationFrame: true,
} ),
} );
Expand Down Expand Up @@ -406,17 +365,16 @@ const UnforwardedPopover = (
fallbackReferenceElement,
fallbackDocument: document,
} );
const scale = getFrameScale( resultingReferenceOwnerDoc );

const resultingReferenceElement = getReferenceElement( {
anchor,
anchorRef,
anchorRect,
getAnchorRect,
fallbackReferenceElement,
scale,
} );

referenceCallbackRef( resultingReferenceElement );
refs.setReference( resultingReferenceElement );
tyxla marked this conversation as resolved.
Show resolved Hide resolved

setReferenceOwnerDocument( resultingReferenceOwnerDoc );
}, [
Expand All @@ -429,23 +387,17 @@ const UnforwardedPopover = (
anchorRect,
getAnchorRect,
fallbackReferenceElement,
referenceCallbackRef,
refs,
] );

// 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
// document scrolls. Also update the frame offset if the view resizes.
// document scrolls.
useLayoutEffect( () => {
if (
// Reference and root documents are the same.
referenceOwnerDocument === document ||
// Reference and floating are in the same document.
referenceOwnerDocument === refs.floating.current?.ownerDocument ||
// The reference's document has no view (i.e. window)
// or frame element (ie. it's not an iframe).
! referenceOwnerDocument?.defaultView?.frameElement
! referenceOwnerDocument ||
! referenceOwnerDocument.defaultView
) {
frameOffsetRef.current = undefined;
return;
}

Expand All @@ -456,23 +408,17 @@ const UnforwardedPopover = (
? getScrollContainer( frameElement )
: null;

const updateFrameOffset = () => {
frameOffsetRef.current = getFrameOffset( referenceOwnerDocument );
update();
};
defaultView.addEventListener( 'resize', updateFrameOffset );
scrollContainer?.addEventListener( 'scroll', updateFrameOffset );

updateFrameOffset();
defaultView.addEventListener( 'resize', update );
scrollContainer?.addEventListener( 'scroll', update );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why we need to attach our own listeners to the iframe's scroll container is described in floating-ui/floating-ui#2518. Once that's fixed, Floating UI will attach them automatically for us, will watch for scroll position changes for us, and our workaround code can be removed.


return () => {
defaultView.removeEventListener( 'resize', updateFrameOffset );
scrollContainer?.removeEventListener( 'scroll', updateFrameOffset );
defaultView.removeEventListener( 'resize', update );
scrollContainer?.removeEventListener( 'scroll', update );
};
}, [ referenceOwnerDocument, update, refs.floating ] );
}, [ referenceOwnerDocument, update ] );

const mergedFloatingRef = useMergeRefs( [
floating,
refs.setFloating,
dialogRef,
forwardedRef,
] );
Expand Down Expand Up @@ -543,18 +489,12 @@ const UnforwardedPopover = (
left:
typeof arrowData?.x !== 'undefined' &&
Number.isFinite( arrowData.x )
? `${
arrowData.x +
( frameOffsetRef.current?.x ?? 0 )
}px`
? `${ arrowData.x }px`
: '',
top:
typeof arrowData?.y !== 'undefined' &&
Number.isFinite( arrowData.y )
? `${
arrowData.y +
( frameOffsetRef.current?.y ?? 0 )
}px`
? `${ arrowData.y }px`
: '',
} }
>
Expand Down
Loading
Loading