Is there any reason you don't add a height attribute to fallback image?

sitegeist / sms-responsive-images

This TYPO3 extension provides ViewHelpers and configuration to render valid responsive images based on TYPO3's image cropping tool.

GNU General Public License v2.0

34 stars 18 forks source link

Is there any reason you don't add a height attribute to fallback image? #78

Open ktallafus opened 3 years ago

ktallafus commented 3 years ago

https://github.com/sitegeist/sms-responsive-images/blob/1a559c160c0e260c17ad2f974365807748c34b7f/Classes/Utility/ResponsiveImagesUtility.php#L221

You could also do $fallbackTag->addAttribute('height', $fallbackImage->getProperty('height')); like it's done in f:media. Or am I missing something?

s2b commented 3 years ago

I think the reason for this was that the height value will be incorrect in most cases. By definition, the different sources of a picture tag will have different aspect ratios, so there can‘t be a correct height.

It can be argued that I should just choose the height of one variant or the one of the fallback image, but I‘m not sure what to choose here. Apart from that it would probably be a breaking change...

There probably will be a solution to that problem in the HTML5 specification:

https://twitter.com/jensimmons/status/1365066057774493697

bernhardberger commented 3 years ago

Btw. the viewhelper is broken when you use an image that doesn't have a width in sys_file_metadata (which can be the case when you're trying to use a file that isn't in your file storage but a static resource from your /Resources/Public/Images folder..)

s2b commented 3 years ago

Could you give me a concrete example? This worked for me:

<sms:image
    src="EXT:sms_responsive_images/Documentation/Images/AdministratorManual/ConstantsEditor.png"
    srcset="200, 400, 600"
    width="400"
/>