WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 10 months ago

Last modified 10 months ago

#41629 closed enhancement (fixed)

Widgets: Default to "custom URL" in the image widget

Reported by: melchoyce Owned by: desrosj
Milestone: 4.9 Priority: high
Severity: normal Version: 4.8
Component: Media Keywords: has-patch needs-testing
Focuses: ui Cc:

Description

To help improve discoverability of custom URLs in the image widget, let's default to showing them rather than Link to: none.

See before/after screenshots.

Attachments (5)

before.png (1.9 MB) - added by melchoyce 11 months ago.
after.png (1.9 MB) - added by melchoyce 11 months ago.
41629.patch (566 bytes) - added by bor0 10 months ago.
41629.2.patch (2.1 KB) - added by desrosj 10 months ago.
41629.diff (2.0 KB) - added by desrosj 10 months ago.
Remove addition of new option and hardcode custom as the default for widgets instead.

Change History (27)

@melchoyce
11 months ago

@melchoyce
11 months ago

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.


11 months ago

#2 follow-up: @joemcgill
11 months ago

  • Keywords reporter-feedback added

@melchoyce do we only want this as the default in the image widget, or across all instances of the media modal, i.e., in the post edit screen as well?

#3 @joemcgill
11 months ago

No sooner did I ask that question than did @desrosj come across #31467 (Images should default to not linking).

#4 in reply to: ↑ 2 @melchoyce
11 months ago

Replying to joemcgill:

@melchoyce do we only want this as the default in the image widget, or across all instances of the media modal, i.e., in the post edit screen as well?

Only the image widget.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


11 months ago

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


10 months ago

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


10 months ago

#8 follow-up: @philmcdonnell
10 months ago

What would be the case if the user didn't enter anything and just submitted the form? The http:// that is there is just a placeholder so it would not save I assume?

#9 in reply to: ↑ 8 ; follow-up: @joemcgill
10 months ago

Replying to philmcdonnell:

What would be the case if the user didn't enter anything and just submitted the form? The http:// that is there is just a placeholder so it would not save I assume?

The behavior should work just like it currently does if the form is submitted while custom URL is selected and the URL field is left blank. The image is inserted with no link at all.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


10 months ago

#11 in reply to: ↑ 9 ; follow-up: @philmcdonnell
10 months ago

Replying to joemcgill:

The behavior should work just like it currently does if the form is submitted while custom URL is selected and the URL field is left blank. The image is inserted with no link at all.

Thanks! That is what I thought, just making sure. So it is trivial to make this work, however there are two links withing the image widget, one is replace image which uses the same code as the original insert image so that would work as expected. There is another link (button) that says edit image, what would the expected behavior be there? If there is no link currently on the inserted image it would just default to 'none' or would that want to default to custom url as well? I would think editing an existing should be set to none if that is what was there originally. Thoughts?

#12 in reply to: ↑ 11 @joemcgill
10 months ago

  • Owner set to desrosj
  • Status changed from new to assigned

Replying to philmcdonnell:

If there is no link currently on the inserted image it would just default to 'none' or would that want to default to custom url as well? I would think editing an existing should be set to none if that is what was there originally. Thoughts?

I think your suggestion makes sense. Additionally, if a site has the image_default_link_type option set, we should probably respect it. So this would only apply as a default when that option is not set.

#13 @joemcgill
10 months ago

  • Keywords needs-patch added; reporter-feedback removed

Also, I'm assigning to @desrosj for further review, but @philmcdonnell feel free to add a patch if you have an approach you think will work. Thanks!

#14 @westonruter
10 months ago

  • Priority changed from normal to high

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


10 months ago

@bor0
10 months ago

#16 @bor0
10 months ago

  • Keywords has-patch added; needs-patch removed

The proposed patch defaults to Link To: Custom URL: http://. If no URL changes are done, on every subsequent edit it will show as Link To: None.

However, if we add http://www.wordpress.org initially as the Custom URL, then it will show as Link To: Custom URL: http://www.wordpress.org, which is as expected.

@desrosj
10 months ago

#17 @desrosj
10 months ago

  • Keywords needs-testing added
  • Version set to 4.8

I tested @bor0's patch. This tackles the situation when editing widgets in the admin, but not in the Customizer

41629.2.patch is an extension of that change that also changes the behavior through the Customizer.

Previously, the default image_default_link_type option was not being respected when selecting an image for an image widget in the Customizer. 41629.2.patch now respects the default option, but introduces a new option specifically for the image widget, image_default_widget_link_type. This will be custom when not present in the database.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


10 months ago

@desrosj
10 months ago

Remove addition of new option and hardcode custom as the default for widgets instead.

#19 @desrosj
10 months ago

As discussed in the component's weekly meeting, 41629.diff removes the addition of a new option in favor of hardcoding custom as the default for image widgets.

#20 @desrosj
10 months ago

@joemcgill tested the original patch and said that it was working for him. I gave 41629.patch another go and it worked correctly for me as well. Not sure why I did not see the correct behavior when testing initially but can confirm that 41629.patch works for me now.

#21 @joemcgill
10 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 41696:

Widgets: Default to "custom URL" in the image widget.

This changes the default value for link_type in the image widget
schema to 'custom'.

Props bor0, desrosj.
Fixes #41629.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


10 months ago

Note: See TracTickets for help on using tickets.