Skip to content

Commit

Permalink
Block toolbar: restrict visible child calculation to known blocks (#6…
Browse files Browse the repository at this point in the history
…6702)

* Refactor getVisibleElementBounds to only check visible children for specific blocks that we know overflow. Rename function and update comments. Introduce rudimentary unit tests.

* Apply suggestions to code comments from code review

Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>

* Add extra tests

* Rebase

* Added a test for viewport clipping and adjusted test descriptions

---------

Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: ndiego <ndiego@git.wordpress.org>
Co-authored-by: getdave <get_dave@git.wordpress.org>
Co-authored-by: stokesman <presstoke@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
Co-authored-by: simom <simo_m@git.wordpress.org>
  • Loading branch information
9 people authored Nov 4, 2024
1 parent 7bf1d96 commit 6df457c
Show file tree
Hide file tree
Showing 4 changed files with 256 additions and 26 deletions.
8 changes: 4 additions & 4 deletions packages/block-editor/src/components/block-popover/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
*/
import { useBlockElement } from '../block-list/use-block-props/use-block-refs';
import usePopoverScroll from './use-popover-scroll';
import { rectUnion, getVisibleElementBounds } from '../../utils/dom';
import { rectUnion, getElementBounds } from '../../utils/dom';

const MAX_POPOVER_RECOMPUTE_COUNTER = Number.MAX_SAFE_INTEGER;

Expand Down Expand Up @@ -90,10 +90,10 @@ function BlockPopover(
getBoundingClientRect() {
return lastSelectedElement
? rectUnion(
getVisibleElementBounds( selectedElement ),
getVisibleElementBounds( lastSelectedElement )
getElementBounds( selectedElement ),
getElementBounds( lastSelectedElement )
)
: getVisibleElementBounds( selectedElement );
: getElementBounds( selectedElement );
},
contextElement: selectedElement,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
import { store as blockEditorStore } from '../../store';
import { useBlockElement } from '../block-list/use-block-props/use-block-refs';
import { hasStickyOrFixedPositionValue } from '../../hooks/position';
import { getVisibleElementBounds } from '../../utils/dom';
import { getElementBounds } from '../../utils/dom';

const COMMON_PROPS = {
placement: 'top-start',
Expand Down Expand Up @@ -68,7 +68,7 @@ function getProps(
// Get how far the content area has been scrolled.
const scrollTop = scrollContainer?.scrollTop || 0;

const blockRect = getVisibleElementBounds( selectedBlockElement );
const blockRect = getElementBounds( selectedBlockElement );
const contentRect = contentElement.getBoundingClientRect();

// Get the vertical position of top of the visible content area.
Expand Down
46 changes: 26 additions & 20 deletions packages/block-editor/src/utils/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,41 +134,47 @@ function isScrollable( element ) {
);
}

export const WITH_OVERFLOW_ELEMENT_BLOCKS = [ 'core/navigation' ];
/**
* Returns the rect of the element including all visible nested elements.
*
* Visible nested elements, including elements that overflow the parent, are
* taken into account.
*
* This function is useful for calculating the visible area of a block that
* contains nested elements that overflow the block, e.g. the Navigation block,
* which can contain overflowing Submenu blocks.
* Returns the bounding rectangle of an element, with special handling for blocks
* that have visible overflowing children (defined in WITH_OVERFLOW_ELEMENT_BLOCKS).
*
* For blocks like Navigation that can have overflowing elements (e.g. submenus),
* this function calculates the combined bounds of both the parent and its visible
* children. The returned rect may extend beyond the viewport.
* The returned rect represents the full extent of the element and its visible
* children, which may extend beyond the viewport.
*
* @param {Element} element Element.
* @return {DOMRect} Bounding client rect of the element and its visible children.
*/
export function getVisibleElementBounds( element ) {
export function getElementBounds( element ) {
const viewport = element.ownerDocument.defaultView;

if ( ! viewport ) {
return new window.DOMRectReadOnly();
}

let bounds = element.getBoundingClientRect();
const stack = [ element ];
let currentElement;

while ( ( currentElement = stack.pop() ) ) {
// Children won’t affect bounds unless the element is not scrollable.
if ( ! isScrollable( currentElement ) ) {
for ( const child of currentElement.children ) {
if ( isElementVisible( child ) ) {
const childBounds = child.getBoundingClientRect();
bounds = rectUnion( bounds, childBounds );
stack.push( child );
const dataType = element.getAttribute( 'data-type' );

/*
* For blocks with overflowing elements (like Navigation), include the bounds
* of visible children that extend beyond the parent container.
*/
if ( dataType && WITH_OVERFLOW_ELEMENT_BLOCKS.includes( dataType ) ) {
const stack = [ element ];
let currentElement;

while ( ( currentElement = stack.pop() ) ) {
// Children won’t affect bounds unless the element is not scrollable.
if ( ! isScrollable( currentElement ) ) {
for ( const child of currentElement.children ) {
if ( isElementVisible( child ) ) {
const childBounds = child.getBoundingClientRect();
bounds = rectUnion( bounds, childBounds );
stack.push( child );
}
}
}
}
Expand Down
224 changes: 224 additions & 0 deletions packages/block-editor/src/utils/test/dom.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
/**
* Internal dependencies
*/
import { getElementBounds, WITH_OVERFLOW_ELEMENT_BLOCKS } from '../dom';
describe( 'dom', () => {
describe( 'getElementBounds', () => {
it( 'should return a DOMRectReadOnly object if the viewport is not available', () => {
const element = {
ownerDocument: {
defaultView: null,
},
};
expect( getElementBounds( element ) ).toEqual(
new window.DOMRectReadOnly()
);
} );
it( 'should return a DOMRectReadOnly object if the viewport is available', () => {
const element = {
ownerDocument: {
defaultView: {
getComputedStyle: () => ( {
display: 'block',
visibility: 'visible',
opacity: '1',
} ),
},
},
getBoundingClientRect: () => ( {
left: 0,
top: 0,
right: 100,
bottom: 100,
width: 100,
height: 100,
} ),
getAttribute: ( x ) => x,
};
expect( getElementBounds( element ) ).toEqual(
new window.DOMRectReadOnly( 0, 0, 100, 100 )
);
} );
it( 'should clip left and right values when an element is larger than the viewport width', () => {
const element = window.document.createElement( 'div' );
element.getBoundingClientRect = jest.fn().mockReturnValue( {
left: -10,
top: 0,
right: window.innerWidth + 10,
bottom: 100,
width: window.innerWidth,
height: 100,
} );
expect( getElementBounds( element ).toJSON() ).toEqual( {
left: 0, // Reset to min left bound.
top: 0,
right: window.innerWidth, // Reset to max right bound.
bottom: 100,
width: window.innerWidth,
height: 100,
x: 0,
y: 0,
} );
} );
it( 'should return the parent DOMRectReadOnly object if the parent block type is not supported', () => {
const element = window.document.createElement( 'div' );
element.getBoundingClientRect = jest.fn().mockReturnValue( {
left: 0,
top: 0,
right: 100,
bottom: 100,
width: 100,
height: 100,
} );
element.setAttribute( 'data-type', 'test' );
const childElement = window.document.createElement( 'div' );
childElement.getBoundingClientRect = jest.fn().mockReturnValue( {
left: 0,
top: 0,
right: 333,
bottom: 333,
width: 333,
height: 333,
x: 0,
y: 0,
} );
element.appendChild( childElement );

expect( getElementBounds( element ).toJSON() ).toEqual( {
left: 0,
top: 0,
right: 100,
bottom: 100,
width: 100,
height: 100,
x: 0,
y: 0,
} );
} );
describe( 'With known block type', () => {
it( 'should return the child DOMRectReadOnly object if it is visible and a known block type', () => {
const element = window.document.createElement( 'div' );
element.getBoundingClientRect = jest.fn().mockReturnValue( {
left: 0,
top: 0,
right: 100,
bottom: 100,
width: 100,
height: 100,
} );
element.setAttribute(
'data-type',
WITH_OVERFLOW_ELEMENT_BLOCKS[ 0 ]
);
const childElement = window.document.createElement( 'div' );
childElement.getBoundingClientRect = jest
.fn()
.mockReturnValue( {
left: 0,
top: 0,
right: 333,
bottom: 333,
width: 333,
height: 333,
x: 0,
y: 0,
} );
element.appendChild( childElement );

expect( getElementBounds( element ).toJSON() ).toEqual( {
left: 0,
top: 0,
right: 333,
bottom: 333,
width: 333,
height: 333,
x: 0,
y: 0,
} );
} );
it( 'should return the parent DOMRectReadOnly if the child is scrollable', () => {
const element = window.document.createElement( 'div' );
element.setAttribute(
'data-type',
WITH_OVERFLOW_ELEMENT_BLOCKS[ 0 ]
);
element.style.overflowX = 'auto';
element.style.overflowY = 'auto';
element.getBoundingClientRect = jest.fn().mockReturnValue( {
left: 0,
top: 0,
right: 100,
bottom: 100,
width: 100,
height: 100,
} );
const childElement = window.document.createElement( 'div' );
childElement.getBoundingClientRect = jest
.fn()
.mockReturnValue( {
left: 0,
top: 0,
right: 333,
bottom: 333,
width: 333,
height: 333,
x: 0,
y: 0,
} );
element.appendChild( childElement );

expect( getElementBounds( element ).toJSON() ).toEqual( {
left: 0,
top: 0,
right: 100,
bottom: 100,
width: 100,
height: 100,
x: 0,
y: 0,
} );
} );
it( 'should return the parent DOMRectReadOnly object if the child element is not visible', () => {
const element = window.document.createElement( 'div' );
element.getBoundingClientRect = jest.fn().mockReturnValue( {
left: 0,
top: 0,
right: 100,
bottom: 100,
width: 100,
height: 100,
} );
element.setAttribute(
'data-type',
WITH_OVERFLOW_ELEMENT_BLOCKS[ 0 ]
);
const childElement = window.document.createElement( 'div' );
childElement.getBoundingClientRect = jest
.fn()
.mockReturnValue( {
left: 0,
top: 0,
right: 333,
bottom: 333,
width: 333,
height: 333,
x: 0,
y: 0,
} );
childElement.style.display = 'none';
element.appendChild( childElement );

expect( getElementBounds( element ).toJSON() ).toEqual( {
left: 0,
top: 0,
right: 100,
bottom: 100,
width: 100,
height: 100,
x: 0,
y: 0,
} );
} );
} );
} );
} );

1 comment on commit 6df457c

@github-actions
Copy link

Choose a reason for hiding this comment

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

Flaky tests detected in 6df457c.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11673198764
📝 Reported issues:

Please sign in to comment.