diff --git a/dotcom-rendering/src/components/DecideFrontsAdSlots.test.ts b/dotcom-rendering/src/components/DecideFrontsAdSlots.test.ts deleted file mode 100644 index f906b9ce0e1..00000000000 --- a/dotcom-rendering/src/components/DecideFrontsAdSlots.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { - decideFrontsBannerAdSlot, - decideMerchHighAndMobileAdSlots, -} from './DecideFrontsAdSlots'; - -describe('decideMerchHighAndMobileAdSlots', () => { - // default parameters - const index = 4; - const collectionCount = 10; - const isPaidContent = false; - const mobileAdPositions: number[] = []; - const hasPageSkin = false; - - it("should return null if we shouldn't render ads", () => { - const result = decideMerchHighAndMobileAdSlots( - false, - index, - collectionCount, - isPaidContent, - mobileAdPositions, - hasPageSkin, - ); - - expect(result).toBeNull(); - }); -}); - -describe('decideFrontsBannerAdSlot', () => { - // default parameters - const renderAds = true; - const hasPageSkin = false; - const index = 2; - const desktopAdPositions = [2, 5]; - - it("should return null if we shouldn't render ads", () => { - const result = decideFrontsBannerAdSlot( - false, - hasPageSkin, - index, - desktopAdPositions, - ); - - expect(result).toBeNull(); - }); - - it("should return null if there's a pageskin", () => { - const result = decideFrontsBannerAdSlot( - renderAds, - true, - index, - desktopAdPositions, - ); - - expect(result).toBeNull(); - }); - - test.each([ - [[2, 5], 3], - [[2, 5], 0], - [[], 1], - ])( - 'should return null if desktopAdPositions %p does NOT contain index %i', - (adPositions, i) => { - const result = decideFrontsBannerAdSlot( - renderAds, - hasPageSkin, - i, - adPositions, - ); - - expect(result).toBeNull(); - }, - ); - - test.each([ - [[2, 5], 2], - [[2, 5], 5], - [[1], 1], - ])( - 'should NOT return null if desktopAdPositions %p contains index %i', - (adPositions, i) => { - const result = decideFrontsBannerAdSlot( - renderAds, - hasPageSkin, - i, - adPositions, - ); - - expect(result).not.toBeNull(); - }, - ); -}); diff --git a/dotcom-rendering/src/components/FrontsAdSlots.test.tsx b/dotcom-rendering/src/components/FrontsAdSlots.test.tsx new file mode 100644 index 00000000000..06a4ce47997 --- /dev/null +++ b/dotcom-rendering/src/components/FrontsAdSlots.test.tsx @@ -0,0 +1,88 @@ +import { render } from '@testing-library/react'; +import { FrontsBannerAdSlot, MerchHighOrMobileAdSlot } from './FrontsAdSlots'; + +describe('MerchHighOrMobileAdSlot', () => { + it("should return null if we shouldn't render ads", () => { + const { container } = render( + , + ); + + expect(container.innerHTML).toBe(''); + }); +}); + +describe('FrontsBannerAdSlot', () => { + it("should return null if we shouldn't render ads", () => { + const { container } = render( + , + ); + + expect(container.innerHTML).toBe(''); + }); + + it("should return null if there's a pageskin", () => { + const { container } = render( + , + ); + + expect(container.innerHTML).toBe(''); + }); + + test.each([ + [[2, 5], 3], + [[2, 5], 0], + [[], 1], + ])( + 'should return null if desktopAdPositions %p does NOT contain index %i', + (adPositions, i) => { + const { container } = render( + , + ); + + expect(container.innerHTML).toBe(''); + }, + ); + + test.each([ + [[2, 5], 2], + [[2, 5], 5], + [[1], 1], + ])( + 'should NOT return null if desktopAdPositions %p contains index %i', + (adPositions, i) => { + const { container } = render( + , + ); + + expect(container.innerHTML).not.toBe(''); + expect(container.innerHTML).toMatch('ad-slot-container'); + }, + ); +}); diff --git a/dotcom-rendering/src/components/DecideFrontsAdSlots.tsx b/dotcom-rendering/src/components/FrontsAdSlots.tsx similarity index 67% rename from dotcom-rendering/src/components/DecideFrontsAdSlots.tsx rename to dotcom-rendering/src/components/FrontsAdSlots.tsx index af16efe188e..de4499a2450 100644 --- a/dotcom-rendering/src/components/DecideFrontsAdSlots.tsx +++ b/dotcom-rendering/src/components/FrontsAdSlots.tsx @@ -1,18 +1,24 @@ 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'; -export const decideMerchHighAndMobileAdSlots = ( - renderAds: boolean, - index: number, - collectionCount: number, - isPaidContent: boolean | undefined, - mobileAdPositions: (number | undefined)[], - hasPageSkin: boolean, -) => { +export const MerchHighOrMobileAdSlot = ({ + renderAds, + index, + collectionCount, + isPaidContent, + mobileAdPositions, + hasPageSkin, +}: { + renderAds: boolean; + index: number; + collectionCount: number; + isPaidContent: boolean | undefined; + mobileAdPositions: (number | undefined)[]; + hasPageSkin: boolean; +}) => { if (!renderAds) return null; const minContainers = isPaidContent ? 1 : 2; @@ -52,12 +58,15 @@ export const decideMerchHighAndMobileAdSlots = ( * Page skins are only active above desktop breakpoints * */ -export const decideMerchandisingSlot = ( - renderAds: boolean, - hasPageSkin: boolean, -) => { +export const MerchandisingSlot = ({ + renderAds, + hasPageSkin, +}: { + renderAds: boolean; + hasPageSkin: boolean; +}) => { if (!renderAds) return null; - const MerchandisingSection = ({ children }: { children: ReactNode }) => ( + return (
- {children} -
- ); - return hasPageSkin ? ( - - + {hasPageSkin ? ( + + + + ) : ( - - - ) : ( - - - + )} + ); }; @@ -87,18 +91,23 @@ export const decideMerchandisingSlot = ( * Renders a fronts-banner ad when in the fronts banner AB test. * Only applies to network fronts on desktop screens and wider. */ -export const decideFrontsBannerAdSlot = ( - renderAds: boolean, - hasPageSkin: boolean, - index: number, - desktopAdPositions: number[], -) => { +export const FrontsBannerAdSlot = ({ + renderAds, + hasPageSkin, + index, + desktopAdPositions, +}: { + renderAds: boolean; + hasPageSkin: boolean; + index: number; + desktopAdPositions: number[]; +}) => { if (!renderAds || hasPageSkin) { return null; } if (desktopAdPositions.includes(index)) { - const adIndex = desktopAdPositions.findIndex((_) => _ === index); + const adIndex = desktopAdPositions.indexOf(index); if (adIndex === -1) return null; return ( diff --git a/dotcom-rendering/src/layouts/FrontLayout.tsx b/dotcom-rendering/src/layouts/FrontLayout.tsx index 2b74539f810..bc1b58542cd 100644 --- a/dotcom-rendering/src/layouts/FrontLayout.tsx +++ b/dotcom-rendering/src/layouts/FrontLayout.tsx @@ -10,14 +10,14 @@ import { Carousel } from '../components/Carousel.importable'; import { ContainerOverrides } from '../components/ContainerOverrides'; import { CPScottHeader } from '../components/CPScottHeader'; import { DecideContainer } from '../components/DecideContainer'; -import { - decideFrontsBannerAdSlot, - decideMerchandisingSlot, - decideMerchHighAndMobileAdSlots, -} from '../components/DecideFrontsAdSlots'; import { EditionSwitcherBanner } from '../components/EditionSwitcherBanner.importable'; import { Footer } from '../components/Footer'; import { FrontMostViewed } from '../components/FrontMostViewed'; +import { + FrontsBannerAdSlot, + MerchandisingSlot, + MerchHighOrMobileAdSlot, +} from '../components/FrontsAdSlots'; import { FrontSection } from '../components/FrontSection'; import { HeaderAdSlot } from '../components/HeaderAdSlot'; import { Island } from '../components/Island'; @@ -349,12 +349,12 @@ export const FrontLayout = ({ front, NAV }: Props) => { containerPalette={collection.containerPalette} >
- {decideFrontsBannerAdSlot( - renderAds, - hasPageSkin, - index, - desktopAdPositions, - )} + {!!trail.embedUri && ( { )} - {decideMerchHighAndMobileAdSlots( - renderAds, - index, - filteredCollections.length, - front.pressedPage.frontProperties - .isPaidContent, - mobileAdPositions, - hasPageSkin, - )} +
); @@ -407,12 +411,12 @@ export const FrontLayout = ({ front, NAV }: Props) => { return (
- {decideFrontsBannerAdSlot( - renderAds, - hasPageSkin, - index, - desktopAdPositions, - )} + { renderAds={renderAds} /> - {decideMerchHighAndMobileAdSlots( - renderAds, - index, - filteredCollections.length, - front.pressedPage.frontProperties - .isPaidContent, - mobileAdPositions, - hasPageSkin, - )} +
); } @@ -515,15 +521,17 @@ export const FrontLayout = ({ front, NAV }: Props) => { } /> - {decideMerchHighAndMobileAdSlots( - renderAds, - index, - filteredCollections.length, - front.pressedPage.frontProperties - .isPaidContent, - mobileAdPositions, - hasPageSkin, - )} + ); } @@ -537,12 +545,12 @@ export const FrontLayout = ({ front, NAV }: Props) => { return (
- {decideFrontsBannerAdSlot( - renderAds, - hasPageSkin, - index, - desktopAdPositions, - )} +
{
- {renderAds && - decideMerchHighAndMobileAdSlots( - renderAds, - index, - filteredCollections.length, +
); } return (
- {decideFrontsBannerAdSlot( - renderAds, - hasPageSkin, - index, - desktopAdPositions, - )} + { } /> - {decideMerchHighAndMobileAdSlots( - renderAds, - index, - filteredCollections.length, - front.pressedPage.frontProperties.isPaidContent, - mobileAdPositions, - hasPageSkin, - )} +
); })} @@ -707,7 +719,10 @@ export const FrontLayout = ({ front, NAV }: Props) => { - {decideMerchandisingSlot(renderAds, hasPageSkin)} + {NAV.subNavSections && (
{ return ( <>
- <> - {renderAds && ( - -
- -
-
- )} + {renderAds && ( + +
+ +
+
+ )} - - +
@@ -143,12 +139,12 @@ export const TagPageLayout = ({ tagPage, NAV }: Props) => { return ( - {decideFrontsBannerAdSlot( - renderAds, - hasPageSkin, - index, - desktopAdPositions, - )} + { isTagPage={true} /> - {decideMerchHighAndMobileAdSlots( - renderAds, - index, - tagPage.groupedTrails.length, - isPaidContent, - mobileAdPositions, - hasPageSkin, - )} + ); })} @@ -202,7 +198,10 @@ export const TagPageLayout = ({ tagPage, NAV }: Props) => {
- {decideMerchandisingSlot(renderAds, hasPageSkin)} + {NAV.subNavSections && (