Skip to content

Social Previews: Simplify featured image media size logic #16952

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.

Sign up for GitHub

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

Jump to bottom
Merged
marekhrabe merged 3 commits into from
Aug 24, 2020
Merged

Social Previews: Simplify featured image media size logic #16952

marekhrabe merged 3 commits into from
Aug 24, 2020

Conversation

@marekhrabe
Copy link
Contributor

@marekhrabe marekhrabe commented Aug 24, 2020

We've had reports of image size being too small and I have confirmed that when running on wpcom. The image size provided was 150px big, which is not enough for bigger previews that might be generated.

Changes proposed in this Pull Request:

  • Switch the logic to look for the large named image size and use the full one as a fallback.

Jetpack product discussion

Does this pull request change what data or activity we track or use?

no

Testing instructions:

Proposed changelog entry for your changes:

@marekhrabe marekhrabe added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. [Status] Needs Team Review Obsolete. Use Needs Review instead. [Block] Social Previews labels Aug 24, 2020
@marekhrabe marekhrabe added this to the 8.9 milestone Aug 24, 2020
@marekhrabe marekhrabe self-assigned this Aug 24, 2020
@matticbot
Copy link
Contributor

matticbot commented Aug 24, 2020

Caution: This PR has changes that must be merged to WordPress.com
Hello marekhrabe! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D48494-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@marekhrabe marekhrabe requested review from a team and jeherve August 24, 2020 12:12
@jetpackbot
Copy link
Collaborator

jetpackbot commented Aug 24, 2020 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16952

Scheduled Jetpack release: September 1, 2020.
Scheduled code freeze: August 25, 2020

Generated by 🚫 dangerJS against 3a9a8b2

@cpapazoglou cpapazoglou self-requested a review August 24, 2020 12:36
cpapazoglou
cpapazoglou previously requested changes Aug 24, 2020
View reviewed changes
Copy link
Contributor

@cpapazoglou cpapazoglou left a comment

Choose a reason for hiding this comment

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

Tested by test instructions and works fine.

Left just a minor comment, apart from that LGTM!

@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. [Status] Needs Team Review Obsolete. Use Needs Review instead. labels Aug 24, 2020
@marekhrabe marekhrabe merged commit 22c7e00 into master Aug 24, 2020
@marekhrabe marekhrabe deleted the update/social-previews-image-size branch August 24, 2020 15:05
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 24, 2020
pereirinha pushed a commit that referenced this pull request Sep 10, 2020
@marekhrabe
[not verified] Social Previews: Simplify featured image media size lo…
57d47ed 
…gic (#16952)

* simplify featured image media size logic

* remove unused dependency

* remove docs for removed param
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

+1 more reviewer

@cpapazoglou cpapazoglou cpapazoglou left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@marekhrabe marekhrabe

Labels

Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Milestone

8.9

Development

Successfully merging this pull request may close these issues.

6 participants

@marekhrabe @matticbot @jetpackbot @jeherve @cpapazoglou