From 6defb8750a447136d8964d74a7de6b268cd0f891 Mon Sep 17 00:00:00 2001 From: Mario Santos <34552881+SantosGuillamot@users.noreply.github.com> Date: Fri, 1 Mar 2024 14:06:38 +0100 Subject: [PATCH] Fix inserting button block when pressing enter in a block with bound `text` attribute (#59361) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add tabindex 0 to all disabled elements in rich text * Insert block when use enter in disabled rich text * Take default block into account * Add e2e tests when pressing enter in bound blocks * Revert initial implementation * Use `insertAfterBlock` instead of `insertDefaultBlock`. * Remove unnecessary `insertDefaultBlock` * Destructure innerBlocks array in tests Co-authored-by: Greg Ziółkowski * Prettify tests * Remove unnecessary select in tests * Destructuring blocks in tests * Revert "Remove unnecessary select in tests" This reverts commit 63ffff87ca048a218567772710dcc45affcfd2aa. * Remove unnecessary select in tests * Adapt tests to latest changes --------- Co-authored-by: Greg Ziółkowski --- .../use-selected-block-event-handlers.js | 10 +- .../editor/various/block-bindings.spec.js | 243 +++++++++++++----- 2 files changed, 184 insertions(+), 69 deletions(-) diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-selected-block-event-handlers.js b/packages/block-editor/src/components/block-list/use-block-props/use-selected-block-event-handlers.js index bf4fc55879448a..01cc462e507ecf 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-selected-block-event-handlers.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-selected-block-event-handlers.js @@ -22,7 +22,7 @@ import { store as blockEditorStore } from '../../../store'; export function useEventHandlers( { clientId, isSelected } ) { const { getBlockRootClientId, getBlockIndex } = useSelect( blockEditorStore ); - const { insertDefaultBlock, removeBlock } = useDispatch( blockEditorStore ); + const { insertAfterBlock, removeBlock } = useDispatch( blockEditorStore ); return useRefEffect( ( node ) => { @@ -57,11 +57,7 @@ export function useEventHandlers( { clientId, isSelected } ) { event.preventDefault(); if ( keyCode === ENTER ) { - insertDefaultBlock( - {}, - getBlockRootClientId( clientId ), - getBlockIndex( clientId ) + 1 - ); + insertAfterBlock( clientId ); } else { removeBlock( clientId ); } @@ -90,7 +86,7 @@ export function useEventHandlers( { clientId, isSelected } ) { isSelected, getBlockRootClientId, getBlockIndex, - insertDefaultBlock, + insertAfterBlock, removeBlock, ] ); diff --git a/test/e2e/specs/editor/various/block-bindings.spec.js b/test/e2e/specs/editor/various/block-bindings.spec.js index ca19c06beff01a..8cd3423290e3fa 100644 --- a/test/e2e/specs/editor/various/block-bindings.spec.js +++ b/test/e2e/specs/editor/various/block-bindings.spec.js @@ -47,7 +47,7 @@ test.describe( 'Block bindings', () => { } ); test.describe( 'Paragraph', () => { - test( 'Should show the value of the custom field', async ( { + test( 'should show the value of the custom field', async ( { editor, } ) => { await editor.insertBlock( { @@ -72,7 +72,7 @@ test.describe( 'Block bindings', () => { ); } ); - test( 'Should lock the appropriate controls with a registered source', async ( { + test( 'should lock the appropriate controls with a registered source', async ( { editor, page, } ) => { @@ -118,7 +118,7 @@ test.describe( 'Block bindings', () => { ); } ); - test( 'Should lock the appropriate controls when source is not defined', async ( { + test( 'should lock the appropriate controls when source is not defined', async ( { editor, page, } ) => { @@ -166,7 +166,7 @@ test.describe( 'Block bindings', () => { } ); test.describe( 'Heading', () => { - test( 'Should show the key of the custom field', async ( { + test( 'should show the key of the custom field', async ( { editor, } ) => { await editor.insertBlock( { @@ -189,7 +189,7 @@ test.describe( 'Block bindings', () => { await expect( headingBlock ).toHaveText( 'text_custom_field' ); } ); - test( 'Should lock the appropriate controls with a registered source', async ( { + test( 'should lock the appropriate controls with a registered source', async ( { editor, page, } ) => { @@ -235,7 +235,7 @@ test.describe( 'Block bindings', () => { ); } ); - test( 'Should lock the appropriate controls when source is not defined', async ( { + test( 'should lock the appropriate controls when source is not defined', async ( { editor, page, } ) => { @@ -283,7 +283,7 @@ test.describe( 'Block bindings', () => { } ); test.describe( 'Button', () => { - test( 'Should show the key of the custom field when text is bound', async ( { + test( 'should show the key of the custom field when text is bound', async ( { editor, } ) => { await editor.insertBlock( { @@ -313,7 +313,7 @@ test.describe( 'Block bindings', () => { await expect( buttonBlock ).toHaveText( 'text_custom_field' ); } ); - test( 'Should lock text controls when text is bound to a registered source', async ( { + test( 'should lock text controls when text is bound to a registered source', async ( { editor, page, } ) => { @@ -375,7 +375,7 @@ test.describe( 'Block bindings', () => { ).toBeVisible(); } ); - test( 'Should lock text controls when text is bound to an undefined source', async ( { + test( 'should lock text controls when text is bound to an undefined source', async ( { editor, page, } ) => { @@ -437,7 +437,7 @@ test.describe( 'Block bindings', () => { ).toBeVisible(); } ); - test( 'Should lock url controls when url is bound to a registered source', async ( { + test( 'should lock url controls when url is bound to a registered source', async ( { editor, page, } ) => { @@ -497,7 +497,7 @@ test.describe( 'Block bindings', () => { ).toBeHidden(); } ); - test( 'Should lock url controls when url is bound to an undefined source', async ( { + test( 'should lock url controls when url is bound to an undefined source', async ( { editor, page, } ) => { @@ -557,7 +557,7 @@ test.describe( 'Block bindings', () => { ).toBeHidden(); } ); - test( 'Should lock url and text controls when both are bound', async ( { + test( 'should lock url and text controls when both are bound', async ( { editor, page, } ) => { @@ -630,7 +630,7 @@ test.describe( 'Block bindings', () => { } ); test.describe( 'Image', () => { - test( 'Should show the upload form when url is not bound', async ( { + test( 'should show the upload form when url is not bound', async ( { editor, } ) => { await editor.insertBlock( { name: 'core/image' } ); @@ -643,7 +643,7 @@ test.describe( 'Block bindings', () => { ).toBeVisible(); } ); - test( 'Should NOT show the upload form when url is bound to a registered source', async ( { + test( 'should NOT show the upload form when url is bound to a registered source', async ( { editor, } ) => { await editor.insertBlock( { @@ -671,7 +671,7 @@ test.describe( 'Block bindings', () => { ).toBeHidden(); } ); - test( 'Should NOT show the upload form when url is bound to an undefined source', async ( { + test( 'should NOT show the upload form when url is bound to an undefined source', async ( { editor, } ) => { await editor.insertBlock( { @@ -699,7 +699,7 @@ test.describe( 'Block bindings', () => { ).toBeHidden(); } ); - test( 'Should lock url controls when url is bound to a registered source', async ( { + test( 'should lock url controls when url is bound to a registered source', async ( { editor, page, } ) => { @@ -768,7 +768,7 @@ test.describe( 'Block bindings', () => { expect( titleValue ).toBe( 'default title value' ); } ); - test( 'Should lock url controls when url is bound to an undefined source', async ( { + test( 'should lock url controls when url is bound to an undefined source', async ( { editor, page, } ) => { @@ -837,7 +837,7 @@ test.describe( 'Block bindings', () => { expect( titleValue ).toBe( 'default title value' ); } ); - test( 'Should disable alt textarea when alt is bound to a registered source', async ( { + test( 'should disable alt textarea when alt is bound to a registered source', async ( { editor, page, } ) => { @@ -900,7 +900,7 @@ test.describe( 'Block bindings', () => { expect( titleValue ).toBe( 'default title value' ); } ); - test( 'Should disable alt textarea when alt is bound to an undefined source', async ( { + test( 'should disable alt textarea when alt is bound to an undefined source', async ( { editor, page, } ) => { @@ -963,7 +963,7 @@ test.describe( 'Block bindings', () => { expect( titleValue ).toBe( 'default title value' ); } ); - test( 'Should disable title input when title is bound to a registered source', async ( { + test( 'should disable title input when title is bound to a registered source', async ( { editor, page, } ) => { @@ -1026,7 +1026,7 @@ test.describe( 'Block bindings', () => { expect( titleValue ).toBe( 'text_custom_field' ); } ); - test( 'Should disable title input when title is bound to an undefined source', async ( { + test( 'should disable title input when title is bound to an undefined source', async ( { editor, page, } ) => { @@ -1168,7 +1168,7 @@ test.describe( 'Block bindings', () => { await admin.createNewPost( { title: 'Test bindings' } ); } ); test.describe( 'Paragraph', () => { - test( 'Should show the value of the custom field when exists', async ( { + test( 'should show the value of the custom field when exists', async ( { editor, page, } ) => { @@ -1210,7 +1210,7 @@ test.describe( 'Block bindings', () => { ); } ); - test( "Should show the value of the key when custom field doesn't exists", async ( { + test( "should show the value of the key when custom field doesn't exist", async ( { editor, page, } ) => { @@ -1312,50 +1312,124 @@ test.describe( 'Block bindings', () => { 'fallback value' ); } ); + + test( 'should add empty paragraph block when pressing enter', async ( { + editor, + page, + } ) => { + await editor.insertBlock( { + name: 'core/paragraph', + attributes: { + content: 'paragraph default content', + metadata: { + bindings: { + content: { + source: 'core/post-meta', + args: { key: 'text_custom_field' }, + }, + }, + }, + }, + } ); + await page.keyboard.press( 'Enter' ); + const [ initialParagraph, newEmptyParagraph ] = + await editor.canvas + .locator( '[data-type="core/paragraph"]' ) + .all(); + await expect( initialParagraph ).toHaveText( + 'Value of the text_custom_field' + ); + await expect( newEmptyParagraph ).toHaveText( '' ); + await expect( newEmptyParagraph ).toBeEditable(); + } ); } ); - test( 'Heading - should show the value of the custom field', async ( { - editor, - page, - } ) => { - await editor.insertBlock( { - name: 'core/heading', - attributes: { - anchor: 'heading-binding', - content: 'heading default content', - metadata: { - bindings: { - content: { - source: 'core/post-meta', - args: { key: 'text_custom_field' }, + test.describe( 'Heading', () => { + test( 'should show the value of the custom field', async ( { + editor, + page, + } ) => { + await editor.insertBlock( { + name: 'core/heading', + attributes: { + anchor: 'heading-binding', + content: 'heading default content', + metadata: { + bindings: { + content: { + source: 'core/post-meta', + args: { key: 'text_custom_field' }, + }, }, }, }, - }, + } ); + const headingBlock = editor.canvas.getByRole( 'document', { + name: 'Block: Heading', + } ); + await expect( headingBlock ).toHaveText( + 'Value of the text_custom_field' + ); + // Heading is not editable. + await expect( headingBlock ).toHaveAttribute( + 'contenteditable', + 'false' + ); + + // Check the frontend shows the value of the custom field. + const postId = await editor.publishPost(); + await page.goto( `/?p=${ postId }` ); + await expect( + page.locator( '#heading-binding' ) + ).toBeVisible(); + await expect( page.locator( '#heading-binding' ) ).toHaveText( + 'Value of the text_custom_field' + ); } ); - const headingBlock = editor.canvas.getByRole( 'document', { - name: 'Block: Heading', + + test( 'should add empty paragraph block when pressing enter', async ( { + editor, + page, + } ) => { + await editor.insertBlock( { + name: 'core/heading', + attributes: { + anchor: 'heading-binding', + content: 'heading default content', + metadata: { + bindings: { + content: { + source: 'core/post-meta', + args: { key: 'text_custom_field' }, + }, + }, + }, + }, + } ); + await page.keyboard.press( 'Enter' ); + // Can't use `editor.getBlocks` because it doesn't return the meta value shown in the editor. + const [ initialHeading, newEmptyParagraph ] = + await editor.canvas.locator( '[data-block]' ).all(); + // First block should be the original block. + await expect( initialHeading ).toHaveAttribute( + 'data-type', + 'core/heading' + ); + await expect( initialHeading ).toHaveText( + 'Value of the text_custom_field' + ); + // Second block should be an empty paragraph block. + await expect( newEmptyParagraph ).toHaveAttribute( + 'data-type', + 'core/paragraph' + ); + await expect( newEmptyParagraph ).toHaveText( '' ); + await expect( newEmptyParagraph ).toBeEditable(); } ); - await expect( headingBlock ).toHaveText( - 'Value of the text_custom_field' - ); - // Heading is not editable. - await expect( headingBlock ).toHaveAttribute( - 'contenteditable', - 'false' - ); - - // Check the frontend shows the value of the custom field. - const postId = await editor.publishPost(); - await page.goto( `/?p=${ postId }` ); - await expect( page.locator( '#heading-binding' ) ).toBeVisible(); - await expect( page.locator( '#heading-binding' ) ).toHaveText( - 'Value of the text_custom_field' - ); } ); test.describe( 'Button', () => { - test( 'Should show the value of the custom field when text is bound', async ( { + test( 'should show the value of the custom field when text is bound', async ( { editor, page, } ) => { @@ -1411,7 +1485,7 @@ test.describe( 'Block bindings', () => { ); } ); - test( 'Should use the value of the custom field when url is bound', async ( { + test( 'should use the value of the custom field when url is bound', async ( { editor, page, } ) => { @@ -1449,7 +1523,7 @@ test.describe( 'Block bindings', () => { ); } ); - test( 'Should use the values of the custom fields when text and url are bound', async ( { + test( 'should use the values of the custom fields when text and url are bound', async ( { editor, page, } ) => { @@ -1492,6 +1566,51 @@ test.describe( 'Block bindings', () => { '#url-custom-field' ); } ); + + test( 'should add empty button block when pressing enter', async ( { + editor, + page, + } ) => { + await editor.insertBlock( { + name: 'core/buttons', + innerBlocks: [ + { + name: 'core/button', + attributes: { + anchor: 'button-text-binding', + text: 'button default text', + url: '#default-url', + metadata: { + bindings: { + text: { + source: 'core/post-meta', + args: { key: 'text_custom_field' }, + }, + }, + }, + }, + }, + ], + } ); + await editor.canvas + .getByRole( 'document', { + name: 'Block: Button', + exact: true, + } ) + .getByRole( 'textbox' ) + .click(); + await page.keyboard.press( 'Enter' ); + const [ initialButton, newEmptyButton ] = await editor.canvas + .locator( '[data-type="core/button"]' ) + .all(); + // First block should be the original block. + await expect( initialButton ).toHaveText( + 'Value of the text_custom_field' + ); + // Second block should be an empty paragraph block. + await expect( newEmptyButton ).toHaveText( '' ); + await expect( newEmptyButton ).toBeEditable(); + } ); } ); test.describe( 'Image', () => { @@ -1518,7 +1637,7 @@ test.describe( 'Block bindings', () => { } ); await page.reload(); } ); - test( 'Should show the value of the custom field when url is bound', async ( { + test( 'should show the value of the custom field when url is bound', async ( { editor, page, BlockBindingsUtils, @@ -1569,7 +1688,7 @@ test.describe( 'Block bindings', () => { ); } ); - test( 'Should show value of the custom field in the alt textarea when alt is bound', async ( { + test( 'should show value of the custom field in the alt textarea when alt is bound', async ( { editor, page, BlockBindingsUtils, @@ -1635,7 +1754,7 @@ test.describe( 'Block bindings', () => { ); } ); - test( 'Should show value of the custom field in the title input when title is bound', async ( { + test( 'should show value of the custom field in the title input when title is bound', async ( { editor, page, BlockBindingsUtils,