Skip to content

Commit 7e732ba

Browse files
authored
✨ Landscape ad support for amp-story-auto-ads (#40264)
* Landscape ad support for amp-story-auto-ads * Made `getAdAspectRatio_` a private function. Extended functionality to text content. Updated unit tests. * Prevent division by 0 * Created experiment `story-ad-allow-fullbleed`. If enabled and AMP is in fullbleed mode, the ads will be shown in fullbleed.
1 parent 411a7c9 commit 7e732ba

File tree

6 files changed

+103
-3
lines changed

6 files changed

+103
-3
lines changed

extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.css

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,26 @@ amp-story-page amp-ad[type='adsense'] {
4242
max-width: calc(3 / 5 * 75vh) !important;
4343
transform: translateX(-50%) !important;
4444
}
45+
46+
/* If experiment `story-ad-allow-fullbleed` is enabled, show fullbleed ads */
47+
.i-amphtml-story-desktop-fullbleed
48+
.i-amphtml-fullbleed-ad
49+
amp-story-grid-layer[template="fill"]
50+
> amp-ad {
51+
min-width: 100% !important;
52+
max-width: 100% !important;
53+
min-height: 100% !important;
54+
max-height: 100% !important;
55+
}
56+
57+
.i-amphtml-story-desktop-fullbleed .i-amphtml-fullbleed-ad iframe {
58+
width: 100% !important;
59+
height: 100% !important;
60+
}
61+
62+
.i-amphtml-story-desktop-fullbleed
63+
.i-amphtml-fullbleed-ad
64+
.i-amphtml-cta-container {
65+
bottom: 0 !important;
66+
top: 0 !important;
67+
}

extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {CommonSignals_Enum} from '#core/constants/common-signals';
22

3-
import {forceExperimentBranch} from '#experiments';
3+
import {forceExperimentBranch, isExperimentOn} from '#experiments';
44
import {divertStoryAdPlacements} from '#experiments/story-ad-placements';
55
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';
66

@@ -91,6 +91,12 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
9191

9292
/** @private {?StoryAdPageManager} */
9393
this.adPageManager_ = null;
94+
95+
/** @private {boolean} */
96+
this.fullbleedAdExpEnabled_ = isExperimentOn(
97+
this.win,
98+
'story-ad-allow-fullbleed'
99+
);
94100
}
95101

96102
/** @override */
@@ -339,7 +345,11 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
339345
this.adBadgeContainer_.removeAttribute(DESKTOP_FULLBLEED);
340346
this.adBadgeContainer_.removeAttribute(DESKTOP_ONE_PANEL);
341347

342-
if (uiState === UIType_Enum.DESKTOP_FULLBLEED) {
348+
if (
349+
uiState === UIType_Enum.DESKTOP_FULLBLEED &&
350+
!this.fullbleedAdExpEnabled_
351+
) {
352+
// Proper landscape ads in DESKTOP_FULLBLEED do not need the attribute
343353
this.adBadgeContainer_.setAttribute(DESKTOP_FULLBLEED, '');
344354
}
345355
if (uiState === UIType_Enum.DESKTOP_ONE_PANEL) {
@@ -466,6 +476,10 @@ export class AmpStoryAutoAds extends AMP.BaseElement {
466476
this.mutateElement(() => {
467477
adPage.toggleVisibility();
468478
this.visibleAdPage_ = adPage;
479+
if (this.fullbleedAdExpEnabled_) {
480+
// Proper landscape ads in DESKTOP_FULLBLEED do not need the attribute
481+
this.adBadgeContainer_.removeAttribute(Attributes.DESKTOP_FULLBLEED);
482+
}
469483
});
470484
}
471485

extensions/amp-story-auto-ads/0.1/story-ad-page.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {setStyle} from '#core/dom/style';
99
import {map} from '#core/types/object';
1010
import {parseJson} from '#core/types/object/json';
1111

12-
import {getExperimentBranch} from '#experiments';
12+
import {getExperimentBranch, isExperimentOn} from '#experiments';
1313
import {StoryAdSegmentExp} from '#experiments/story-ad-progress-segment';
1414

1515
import {getData, listen} from '#utils/event-helper';
@@ -54,6 +54,9 @@ const GLASS_PANE_CLASS = 'i-amphtml-glass-pane';
5454
/** @const {string} */
5555
const DESKTOP_FULLBLEED_CLASS = 'i-amphtml-story-ad-fullbleed';
5656

57+
/** @const {string} */
58+
const FULLBLEED_AD_CLASS = 'i-amphtml-fullbleed-ad';
59+
5760
/** @enum {string} */
5861
const PageAttributes = {
5962
LOADING: 'i-amphtml-loading',
@@ -428,6 +431,10 @@ export class StoryAdPage {
428431
);
429432
}
430433

434+
if (isExperimentOn(this.win_, 'story-ad-allow-fullbleed')) {
435+
this.pageElement_.classList.add(FULLBLEED_AD_CLASS);
436+
}
437+
431438
this.loaded_ = true;
432439

433440
this.loadCallbacks_.forEach((cb) => cb());

extensions/amp-story-auto-ads/0.1/test/test-amp-story-auto-ads.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,34 @@ describes.realWin(
419419
storeService.dispatch(Action.TOGGLE_RTL, true);
420420
expect(adBadgeContainer).to.have.attribute(Attributes.DIR, 'rtl');
421421
});
422+
423+
it('should propagate the desktop-fullbleed attribute to ad badge', () => {
424+
const adBadgeContainer = doc.querySelector(
425+
'.i-amphtml-ad-overlay-container'
426+
);
427+
expect(adBadgeContainer).not.to.have.attribute(
428+
Attributes.DESKTOP_FULLBLEED
429+
);
430+
storeService.dispatch(Action.TOGGLE_UI, UIType_Enum.DESKTOP_FULLBLEED);
431+
expect(adBadgeContainer).to.have.attribute(
432+
Attributes.DESKTOP_FULLBLEED
433+
);
434+
});
435+
436+
it('should not propagate the desktop-fullbleed attribute to ad badge for landscape ads', () => {
437+
autoAds.fullbleedAdExpEnabled_ = true;
438+
const adBadgeContainer = doc.querySelector(
439+
'.i-amphtml-ad-overlay-container'
440+
);
441+
expect(adBadgeContainer).not.to.have.attribute(
442+
Attributes.DESKTOP_FULLBLEED
443+
);
444+
storeService.dispatch(Action.TOGGLE_UI, UIType_Enum.DESKTOP_FULLBLEED);
445+
expect(adBadgeContainer).not.to.have.attribute(
446+
Attributes.DESKTOP_FULLBLEED
447+
);
448+
autoAds.fullbleedAdExpEnabled_ = false;
449+
});
422450
});
423451

424452
describe('analytics triggers', () => {

extensions/amp-story-auto-ads/0.1/test/test-story-ad-page.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import {CommonSignals_Enum} from '#core/constants/common-signals';
22

3+
import {toggleExperiment} from '#experiments';
4+
35
import {macroTask} from '#testing/helpers';
46

57
import {Gestures} from '../../../../src/gesture';
@@ -457,6 +459,27 @@ describes.realWin('story-ad-page', {amp: true}, (env) => {
457459
});
458460
});
459461

462+
describe('fullbleed ads', () => {
463+
it('should set fullbleed ad class', async () => {
464+
const fullbleedAdClass = 'i-amphtml-fullbleed-ad';
465+
const iframe = doc.createElement('iframe');
466+
const video = doc.createElement('video');
467+
const pageElement = storyAdPage.build();
468+
469+
toggleExperiment(env.win, 'story-ad-allow-fullbleed', true);
470+
doc.body.appendChild(pageElement);
471+
pageElement.getImpl = () => Promise.resolve(pageImplMock);
472+
473+
const ampAdElement = doc.querySelector('amp-ad');
474+
ampAdElement.appendChild(iframe);
475+
iframe.contentDocument.body.appendChild(video);
476+
await ampAdElement.signals().signal(CommonSignals_Enum.INI_LOAD);
477+
478+
expect(pageElement).to.have.class(fullbleedAdClass);
479+
toggleExperiment(env.win, 'story-ad-allow-fullbleed', false);
480+
});
481+
});
482+
460483
describe('page level analytics', () => {
461484
let fireEventStub;
462485

tools/experiments/experiments-config.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,4 +203,9 @@ export const EXPERIMENTS = [
203203
name: 'Enable new INP metrics reporting on amp-analytics',
204204
spec: 'https://github.com/ampproject/amphtml/issues/38470',
205205
},
206+
{
207+
id: 'story-ad-allow-fullbleed',
208+
name: 'Allows story ads in AMP fullbleed mode to fill the whole screen.',
209+
spec: 'https://github.com/ampproject/amphtml/pull/40264',
210+
},
206211
];

0 commit comments

Comments
 (0)