From a7daa3963a4733e9718f2eb801827c14100a7edf Mon Sep 17 00:00:00 2001 From: Charlotte Date: Wed, 8 Jan 2025 16:02:21 +0000 Subject: [PATCH] use second slot as merch high instead of static slot --- .../src/components/DecideFrontsAdSlots.tsx | 9 ++++- .../src/lib/getFrontsAdPositions.ts | 39 ++++++------------- .../src/lib/getTagPageAdPositions.ts | 9 +---- 3 files changed, 20 insertions(+), 37 deletions(-) diff --git a/dotcom-rendering/src/components/DecideFrontsAdSlots.tsx b/dotcom-rendering/src/components/DecideFrontsAdSlots.tsx index af16efe188e..e038fbe8ca2 100644 --- a/dotcom-rendering/src/components/DecideFrontsAdSlots.tsx +++ b/dotcom-rendering/src/components/DecideFrontsAdSlots.tsx @@ -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, @@ -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 ( diff --git a/dotcom-rendering/src/lib/getFrontsAdPositions.ts b/dotcom-rendering/src/lib/getFrontsAdPositions.ts index 44179f544c9..259af7c0101 100644 --- a/dotcom-rendering/src/lib/getFrontsAdPositions.ts +++ b/dotcom-rendering/src/lib/getFrontsAdPositions.ts @@ -17,9 +17,6 @@ type GroupedCounts = { type AdCandidate = Pick; -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 @@ -27,26 +24,22 @@ const getMerchHighPosition = (collectionCount: number): number => 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; @@ -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) @@ -268,10 +259,4 @@ const getFrontsBannerAdPositions = ( { heightSinceAd: 0, adPositions: [] }, ).adPositions; -export { - isEvenIndex, - isMerchHighPosition, - getMerchHighPosition, - getMobileAdPositions, - getFrontsBannerAdPositions, -}; +export { isEvenIndex, getMobileAdPositions, getFrontsBannerAdPositions }; diff --git a/dotcom-rendering/src/lib/getTagPageAdPositions.ts b/dotcom-rendering/src/lib/getTagPageAdPositions.ts index 0ea66a4f1fc..5dff6c11b07 100644 --- a/dotcom-rendering/src/lib/getTagPageAdPositions.ts +++ b/dotcom-rendering/src/lib/getTagPageAdPositions.ts @@ -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 @@ -20,10 +16,7 @@ import { const getTagPageMobileAdPositions = ( collections: Array, ): number[] => { - const merchHighPosition = getMerchHighPosition(collections.length); - return collections - .filter((_, index) => !isMerchHighPosition(index, merchHighPosition)) .filter(isEvenIndex) .map((collection) => collections.findIndex(