Skip to content

Commit

Permalink
Block Toolbar & Popover component - Prevent sticky position from caus…
Browse files Browse the repository at this point in the history 
…ing permanently obscured areas of the selected block. (#33981)

* try using next block as anchor

* place popover below block if available when sticky

* if starts at bottom of block, keep there

* fix edge cases

* add bottom sticky position

* fix jumpy bug on using toolbar

* revert to default behavior as soon as there is room.  no bottom sticky necessary

* refactor logic and comment

* remove unnecessary else

* gross selectors

* pass content wrapper through to popover as prop

* minor refactor

* use bottom sticky if necessary

* update names, comments, JSDocs

* add basic store tests

* remove store goo

* use the contentRef

* Update packages/block-editor/src/components/block-tools/block-popover.js
  • Loading branch information
@Addison-Stavlo
Addison-Stavlo authored Aug 25, 2021
1 parent 599b60d commit 60d907b
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 35 deletions.
3 changes: 3 additions & 0 deletions packages/block-editor/src/components/block-tools/block-popover.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@ function BlockPopover( {
// Observe movement for block animations (especially horizontal).
__unstableObserveElement={ node }
shouldAnchorIncludePadding
// Used to safeguard sticky position behavior against cases where it would permanently
// obscure specific sections of a block.
__unstableEditorCanvasWrapper={ __unstableContentRef?.current }
>
{ ( shouldShowContextualToolbar || isToolbarForced ) && (
<div
Expand Down
4 changes: 3 additions & 1 deletion packages/components/src/popover/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ const Popover = (
__unstableBoundaryParent,
__unstableForcePosition,
__unstableForceXAlignment,
__unstableEditorCanvasWrapper,
/* eslint-enable no-unused-vars */
...contentProps
},
Expand Down Expand Up @@ -352,7 +353,8 @@ const Popover = (
relativeOffsetTop,
boundaryElement,
__unstableForcePosition,
__unstableForceXAlignment
__unstableForceXAlignment,
__unstableEditorCanvasWrapper
);

if (
Expand Down
99 changes: 65 additions & 34 deletions packages/components/src/popover/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,18 +159,18 @@ export function computePopoverXAxisPosition(
/**
* Utility used to compute the popover position over the yAxis
*
* @param {Object} anchorRect Anchor Rect.
* @param {Object} contentSize Content Size.
* @param {string} yAxis Desired yAxis.
* @param {string} corner Desired corner.
* @param {boolean} stickyBoundaryElement The boundary element to use when
* switching between sticky and normal
* position.
* @param {Element} anchorRef The anchor element.
* @param {Element} relativeOffsetTop If applicable, top offset of the
* relative positioned parent container.
* @param {boolean} forcePosition Don't adjust position based on anchor.
*
* @param {Object} anchorRect Anchor Rect.
* @param {Object} contentSize Content Size.
* @param {string} yAxis Desired yAxis.
* @param {string} corner Desired corner.
* @param {boolean} stickyBoundaryElement The boundary element to use when switching between sticky
* and normal position.
* @param {Element} anchorRef The anchor element.
* @param {Element} relativeOffsetTop If applicable, top offset of the relative positioned
* parent container.
* @param {boolean} forcePosition Don't adjust position based on anchor.
* @param {Element|null} editorWrapper Element that wraps the editor content. Used to access
* scroll position to determine sticky behavior.
* @return {Object} Popover xAxis position and constraints.
*/
export function computePopoverYAxisPosition(
Expand All @@ -181,18 +181,47 @@ export function computePopoverYAxisPosition(
stickyBoundaryElement,
anchorRef,
relativeOffsetTop,
forcePosition
forcePosition,
editorWrapper
) {
const { height } = contentSize;

if ( stickyBoundaryElement ) {
const stickyRect = stickyBoundaryElement.getBoundingClientRect();
const stickyPosition = stickyRect.top + height - relativeOffsetTop;

if ( anchorRect.top <= stickyPosition ) {
const stickyPositionTop = stickyRect.top + height - relativeOffsetTop;
const stickyPositionBottom =
stickyRect.bottom - height - relativeOffsetTop;

if ( anchorRect.top <= stickyPositionTop ) {
if ( editorWrapper ) {
// If a popover cannot be positioned above the anchor, even after scrolling, we must
// ensure we use the bottom position instead of the popover slot. This prevents the
// popover from always restricting block content and interaction while selected if the
// block is near the top of the site editor.

const isRoomAboveInCanvas =
height + HEIGHT_OFFSET <
editorWrapper.scrollTop + anchorRect.top;
if ( ! isRoomAboveInCanvas ) {
return {
yAxis: 'bottom',
// If the bottom of the block is also below the bottom sticky position (ex -
// block is also taller than the editor window), return the bottom sticky
// position instead. We do this instead of the top sticky position both to
// allow a smooth transition and more importantly to ensure every section of
// the block can be free from popover obscuration at some point in the
// scroll position.
popoverTop: Math.min(
anchorRect.bottom,
stickyPositionBottom
),
};
}
}
// Default sticky behavior.
return {
yAxis,
popoverTop: Math.min( anchorRect.bottom, stickyPosition ),
popoverTop: Math.min( anchorRect.bottom, stickyPositionTop ),
};
}
}
Expand Down Expand Up @@ -274,22 +303,22 @@ export function computePopoverYAxisPosition(
}

/**
* Utility used to compute the popover position and the content max width/height
* for a popover given its anchor rect and its content size.
*
* @param {Object} anchorRect Anchor Rect.
* @param {Object} contentSize Content Size.
* @param {string} position Position.
* @param {boolean} stickyBoundaryElement The boundary element to use when
* switching between sticky and normal
* position.
* @param {Element} anchorRef The anchor element.
* @param {number} relativeOffsetTop If applicable, top offset of the
* relative positioned parent container.
* @param {Element} boundaryElement Boundary element.
* @param {boolean} forcePosition Don't adjust position based on anchor.
* @param {boolean} forceXAlignment Don't adjust alignment based on YAxis
* Utility used to compute the popover position and the content max width/height for a popover given
* its anchor rect and its content size.
*
* @param {Object} anchorRect Anchor Rect.
* @param {Object} contentSize Content Size.
* @param {string} position Position.
* @param {boolean} stickyBoundaryElement The boundary element to use when switching between
* sticky and normal position.
* @param {Element} anchorRef The anchor element.
* @param {number} relativeOffsetTop If applicable, top offset of the relative positioned
* parent container.
* @param {Element} boundaryElement Boundary element.
* @param {boolean} forcePosition Don't adjust position based on anchor.
* @param {boolean} forceXAlignment Don't adjust alignment based on YAxis
* @param {Element|null} editorWrapper Element that wraps the editor content. Used to access
* scroll position to determine sticky behavior.
* @return {Object} Popover position and constraints.
*/
export function computePopoverPosition(
Expand All @@ -301,7 +330,8 @@ export function computePopoverPosition(
relativeOffsetTop,
boundaryElement,
forcePosition,
forceXAlignment
forceXAlignment,
editorWrapper
) {
const [ yAxis, xAxis = 'center', corner ] = position.split( ' ' );

Expand All @@ -313,7 +343,8 @@ export function computePopoverPosition(
stickyBoundaryElement,
anchorRef,
relativeOffsetTop,
forcePosition
forcePosition,
editorWrapper
);
const xAxisPosition = computePopoverXAxisPosition(
anchorRect,
Expand Down

0 comments on commit 60d907b

Please sign in to comment.