From 849df51e11dcb1e394b8d073bdd06e30d5d1d6a4 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Thu, 6 Mar 2025 19:32:21 -0500 Subject: [PATCH 1/8] Use footer text content ad for unknown themes This is much less intrusive than the stickybox, and allows us to still have an ad injected, but not cover the content for users when this happens. --- src/ethicalads.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/ethicalads.js b/src/ethicalads.js index 878443f5..54d93d2c 100644 --- a/src/ethicalads.js +++ b/src/ethicalads.js @@ -196,18 +196,18 @@ export class EthicalAdsAddon extends AddonBase { if (elementToAppend) { elementToAppend.append(placement); } - } else if (window.innerWidth > 1300) { - // https://ethical-ad-client.readthedocs.io/en/latest/#stickybox - placement.setAttribute("data-ea-type", "image"); - placement.setAttribute("data-ea-style", "stickybox"); - this.addEaPlacementToElement(placement); - // `document.body` here is not too much relevant, since we are going to - // use this selector only for a floating stickybox ad - const elementInsertBefore = document.body; - elementInsertBefore.insertBefore( - placement, - elementInsertBefore.lastChild, - ); + } else { + // Default to a text ad appended to the root selector when no known placement found + placement.setAttribute("data-ea-type", "text"); + + const rootSelector = docTool.getRootSelector(); + const rootElement = document.querySelector(rootSelector); + + if (rootElement) { + rootElement.append(placement); + } else { + console.debug("Could not find root element to append ad"); + } } } From 45a7b8f7502ac18224e56c8f316ba01952196d5c Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Thu, 6 Mar 2025 19:36:56 -0500 Subject: [PATCH 2/8] Fix bug with elementAboveTheFold not existing --- src/ethicalads.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ethicalads.js b/src/ethicalads.js index 54d93d2c..bf374674 100644 --- a/src/ethicalads.js +++ b/src/ethicalads.js @@ -199,6 +199,7 @@ export class EthicalAdsAddon extends AddonBase { } else { // Default to a text ad appended to the root selector when no known placement found placement.setAttribute("data-ea-type", "text"); + placement.setAttribute("id", "readthedocs-text-footer"); const rootSelector = docTool.getRootSelector(); const rootElement = document.querySelector(rootSelector); @@ -239,6 +240,11 @@ export class EthicalAdsAddon extends AddonBase { } elementAboveTheFold(element) { + // Return false if element doesn't exist + if (!element) { + return false; + } + // Determine if this element would be above the fold. // If this is off screen, instead create an ad in the footer. // Assumes the ad would be AD_SIZE pixels high. From ac6b1f0c53552588722ed552c96df12b049d013c Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Thu, 6 Mar 2025 19:42:46 -0500 Subject: [PATCH 3/8] fix tests --- src/ethicalads.js | 2 ++ tests/__snapshots__/ethicalads.test.snap.js | 5 ++--- tests/ethicalads.test.html | 9 +++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/ethicalads.js b/src/ethicalads.js index bf374674..25f95c5a 100644 --- a/src/ethicalads.js +++ b/src/ethicalads.js @@ -199,6 +199,8 @@ export class EthicalAdsAddon extends AddonBase { } else { // Default to a text ad appended to the root selector when no known placement found placement.setAttribute("data-ea-type", "text"); + // TODO: Check this placement on the dashboard, + // and see how this is performing. placement.setAttribute("id", "readthedocs-text-footer"); const rootSelector = docTool.getRootSelector(); diff --git a/tests/__snapshots__/ethicalads.test.snap.js b/tests/__snapshots__/ethicalads.test.snap.js index 0c821bb8..d09d79bf 100644 --- a/tests/__snapshots__/ethicalads.test.snap.js +++ b/tests/__snapshots__/ethicalads.test.snap.js @@ -20,9 +20,8 @@ snapshots["EthicalAd addon ad placement injected"] = data-ea-keywords="docs|data-science" data-ea-manual="true" data-ea-publisher="readthedocs" - data-ea-style="stickybox" - data-ea-type="image" - id="readthedocs-ea" + data-ea-type="text" + id="readthedocs-text-footer" > `; diff --git a/tests/ethicalads.test.html b/tests/ethicalads.test.html index 7512a946..6f8fd350 100644 --- a/tests/ethicalads.test.html +++ b/tests/ethicalads.test.html @@ -66,11 +66,12 @@ }); it("ad placement injected", async () => { - // We need the width to be bigger than 1300px for the ad to be injected - await setViewport({ width: 1600, height: 640 }); - const addon = new ethicalads.EthicalAdsAddon(config); - const element = document.querySelector("#readthedocs-ea"); + const element = document.querySelector("#readthedocs-text-footer"); + + expect(element).to.exist; + expect(element).to.have.attribute("data-ea-type", "text"); + expect(element).to.have.attribute("data-ea-publisher", "readthedocs"); await expect(element).dom.to.equalSnapshot(); }); From 0ff05ccd735bace8d420d8fd562e8a13dc4c881a Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Thu, 6 Mar 2025 20:53:27 -0500 Subject: [PATCH 4/8] Fix lint --- tests/ethicalads.test.html | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/ethicalads.test.html b/tests/ethicalads.test.html index 6f8fd350..3d0515e3 100644 --- a/tests/ethicalads.test.html +++ b/tests/ethicalads.test.html @@ -71,7 +71,10 @@ expect(element).to.exist; expect(element).to.have.attribute("data-ea-type", "text"); - expect(element).to.have.attribute("data-ea-publisher", "readthedocs"); + expect(element).to.have.attribute( + "data-ea-publisher", + "readthedocs", + ); await expect(element).dom.to.equalSnapshot(); }); From 2b6e0d12f8857eff2b6f0f8c81ae01d9e5880e69 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Mon, 17 Mar 2025 17:33:42 -0400 Subject: [PATCH 5/8] Add doctool name to text-footer --- src/ethicalads.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ethicalads.js b/src/ethicalads.js index 25f95c5a..1aa3692e 100644 --- a/src/ethicalads.js +++ b/src/ethicalads.js @@ -201,7 +201,9 @@ export class EthicalAdsAddon extends AddonBase { placement.setAttribute("data-ea-type", "text"); // TODO: Check this placement on the dashboard, // and see how this is performing. - placement.setAttribute("id", "readthedocs-text-footer"); + const docToolName = docTool.getDocumentationTool(); + const idSuffix = docToolName ? `-${docToolName}` : ""; + placement.setAttribute("id", `readthedocs-ea-text-footer${idSuffix}`); const rootSelector = docTool.getRootSelector(); const rootElement = document.querySelector(rootSelector); From 37f45fb783b0e8c3926a850ce13aa7fffd22136f Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Mon, 17 Mar 2025 17:39:30 -0400 Subject: [PATCH 6/8] Fix test --- tests/__snapshots__/ethicalads.test.snap.js | 2 +- tests/ethicalads.test.html | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/__snapshots__/ethicalads.test.snap.js b/tests/__snapshots__/ethicalads.test.snap.js index d09d79bf..b56c9bfc 100644 --- a/tests/__snapshots__/ethicalads.test.snap.js +++ b/tests/__snapshots__/ethicalads.test.snap.js @@ -21,7 +21,7 @@ snapshots["EthicalAd addon ad placement injected"] = data-ea-manual="true" data-ea-publisher="readthedocs" data-ea-type="text" - id="readthedocs-text-footer" + id="readthedocs-ea-text-footer" > `; diff --git a/tests/ethicalads.test.html b/tests/ethicalads.test.html index 3d0515e3..93d69f88 100644 --- a/tests/ethicalads.test.html +++ b/tests/ethicalads.test.html @@ -1,4 +1,4 @@ - +html>