Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use second ad slot as merch high instead of static slot #13110

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
+29 −66
Draft

Use second ad slot as merch high instead of static slot #13110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7daa39
use second slot as merch high instead of static slot
cemms1 Jan 8, 2025
80f1e4d
update tests
cemms1 Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions dotcom-rendering/src/components/DecideFrontsAdSlots.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { Hide } from '@guardian/source/react-components';
import type { ReactNode } from 'react';
import { getMerchHighPosition } from '../lib/getFrontsAdPositions';
import { palette as themePalette } from '../palette';
import { AdSlot } from './AdSlot.web';
import { Section } from './Section';

/** The merchandising high slot is usually the second ad position on a page.
* If there are fewer than 4 collections it is first ad position. */
const getMerchHighSlot = (collectionCount: number): number =>
collectionCount >= 4 ? 1 : 0;

export const decideMerchHighAndMobileAdSlots = (
renderAds: boolean,
index: number,
Expand All @@ -16,10 +20,11 @@ export const decideMerchHighAndMobileAdSlots = (
if (!renderAds) return null;

const minContainers = isPaidContent ? 1 : 2;
const merchHighSlot = getMerchHighSlot(collectionCount);
const shouldInsertMerchHighSlot =
!hasPageSkin &&
collectionCount > minContainers &&
index === getMerchHighPosition(collectionCount);
index === mobileAdPositions[merchHighSlot];

if (shouldInsertMerchHighSlot) {
return (
Expand Down
38 changes: 9 additions & 29 deletions dotcom-rendering/src/lib/getFrontsAdPositions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,35 +25,15 @@ describe('Mobile Ads', () => {
expect(mobileAdPositions).not.toContain(0);
});

// MerchandiseHigh is in position:
// 2: when it's a network front and collections are equal or more than 4
// 0: when collections are less than 4
it.each([
[4, 2],
[5, 2],
[99, 2],
[3, 0],
[2, 0],
[0, 0],
])(
`should not insert an ad in the merchandising-high position when there are %i collections and merchandising-high is in position %i`,
(numCollections, merchHighPosition) => {
const mobileAdPositions = getMobileAdPositions(
defaultTestCollections.slice(0, numCollections),
);
expect(mobileAdPositions).not.toContain(merchHighPosition);
},
);

it('Should not insert ad after a thrasher container', () => {
it('Should not insert ad before a thrasher container', () => {
const testCollections = [...defaultTestCollections];
testCollections.splice(6, 0, { collectionType: 'fixed/thrasher' });
testCollections.splice(9, 0, { collectionType: 'fixed/thrasher' });

const mobileAdPositions = getMobileAdPositions(testCollections);

expect(mobileAdPositions).not.toContain(7);
expect(mobileAdPositions).not.toContain(10);
expect(mobileAdPositions).not.toContain(5);
expect(mobileAdPositions).not.toContain(8);
});

// We used https://www.theguardian.com/uk/commentisfree as a blueprint
Expand All @@ -76,7 +56,7 @@ describe('Mobile Ads', () => {

const mobileAdPositions = getMobileAdPositions(testCollections);

expect(mobileAdPositions).toEqual([0, 3, 5, 7, 9, 11]);
expect(mobileAdPositions).toEqual([0, 2, 4, 6, 8, 10]);
});

// We used https://www.theguardian.com/uk as a blueprint
Expand Down Expand Up @@ -110,7 +90,7 @@ describe('Mobile Ads', () => {

const mobileAdPositions = getMobileAdPositions(testCollections);

expect(mobileAdPositions).toEqual([0, 3, 6, 9, 12, 16, 18, 20, 22]);
expect(mobileAdPositions).toEqual([0, 2, 4, 8, 11, 14, 17, 19, 21]);
});

// We used https://www.theguardian.com/international as a blueprint
Expand Down Expand Up @@ -140,7 +120,7 @@ describe('Mobile Ads', () => {

const mobileAdPositions = getMobileAdPositions(testCollections);

expect(mobileAdPositions).toEqual([0, 3, 6, 9, 13, 15, 17]);
expect(mobileAdPositions).toEqual([0, 2, 5, 7, 11, 14, 16, 18]);
});

// We used https://www.theguardian.com/us as a blueprint
Expand Down Expand Up @@ -171,7 +151,7 @@ describe('Mobile Ads', () => {

const mobileAdPositions = getMobileAdPositions(testCollections);

expect(mobileAdPositions).toEqual([0, 4, 7, 10, 13, 15, 17]);
expect(mobileAdPositions).toEqual([0, 2, 5, 9, 12, 14, 16, 19]);
});

// We used https://www.theguardian.com/uk/lifeandstyle as a blueprint
Expand All @@ -197,7 +177,7 @@ describe('Mobile Ads', () => {

const mobileAdPositions = getMobileAdPositions(testCollections);

expect(mobileAdPositions).toEqual([0, 4, 8, 10, 13]);
expect(mobileAdPositions).toEqual([0, 3, 6, 9, 12, 14]);
});

// We used https://www.theguardian.com/tone/recipes as a blueprint
Expand All @@ -221,7 +201,7 @@ describe('Mobile Ads', () => {

const mobileAdPositions = getMobileAdPositions(testCollections);

expect(mobileAdPositions).toEqual([1, 4, 6, 8, 10, 12]);
expect(mobileAdPositions).toEqual([1, 3, 5, 7, 9, 11]);
});
});

Expand Down
39 changes: 12 additions & 27 deletions dotcom-rendering/src/lib/getFrontsAdPositions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,36 +17,29 @@ type GroupedCounts = {

type AdCandidate = Pick<DCRCollectionType, 'collectionType'>;

const getMerchHighPosition = (collectionCount: number): number =>
collectionCount >= 4 ? 2 : 0;

/**
* This happens on the recipes front, where the first container is a thrasher
* @see https://github.com/guardian/frontend/pull/20617
*/
const isFirstContainerAndThrasher = (collectionType: string, index: number) =>
index === 0 && collectionType === 'fixed/thrasher';

const isMerchHighPosition = (
collectionIndex: number,
merchHighPosition: number,
): boolean => collectionIndex === merchHighPosition;

const isBeforeThrasher = (index: number, collections: AdCandidate[]) =>
collections[index + 1]?.collectionType === 'fixed/thrasher';

const isMostViewedContainer = (collection: AdCandidate) =>
collection.collectionType === 'news/most-popular';

const shouldInsertAd =
(merchHighPosition: number) =>
(collection: AdCandidate, index: number, collections: AdCandidate[]) =>
!(
isFirstContainerAndThrasher(collection.collectionType, index) ||
isMerchHighPosition(index, merchHighPosition) ||
isBeforeThrasher(index, collections) ||
isMostViewedContainer(collection)
);
const shouldInsertAd = (
collection: AdCandidate,
index: number,
collections: AdCandidate[],
) =>
!(
isFirstContainerAndThrasher(collection.collectionType, index) ||
isBeforeThrasher(index, collections) ||
isMostViewedContainer(collection)
);

const isEvenIndex = (_collection: unknown, index: number): boolean =>
index % 2 === 0;
Expand All @@ -61,10 +54,8 @@ const isEvenIndex = (_collection: unknown, index: number): boolean =>
* we take every other container, up to a maximum of 10, for targeting MPU insertion.
*/
const getMobileAdPositions = (collections: AdCandidate[]): number[] => {
const merchHighPosition = getMerchHighPosition(collections.length);

return collections
.filter(shouldInsertAd(merchHighPosition))
.filter(shouldInsertAd)
.filter(isEvenIndex)
.map((collection: AdCandidate) => collections.indexOf(collection))
.filter((adPosition: number) => adPosition !== -1)
Expand Down Expand Up @@ -268,10 +259,4 @@ const getFrontsBannerAdPositions = (
{ heightSinceAd: 0, adPositions: [] },
).adPositions;

export {
isEvenIndex,
isMerchHighPosition,
getMerchHighPosition,
getMobileAdPositions,
getFrontsBannerAdPositions,
};
export { isEvenIndex, getMobileAdPositions, getFrontsBannerAdPositions };
9 changes: 1 addition & 8 deletions dotcom-rendering/src/lib/getTagPageAdPositions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@ import {
MAX_FRONTS_BANNER_ADS,
MAX_FRONTS_MOBILE_ADS,
} from './commercial-constants';
import {
getMerchHighPosition,
isEvenIndex,
isMerchHighPosition,
} from './getFrontsAdPositions';
import { isEvenIndex } from './getFrontsAdPositions';

/**
* Uses a very similar approach to pressed fronts, except we
Expand All @@ -20,10 +16,7 @@ import {
const getTagPageMobileAdPositions = (
collections: Array<GroupedTrailsBase>,
): number[] => {
const merchHighPosition = getMerchHighPosition(collections.length);

return collections
.filter((_, index) => !isMerchHighPosition(index, merchHighPosition))
.filter(isEvenIndex)
.map((collection) =>
collections.findIndex(
Expand Down
Loading