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

Jump to bottom

Block Editor: Improve act() usage in UrlPopover tests #46230

Closed
wants to merge 1 commit into from
Closed

Block Editor: Improve act() usage in UrlPopover tests #46230

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 21 additions & 5 deletions packages/block-editor/src/components/url-popover/test/index.js
View file
Open in desktop
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { act, render, screen } from '@testing-library/react';
import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

/**
Expand All @@ -11,6 +11,16 @@ import URLPopover from '../';

jest.useRealTimers();

/**
* Returns the first found popover element up the DOM tree.
*
* @param {HTMLElement} element Element to start with.
* @return {HTMLElement|null} Popover element, or `null` if not found.
*/
function getWrappingPopoverElement( element ) {
return element.closest( '.components-popover' );
}

describe( 'URLPopover', () => {
it( 'matches the snapshot in its default state', async () => {
const { container } = render(
Expand All @@ -22,8 +32,11 @@ describe( 'URLPopover', () => {
</URLPopover>
);

// wait for `Popover` effects to finish
await act( () => Promise.resolve() );
await waitFor( () =>
expect(
getWrappingPopoverElement( screen.getByText( 'Editor' ) )
).toBePositionedPopover()
);
Copy link
Member

Choose a reason for hiding this comment

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

One interesting thing here is that the test doesn't pass an anchor prop to the URLPopover, so there should be no positioning.

However, the Popover component has a fallback for that case, where it will use the popover element's parent as anchor. @ciampo Are there cases where the fallback is actually used and does something useful? I can imagine using a popover inside a paragraph for example:

<p>Lorem ipsum <span>dolor<Popover>latin for pain</Popover></span> sit amet<p>

and the it will be permanently attached to the span with word "dolor"? Is that what the fallback element is for?

If we didn't do any fallback for anchor, then it would remain null and floating-ui wouldn't do any positioning at all. The popover would have a position: absolute style and that's all.


expect( container ).toMatchSnapshot();
} );
Expand Down Expand Up @@ -53,8 +66,11 @@ describe( 'URLPopover', () => {
</URLPopover>
);

// wait for `Popover` effects to finish
await act( () => Promise.resolve() );
await waitFor( () =>
expect(
getWrappingPopoverElement( screen.getByText( 'Editor' ) )
).toBePositionedPopover()
);

expect( container ).toMatchSnapshot();
} );
Expand Down