Skip to content

Add picture element support #73

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
adamsilverstein merged 86 commits into from
May 30, 2024
Merged

Add picture element support #73

adamsilverstein merged 86 commits into from
May 30, 2024

Conversation

@adamsilverstein
Copy link
Member

@adamsilverstein adamsilverstein commented Dec 31, 2021 •
edited by westonruter
Loading

Fixes #21
Closes #1236

  • Filters on wp_content_img_tag to potentially wrap (media) images in a picture element.
  • Works best with themes that support responsive images, for example twentysixteen.
  • Check the network panel to see which images the browser actually loads.

Picture element and AVIF support by version

AVIF support is being worked on and picture element support can be especially useful for adding AVIFs to your site if you have users on browsers that don't support AVIF.

Browser picture Support (Version, Date) AVIF Support (Version, Date)
Chrome 4 (January 2010) 85 (August 2020)
Edge 12 (July 2015) 121 (January 2024)
Safari 9 (September 2015) 16.1 (October 2023)
Firefox 38 (May 2015) 93 (October 2021)

@adamsilverstein adamsilverstein linked an issue Dec 31, 2021 that may be closed by this pull request
Enable <picture> element support #21
Closed
@adamsilverstein adamsilverstein modified the milestone: 1.0.0-beta.1 Dec 31, 2021
@adamsilverstein adamsilverstein added the [Type] Enhancement A suggestion for improvement of an existing feature label Dec 31, 2021
@eugene-manuilov
Copy link
Contributor

eugene-manuilov commented Jan 5, 2022

Filters on the_content to potentially wrap (media) images in a picture element.

@adamsilverstein did you add the hook for the the_content filter just for testing purposes or we are going to keep it?

@mitogh mitogh marked this pull request as ready for review January 6, 2022 11:09
@mitogh mitogh marked this pull request as draft January 6, 2022 16:35
@adamsilverstein
Copy link
Member Author

adamsilverstein commented Jan 26, 2022

@adamsilverstein did you add the hook for the the_content filter just for testing purposes or we are going to keep it?

The entire PR is for testing.

adamsilverstein
adamsilverstein commented Jan 27, 2022
View reviewed changes
spacedmonkey
spacedmonkey reviewed Feb 1, 2022
View reviewed changes
@adamsilverstein
Copy link
Member Author

adamsilverstein commented May 21, 2024

reopening to bring this PR up to date...

@adamsilverstein adamsilverstein added this to the webp-uploads n.e.x.t milestone May 21, 2024
@adamsilverstein adamsilverstein changed the title Add picture element module Add picture element support May 21, 2024
@adamsilverstein
Copy link
Member Author

adamsilverstein commented May 29, 2024

Confirmed this works with AVIF selected as the output format:

image

joemcgill
joemcgill reviewed May 29, 2024
View reviewed changes
Copy link
Member

@joemcgill joemcgill left a comment

Choose a reason for hiding this comment

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

This is looking good and surprisingly doesn't seem to impact default styling of images even though the markup changes. Left a couple of questions.

plugins/webp-uploads/picture-element.php
Comment on lines 78 to 81
// If the original mime types is the only one available, no picture element is needed.
if ( 1 === count( $mime_types ) && $mime_types[0] === $original_file_mime_type ) {
return $image;
}
Copy link
Member

@joemcgill joemcgill May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

For sites that don't support AVIF or WebP, if someone uploads an original in one of those formats, won't all the intermediate sizes be JPEG? If so, do we want this to result in using picture or would a simple img element be preferred?

Copy link
Member Author

@adamsilverstein adamsilverstein May 29, 2024

Choose a reason for hiding this comment

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

I haven't tested with WebP support missing, but my expectation is that you shouldn't be able to upload a WebP image if your server doesn't support it. This may not be working correctly in the block editor currently, so for now I am referring to uploading in the media library. I'm not sure I have a test version of PHP that doesn't support WebP.

~98% of WordPress sites are on a host that supports WebP so this is an edge case.

Copy link
Member Author

@adamsilverstein adamsilverstein May 30, 2024

Choose a reason for hiding this comment

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

@joemcgill - I created this follow up Issue to work on improving the settings screen in particular when WebP isn't supported.

@westonruter
Copy link
Member

westonruter commented May 29, 2024

I found that the Settings link was pointing to the "Also output JPEG" checkbox:

image

I've fixed this in 2e68884 so that the settings link points to the entire section.

westonruter
westonruter reviewed May 29, 2024
View reviewed changes
westonruter
westonruter reviewed May 29, 2024
View reviewed changes
westonruter
westonruter reviewed May 29, 2024
View reviewed changes
westonruter
westonruter reviewed May 29, 2024
View reviewed changes
@adamsilverstein
Copy link
Member Author

adamsilverstein commented May 29, 2024

I found that the Settings link was pointing to the "Also output JPEG" checkbox:

image

I've fixed this in 2e68884 so that the settings link points to the entire section.

Nice catch!

westonruter
westonruter reviewed May 29, 2024
View reviewed changes
westonruter
westonruter approved these changes May 29, 2024
View reviewed changes
Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

With https://github.com/WordPress/performance/pull/73/files#r1619520322 this looks good to go! 🥳

@adamsilverstein
Copy link
Member Author

adamsilverstein commented May 29, 2024

With #73 (files) this looks good to go! 🥳

Thanks the reviews and feedback🎉 !

@westonruter westonruter requested a review from joemcgill May 29, 2024 22:30
@westonruter westonruter mentioned this pull request May 29, 2024
Merged
@adamsilverstein adamsilverstein mentioned this pull request May 30, 2024
Closed
@adamsilverstein adamsilverstein merged commit d998aa9 into trunk May 30, 2024
@adamsilverstein adamsilverstein deleted the add/picture-support branch May 30, 2024 13:25
@adamsilverstein adamsilverstein mentioned this pull request May 30, 2024
Closed
@westonruter westonruter added [Type] Feature A new feature within an existing module and removed [Type] Enhancement A suggestion for improvement of an existing feature labels Jun 6, 2024
@swissspidy swissspidy added [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) and removed [Module] Picture Element labels Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spacedmonkey spacedmonkey spacedmonkey left review comments

@westonruter westonruter westonruter approved these changes

@felixarntz felixarntz Awaiting requested review from felixarntz felixarntz is a code owner

@pbearne pbearne Awaiting requested review from pbearne

@mukeshpanchal27 mukeshpanchal27 Awaiting requested review from mukeshpanchal27

@thelovekesh thelovekesh Awaiting requested review from thelovekesh

@joemcgill joemcgill Awaiting requested review from joemcgill

Assignees

No one assigned

Labels

[Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Type] Feature A new feature within an existing module

Projects

None yet

Milestone

webp-uploads 2.0.0

Development

Successfully merging this pull request may close these issues.

Enable <picture> element support

8 participants

@adamsilverstein @eugene-manuilov @westonruter @spacedmonkey @joemcgill @swissspidy @thelovekesh