From be4e01ec910a10ab68d5e7eb1cb6148d6864e4fe Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 28 Feb 2020 18:13:03 -0500 Subject: [PATCH] Editor: Shim meta attributes for early block registrations (#20544) * Editor: Shim meta attributes for early block registrations * E2E Tests: Add meta block early registration test * Editor: Clarify comment regarding meta attribute shim --- .../plugins/meta-attribute-block.php | 16 ++- .../plugins/meta-attribute-block/early.js | 32 +++++ .../plugins/meta-attribute-block/index.js | 35 ------ .../plugins/meta-attribute-block/late.js | 32 +++++ .../meta-attribute-block.test.js.snap | 8 +- .../plugins/meta-attribute-block.test.js | 117 ++++++++++-------- .../custom-sources-backwards-compatibility.js | 25 +++- 7 files changed, 169 insertions(+), 96 deletions(-) create mode 100644 packages/e2e-tests/plugins/meta-attribute-block/early.js delete mode 100644 packages/e2e-tests/plugins/meta-attribute-block/index.js create mode 100644 packages/e2e-tests/plugins/meta-attribute-block/late.js diff --git a/packages/e2e-tests/plugins/meta-attribute-block.php b/packages/e2e-tests/plugins/meta-attribute-block.php index 58eec7c04244af..4513ebafde26aa 100644 --- a/packages/e2e-tests/plugins/meta-attribute-block.php +++ b/packages/e2e-tests/plugins/meta-attribute-block.php @@ -29,13 +29,23 @@ function init_test_meta_attribute_block_plugin() { */ function enqueue_test_meta_attribute_block() { wp_enqueue_script( - 'gutenberg-test-meta-attribute-block', - plugins_url( 'meta-attribute-block/index.js', __FILE__ ), + 'gutenberg-test-meta-attribute-block-early', + plugins_url( 'meta-attribute-block/early.js', __FILE__ ), array( 'wp-blocks', 'wp-element', ), - filemtime( plugin_dir_path( __FILE__ ) . 'meta-attribute-block/index.js' ), + filemtime( plugin_dir_path( __FILE__ ) . 'meta-attribute-block/early.js' ) + ); + + wp_enqueue_script( + 'gutenberg-test-meta-attribute-block-late', + plugins_url( 'meta-attribute-block/late.js', __FILE__ ), + array( + 'wp-blocks', + 'wp-element', + ), + filemtime( plugin_dir_path( __FILE__ ) . 'meta-attribute-block/late.js' ), true ); } diff --git a/packages/e2e-tests/plugins/meta-attribute-block/early.js b/packages/e2e-tests/plugins/meta-attribute-block/early.js new file mode 100644 index 00000000000000..7c2831e84e4de2 --- /dev/null +++ b/packages/e2e-tests/plugins/meta-attribute-block/early.js @@ -0,0 +1,32 @@ +( function() { + var registerBlockType = wp.blocks.registerBlockType; + var el = wp.element.createElement; + + registerBlockType( 'test/test-meta-attribute-block-early', { + title: 'Test Meta Attribute Block (Early Registration)', + icon: 'star', + category: 'common', + + attributes: { + content: { + type: 'string', + source: 'meta', + meta: 'my_meta', + }, + }, + + edit: function( props ) { + return el( 'input', { + className: 'my-meta-input', + value: props.attributes.content, + onChange: function( event ) { + props.setAttributes( { content: event.target.value } ); + }, + } ); + }, + + save: function() { + return null; + }, + } ); +} )(); diff --git a/packages/e2e-tests/plugins/meta-attribute-block/index.js b/packages/e2e-tests/plugins/meta-attribute-block/index.js deleted file mode 100644 index 0910e648299aef..00000000000000 --- a/packages/e2e-tests/plugins/meta-attribute-block/index.js +++ /dev/null @@ -1,35 +0,0 @@ -( function() { - var registerBlockType = wp.blocks.registerBlockType; - var el = wp.element.createElement; - - registerBlockType( 'test/test-meta-attribute-block', { - title: 'Test Meta Attribute Block', - icon: 'star', - category: 'common', - - attributes: { - content: { - type: "string", - source: "meta", - meta: "my_meta", - }, - }, - - edit: function( props ) { - return el( - 'input', - { - className: 'my-meta-input', - value: props.attributes.content, - onChange: function( event ) { - props.setAttributes( { content: event.target.value } ); - }, - } - ); - }, - - save: function() { - return null; - }, - } ); -} )(); diff --git a/packages/e2e-tests/plugins/meta-attribute-block/late.js b/packages/e2e-tests/plugins/meta-attribute-block/late.js new file mode 100644 index 00000000000000..362acab0f776a7 --- /dev/null +++ b/packages/e2e-tests/plugins/meta-attribute-block/late.js @@ -0,0 +1,32 @@ +( function() { + var registerBlockType = wp.blocks.registerBlockType; + var el = wp.element.createElement; + + registerBlockType( 'test/test-meta-attribute-block-late', { + title: 'Test Meta Attribute Block (Late Registration)', + icon: 'star', + category: 'common', + + attributes: { + content: { + type: 'string', + source: 'meta', + meta: 'my_meta', + }, + }, + + edit: function( props ) { + return el( 'input', { + className: 'my-meta-input', + value: props.attributes.content, + onChange: function( event ) { + props.setAttributes( { content: event.target.value } ); + }, + } ); + }, + + save: function() { + return null; + }, + } ); +} )(); diff --git a/packages/e2e-tests/specs/editor/plugins/__snapshots__/meta-attribute-block.test.js.snap b/packages/e2e-tests/specs/editor/plugins/__snapshots__/meta-attribute-block.test.js.snap index 1b5c1303664027..268c8b45d059f2 100644 --- a/packages/e2e-tests/specs/editor/plugins/__snapshots__/meta-attribute-block.test.js.snap +++ b/packages/e2e-tests/specs/editor/plugins/__snapshots__/meta-attribute-block.test.js.snap @@ -1,5 +1,9 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Block with a meta attribute Should persist the meta attribute properly 1`] = `""`; +exports[`Block with a meta attribute Early Registration Should persist the meta attribute properly 1`] = `""`; -exports[`Block with a meta attribute Should persist the meta attribute properly in a different post type 1`] = `""`; +exports[`Block with a meta attribute Early Registration Should persist the meta attribute properly in a different post type 1`] = `""`; + +exports[`Block with a meta attribute Late Registration Should persist the meta attribute properly 1`] = `""`; + +exports[`Block with a meta attribute Late Registration Should persist the meta attribute properly in a different post type 1`] = `""`; diff --git a/packages/e2e-tests/specs/editor/plugins/meta-attribute-block.test.js b/packages/e2e-tests/specs/editor/plugins/meta-attribute-block.test.js index 68f4de2768fbb5..22569268de6296 100644 --- a/packages/e2e-tests/specs/editor/plugins/meta-attribute-block.test.js +++ b/packages/e2e-tests/specs/editor/plugins/meta-attribute-block.test.js @@ -24,68 +24,75 @@ describe( 'Block with a meta attribute', () => { await deactivatePlugin( 'gutenberg-test-meta-attribute-block' ); } ); - it( 'Should persist the meta attribute properly', async () => { - await insertBlock( 'Test Meta Attribute Block' ); - await page.keyboard.type( 'Value' ); + describe.each( [ [ 'Early Registration' ], [ 'Late Registration' ] ] )( + '%s', + ( variant ) => { + it( 'Should persist the meta attribute properly', async () => { + await insertBlock( `Test Meta Attribute Block (${ variant })` ); + await page.keyboard.type( 'Value' ); - // Regression Test: Previously the caret would wrongly reset to the end - // of any input for meta-sourced attributes, due to syncing behavior of - // meta attribute updates. - // - // See: https://github.com/WordPress/gutenberg/issues/15739 - await pressKeyTimes( 'ArrowLeft', 5 ); - await page.keyboard.type( 'Meta ' ); + // Regression Test: Previously the caret would wrongly reset to the end + // of any input for meta-sourced attributes, due to syncing behavior of + // meta attribute updates. + // + // See: https://github.com/WordPress/gutenberg/issues/15739 + await pressKeyTimes( 'ArrowLeft', 5 ); + await page.keyboard.type( 'Meta ' ); - await saveDraft(); - await page.reload(); + await saveDraft(); + await page.reload(); - expect( await getEditedPostContent() ).toMatchSnapshot(); - const persistedValue = await page.evaluate( - () => document.querySelector( '.my-meta-input' ).value - ); - expect( persistedValue ).toBe( 'Meta Value' ); - } ); + expect( await getEditedPostContent() ).toMatchSnapshot(); + const persistedValue = await page.evaluate( + () => document.querySelector( '.my-meta-input' ).value + ); + expect( persistedValue ).toBe( 'Meta Value' ); + } ); - it( 'Should use the same value in all the blocks', async () => { - await insertBlock( 'Test Meta Attribute Block' ); - await insertBlock( 'Test Meta Attribute Block' ); - await insertBlock( 'Test Meta Attribute Block' ); - await page.keyboard.type( 'Meta Value' ); + it( 'Should use the same value in all the blocks', async () => { + await insertBlock( `Test Meta Attribute Block (${ variant })` ); + await insertBlock( `Test Meta Attribute Block (${ variant })` ); + await insertBlock( `Test Meta Attribute Block (${ variant })` ); + await page.keyboard.type( 'Meta Value' ); - const inputs = await page.$$( '.my-meta-input' ); - await Promise.all( - inputs.map( async ( input ) => { - // Clicking the input selects the block, - // and selecting the block enables the sync data mode - // as otherwise the asynchronous re-rendering of unselected blocks - // may cause the input to have not yet been updated for the other blocks - await input.click(); - const inputValue = await input.getProperty( 'value' ); - expect( await inputValue.jsonValue() ).toBe( 'Meta Value' ); - } ) - ); - } ); + const inputs = await page.$$( '.my-meta-input' ); + await Promise.all( + inputs.map( async ( input ) => { + // Clicking the input selects the block, + // and selecting the block enables the sync data mode + // as otherwise the asynchronous re-rendering of unselected blocks + // may cause the input to have not yet been updated for the other blocks + await input.click(); + const inputValue = await input.getProperty( 'value' ); + expect( await inputValue.jsonValue() ).toBe( + 'Meta Value' + ); + } ) + ); + } ); - it( 'Should persist the meta attribute properly in a different post type', async () => { - await createNewPost( { postType: 'page' } ); - await insertBlock( 'Test Meta Attribute Block' ); - await page.keyboard.type( 'Value' ); + it( 'Should persist the meta attribute properly in a different post type', async () => { + await createNewPost( { postType: 'page' } ); + await insertBlock( `Test Meta Attribute Block (${ variant })` ); + await page.keyboard.type( 'Value' ); - // Regression Test: Previously the caret would wrongly reset to the end - // of any input for meta-sourced attributes, due to syncing behavior of - // meta attribute updates. - // - // See: https://github.com/WordPress/gutenberg/issues/15739 - await pressKeyTimes( 'ArrowLeft', 5 ); - await page.keyboard.type( 'Meta ' ); + // Regression Test: Previously the caret would wrongly reset to the end + // of any input for meta-sourced attributes, due to syncing behavior of + // meta attribute updates. + // + // See: https://github.com/WordPress/gutenberg/issues/15739 + await pressKeyTimes( 'ArrowLeft', 5 ); + await page.keyboard.type( 'Meta ' ); - await saveDraft(); - await page.reload(); + await saveDraft(); + await page.reload(); - expect( await getEditedPostContent() ).toMatchSnapshot(); - const persistedValue = await page.evaluate( - () => document.querySelector( '.my-meta-input' ).value - ); - expect( persistedValue ).toBe( 'Meta Value' ); - } ); + expect( await getEditedPostContent() ).toMatchSnapshot(); + const persistedValue = await page.evaluate( + () => document.querySelector( '.my-meta-input' ).value + ); + expect( persistedValue ).toBe( 'Meta Value' ); + } ); + } + ); } ); diff --git a/packages/editor/src/hooks/custom-sources-backwards-compatibility.js b/packages/editor/src/hooks/custom-sources-backwards-compatibility.js index 8c91cd625da62e..51d5bf4ac5fda0 100644 --- a/packages/editor/src/hooks/custom-sources-backwards-compatibility.js +++ b/packages/editor/src/hooks/custom-sources-backwards-compatibility.js @@ -6,7 +6,7 @@ import { pickBy, mapValues, isEmpty, mapKeys } from 'lodash'; /** * WordPress dependencies */ -import { useSelect } from '@wordpress/data'; +import { select as globalSelect, useSelect } from '@wordpress/data'; import { useEntityProp } from '@wordpress/core-data'; import { useMemo } from '@wordpress/element'; import { createHigherOrderComponent } from '@wordpress/compose'; @@ -116,3 +116,26 @@ addFilter( 'core/editor/custom-sources-backwards-compatibility/shim-attribute-source', shimAttributeSource ); + +// The above filter will only capture blocks registered after the filter was +// added. There may already be blocks registered by this point, and those must +// be updated to apply the shim. +// +// The following implementation achieves this, albeit with a couple caveats: +// - Only blocks registered on the global store will be modified. +// - The block settings are directly mutated, since there is currently no +// mechanism to update an existing block registration. This is the reason for +// `getBlockType` separate from `getBlockTypes`, since the latter returns a +// _copy_ of the block registration (i.e. the mutation would not affect the +// actual registered block settings). +// +// `getBlockTypes` or `getBlockType` implementation could change in the future +// in regards to creating settings clones, but the corresponding end-to-end +// tests for meta blocks should cover against any potential regressions. +// +// In the future, we could support updating block settings, at which point this +// implementation could use that mechanism instead. +globalSelect( 'core/blocks' ) + .getBlockTypes() + .map( ( { name } ) => globalSelect( 'core/blocks' ).getBlockType( name ) ) + .forEach( shimAttributeSource );