Update target link in core/image when an image is selected

[SeanDS]

Closes #13749.

Description

The image block currently assumes that the default media target is always none, so it doesn't update the target link when the user selects an image; it only does this when the user explicitly changes the target using the drop-down box in the sidebar (using the onSetLinkDestination callback). This means plugins cannot control the default link target by simply setting linkDestination.default to something other than none: the drop-down box will show the new default, but the target link will not be properly set when an image is chosen/uploaded (it will still be blank, and the saved post will not therefore link to e.g. the media or attachment page).

This patch simply runs the callback onSetLinkDestination, which is already called when a new target selection is made by the user explicitly, when an image is uploaded or selected from the media manager. This ensures that if a plugin changes the default link target, the URL for that target is properly set when the image is selected.

How has this been tested?

I used the latest development release of Gutenberg (cloned at a59aaf2), and WordPress 5.0.3, using VVV. I created a plugin to change the default link target to media:

function modifyImageLinkDestinationDefault( settings, name ) {
    // Link target to override core default.
    var target = "media";

    if ( name == "core/image" ) {
        // Set image block default destination.
        settings.attributes.linkDestination.default = target;
    } else if ( name == "core/gallery" ) {
        // Set gallery block default destination.
        settings.attributes.linkTo.default = target;
    }

    return settings;
}

// Add settings filter to block registration.
wp.hooks.addFilter(
    "blocks.registerBlockType",
    "my-plugin/modify-image-link-destination-default",
    modifyImageLinkDestinationDefault
);

and enqueued it on enqueue_block_editor_assets using:

wp_enqueue_script(
	'modify-image-link-destination-default',
	'/path/to/plugin/js/index.js',
	array(
		'wp-hooks',
	)
);

Screenshots

Default behaviour. My plugin changes the default target, but the link URL is not properly set when I select the image:

Patch applied, the link URL is properly set when an image is selected:

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.

[talldan]

Hey @SeanDS, thanks for the contribution, I've left a comment about the consecutive calls to setAttributes.

[SeanDS]

@talldan, thanks for the pointers. I've updated the code in 7e22653 and tested it and it seems to work.

One thing I wasn't 100% sure about was how I deal with situations when destination is not one of the defined constants (i.e. the else block in buildLinkAttributes. I've left it as using this.props.attributes.href like before. I guess since this property isn't derived from the selected image, this is fine?

[SeanDS]

Is this good to go? Or any changes needed?

[talldan]

@SeanDS Sorry for the delay, will give this another review and test.

One thing I wasn't 100% sure about was how I deal with situations when destination is not one of the defined constants (i.e. the else block in buildLinkAttributes. I've left it as using this.props.attributes.href like before. I guess since this property isn't derived from the selected image, this is fine?

I think the aim there is to leave the href unchanged if the link destination is a custom one or an unknown value. It shouldn't ever be an unknown value, so this mostly seems to be handling custom urls.

[talldan]

I spotted an issue in testing - the Link To 'Attachment' option no longer works. Left a comment mentioning where the issue is in the code.

Also in testing I noticed a few things about the general approach of using a plugin to change attribute defaults in this way:

• Existing image blocks (added before the plugin was activated) end up with the new default linkDestination applied, but the href isn't set.
• If such a plugin is disabled, the linkDestination reverts to a default of 'None', but image blocks added while the plugin was active still have an href value.

Not sure what the best way to solve those issues is. 🤔

[talldan]

The image block doesn't have link or source_url attributes. It looks like these should be this.props.image.link and this.props.image.source_url

[SeanDS]

Hmm, I get an this.props.image is null error. I think this is again a case of the information not yet being available to set the link target like before, and it'll probably need another hack to get it right.

I'm starting to think this is a lot more work than it's worth. It's unfortunate that this default behaviour cannot be changed easily, because I believe having an image link to the larger version is a sane default that many users will want (indeed, it's the default in the classic editor, so it's what they'd also probably expect).

I might look into this again at some point but I can't dedicate much more time to fixing this right now. If someone else wants to have a go, please feel free!

[jesusangel]

I agree with you. I've come to this pull request while I was searching for a way to change the default behaviour of the Gutenberg image block.

[jesusangel]

Hi,

I've made a pull request (#14398) that fixes @talldan comment:

Also in testing I noticed a few things about the general approach of using a plugin to change attribute defaults in this way:

• Existing image blocks (added before the plugin was activated) end up with the new default linkDestination applied, but the href isn't set.
• If such a plugin is disabled, the linkDestination reverts to a default of 'None', but image blocks added while the plugin was active still have an href value.

I've added some code in ComponentDidMount method to figure out which is the right value for the LinkDestination attribute. For example, supose that you have changed the default value for LinkDestination from none to media and you edit some old post with some images in it. If the block markup don't have a value for the LinkDestination, then that's because the default value was "none", so the image isn't linked to any URL, but now the default value is "media". My code checks that the image hasn't got an anchor (<a href=""><img /></a>) and sets its LinkDestination value to none.

Hope this helps.

Kind regards,

[okainov]

Any update on this one? Really would like to see it in!

[youknowriad]

Hi There! I've triaging PRs today and it seems this one is stale. I'm going to close now but please consider reopening if you have some time to dedicate to it. Thanks for your efforts.

[SeanDS]

@youknowriad I think this was invalidated by the changes to the image block anyway, namely how the URL is picked via a new popup instead of the sidebar. I'm still interested in getting this to work though, as it's still annoying that the default URL target is still not possible to set.

[thanh25896]

When I added your code to function.php I got error : "Parse error: syntax error, unexpected ',', expecting variable (T_VARIABLE) "

I used lastest gutenberg plugin