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

Closes #504 AVIF support #801

Merged
merged 33 commits into from
Mar 6, 2024
Merged

Closes #504 AVIF support #801

merged 33 commits into from
Mar 6, 2024

Conversation

Tabrisrp
Copy link
Contributor

Description

Add AVIF support in the plugin

Closes #504

Documentation

User documentation

Explain how this code impacts users.

Technical documentation

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

Type of change

Delete options that are not relevant.

  • New feature (non-breaking change which adds functionality).
  • Bug fix (non-breaking change which fixes an issue).
  • Enhancement (non-breaking change which improves an existing functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as before).

New dependencies

List any new dependencies that are required for this change.

Risks

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

Checklists

Feature validation

  • I validated all the Acceptance Criteria. If possible, provide sreenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I implemented built-in tests to cover the new/changed code.

Documentation

  • I prepared the user documentation for the feature/enhancement and shared it in the PR or the GitHub issue.
  • The user documentation covers new/changed entry points (endpoints, WP hooks, configuration files, ...).
  • I prepared the technical documentation if needed, and shared it in the PR or the GitHub issue.

Code style

  • I wrote self-explanatory code about what it does.
  • I wrote comments to explain why it does it.
  • I named variables and functions explicitely.
  • I protected entry points against unexpected inputs.
  • I did not introduce unecessary complexity.
  • I listed the introduced external dependencies explicitely on the PR.
  • I validated the repo-specific guidelines from CONTRIBUTING.md.

Observability

  • I handled errors when needed.
  •  I wrote user-facing messages that are understandable and provide actionable feedbacks.
  • I prepared ways to observe the implemented system (logs, data, etc.).

Risks

  •  I explicitely mentioned performance risks in the PR.
  • I explicitely mentioned security risks in the PR.

@Mai-Saad
Copy link

Notes [WIP]:
1- Bulk optimization notice displayed when enable/disable create AVIF (When enable AVIF, despite of having 4 images in media library, AVIF was created for single image and media progress was stuck at 50% till we reoptimize any image, it will stop . @piotrbak is there any need to show progress in bulk optimization once enable AVIF?) => on trunk, was only displayed when click bulk optimization
Recommendation:

  • no need for bulk optimization notice as we are only generating AVIF and not re-optimizing all images
  • even if we agreed to display the notice, it shouldn't be on plugins/post/users... pages

@Mai-Saad
Copy link

Mai-Saad commented Mar 4, 2024

@Tabrisrp Tabrisrp added this to the 2.2 milestone Mar 4, 2024
@Tabrisrp Tabrisrp marked this pull request as ready for review March 5, 2024 14:37
Copy link

@Mai-Saad Mai-Saad left a comment

Choose a reason for hiding this comment

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

Working as expected with the following notes:

  • With latest WP nighty version, if we uploaded avif , in media library it is mentioned as not supported => possible enhancement may be at core to have thumbnail for this image as same as with webP , while on our side , optimizing this image as same as when uploading webp
  • Avif isnot created for gif nor webp images (gif have separate GH)
  • Generate missing next-gen images count may be -ve or invalid in some cases
  • Disable avif won't generate webp by default for existing images although generating next-gen will be loading in settings
  • Displayed size in media library is always webp if it exists while avif is there (have separate GH)
  • The missing next-gen count not handling the case when both avif and webp are on (in this case it will count only avif)
    testrail-report-580.pdf

@Tabrisrp Tabrisrp added this pull request to the merge queue Mar 6, 2024
Merged via the queue into develop with commit 5684433 Mar 6, 2024
5 of 7 checks passed
@Tabrisrp Tabrisrp deleted the feature/avif branch March 6, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support AVIF image format
6 participants