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.

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

Better handling for plugin modification of core blocks #10204

Open
danielbachhuber opened this issue Sep 26, 2018 · 9 comments
Open

Better handling for plugin modification of core blocks #10204

danielbachhuber opened this issue Sep 26, 2018 · 9 comments
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Feature] Block API API that allows to express the block paradigm. [Feature] Block Transforms Block transforms from one block to another [Feature] Blocks Overall functionality of blocks [Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Media Anything that impacts the experience of managing media [Type] Enhancement A suggestion for improvement.
Milestone

Comments

@danielbachhuber
Copy link
Member

danielbachhuber commented Sep 26, 2018

A plugin can use the blocks.getSaveElement to modify the saved output of a Gutenberg core block. I started documenting this here: #8470 (review)

For instance, I may want to write a plugin which adds a data-pin-description attribute to an Image Block:

<!-- wp:image {"id":33863} -->
<figure class="wp-block-image"><img data-pin-description="My Pinterest description" src="http://tastyrecipes.test/wp-content/uploads/2018/03/food-dinner-pasta-broccoli.jpg" alt="" class="wp-image-33863" /></figure>
<!-- /wp:image -->

However, if the plugin is deactivated after the user adds a Pinterest Description to their Image Block, the Image Block, the block will enter into an invalid, crashed state.

The Classic Editor permits manipulation of any HTML5 element, in accordance to the entire spec. It would be ideal of Gutenberg permitted the same: #6878 (comment)

Related #7811
Related #8472

@danielbachhuber danielbachhuber added [Feature] Block API API that allows to express the block paradigm. [Feature] Blocks Overall functionality of blocks [Feature] Block Transforms Block transforms from one block to another labels Sep 26, 2018
@mtias mtias removed this from the Merge: Plugins milestone Oct 3, 2018
@danielbachhuber danielbachhuber added [Feature] Extensibility The ability to extend blocks or the editing experience Backwards Compatibility Issues or PRs that impact backwards compatability labels Oct 8, 2018
@danielbachhuber danielbachhuber added this to the WordPress 5.0 milestone Oct 15, 2018
@danielbachhuber
Copy link
Member Author

My workaround for this was to store my plugin data in post meta instead. There were a couple of notable downsides:

  • Because the data is stored in post meta and filtered into post_content, it won't persist as the user expects it to persist when the plugin is deactivated.
  • Storing JSON-serialized data in post meta feels quite dirty. See https://core.trac.wordpress.org/ticket/43392 for a related conversation.

On the plus side, register_meta() worked surprisingly well in tandem to registering the Block attribute.

Some partial code snippets:

/**
 * Register our Pinterest meta attributes after we know all post types in the REST API.
 */
public static function action_init_late_register_meta() {
	foreach ( get_post_types( array( 'show_in_rest' => true ), 'objects' ) as $post_type ) {
		$class = ! empty( $post_type->rest_controller_class ) ? $post_type->rest_controller_class : 'WP_REST_Posts_Controller';

		if ( ! class_exists( $class ) ) {
			continue;
		}
		$controller = new $class( $post_type->name );
		if ( ! is_subclass_of( $controller, 'WP_REST_Controller' ) ) {
			continue;
		}

		if ( ! post_type_supports( $post_type->name, 'editor' ) ) {
			continue;
		}

		register_meta(
			$post_type->name,
			self::TEXT_KEY,
			array(
				'show_in_rest' => true,
				'single'       => true,
				'type'         => 'string',
			)
		);
	}
}
/**
 * Register 'data-pin-description' attribute to 'core/image' block.
 */
hooks.addFilter(
	'blocks.registerBlockType',
	'wp-tasty/tasty-pins',
	function( settings, name ) {

		if ( 'core/image' !== name ) {
			return settings;
		}
		settings = lodash.assign( {}, settings, {
			attributes: lodash.assign( {}, settings.attributes, {
				pinterestText: {
					attribute: 'data-pin-description',
					default: '',
					type: 'string',
					source: 'meta',
					meta: tastyPinsBlockEditor.text_key
				}
			} ),
		} );

		return settings;
	} );
// Because we share one post meta for all stored values,
// we need to keep the meta value in sync between blocks.
var descriptionValues = null;

/**
 * Register a control for editing 'data-pin-description'.
 */
hooks.addFilter(
	'editor.BlockEdit',
	'wp-tasty/tasty-pins',
	wp.compose.createHigherOrderComponent( function( BlockEdit ) {
		return function( props ) {
			if ( 'core/image' !== props.name || ! props.attributes.id ) {
				return el( BlockEdit, props );
			}

			if ( null === descriptionValues ) {
				if ( props.attributes.pinterestText ) {
					descriptionValues = JSON.parse( props.attributes.pinterestText );
				} else {
					descriptionValues = {};
				}
			}
			var descriptionValue = '';
			var valueKey = 'image_' + props.attributes.id;
			if ( typeof descriptionValues === 'object' && typeof descriptionValues[ valueKey ] !== 'undefined' ) {
				descriptionValue = descriptionValues[ valueKey ];
			}

			return el(
				wp.element.Fragment,
				{},
				el(
					wp.editor.InspectorControls,
					{},
					el(
						wp.components.PanelBody,
						{ title: 'Tasty Pins' },
						el(
							wp.components.TextareaControl,
							{
								defaultValue: descriptionValue,
								onChange: function( newValue ) {
									descriptionValues[ valueKey ] = newValue;
									props.setAttributes( {
										pinterestText: JSON.stringify( descriptionValues ),
									} );
								},
								label: 'Pinterest Text',
								placeholder: '',
							}
						)
					)
				),
				el( BlockEdit, props )
			);
		};
	} ) );

@chrisvanpatten
Copy link
Member

Thanks for leaving the code sample @danielbachhuber I have a feeling this will come in handy on some client projects.

This also relates to #10444 which suggests allowing blocks to rebuild themselves automatically if they are able to do so.

@danielbachhuber
Copy link
Member Author

I ran into an odd edge case with how I implemented this with a second register_meta() call.

register_meta(
	$post_type->name,
	self::TEXT_KEY,
	array(
		'show_in_rest' => true,
		'single'       => true,
		'type'         => 'string',
	)
);
register_meta(
	$post_type->name,
	self::NOPIN_KEY,
	array(
		'show_in_rest' => true,
		'single'       => true,
		'type'         => 'string',
	)
);

When I hit "Save draft", I'd see an "Updating failed." message:

image
image

The source of the error was rest_meta_database_error, which I tracked down to:

  1. Gutenberg sends both meta attributes, even when only one has updated.
  2. Because one of the meta attributes hasn't changed, the REST API fails with its update_metadata() call: https://github.com/WordPress/WordPress/blob/56c162fbc9867f923862f64f1b4570d885f1ff03/wp-includes/rest-api/fields/class-wp-rest-meta-fields.php#L332

My amazingly hacky solution was to set a random value on both blobs each time the value for one attribute is changed:

// Set all property attributes at the same time.
// Doing so ensures new values present in the REST request
// and avoids 'rest_meta_database_error' failure.
function propsSetAttributes() {
	descriptionValues['int_random'] = Math.random();
	noPinValues['int_random'] = Math.random();
	props.setAttributes( {
		pinterestText: JSON.stringify( descriptionValues ),
		pinterestNoPin: JSON.stringify( noPinValues ),
	} );
};

@danielbachhuber danielbachhuber added the [Feature] Media Anything that impacts the experience of managing media label Oct 26, 2018
@antpb antpb added the [Priority] High Used to indicate top priority items that need quick attention label Oct 31, 2018
@danielbachhuber
Copy link
Member Author

My workaround for this was to store my plugin data in post meta instead. There were a couple of notable downsides:

This ended up being a horrible workaround. I've instead gone back to directly modifying the block. Here's my full implementation:

const {
	CheckboxControl,
	PanelBody,
	TextControl,
	TextareaControl,
} = wp.components;

const {
	createHigherOrderComponent
} = wp.compose;

const {
	InspectorControls
} = wp.editor;

const {
	isEmpty
} = lodash;

const {
	Fragment,
	RawHTML,
	renderToString
} = wp.element;

const { hooks } = wp;

const pinAttributes = {
	pinterestText: {
		type: 'string',
		source: 'attribute',
		selector: 'img',
		attribute: 'data-pin-description',
		default: '',
	}
};

const registerAttributes = ( settings, name ) => {

	if ( 'core/image' !== name ) {
		return settings;
	}

	settings.attributes = Object.assign( settings.attributes, pinAttributes );
	return settings;
};

hooks.addFilter( 'blocks.registerBlockType', 'wp-tasty/tasty-pins', registerAttributes );

const registerFields = createHigherOrderComponent( ( BlockEdit ) => {
	return ( props ) => {
		if ( 'core/image' !== props.name || ! props.attributes.id ) {
			return (
				<BlockEdit {...props} />
			);
		}

		const {
			attributes,
			setAttributes
		} = props;

		const {
			pinterestText
		} = attributes;

		return (
			<Fragment>
				<InspectorControls>
					<PanelBody title={ 'Tasty Pins' }>
						<TextareaControl
							defaultValue={ pinterestText }
							onChange={( value ) => setAttributes({ pinterestText: value })}
							label={ 'Pinterest Text' }
							placeholder={ '' }
							/>
					</PanelBody>
				</InspectorControls>
				<BlockEdit { ...props } />
			</Fragment>
		);
	};
} );

hooks.addFilter( 'editor.BlockEdit', 'wp-tasty/tasty-pins', registerFields );
const saveAttributes = ( element, blockType, attributes ) => {
	if ( 'core/image' !== blockType.name ) {
		return element;
	}

	const {
		pinterestText
	} = attributes;

	const imageProps = [];
	if ( ! isEmpty( pinterestText ) ) {
		imageProps.push({
			attribute: 'data-pin-description',
			value: pinterestText
		});
	}

	// Don't need to modify when all attributes are empty.
	if ( isEmpty( imageProps ) ) {
		return element;
	}
	let elementAsString = renderToString( element );
	imageProps.forEach(({ attribute, value }) => {
		elementAsString = elementAsString.replace( '<img ', `<img ${attribute}="${value}" ` );
	});

	return (
		<RawHTML>{elementAsString}</RawHTML>
	);
};

hooks.addFilter( 'blocks.getSaveElement', 'wp-tasty/tasty-pins', saveAttributes );

@aubirdgine
Copy link

@danielbachhuber I'm having the exact same issue with Gutenberg pulling out the data-pin-description HTML that Tasty Pins used to add in the classic editor! Except I am a total coding novice... Is it simple enough to implement the fix you have here (simple enough that I can do it through FTP or cPanel with basic copying and pasting) or am I just way too optimistic? I'd love to be able to have the data-pin-description code in my HTML still, but I'm pretty wary about actually modifying code cuz I'd prefer not to break my site!

@gziolo gziolo added this to To Do in Extensibility Nov 17, 2018
@mtias
Copy link
Member

mtias commented Nov 20, 2018

Let's continue to improve handling as outlined in #11440. The current state is our best tradeoff — things can be extended, but if the plugin code is missing a block might become invalid. Which is correct in the sense that the block wouldn't know how to further handle the extra attributes and code present in the block.

This is important for other editor cases, like mobile, where handling attributes might not be as straightforward as preserving a set of HTML attributes.

That said, work on allowing unknown attributes to carry on without blocking it has already begun, and we can continue refining the details of the implementation.

@florianbrinkmann
Copy link

I have the same issue that @danielbachhuber described in his comment (#10204 (comment)), I get a rest_meta_database_error error after trying to update a post which updates multiple meta values. Changing the problematic field lets me save the post, but that is a not so great workaroud.

Is that something that can/should be fixed in Gutenberg?

@vreemt
Copy link

vreemt commented Jan 20, 2023

related #28923

@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Aug 1, 2023
@carstingaxion
Copy link
Contributor

I just stumbled upon rest_meta_database_error with the same situation like described by @danielbachhuber & @florianbrinkmann , while working with the Block Bindings API. Having this bug while only using core-functions surprised me a little, as I would have thought, someone else might have mentioned this before.

This 3 year old ticket seems related: - #53942 (Non-single post meta multiple value update breaks post save through REST API) – WordPress Trac

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Feature] Block API API that allows to express the block paradigm. [Feature] Block Transforms Block transforms from one block to another [Feature] Blocks Overall functionality of blocks [Feature] Extensibility The ability to extend blocks or the editing experience [Feature] Media Anything that impacts the experience of managing media [Type] Enhancement A suggestion for improvement.
Projects
No open projects
Extensibility
  
To Do
Development

No branches or pull requests

9 participants