Skip to content

Add ads in galleries on both web and app #14396

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

Merged
merged 8 commits into from
Aug 19, 2025
Merged

Add ads in galleries on both web and app #14396

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
894b96c
Add ehance ad placeholders for Gallery DCAR
deedeeh Aug 12, 2025
13ae790
WIP
deedeeh Aug 12, 2025
2949eed
fix gallery ad insertion for both web and app
marjisound Aug 12, 2025
3b26c76
Insert the add after every forth image
marjisound Aug 13, 2025
e0344ec
Change variable name elementCounter to imageBlockElementCounter
deedeeh Aug 18, 2025
bf29a4f
Merge branch 'main' into dina/ads-in-galleries-dcar
deedeeh Aug 18, 2025
361ebe3
Refactor for better performance and property naming
deedeeh Aug 18, 2025
7c018c7
Merge remote-tracking branch 'origin' into dina/ads-in-galleries-dcar
deedeeh Aug 19, 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
91 changes: 52 additions & 39 deletions dotcom-rendering/src/layouts/GalleryLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
} from '@guardian/source/foundations';
import { Hide } from '@guardian/source/react-components';
import { Fragment } from 'react';
import { AdPlaceholder } from '../components/AdPlaceholder.apps';
import { AdPortals } from '../components/AdPortals.importable';
import { AdSlot } from '../components/AdSlot.web';
import { AppsFooter } from '../components/AppsFooter.importable';
import { ArticleHeadline } from '../components/ArticleHeadline';
Expand All @@ -30,7 +32,6 @@ import { type ArticleFormat, ArticleSpecial } from '../lib/articleFormat';
import { canRenderAds } from '../lib/canRenderAds';
import { getContributionsServiceUrl } from '../lib/contributions';
import { decideMainMediaCaption } from '../lib/decide-caption';
import { getAdPositions } from '../lib/getGalleryAdPositions';
import type { NavType } from '../model/extract-nav';
import { palette as themePalette } from '../palette';
import type { Gallery } from '../types/article';
Expand Down Expand Up @@ -159,10 +160,6 @@ export const GalleryLayout = (props: WebProps | AppProps) => {

const contributionsServiceUrl = getContributionsServiceUrl(frontendData);

const adPositions: number[] = renderAds
? getAdPositions(gallery.images)
: [];

return (
<>
{isWeb && (
Expand Down Expand Up @@ -200,11 +197,17 @@ export const GalleryLayout = (props: WebProps | AppProps) => {
/>
</div>
)}

<main
css={{
backgroundColor: themePalette('--article-background'),
}}
>
{isApps && renderAds && (
<Island priority="critical">
<AdPortals />
</Island>
)}
<div css={border}>Labs header</div>
<header css={headerStyles}>
<MainMediaGallery
Expand Down Expand Up @@ -289,41 +292,51 @@ export const GalleryLayout = (props: WebProps | AppProps) => {
) : null}
</div>
</header>
{gallery.images.map((element, idx) => {
const index = idx + 1;
const shouldShowAds = adPositions.includes(index);

{gallery.images.map((element, index) => {
const isImage =
element._type ===
'model.dotcomrendering.pageElements.ImageBlockElement';
const shouldShowAds =
element._type ===
'model.dotcomrendering.pageElements.AdPlaceholderBlockElement';
return (
<Fragment key={element.elementId}>
<GalleryImage
image={element}
format={format}
pageId={frontendData.pageId}
webTitle={frontendData.webTitle}
renderingTarget={props.renderingTarget}
/>
{isWeb && shouldShowAds && (
<div css={galleryItemAdvertStyles}>
<div css={galleryInlineAdContainerStyles}>
<Hide until="tablet">
<DesktopAdSlot
renderAds={renderAds}
adSlotIndex={adPositions.indexOf(
index,
)}
/>
</Hide>
<Hide from="tablet">
<MobileAdSlot
renderAds={renderAds}
adSlotIndex={adPositions.indexOf(
index,
)}
/>
</Hide>
</div>
<div css={galleryBorder}></div>
</div>
<Fragment key={isImage ? element.elementId : index}>
{isImage && (
<GalleryImage
image={element}
format={format}
pageId={frontendData.pageId}
webTitle={frontendData.webTitle}
renderingTarget={props.renderingTarget}
/>
)}
{shouldShowAds && renderAds && (
<>
{isWeb && (
<div css={galleryItemAdvertStyles}>
<div
css={
galleryInlineAdContainerStyles
}
>
<Hide until="tablet">
<DesktopAdSlot
renderAds={renderAds}
adSlotIndex={index}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps not one for this PR, but @marjisound we may want to look at abstracting some of this layout file into sub-components? This function is now nearly 300 lines long and hitting 13 levels of indentation in some places, which may make it difficult to read and understand, particularly when looking at a diff?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's always better to have less lines of code in a function and refactor when needed but I second you regarding that could be in a separate PR.

/>
</Hide>
<Hide from="tablet">
<MobileAdSlot
renderAds={renderAds}
adSlotIndex={index}
/>
</Hide>
</div>
<div css={galleryBorder}></div>
</div>
)}
{isApps && <AdPlaceholder />}
</>
)}
</Fragment>
);
Expand Down
17 changes: 0 additions & 17 deletions dotcom-rendering/src/lib/getGalleryAdPositions.ts
View file
Open in desktop

This file was deleted.

85 changes: 78 additions & 7 deletions dotcom-rendering/src/model/enhance-ad-placeholders.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,65 @@ const insertPlaceholder = (
return [...prevElements, placeholder, currentElement];
};

const insertPlaceholderAfterCurrentElement = (
prevElements: FEElement[],
currentElement: FEElement,
): FEElement[] => {
const placeholder: AdPlaceholderBlockElement = {
_type: 'model.dotcomrendering.pageElements.AdPlaceholderBlockElement',
};
return [...prevElements, currentElement, placeholder];
};

type ReducerAccumulatorGallery = {
elements: FEElement[];
imageBlockElementCounter: number;
};

/**
* Insert ad placeholders for gallery articles.
* Gallery-specific rules:
* - Place ads after every 4th image
* - Start placing ads after the 4th image
* @param elements - The array of elements to enhance
* @returns The enhanced array of elements with ad placeholders inserted
*/
const insertAdPlaceholdersForGallery = (elements: FEElement[]): FEElement[] => {
const elementsWithReducerContext = elements.reduce(
(
prev: ReducerAccumulatorGallery,
currentElement: FEElement,
): ReducerAccumulatorGallery => {
const imageBlockElementCounter =
currentElement._type ===
'model.dotcomrendering.pageElements.ImageBlockElement'
? prev.imageBlockElementCounter + 1
: prev.imageBlockElementCounter;

const shouldInsertAd = imageBlockElementCounter % 4 === 0;

const currentElements = [...prev.elements, currentElement];

return {
elements: shouldInsertAd
? insertPlaceholderAfterCurrentElement(
prev.elements,
currentElement,
)
: currentElements,
imageBlockElementCounter,
};
},
// Initial value for reducer function
{
elements: [],
imageBlockElementCounter: 0,
},
);

return elementsWithReducerContext.elements;
};

/**
* - elementCounter: the number of paragraphs and images that we've counted when considering ad insertion
* - lastAdIndex: the index of the most recently inserted ad, used to calculate the elements between subsequent ads
Expand Down Expand Up @@ -140,10 +199,22 @@ export const enhanceAdPlaceholders =
renderingTarget: RenderingTarget,
shouldHideAds: boolean,
) =>
(elements: FEElement[]): FEElement[] =>
renderingTarget === 'Apps' &&
!shouldHideAds &&
format.design !== ArticleDesign.LiveBlog &&
format.design !== ArticleDesign.DeadBlog
? insertAdPlaceholders(elements)
: elements;
(elements: FEElement[]): FEElement[] => {
if (shouldHideAds) return elements;

// In galleries the AdPlaceholders are inserted in both
// Web & App because the same logic is used for both
if (format.design === ArticleDesign.Gallery) {
return insertAdPlaceholdersForGallery(elements);
}

if (
renderingTarget === 'Apps' &&
format.design !== ArticleDesign.LiveBlog &&
format.design !== ArticleDesign.DeadBlog
) {
return insertAdPlaceholders(elements);
}

return elements;
};
13 changes: 10 additions & 3 deletions dotcom-rendering/src/types/article.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ import {
type TableOfContentsItem,
} from '../model/enhanceTableOfContents';
import { enhancePinnedPost } from '../model/pinnedPost';
import type { FEElement, ImageBlockElement, ImageForLightbox } from './content';
import type {
AdPlaceholderBlockElement,
FEElement,
ImageBlockElement,
ImageForLightbox,
} from './content';
import { type RenderingTarget } from './renderingTarget';

/**
Expand All @@ -40,7 +45,7 @@ export type ArticleFields = {

export type Gallery = ArticleFields & {
design: ArticleDesign.Gallery;
images: ImageBlockElement[];
images: (ImageBlockElement | AdPlaceholderBlockElement)[];
mainMedia: ImageBlockElement;
};

Expand Down Expand Up @@ -134,7 +139,9 @@ export const enhanceArticleType = (
block.elements.filter(
(element) =>
element._type ===
'model.dotcomrendering.pageElements.ImageBlockElement',
'model.dotcomrendering.pageElements.ImageBlockElement' ||
element._type ===
'model.dotcomrendering.pageElements.AdPlaceholderBlockElement',
),
),
mainMedia: getGalleryMainMedia(
Expand Down
Loading