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

new_audit: add unsized-images to experimental config #11115

Merged
merged 50 commits into from
Jul 29, 2020
Merged

Conversation

lemcardenas
Copy link
Contributor

@lemcardenas lemcardenas commented Jul 17, 2020

Summary

This PR is a first iteration in creating a new audit that confirms users' images are explicitly sized; ultimately preventing layout shift and improving CLS (cumulative layout shift).

Setting explicit sizes for images is an effective way to reduce CLS since images without sizes are a leading contributor to CLS.

Design and detail about explicit sizing on image elements can be found here.
Design and discussion about explicit sizing on various HTML elements can be found here.

This audit is set to remain in experimental config until CSS sizing is included in a future image-elements gatherer PR.

Related Issues/PRs

#10085

unsized-images-failing-3 0
unsized-images-passing-3 0

@lemcardenas lemcardenas reopened this Jul 17, 2020
@lemcardenas lemcardenas changed the title Unsized images new_audit: add sized-images audit Jul 17, 2020
Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

this looks awesome @lemcardenas! one of the cleanest "draft" PRs I've ever seen 😄

a PR overview with references to what stage this is and what planned followups should be expected (maybe linking to the doc?) would be awesome. I'm thinking we should land this in the experimental-config.js rather than default-config.js until we can update it to support CSS images, but maybe you already discussed an alternative plan/timeline I missed?

lighthouse-core/gather/gatherers/image-elements.js Outdated Show resolved Hide resolved
lighthouse-core/scripts/i18n/collect-strings.js Outdated Show resolved Hide resolved
types/artifacts.d.ts Outdated Show resolved Hide resolved
lighthouse-core/audits/sized-images.js Outdated Show resolved Hide resolved
@lemcardenas
Copy link
Contributor Author

a PR overview with references to what stage this is and what planned followups should be expected (maybe linking to the doc?) would be awesome.

Yes! I am planning on adding all of that, I didn't expect feedback on a draft PR 😅

I'm thinking we should land this in the experimental-config.js rather than default-config.js until we can update it to support CSS images, but maybe you already discussed an alternative plan/timeline I missed?

I think placing it in experimental-config is a good idea! I'll switch it over to that today

@lemcardenas lemcardenas marked this pull request as ready for review July 27, 2020 16:41
Copy link
Collaborator

@connorjclark connorjclark left a comment

Choose a reason for hiding this comment

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

only a couple of minor style nits from me. after that it'll be LGTM

@vercel vercel bot temporarily deployed to Preview July 29, 2020 00:27 Inactive
@connorjclark connorjclark merged commit c75f1ba into master Jul 29, 2020
@connorjclark connorjclark deleted the unsized-images branch July 29, 2020 01:16
@lemcardenas lemcardenas changed the title new_audit: add sized-images to experimental config new_audit: add unsized-images to experimental config Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants