Skip to content

Have createNewPost wait for editor canvas contents #51824

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

Merged
Mamaduka merged 10 commits into from
Jul 5, 2023
Merged

Have createNewPost wait for editor canvas contents #51824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
120768f
Wait for editor canvas contents
stokesman Jun 23, 2023
64eacbd
Update e2e block-api test/plugin to block api v3
stokesman Jun 23, 2023
f13a46d
Update e2e deprecated-node-matcher to block api v3
stokesman Jun 23, 2023
14e8f00
Update e2e inner-blocks-allowed-blocks to block api v3
stokesman Jun 24, 2023
0af525c
Update e2e inner-blocks-templates to block api v3
stokesman Jun 24, 2023
9b59eca
Allow awaiting non-iframed canvas in `createNewPost`
stokesman Jun 25, 2023
120a080
Update e2e wp-editor-meta-box test
stokesman Jun 25, 2023
390a321
Generalize the locator for the iframed editor canvas
stokesman Jun 30, 2023
3bb740f
Use block props hook in all blocks of inner-blocks-templates plugin
stokesman Jul 5, 2023
c6925b2
Update container-blocks test for iframed canvas
stokesman Jul 5, 2023
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
12 changes: 11 additions & 1 deletion packages/e2e-test-utils-playwright/src/admin/create-new-post.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ import { addQueryArgs } from '@wordpress/url';
* @param {string} [object.content] Content of the new post.
* @param {string} [object.excerpt] Excerpt of the new post.
* @param {boolean} [object.showWelcomeGuide] Whether to show the welcome guide.
* @param {boolean} [object.legacyCanvas] Whether the non-iframed editor canvas is awaited.
*/
export async function createNewPost( {
postType,
title,
content,
excerpt,
showWelcomeGuide = false,
legacyCanvas = false,
} = {} ) {
const query = addQueryArgs( '', {
post_type: postType,
Expand All @@ -29,7 +31,15 @@ export async function createNewPost( {
} ).slice( 1 );

await this.visitAdminPage( 'post-new.php', query );
await this.page.waitForSelector( '.edit-post-layout' );

const canvasReadyLocator = legacyCanvas
? this.page.locator( '.edit-post-layout' )
: this.page
.frameLocator( '[name=editor-canvas]' )
.locator( 'body > *' )
.first();

await canvasReadyLocator.waitFor();

await this.page.evaluate( ( welcomeGuide ) => {
window.wp.data
Expand Down
2 changes: 2 additions & 0 deletions packages/e2e-tests/plugins/block-api.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ function enqueue_block_api_plugin_script() {
plugins_url( 'block-api/index.js', __FILE__ ),
array(
'wp-blocks',
'wp-block-editor',
'wp-element',
'wp-hooks',
),
filemtime( plugin_dir_path( __FILE__ ) . 'block-api/index.js' ),
Expand Down
7 changes: 5 additions & 2 deletions packages/e2e-tests/plugins/block-api/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
( function () {
const { registerBlockType } = wp.blocks;
const { useBlockProps } = wp.blockEditor;
const { createElement: el } = wp.element;
const { addFilter } = wp.hooks;

registerBlockType( 'e2e-tests/hello-world', {
apiVersion: 3,
title: 'Hello World',
description: 'Hello World test block.',
category: 'widgets',
edit() {
return 'Hello Editor!';
edit: function Edit() {
return el( 'p', useBlockProps(), 'Hello Editor!' );
},
save() {
return 'Hello Frontend!';
Expand Down
1 change: 0 additions & 1 deletion packages/e2e-tests/plugins/deprecated-node-matcher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ function enqueue_deprecated_node_matcher_plugin_script() {
'gutenberg-test-deprecated-node-matcher',
plugins_url( 'deprecated-node-matcher/index.js', __FILE__ ),
array(
'lodash',
'wp-blocks',
'wp-element',
'wp-block-editor',
Expand Down
28 changes: 12 additions & 16 deletions packages/e2e-tests/plugins/deprecated-node-matcher/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
( function () {
const registerBlockType = wp.blocks.registerBlockType;
const RichText = wp.blockEditor.RichText;
const { useBlockProps, RichText } = wp.blockEditor;
const el = wp.element.createElement;

registerBlockType( 'core/deprecated-children-matcher', {
apiVersion: 3,
title: 'Deprecated Children Matcher',
attributes: {
value: {
Expand All @@ -13,40 +14,35 @@
},
},
category: 'text',
edit( { attributes, setAttributes } ) {
edit: function EditChildrenMatcher( { attributes, setAttributes } ) {
return el( RichText, {
tagName: 'p',
value: attributes.value,
onChange( nextValue ) {
setAttributes( { value: nextValue } );
},
...useBlockProps(),
} );
},
save( { attributes } ) {
return el( RichText.Content, {
tagName: 'p',
value: attributes.value,
...useBlockProps.save(),
} );
},
} );

function toRichTextValue( value ) {
// eslint-disable-next-line no-undef
return _.map( value, function ( subValue ) {
return subValue.children;
} );
return value?.map( ( { children } ) => children ) ?? [];
}

function fromRichTextValue( value ) {
// eslint-disable-next-line no-undef
return _.map( value, function ( subValue ) {
return {
children: subValue,
};
} );
return value.map( ( subValue ) => ( { children: subValue } ) );
}

registerBlockType( 'core/deprecated-node-matcher', {
apiVersion: 3,
title: 'Deprecated Node Matcher',
attributes: {
value: {
Expand All @@ -61,10 +57,10 @@
},
},
category: 'text',
edit( { attributes, setAttributes } ) {
edit: function EditNodeMatcher( { attributes, setAttributes } ) {
return el(
'blockquote',
{},
useBlockProps(),
el( RichText, {
multiline: 'p',
value: toRichTextValue( attributes.value ),
Expand All @@ -74,12 +70,12 @@
} );
},
} )
);
)
},
save( { attributes } ) {
return el(
'blockquote',
{},
useBlockProps.save(),
el( RichText.Content, {
value: toRichTextValue( attributes.value ),
} )
Expand Down
1 change: 1 addition & 0 deletions packages/e2e-tests/plugins/inner-blocks-allowed-blocks/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
const allowedBlocksWhenMultipleChildren = [ 'core/gallery', 'core/video' ];

registerBlockType( 'test/allowed-blocks-dynamic', {
apiVersion: 3,
title: 'Allowed Blocks Dynamic',
icon: 'carrot',
category: 'text',
Expand Down
63 changes: 33 additions & 30 deletions packages/e2e-tests/plugins/inner-blocks-templates/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const registerBlockType = wp.blocks.registerBlockType;
const createBlock = wp.blocks.createBlock;
const el = wp.element.createElement;
const InnerBlocks = wp.blockEditor.InnerBlocks;
const { InnerBlocks, useBlockProps } = wp.blockEditor;
const useState = window.wp.element.useState;

const TEMPLATE = [
Expand Down Expand Up @@ -47,35 +47,38 @@
};

registerBlockType( 'test/test-inner-blocks-no-locking', {
apiVersion: 3,
title: 'Test Inner Blocks no locking',
icon: 'cart',
category: 'text',

edit() {
return el( InnerBlocks, {
edit: function InnerBlocksNoLockingEdit() {
return el( 'div', useBlockProps(), el( InnerBlocks, {
template: TEMPLATE,
} );
} ) );
},

save,
} );

registerBlockType( 'test/test-inner-blocks-locking-all', {
apiVersion: 3,
title: 'Test InnerBlocks locking all',
icon: 'cart',
category: 'text',

edit() {
return el( InnerBlocks, {
edit: function InnerBlocksBlocksLockingAllEdit() {
return el( 'div', useBlockProps(), el( InnerBlocks, {
template: TEMPLATE,
templateLock: 'all',
} );
} ) );
},

save,
} );

registerBlockType( 'test/test-inner-blocks-update-locked-template', {
apiVersion: 3,
title: 'Test Inner Blocks update locked template',
icon: 'cart',
category: 'text',
Expand All @@ -87,7 +90,7 @@
},
},

edit( props ) {
edit: function InnerBlocksUpdateLockedTemplateEdit( props ) {
const hasUpdatedTemplated = props.attributes.hasUpdatedTemplate;
return el( 'div', null, [
el(
Expand All @@ -99,34 +102,36 @@
},
'Update template'
),
el( InnerBlocks, {
el( 'div', useBlockProps(), el( InnerBlocks, {
template: hasUpdatedTemplated
? TEMPLATE_TWO_PARAGRAPHS
: TEMPLATE,
templateLock: 'all',
} ),
} ) ),
] );
},

save,
} );

registerBlockType( 'test/test-inner-blocks-paragraph-placeholder', {
apiVersion: 3,
title: 'Test Inner Blocks Paragraph Placeholder',
icon: 'cart',
category: 'text',

edit() {
return el( InnerBlocks, {
edit: function InnerBlocksParagraphPlaceholderEdit() {
return el( 'div', useBlockProps(), el( InnerBlocks, {
template: TEMPLATE_PARAGRAPH_PLACEHOLDER,
templateInsertUpdatesSelection: true,
} );
} ) );
},

save,
} );

registerBlockType( 'test/test-inner-blocks-transformer-target', {
apiVersion: 3,
title: 'Test Inner Blocks transformer target',
icon: 'cart',
category: 'text',
Expand Down Expand Up @@ -165,36 +170,34 @@
],
},

edit() {
return el( InnerBlocks, {
edit: function InnerBlocksTransformerTargetEdit() {
return el( 'div', useBlockProps(), el( InnerBlocks, {
template: TEMPLATE,
} );
} ) );
},

save,
} );


function InnerBlocksAsyncTemplateEdit() {
const [ template, setTemplate ] = useState( [] );

setInterval( () => {
setTemplate( TEMPLATE_TWO_PARAGRAPHS );
}, 1000 );

return el( InnerBlocks, {
template,
} );
}

registerBlockType(
'test/test-inner-blocks-async-template',
{
apiVersion: 3,
title: 'Test Inner Blocks Async Template',
icon: 'cart',
category: 'text',

edit: InnerBlocksAsyncTemplateEdit,
edit: function InnerBlocksAsyncTemplateEdit() {
const [ template, setTemplate ] = useState( [] );

setInterval( () => {
setTemplate( TEMPLATE_TWO_PARAGRAPHS );
}, 1000 );

return el('div', useBlockProps(), el( InnerBlocks, {
template,
} ) );
},

// Purposely do not save inner blocks so that it's possible to test template resolution.
save() {},
Expand Down
3 changes: 2 additions & 1 deletion packages/e2e-tests/specs/editor/plugins/container-blocks.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
insertBlock,
switchEditorModeTo,
pressKeyWithModifier,
canvas,
} from '@wordpress/e2e-test-utils';

describe( 'InnerBlocks Template Sync', () => {
Expand Down Expand Up @@ -75,7 +76,7 @@ describe( 'InnerBlocks Template Sync', () => {
expect( await getEditedPostContent() ).toMatchSnapshot();

// Trigger a template update and assert that a second block is now present.
const [ button ] = await page.$x(
const [ button ] = await canvas().$x(
`//button[contains(text(), 'Update template')]`
);
await button.click();
Expand Down
5 changes: 3 additions & 2 deletions test/e2e/specs/editor/plugins/block-api.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ test.describe( 'Using Block API', () => {
test( 'Inserts the filtered hello world block even when filter added after block registration', async ( {
admin,
editor,
page,
} ) => {
await admin.createNewPost();

await editor.insertBlock( { name: 'e2e-tests/hello-world' } );

const block = page.locator( '[data-type="e2e-tests/hello-world"]' );
const block = editor.canvas.locator(
'[data-type="e2e-tests/hello-world"]'
);
await expect( block ).toHaveText( 'Hello Editor!' );
} );
} );
2 changes: 0 additions & 2 deletions test/e2e/specs/editor/plugins/deprecated-node-matcher.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ test.describe( 'Deprecated Node Matcher', () => {
page,
editor,
} ) => {
// await insertBlock( 'Deprecated Node Matcher' );
await editor.insertBlock( { name: 'core/deprecated-node-matcher' } );
await page.keyboard.type( 'test' );
await page.keyboard.press( 'Enter' );
Expand All @@ -39,7 +38,6 @@ test.describe( 'Deprecated Node Matcher', () => {
editor,
pageUtils,
} ) => {
// await insertBlock( 'Deprecated Children Matcher' );
await editor.insertBlock( {
name: 'core/deprecated-children-matcher',
} );
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/specs/editor/plugins/wp-editor-meta-box.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ test.describe( 'WP Editor Meta Boxes', () => {
} );

test( 'Should save the changes', async ( { admin, editor, page } ) => {
await admin.createNewPost();
await admin.createNewPost( { legacyCanvas: true } );

// Add title to enable valid non-empty post save.
await editor.canvas.type(
Expand Down
8 changes: 5 additions & 3 deletions test/e2e/specs/editor/various/inner-blocks-templates.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ test.describe( 'Inner blocks templates', () => {
name: 'test/test-inner-blocks-async-template',
} );

const blockWithTemplateContent = editor.canvas.locator(
'role=document[name="Block: Test Inner Blocks Async Template"i] >> text=OneTwo'
);
const blockWithTemplateContent = page
.frameLocator( '[name=editor-canvas]' )
Copy link
Member

@kevin940726 kevin940726 Jun 28, 2023

Choose a reason for hiding this comment

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

Could you explain why we need to use frameLocator here? Is it still needed if we wait for the page to load in createNewPost?

Copy link
Contributor Author

@stokesman stokesman Jun 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's for reuse after the page.reload() that follows. Now that this test runs in the iframed editor editor.canvas returns a Frame and the locator formed from that isn't reusable after a page reload.

So this can be done differently if there's a style that seems better to you or I can add a comment. I thought it might be ideal to consider doing a follow up to try having editor.canvas return a FrameLocator instead of a Frame and then this could revert back to using that.

Copy link
Member

@kevin940726 kevin940726 Jun 29, 2023

Choose a reason for hiding this comment

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

I thought it might be ideal to consider doing a follow up to try having editor.canvas return a FrameLocator instead of a Frame and then this could revert back to using that.

Yeah, I regret using frame in the first place 😅 . One thing to consider is that there are code that depend on some frame-specific API so it's not easy to switch to frameLocator now. I've been trying to find alternatives but haven't found any. Please share if you have an idea! 🙇

Copy link
Contributor Author

@stokesman stokesman Jun 29, 2023

Choose a reason for hiding this comment

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

One thing to consider is that there are code that depend on some frame-specific API so it's not easy to switch to frameLocator now

I'd noticed but I figured it should possible to refactor any dependent tests/code. It also looks like many of the Frame methods are deprecated so it might be good for scrubbing that in case we are using some. I don't know Playwright well enough to have a clue if there's any easier alternative.

Copy link
Member

@kevin940726 kevin940726 Jul 4, 2023

Choose a reason for hiding this comment

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

I couldn't find an alternative to evaluate or waitForFunction though, which I believe some existing code rely on?

Copy link
Contributor Author

@stokesman stokesman Jul 5, 2023

Choose a reason for hiding this comment

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

Thanks for those specifics. It does look like that is a sticking point 🤔.

.locator(
'role=document[name="Block: Test Inner Blocks Async Template"i] >> text=OneTwo'
);

// The block template content appears asynchronously, so wait for it.
await expect( blockWithTemplateContent ).toBeVisible();
Expand Down