Output placeholder image as data URI #16

alex-ketch · 2016-09-09T01:28:40Z

Hello again!
I know I said I'd make a PR for how file extensions are handled, but needed this first :)

This PR outputs a placeholder image as a data uri to save network requests, addressing issue #4.

The size of the placeholder image can be configured with a placeholderWidth option, but defaults to 40px wide.

@herrstucki If you have a moment, I would appreciate some extra eyes to ensure things make sense please. In particular this line, where I check if the output width equals placeholder with, then don't write the file but output as data uri instead.
The potential problem with this is if someone wants to actually write out a file at the placeholder width size.

A solution could be to add a flag to resizeImage() specifying whether the file should be written or not.

To Do:

Update README to mention new feature

jstcki · 2016-09-12T09:11:15Z

Nice, thanks!

I don't think it's a common case that someone resizes an image to placeholder width but it would be weird if that file got omitted without notice. So, size=40&placeholder should emit a file and a placeholder data uri. I probably wouldn't add the placeholder width to widthsToGenerate, but use a separate function for the placeholder (e.g. q.defer(resizePlaceholder, placeholderWidth) because we don't need to worry about filenames etc. anyway.
Will the placeholder use the same quality/background settings as the other images or should that be separate too? Maybe make the placeholder option an object which gets merged with default settings?
Maybe use placeholderSize (or placeholder.size if we use an object) as an option name instead of placeholderWidth because of consistency?
Do we need to add height information to the loader output, so the not-yet-loaded image can be scaled properly (this doesn't only apply to the placeholder)?

oscar-b · 2016-09-27T06:53:08Z

I just tried the suggested solution in the updated README, adding a base64 encoded small image to src and the original full image as srcset, and as far as I can see Chrome (53) never displays the small image (tried with network shaping to get it to load slowly). Do you have any reference on this method?

I don't know if it's out of scope for this plugin to recommend how to actually use the placeholders, but it would be nice :)

alex-ketch · 2016-09-27T15:43:12Z

Sorry, I realize I've been lagging very behind. I'll finish this before the week is up.

@oscar-b As for the placeholder image, you're right and the README needs changing.
Using the placeholder as src doesn't work with browsers that support srcSet, as that tag is never displayed. Instead you have to set it as the background image, and set backgroundSize: cover.

React.render(
  <img
      src={responsiveImage.src}
      srcSet={responsiveImage.srcSet}
      style={{backgroundImage: responsiveImage.placeholder; backgroundSize: cover}}
  />, el
);

oscar-b · 2016-09-27T19:48:34Z

@alex-ketch That's a nice way of doing it, never seen that before! I'm trying to figure out if it's feasible to use with responsive images though, since the background-size: cover won't behave the same as a regular image (at least without object-fit) and the tag easily ends up being 0x0 in size before loading.

jstcki · 2016-09-27T21:44:07Z

That's why I think we should add width and height information to the loader output, so you can calculate the proper aspect ratio and set the dimensions on the image.

oscar-b · 2016-09-28T03:42:07Z

Yes that would be very useful.

alex-ketch · 2016-09-28T19:18:53Z

@herrstucki So this is what I'm thinking:

Separate placeholder generation into separate function, outside of widthsToGenerate
Change setting to placeholderSize from placeholderWidth
Add height/width output as part of the output object
- Should we add a convenience feature to set height/width/background automatically? Goal would be to make usage a bit more concise. Would need React & regular CSS output.

Also regarding:

Will the placeholder use the same quality/background settings as the other images or should that be separate too? Maybe make the placeholder option an object which gets merged with default settings?

I don't think full featured settings are necessary for the placeholder images, but open to be convinced otherwise. As those images are ~1KB at 40px wide, but if settings shave off significant bytes, then sure.

jstcki · 2016-10-01T19:56:57Z

@alex-ketch Yeah, let's go for it! 👍 Very exciting. I was thinking of compiling some links to articles about different techniques of using placeholders too.

Actually, @lnhrdt has already added height to the output in #19, so just make sure the placeholder uses that too.

I probably wouldn't add more "convenience" output to the loader, since people may have different use cases (e.g. use the placeholder in canvas or svg). And how would that work in CSS?

oscar-b · 2016-10-05T06:42:44Z

@alex-ketch Do you have any eta for this PR? Eagerly awaiting it :)

alex-ketch · 2016-10-09T14:11:26Z

@oscar-b Sorry for the delays, but I believe this should be it!

Would appreciate extra 👀

jstcki · 2016-10-09T16:19:33Z

@alex-ketch I'm currently on vacation and won't be able to review/publish for at least a week. I've made you a collaborator in case you want to move more quickly 👍

oscar-b · 2016-10-09T14:56:56Z

README.md

@@ -62,7 +73,9 @@ module.exports = {
    ]}
  },
  responsiveLoader: {
-    sizes: [300, 600, 1200, 2000]
+    sizes: [300, 600, 1200, 2000],


webpack 2 doesn't allow this, options should be passed to the plugin afaik.

I haven't really worked with v2 much and will leave updating documentation to someone more knowledgable. We would need to show examples for both versions, as webpack 2 isn't officially out yet.

Basically this, but yeah it's probably for another PR: https://webpack.js.org/how-to/upgrade-from-webpack-1/#loaderoptionplugin-context

oscar-b · 2016-10-09T19:26:08Z

README.md

@@ -38,13 +38,24 @@ Or use it in CSS (only the first resized image will be used, if you use multiple
 }
 ```

+```js
+// Outputs placeholder image as a data URI, and three images with 100, 200, and 300px widths
+const responsiveImage = require('responsive?placeholder=true&sizes[]=100,sizes[]=200,sizes[]=300!myImage.jpg');


Maybe an example using import as well?

What is the syntax for using query configuration with import? Is it the same?
Otherwise I'd rather keep this consistent with the rest of the examples, and create them with the Webpack v2 documentation update.

Yes sounds reasonable :)

oscar-b · 2016-10-09T19:33:33Z

index.js

      const srcset = files.map(f => f.src).join('+","+');

      const images = files.map(f => '{path:' + f.path + ',width:' + f.width + ',height:' + f.height + '}').join(',');

      const firstImagePath = files[0].path;

-      loaderCallback(null, 'module.exports = {srcSet:' + srcset + ',images:[' + images + '],src:' + firstImagePath + ',toString:function(){return ' + firstImagePath + '}};');
+      loaderCallback(null, 'module.exports = {' +


Isn't width and height missing here?

Yes, still too jet lagged. I've added the original image dimensions to the output.

Shouldn't this be the dimensions of the first resized image to align it with src and toString()?

Now that would make sense, wouldn't it 😅
Will update

oscar-b · 2016-10-09T21:11:33Z

README.md

+const responsiveImage = require('responsive?placeholder=true&sizes[]=100,sizes[]=200,sizes[]=300!myImage.jpg');
+
+// responsiveImage.placeholder => 'data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wCEAAIBAQE…'
+React.render(<img src={responsiveImage.src} srcSet={responsiveImage.srcSet} style={{height: responsiveImage.height, width: responsiveImage.width, backgroundImage: 'url("' + responsiveImage.placeholder + '")'}} />, el);


I think you need a background-size: cover in here as well, no?

oscar-b · 2016-10-11T04:49:43Z

I will try to test it in my environment asap, but looks good to me.

oscar-b · 2016-10-17T21:09:13Z

@alex-ketch This seems quite solid, I haven't had any problems using it at all. Nice work!

oscar-b · 2016-10-17T21:14:52Z

And fwiw the following works for ES6 style imports:

import topImage from 'responsive?placeholder=true,sizes[]=500w,sizes[]=1000w!./images/about-us.jpg';

alex-ketch · 2016-10-18T12:45:26Z

@oscar-b Awesome! Thanks for battle-testing this @oscar-b.
I'll merge this if you're off for a little longer @herrstucki, otherwise I'll leave it for you.

jstcki · 2016-10-18T13:36:41Z

index.js

      const srcset = files.map(f => f.src).join('+","+');

      const images = files.map(f => '{path:' + f.path + ',width:' + f.width + ',height:' + f.height + '}').join(',');

      const firstImagePath = files[0].path;

-      loaderCallback(null, 'module.exports = {srcSet:' + srcset + ',images:[' + images + '],src:' + firstImagePath + ',toString:function(){return ' + firstImagePath + '}};');
+      loaderCallback(null, 'module.exports = {' +


Shouldn't this be the dimensions of the first resized image to align it with src and toString()?

jstcki · 2016-10-18T13:39:18Z

index.js

    return q.awaitAll((queueErr, files) => {
+      'use strict'; // eslint-disable-line


Is 'use strict' necessary?

I believe I was getting the following error in older versions of node, complaining about use of const and let, which is why I added it.

SyntaxError: Block-scoped declarations (let, const, function, class) not yet supported outside strict mode

But not with node >= 4, no? If we want to support older versions (I don't really 😁 ), we should add tests for those too in travis.yml.

Thought so too, but it was failing with v4 and v5, only passed with v6.
Let me look into this a bit more...

jstcki · 2016-10-18T13:47:40Z

@alex-ketch thank you so much for the outstanding work and @oscar-b for reviewing and battle-testing! I added two more tiny comments but otherwise I think it's good to go. I'll leave the honor of merging to you 😄

If you'd like I can also give you npm publish rights too (I'll need your username though).

oscar-b · 2016-10-19T20:50:01Z

Just a quick heads up; I'm having difficulties displaying the base64 encoded placeholder image as a background-image in Safari. Don't know why yet. Found some info that Safari doesn't display progressive JPEGs correctly when used as backgrounds, not sure how Jimp handles that (if the original is progressive, will the base64 also be progressive?).

oscar-b · 2016-10-25T20:44:20Z

It wasn't the progressive part, it was using background-image on a <img /> which Safari doesn't seem to like. I refactored it a bit to display the placeholder on the container <div /> instead, works beautifully now!

This reverts commit 0934e9a.

alex-ketch · 2016-10-28T16:24:12Z

Thanks for bring this to a close @herrstucki, I'll update the Readme today to update mention not setting placeholder image as a background on an img tag and we should be good to go I think.

alex-ketch · 2016-11-14T17:38:55Z

@herrstucki All clear to merge this?

jstcki · 2016-11-14T21:33:28Z

@alex-ketch done and released in v0.7.0 ✌️ Thanks for all the hard work!

alex-ketch force-pushed the master branch from b537862 to 42c7260 Compare September 9, 2016 02:14

jstcki mentioned this pull request Oct 1, 2016

Add a height attribute to images output #19

Merged

alex-ketch added 4 commits October 5, 2016 17:59

Initial pass at outputting placeholder image

5b9f01d

Fix failing tests

f1814e1

Update README with placeholder documentation

b551b67

Refactor placeholder image generation

ed151da

alex-ketch force-pushed the master branch from ad4a166 to ed151da Compare October 9, 2016 14:07

Fix option names in README

a85bea3

oscar-b reviewed Oct 9, 2016

View reviewed changes

alex-ketch added 2 commits October 10, 2016 00:10

Output original image dimensions

1dd0c0d

Add tests for outputting dimensions

d382f3d

oscar-b reviewed Oct 9, 2016

View reviewed changes

Add backgroundSize to README

579a9d5

jstcki reviewed Oct 18, 2016

View reviewed changes

jstcki added 3 commits October 28, 2016 16:47

Use first image's width and height

1888075

Remove 'use strict'

0934e9a

Revert "Remove 'use strict'"

7a60f5c

This reverts commit 0934e9a.

jstcki mentioned this pull request Oct 29, 2016

Optimize file size #3

Closed

Update placeholder BGs & ReactDOM.render in README

281a0f2

jstcki merged commit bb3dd0f into dazuaz:master Nov 14, 2016

jstcki mentioned this pull request Dec 29, 2016

Add placeholder support? #4

Closed

alex-ketch mentioned this pull request Feb 17, 2017

Added min, max and steps arguments as an alternative to sizes #31

Merged

		return q.awaitAll((queueErr, files) => {
		'use strict'; // eslint-disable-line

Output placeholder image as data URI #16

Output placeholder image as data URI #16

Conversation

alex-ketch commented Sep 9, 2016 • edited Loading

jstcki commented Sep 12, 2016

oscar-b commented Sep 27, 2016

alex-ketch commented Sep 27, 2016

oscar-b commented Sep 27, 2016

jstcki commented Sep 27, 2016

oscar-b commented Sep 28, 2016

alex-ketch commented Sep 28, 2016 • edited Loading

jstcki commented Oct 1, 2016

oscar-b commented Oct 5, 2016

alex-ketch commented Oct 9, 2016

jstcki commented Oct 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-ketch Oct 9, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oscar-b commented Oct 11, 2016

oscar-b commented Oct 17, 2016

oscar-b commented Oct 17, 2016

alex-ketch commented Oct 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-ketch Oct 18, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jstcki commented Oct 18, 2016

oscar-b commented Oct 19, 2016

oscar-b commented Oct 25, 2016 • edited Loading

alex-ketch commented Oct 28, 2016

alex-ketch commented Nov 14, 2016

jstcki commented Nov 14, 2016

alex-ketch commented Sep 9, 2016 •

edited

Loading

alex-ketch commented Sep 28, 2016 •

edited

Loading

alex-ketch Oct 9, 2016 •

edited

Loading

alex-ketch Oct 18, 2016 •

edited

Loading

oscar-b commented Oct 25, 2016 •

edited

Loading