wp-media / imagify-plugin

Speed up your website with lighter images without losing quality.
https://imagify.io
69 stars 24 forks source link

Closes #794 display the smaller size on image library data #810

Closed Khadreal closed 3 months ago

Khadreal commented 3 months ago

Description

Display the smaller size of the image(avif)

Fixes #794

Documentation

User documentation

This will show the avif image size & percentage when available

Technical documentation

Explain how this code works. Diagram & drawings are welcomed.

Type of change

New dependencies

N/A

Risks

List possible performance & security issues or risks, and explain how they have been mitigated.

Checklists

Feature validation

Code style

Khadreal commented 3 months ago

I think we should go further on this one, updating this method and other methods in the class that reference webp, to make it work with the new implementation (using next-gen). This would also require to update the interface.

If I understand correctly, we should update the method from webp to nextgen ?

Tabrisrp commented 3 months ago

Yes, update all occurrences of webp to nextgen. I see there is also a use of imagify_path_to_webp() that should be updated in this class for example.

Mai-Saad commented 3 months ago

@Khadreal Thanks for the PR. please find exploratory note below:

Khadreal commented 3 months ago

@Khadreal Thanks for the PR. please find exploratory note below:

  • when upload image while avif is off, we arenot generating webP by default
  • when enable avif after that, we arenot generating avif for uploaded images in previous step can you please check? (those points were working on the main feature branch)

The branch has been updated to the latest feature branch

Mai-Saad commented 3 months ago

@Khadreal Thanks for the PR. working as in trunk after merging main branch to the PR locally while image have only webp.

Note: we have some discussion about sizes here (probably it isnot always the smallest on trunk), however, the agreed expected shall be in separate GH as it's the case on trunk.

Update: as per slack discussion, we shall display here the size of avif , if not exist then webp, if not exist then optimized => now we display in media library new size as per enabled option. which doesn't meet this update from slack. @Khadreal can you please check? steps: 1- avif enabled 2- upload image => avif created and new size =that of avif 3- disable avif => webp created 4- check media library => new size = webp size instead of avif bird-1905255_1280

/cc @piotrbak

Khadreal commented 3 months ago

This PR https://github.com/wp-media/imagify-plugin/pull/851 once merge will fixed the failing test error

MathieuLamiot commented 3 months ago

@Khadreal should this go back to ready for review, or there are still work needed here?

Khadreal commented 3 months ago

@Khadreal should this go back to ready for review, or there are still work needed here?

I'll move it, done with the work but didn't move it because of the test failing

Mai-Saad commented 3 months ago

@Khadreal Thanks for the update. Please find exploratory note below:

or 1- activate avif 2- upload the same image

Screenshot from 2024-03-14 14-24-53

can you please check?

Note: in general, related test case https://wpmediaqa.testrail.io/index.php?/cases/view/15271

Khadreal commented 3 months ago

Got this error, wondering if you're seeing something like this as well. This is when avif is not enable.

Screenshot 2024-03-15 at 15 33 51
Khadreal commented 3 months ago

794

Got this error, wondering if you're seeing something like this as well. This is when avif is not enable.

Screenshot 2024-03-15 at 15 33 51

My last update fixed this too

Mai-Saad commented 3 months ago

@Khadreal Thanks for the update, fixed in both scenarios mentioned above but can see problem if we refresh admin after scenario 2 i.e 1- install and activate imagify 2- enable avif 3- upload the image => avif created and no error 4- refresh media page , the warning is there

in log can see [18-Mar-2024 09:52:12 UTC] PHP Warning: Undefined array key "full@imagify-webp" in /home/mai/Local Sites/imagifynginx6/app/public/wp-content/plugins/imagify-plugin/classes/Optimization/Data/AbstractData.php on line 314

note: it happens with any optimized image not just the previous one screen-capture (88).webm

Khadreal commented 3 months ago

Fixed. please check again. Sorry for the back and forth

Mai-Saad commented 3 months ago

@Khadreal Thanks for the update. Working fine now with the following note (which is fine as per https://wp-media.slack.com/archives/CU0F6EGQ1/p1709546156479809?thread_ts=1709458320.636499&cid=CU0F6EGQ1)