Support srcset in ContentfulImageTagHelper about contentful.net HOT 14 CLOSED

Excellent suggestion! I'll look into this.

from contentful.net.

I think it might get a bit bloated to add array properties for all values on a contentful-image directly.

What do you think of an approach similar to this:

<contentful-source-sets sizes="1x, 2x">
   <contentful-image width="500" etc... />
   <contentful-image width="1000" etc... />
</contentful-source-sets>

This allows greater flexibility; You can set a completely different url for different sizes if you'd like, also control things like cropping and padding differently.

from contentful.net.

I see your point. But you should also consider that the next logical step would be support responsive images (https://responsiveimages.org/) where you have several images with different aspect ratios which are triggered by media queries.

So things like:

<picture>
  <source media="(min-width: 40em)"
    srcset="big.jpg 1x, big-hd.jpg 2x">
  <source 
    srcset="small.jpg 1x, small-hd.jpg 2x">
  <img src="fallback.jpg" alt="">
</picture>

Things like cropping and padding might be more apropriate for this scenario as with srcset you would usually use the same image at different resolutions.

from contentful.net.

True, I was considering allowing to choose how the tag should be rendered (as a picture or an img with sets), I think as a first iteration just img with srcset would be fine.

from contentful.net.

Just to give you an overall perspective:

In my current project we are in fact using <picture> with assets taken from contentful to support responsive images. In Contentful we do this by defining a collection of assets. Each asset represents the highest resolution for a specific aspect ratio.

Inside the <picture> element each asset will result in a <source> entry. Inside the <source> entry we generate the srcset from a list of fixed widths that we want to support. Sometimes we also want to specify the jpeg quality for each resolution as the quality is very dependent of the final resolution.

At a first step I would agree that it is sufficient to support <img> with srcset.

from contentful.net.

Cool, hopefully this will save you some work then.

I will publish a new branch soon that you could try out and give some feedback on.

from contentful.net.

A PR #88 is now open. As a next iteration we could look at adding support for <picture> as well. Maybe in the same tag or in a new one.

Let me know what you think and if it works for you. Here's an example that works on my machine 😄

<contentful-source-sets>
    <contentful-source is-default="true" size="500w" url="@Model.First().Image.FirstOrDefault()?.File.Url" width="200" height="200" resize-behaviour="Pad" background-color="#cc0000" />
    <contentful-source is-default="false" size="1000w" url="@Model.First().Image.FirstOrDefault()?.File.Url" width="500" height="500" resize-behaviour="Pad" background-color="#ccffff" />
</contentful-source-sets>

from contentful.net.

Thanks for the quick implementation!

I'm not convinced that specifying the url in every source-element is really convenient. I would rather see it defined once in the parent element. Maybe overriding the url could be an optional feature, if needed. So a standard case would look more like:

<contentful-image asset="@Model.First().Image">
    <contentful-source is-default="true" size="500w" width="200" height="200" resize-behaviour="Pad" background-color="#cc0000" />
    <contentful-source is-default="false" size="1000w" width="500" height="500" resize-behaviour="Pad" background-color="#ccffff" />
</contentful-source-sets>

We could also define that the properties specified on the parent element refer to the default, so that the is-default property as well as repeating the same property is not required.

Again, my reasoning is that there is no reason I can think of where images in the srcset would have different content. Another thing is the picture/source stuff, where you WANT to have different aspect ratios, etc. but the whole purpose of srcset is to give the browser the same image in different resolutions in order to save bandwidth.

The nice thing would also to have the <contentful-image> work as a standalone tag helper that produces <img> elements with a simple src attribute. Then, when needed, you add child elements to fill the srcset and sizes attributes.

Edit: I noticed that my last comment did not format property as I forgot to mark the html elements as code. Because of this they were not visible in the post. This might have changed the meaning in some sentences. It is fixed now. Please see the updated version.

from contentful.net.

Yeah, I agree in most cases when using images from Contentful you'd want the same exact url. I still like the flexibility this brings though, but the repetition becomes painful I can see.

The idea of using a contentful-image as a parent is really interesting, I'll look into that tomorrow. That'd avoid repetition and make it cleaner overall.

from contentful.net.

Pushed an update. Refactored so that you can now use a contentful-image instead with defaults carrying over to the children. Example:

<contentful-image url="@product.Image.FirstOrDefault()?.File.Url" width="200" height="200" resize-behaviour="Pad" background-color="#cc0000">
                    <contentful-source size="200w" />
                    <contentful-source size="800w" width="800" />
</contentful-image>

Check it out and see how it works for you.

from contentful.net.

Hey,

I just had a look. So far it works quite well for our use cases. Some minor observations:

• At the moment not specifying a "size" attribute will result in an invalid srcset entry since a size value is required. At the same time when using the "w" and "h" sizes you have to specify the physical width or height of the image, which we also already pass to contentful for rescaling. So I was thinking instead of having to repeat ourselves, maybe we can provide a smart default to size, so that when the user doesn't specify an explicit value it will automatically use the specified height or width.
• This is not directly related, but I noticed that currently you have to specify an asset id or an url in the tag helper. Both require that you have to delve into a subobject (either File or SystemProperties) and in some scenarios you might not even know in the view if you have a fully loaded asset with the url or if you only have the id. I think it would be beneficial to have the option to set the asset object directly and then let the tag helper either use the url or load the asset by Id if no url is present. This would also open up the possibility of other future enhancements, like using the asset description as the alt-Attribute.

What do you think?

from contentful.net.

I like both the suggestions and they shouldn't be too hard to fix. I'll get on it and let you know when you can test it.

from contentful.net.

PR updated with your suggestions. I like how this is turning out. Feel free to test and let me know if you have any other feedback. I'll try to get this merged and released today or tomorrow.

from contentful.net.

New version out on NuGet 0.7.1-beta.

Keep the improvement suggestions coming! 😄

from contentful.net.