Poor Description of add_image_size Params

[miqrogroove]

From https://core.trac.wordpress.org/browser/tags/4.8/src/wp-includes/media.php#L274

274	 * @param bool|array $crop   Optional. Whether to crop images to specified width and height or resize.
275	 *                           An array can specify positioning of the crop area. Default false.

So it literally says to do one thing or another. But which is what? "Default false" has a meaning, but it is totally lost in this documentation.

[nikolastoqnow]

Add better description about crop parameter in add_image_size function

[ocean90]

Introduced in [27472].

@nikolastoqnow Thanks for your patch! Looks like an empty line slipped into the patch. The use of "we" should also be avoided.

[ketuchetan]

Hi @ocean90 ,

I have made the changes as you requested in @nikolastoqnow patch.

Please let me know if any changes on that.

Thanks,
Chetan

[miqrogroove]

How about adding something to the effect of:

• Resize to fit one dimension, then crop image to fit the other dimension.
• Keep source aspect and resize to fit both dimensions, allowing one dimension to become smaller than specified.

This is roughly how I would describe the two different outcomes. And assign them to "true" and "false".

[kirasong]

Taking a look at this, and noticed that there is an existing extended description further up in the docs that was added in [27472].

Is this sufficient? Should we move some of that documentation down, or add to it?

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/media.php#L253

[miqrogroove]

The docblock is totally inadequate. My reading of it is literally "We will either scale or crop your photo, and the default is false." So wth? Am I to understand that if I set "true", then a 1000 x 1000 image will be "cropped" to, say, the middle 100x100 pixels and never resized? Based on my experience, that is not even accurate.

[peterwilsoncc]

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

[audrasjb]

The patch doesn't applies cleanly.

[audrasjb]

42463 Docs patch refresh

[audrasjb]

I don't checked the wording itself but it is now refreshed against current trunk.

[joemcgill]

Thanks for the refresh @audrasjb. The latest change to the wording improves the understanding of the boolean value, but omits the behavior of the $crop parameter if it's set as an array. Ideally, the description for this parameter would summarize the behavior outlined in the description for this function:

If false (default), images will be scaled, not cropped.
If an array in the form of array( x_crop_position, y_crop_position ):
x_crop_position accepts ‘left’ ‘center’, or ‘right’.
y_crop_position accepts ‘top’, ‘center’, or ‘bottom’. Images will be cropped to the specified dimensions within the defined crop area.
If true, images will be cropped to the specified dimensions using center positions.

[killua99]

According with the discussion on slack channel the wording wasn't accurate and the need of mention about the extended description on the summary could help to describe the $crop argument.

[joemcgill]

Thanks for the update, @killua99. I think we can still improve this slightly before committing. However, with 5.3 RC1 so close, I'm going to go ahead and move this forward to the 5.3 milestone and we can continue to iterate.

[killua99]

I feel the same @joemcgill with the 5.2 RC 1 so close better to move it to 5.3. Let me know what do you have on your mind about grammar improvement.

[pierlo]

In the patch 42463.5.patch I've moved the description of the cropping behavior to the $crop param doc.

This should resolve all ambiguity with the current explanation of the $crop param, as it feels silly to re-explain the description of it provided above.

[kirasong]

@joemcgill Do you want to do anything specific here, or should I make a couple final adjustments and commit?

Moving the docs to the param should allow IDEs to show the details as expected, correct?

[joemcgill]

I think the approach @pierlo is taking in 42463.5.patch​ is really close. I've cleaned it up a bit in 42463.2.diff​ and think it's ready to commit.

[joemcgill]

42463.5.diff​ just cleans up some whitespacing but is otherwise identical to 42463.2.diff​. Aside: auto-numbering works a lot better when we use diff as the extension since that's what grunt upload_patch uses.

[joemcgill]

In 46376:

Media: Improve documentation for add_image_size()

This improves the description of the $crop parameter to clarify behavior.

Props nikolastoqnow, ketuchetan, audrasjb, killua99, pierlo.
Fixes #42463.