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

Block Bindings: Post metadata persists after connected block is removed #65683

Open
2 tasks done
artemiomorales opened this issue Sep 26, 2024 · 6 comments
Open
2 tasks done
Labels
[Feature] Block bindings Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended

Comments

@artemiomorales
Copy link
Contributor

artemiomorales commented Sep 26, 2024

Description

When metadata is created via blocks bound to custom attributes, the metadata persists after the block is removed. This can result in unexpected behavior, such as that metadata continuing to erroneously render in a template.

This discussion also touches on overall Block Bindings UX, and may have implications for how we model content with bindings in the post editor, and potentially the site editor.

Step-by-step reproduction instructions

1. Register some custom post meta. For an easy example, here's code to register a custom post type with related meta:
function register_video_post_type(): void {
	$args = array(
		'label'               => __( 'Videos', 'video-features' ),
		'description'         => __( 'Videos catalogue', 'video-features' ),
		'supports'            => array(
			'title',
			'editor',
			'author',
			'thumbnail',
			'revisions',
			'custom-fields',
		),
		'hierarchical'        => false,
		'public'              => true,
		'show_ui'             => true,
		'show_in_rest'        => true,
		'show_in_menu'        => true,
		'show_in_nav_menus'   => true,
		'show_in_admin_bar'   => true,
		'menu_position'       => 5,
		'can_export'          => true,
		'has_archive'         => true,
		'exclude_from_search' => false,
		'publicly_queryable'  => true,
		'capability_type'     => 'post',
		'menu_icon'           => 'dashicons-format-video',
	);
	register_post_type( 'video', $args );

	register_post_meta(
		'video',
		'director',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'default'           => '[Director here]',
		)
	);

	register_post_meta(
		'video',
		'genre',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'default'           => '[Genre here]',
		)
	);

	register_post_meta(
		'video',
		'length',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'default'           => '[Length here]',
		)
	);

	register_post_meta(
		'video',
		'trailer_url',
		array(
			'show_in_rest'      => true,
			'single'            => true,
			'type'              => 'string',
			'default'           => '',
			'sanitize_callback' => 'sanitize_text_field',
			'revisions_enabled' => true,
		)
	);
}
add_action( 'init', 'register_video_post_type' );
  1. In a template for the post type, add a paragraph and bind it to one of the meta fields.
  2. Create a new post.
  3. In the post, add a paragraph and bind it to the same meta field as in step 2, and override the value.
  4. Delete the paragraph.
  5. View the post.
  6. See that the metadata is still visible.

Screenshots, screen recording, code snippet

removing-metadata.mp4

Environment info

  • Gutenberg 19.3 (trunk)
  • WP 6.6.2

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes
@artemiomorales artemiomorales added [Type] Bug An existing feature does not function as intended [Feature] Block bindings labels Sep 26, 2024
@artemiomorales
Copy link
Contributor Author

@cbravobernal I made an issue for this persisting metadata issue you raised at WCUS. Was there anyone else in particular who mentioned this on your end?

On my end, this was brought up to me by at least two other people at WCUS, who I've pinged for feedback on their use cases.

It may be good to loop in additional folks to see how they're modeling content with bindings and how this issue shows up for them.

@SantosGuillamot
Copy link
Contributor

Thanks for opening the issue. It'd be great to discuss what the expected behavior is. I see three main options: revert the changes made to post meta, set an empty value for the meta field, or keep the changes made (as it works right now).

If I am not mistaken, other blocks like Post Title or Post Excerpt work the same way. You can insert a post excerpt, edit it, remove it, and the changes to post excerpt will prevail if you save the post. Whatever is decided I guess should apply to any of those scenarios.

@cbravobernal
Copy link
Contributor

cbravobernal commented Sep 26, 2024

@cbravobernal I made an issue for this persisting metadata issue you raised at WCUS. Was there anyone else in particular who mentioned this on your end?

On my end, this was brought up to me by at least two other people at WCUS, who I've pinged for feedback on their use cases.

It may be good to loop in additional folks to see how they're modeling content with bindings and how this issue shows up for them.

No one else reported to me, but it is true that is tricky. One problem for example is that more than one block in the same post can be bound to the same field.

Screen.Recording.2024-09-26.at.18.53.18.mov

Also, if you have a custom template with your layout using block bindings, there is not an easy way to edit their values without adding and removing paragraphs. As there is no option to edit the content inside the template, there is also no option to edit the values of the post meta data without adding a paragraph bound to it, and you may not need that paragraph.

Screen.Recording.2024-09-26.at.19.13.32.mov

@cbravobernal
Copy link
Contributor

cbravobernal commented Sep 26, 2024

revert the changes made to post meta

This seems the most secure option, but we will need to save the previous value somewhat and restore it on block deletion or block attribute reset.

set an empty value for the meta field

This could break another blocks pointing to that field.

Keep the changes made (as it works right now).

This one causes confusion in the workflow. But also prevents error on different blocks with the same field bound to.

@artemiomorales
Copy link
Contributor Author

Thanks for opening the issue. It'd be great to discuss what the expected behavior is. I see three main options: revert the changes made to post meta, set an empty value for the meta field, or keep the changes made (as it works right now).

There's a fourth option. We could show a popup and ask the user something along these lines: "This block has overridden a meta field. Would you like to reset it?"

However, this may be too complicated and could introduce inconsistencies in the UX if we decide to allow connections to other default post meta in the future, such as title, publish date, etc.

Also, if you have a custom template with your layout using block bindings, there is not an easy way to edit their values without adding and removing paragraphs. As there is no option to edit the content inside the template, there is also no option to edit the values of the post meta data without adding a paragraph bound to it, and you may not need that paragraph.

Yeah I believe this discussion may have broader implications for the UX of content modeling with block bindings. The use case @cbravobernal surfaces in that video is one that I hadn't even considered before he showed it to me at WCUS.

Here's a video reviewing what I thought was the primary use case while we were reviewing #64072. It has a clear way to modify the values:

custom-post-types-trimmed.mp4

It'd be great to hear on real-world use cases to get more information regarding how we could approach this, and what considerations to the UX may be in order.

@cbravobernal cbravobernal added the Needs Design Feedback Needs general design feedback. label Sep 26, 2024
@gziolo
Copy link
Member

gziolo commented Oct 2, 2024

I’m not sure I understand the severity of the problem discussed. All scenarios covered work as expected. When you publish the post, all connected sources get their values stored on the server if configured similarly to the Post Meta source. The fact that the block with a connected source gets removed from the content should not impact the value of an external source. The same behavior could be observed when you change the binding source for the block. It also isn't different from how core blocks work like Post Title, Post Date, Post Featured Image, etc.

For me it's a similar problem to what is tracked in these issues:

It's a recurring question whether to surface the detailed breakdown of all modified external sources when publishing a post similar to what exists in the site editor. If that would exist then it would be up to the user to decide what to do about the changes made through the connected Paragraph block that got removed afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block bindings Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

4 participants