-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
core(unsized-images): pass with explicit aspect-ratio #12377
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
impl LGTM
a smoketest would be great here too since all of this working correctly hinges on us getting the protocol styles response right ;) byte efficiency tester should have plenty of examples to modify or copy
const htmlWidthIsExplicit = UnsizedImages.doesHtmlAttrProvideExplicitSize(attrWidth); | ||
const cssWidthIsExplicit = UnsizedImages.doesCssPropProvideExplicitSize(cssWidth); | ||
const htmlHeightIsExplicit = UnsizedImages.doesHtmlAttrProvideExplicitSize(attrHeight); | ||
const cssHeightIsExplicit = UnsizedImages.doesCssPropProvideExplicitSize(cssHeight); | ||
const explicitAspectRatio = UnsizedImages.doesCssPropProvideExplicitSize(cssAspectRatio); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename this method? I was expecting a check for numbers or size markers, forgetting that all we care about is !== 'auto'
:)
now that we're using it for things that aren't just size, perhaps something more generic like isCssPropExplicitlySet
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually come to think of it we should probably be banning initial
/unset
/inherit
too, do those come through on the protocol?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fixes #12218
We consider the image "sized" (thus passing) if
aspect-ratio
is used in conjunction with either a width or a height:lighthouse/core/audits/unsized-images.js
Lines 99 to 100 in f212b1f