Size mapping functionality #651

mkendall07 · 2016-09-22T15:50:42Z

Type of change

Feature

Description of change

Adding support for #87 - size mapping.

Usage:

var adUnit = {
  code: 'code',
  sizeMapping: [  //new!
    {
      minWidth : 1024,  //if device screen width is greater than 1024, use these sizes
      sizes : [[300,250],[728,90]]
    },
    {
      minWidth : 480,  //if device screen width is < 1024 && > 480, use these sizes
      sizes : [120,60]
    },
    {
      minWidth : 0,  //if device screen width is < 480 && > 0, use these sizes
      sizes : [20,20]
    }
  ]
  bids: [...]
}

Other information

@prebid/core for review.

matthewlane · 2016-09-23T18:33:43Z

src/sizeMapping.js

+  if(!isSizeMappingValid(adUnit.sizeMapping)){
+    return adUnit.sizes;
+  }
+  const width = this.getScreenWidth();


On a test page, this line gave me TypeError: Cannot read property 'getScreenWidth' of undefined due to this module using CommonJS-style exports but adapter manager using ES2015-style imports. Line 4 of adaptermanager could be changed to use a require statement, or the functions in this module could be exported with the ES2015 export keyword--in that case the unit tests fail though and would need to be changed as well

matthewlane · 2016-09-23T18:42:32Z

src/sizeMapping.js

+  if(utils.isArray(sizeMapping) && sizeMapping.length > 0){
+    return true;
+  }
+  utils.logError('sizeMapping needs at least one screen size defined');


On pages that haven't set up any sizeMappings but are still using sizes array(s), this message still gets logged

matthewlane · 2016-09-23T18:44:20Z

src/sizeMapping.js

+  const sizes = adUnit.sizeMapping.find(sizeMapping =>{
+    return width > sizeMapping.minWidth;
+  }).sizes;
+  utils.logMessage(`AdUnit : ${adUnit.code} resized based on device width to : ${adUnit.sizes}`);


If a sizeMapping is configured, is that meant to replace the sizes property? If so this message will show width as undefined

matthewlane · 2016-09-23T18:44:42Z

test/spec/sizeMapping_spec.js

+  it('mapSizes 1029 width', function() {
+    let stub = sinon.stub(sizeMapping, 'getScreenWidth').returns(1029);
+    let sizes = sizeMapping.mapSizes(validAdUnit);
+    console.log(sizeMapping.getScreenWidth());


May want to remove this line

mkendall07 · 2016-09-26T16:45:00Z

@matthewlane
PR notes addressed.

matthewlane

Tests pass, functionality works on test page, LGTM

matthewlane · 2016-09-26T17:06:42Z

src/sizeMapping.js

+    return adUnit.sizes;
+  }
+  const width = getScreenWidth();
+  console.log('internal screen width: ' + width);


console.log snuck in here

* Size mapping initial draft. * Unit tests * Use desktop size if cannot determine width * Hygiene. * Increase unit coverage to 100% * Change to ES6 style export and updating unit tests * Remove console.log

…ebid-0.13.1 to release/1.4.0 * commit 'd174ef271e55e7f26210f50caac52c0a036bc9a1': CHANGELOG. 0.13.1 release add an --https flag to run local dev server over https (prebid#670) test ie variant of url parse pathname (prebid#669) Prevent SpringServe TypeError (prebid#663) XDomainRequest does not support `readyState` and was not executing the callback. (prebid#668) Prevent Sovrn TypeError (prebid#664) Prevent TripleLift TypeError (prebid#662) fixed bug with using non standard "standard" keys and sendAllBids (prebid#665) Initial value 0 for adder. (prebid#656) Prevent bidmanager TypeError (prebid#661) Prevent Pubmatic TypeError (prebid#666) Size mapping functionality (prebid#651) Added '320x80': 59, '320x320': 72, '320x160': 73 to RUBICON_SIZE_MAP (prebid#649) Add defy alias + increment version. (prebid#650) Add additional sizes to rubicon size mapping (prebid#646)

…4.0 to master * commit '5e7eec42d84ec5de47a28ba493a9357b595761d3': CHANGELOG. New adapter. 0.13.1 release add an --https flag to run local dev server over https (prebid#670) test ie variant of url parse pathname (prebid#669) Prevent SpringServe TypeError (prebid#663) XDomainRequest does not support `readyState` and was not executing the callback. (prebid#668) Prevent Sovrn TypeError (prebid#664) Prevent TripleLift TypeError (prebid#662) fixed bug with using non standard "standard" keys and sendAllBids (prebid#665) Initial value 0 for adder. (prebid#656) Prevent bidmanager TypeError (prebid#661) Prevent Pubmatic TypeError (prebid#666) Size mapping functionality (prebid#651) Added '320x80': 59, '320x320': 72, '320x160': 73 to RUBICON_SIZE_MAP (prebid#649) Add defy alias + increment version. (prebid#650) Add additional sizes to rubicon size mapping (prebid#646)

mkendall07 added 5 commits September 20, 2016 17:00

Size mapping initial draft.

84e94d4

Unit tests

b838975

Use desktop size if cannot determine width

fb8a0f8

Hygiene.

dbf7595

Increase unit coverage to 100%

eec226d

mkendall07 mentioned this pull request Sep 22, 2016

Support sizemapping for responsive ads #87

Closed

matthewlane reviewed Sep 23, 2016

View reviewed changes

mkendall07 added the needs update label Sep 23, 2016

Change to ES6 style export and updating unit tests

dfeb984

matthewlane approved these changes Sep 26, 2016

View reviewed changes

Remove console.log

fa2b243

mkendall07 merged commit 2c0ca80 into master Sep 26, 2016

mkendall07 deleted the size_mapping_functionality branch September 30, 2016 18:45

rmloveland mentioned this pull request Oct 3, 2016

Document size mapping feature prebid/prebid.github.io#102

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Size mapping functionality #651

Size mapping functionality #651

mkendall07 commented Sep 22, 2016 •

edited

Loading

matthewlane Sep 23, 2016 •

edited

Loading

matthewlane Sep 23, 2016

matthewlane Sep 23, 2016

matthewlane Sep 23, 2016 •

edited

Loading

mkendall07 commented Sep 26, 2016

matthewlane left a comment

matthewlane Sep 26, 2016

Size mapping functionality #651

Size mapping functionality #651

Conversation

mkendall07 commented Sep 22, 2016 • edited Loading

Type of change

Description of change

Other information

matthewlane Sep 23, 2016 • edited Loading

Choose a reason for hiding this comment

matthewlane Sep 23, 2016

Choose a reason for hiding this comment

matthewlane Sep 23, 2016

Choose a reason for hiding this comment

matthewlane Sep 23, 2016 • edited Loading

Choose a reason for hiding this comment

mkendall07 commented Sep 26, 2016

matthewlane left a comment

Choose a reason for hiding this comment

matthewlane Sep 26, 2016

Choose a reason for hiding this comment

mkendall07 commented Sep 22, 2016 •

edited

Loading

matthewlane Sep 23, 2016 •

edited

Loading

matthewlane Sep 23, 2016 •

edited

Loading