benfoster / ImageResizer.FluentExtensions

Fluent Extensions for the ImageResizer image processing module
MIT License
34 stars 10 forks source link

Named configuration expression #7

Open benfoster opened 9 years ago

benfoster commented 9 years ago

To support HTML5 srcset, sizes and <picture> element it should be possible to define named expressions when configuring an ImageUrlBuilder instance.

Right now if we were to use srcset we'd have to do something like this:

<img src="@Url.ImageUrl("image.jpg", img => img.Resize(x => x.Width(300)))"
     srcset="@Url.ImageUrl("image.jpg", img => img.Resize(x => x.Width(600))) 2x, @Url.ImageUrl("image.jpg", img => img.Resize(x => x.Width(900))) 3x"
    alt="" />

Of course we can create an image helper for this:

@Html.BuildImage(builder1, "Alt text")
    .AddSrc("2x", builder2)
    .AddSrc("3x", builder3)
    .Render();

This would still require three ImageUrlBuilder instances to be created. More often than not, when working with responsive images, it's just the size that needs to change depending on the resolution or display size. If you're using other configurations (styling, filters etc.) or modifiers, this means you need to define them again for each builder.

A solution to this is to define named configuration expressions that are applied conditionally when building the image URL. For example:

var builder = new ImageUrlBuilder()
        .Resize(img => img.Resize(x => x.Width(300))) // Default
        .Config("2x")
            .Resize(img => img.Resize(x => x.Width(600)))
        .Config("3x")
            .Resize(img => img.Resize(x => x.Width(900)));

var defaultImage = builder.BuildUrl("someimage.jpg");
var retinaImage = builder.BuildUrl("someimage.jpg", "2x");

Alternatively we could require each builder expression to define a configuration key:

var builder = new ImageUrlBuilder()
        .Resize(img => img.Resize(x => x.Width(300))) // Default
        .Resize("2x", img => img.Resize(x => x.Width(600)))
        .Resize("3x", img => img.Resize(x => x.Width(900)));

var defaultImage = builder.BuildUrl("someimage.jpg");
var retinaImage = builder.BuildUrl("someimage.jpg", "2x");
lilith commented 9 years ago

I had to read this through twice to figure out what was going on.

I'd suggest implementing a .Copy() method and letting users track configurations via variables instead. Otherwise we're expanding the mental model.

var builder = new ImageUrlBuilder() .Resize(img => img.Resize(x => x.Width(300))) // Default var builder2x = builder.Copy().Resize(img => img.Resize(x => x.Width(600))); var builder3x = builder.Copy().Resize(img => img.Resize(x => x.Width(600))); var defaultImage = builder.BuildUrl("someimage.jpg"); var retinaImage = builder2x.BuildUrl("someimage.jpg");

For dppx variations, we have the zoom command that directly corresponds with the pixel density factor, so a helper method to manage that could be nice.

Maybe we should look at this from a different angle - from the HTML side. There, we have to indicate constraints. In most situations we can use these constraints to calculate which changes we need to make to the querystring. Creating a new class to deal with generating both these constraints and creating multiple variants of a provided ImageUrlBuilder seems like a better level for abstraction.

Keep in mind that viewport width and element/image width are not always correlated; It might be a good idea to define an interface that can map viewport constraints (say width) to element width, taking breakpoints into account. This interface would need to know the image css class, I presume, so unless the abstraction generates the whole image element HTML it's a difficult problem to solve perfectly. I know full element HTML generation is likely a bigger scope than you're wanting to tackle, but I'm not sure that ImageUrlBuilder is the best place to put the new functionality.

Eventually I suspect there will be tooling that accesses your generated CSS for the site, looks at your image selector, the original image size, and generates all of the appropriate breakpoints/constraints. CSS parsing is so easy it's almost fun, but it would need to be an offline process; not something you do dynamically.

lilith commented 9 years ago

I suppose, given that srcset x selectors are the only widely supported feature, apply YAGNI and create a helper for specifically that use case. It's not mobile friendly, but it fixes the retina problem.

I'd also apply Slimmage.js' heuristic for jpeg quality - if dppx > 1.5, use quality 80 instead of 90.

benfoster commented 9 years ago

ImageUrlBuilder.Copy does look like a better option. The reason for adding this functionality into the builder is that I don't want to be tied to any particular web framework so the more that we can do in core, the better.