WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#14390 closed defect (bug) (wontfix)

post image (thumbnail) in [gallery]

Reported by: thomask Owned by: nkuttler
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Post Thumbnails Keywords: has-patch dev-feedback, 2nd-opinion
Focuses: Cc:

Description

the new (featured) post image (thumbnail) function is great, but it got one big problem - the featured image become part of the [gallery], so if you want to use featured image for posts AND gallery (e.g. because you are using special size of featured image or just simply special image e.g. for category pages), you can't use it.
And AFAIK it is not possible to even filter it out, there is no way to use something like _thumbnail_id IS NULL in the gallery shortcode or get_children or even get_posts function

Attachments (2)

exclude_post_thumbnail_from_gallery_shortcode.diff (688 bytes) - added by nkuttler 5 years ago.
14390.diff (380 bytes) - added by coffee2code 5 years ago.
Alternative approach.

Download all attachments as: .zip

Change History (17)

comment:1 @nkuttler5 years ago

  • Keywords has-patch dev-feedback added
  • Owner set to nkuttler
  • Status changed from new to accepted

This is indeed a bug, and a bad one. I think the post thumbnail should always be excluded. This changes the current behaviour.

I'd be happy to add in a parameter to add the post thumbnail to the gallery. Or keep the thumbnail in the gallery by default, but make it possible to exclude it easily (even if I think that would be the Wrong Thing to do).

comment:2 @nkuttler5 years ago

  • Keywords ui-feedback added

comment:3 follow-ups: @coffee2code5 years ago

I've attached 14390.diff with an alternative approach. It simply sets the get_post_thumbnail_id() value as the shortcode's default 'exclude' value.

The difference between @nkuttler's approach and this one is that @nkuttler's approach always excludes the post's thumbnail image whereas 14390.diff only does so by default, but if the user specifies anything via the shortcode's "exclude" attribute, then the post thumbnail image is not excluded unless the user does so explicitly.

My approach presumes that if the user is excluding at least one image, they will know enough to be able to exclude the post thumbnail as well. And if for some reason they'd like to include the post thumbnail image as part of the gallery, they can still do so.

With the previous patch there would be no way to include the post thumbnail in the gallery (as @nkuttler also noted).

This actually prompted me to work up a patch to allow filtering shortcode_atts() so that this could be done in a better, more customizable fashion (and not just for this shortcode). See #15155 for an example.

@coffee2code5 years ago

Alternative approach.

comment:4 in reply to: ↑ 3 @nkuttler5 years ago

  • Keywords 2nd-opinion added

Replying to coffee2code:

I've attached 14390.diff with an alternative approach. It simply sets the get_post_thumbnail_id() value as the shortcode's default 'exclude' value.

This approach is certainly better, but I'm not sure about the additional complexity it introduces for end-users. I think that a new parameter that either defaults to include or exclude post thumbnail would be better suited.

comment:5 @coffee2code5 years ago

Agreed that a newly introduced boolean attribute (such as 'include_thumbnail') would be the most user-friendly. We'd have to decide on precedence though. Would 'include_thumbnail' always take precendence if defined even if the thumbnail id appeared in an 'include'/'exclude'? (i.e. 'include_thumbnail' => true, 'exclude' => $thumbnail_id). Or would it be vice-versa?

comment:6 @nkuttler5 years ago

I'd really like some committer feedback on this one, if this is even perceived as a bug, as the problem could probably be fixed through a plugin as well if there were some filter in place.

How about adding a new "featured" attribute to the include and exclude parameters? It would simply be translated into the attachment ID. I think this would offer a user-friendly way to include or exclude the featured image, on a per-post basis. As for the default, well, dev feedback?

comment:7 @nacin4 years ago

  • Keywords changed from has-patch, dev-feedback, ui-feedback, 2nd-opinion to has-patch dev-feedback, ui-feedback, 2nd-opinion
  • Milestone changed from Awaiting Review to 3.1

Not sure how best to handle this but I agree we need a fix.

comment:8 @nkuttler4 years ago

So.. should we just propose more patches? Not sure about the recommended workflow..

comment:9 @JohnONolan4 years ago

  • Keywords ui-feedback removed

comment:10 @idealien4 years ago

I agree that this is something that should be addressed.

Of the options discussed, I would be most in favour of a boolean attribute (such as 'include_thumbnail'). Regarding precedence, since include / exclude references specific object IDs I would think it to take precedence over this new boolean.

Adding this selection into the gallery media screen - include featured image (yes / no) - might open up more questions that are beyond scope of 3.1. Should there be an 'exclude from gallery' checkbox beside each attached image in the gallery? Why is it mandatory that because an image is attached to a post that it be a part of the gallery? etc.

comment:11 @idealien4 years ago

  • Cc idealien added

comment:12 @thee174 years ago

What if we added an option that is stored in the postmeta for the image "exclude from gallery". Perhaps you may want to post something in the post but not part of the gallery too.

comment:13 @ryan4 years ago

  • Milestone changed from 3.1 to Future Release

comment:14 in reply to: ↑ 3 @nkuttler4 years ago

  • Resolution set to wontfix
  • Status changed from accepted to closed

Replying to coffee2code:

This actually prompted me to work up a patch to allow filtering shortcode_atts() so that this could be done in a better, more customizable fashion (and not just for this shortcode). See #15155 for an example.

Yes, much better. I'm setting this to wontifx, I don't think the current default behavior should change, and adding a filter looks much more useful.

comment:15 @nacin4 years ago

  • Milestone Future Release deleted
Note: See TracTickets for help on using tickets.