WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 4 months ago

#31467 closed enhancement (fixed)

Images should default to not linking

Reported by: melchoyce Owned by: wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: Media Keywords: make-flow has-patch
Focuses: ui Cc:

Description

For the reasons discussed in this Slack conversation: https://wordpress.slack.com/archives/core-flow/p1424973816000003

In conversation with developers and users there is a general feeling of frustration about the default "link to" for images and image galleries being set to Attachment Page. Users generally don't understand what this is and few theme developers bother to build a template for it — @mor10

I would LOVE if the default link for images was none. That confuses my students every time. They add a picture and have no idea why it's linking to the Attachment page and what an attachment page even is. I think linking should take action, not unlinking — @liljimmi

same here. also there is an issue where i see people naming an image and a post the exact same slug and this shows the attachment page of the image instead of the post they were expecting. — @andrea_r

I agree — changing the default to not link is the expected behavior for the vast majority of people. Let's turn off linking by default.

Attachments (4)

31467.patch (1.2 KB) - added by liljimmi 3 years ago.
Sets image_default_link_type to none on install. Removed "selected" from file option field and added it to none.
31467-2.patch (1.1 KB) - added by janhenckens 3 years ago.
Puts "none" in first place in the dropdown, making it the default for images
31467-3.patch (1.9 KB) - added by eherman24 3 years ago.
Sets image_default_link_type to none on install. Removed "selected" from file option field and added it to none. Puts "none" in first place in the dropdown, making it the default for images. Introduces new filter to allow override of image_default_link_type.
31467.diff (2.8 KB) - added by wonderboymusic 2 years ago.

Download all attachments as: .zip

Change History (35)

This ticket was mentioned in Slack in #core-flow by melchoyce. View the logs.


3 years ago

#2 @melchoyce
3 years ago

  • Keywords good-first-bug added

#3 @melchoyce
3 years ago

  • Keywords make-flow added

#4 @joedolson
3 years ago

Yes, yes, yes. That would be fabulous.

@liljimmi
3 years ago

Sets image_default_link_type to none on install. Removed "selected" from file option field and added it to none.

#5 @liljimmi
3 years ago

I couldn't figure out how to bump "None" up to the first option on the drop-down because of all the conditionals, but this sets none as the default after a new install.

@janhenckens
3 years ago

Puts "none" in first place in the dropdown, making it the default for images

#6 @janhenckens
3 years ago

  • Keywords has-patch needs-testing added

My patch bumps the "None" up to the top of the dropdown if the file in question is an image, fixing this for existing installs.

#7 @ryan
3 years ago

  • Keywords has-patch needs-testing removed

Agreed. Linking to the attachment page (as well as linking to bare images) results in confusion. It's particularly cumbersome on mobile devices. As mentioned in the chat, we should consider baking something along the line's of wp.com/Jetpack's Carousel into core. We should be able to swipe through galleries on mobile out of the box. Access to full-width images should be straightforward. Diving in and out of attachment pages or bare images is not a good experience. This ticket seems a good step 1.

Last edited 3 years ago by ryan (previous) (diff)

#8 @SergeyBiryukov
3 years ago

also there is an issue where i see people naming an image and a post the exact same slug and this shows the attachment page of the image instead of the post they were expecting.

Related: #24612

#9 @helen
3 years ago

Suggest taking a look at the image_default_link_type option. There's more than meets the eye here. :)

#10 @krogsgard
3 years ago

I don't disagree with the ticket and agree with @ryan that it's a great first step. But perhaps we should look at the bigger picture (pun intended!).

I posted on the Make Flow blog about support for a new option (and perhaps default) entirely.
https://make.wordpress.org/flow/2015/02/26/core-support-for-wordpress-images-to-open-in-a-modal-window/

This ticket was mentioned in Slack in #core-flow by drew. View the logs.


3 years ago

#12 @DrewAPicture
3 years ago

  • Keywords needs-patch added
  • Owner set to janhenckens
  • Status changed from new to assigned

See comment:9 for a suggestion on how to approach a new patch.

#13 @mor10
3 years ago

Yes to all of this. I have nothing useful to add other than that.

@eherman24
3 years ago

Sets image_default_link_type to none on install. Removed "selected" from file option field and added it to none. Puts "none" in first place in the dropdown, making it the default for images. Introduces new filter to allow override of image_default_link_type.

This ticket was mentioned in Slack in #feature-imageflow by mor10. View the logs.


3 years ago

#15 @DrewAPicture
3 years ago

  • Keywords has-patch added; needs-patch removed

@eherman24: I think introducing the new filter falls out of scope of this ticket.

#16 @helen
3 years ago

Doesn't need a new filter, all options have filters.

#17 @ryan
3 years ago

There's also a reference to image_default_link_type in wp-admin/includes/media.php.

wp-admin/includes/media.php
1182:			'html'       => image_link_input_fields($post, get_option('image_default_link_type')),

Link To in the Image Details modal may also need to be reordered to put None at the top.

Is the image_default_link_type option exposed in UI anywhere?

In the image modals, the last Size used is remembered. Should the last Link To used also be remembered?

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


3 years ago

#19 @johnbillion
3 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 4.2 to Future Release

Patch needs updating to incorporate the image details modal as per Ryan's comment.

#20 @DrewAPicture
3 years ago

  • Keywords has-patch added; needs-patch removed

The latest patch still includes the filter which is out of scope. Basically comment:9 and comment:17 sort of outline the pitfalls of this UI change because there's an option and a user setting in play and we have to make sure we don't change an existing expectation. Punting to a future release.

#21 @melchoyce
3 years ago

Let's get some movement back in this ticket. What needs to happen next?

#22 @janhenckens
3 years ago

Whoops, I lost track of this one! I'll add a new patch that includes @ryan's remark and report back here.

#23 @wonderboymusic
2 years ago

  • Keywords needs-refresh added
  • Milestone changed from Future Release to 4.4
  • Owner janhenckens deleted

This ticket was mentioned in Slack in #design by melchoyce. View the logs.


2 years ago

#25 @wonderboymusic
2 years ago

  • Owner set to wonderboymusic

@wonderboymusic
2 years ago

#26 @wonderboymusic
2 years ago

  • Keywords good-first-bug needs-refresh removed

31467.diff makes this work - the user setting will override the default prop. Not sure why these were switched previously

#27 follow-up: @azaozz
2 years ago

Looking at 31467.diff, there is no point in doing

getUserSetting( 'align', 'none' ) || defaultProps.align

getUserSetting() takes two args: the setting's name and default value. If it's not set, it will return the default value, and defaultProps.align can never be reached.

If we still want to use defaultProps.align as default value, can do:

getUserSetting( 'align', defaultProps.align ) || 'none'

(the || 'none' bit ensures we have a setting in case defaultProps.align is not set.)

Last edited 2 years ago by azaozz (previous) (diff)

#28 in reply to: ↑ 27 @wonderboymusic
2 years ago

Replying to azaozz:

If we still want to use defaultProps.align as default value, can do:

getUserSetting( 'align', defaultProps.align ) || 'none'

(the || 'none' bit ensures we have a setting in case defaultProps.align is not set.)

Yeah, that makes sense

#29 @wonderboymusic
2 years ago

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

In 33729:

Media:

When inserting an image into a post, the values in wp.media.controller.Library should not default to linking the image when no user settings are present.

The default display setting value for link is now none. User settings persist and will override or confirm this value based on user actions.

Props liljimmi, janhenckens, eherman24, wonderboymusic.
Fixes #31467.

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


2 years ago

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


4 months ago

Note: See TracTickets for help on using tickets.