From 313683935871184606a3fac044747da14dd817c7 Mon Sep 17 00:00:00 2001 From: Dominik Lander Date: Mon, 29 Jun 2026 10:25:18 +0100 Subject: [PATCH 1/2] refactor: rename function, use ad size constants instead of numbers --- .../src/components/AdSlot.web.tsx | 52 ++++++++++++------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/dotcom-rendering/src/components/AdSlot.web.tsx b/dotcom-rendering/src/components/AdSlot.web.tsx index 6b92ff4bc20..43b4c46d82a 100644 --- a/dotcom-rendering/src/components/AdSlot.web.tsx +++ b/dotcom-rendering/src/components/AdSlot.web.tsx @@ -91,8 +91,11 @@ type Props = DefaultProps & const halfPageAdHeight = adSizes.halfPage.height; -/** Calculates the minimum height for an ad slot. Used to avoid CLS */ -const getMinHeight = (adHeight: number, padding?: number): number => +/** + * Calculates the minimum height for an ad slot. Assumes an ad label of + * fixed height will be present above the advert. Used to avoid CLS. + */ +const getMinHeightOfAdSlot = (adHeight: number, padding?: number): number => adHeight + labelHeight + (padding ?? 0); const outOfPageStyles = css` @@ -105,7 +108,7 @@ const hideBelowDesktop = css` } `; -const containerMinHeight = getMinHeight(250, space[5]); +const containerMinHeight = getMinHeightOfAdSlot(250, space[5]); const topAboveNavContainerStyles = css` padding-bottom: ${space[5]}px; @@ -126,8 +129,8 @@ const topAboveNavContainerStyles = css` ::before { content: ''; position: absolute; - height: 250px; - width: 728px; + height: ${adSizes.billboard.height}px; + width: ${adSizes.leaderboard.width}px; top: ${labelHeight}px; left: 50%; transform: translateX(-50%); @@ -135,7 +138,7 @@ const topAboveNavContainerStyles = css` } ${from.desktop} { ::before { - width: 970px; + width: ${adSizes.billboard.width}px; } } } @@ -148,7 +151,7 @@ const topAboveNavContainerStyles = css` * render first, we need to reserve space for the ad to avoid CLS. */ const showcaseRightColumnContainerStyles = css` - min-height: ${getMinHeight(halfPageAdHeight)}px; + min-height: ${getMinHeightOfAdSlot(halfPageAdHeight)}px; `; const showcaseRightColumnStyles = css` @@ -178,7 +181,7 @@ const allowFullWidthAdContainerStyles = css` const merchandisingAdStyles = css` position: relative; - min-height: ${getMinHeight(adSizes.billboard.height, space[3])}px; + min-height: ${getMinHeightOfAdSlot(adSizes.billboard.height, space[3])}px; margin: ${space[3]}px auto; max-width: ${breakpoints['wide']}px; overflow: hidden; @@ -187,7 +190,10 @@ const merchandisingAdStyles = css` ${from.desktop} { margin: 0; padding-bottom: ${space[5]}px; - min-height: ${getMinHeight(adSizes.billboard.height, space[5])}px; + min-height: ${getMinHeightOfAdSlot( + adSizes.billboard.height, + space[5], + )}px; } &:not(.ad-slot--fluid).ad-slot--rendered { @@ -220,7 +226,7 @@ const liveblogInlineContainerStyles = css` const liveblogInlineAdStyles = css` position: relative; - min-height: ${getMinHeight(adSizes.mpu.height)}px; + min-height: ${getMinHeightOfAdSlot(adSizes.mpu.height)}px; background-color: ${schemedPalette('--ad-background-article-inner')}; ${until.tablet} { @@ -230,7 +236,7 @@ const liveblogInlineAdStyles = css` const liveblogInlineMobileAdStyles = css` position: relative; - min-height: ${getMinHeight(adSizes.outstreamMobile.height)}px; + min-height: ${getMinHeightOfAdSlot(adSizes.outstreamMobile.height)}px; ${from.tablet} { display: none; @@ -239,7 +245,7 @@ const liveblogInlineMobileAdStyles = css` const mobileFrontAdStyles = css` position: relative; - min-height: ${getMinHeight(adSizes.mpu.height, space[3])}px; + min-height: ${getMinHeightOfAdSlot(adSizes.mpu.height, space[3])}px; min-width: 300px; width: 300px; margin: ${space[3]}px auto; @@ -255,11 +261,17 @@ const frontsBannerAdTopContainerStyles = css` ${from.tablet} { display: flex; justify-content: center; - min-height: ${getMinHeight(adSizes.leaderboard.height, space[6])}px; + min-height: ${getMinHeightOfAdSlot( + adSizes.leaderboard.height, + space[6], + )}px; background-color: ${schemedPalette('--ad-background')}; } ${from.desktop} { - min-height: ${getMinHeight(adSizes.billboard.height, space[6])}px; + min-height: ${getMinHeightOfAdSlot( + adSizes.billboard.height, + space[6], + )}px; } `; @@ -277,7 +289,7 @@ const frontsBannerCollapseStyles = css` const frontsBannerAdStyles = css` position: relative; - min-height: ${getMinHeight(adSizes.leaderboard.height, space[6])}px; + min-height: ${getMinHeightOfAdSlot(adSizes.leaderboard.height, space[6])}px; max-width: ${adSizes.leaderboard.width}px; overflow: hidden; padding-bottom: ${space[6]}px; @@ -300,7 +312,7 @@ const articleEndAdStyles = css` const mostPopAdStyles = css` position: relative; - min-height: ${getMinHeight(adSizes.mpu.height)}px; + min-height: ${getMinHeightOfAdSlot(adSizes.mpu.height)}px; min-width: ${adSizes.mpu.width}px; max-width: ${adSizes.mpu.width}px; text-align: center; @@ -314,7 +326,7 @@ const mostPopAdStyles = css` `; const mostPopContainerStyles = css` - min-height: ${getMinHeight(adSizes.mpu.height)}px; + min-height: ${getMinHeightOfAdSlot(adSizes.mpu.height)}px; min-width: ${adSizes.mpu.width}px; width: fit-content; max-width: ${adSizes.mpu.width}px; @@ -328,7 +340,7 @@ const mostPopContainerStyles = css` `; const liveBlogTopAdStyles = css` - min-height: ${getMinHeight(adSizes.mpu.height)}px; + min-height: ${getMinHeightOfAdSlot(adSizes.mpu.height)}px; min-width: ${adSizes.mpu.width}px; width: fit-content; max-width: ${adSizes.mpu.width}px; @@ -409,12 +421,12 @@ const mobileStickyAdStyles = css` `; const crosswordBannerMobileAdStyles = css` - min-height: ${getMinHeight(adSizes.mobilesticky.height)}px; + min-height: ${getMinHeightOfAdSlot(adSizes.mobilesticky.height)}px; `; const galleryInlineAdStyles = css` margin: ${space[3]}px auto; - min-height: ${getMinHeight(adSizes.mpu.height)}px; + min-height: ${getMinHeightOfAdSlot(adSizes.mpu.height)}px; &.ad-slot--fluid { margin: ${space[3]}px auto; From 11b4ea4a32119548c3237e35381fca28a06a8427 Mon Sep 17 00:00:00 2001 From: Dominik Lander Date: Mon, 29 Jun 2026 10:25:52 +0100 Subject: [PATCH 2/2] Reserve 90px ad space for top-above-nav on tablet --- dotcom-rendering/src/components/AdSlot.web.tsx | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/dotcom-rendering/src/components/AdSlot.web.tsx b/dotcom-rendering/src/components/AdSlot.web.tsx index 43b4c46d82a..04d58b7de4b 100644 --- a/dotcom-rendering/src/components/AdSlot.web.tsx +++ b/dotcom-rendering/src/components/AdSlot.web.tsx @@ -108,7 +108,7 @@ const hideBelowDesktop = css` } `; -const containerMinHeight = getMinHeightOfAdSlot(250, space[5]); +const topAboveNavPaddingHeight = space[5]; const topAboveNavContainerStyles = css` padding-bottom: ${space[5]}px; @@ -117,7 +117,17 @@ const topAboveNavContainerStyles = css` text-align: left; display: block; width: 100%; - min-height: ${containerMinHeight}px; + + min-height: ${getMinHeightOfAdSlot( + adSizes.leaderboard.height, + topAboveNavPaddingHeight, + )}px; + ${from.desktop} { + min-height: ${getMinHeightOfAdSlot( + adSizes.billboard.height, + topAboveNavPaddingHeight, + )}px; + } /* Remove the min-height when the ad has rendered, so that the container can shrink if the ad is smaller */ &[top-above-nav-ad-rendered='true'] { @@ -129,7 +139,7 @@ const topAboveNavContainerStyles = css` ::before { content: ''; position: absolute; - height: ${adSizes.billboard.height}px; + height: ${adSizes.leaderboard.height}px; width: ${adSizes.leaderboard.width}px; top: ${labelHeight}px; left: 50%; @@ -138,6 +148,7 @@ const topAboveNavContainerStyles = css` } ${from.desktop} { ::before { + height: ${adSizes.billboard.height}px; width: ${adSizes.billboard.width}px; } }