WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 months ago

#36661 assigned enhancement

Improve `wp_get_attachment_image()` and `wp_get_attachment_image_attributes` docs.

Reported by: juanfra Owned by: antpb
Milestone: 5.4 Priority: normal
Severity: minor Version: 4.4
Component: Media Keywords: has-patch dev-feedback needs-refresh
Focuses: docs Cc:
PR Number:

Description

Hey there,

While looking for a solution for another ticket I've seen that there's not much documentation for wp_get_attachment_image(), specifically for the string|array of the $atts parameter. I know, that you can get through the code and see that some of them are automatically generated (src, class, alt, srcset, sizes) and that you override them and at the same time you can define an unlimited amount of $atts, but perhaps there's other people trying to use the function and they do not know that. We might end up seeing ugly implementations of people trying to add data attributes to images, etc. This function is not documented in the codex, only in the Code Reference site.

I think the most important part to explain is the scrset attribute as many people usually tries to use this type of function to deliver different images depending on the device in which the site is seen (Also, this is kind of covered by core in some ways now, but it is poorly documented). Or that you can add classes for retina support.

The same is happening with the wp_get_attachment_image_attributes filter within that function. It is not listing the list of attributes that you get by default.

I consider it is worth it to modify the phpdoc (at least) in the hook description, but I'm not sure if the phpdoc should be modified for the function. That's why I wanted to have some feedback before sending a patch.

What do you think? Shall we add docs within the code or we should we leave notes on the code reference site?

Attachments (1)

36661.diff (668 bytes) - added by Mte90 3 years ago.
draft for doc on wp_get_attachment_image_attributes

Download all attachments as: .zip

Change History (21)

#1 @DrewAPicture
4 years ago

  • Keywords needs-patch added
  • Version changed from 4.5 to 4.4

Hi @juanfra: I think the inline documentation for return values could benefit from specifying exactly which attributes you'll get and in which circumstances. Would you like to submit a patch?

#2 @DrewAPicture
4 years ago

  • Component changed from General to Media

#3 @juanfra
4 years ago

  • Severity changed from normal to minor

Hi @DrewAPicture!

Thanks for your time.

Yeah totally agree. Do you think by documenting the return values of the function is enough to not specify the default params($atts) for the wp_get_attachment_image_attributes filter? I think if we include them as well, we won't cause any harm and it could help someone out there trying to learn or implement something cool with images.

Last edited 4 years ago by juanfra (previous) (diff)

This ticket was mentioned in Slack in #docs by morganestes. View the logs.


3 years ago

@Mte90
3 years ago

draft for doc on wp_get_attachment_image_attributes

#5 @Mte90
3 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

My idea for the docs, or in case we need to explain every value but I don't think that is the best thing.

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


22 months ago

#7 @SergeyBiryukov
22 months ago

  • Milestone changed from Awaiting Review to 5.0

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


16 months ago

#9 @peterwilsoncc
14 months ago

  • Milestone changed from 5.0 to 5.1

Moving to the 5.1 milestone due to the WordPress 5.0 focus on the new
editor (Gutenberg).

#10 @pento
11 months ago

  • Milestone changed from 5.1 to 5.2

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


11 months ago

#12 @joemcgill
11 months ago

The current patch is an improvement, but I'd prefer to see the whole array documented per our documentation in the handbook.

#13 @antpb
11 months ago

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

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


10 months ago

#15 @desrosj
9 months ago

  • Keywords needs-refresh added

This needs a patch refresh per the latest comment above.

#16 @desrosj
9 months ago

  • Milestone changed from 5.2 to 5.3

Since this still needs an updated patch and 5.2 beta is in less than 2 days, I am going to punt this. @antpb if you have a new patch and feel it is ready in time, feel free to move back.

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


8 months ago

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


3 months ago

#19 @mikeschroder
3 months ago

@antpb Are you planning on doing any more work here this release?

I'm noticing this ticket is marked an enhancement, but docs changes can land at any time.

If you have time this release for review, would love to see it land (whether now or during beta), but we'll need to either reclassify it, or move it out of the milestone for triage purposes.

#20 @davidbaumwald
3 months ago

  • Milestone changed from 5.3 to 5.4

Although doc changes can be made between beta 1 and RC, there are 200 bug tickets still to triage. With 5.3 Beta 1 landing in a couple of hours, this is being moved for consideration in 5.4 If a committer feels this change should be made before RC, they can move it back and commit.

Note: See TracTickets for help on using tickets.