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.

Sign up for GitHub

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

WIP: Try to fix Popover obstructs blocks #43162

Closed
wants to merge 2 commits into from
Closed

WIP: Try to fix Popover obstructs blocks #43162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
acbcc99
update floating-ui + fix
ntsekouras Aug 11, 2022
3c923a9
only check `top` popovers
ntsekouras Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 24 additions & 24 deletions package-lock.json
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion packages/components/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"@emotion/serialize": "^1.0.2",
"@emotion/styled": "^11.6.0",
"@emotion/utils": "1.0.0",
"@floating-ui/react-dom": "0.6.3",
"@floating-ui/react-dom": "1.0.0",
"@use-gesture/react": "^10.2.6",
"@wordpress/a11y": "file:../a11y",
"@wordpress/compose": "file:../compose",
Expand Down
100 changes: 85 additions & 15 deletions packages/components/src/popover/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,13 @@ const placementToAnimationOrigin = ( placement ) => {
return x + ' ' + y;
};

const isTopBottomPlacement = ( placement ) =>
placement.trim().startsWith( 'top' ) ||
placement.trim().startsWith( 'bottom' );
const hasBeforePlacement = ( placement ) =>
placement.trim().startsWith( 'top' ) ||
placement.trim().startsWith( 'left' );

const Popover = (
{
range,
Expand Down Expand Up @@ -202,33 +209,55 @@ const Popover = (

const middleware = [
frameOffset || offset
? offsetMiddleware( ( { placement: currentPlacement } ) => {
? offsetMiddleware( ( middlewareArguments ) => {
if ( ! frameOffset ) {
return offset;
}

const isTopBottomPlacement =
currentPlacement.includes( 'top' ) ||
currentPlacement.includes( 'bottom' );

const {
placement: currentPlacement,
rects: {
reference: referenceRect,
floating: floatingRect,
},
} = middlewareArguments;
// The main axis should represent the gap between the
// floating element and the reference element. The cross
// axis is always perpendicular to the main axis.
const mainAxis = isTopBottomPlacement ? 'y' : 'x';
const mainAxis = isTopBottomPlacement( currentPlacement )
? 'y'
: 'x';
const crossAxis = mainAxis === 'x' ? 'y' : 'x';

// When the popover is before the reference, subtract the offset,
// of the main axis else add it.
const hasBeforePlacement =
currentPlacement.includes( 'top' ) ||
currentPlacement.includes( 'left' );
const mainAxisModifier = hasBeforePlacement ? -1 : 1;
const mainAxisModifier = hasBeforePlacement(
currentPlacement
)
? -1
: 1;
const normalizedOffset = offset ? offset : 0;

let totalOffset =
frameOffset[ mainAxis ] + normalizedOffset;
const mainDimension = mainAxis === 'y' ? 'height' : 'width';
if (
// TODO: does this make sense only for top positioned popovers?
currentPlacement.startsWith( 'top' ) &&
// If the reference has no height we don't need to do anything.
referenceRect[ mainDimension ] &&
referenceRect[ mainAxis ] < 0 &&
ownerDocument.documentElement.scrollTop <=
Copy link
Contributor

Choose a reason for hiding this comment

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

Just flagging that reading the scrollTop property could cause reflow / layout trashing, which could affect the page's performance (especially since this middleware is computed as the user scrolls and interacts with the page)

referenceRect[ mainAxis ] +
normalizedOffset +
frameOffset[ mainAxis ]
) {
totalOffset +=
referenceRect[ mainDimension ] +
floatingRect[ mainDimension ];
}

return {
mainAxis:
normalizedOffset +
frameOffset[ mainAxis ] * mainAxisModifier,
mainAxis: mainAxisModifier * totalOffset,
crossAxis: frameOffset[ crossAxis ],
};
} )
Expand All @@ -250,7 +279,48 @@ const Popover = (
__unstableShift
? shift( {
crossAxis: true,
limiter: limitShift(),
limiter: limitShift( {
offset: ( middlewareArguments ) => {
// The following calculations are aimed at allowing the floating
// element to shift fully below the reference element, when the
// reference element is in a different document (i.e. an iFrame).
if ( ownerDocument === document ) {
return 0;
}
const {
placement: currentPlacement,
rects: { reference: referenceRect },
} = middlewareArguments;

// The main axis (according to floating UI's docs) is the "x" axis
// for 'top' and 'bottom' placements, and the "y" axis for 'left'
// and 'right' placements.
const mainAxis = isTopBottomPlacement(
currentPlacement
)
? 'x'
: 'y';
const crossAxis = mainAxis === 'x' ? 'y' : 'x';

const crossAxisModifier = hasBeforePlacement(
currentPlacement
)
? -1
: 1;

return {
mainAxis: -Math.min(
frameOffset[ mainAxis ],
middlewareArguments[ mainAxis ] -
referenceRect[ mainAxis ] -
frameOffset[ mainAxis ]
),
crossAxis:
crossAxisModifier *
frameOffset[ crossAxis ],
};
},
} ),
padding: 1, // Necessary to avoid flickering at the edge of the viewport.
} )
: undefined,
Expand Down