From d3183a673336931670c11d2d1d7eff1c284b1377 Mon Sep 17 00:00:00 2001 From: Frederick Fogerty Date: Sat, 30 Jun 2018 11:24:02 +1200 Subject: [PATCH] fix: alt text no longer cause images to render at wrong dimensions (#146) * test: add failing test for images with alt tag * fix: fix image rendering with alt tag #41 * chore: add babel-polyfill to add Promises support for IE 11 * test: add test for fluid behaviour * fix: change empty image to use data-uri to fix ie 11 support --- package.json | 1 + src/react-imgix.js | 12 +++- test/integration/react-imgix.test.js | 56 +++++++++++++++++- test/setupIntegration.js | 2 + test/unit/react-imgix.test.js | 88 ++++++++++++++++++++++++++-- 5 files changed, 150 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index a8100239..1ccfa847 100644 --- a/package.json +++ b/package.json @@ -58,6 +58,7 @@ "babel-loader": "^7.1.4", "babel-plugin-inline-package-json": "^2.0.0", "babel-plugin-transform-object-assign": "^6.22.0", + "babel-polyfill": "^6.26.0", "babel-preset-env": "^1.6.1", "babel-preset-react": "^6.24.1", "babel-preset-stage-0": "^6.24.1", diff --git a/src/react-imgix.js b/src/react-imgix.js index 6d8a81dd..efe50317 100644 --- a/src/react-imgix.js +++ b/src/react-imgix.js @@ -6,6 +6,11 @@ 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="; + const PACKAGE_VERSION = require("../package.json").version; const roundToNearest = (size, precision) => @@ -109,7 +114,7 @@ export default class ReactImgix extends Component { type, ...other } = this.props; - let _src = null; + let _src = EMPTY_IMAGE_SRC; let srcSet = null; let _component = component; @@ -146,11 +151,14 @@ export default class ReactImgix extends Component { srcSet = `${dpr2} 2x, ${dpr3} 3x`; } + let _alt = (this.props.imgProps || {}).alt; + let childProps = { ...this.props.imgProps, className: this.props.className, width: other.width <= 1 ? null : other.width, - height: other.height <= 1 ? null : other.height + height: other.height <= 1 ? null : other.height, + alt: this.state.mounted || aggressiveLoad ? _alt : undefined }; switch (type) { diff --git a/test/integration/react-imgix.test.js b/test/integration/react-imgix.test.js index 9c5e147e..384e8682 100644 --- a/test/integration/react-imgix.test.js +++ b/test/integration/react-imgix.test.js @@ -3,7 +3,7 @@ import Imgix from "react-imgix"; import React from "react"; import { mount, shallow } from "enzyme"; -const src = "http://assets.imgix.net/unsplash/lighthouse.jpg"; +const src = "https://assets.imgix.net/examples/pione.jpg"; let containerDiv; beforeEach(() => { containerDiv = global.document.createElement("div"); @@ -18,6 +18,22 @@ const renderIntoContainer = element => { return mount(element, { attachTo: containerDiv }); }; +const renderAndWaitForImageLoad = async element => { + return new Promise((resolve, reject) => { + let renderedEl; + const elementWithOnMounted = React.cloneElement(element, { + onMounted: () => {}, + imgProps: { + ...(element.props.imgProps || {}), + onLoad: () => { + setImmediate(() => resolve(renderedEl)); + } + } + }); + renderedEl = renderIntoContainer(elementWithOnMounted); + }); +}; + describe("When in default mode", () => { const renderImage = () => renderIntoContainer(); @@ -31,4 +47,42 @@ describe("When in default mode", () => { .props().src ).toContain(src); }); + + it("should render properly with an alt tag set", async () => { + const renderedImage = await renderAndWaitForImageLoad( + + ); + + let { width, height } = renderedImage.getDOMNode().getBoundingClientRect(); + + expect({ width, height }).toMatchObject({ + width: 532, + height: 800 + }); + }); + it("should render image with dimensions of the size of the element", async () => { + const sut = renderIntoContainer( +
+ + +
+ ); + + let renderedSrc = sut.find("img").props().src; + expect(renderedSrc).toContain(`w=266`); + expect(renderedSrc).toContain(`h=400`); + }); }); diff --git a/test/setupIntegration.js b/test/setupIntegration.js index cfac7edb..2f948ab0 100644 --- a/test/setupIntegration.js +++ b/test/setupIntegration.js @@ -1,5 +1,7 @@ require("./setup"); +require("babel-polyfill"); + import { configure } from "enzyme"; import Adapter from "enzyme-adapter-react-16"; const configureEnzymeWithAdapter = () => { diff --git a/test/unit/react-imgix.test.js b/test/unit/react-imgix.test.js index 51982fa6..a0868f34 100644 --- a/test/unit/react-imgix.test.js +++ b/test/unit/react-imgix.test.js @@ -1,13 +1,39 @@ import sinon from "sinon"; import React from "react"; import ReactDOM from "react-dom"; -import { shallow } from "enzyme"; +import { shallow, mount } from "enzyme"; import PropTypes from "prop-types"; import Imgix from "react-imgix"; const src = "http://domain.imgix.net/image.jpg"; let sut, vdom, instance; +const EMPTY_IMAGE_SRC = + "data:image/gif;base64,R0lGODlhAQABAAD/ACwAAAAAAQABAAACADs="; + +const renderImageAndBreakInStages = async ({ + element, + mockImage = , + afterFirstRender = async () => {}, + afterSecondRender = async () => {} +}) => { + sinon.stub(ReactDOM, "findDOMNode").callsFake(() => mockImage); + + const sut = shallow(element, { + disableLifecycleMethods: true + }); + + await afterFirstRender(sut); + + await sut.instance().componentDidMount(); + sut.update(); + + await afterSecondRender(sut); + + ReactDOM.findDOMNode.restore(); + + return sut; +}; describe("When using aggressiveLoad", () => { const renderImage = () => @@ -33,9 +59,9 @@ describe("When in image mode", () => { shallow(, { disableLifecycleMethods: true }); - it("the rendered element's src should be null", () => { + it("the rendered element's src should be empty", () => { const props = renderImage().props(); - expect(props.src).toBe(null); + expect(props.src).toBe(EMPTY_IMAGE_SRC); expect(props.srcSet).toBe(null); }); }); @@ -328,7 +354,7 @@ describe("When in background mode", () => { it("the element's url() should not be set", () => { expect(sut.props()).toMatchObject({ style: { - backgroundImage: null, + backgroundImage: `url('${EMPTY_IMAGE_SRC}')`, backgroundSize: "cover" } }); @@ -579,9 +605,60 @@ describe("When using the component", () => { expect(sut.props().width).toEqual(width); }); + it("an alt attribute should be set if loading aggressively", async () => { + const imgProps = { + alt: "Example alt attribute" + }; + sut = await renderImageAndBreakInStages({ + element: ( + + ), + afterFirstRender: async sut => { + expect( + sut + .find("img") + .first() + .props().alt + ).toEqual(imgProps.alt); + } + }); + }); + it("an alt attribute should only be set after the component's size has been found", async () => { + const imgProps = { + alt: "Example alt attribute" + }; + sut = await renderImageAndBreakInStages({ + element: ( + + ), + afterFirstRender: async sut => { + expect( + sut + .find("img") + .first() + .props().alt + ).toBeFalsy(); + }, + afterSecondRender: async sut => { + expect( + sut + .find("img") + .first() + .props().alt + ).toEqual(imgProps.alt); + } + }); + }); + it("any attributes passed via imgProps should be added to the rendered element", () => { const imgProps = { - alt: "Example alt attribute", "data-src": "https://mysource.imgix.net/demo.png" }; sut = shallow( @@ -591,7 +668,6 @@ describe("When using the component", () => { } ); - expect(sut.props().alt).toEqual(imgProps.alt); expect(sut.props()["data-src"]).toEqual(imgProps["data-src"]); });