diff --git a/e2e-test/cypress/integration/responsive.spec.js b/e2e-test/cypress/integration/responsive.spec.js index 1475403..26d16eb 100644 --- a/e2e-test/cypress/integration/responsive.spec.js +++ b/e2e-test/cypress/integration/responsive.spec.js @@ -1,25 +1,44 @@ +/** + * When 'responsive' is passed to the Image component, cloudinary-core generates a data-src attribute instead of src. + * Then cloudinary.responsive() is called, it will add a 'src' attribute with calculated width according to the size + * of the element's container. + * In these tests we first test for existence of 'data-src' attribute, and then we check that 'src' was also generated. + * Remember that Cypress removes the need to add a wait or timeout, and waits for a test to be true until it's default + * timeout is reached. + */ describe('Responsive Image', () => { beforeEach(() => { // runs before each test in the block cy.visit('/'); cy.get('#responsiveBtn').click(); // Click on button }); + // Here responsive() is called, but since a specified width value was passed (100) alongside responsive, + // The responsive() function does not change the width param of the transformation, because the specified width + // "overrides" the responsiveness. in other words, specifying a width value is stronger then the 'responsive' prop. + it('Should generate transformation with specified width value', () => { + cy.get('#responsive-override') + .should('have.attr', 'data-src').should('equal', 'http://res.cloudinary.com/demo/image/upload/c_scale,w_100/sample') + cy.get('#responsive-override') + .should('have.attr', 'src').should('equal', 'http://res.cloudinary.com/demo/image/upload/c_scale,w_100/sample') + cy.get('#responsive-override') + .should('have.attr', 'width').should('equal', '100') + }); it('Responsive Image', () => { cy.get('#responsive') - .should('have.attr', 'data-src').should('equal','http://res.cloudinary.com/demo/image/upload/c_scale,w_auto/sample') + .should('have.attr', 'data-src').should('equal', 'http://res.cloudinary.com/demo/image/upload/c_scale,w_auto/sample') cy.get('#responsive') - .should('have.attr', 'src').should('equal','http://res.cloudinary.com/demo/image/upload/c_scale,w_400/sample') + .should('have.attr', 'src').should('equal', 'http://res.cloudinary.com/demo/image/upload/c_scale,w_400/sample') }); it('Disabled Breakpoints', () => { cy.get('#responsive') - .should('have.attr', 'data-src').should('equal','http://res.cloudinary.com/demo/image/upload/c_scale,w_auto/sample') + .should('have.attr', 'data-src').should('equal', 'http://res.cloudinary.com/demo/image/upload/c_scale,w_auto/sample') cy.get('#disable-breakpoints') - .should('have.attr', 'src').should('equal','http://res.cloudinary.com/demo/image/upload/c_scale,w_330/sample') + .should('have.attr', 'src').should('equal', 'http://res.cloudinary.com/demo/image/upload/c_scale,w_330/sample') }); it('Enabled Breakpoints', () => { cy.get('#responsive') - .should('have.attr', 'data-src').should('equal','http://res.cloudinary.com/demo/image/upload/c_scale,w_auto/sample') + .should('have.attr', 'data-src').should('equal', 'http://res.cloudinary.com/demo/image/upload/c_scale,w_auto/sample') cy.get('#breakpoints') - .should('have.attr', 'src').should('equal','http://res.cloudinary.com/demo/image/upload/c_scale,w_450/sample') + .should('have.attr', 'src').should('equal', 'http://res.cloudinary.com/demo/image/upload/c_scale,w_450/sample') }); }); \ No newline at end of file diff --git a/e2e-test/src/App.js b/e2e-test/src/App.js index 6bd0317..79e3444 100644 --- a/e2e-test/src/App.js +++ b/e2e-test/src/App.js @@ -28,6 +28,9 @@ function App() { {test === 'responsive' &&

Responsive Image

+
+ +
diff --git a/package.json b/package.json index 7a6f105..6c89523 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "webpack-cli": "^3.1.2" }, "dependencies": { - "cloudinary-core": "^2.10.2", + "cloudinary-core": "^2.10.3", "prop-types": "^15.6.2" }, "peerDependencies": { diff --git a/src/components/Image/Image.js b/src/components/Image/Image.js index cfe2da2..2392a80 100644 --- a/src/components/Image/Image.js +++ b/src/components/Image/Image.js @@ -1,9 +1,17 @@ import React, {createRef, Fragment} from 'react'; import CloudinaryComponent from '../CloudinaryComponent'; -import {extractCloudinaryProps, getImageTag, makeElementResponsive, getConfiguredCloudinary} from "../../Util"; +import {extractCloudinaryProps, getImageTag, makeElementResponsive} from "../../Util"; import {Util} from "cloudinary-core"; import PropTypes from "prop-types"; +const RESPONSIVE_OVERRIDE_WARNING = [ + `Warning: passing a number value for width cancels the 'responsive' prop's effect on the image transformation.`, + `The 'responsive' prop affects the image transformation only when width === 'auto'.`, + `Passing 'width="auto" responsive' will affect the actual image width that is fetched from Cloudinary.`, + `The 'responsive' prop causes the Image component to request an image which width is equal to the width of it's container.`, + `When passing 'width="auto" responsive', you can set the element width by passing a 'style' prop` +].join('\n'); + /** * A component representing a Cloudinary served image */ @@ -19,7 +27,12 @@ class Image extends CloudinaryComponent { * @return true when this image element should be made responsive, false otherwise. */ isResponsive = () => { - return this.props.responsive && this.imgElement && this.imgElement.current; + const {responsive, width} = this.getExtendedProps(); + if (responsive && width !== 'auto') { + console.warn(RESPONSIVE_OVERRIDE_WARNING); + } + + return responsive && this.imgElement && this.imgElement.current; } /** @@ -44,20 +57,31 @@ class Image extends CloudinaryComponent { let attributes = {...getImageTag(options).attributes(), ...nonCloudinaryProps}; + //React requires camelCase instead of snake_case attributes + attributes = Util.withCamelCaseKeys(attributes); + // Set placeholder Id if (placeholder && attributes.id) { attributes.id = attributes.id + '-cld-placeholder'; } + // Set dataSrc if lazy loading and not in view + if (!isInView && this.shouldLazyLoad(options)) { + attributes.dataSrc = attributes.dataSrc || attributes.src; + delete attributes.src; + } + + // The data-src attribute was turned into dataSrc by the camelCase function, + // But it's needed by cloudinary-core's responsive() function. Notice that it's not snake_case. + if (attributes.dataSrc) { + attributes['data-src'] = attributes.dataSrc; + } + // Remove unneeded attributes, - ["src", "accessibility", "placeholder"].forEach(attr => { + ['dataSrc', 'accessibility', 'placeholder', 'breakpoints'].forEach(attr => { delete attributes[attr]; }); - // Set src or data-src attribute - const srcAttrName = isInView || !this.shouldLazyLoad(options) ? "src" : "data-src"; - attributes[srcAttrName] = getConfiguredCloudinary(options).url(options.publicId, options); - return attributes; } @@ -65,12 +89,14 @@ class Image extends CloudinaryComponent { * Update this image using cloudinary-core */ update = () => { + const {isInView} = this.state; + if (this.isResponsive()) { const removeListener = makeElementResponsive(this.imgElement.current, this.getOptions()); this.listenerRemovers.push(removeListener); } - if (this.shouldLazyLoad(this.getExtendedProps())) { + if (!isInView && this.shouldLazyLoad(this.getExtendedProps())) { Util.detectIntersection(this.imgElement.current, this.onIntersect); } } @@ -153,7 +179,6 @@ class Image extends CloudinaryComponent {
); } - return } } diff --git a/test/ImageTest.js b/test/ImageTest.js index a29aa42..b7489ae 100644 --- a/test/ImageTest.js +++ b/test/ImageTest.js @@ -94,11 +94,6 @@ describe('Image', () => { expect(tag.find('img').prop('src')).to.match(/fn_wasm:blur.wasm\/sample/); }); - it('should support responsive prop', () => { - let tag = mount(); - tag.setProps({responsive: true}); - expect(tag.find('img').prop('data-src')).to.equal('http://res.cloudinary.com/demo/image/upload/sample'); - }); it('Should support forwarding innerRef to underlying image element', function () { const expected = 'http://res.cloudinary.com/demo/image/upload/sample'; let myRef = React.createRef(); @@ -129,6 +124,17 @@ describe('Image', () => { expect(tag.find('img').prop('src')).to.equal(expected); }); + describe('Responsive', () => { + it('should support responsive prop', () => { + let tag = mount(); + tag.setProps({responsive: true}); + expect(tag.find('img').prop('data-src')).to.equal('http://res.cloudinary.com/demo/image/upload/sample'); + }); + it('should set image width even when responsive prop is passed', () => { + let tag = mount(); + expect(tag.find('img').prop('width')).to.equal(100); + }); + }); describe('Placeholder', () => { it(`should not have opacity and position when placeholder doesn't exists`, () => { const tag = shallow(); @@ -185,7 +191,7 @@ describe('Image', () => { }); describe('Responsive Placeholder With Lazy Loading', () => { let tag = shallow( - + );