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

A fitting image source should be available for all screen sizes #60

Open
rocc-o opened this issue Dec 30, 2021 · 4 comments
Open

A fitting image source should be available for all screen sizes #60

rocc-o opened this issue Dec 30, 2021 · 4 comments

Comments

@rocc-o
Copy link

rocc-o commented Dec 30, 2021

Hello, I get fitting image source problem as follows:
At a viewport of 320x427 the image was displayed 272 pixels wide, but the closest provided image has a width of 603 which is 55% off. The affected viewports are 300x169-340x453.

Strange though that in my source is specified media="(min-width: 568px) for srcset width 603w, while media="(max-width: 567px)" has the 320w and I believe is this one that Lint should consider for viewports 300x169-340x453. Am I missing something?

Here's my markup:

<picture class="promo__image">
	<source
		media="(max-width: 567px)"
		sizes="(min-width: 1740px) 630px, (min-width: 1040px) calc(37.94vw - 23px), 85vw"
		srcset="
			/assets/images/manifesto/webp/portrait/320.webp 320w,
			/assets/images/manifesto/webp/portrait/640.webp 640w
		"
		type="image/webp"
	>
	<source
		media="(min-width: 568px) and (max-width: 2560px)"
		sizes="(min-width: 1740px) 630px, (min-width: 1040px) calc(37.94vw - 23px), 85vw"
		srcset="
			/assets/images/manifesto/webp/landscape/603.webp 603w,
			/assets/images/manifesto/webp/landscape/1260.webp 1260w
		"
		type="image/webp"
	>
	<source
		media="(max-width: 567px)"
		sizes="(min-width: 1740px) 630px, (min-width: 1040px) calc(37.94vw - 23px), 85vw"
		srcset="
			/assets/images/manifesto/jpeg/portrait/320.jpg 320w,
			/assets/images/manifesto/jpeg/portrait/640.jpg 640w
		"
		type="image/jpeg"
	>
	<img
		sizes="(min-width: 1740px) 630px, (min-width: 1040px) calc(37.94vw - 23px), 85vw"
		srcset="
			/assets/images/manifesto/jpeg/landscape/603.jpg 603w,
			/assets/images/manifesto/jpeg/landscape/1260.jpg 1260w
		"
		src="/assets/images/manifesto/jpeg/landscape/1260.jpg"
		alt=""
		loading="lazy"
		width="1260"
		height="709"
	>
</picture> 
@ausi
Copy link
Owner

ausi commented Dec 30, 2021

Can you please try if the error goes away if you remove the type="image/jpeg" from the last <source> element?

@rocc-o
Copy link
Author

rocc-o commented Dec 30, 2021

Yes! It's gone! But why? And also, is fine to leave it like that, without specifying type="image?

@ausi
Copy link
Owner

ausi commented Dec 30, 2021

And also, is fine to leave it like that, without specifying type="image?

Yes, type="image/jpeg" in not needed because every browser supports that anyways.

But why?

Because the linter thinks that maybe some browser doesn’t support image/jpeg in which case they would fallback to the <img>.

@ausi
Copy link
Owner

ausi commented Jan 30, 2022

I’d like to keep this issue open to remind me of adding a check for type="image/jpeg" and similar types.

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

2 participants