Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#34612 closed defect (bug) (fixed)

Responsive Images: Make new function/filter signatures more consistent

Reported by: joemcgill's profile joemcgill Owned by: azaozz's profile azaozz
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch
Focuses: Cc:

Description

As a part of the responsive images features being added in 4.4, we should promote internal consistency in the new functions and filters we're adding. To that end, several of us working on this feature have discussed splitting wp_get_attachment_image_sizes() into two functions: an internally used function that calculates the sizes attribute, and an external use function that can be used easily in themes. This mimics the pattern we already established with wp_calculate_image_srcset() and wp_get_attachment_image_srceset().

Attachments (6)

34612.diff (12.5 KB) - added by joemcgill 10 years ago.
34612.filter.diff (2.3 KB) - added by joemcgill 10 years ago.
Update params for wp_calculate_image_srcset filter.
34612.filter.2.diff (2.3 KB) - added by joemcgill 10 years ago.
34612.2.diff (1013 bytes) - added by kraftbj 10 years ago.
Minor: Remove blank line from inline docs.
34612.3.diff (563 bytes) - added by kraftbj 10 years ago.
.2 without the extra stuff :) -- removing blank line.
34612.4.diff (2.7 KB) - added by joemcgill 10 years ago.

Download all attachments as: .zip

Change History (19)

@joemcgill
10 years ago

#1 @joemcgill
10 years ago

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

34612.diff moves the logic of wp_get_attachment_image_sizes() to wp_calculate_image_sizes() and renames the filter wp_calculate_image_sizes(). The parameter order now matches the order or params in wp_calculate_image_srcset().

The previous function name: wp_get_attachment_image_sizes() now accepts an attachment id and optional size parameter, which matches the signature of wp_get_attachment_image_srcset() and is consistent with the other wp_get_attachment_image_ template functions.

Tests and docs have been updated.

This ticket was mentioned in Slack in #feature-respimg by joemcgill. View the logs.


10 years ago

#3 @azaozz
10 years ago

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

In 35569:

Responsive images: make the new functions and filters signatures more consistent.

Props joemcgill.
Fixes #34612.

@joemcgill
10 years ago

Update params for wp_calculate_image_srcset filter.

#4 follow-up: @joemcgill
10 years ago

Looks like I missed updating the params for wp_calculate_image_srcset filter in 34612.diff. I've uploaded 34612.filter.diff, which corrects the param order and adds $image_src to the filter. Inline docs are updated as well.

#5 @kirasong
10 years ago

  • Keywords commit added

34612.filter.diff looks good; thanks!

#6 in reply to: ↑ 4 @DrewAPicture
10 years ago

  • Keywords commit removed

Replying to joemcgill:

Looks like I missed updating the params for wp_calculate_image_srcset filter in 34612.diff. I've uploaded 34612.filter.diff, which corrects the param order and adds $image_src to the filter. Inline docs are updated as well.

We're going to have to move the code example into the hook doc description. As-is, it would be read as a sentence with the extra spaces removed. So let's move it to the description, four-space indented and figure out a good way to refer back to it from the parameter description.

#7 @joemcgill
10 years ago

After discussion with @DrewAPicture, in 34612.filter.2.diff I've changed the inline docs to use proper @type docs for the array of sources being passed in the wp_calculate_image_srcset filter, per the inline docs handbook.

#8 @azaozz
10 years ago

In 35591:

Responsive images: properly arrange the parameters for the wp_calculate_image_srcset filter and add fix the inline documentation.

Props joemcgill.
Fixes #34612.

#9 @DrewAPicture
10 years ago

In 35592:

Docs: Fix some formatting in the hook doc for the wp_calculate_image_srcset filter and clarify the summary.

See #34612.

@kraftbj
10 years ago

Minor: Remove blank line from inline docs.

@kraftbj
10 years ago

.2 without the extra stuff :) -- removing blank line.

#10 @kraftbj
10 years ago

@DrewAPicture Super minor, but there's a blank line in the inline docs L1092. Removed in .2.diff.

#11 @DrewAPicture
10 years ago

In 35601:

Docs: Remove an empty line from the hook doc for the wp_calculate_image_srcset filter, introduced in [35592].

Props kraftbj.
See #34612.

@joemcgill
10 years ago

#12 @joemcgill
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I noticed that we had marked several params in wp_calcuate_image_sizes() as optional, but had not defined default values. 34612.4.diff corrects this issue and includes a few other fixes to the docblocs for this function and filter.

#13 @DrewAPicture
10 years ago

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

In 35672:

Docs: Properly mark optional parameters as such in the DocBlock and function signature for wp_calculate_image_sizes().

Also updates the subsequent hook docs for the wp_calculate_image_sizes filter.

Props joemcgill.
Fixes #34612.

Note: See TracTickets for help on using tickets.