Our-Umbraco-TagHelpers icon indicating copy to clipboard operation
Our-Umbraco-TagHelpers copied to clipboard

Our-img height

Open kahlan88 opened this issue 2 years ago • 1 comments
trafficstars

We're using <our-img> with media-item and responsive widths and heights.

According to example 4 in the readme - the helper should output height attribute and in the srcset on <source> tags.

<our-img media-item="Model.Image" width="200" width--s="400" width--m="600" cropalias="mobile" cropalias--m="desktop" />
<picture>
    <source srcset="/media/path/image.jpg?width=600&amp;height=300&amp;rnd=133087885756657361" media="(min-width: 768px)" width="600" height="300">
    <source srcset="/media/path/image.jpg?width=400&amp;height=400&amp;rnd=133087885756657361" media="(min-width: 576px)" width="400" height="400">
    <img alt="Some alt text" width="200" height="200" src="/media/path/image.jpg?width=200&amp;height=200&amp;rnd=133087885756657361" loading="lazy" decoding="async" fetchpriority="low">
</picture>

Our example code:

<our-img
        media-item="@image"
        width="540"
        height="480"
        width--s="590"
        height--s="660"
    />

Output:

<picture>
    <source srcset="/media/path/image.jpg?width=590&amp;rnd=133282895487345646" media="(min-width: 768px)" width="590">
    <img alt="Some alt text" width="540" height="360.02313624678663" src="/media/path/image.jpg?width=540&amp;rnd=133282895487345646" loading="lazy" decoding="async" fetchpriority="low">
</picture>

Expected:

<picture>
    <source srcset="/media/path/image.jpg?width=590&height=480&amp;rnd=133282895487345646" media="(min-width: 768px)" width="590" height="480">
    <img alt="Some alt text" width="540" height="360.02313624678663" src="/media/path/image.jpg?width=540&amp;rnd=133282895487345646" loading="lazy" decoding="async" fetchpriority="low">
</picture>

We need slightly different aspect ratios based on screen size. Is it possible?

I think that height is missing from the output. Am I missing something or is this a defect?

appsettings.json:

"Our.Umbraco.TagHelpers": {
    "OurIMG": {
      "MobileFirst": true,
      "MediaQueries": {
        "Small": 768,
        "Medium": 1024,
        "Large": 1280,
        "ExtraLarge": 1440,
        "ExtraExtraLarge": 1680
      },
      "UseNativeLazyLoading": true,
      "ApplyAspectRatio": false,
      "AlternativeTextMediaTypePropertyAlias": "altText"
    }
  }

kahlan88 avatar Jun 07 '23 17:06 kahlan88

Having looked into the code, I can see that when MediaItem is provided, the height gets omitted and the code takes only the crops into account.

It applies height when providing src instead.

I think it would be great if height was used if provided. There could be a flag to say "ignore crops", but I think just the height being present could be used to tell the tag helper to respect it and ignore crops?

Note: not all sites even use crops - for example none of the sites I generally work on utilises this much, and we'd almost always want to use height to override crops when providing media-item, so it would be really useful to have the option. I will see if I can PR this change.

kahlan88 avatar Jun 08 '23 10:06 kahlan88