Include an example of blocks.getSaveElement use

[danielbachhuber]

No description provided.

[aduth]

This sort of usage (and in fact, the filter itself) may be something to discourage, as if the plugin is ever to later to be deactivated, the user will be presented with the "Modified Externally" warning, since the attribute would be destroyed on subsequent re-saves.

At minimum, we should be very clear about this caution, or avoid documenting getSaveElement altogether.

[danielbachhuber]

This sort of usage (and in fact, the filter itself) may be something to discourage, as if the plugin is ever to later to be deactivated, the user will be presented with the "Modified Externally" warning, since the attribute would be destroyed on subsequent re-saves.

Related #6878 (comment)

At minimum, we should be very clear about this caution, or avoid documenting getSaveElement altogether.

Updated in f2f1c56

[aduth]

• This should probably be using cloneElement to override props
• Deep object assignment can be improved by using _.merge instead

[danielbachhuber]

Can you provide an example?

[aduth]

// Bad:
return _.assign( {}, element, { myprop: 'foo' } );

// Good:
return wp.element.cloneElement( element, { myprop: 'foo' } );

[aduth]

This will not assign data-custom-attribute when the image is assigned a URL. The deep override should be a signal of its fragility.

[danielbachhuber]

I don't follow this either. Can you explain further?

[aduth]

The markup shape changes depending on whether the image block has a url attribute assigned.

Aside: It's maybe telling that this is an easy thing to overlook.

[danielbachhuber]

I probably won't have time to pick this back up this week, so assigning to myself to clean up early next.

[aduth]

Are you able to resume work here?

[danielbachhuber]

Are you able to resume work here?

I haven't been able to come back to this yet. The project work is still on my todo list though, so I'll pick up the documentation when I'm back on the project.