Fix dfp ad dimensioned undefined. Refactor dfp ad component to remove unnecessary variables and rewrite copy-pasted code.

assets/javascripts/discourse/components/google-dfp-ad.js.es6

@@ -1,12 +1,7 @@
 import loadScript from 'discourse/lib/load-script';
 
-var const_width = '';
+var currentUser = Discourse.User.current(),
-var const_height = '';
+    _loaded = false,
-var const_mobile_width = 320;
-var const_mobile_height = 50;
-var currentUser = Discourse.User.current();
-
-var _loaded = false,
     _promise = null,
     ads = {};
 
@@ -60,77 +55,66 @@ function custom_targeting(key_array, value_array, location) {
   }
 }
 
-function defineSlot(placement, settings) {
+function defineSlot(placement, settings, isMobile) {
+  var ad, width, height, divId;
 
   if (ads[placement]) {
     return ads[placement];
   }
 
+  divId = "div-gpt-ad-" + placement;
+
+  if (isMobile) {
+    // There are no settings for customizing the mobile ad sizes.
+    width = 320;
+    height = 50;
+  }
+
   if (placement === "topic-list-top" && settings.dfp_topic_list_top_code && settings.dfp_topic_list_top_ad_sizes) {
-    const_width = parseInt(splitWidthInt(settings.dfp_topic_list_top_ad_sizes));
+    if (isMobile) {
-    const_height = parseInt(splitHeightInt(settings.dfp_topic_list_top_ad_sizes));
+      ad = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_mobile_topic_list_top_code, [width,height], divId).addService(window.googletag.pubads());
-    if (Discourse.Mobile.mobileView) {
+      custom_targeting((keyParse(Discourse.SiteSettings.dfp_target_topic_list_top_key_code)), (keyParse(settings.dfp_target_topic_list_top_value_code)), ad);
-      var topic_list_top_mobile = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_mobile_topic_list_top_code, [320,50], 'div-gpt-ad-topic-list-top').addService(window.googletag.pubads());
-      ads['topic-list-top'] = topic_list_top_mobile;
-      custom_targeting((keyParse(Discourse.SiteSettings.dfp_target_topic_list_top_key_code)), (keyParse(settings.dfp_target_topic_list_top_value_code)), topic_list_top_mobile);
     } else {
-      var topic_list_top = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_topic_list_top_code, [parseInt(splitWidthInt(settings.dfp_topic_list_top_ad_sizes)), parseInt(splitHeightInt(settings.dfp_topic_list_top_ad_sizes))], 'div-gpt-ad-topic-list-top').addService(window.googletag.pubads());
+      width = parseInt(splitWidthInt(settings.dfp_topic_list_top_ad_sizes));
-      ads['topic-list-top'] = topic_list_top;
+      height = parseInt(splitHeightInt(settings.dfp_topic_list_top_ad_sizes));
-      custom_targeting((keyParse(settings.dfp_target_topic_list_top_key_code)), (keyParse(settings.dfp_target_topic_list_top_value_code)), topic_list_top);
+      ad = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_topic_list_top_code, [width, height], divId).addService(window.googletag.pubads());
+      custom_targeting((keyParse(settings.dfp_target_topic_list_top_key_code)), (keyParse(settings.dfp_target_topic_list_top_value_code)), ad);
+    }
+  } else if (placement === "topic-above-post-stream" && settings.dfp_topic_above_post_stream_code && settings.dfp_topic_above_post_stream_ad_sizes) {
+    if (isMobile) {
+      ad = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_mobile_topic_above_post_stream_code, [width,height], divId).addService(window.googletag.pubads());
+      custom_targeting((keyParse(settings.dfp_target_topic_above_post_stream_key_code)), (keyParse(settings.dfp_target_topic_above_post_stream_value_code)), ad);
+    } else {
+      width = parseInt(splitWidthInt(settings.dfp_topic_above_post_stream_ad_sizes));
+      height = parseInt(splitHeightInt(settings.dfp_topic_above_post_stream_ad_sizes));
+      ad = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_topic_above_post_stream_code, [width, height], divId).addService(window.googletag.pubads());
+      custom_targeting((keyParse(settings.dfp_target_topic_above_post_stream_key_code)), (keyParse(settings.dfp_target_topic_above_post_stream_value_code)), ad);
+    }
+  } else if (placement === "topic-above-suggested" && settings.dfp_topic_above_suggested_code && settings.dfp_topic_above_suggested_ad_sizes) {
+    if (isMobile) {
+      ad = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_mobile_topic_above_suggested_code, [width,height], divId).addService(window.googletag.pubads());
+      custom_targeting((keyParse(settings.dfp_target_topic_above_suggested_key_code)), (keyParse(settings.dfp_target_topic_above_suggested_value_code)), ad);
+    } else {
+      width = parseInt(splitWidthInt(settings.dfp_topic_above_suggested_ad_sizes));
+      height = parseInt(splitHeightInt(settings.dfp_topic_above_suggested_ad_sizes));
+      ad = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_topic_above_suggested_code, [width, height], divId).addService(window.googletag.pubads());
+      custom_targeting((keyParse(settings.dfp_target_topic_above_suggested_key_code)), (keyParse(settings.dfp_target_topic_above_suggested_value_code)), ad);
+    }
+  } else if (placement === "post-bottom" && settings.dfp_post_bottom_code && settings.dfp_post_bottom_ad_sizes) {
+    if (isMobile) {
+      ad = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_mobile_post_bottom_code, [width,height], divId).addService(window.googletag.pubads());
+      custom_targeting((keyParse(settings.dfp_target_post_bottom_key_code)), (keyParse(settings.dfp_target_post_bottom_value_code)), ad);
+    } else {
+      width = parseInt(splitWidthInt(settings.dfp_post_bottom_ad_sizes));
+      height = parseInt(splitHeightInt(settings.dfp_post_bottom_ad_sizes));
+      ad = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_post_bottom_code, [width, height], divId).addService(window.googletag.pubads());
+      custom_targeting((keyParse(settings.dfp_target_post_bottom_key_code)), (keyParse(settings.dfp_target_post_bottom_value_code)), ad);
     }
-
-    return ads['topic-list-top'];
   }
 
-  if (placement === "topic-above-post-stream" && settings.dfp_topic_above_post_stream_code && settings.dfp_topic_above_post_stream_ad_sizes) {
+  if (ad) {
-    const_width = parseInt(splitWidthInt(settings.dfp_topic_above_post_stream_ad_sizes));
+    ads[placement] = {ad: ad, width: width, height: height};
-    const_height = parseInt(splitHeightInt(settings.dfp_topic_above_post_stream_ad_sizes));
+    return ads[placement];
-    if (Discourse.Mobile.mobileView) {
-      var topic_above_post_stream_mobile = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_mobile_topic_above_post_stream_code, [320,50], 'div-gpt-ad-topic-above-post-stream').addService(window.googletag.pubads());
-      ads['topic-above-post-stream'] = topic_above_post_stream_mobile;
-      custom_targeting((keyParse(settings.dfp_target_topic_above_post_stream_key_code)), (keyParse(settings.dfp_target_topic_above_post_stream_value_code)), topic_above_post_stream_mobile);
-    }
-    else {
-      var topic_above_post_stream = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_topic_above_post_stream_code, [parseInt(splitWidthInt(settings.dfp_topic_above_post_stream_ad_sizes)), parseInt(splitHeightInt(settings.dfp_topic_above_post_stream_ad_sizes))], 'div-gpt-ad-topic-above-post-stream').addService(window.googletag.pubads());
-      ads['topic-above-post-stream'] = topic_above_post_stream;
-      custom_targeting((keyParse(settings.dfp_target_topic_above_post_stream_key_code)), (keyParse(settings.dfp_target_topic_above_post_stream_value_code)), topic_above_post_stream);
-    }
-
-    return ads['topic-above-post-stream'];
-  }
-
-  if (placement === "topic-above-suggested" && settings.dfp_topic_above_suggested_code && settings.dfp_topic_above_suggested_ad_sizes) {
-    const_width = parseInt(splitWidthInt(settings.dfp_topic_above_suggested_ad_sizes));
-    const_height = parseInt(splitHeightInt(settings.dfp_topic_above_suggested_ad_sizes));
-    if (Discourse.Mobile.mobileView) {
-      var topic_above_suggested_mobile = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_mobile_topic_above_suggested_code, [320,50], 'div-gpt-ad-topic-above-suggested').addService(window.googletag.pubads());
-      ads['topic-above-suggested'] = topic_above_suggested_mobile;
-      custom_targeting((keyParse(settings.dfp_target_topic_above_suggested_key_code)), (keyParse(settings.dfp_target_topic_above_suggested_value_code)), topic_above_suggested_mobile);
-    }
-    else {
-      var topic_above_suggested = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_topic_above_suggested_code, [parseInt(splitWidthInt(settings.dfp_topic_above_suggested_ad_sizes)), parseInt(splitHeightInt(settings.dfp_topic_above_suggested_ad_sizes))], 'div-gpt-ad-topic-above-suggested').addService(window.googletag.pubads());
-      ads['topic-above-suggested'] = topic_above_suggested;
-      custom_targeting((keyParse(settings.dfp_target_topic_above_suggested_key_code)), (keyParse(settings.dfp_target_topic_above_suggested_value_code)), topic_above_suggested);
-    }
-
-    return ads['topic-above-suggested'];
-  }
-
-  if (placement === "post-bottom" && settings.dfp_post_bottom_code && settings.dfp_post_bottom_ad_sizes) {
-    const_width = parseInt(splitWidthInt(settings.dfp_post_bottom_ad_sizes));
-    const_height = parseInt(splitHeightInt(settings.dfp_post_bottom_ad_sizes));
-    if (Discourse.Mobile.mobileView) {
-      var post_bottom_mobile = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_mobile_post_bottom_code, [320,50], 'div-gpt-ad-post-bottom').addService(window.googletag.pubads());
-      ads['post-bottom'] = post_bottom_mobile;
-      custom_targeting((keyParse(settings.dfp_target_post_bottom_key_code)), (keyParse(settings.dfp_target_post_bottom_value_code)), post_bottom_mobile);
-    }
-    else {
-      var post_bottom = window.googletag.defineSlot('/' + settings.dfp_publisher_id + '/' + settings.dfp_post_bottom_code, [parseInt(splitWidthInt(settings.dfp_post_bottom_ad_sizes)), parseInt(splitHeightInt(settings.dfp_post_bottom_ad_sizes))], 'div-gpt-ad-post-bottom').addService(window.googletag.pubads());
-      ads['post-bottom'] = post_bottom;
-      custom_targeting((keyParse(settings.dfp_target_post_bottom_key_code)), (keyParse(settings.dfp_target_post_bottom_value_code)), post_bottom);
-    }
-
-    return ads['post-bottom'];
   }
 }
 
@@ -164,41 +148,35 @@ function loadGoogle() {
 
 // Ember component - the class is the adblock and css
 export default Ember.Component.extend({
-  const_width: const_width,
+  width: 728,
-  const_height: const_height,
+  height: 90,
-  const_mobile_width: const_mobile_width,
-  const_mobile_height: const_mobile_height,
 
   classNames: ['google-dfp-ad'],
   loadedGoogletag: false,
   refreshOnChange: null,
 
-  // Part of the divID of the div part of the GPT
   divId: function() {
     return "div-gpt-ad-" + this.get('placement');
   }.property('placement'),
 
   adWrapperStyle: function() {
-    return `width: ${this.get('const_width')}px; height: ${this.get('const_height')}px;`.htmlSafe();
+    return `width: ${this.get('width')}px; height: ${this.get('height')}px;`.htmlSafe();
-  }.property('const_width', 'const_height'),
+  }.property('width', 'height'),
-
-  adWrapperStyleMobile: function() {
-    return `width: ${this.get('const_mobile_width')}px; height: ${this.get('const_mobile_height')}px;`.htmlSafe();
-  }.property('const_mobile_width', 'const_mobile_height'),
 
   adTitleStyleMobile: function() {
-    return `width: ${this.get('const_mobile_width')}px;`.htmlSafe();
+    return `width: ${this.get('width')}px;`.htmlSafe();
-  }.property('const_mobile_width'),
+  }.property('width'),
 
   checkTrustLevels: function() {
     return !((currentUser) && (currentUser.get('trust_level') > Discourse.SiteSettings.dfp_through_trust_level));
   }.property('trust_level'),
 
   refreshAd: function() {
-    var ad = ads[this.get('placement')];
+    var slot = ads[this.get('placement')];
-    if (!ad) { return; }
+    if (!(slot && slot.ad)) { return; }
 
-    var self = this;
+    var self = this,
+        ad = slot.ad;
 
     if (this.get('loadedGoogletag') && this.get('refreshOnChange')) {
       window.googletag.cmd.push(function() {
@@ -213,11 +191,13 @@ export default Ember.Component.extend({
     loadGoogle(this.siteSettings).then(function() {
       self.set('loadedGoogletag', true);
       window.googletag.cmd.push(function() {
-        var ad = defineSlot(self.get('placement'), self.siteSettings);
+        let slot = defineSlot(self.get('placement'), self.siteSettings, self.site.mobileView);
-        if (ad) {
+        if (slot && slot.ad) {
-          ad.setTargeting('discourse-category', self.get('category') ? self.get('category') : '0');
+          slot.ad.setTargeting('discourse-category', self.get('category') ? self.get('category') : '0');
+          self.set('width', slot.width);
+          self.set('height', slot.height);
           window.googletag.display(self.get('divId'));
-          window.googletag.pubads().refresh([ad]);
+          window.googletag.pubads().refresh([slot.ad]);
         }
       });
     });

assets/javascripts/discourse/templates/components/google-dfp-ad.hbs

@@ -1,10 +1,9 @@
 {{#if checkTrustLevels}}
-
   {{#if site.mobileView}}
     <div class="google-dfp-ad-label" style={{adTitleStyleMobile}}><h2>ADVERTISEMENT</h2></div>
-    <div id={{divId}} style={{adWrapperStyleMobile}} class="dfp-ad-unit" align=center></div>
+    <div id={{divId}} style={{adWrapperStyle}} class="dfp-ad-unit" align=center></div>
   {{else}}
     <div class="google-dfp-ad-label"><h2>ADVERTISEMENT</h2></div>
     <div id={{divId}} style={{adWrapperStyle}} class="dfp-ad-unit" align=center></div>
   {{/if}}
-{{/if}}
+{{/if}}