Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Top Posts Widget: allow non-square image sizes #6403

Merged

Conversation

stoyan0v
Copy link
Contributor

Fixes #3682

Changes proposed in this Pull Request:

  • Add width and height options to $get_image_options, that will allow users to change the size of the image used in Top Posts Widget.

Testing instructions:

  • Used to following code to change the image size.
function custom_thumb_size( $get_image_options ) {
        $get_image_options['width'] = 500;
        $get_image_options['height'] = 600;
 
        return $get_image_options;
}
add_filter( 'jetpack_top_posts_widget_image_options', 'custom_thumb_size' );

Is should also work, if you provide only $get_image_options['avatar_size'], but only if you want square images.

@stoyan0v stoyan0v changed the title Add width and height options to get_image_options Top Posts Widget: allow non-square image sizes Feb 14, 2017
Copy link
Contributor

@singerb singerb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor docs/comment update, otherwise this is basically the way I was planning on tackling this too.

@@ -251,6 +251,8 @@ function widget( $args, $instance ) {
/** This filter is documented in modules/stats.php */
'gravatar_default' => apply_filters( 'jetpack_static_url', set_url_scheme( 'https://en.wordpress.com/i/logo/white-gray-80.png' ) ),
'avatar_size' => 40,
'width' => null,
'height' => null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add these to the docblock about jetpack_top_posts_widget_image_options below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @singerb, I tried to make the code bulletproof, but I totally forgot about the filter docs ( shame on me 😄 ). Thanks for your comment, the new options have been added.

@singerb singerb added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Feb 14, 2017
@jeherve jeherve added [Feature] Extra Sidebar Widgets [Pri] Low [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Feb 14, 2017
@singerb singerb added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Feb 15, 2017
@singerb
Copy link
Contributor

singerb commented Feb 15, 2017

LGTM.

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just left 2 minor comments, but other than that it should be good to go! 👍

$post['post_id'],
array(
'fallback_to_avatars' => true,
'avatar_size' => (int) $get_image_options['avatar_size']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're reorganizing this code, could you add a comma, since each line in an array declaration must end with a comma?

@@ -267,6 +269,8 @@ function widget( $args, $instance ) {
* @type bool true Should we default to Gravatars when no image is found? Default is true.
* @type string $gravatar_default Default Image URL if no Gravatar is found.
* @type int $avatar_size Default Image size.
* @type mixed $width Image width, not set by default and $avarat_size is used instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you have a little typo here.

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 15, 2017
@stoyan0v stoyan0v force-pushed the add/top-posts-widget/image-size-options branch from cf41e10 to c2dc92c Compare February 16, 2017 08:19
@stoyan0v
Copy link
Contributor Author

Thanks @jeherve, I have addressed the issues.

@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Feb 16, 2017
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 16, 2017
@jeherve jeherve merged commit 5a21bef into Automattic:master Feb 16, 2017
@jeherve jeherve added [Status] Needs Changelog [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Ready to Merge Go ahead, you can push that green button! [Status] Needs Changelog labels Feb 16, 2017
jeherve added a commit that referenced this pull request Feb 21, 2017
dereksmart pushed a commit that referenced this pull request Feb 28, 2017
* Changelog: update stable tag and move changelog to changelog.txt

Also remove old releases from readme.txt to keep the changelog tab short.

* Changelog: add #5883

Also update the filter's docblock to match new version.

* Changelog: add #5938

* Changelog: add #6298

* Changelog: add #3405

* Changelog: add #5941

* Changelog: add #6239

* Changelog: add #6281

* Changelog: add #6303

* Changelog: add #6018

* Changelog: add #6300

* Changelog: add #6296

* Changelog: add #6130

* Changelog: add #6292

* Readme: remove extra "on".

* Changelog: add #6307

* Changelog: add #3297

* Changelog: add #6275

* Changelog: add #6321

* Changelog: add #6297

* Readme: update the support forum link anchor.

Anchor changed when WordPress.org forums were updated to bbPress 2

* Readme: update list of a12s, it wasn't up to date anymore!

* Changelog: add #6338

* Changelog: add #6337

* Changelog: add #6335

* Changelog: add #6333

* Testing List: first version of the 4.7 testing list.

* Changelog: add #6332

* Changelog: add #6325

* Changelog: add #6326

* Changelog: add #6339

* Changelog: add #6342

* Changelog: add #6343

* Changelog: add #6346

* Changelog: add #6347

* Changelog: add #6279

* Changelog: add #6306

* Changelog: add #6312

* Changelog: add #6316

* Changelog: add #6171

* Changelog: add #6317

* Changelog: add #6246

* Changelog: add #6263

* Changelog: add #4220

* Changelog: add #5888

* Changelog: add #3406

* Changelog: add #3637

* Changelog: add #6320

* Changelog: add #5992

* Changelog: add #6322

* Changelog: add #6324

* Changelog: add #6352

* Changelog: add #6355

* Changelog: add #6360

* Changelog: add #6362

* Changelog: add #6369, #6382

* Changelog: add #6370

* Changelog: add #6375

* Changelog: add #6383

* Changelog: add #6384

* Changelog: add #6386

* Changelog: add #6395

* Changelog: add #6403

* Changelog: add #6406

* Changelog: add #6418

* Changelog: add #6419

* Changelog: add #6434

* Changelog: add #6446

* Changelog: add #6006

* Changelog: add #6096

* Changelog: add #6399

* Changelog: fix typo.

@see #6331 (comment)

* Changelog: add #6440

* Changelog: add #6443

* Changelog: add #6445

* Changelog: add #6463

* Changelog: add #6468

* Changelog: add #6471

* Changelog: add #6474

* Changelog: add #6480

* Changelog: add #6497

* Changelog: add #6499

* Changelog: add #6514

* Changelog: add #6267

* Changelog: add #5940

* Changelog: add #6492

* Changelog: add #5281

* Changelog: add #6327

* Changelog: add #6451

* Changelog: add #6525

* Changelog: add #6530
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extra Sidebar Widgets [Pri] Low Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants