feat: implement responsiveness with srcSet and sizes (#159)

* chore(readme): fix typo and update codesandbox examples (#158) * chore(readme): fix typo in readme * chore(readme): edit codesandbox iframes to be links instead * chore(jest): upgrade jest to support new expect functions * test: use srcset and sizes - generateSrcSet -> disableSrcSet - remove aggressiveLoad - remove precision - add sizes top-level prop * chore: update build:watch to build es instead of commonjs * feat: add constants file and targetWidths from imgix.js * test: mock window.screen for unit tests * feat: use srcset and sizes - generateSrcSet -> disableSrcSet - remove - remove precision - add sizes top-level prop - remove empty img - remove defaultWidth and defaultHeight - remove fluid prop - remove auto-sizing behaviour in favour of srcsets - add check to ensure sizes is passed when in image mode and no width and heights have been passed * test: refactor alt test * test: refactor ixlib tests to use expectSrcTo * chore: stop using ...other in props * chore: rename support.js -> constructUrl.js * feat: add fallback srcset when using sizes * feat: update targetWidths with up to date reses, and remove max restriction * chore: remove fixed TODO * docs: update README for new srcSet/sizes * test: add srcSet test for <source> * feat: use 2x and 3x srcSets for pictures * docs: update markdown link * docs: add sizes doc * chore: cleanup unused code * chore: update targetWidth * feat: remove type=bg and add reference to issue * chore: remove availWidth and availHeight constants as they're not used * chore: update fit logic * test: remove type=bg tests * docs: remove reference to unimplemented aspect ratio behaviour * docs: add upgrade guide * chore: refactor some code to be more readable
imgix · Jul 30, 2018 · fa68df6 · fa68df6
1 parent 8aec071
commit fa68df6
Show file tree

Hide file tree

Showing 10 changed files with 886 additions and 1,134 deletions.
diff --git a/README.md b/README.md
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -29,17 +29,17 @@
     "build": "npm run clean && npm run build:commonjs && npm run build:es",
     "build:commonjs": "cross-env BABEL_ENV=commonjs babel src --out-dir lib --source-maps",
     "build:es": "cross-env BABEL_ENV=es babel src --out-dir es --source-maps",
-    "build:watch": "cross-env BABEL_ENV=commonjs babel src --out-dir lib --watch",
+    "build:watch": "cross-env BABEL_ENV=es babel src --out-dir es --watch",
     "clean": "rimraf lib es",
     "prepublish": "npm run build",
     "pretty": "prettier --write \"src/**/*.js\"",
     "release": "standard-version",
     "test:headless": "karma start config/karma/karmaConfigHeadless.js --singleRun",
     "test:headless:watch": "karma start config/karma/karmaConfigHeadless.js",
     "test:browser": "karma start config/karma/karmaConfigLocal.js --singleRun",
-		"test:browser:watch": "karma start config/karma/karmaConfigLocal.js",
-		"test:browser:ci": "karma start config/karma/karmaConfigHeadlessCI.js --singleRun",
-		"test:browser:browserstack": "karma start config/karma/karmaConfigBrowserStack.js --singleRun",
+    "test:browser:watch": "karma start config/karma/karmaConfigLocal.js",
+    "test:browser:ci": "karma start config/karma/karmaConfigHeadlessCI.js --singleRun",
+    "test:browser:browserstack": "karma start config/karma/karmaConfigBrowserStack.js --singleRun",
     "test:browser:all": "npm run test:browser:ci && npm run test:browser:browserstack",
     "test:watch": "jest --watch",
     "test": "jest && npm run test:browser:all"
@@ -68,8 +68,8 @@
     "cross-env": "^5.1.1",
     "enzyme": "^3.3.0",
     "enzyme-adapter-react-16": "^1.1.1",
-    "expect": "^21.2.1",
-    "jest": "^22.0.0",
+    "expect": "^23.4.0",
+    "jest": "^23.4.1",
     "jest-extended": "^0.7.2",
     "karma": "^2.0.4",
     "karma-browserstack-launcher": "^1.3.0",

diff --git a/src/common.js b/src/common.js
@@ -0,0 +1 @@
+export { default as CONSTANTS } from "./constants";
diff --git a/src/constants.js b/src/constants.js
@@ -0,0 +1,3 @@
+const CONSTANTS = {};
+
+export default CONSTANTS;
diff --git a/src/support.js → src/constructUrl.js b/src/support.js → src/constructUrl.js
@@ -101,11 +101,10 @@ var PARAM_EXPANSION = Object.freeze({
 });
 
 var DEFAULT_OPTIONS = Object.freeze({
-  auto: "format", // http://www.imgix.com/docs/reference/automatic#param-auto
-  dpr: 1
+  auto: "format" // http://www.imgix.com/docs/reference/automatic#param-auto
 });
 
-function constructUrl(src, params) {
+function constructUrlFromParams(src, params) {
   var optionKeys = Object.keys(params);
   var fullUrl = optionKeys.reduce(function(uri, key) {
     return uri.addQueryParam(key, params[key]);
@@ -121,7 +120,7 @@ function constructUrl(src, params) {
  * @param  {Object} longOptions map of image API options, in long or short form per expansion rules
  * @return {String}             URL of image src transformed by Imgix
  */
-function processImage(src, longOptions) {
+function constructUrl(src, longOptions) {
   if (!src) {
     return "";
   }
@@ -143,7 +142,7 @@ function processImage(src, longOptions) {
     shortOptions[key] = val;
   });
 
-  return constructUrl(src, shortOptions);
+  return constructUrlFromParams(src, shortOptions);
 }
 
-module.exports = processImage;
+export default constructUrl;
diff --git a/src/react-imgix.js b/src/react-imgix.js
@@ -4,44 +4,20 @@ import ReactDOM from "react-dom";
 import React, { Component } from "react";
 import PropTypes from "prop-types";
 
-import processImage from "./support.js";
-
-// Best way to include an img with an empty src https://stackoverflow.com/a/5775621/515634 and https://stackoverflow.com/a/19126281/515634
-// Using '//:0' doesn't work in IE 11, but using a data-uri works.
-const EMPTY_IMAGE_SRC =
-  "data:image/gif;base64,R0lGODlhAQABAAD/ACwAAAAAAQABAAACADs=";
+import targetWidths from "./targetWidths";
+import constructUrl from "./constructUrl";
 
 const PACKAGE_VERSION = require("../package.json").version;
-
-const roundToNearest = (size, precision) =>
-  precision * Math.ceil(size / precision);
+const NODE_ENV = process.env.NODE_ENV;
 
 const isStringNotEmpty = str =>
   str && typeof str === "string" && str.length > 0;
 const buildKey = idx => `react-imgix-${idx}`;
 
-const validTypes = ["bg", "img", "picture", "source"];
-
-const defaultMap = {
-  width: "defaultWidth",
-  height: "defaultHeight"
-};
-
-const findSizeForDimension = (dim, props = {}, state = {}) => {
-  if (props[dim]) {
-    return props[dim];
-  } else if (props.fluid && state[dim]) {
-    return roundToNearest(state[dim], props.precision);
-  } else if (props[defaultMap[dim]]) {
-    return props[defaultMap[dim]];
-  } else {
-    return 1;
-  }
-};
+const validTypes = ["img", "picture", "source"];
 
 export default class ReactImgix extends Component {
   static propTypes = {
-    aggressiveLoad: PropTypes.bool,
     auto: PropTypes.array,
     children: PropTypes.any,
     className: PropTypes.string,
@@ -51,136 +27,158 @@ export default class ReactImgix extends Component {
     entropy: PropTypes.bool,
     faces: PropTypes.bool,
     fit: PropTypes.string,
-    fluid: PropTypes.bool,
-    generateSrcSet: PropTypes.bool,
+    disableSrcSet: PropTypes.bool,
     onMounted: PropTypes.func,
+    sizes: PropTypes.string,
     src: PropTypes.string.isRequired,
     type: PropTypes.oneOf(validTypes),
     width: PropTypes.number,
     height: PropTypes.number,
-    defaultHeight: PropTypes.number,
-    defaultWidth: PropTypes.number,
     disableLibraryParam: PropTypes.bool
   };
   static defaultProps = {
-    aggressiveLoad: false,
     auto: ["format"],
     entropy: false,
     faces: true,
     fit: "crop",
-    fluid: true,
-    generateSrcSet: true,
+    disableSrcSet: false,
     onMounted: () => {},
-    precision: 100,
     type: "img"
   };
-  state = {
-    width: null,
-    height: null,
-    mounted: false
-  };
 
-  forceLayout = () => {
+  componentDidMount = () => {
     const node = ReactDOM.findDOMNode(this);
-    this.setState({
-      width: node.scrollWidth,
-      height: node.scrollHeight,
-      mounted: true
-    });
     this.props.onMounted(node);
   };
 
-  componentDidMount = () => {
-    this.forceLayout();
-  };
+  buildSrcs = () => {
+    const props = this.props;
+    const {
+      width,
+      height,
+      entropy,
+      faces,
+      auto,
+      customParams,
+      disableLibraryParam,
+      disableSrcSet,
+      type
+    } = props;
+
+    let crop = false;
+    if (faces) crop = "faces";
+    if (entropy) crop = "entropy";
+    if (props.crop) crop = props.crop;
+
+    let fit = false;
+    if (entropy || faces) fit = "crop";
+    if (props.fit) fit = props.fit;
+
+    const fixedSize = width != null || height != null;
+
+    const srcOptions = {
+      auto,
+      ...customParams,
+      crop,
+      fit,
+      ...(disableLibraryParam ? {} : { ixlib: `react-${PACKAGE_VERSION}` }),
+      ...(fixedSize && height ? { height } : {}),
+      ...(fixedSize && width ? { width } : {})
+    };
 
-  _findSizeForDimension = dim =>
-    findSizeForDimension(dim, this.props, this.state);
+    const src = constructUrl(this.props.src, srcOptions);
+
+    let srcSet;
+
+    if (disableSrcSet) {
+      srcSet = src;
+    } else {
+      if (fixedSize || type === "source") {
+        const dpr2 = constructUrl(this.props.src, { ...srcOptions, dpr: 2 });
+        const dpr3 = constructUrl(this.props.src, { ...srcOptions, dpr: 3 });
+        srcSet = `${dpr2} 2x, ${dpr3} 3x`;
+      } else {
+        const buildSrcSetPair = targetWidth => {
+          const url = constructUrl(this.props.src, {
+            ...srcOptions,
+            width: targetWidth
+          });
+          return `${url} ${targetWidth}w`;
+        };
+        const addFallbackSrc = srcSet => srcSet.concat(src);
+        srcSet = addFallbackSrc(targetWidths.map(buildSrcSetPair)).join(", ");
+      }
+    }
+
+    return {
+      src,
+      srcSet
+    };
+  };
 
   render() {
     const {
-      aggressiveLoad,
       auto,
-      bg,
       children,
       component,
       customParams,
       crop,
       entropy,
       faces,
       fit,
-      generateSrcSet,
-      src,
+      disableSrcSet,
       type,
-      ...other
+      width,
+      height
     } = this.props;
-    let _src = EMPTY_IMAGE_SRC;
-    let srcSet = null;
-    let _component = component;
-
-    let width = this._findSizeForDimension("width");
-    let height = this._findSizeForDimension("height");
 
-    let _crop = false;
-    if (faces) _crop = "faces";
-    if (entropy) _crop = "entropy";
-    if (crop) _crop = crop;
+    // Pre-render checks
+    if (NODE_ENV !== "production") {
+      if (
+        type === "img" &&
+        width == null &&
+        height == null &&
+        this.props.sizes == null &&
+        !this.props._inPicture
+      ) {
+        console.warn(
+          "If width and height are not set, a sizes attribute should be passed."
+        );
+      }
+    }
 
-    let _fit = false;
-    if (entropy) _fit = "crop";
-    if (fit) _fit = fit;
+    let _component = component;
+    const imgProps = this.props.imgProps || {};
 
     let _children = children;
 
-    if (this.state.mounted || aggressiveLoad) {
-      const srcOptions = {
-        auto: auto,
-        ...customParams,
-        crop: _crop,
-        fit: _fit,
-        width,
-        height,
-        ...(this.props.disableLibraryParam
-          ? {}
-          : { ixlib: `react-${PACKAGE_VERSION}` })
-      };
-
-      _src = processImage(src, srcOptions);
-      const dpr2 = processImage(src, { ...srcOptions, dpr: 2 });
-      const dpr3 = processImage(src, { ...srcOptions, dpr: 3 });
-      srcSet = `${dpr2} 2x, ${dpr3} 3x`;
-    }
-
-    let _alt = (this.props.imgProps || {}).alt;
+    const { src, srcSet } = this.buildSrcs();
 
     let childProps = {
       ...this.props.imgProps,
+      sizes: this.props.sizes,
       className: this.props.className,
-      width: other.width <= 1 ? null : other.width,
-      height: other.height <= 1 ? null : other.height,
-      alt: this.state.mounted || aggressiveLoad ? _alt : undefined
+      width: width <= 1 ? null : width,
+      height: height <= 1 ? null : height,
+      alt: imgProps.alt
     };
 
     switch (type) {
       case "bg":
-        if (!component) {
-          _component = "div";
-        }
-        childProps.style = {
-          backgroundSize: "cover",
-          backgroundImage: isStringNotEmpty(_src) ? `url('${_src}')` : null,
-          ...childProps.style
-        };
+        // TODO: Remove in v9
+        throw new Error(
+          `type='bg' has been removed in this version of react-imgix. If you would like this re-implemented please give this issues a thumbs up: https://github.com/imgix/react-imgix/issues/160`
+        );
         break;
       case "img":
         if (!component) {
           _component = "img";
         }
 
-        if (generateSrcSet) {
+        if (!disableSrcSet) {
           childProps.srcSet = srcSet;
         }
-        childProps.src = _src;
+        childProps.src = src;
         break;
       case "source":
         if (!component) {
@@ -189,14 +187,15 @@ export default class ReactImgix extends Component {
 
         // strip out the "alt" tag from childProps since it's not allowed
         delete childProps.alt;
+        delete childProps.src;
 
         // inside of a <picture> element a <source> element ignores its src
         // attribute in favor of srcSet so we set that with either an actual
         // srcSet or a single src
-        if (generateSrcSet) {
-          childProps.srcSet = `${_src}, ${srcSet}`;
+        if (disableSrcSet) {
+          childProps.srcSet = src;
         } else {
-          childProps.srcSet = _src;
+          childProps.srcSet = `${src}, ${srcSet}`;
         }
         // for now we'll take media from imgProps which isn't ideal because
         //   a) this isn't an <img>
@@ -220,7 +219,10 @@ export default class ReactImgix extends Component {
         // make sure all of our children have key set, otherwise we get react warnings
         _children =
           React.Children.map(children, (child, idx) =>
-            React.cloneElement(child, { key: buildKey(idx) })
+            React.cloneElement(child, {
+              key: buildKey(idx),
+              _inPicture: true
+            })
           ) || [];
 
         // look for an <img> or <ReactImgix type='img'> - at the bare minimum we
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		export { default as CONSTANTS } from "./constants";
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		const CONSTANTS = {};

		export default CONSTANTS;