Make WordPress Core

Opened 6 years ago

Closed 4 months ago

Last modified 3 months ago

#41574 closed enhancement (fixed)

Add filter: wp_get_attachment_link_attributes

Reported by: nathanatmoz's profile NathanAtmoz Owned by: audrasjb's profile audrasjb
Milestone: 6.2 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch has-unit-tests commit needs-dev-note
Focuses: Cc:

Description

Similar to wp_get_attachment_image_attributes, it would be nice to be able to filter the link attributes when getting the attachment link.

Attachments (2)

41574.1.diff (1.5 KB) - added by NathanAtmoz 6 years ago.
41574.2.diff (1.5 KB) - added by NathanAtmoz 6 years ago.
add missing >

Download all attachments as: .zip

Change History (18)

@NathanAtmoz
6 years ago

#1 @SergeyBiryukov
6 years ago

  • Component changed from General to Media
  • Keywords has-patch added

@NathanAtmoz
6 years ago

add missing >

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


4 years ago

#3 @aaroncampbell
4 years ago

  • Keywords 2nd-opinion added

Overall I like the idea here. Being able to add classes and such to the <a> element would be helpful, and with the download attribute gaining browser support, I can see that being useful too.

I do have a small concern here though: The $attr that’s passed into wp_get_attachment_link() currently just passes through to wp_get_attachment_image(). That’s going to be really weird once the links support attributes themselves.

Does anyone have any thoughts on what can be done there to keep back compat and ease confusion? Maybe rename that parameter? Do we need to add yet ANOTHER parameter there for consistency?

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


7 months ago

#5 @antpb
7 months ago

  • Milestone changed from Awaiting Review to 6.2

Moving this to 6.2. The changes proposed seem safe and the concerns above seem to be okay. Will investigate.

#6 @antpb
5 months ago

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

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


4 months ago

This ticket was mentioned in PR #3922 on WordPress/wordpress-develop by @costdev.


4 months ago
#8

  • Keywords has-unit-tests added

This PR refreshes patch: 41574.2.diff to:

  • Fix CS issues.
  • Backward compatibility: Use single quotes for attribute values.
  • Correct @since annotation and remove an empty line from the docblock.
  • Add tests.

#9 @costdev
4 months ago

@antpb If you get a chance, could you review PR 3922 and see if it's appropriate for commit?

#10 @antpb
4 months ago

  • Keywords commit added

@costdev this looks great! Thank you for adding tests there. Marking commit.

#11 @audrasjb
4 months ago

  • Owner changed from antpb to audrasjb
  • Status changed from assigned to accepted

Self assigning as WP 6.2 beta 1 is due in a couple hours.

#12 @audrasjb
4 months ago

  • Keywords 2nd-opinion removed

#13 @audrasjb
4 months ago

  • Keywords needs-dev-note added

#14 @audrasjb
4 months ago

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

In 55262:

Media: Introduce wp_get_attachment_link_attributes filter.

This changeset introduces the wp_get_attachment_link_attributes hook to allow developers to filter the link attributes when getting the attachment link.

Props NathanAtmoz, aaroncampbell, antpb, costdev.
Fixes #41574.

Note: See TracTickets for help on using tickets.