From 3721a1c0172cdf24c5ad53880e1cadc8d759b9e5 Mon Sep 17 00:00:00 2001 From: kjacovino Date: Tue, 6 Jun 2017 15:32:30 -0400 Subject: [PATCH 1/8] adding creativeSize attribute to pass in a VAST size --- elements/bulbs-video/components/revealed.js | 8 +++++++- .../bulbs-video/elements/rail-player/components/root.js | 2 ++ elements/bulbs-video/elements/rail-player/rail-player.js | 1 + 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/elements/bulbs-video/components/revealed.js b/elements/bulbs-video/components/revealed.js index bd9e8a91..c9b94a87 100644 --- a/elements/bulbs-video/components/revealed.js +++ b/elements/bulbs-video/components/revealed.js @@ -55,6 +55,9 @@ export default class Revealed extends React.Component { let specialCoverage = this.props.targetSpecialCoverage || 'None'; let autoplayInViewBool = typeof this.props.autoplayInView === 'string'; + // Allowing creativeSize to be passed in in root.js + let creativeSize = this.props.creativeSize; + let videoAdConfig = 'None'; if (this.props.disableAds || this.props.video.disable_ads) { videoAdConfig = 'disable-ads'; @@ -79,6 +82,8 @@ export default class Revealed extends React.Component { dimensions ); + + // Making assignment copies here so we can mutate object structure. let videoMeta = Object.assign({}, this.props.video); videoMeta.hostChannel = hostChannel; @@ -171,7 +176,7 @@ export default class Revealed extends React.Component { let type; // See docs (https://support.google.com/dfp_premium/answer/1068325?hl=en) for param info - baseUrl += '?sz=640x480'; + baseUrl += `?sz=${this.props.creativeSize}`; baseUrl += `&iu=/4246/${window.Bulbs.settings.DFP_SITE_CODE}`; baseUrl += '&impl=s'; baseUrl += '&gdfp_req=1'; @@ -361,6 +366,7 @@ Revealed.propTypes = { autoplay: PropTypes.bool, autoplayInView: PropTypes.string, autoplayNext: PropTypes.bool, + creativeSize: PropTypes.object.string, controller: PropTypes.object.isRequired, defaultCaptions: PropTypes.bool, disableAds: PropTypes.bool, diff --git a/elements/bulbs-video/elements/rail-player/components/root.js b/elements/bulbs-video/elements/rail-player/components/root.js index 98f9e12a..73c6f9de 100644 --- a/elements/bulbs-video/elements/rail-player/components/root.js +++ b/elements/bulbs-video/elements/rail-player/components/root.js @@ -32,6 +32,7 @@ export default class Root extends React.Component { Date: Tue, 6 Jun 2017 15:35:33 -0400 Subject: [PATCH 2/8] meant to leave it at 640x480 --- elements/bulbs-video/elements/rail-player/components/root.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/elements/bulbs-video/elements/rail-player/components/root.js b/elements/bulbs-video/elements/rail-player/components/root.js index 73c6f9de..668ebca9 100644 --- a/elements/bulbs-video/elements/rail-player/components/root.js +++ b/elements/bulbs-video/elements/rail-player/components/root.js @@ -32,7 +32,7 @@ export default class Root extends React.Component { Date: Tue, 6 Jun 2017 15:51:13 -0400 Subject: [PATCH 3/8] removing unnecessary PropTypes and making the size attribute optional --- elements/bulbs-video/components/revealed.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/elements/bulbs-video/components/revealed.js b/elements/bulbs-video/components/revealed.js index c9b94a87..ce195e2e 100644 --- a/elements/bulbs-video/components/revealed.js +++ b/elements/bulbs-video/components/revealed.js @@ -56,7 +56,7 @@ export default class Revealed extends React.Component { let autoplayInViewBool = typeof this.props.autoplayInView === 'string'; // Allowing creativeSize to be passed in in root.js - let creativeSize = this.props.creativeSize; + let creativeSize = this.props.creativeSize || '640x480'; let videoAdConfig = 'None'; if (this.props.disableAds || this.props.video.disable_ads) { @@ -366,7 +366,6 @@ Revealed.propTypes = { autoplay: PropTypes.bool, autoplayInView: PropTypes.string, autoplayNext: PropTypes.bool, - creativeSize: PropTypes.object.string, controller: PropTypes.object.isRequired, defaultCaptions: PropTypes.bool, disableAds: PropTypes.bool, From 44d65e9c2588b196063cc38958d3c60d96405f46 Mon Sep 17 00:00:00 2001 From: kjacovino Date: Tue, 6 Jun 2017 16:08:36 -0400 Subject: [PATCH 4/8] fixing the proptype things --- elements/bulbs-video/components/revealed.js | 1 + elements/bulbs-video/elements/rail-player/components/root.js | 3 +-- elements/bulbs-video/elements/rail-player/rail-player.js | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/elements/bulbs-video/components/revealed.js b/elements/bulbs-video/components/revealed.js index ce195e2e..365d994a 100644 --- a/elements/bulbs-video/components/revealed.js +++ b/elements/bulbs-video/components/revealed.js @@ -366,6 +366,7 @@ Revealed.propTypes = { autoplay: PropTypes.bool, autoplayInView: PropTypes.string, autoplayNext: PropTypes.bool, + creativeSize: PropTypes.string, controller: PropTypes.object.isRequired, defaultCaptions: PropTypes.bool, disableAds: PropTypes.bool, diff --git a/elements/bulbs-video/elements/rail-player/components/root.js b/elements/bulbs-video/elements/rail-player/components/root.js index 668ebca9..0bd52dee 100644 --- a/elements/bulbs-video/elements/rail-player/components/root.js +++ b/elements/bulbs-video/elements/rail-player/components/root.js @@ -32,7 +32,7 @@ export default class Root extends React.Component { Date: Tue, 6 Jun 2017 16:13:21 -0400 Subject: [PATCH 5/8] moving the let statement to the right function --- elements/bulbs-video/components/revealed.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/elements/bulbs-video/components/revealed.js b/elements/bulbs-video/components/revealed.js index 365d994a..0b91c269 100644 --- a/elements/bulbs-video/components/revealed.js +++ b/elements/bulbs-video/components/revealed.js @@ -55,9 +55,6 @@ export default class Revealed extends React.Component { let specialCoverage = this.props.targetSpecialCoverage || 'None'; let autoplayInViewBool = typeof this.props.autoplayInView === 'string'; - // Allowing creativeSize to be passed in in root.js - let creativeSize = this.props.creativeSize || '640x480'; - let videoAdConfig = 'None'; if (this.props.disableAds || this.props.video.disable_ads) { videoAdConfig = 'disable-ads'; @@ -174,9 +171,11 @@ export default class Revealed extends React.Component { let vastTestId = this.vastTest(window.location.search); let type; + // Allowing creativeSize to be passed in in root.js + let creativeSize = this.props.creativeSize || '640x480'; // See docs (https://support.google.com/dfp_premium/answer/1068325?hl=en) for param info - baseUrl += `?sz=${this.props.creativeSize}`; + baseUrl += `?sz=${creativeSize}`; baseUrl += `&iu=/4246/${window.Bulbs.settings.DFP_SITE_CODE}`; baseUrl += '&impl=s'; baseUrl += '&gdfp_req=1'; From b4ee622f11b425beea4306ec3b4c97509c4aba4e Mon Sep 17 00:00:00 2001 From: kjacovino Date: Mon, 12 Jun 2017 14:43:04 -0400 Subject: [PATCH 6/8] seems like a duplicate test --- .../bulbs-video/components/revealed.test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/elements/bulbs-video/components/revealed.test.js b/elements/bulbs-video/components/revealed.test.js index 1768b8b1..293fc916 100644 --- a/elements/bulbs-video/components/revealed.test.js +++ b/elements/bulbs-video/components/revealed.test.js @@ -41,6 +41,10 @@ describe(' ', () => { expect(subject.muted).to.eql(PropTypes.bool); }); + it('accepts creativeSize string', () => { + expect(subject.creativeSize).to.eql(PropTypes.string); + }); + it('accepts disableAds boolean', () => { expect(subject.disableAds).to.eql(PropTypes.bool); }); @@ -609,6 +613,20 @@ describe(' ', () => { delete window.Bulbs; }); + it('defaults to 640x480 if creativeSize is not overridden', () => { + let vastUrl = Revealed.prototype.vastUrl.call({ + cacheBuster: cacheBusterStub, + vastTest: vastTestStub, + props: {}, + }, videoMeta); + let parsed = url.parse(vastUrl, true); + expect(parsed.query.sz).to.eql('640x480'); + }); + + it('allows creativeSize to be overridden', () => { + // ??? + }); + it('returns the vast url', function () { let vastUrl = Revealed.prototype.vastUrl.call({ cacheBuster: cacheBusterStub, From 9af05ae5373a3e1f5c98fa6a93368de74e14ba50 Mon Sep 17 00:00:00 2001 From: Chris Sprehe Date: Tue, 13 Jun 2017 13:52:50 -0500 Subject: [PATCH 7/8] Fixing tests --- .../bulbs-video/elements/rail-player/components/root.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/elements/bulbs-video/elements/rail-player/components/root.test.js b/elements/bulbs-video/elements/rail-player/components/root.test.js index 83dd232c..4aef91eb 100644 --- a/elements/bulbs-video/elements/rail-player/components/root.test.js +++ b/elements/bulbs-video/elements/rail-player/components/root.test.js @@ -68,6 +68,7 @@ describe(' ', () => { Date: Tue, 13 Jun 2017 13:55:00 -0500 Subject: [PATCH 8/8] Test for: It allows creativeSize for preroll vast tag URL to be overridden from the default of 640x480 --- .../bulbs-video/components/revealed.test.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/elements/bulbs-video/components/revealed.test.js b/elements/bulbs-video/components/revealed.test.js index 293fc916..3c8607b7 100644 --- a/elements/bulbs-video/components/revealed.test.js +++ b/elements/bulbs-video/components/revealed.test.js @@ -623,10 +623,6 @@ describe(' ', () => { expect(parsed.query.sz).to.eql('640x480'); }); - it('allows creativeSize to be overridden', () => { - // ??? - }); - it('returns the vast url', function () { let vastUrl = Revealed.prototype.vastUrl.call({ cacheBuster: cacheBusterStub, @@ -706,6 +702,20 @@ describe(' ', () => { }); }); + context('overrides', () => { + it('allows creativeSize to be overridden', () => { + let vastUrl = Revealed.prototype.vastUrl.call({ + cacheBuster: cacheBusterStub, + vastTest: vastTestStub, + props: { + creativeSize: '400x300', + }, + }, videoMeta); + let parsed = url.parse(vastUrl, true); + expect(parsed.query.sz).to.eql('400x300'); + }); + }); + context('when a test link is provided', () => { beforeEach(() => { window.Bulbs = {