lasso-js / lasso

Advanced JavaScript module bundler, asset pipeline and optimizer
580 stars 75 forks source link

Responsive <lasso-img> #151

Open oxala opened 8 years ago

oxala commented 8 years ago

I'm really missing the functionality to have srcset attribute within the <lasso-img> tag. Paths inside srcset attribute are not being processed by lasso. So, after generation i have something like:

<lasso-img src="./images/camera.png" srcset="./images/camera@2x.png 2x"/>

// Turns into

<img src="/static/app$0.0.6/src/module/images/camera.png" srcset="./images/camera@2x.png 2x" width="101" height="86">

Could we have a processing on srcset paths as well?

Please!

patrick-steele-idem commented 8 years ago

Hey @oxala, you are correct that we are missing support for the srcset attribute and I think that would be a nice improvement. Any interest in submitting a Pull Request to add support for processing the srcset attribute? If so, that would be awesome. The code changes would be limited to the following files:

If you are interested, I can definitely provide more guidance. Otherwise, we can add this feature to our backlog, and you can use the <lasso-resource> tag as a workaround. Let me know what. Thanks for opening the issue.

oxala commented 8 years ago

To be fair - I've tried myself implement it before opening the issue. :-(

But if you would give me a bit more guidance in terms of how exactly i can process the srcset value, i could definitely give it a go one more time.

patrick-steele-idem commented 8 years ago

After giving it some more thought we need to make it clear exactly what we want from Lasso. In theory, Lasso could generate the smaller images automatically if you provide the high resolution image. Is that something we want to consider?

The other issue is that parsing the srcset might get kind of ugly to parse. In theory, we could require a JavaScript object similar to the following:

<lasso-img src="./images/camera.png" srcset={
 "1x": "./images/camera@1x.png",
 "2x": "./images/camera@2x.png"
}/>

Thoughts?

oxala commented 8 years ago

To be fair, i initially was thinking about very simple solution for case like:

<img src="image.png" srcset="image@2x.png">

Not specifying the ratio. It should cover most of the scenarios. And personally i never use anything else than just image for any retina screen.

But the object version looks decent as well, i think.

Generating smaller images - i think it's overkill. I do love automation, but i believe there should be a reasonable limit (especially, if at the moment nobody asked for it).

So, going back to srcset. I believe if we will think MVP, single string path should suffice for now. Unless you have some other analysis and know that i'm not the only one asked for this. :-)

mlrawlings commented 7 years ago

In theory, Lasso could generate the smaller images automatically if you provide the high resolution image. Is that something we want to consider?

Maybe. Here's a possible implementation. Given this:

<lasso-img src="./path/to/img.png" width=100 />
  1. <lasso-img> could look at the image referenced by src and based on the aspect-ratio, populate height.
  2. It would then create an image that was 100px wide and place the url for that in src.
  3. If the original image was more than 200px wide (100@2x) then it would generate an image 200px wide and place that in srcset.
  4. It would repeat the process for @3x, @4x, and so on until the image was not large enough or up to a reasonable maximum.

Perhaps it could be made to be opt-in and more explicit:

<lasso-img src="./path/to/img.png" width=100 @2x @3x @4x />

Or to avoid repetition:

<lasso-img src="./path/to/img.png" width=100 @2x-4x />
mlrawlings commented 7 years ago

Another possible syntax:

<lasso-img src="./path/to/img.png" srcset(100x200, 2x-4x) />
<lasso-img src="./path/to/img.png" srcset(100w, 2x-4x) />
<lasso-img src="./path/to/img.png" srcset(200h, 2x-4x) />