(refactor) allow ['fluid'] slotSize fixes #72 (#74)

potench · web-flow · commit 08a9b1c10ca6 · 2018-03-12T17:40:28.000-07:00
* (refactor) allow ['fluid'] slotSize

* (refactor) remove logs and broken test
diff --git a/.gitignore b/.gitignore
@@ -4,4 +4,5 @@ lib/
 dist/
 coverage/
 npm-debug.log
+yarn-error.log
 .DS_Store
diff --git a/src/Bling.js b/src/Bling.js
@@ -1,13 +1,13 @@
 /* eslint-disable react/sort-comp */
-import React, {Component} from "react";
+import React, { Component } from "react";
 import PropTypes from "prop-types";
 import ReactDOM from "react-dom";
 import invariant from "invariant";
 import deepEqual from "deep-equal";
 import hoistStatics from "hoist-non-react-statics";
 import Events from "./Events";
 import filterPropsSimple from "./utils/filterProps";
-import {createManager, pubadsAPI} from "./createManager";
+import { createManager, pubadsAPI } from "./createManager";
 /**
  * An Ad Component using Google Publisher Tags.
  * This component should work standalone w/o context.
@@ -387,8 +387,8 @@ class Bling extends Component {
     }
 
     componentWillReceiveProps(nextProps) {
-        const {propsEqual} = Bling._config;
-        const {sizeMapping} = this.props;
+        const { propsEqual } = Bling._config;
+        const { sizeMapping } = this.props;
         if (
             (nextProps.sizeMapping || sizeMapping) &&
             !propsEqual(nextProps.sizeMapping, sizeMapping)
@@ -400,7 +400,7 @@ class Bling extends Component {
     shouldComponentUpdate(nextProps, nextState) {
         // if adUnitPath changes, need to create a new slot, re-render
         // otherwise, just refresh
-        const {scriptLoaded, inViewport} = nextState;
+        const { scriptLoaded, inViewport } = nextState;
         const notInViewport = this.notInViewport(nextProps, nextState);
         const inViewportChanged = this.state.inViewport !== inViewport;
         const isScriptLoaded = this.state.scriptLoaded !== scriptLoaded;
@@ -412,7 +412,7 @@ class Bling extends Component {
             return true;
         }
 
-        const {filterProps, propsEqual} = Bling._config;
+        const { filterProps, propsEqual } = Bling._config;
         const refreshableProps = filterProps(
             Bling.refreshableProps,
             this.props,
@@ -430,7 +430,6 @@ class Bling extends Component {
         const shouldRefresh =
             !shouldRender &&
             !propsEqual(refreshableProps.props, refreshableProps.nextProps);
-        // console.log(`shouldRefresh: ${shouldRefresh}, shouldRender: ${shouldRender}, isScriptLoaded: ${isScriptLoaded}, syncCorrelator: ${Bling._adManager._syncCorrelator}`);
 
         if (shouldRefresh) {
             this.configureSlot(this._adSlot, nextProps);
@@ -479,12 +478,12 @@ class Bling extends Component {
     }
 
     onScriptLoaded() {
-        const {onScriptLoaded} = this.props;
+        const { onScriptLoaded } = this.props;
 
         if (this.getRenderWhenViewable()) {
             this.foldCheck();
         }
-        this.setState({scriptLoaded: true}, onScriptLoaded); // eslint-disable-line react/no-did-mount-set-state
+        this.setState({ scriptLoaded: true }, onScriptLoaded); // eslint-disable-line react/no-did-mount-set-state
     }
 
     onScriptError(err) {
@@ -509,7 +508,10 @@ class Bling extends Component {
         if (Array.isArray(slotSize) && Array.isArray(slotSize[0])) {
             slotSize = slotSize[0];
         }
-        if (slotSize === "fluid") {
+        if (
+            slotSize === "fluid" ||
+            (Array.isArray(slotSize) && slotSize[0] === "fluid")
+        ) {
             slotSize = [0, 0];
         }
 
@@ -519,7 +521,7 @@ class Bling extends Component {
             this.viewableThreshold
         );
         if (inViewport) {
-            this.setState({inViewport: true});
+            this.setState({ inViewport: true });
         }
     }
 
@@ -594,12 +596,12 @@ class Bling extends Component {
     }
 
     notInViewport(props = this.props, state = this.state) {
-        const {inViewport} = state;
+        const { inViewport } = state;
         return this.getRenderWhenViewable(props) && !inViewport;
     }
 
     defineSlot() {
-        const {adUnitPath, outOfPage} = this.props;
+        const { adUnitPath, outOfPage } = this.props;
         const divId = this._divId;
         const slotSize = this.getSlotSize();
 
@@ -690,7 +692,7 @@ class Bling extends Component {
     }
 
     display() {
-        const {content} = this.props;
+        const { content } = this.props;
         const divId = this._divId;
         const adSlot = this._adSlot;
 
@@ -735,8 +737,8 @@ class Bling extends Component {
     }
 
     render() {
-        const {scriptLoaded} = this.state;
-        const {id, outOfPage, style} = this.props;
+        const { scriptLoaded } = this.state;
+        const { id, outOfPage, style } = this.props;
         const shouldNotRender = this.notInViewport(this.props, this.state);
 
         if (!scriptLoaded || shouldNotRender) {
@@ -753,7 +755,10 @@ class Bling extends Component {
                 slotSize = slotSize[0];
             }
             // https://developers.google.com/doubleclick-gpt/reference?hl=en#googletag.NamedSize
-            if (slotSize === "fluid") {
+            if (
+                slotSize === "fluid" ||
+                (Array.isArray(slotSize) && slotSize[0] === "fluid")
+            ) {
                 slotSize = ["auto", "auto"];
             }
             const emptyStyle = slotSize && {
@@ -780,7 +785,8 @@ class Bling extends Component {
 export default hoistStatics(
     Bling,
     pubadsAPI.reduce((api, method) => {
-        api[method] = (...args) => Bling._adManager.pubadsProxy({method, args});
+        api[method] = (...args) =>
+            Bling._adManager.pubadsProxy({ method, args });
         return api;
     }, {})
 );
diff --git a/test/Bling.spec.js b/test/Bling.spec.js
@@ -1,20 +1,20 @@
 /* eslint-disable react/no-multi-comp */
-import React, {Component} from "react";
+import React, { Component } from "react";
 import PropTypes from "prop-types";
 import ReactTestUtils from "react-dom/test-utils";
 import ShallowRenderer from "react-test-renderer/shallow";
 import Bling from "../src/Bling";
 import Events from "../src/Events";
-import {pubadsAPI, APIToCallBeforeServiceEnabled} from "../src/createManager";
-import {gptVersion, pubadsVersion} from "../src/utils/apiList";
-import {createManagerTest} from "../src/utils/createManagerTest";
+import { pubadsAPI, APIToCallBeforeServiceEnabled } from "../src/createManager";
+import { gptVersion, pubadsVersion } from "../src/utils/apiList";
+import { createManagerTest } from "../src/utils/createManagerTest";
 
 describe("Bling", () => {
     let googletag;
     const stubs = [];
 
     beforeEach(() => {
-        Bling.configure({renderWhenViewable: false});
+        Bling.configure({ renderWhenViewable: false });
         Bling.testManager = createManagerTest();
         googletag = Bling._adManager.googletag;
     });
@@ -43,7 +43,27 @@ describe("Bling", () => {
         );
         const result = renderer.getRenderOutput();
         expect(result.type).to.equal("div");
-        expect(result.props.style).to.eql({width: 728, height: 90});
+        expect(result.props.style).to.eql({ width: 728, height: 90 });
+    });
+
+    it("renders fluid to auto width and height", () => {
+        const renderer = new ShallowRenderer();
+        renderer.render(
+            <Bling adUnitPath="/4595/nfl.test.open" slotSize={"fluid"} />
+        );
+        const result = renderer.getRenderOutput();
+        expect(result.type).to.equal("div");
+        expect(result.props.style).to.eql({ width: "auto", height: "auto" });
+    });
+
+    it("renders ['fluid'] to auto width and height", () => {
+        const renderer = new ShallowRenderer();
+        renderer.render(
+            <Bling adUnitPath="/4595/nfl.test.open" slotSize={["fluid"]} />
+        );
+        const result = renderer.getRenderOutput();
+        expect(result.type).to.equal("div");
+        expect(result.props.style).to.eql({ width: "auto", height: "auto" });
     });
 
     it("returns gpt version", done => {
@@ -239,7 +259,7 @@ describe("Bling", () => {
 
     it("calls getServices on adSlot on clear", () => {
         const instance = new Bling();
-        const adSlot = sinon.mock({getServices: () => {}});
+        const adSlot = sinon.mock({ getServices: () => {} });
         adSlot.expects("getServices").once();
         instance._adSlot = adSlot;
         instance.clear();
@@ -296,16 +316,16 @@ describe("Bling", () => {
             <Bling
                 adUnitPath="/4595/nfl.test.open"
                 sizeMapping={[
-                    {viewport: [0, 0], slot: [320, 50]},
-                    {viewport: [750, 200], slot: [728, 90]},
-                    {viewport: [1050, 200], slot: [1024, 120]}
+                    { viewport: [0, 0], slot: [320, 50] },
+                    { viewport: [750, 200], slot: [728, 90] },
+                    { viewport: [1050, 200], slot: [1024, 120] }
                 ]}
             />
         );
     });
 
     it("reflects targeting props to adSlot", done => {
-        const targeting = {t1: "v1", t2: [1, 2, 3]};
+        const targeting = { t1: "v1", t2: [1, 2, 3] };
 
         Bling.once(Events.RENDER, () => {
             const adSlot = instance.adSlot;
@@ -372,7 +392,7 @@ describe("Bling", () => {
         const instance = ReactTestUtils.renderIntoDocument(
             <Bling
                 adUnitPath="/4595/nfl.test.open"
-                attributes={{attr1: "val1", attr2: "val2"}}
+                attributes={{ attr1: "val1", attr2: "val2" }}
                 slotSize={[300, 250]}
             />
         );
@@ -575,20 +595,53 @@ describe("Bling", () => {
         );
     });
 
+    it("renders slotSize with an array", () => {
+        const renderer = new ShallowRenderer();
+        renderer.render(
+            <Bling adUnitPath="/4595/nfl.test.open" slotSize={[[300, 250]]} />
+        );
+        const result = renderer.getRenderOutput();
+        expect(result.type).to.equal("div");
+        expect(result.props.style).to.eql({ width: 300, height: 250 });
+    });
+
+    it("sets slotSize to 0,0 on foldCheck of 'fluid' or ['fluid']", done => {
+        const isInViewport = sinon.spy(Bling._adManager, "isInViewport");
+
+        class Wrapper extends Component {
+            onSlotRenderEnded = () => {
+                expect(isInViewport.args[0][1].join()).to.equal([0, 0].join());
+                done();
+            };
+            render() {
+                return (
+                    <Bling
+                        adUnitPath="/4595/nfl.test.open"
+                        renderWhenViewable={true}
+                        slotSize={["fluid"]}
+                        onSlotRenderEnded={this.onSlotRenderEnded}
+                    />
+                );
+            }
+        }
+
+        ReactTestUtils.renderIntoDocument(<Wrapper />);
+    });
+
     it("refreshes ad when refreshable prop changes", done => {
         let count = 0;
 
         Bling.syncCorrelator();
 
         class Wrapper extends Component {
             state = {
-                targeting: {prop: "val"}
+                targeting: { prop: "val" }
             };
             onSlotRenderEnded = event => {
                 if (count === 0) {
                     expect(event.slot.getTargeting("prop")).to.equal("val");
                     this.setState({
-                        targeting: {prop: "val2"}
+                        targeting: { prop: "val2" }
                     });
                     count++;
                 } else {
@@ -597,7 +650,7 @@ describe("Bling", () => {
                 }
             };
             render() {
-                const {targeting} = this.state;
+                const { targeting } = this.state;
                 return (
                     <Bling
                         adUnitPath="/4595/nfl.test.open"
@@ -619,13 +672,13 @@ describe("Bling", () => {
 
         class Wrapper extends Component {
             state = {
-                targeting: {prop: "val"}
+                targeting: { prop: "val" }
             };
             onSlotRenderEnded = event => {
                 if (count === 0) {
                     expect(event.slot.getTargeting("prop")).to.equal("val");
                     this.setState({
-                        targeting: {prop: "val2"}
+                        targeting: { prop: "val2" }
                     });
                     count++;
                 } else {
@@ -634,7 +687,7 @@ describe("Bling", () => {
                 }
             };
             render() {
-                const {targeting} = this.state;
+                const { targeting } = this.state;
                 return (
                     <Bling
                         adUnitPath="/4595/nfl.test.open"
@@ -675,7 +728,7 @@ describe("Bling", () => {
                 }
             };
             render() {
-                const {adUnitPath} = this.state;
+                const { adUnitPath } = this.state;
                 return (
                     <Bling
                         adUnitPath={adUnitPath}
diff --git a/yarn.lock b/yarn.lock
@@ -4371,9 +4371,9 @@ preserve@^0.2.0:
   version "0.2.0"
   resolved "https://registry.yarnpkg.com/preserve/-/preserve-0.2.0.tgz#815ed1f6ebc65926f865b310c0713bcb3315ce4b"
 
-prettier@^1.7.0:
-  version "1.7.0"
-  resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.7.0.tgz#47481588f41f7c90f63938feb202ac82554e7150"
+prettier@^1.9.2:
+  version "1.11.1"
+  resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.11.1.tgz#61e43fc4cd44e68f2b0dfc2c38cd4bb0fccdcc75"
 
 prettycli@^1.4.3:
   version "1.4.3"
@@ -4530,9 +4530,9 @@ react-addons-test-utils@^15.0.1:
   version "15.6.0"
   resolved "https://registry.yarnpkg.com/react-addons-test-utils/-/react-addons-test-utils-15.6.0.tgz#062d36117fe8d18f3ba5e06eb33383b0b85ea5b9"
 
-"react-dom@^15.0.1 || ^16.0.0":
-  version "16.0.0"
-  resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.0.0.tgz#9cc3079c3dcd70d4c6e01b84aab2a7e34c303f58"
+react-dom@^16.0.0:
+  version "16.2.0"
+  resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.2.0.tgz#69003178601c0ca19b709b33a83369fe6124c044"
   dependencies:
     fbjs "^0.8.16"
     loose-envify "^1.1.0"
@@ -4546,9 +4546,9 @@ react-test-renderer@^16.0.0:
     fbjs "^0.8.16"
     object-assign "^4.1.1"
 
-"react@^15.0.1 || ^16.0.0":
-  version "16.0.0"
-  resolved "https://registry.yarnpkg.com/react/-/react-16.0.0.tgz#ce7df8f1941b036f02b2cca9dbd0cb1f0e855e2d"
+react@^16.0.0:
+  version "16.2.0"
+  resolved "https://registry.yarnpkg.com/react/-/react-16.2.0.tgz#a31bd2dab89bff65d42134fa187f24d054c273ba"
   dependencies:
     fbjs "^0.8.16"
     loose-envify "^1.1.0"
