search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

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

Open artemiomorales opened 1 month ago

artemiomorales commented 1 month ago

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: ```php 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

https://github.com/user-attachments/assets/ebd699e9-864f-4225-b14d-6824a0d2dc58

Environment info

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

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

artemiomorales commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

@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.

https://github.com/user-attachments/assets/fa50a899-fb8a-4dd5-82b6-bccc1475920b

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.

https://github.com/user-attachments/assets/582a2bbe-3751-41c7-8f55-621184020386

cbravobernal commented 1 month ago

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 commented 1 month ago

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 https://github.com/WordPress/gutenberg/pull/64072. It has a clear way to modify the values:

https://github.com/user-attachments/assets/42794ba8-c820-4f94-a8ed-18438ef4ca06

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.

gziolo commented 1 month ago

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.