Skip to content

Improve Jetpack products illustrations file size - #21514

Merged
kraftbj merged 4 commits into
masterfrom
fix/jetpack-products-illustration-optimization
Oct 25, 2021
Merged

Improve Jetpack products illustrations file size#21514
kraftbj merged 4 commits into
masterfrom
fix/jetpack-products-illustration-optimization

Conversation

@IanRamosC

@IanRamosC IanRamosC commented Oct 22, 2021

Copy link
Copy Markdown
Contributor

Fixes #21497 (review)

Changes proposed in this Pull Request:

  • This PR improves the product illustration images by running them through TinyPNG smart loss compression (no visual change/loss). The goal is to reduce the file size.

Jetpack product discussion

Context: #21497 (review)

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

Nope

Testing instructions:

  • Make sure the new images are not blurry
  • Go to the following pages and confirm the images don't have any visual changes:
    • /wp-admin/admin.php?page=jetpack#/product/akismet
    • /wp-admin/admin.php?page=jetpack#/product/backup
    • /wp-admin/admin.php?page=jetpack#/product/scan
    • /wp-admin/admin.php?page=jetpack#/product/search
    • /wp-admin/admin.php?page=jetpack#/product/security

@IanRamosC IanRamosC added [Status] Needs Review This PR is ready for review. Admin Page React-powered dashboard under the Jetpack menu labels Oct 22, 2021
@IanRamosC
IanRamosC requested a review from jeherve October 22, 2021 19:49
@IanRamosC IanRamosC self-assigned this Oct 22, 2021
@github-actions github-actions Bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Oct 22, 2021
@github-actions

github-actions Bot commented Oct 22, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

  • Next scheduled release: November 2, 2021.
  • Scheduled code freeze: October 25, 2021.

@IanRamosC
IanRamosC requested review from a team and removed request for jeherve October 22, 2021 20:13
@IanRamosC IanRamosC changed the title Fix/jetpack products illustration optimization Improve jetpack products illustration optimization Oct 23, 2021
@IanRamosC IanRamosC changed the title Improve jetpack products illustration optimization Improve Jetpack products illustrations file size Oct 23, 2021
@kallehauge

kallehauge commented Oct 25, 2021

Copy link
Copy Markdown
Contributor

Copying my comment over here as I didn't notice this PR existed 😅

@IanRamosC & @jeherve the bigger image sizes were originally a request from @keoshi to support retina.
I would suggest introducing srcset and add the images in different sizes.

I'm mentioning it so we don't just reduce the current images' sizes and call it a day 😄

Source

@kallehauge kallehauge mentioned this pull request Oct 25, 2021
Merged
@jeherve jeherve added this to the jetpack/10.3 milestone Oct 25, 2021
@jeherve jeherve added [Status] Blocked / Hold and removed [Status] Needs Review This PR is ready for review. labels Oct 25, 2021
@jeherve

jeherve commented Oct 25, 2021

Copy link
Copy Markdown
Member

Putting this on hold pending the merge of #21497.

@kallehauge Do we need the extra size if the images can be made small enough with optimizations?

@kallehauge

kallehauge commented Oct 25, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

With our very low mobile usage in WPA and the sizes being e.g. 50.4 KB for the Anti Spam illustration, I would say no.
I'm just surprised that they could be reduced that much further since I already ran it through compression when I initially added them 😅

@kallehauge

kallehauge commented Oct 25, 2021

Copy link
Copy Markdown
Contributor

Oi! A comment for the actual implementation:
I can see projects/plugins/jetpack/images/products/illustration-videopress.png is smaller (resolution wise) compared to the other product illustrations. We should update it to have approx. the same size (~1200px width) for better retina support

@IanRamosC

IanRamosC commented Oct 25, 2021

Copy link
Copy Markdown
Contributor Author

Oi! A comment for the actual implementation:
I can see projects/plugins/jetpack/images/products/illustration-videopress.png is smaller (resolution wise) compared to the other product illustrations. We should update it to have approx. the same size (~1200px width) for better retina support

I'm pinging the design team to get a higher resolution illustration for VideoPress (around 1200px).

@IanRamosC

IanRamosC commented Oct 25, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Added the new image (2x) and it goes from 67kb to ~250kb (after TinyPNG). It's quite a lot, but this one has more details than the other illustrations - that we have a lot of white.

Moved to "Needs Review" again since #21497 got merged.

@IanRamosC IanRamosC added [Status] Needs Review This PR is ready for review. and removed [Status] Blocked / Hold labels Oct 25, 2021
@kraftbj
kraftbj enabled auto-merge (squash) October 25, 2021 18:21
@kraftbj kraftbj 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 Oct 25, 2021
@kraftbj

kraftbj commented Oct 25, 2021

Copy link
Copy Markdown
Contributor

Had to resolve a merge conflict with this one. @IanRamosC confirmed via Slack that the right VideoPress image came through p1635186042168000/1635179668.165900-slack-CBG1CP4EN

@kraftbj
kraftbj merged commit 2004c93 into master Oct 25, 2021
@kraftbj
kraftbj deleted the fix/jetpack-products-illustration-optimization branch October 25, 2021 18:26
@github-actions github-actions Bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 25, 2021
davidlonjon added a commit that referenced this pull request Oct 26, 2021
@davidlonjon
Merge branch 'master' into add/boost-e2e-ci-integration
b4a66c0 
* master: (27 commits)
  Admin Page: Update upgrade buttons Tracks events (#21483)
  Debug Helper IDC Simulator: Add the ability to spoof the home url (#21516)
  Map block: Add `address` block attribute (#21412)
  Update wordpress monorepo (#21522)
  Stats Page: Add VideoPress nudge (#21513)
  Improve Jetpack products illustrations file size (#21514)
  Update dependency mediawiki/mediawiki-codesniffer to v38 (#21523)
  Update dependency yoast/phpunit-polyfills to v1.0.2 (#21524)
  Widgets: add Strava to Social Icons Widget and Social Menu (#21518)
  Boost: Add active modules to speed score request (#21471)
  Markdown: Better accessibility for Footnotes (#21495)
  Jetpack Plugin: Add VideoPress upgrade page (#21497)
  Admin Menu: moves Add new (plugin) menu item to the top for Atomic sites (#21506)
  General: Check the return value of get_comment() (#21080)
  Instagram Widget: link to user connection if user is not connected (#21512)
  Search: Create a new search package, rename Search plugin (#21502)
  Identity Crisis: Remove the unused jetpack_idc_option transient (#21492)
  Stats: do not update the wpcom blog details (#21441)
  Admin: do not display upgrade messages when on offline mode (#21444)
  jetpack: Catch calls to `exit()` and `die()` in PHPUnit (#21043)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kraftbj kraftbj kraftbj approved these changes

Assignees

@IanRamosC IanRamosC

Labels

Admin Page React-powered dashboard under the Jetpack menu [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/

Projects

None yet

Milestone

jetpack/10.3

Development

Successfully merging this pull request may close these issues.

4 participants

@IanRamosC @kallehauge @jeherve @kraftbj