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

Overly-verbose results when image has zero width #34

Open
eeeps opened this issue Sep 26, 2018 · 5 comments
Open

Overly-verbose results when image has zero width #34

eeeps opened this issue Sep 26, 2018 · 5 comments
Assignees

Comments

@eeeps
Copy link

eeeps commented Sep 26, 2018

Ran the bookmarklet on a page with an image whose initial state was display: none today. Hoo boy.

Here's a reduced test case: https://codepen.io/eeeps/pen/YJKPew

And the results:

screen shot 2018-09-26 at 2 49 24 pm

@eeeps
Copy link
Author

eeeps commented Sep 26, 2018

I wonder if the output here should be "Try using sizes="0px"", or if the bookmarklet can be even smarter about display: none’d images than that (skip them?).

@ausi ausi self-assigned this Sep 26, 2018
@ausi ausi added the bug label Sep 26, 2018
@ausi
Copy link
Owner

ausi commented Sep 26, 2018

I think the specific rule that checks the sizes attribute should ignore display:none images.

But maybe we should add a new rule that checks for display:none?
Something like “The image seems to be hidden. It should be removed from the webpage to reduce bandwidth.”.

@eeeps
Copy link
Author

eeeps commented Sep 26, 2018

Sounds good to me. There are good use cases for having the image preloaded but initially invisible -- but most of the time, best practice is going to be to lazy-load it when whatever event that makes it visible happens.

@danielbachhuber
Copy link

Here's an example of an image hidden <768px:

image

However, once I added sizes="(min-width: 2540px) 2400px, (min-width: 780px) calc(93.1vw + 54px), 100vw", the linter no longer complained.

@ausi
Copy link
Owner

ausi commented Jan 20, 2022

Fixed the overly verbose suggestion in 51ecc89

I keep this issue open for the feature to report warnings for display:none images in general.

@ausi ausi added enhancement and removed bug labels Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants