diff --git a/assets/javascripts/discourse/components/ad-slot.hbs b/assets/javascripts/discourse/components/ad-slot.hbs index 7c71f75..ae10615 100644 --- a/assets/javascripts/discourse/components/ad-slot.hbs +++ b/assets/javascripts/discourse/components/ad-slot.hbs @@ -1,12 +1,13 @@ {{#each adComponents as |adComponent|}} - {{component - adComponent - placement=placement - refreshOnChange=refreshOnChange - category=category - listLoading=listLoading - postNumber=postNumber - indexNumber=indexNumber - tagName=childTagName - }} + {{! Trick to force full destroy/re-render of component when route changes }} + {{#each (array this.router.currentRoute)}} + {{component + adComponent + placement=placement + category=category + postNumber=postNumber + indexNumber=indexNumber + tagName=childTagName + }} + {{/each}} {{/each}} \ No newline at end of file diff --git a/assets/javascripts/discourse/components/ad-slot.js b/assets/javascripts/discourse/components/ad-slot.js index 8309d4e..c9d997d 100644 --- a/assets/javascripts/discourse/components/ad-slot.js +++ b/assets/javascripts/discourse/components/ad-slot.js @@ -1,6 +1,7 @@ import EmberObject from "@ember/object"; +import { inject as service } from "@ember/service"; import { isBlank } from "@ember/utils"; -import discourseComputed, { observes } from "discourse-common/utils/decorators"; +import discourseComputed from "discourse-common/utils/decorators"; import AdComponent from "discourse/plugins/discourse-adplugin/discourse/components/ad-component"; import { isNthPost, @@ -173,7 +174,7 @@ export function slotContenders( } export default AdComponent.extend({ - needsUpdate: false, + router: service(), tagName: "", /** @@ -191,27 +192,14 @@ export default AdComponent.extend({ ); }, - /** - * When house ads are configured to alternate with other ad networks, we - * need to trigger an update of which ad component is shown after - * navigating between topic lists or topics. - */ - @observes("refreshOnChange") - changed() { - if (this.get("listLoading")) { - return; - } - - // force adComponents to be recomputed - this.notifyPropertyChange("needsUpdate"); - }, - /** * Returns a list of the names of ad components that should be rendered * in the given ad placement. It handles alternating between house ads * and other ad networks. + * + * Depends on `router.currentRoute` so that we refresh ads when navigating around. */ - @discourseComputed("placement", "availableAdTypes", "needsUpdate") + @discourseComputed("placement", "availableAdTypes", "router.currentRoute") adComponents(placement, availableAdTypes) { if ( !availableAdTypes.includes("house-ad") || diff --git a/assets/javascripts/discourse/components/adbutler-ad.js b/assets/javascripts/discourse/components/adbutler-ad.js index b900796..ab78698 100644 --- a/assets/javascripts/discourse/components/adbutler-ad.js +++ b/assets/javascripts/discourse/components/adbutler-ad.js @@ -2,7 +2,7 @@ import { scheduleOnce } from "@ember/runloop"; import RSVP from "rsvp"; import loadScript from "discourse/lib/load-script"; import { isTesting } from "discourse-common/config/environment"; -import discourseComputed, { observes } from "discourse-common/utils/decorators"; +import discourseComputed from "discourse-common/utils/decorators"; import AdComponent from "discourse/plugins/discourse-adplugin/discourse/components/ad-component"; let _loaded = false, @@ -111,16 +111,6 @@ export default AdComponent.extend({ scheduleOnce("afterRender", this, this._triggerAds); }, - @observes("listLoading") - waitForLoad() { - if (this.get("adRequested")) { - return; - } // already requested that this ad unit be populated - if (!this.get("listLoading")) { - scheduleOnce("afterRender", this, this._triggerAds); - } - }, - @discourseComputed("currentUser.trust_level") showToTrustLevel(trustLevel) { return !( diff --git a/assets/javascripts/discourse/components/google-adsense.js b/assets/javascripts/discourse/components/google-adsense.js index 2bf628b..00635dd 100644 --- a/assets/javascripts/discourse/components/google-adsense.js +++ b/assets/javascripts/discourse/components/google-adsense.js @@ -3,7 +3,7 @@ import { htmlSafe } from "@ember/template"; import RSVP from "rsvp"; import loadScript from "discourse/lib/load-script"; import { isTesting } from "discourse-common/config/environment"; -import discourseComputed, { observes } from "discourse-common/utils/decorators"; +import discourseComputed from "discourse-common/utils/decorators"; import AdComponent from "discourse/plugins/discourse-adplugin/discourse/components/ad-component"; let _loaded = false, @@ -163,23 +163,9 @@ export default AdComponent.extend({ return; } - if (this.get("listLoading")) { - return; - } - scheduleOnce("afterRender", this, this._triggerAds); }, - @observes("listLoading") - waitForLoad() { - if (this.get("adRequested")) { - return; - } // already requested that this ad unit be populated - if (!this.get("listLoading")) { - scheduleOnce("afterRender", this, this._triggerAds); - } - }, - @discourseComputed("ad_width") isResponsive(adWidth) { return ["auto", "fluid"].includes(adWidth); diff --git a/assets/javascripts/discourse/components/google-dfp-ad.js b/assets/javascripts/discourse/components/google-dfp-ad.js index 25d4575..b09f7a7 100755 --- a/assets/javascripts/discourse/components/google-dfp-ad.js +++ b/assets/javascripts/discourse/components/google-dfp-ad.js @@ -242,7 +242,6 @@ export default AdComponent.extend({ classNameBindings: ["adUnitClass"], classNames: ["google-dfp-ad"], loadedGoogletag: false, - refreshOnChange: null, lastAdRefresh: null, width: alias("size.width"), height: alias("size.height"), @@ -353,7 +352,7 @@ export default AdComponent.extend({ @on("didUpdate") updated() { - if (this.get("listLoading") || !this.shouldRefreshAd()) { + if (!this.shouldRefreshAd()) { return; } diff --git a/assets/javascripts/discourse/components/house-ad.js b/assets/javascripts/discourse/components/house-ad.js index ea97927..a80d564 100644 --- a/assets/javascripts/discourse/components/house-ad.js +++ b/assets/javascripts/discourse/components/house-ad.js @@ -1,5 +1,5 @@ import { isBlank } from "@ember/utils"; -import discourseComputed, { observes } from "discourse-common/utils/decorators"; +import discourseComputed from "discourse-common/utils/decorators"; import AdComponent from "discourse/plugins/discourse-adplugin/discourse/components/ad-component"; const adIndex = { @@ -100,12 +100,7 @@ export default AdComponent.extend({ } }, - @observes("refreshOnChange") refreshAd() { - if (this.get("listLoading")) { - return; - } - this.set("adHtml", this.chooseAdHtml()); }, @@ -116,10 +111,6 @@ export default AdComponent.extend({ return; } - if (this.get("listLoading")) { - return; - } - if (adIndex.topic_list_top === null) { // start at a random spot in the ad inventory Object.keys(adIndex).forEach((placement) => { diff --git a/assets/javascripts/discourse/connectors/after-topic-list-item/discourse-adplugin.hbs b/assets/javascripts/discourse/connectors/after-topic-list-item/discourse-adplugin.hbs index 83836f5..73adb77 100644 --- a/assets/javascripts/discourse/connectors/after-topic-list-item/discourse-adplugin.hbs +++ b/assets/javascripts/discourse/connectors/after-topic-list-item/discourse-adplugin.hbs @@ -1,8 +1,6 @@ {{ad-slot placement="topic-list-between" - refreshOnChange=listLoading category=category.slug - listLoading=listLoading indexNumber=index childTagName="td" }} \ No newline at end of file diff --git a/assets/javascripts/discourse/connectors/discovery-list-container-top/discourse-adplugin.hbs b/assets/javascripts/discourse/connectors/discovery-list-container-top/discourse-adplugin.hbs index d6369a0..5caac05 100644 --- a/assets/javascripts/discourse/connectors/discovery-list-container-top/discourse-adplugin.hbs +++ b/assets/javascripts/discourse/connectors/discovery-list-container-top/discourse-adplugin.hbs @@ -1,6 +1 @@ -{{ad-slot - placement="topic-list-top" - refreshOnChange=listLoading - category=category.slug - listLoading=listLoading -}} \ No newline at end of file +{{ad-slot placement="topic-list-top" category=category.slug}} \ No newline at end of file diff --git a/assets/javascripts/discourse/connectors/topic-above-post-stream/discourse-adplugin.hbs b/assets/javascripts/discourse/connectors/topic-above-post-stream/discourse-adplugin.hbs index 5b792f8..5829855 100644 --- a/assets/javascripts/discourse/connectors/topic-above-post-stream/discourse-adplugin.hbs +++ b/assets/javascripts/discourse/connectors/topic-above-post-stream/discourse-adplugin.hbs @@ -1,5 +1 @@ -{{ad-slot - placement="topic-above-post-stream" - refreshOnChange=model.id - category=model.category.slug -}} \ No newline at end of file +{{ad-slot placement="topic-above-post-stream" category=model.category.slug}} \ No newline at end of file diff --git a/assets/javascripts/discourse/connectors/topic-above-suggested/discourse-adplugin.hbs b/assets/javascripts/discourse/connectors/topic-above-suggested/discourse-adplugin.hbs index a0cd3af..7f4d449 100644 --- a/assets/javascripts/discourse/connectors/topic-above-suggested/discourse-adplugin.hbs +++ b/assets/javascripts/discourse/connectors/topic-above-suggested/discourse-adplugin.hbs @@ -1,5 +1 @@ -{{ad-slot - placement="topic-above-suggested" - refreshOnChange=model.id - category=model.category.slug -}} \ No newline at end of file +{{ad-slot placement="topic-above-suggested" category=model.category.slug}} \ No newline at end of file diff --git a/test/javascripts/acceptance/house-ad-test.js b/test/javascripts/acceptance/house-ad-test.js index dfb71e0..faef2b4 100644 --- a/test/javascripts/acceptance/house-ad-test.js +++ b/test/javascripts/acceptance/house-ad-test.js @@ -2,6 +2,7 @@ import { visit } from "@ember/test-helpers"; import { test } from "qunit"; import { acceptance, + query, updateCurrentUser, } from "discourse/tests/helpers/qunit-helpers"; @@ -69,12 +70,21 @@ acceptance("House Ads", function (needs) { assert .dom(".h-topic-list") .exists({ count: 1 }, "it should render ad above topic list"); + const originalTopAdElement = query(".h-topic-list"); - await visit("/latest"); assert .dom(".h-between-topic-list") .exists({ count: 5 }, "it should render 5 ads between topics"); + await visit("/top"); + const newTopAdElement = query(".h-topic-list"); + + assert.notStrictEqual( + originalTopAdElement, + newTopAdElement, + "ad is fully re-rendered when changing pages" + ); + await visit("/t/28830"); assert