From 5c2383e8c43ac5f200994f1ed213a9085e47570b Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Mon, 26 Jun 2023 15:18:11 +0100 Subject: [PATCH 01/25] feat: Add support for tuples Co-Authored-By: Max Duval --- dotcom-rendering/src/lib/tuple.ts | 56 +++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 dotcom-rendering/src/lib/tuple.ts diff --git a/dotcom-rendering/src/lib/tuple.ts b/dotcom-rendering/src/lib/tuple.ts new file mode 100644 index 00000000000..2b1caf7fb31 --- /dev/null +++ b/dotcom-rendering/src/lib/tuple.ts @@ -0,0 +1,56 @@ +/** + * Type-guard that check whether an array contains at least a single item. + * + * If `true`, the type is narrowed from T[] to the [T, ...T[]] tuple, + * which safely allows accessing the first item. + */ +export const nonEmpty = (arr: T[]): arr is [T, ...T[]] => arr.length >= 1; + +/** A tuple of up to 12 items. Larger tuples will not be narrowed */ +export type Tuple = N extends 12 + ? [T, T, T, T, T, T, T, T, T, T, T, T] + : N extends 11 + ? [T, T, T, T, T, T, T, T, T, T, T] + : N extends 10 + ? [T, T, T, T, T, T, T, T, T, T] + : N extends 9 + ? [T, T, T, T, T, T, T, T, T] + : N extends 8 + ? [T, T, T, T, T, T, T, T] + : N extends 7 + ? [T, T, T, T, T, T, T] + : N extends 6 + ? [T, T, T, T, T, T] + : N extends 5 + ? [T, T, T, T, T] + : N extends 4 + ? [T, T, T, T] + : N extends 3 + ? [T, T, T] + : N extends 2 + ? [T, T] + : N extends 1 + ? [T] + : N extends 0 + ? [] + : T[]; + +/** + * Type-guard for whether an array is a tuple of exact length. + * + * Only tuples of 12 elements or less will be narrowed. + */ +export const isTuple = ( + arr: Array | ReadonlyArray, + count: N, +): arr is Tuple => arr.length === count; + +/** + * Type-guard for whether an array is at least a certain number of elements + * + * Can only guarantee up to 12 elements + */ +export const isTupleOrGreater = ( + arr: Array | ReadonlyArray, + count: N, +): arr is [...Tuple, ...Array] => arr.length >= count; From 6bb444f37aa0266ed22c7fd708d84d7065fc69ae Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Mon, 26 Jun 2023 15:18:36 +0100 Subject: [PATCH 02/25] feat: Add support for fast & slow containers on tag fronts --- .../components/DecideContainerByTrails.tsx | 267 ++++++++++++++++-- 1 file changed, 240 insertions(+), 27 deletions(-) diff --git a/dotcom-rendering/src/components/DecideContainerByTrails.tsx b/dotcom-rendering/src/components/DecideContainerByTrails.tsx index 12a3cb1cd78..5c12db5fabf 100644 --- a/dotcom-rendering/src/components/DecideContainerByTrails.tsx +++ b/dotcom-rendering/src/components/DecideContainerByTrails.tsx @@ -1,5 +1,6 @@ import { Card100Media50, + Card100Media75, Card25Media25, Card33Media33, Card50Media50, @@ -24,7 +25,17 @@ export const OneCardFast = ({ trail }: { trail: DCRFrontCard }) => { ); }; -export const TwoCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { +export const OneCardSlow = ({ trail }: { trail: DCRFrontCard }) => { + return ( +
    +
  • + +
    • +
+ ); +}; + +export const TwoCard = ({ trails }: { trails: DCRFrontCard[] }) => { if (!trails[0] || !trails[1]) return <>; return (
    @@ -38,7 +49,7 @@ export const TwoCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const ThreeCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { +export const ThreeCard = ({ trails }: { trails: DCRFrontCard[] }) => { if (!trails[0] || !trails[1] || !trails[2]) return <>; return (
      @@ -55,7 +66,7 @@ export const ThreeCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const FourCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { +export const FourCard = ({ trails }: { trails: DCRFrontCard[] }) => { if (!trails[0] || !trails[1] || !trails[2] || !trails[3]) return <>; return (
        @@ -103,6 +114,34 @@ export const FiveCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; +export const FiveCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { + if (!trails[0] || !trails[1] || !trails[2] || !trails[3] || !trails[4]) + return <>; + return ( + <> +
          +
        • + +
          • +
        • + +
          • +
        +
          +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        + + ); +}; + export const SixCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { if ( !trails[0] || @@ -141,6 +180,45 @@ export const SixCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; + +export const SixCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { + if ( + !trails[0] || + !trails[1] || + !trails[2] || + !trails[3] || + !trails[4] || + !trails[5] + ) + return <>; + return ( + <> +
          +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        +
          +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        + + ); +}; + export const SevenCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { if ( !trails[0] || @@ -184,6 +262,48 @@ export const SevenCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; +export const SevenCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { + if ( + !trails[0] || + !trails[1] || + !trails[2] || + !trails[3] || + !trails[4] || + !trails[5] || + !trails[6] + ) + return <>; + return ( + <> +
          +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        +
          +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        + + ); +}; + export const EightCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { if ( !trails[0] || @@ -231,7 +351,53 @@ export const EightCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const BeyondEight = ({ trails }: { trails: DCRFrontCard[] }) => { +export const EightCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { + if ( + !trails[0] || + !trails[1] || + !trails[2] || + !trails[3] || + !trails[4] || + !trails[5] || + !trails[6] || + !trails[7] + ) + return <>; + return ( + <> +
          +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        +
          +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        + + ); +}; + +export const BeyondEightFast = ({ trails }: { trails: DCRFrontCard[] }) => { const afterEight = trails.slice(8); return ( @@ -253,29 +419,76 @@ export const BeyondEight = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const DecideContainerByTrails = ({ trails }: Props) => { - // TODO: Respect 'speed' +export const BeyondEightSlow = ({ trails }: { trails: DCRFrontCard[] }) => { + const afterEight = trails.slice(8); + + return ( + <> + ; +
          + {afterEight.map((trail, index) => ( +
        • + +
          • + ))} +
        + + ); +}; + +export const DecideContainerByTrails = ({ trails, speed }: Props) => { + // TODO: Respect MPUs - switch (trails.length) { - case 1: - return trails[0] !== undefined ? ( - - ) : null; - case 2: - return ; - case 3: - return ; - case 4: - return ; - case 5: - return ; - case 6: - return ; - case 7: - return ; - case 8: - return ; - default: - return ; + if (speed === 'fast') { + switch (trails.length) { + case 1: + return trails[0] !== undefined ? ( + + ) : null; + case 2: + return ; + case 3: + return ; + case 4: + return ; + case 5: + return ; + case 6: + return ; + case 7: + return ; + case 8: + return ; + default: + return ; + } + } else { + switch (trails.length) { + case 1: + return trails[0] !== undefined ? ( + + ) : null; + case 2: + return ; + case 3: + return ; + case 4: + return ; + case 5: + return ; + case 6: + return ; + case 7: + return ; + case 8: + return ; + default: + return ; + } } }; From 3062bca64b8d5104d7294548699b8876d5cc5e30 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Mon, 26 Jun 2023 15:19:03 +0100 Subject: [PATCH 03/25] feat: Inject MPU states into containers on Tag fronts Revert "feat: Inject MPU states into containers on Tag fronts" This reverts commit 4d8140dd3ae5e31ebbaa340b332862e1cf5c68ab. Revert "Revert "feat: Inject MPU states into containers on Tag fronts"" This reverts commit 7bca0a66bd23c90d7a4b1c129c72959245d16e51. --- .../src/model/groupTrailsByDates.ts | 11 +-- .../src/model/injectMpuIntoGroupedTrails.ts | 68 +++++++++++++++++++ .../src/server/index.front.web.ts | 10 ++- dotcom-rendering/src/types/tagFront.ts | 42 +++++++++++- 4 files changed, 115 insertions(+), 16 deletions(-) create mode 100644 dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts diff --git a/dotcom-rendering/src/model/groupTrailsByDates.ts b/dotcom-rendering/src/model/groupTrailsByDates.ts index b750df0a998..687fe2b1567 100644 --- a/dotcom-rendering/src/model/groupTrailsByDates.ts +++ b/dotcom-rendering/src/model/groupTrailsByDates.ts @@ -1,4 +1,5 @@ import type { DCRFrontCard } from '../types/front'; +import type { GroupedTrails } from '../types/tagFront'; /** * The number of trails per day required (on average) for trails to be @@ -15,16 +16,6 @@ interface TrailAndDate { date: Date; } -/** - * Represents a set of trails grouped by their year, month & optionally day of publication. - */ -export interface GroupedTrails { - year: number; - month: number; - day: number | undefined; - trails: DCRFrontCard[]; -} - /** * Calculates the average number of trails per day across a set of days */ diff --git a/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts b/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts new file mode 100644 index 00000000000..819a8d02b3a --- /dev/null +++ b/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts @@ -0,0 +1,68 @@ +import { isTuple, isTupleOrGreater } from '../lib/tuple'; +import type { + GroupedTrails, + GroupedTrailsFastMpu, + GroupedTrailsSlowMpu, +} from '../types/tagFront'; + +/** + * Injects an MPU container into a list of grouped trails + * + * For both slow & fast tag fronts, containers of certain lengths can receive an MPU slot. + * The code looks for the first container of the right length and injects the ad slot + * object (GroupedTrailsSlowMpu / GroupedTrailsFastMpu). + */ +export const injectMpuIntoGroupedTrails = ( + groupedTrails: GroupedTrails[], + speed: 'slow' | 'fast', +): Array => { + let injected = false; + const result: Array< + GroupedTrails | GroupedTrailsFastMpu | GroupedTrailsSlowMpu + > = []; + + groupedTrails.forEach((grouped) => { + if (injected) { + result.push(grouped); + return; + } + + if (speed === 'fast') { + if ( + isTuple(grouped.trails, 2) || + isTuple(grouped.trails, 4) || + isTuple(grouped.trails, 6) || + isTupleOrGreater(grouped.trails, 9) + ) { + injected = true; + result.push({ + day: grouped.day, + month: grouped.month, + year: grouped.year, + trails: grouped.trails, + injected: true, + speed: 'fast', + }); + } + } else { + if ( + isTuple(grouped.trails, 2) || + isTuple(grouped.trails, 4) || + isTuple(grouped.trails, 5) || + isTuple(grouped.trails, 7) + ) { + injected = true; + result.push({ + day: grouped.day, + month: grouped.month, + year: grouped.year, + trails: grouped.trails, + injected: true, + speed: 'slow', + }); + } + } + }); + + return result; +}; diff --git a/dotcom-rendering/src/server/index.front.web.ts b/dotcom-rendering/src/server/index.front.web.ts index 150ed8e79fa..ee0cfd2485b 100644 --- a/dotcom-rendering/src/server/index.front.web.ts +++ b/dotcom-rendering/src/server/index.front.web.ts @@ -7,6 +7,7 @@ import { extractTrendingTopicsFomFront, } from '../model/extractTrendingTopics'; import { groupTrailsByDates } from '../model/groupTrailsByDates'; +import { injectMpuIntoGroupedTrails } from '../model/injectMpuIntoGroupedTrails'; import { getSpeedFromTrails } from '../model/slowOrFastByTrails'; import { validateAsFrontType, validateAsTagFrontType } from '../model/validate'; import { recordTypeAndPlatform } from '../server/lib/logging-store'; @@ -52,9 +53,12 @@ const enhanceTagFront = (body: unknown): DCRTagFrontType => { return { ...data, tags: data.tags.tags, - groupedTrails: groupTrailsByDates( - enhancedCards, - speed === 'slow' || data.forceDay, + groupedTrails: injectMpuIntoGroupedTrails( + groupTrailsByDates( + enhancedCards, + speed === 'slow' || data.forceDay, + ), + speed, ), speed, // Pagination information comes from the first tag diff --git a/dotcom-rendering/src/types/tagFront.ts b/dotcom-rendering/src/types/tagFront.ts index d0cff6ac6ce..d199f2f181d 100644 --- a/dotcom-rendering/src/types/tagFront.ts +++ b/dotcom-rendering/src/types/tagFront.ts @@ -1,7 +1,7 @@ import type { EditionId } from '../lib/edition'; -import type { GroupedTrails } from '../model/groupTrailsByDates'; +import type { Tuple } from '../lib/tuple'; import type { FooterType } from './footer'; -import type { FEFrontCard, FEFrontConfigType } from './front'; +import type { DCRFrontCard, FEFrontCard, FEFrontConfigType } from './front'; import type { FETagType } from './tag'; export interface FETagFrontType { @@ -23,8 +23,44 @@ export interface FETagFrontType { forceDay: boolean; } +/** + * Represents a set of trails grouped by their year, month & optionally day of publication. + */ +export interface GroupedTrailsBase { + year: number; + month: number; + day: number | undefined; +} + +export interface GroupedTrails extends GroupedTrailsBase { + trails: DCRFrontCard[]; +} + +export interface GroupedTrailsFastMpu extends GroupedTrailsBase { + injected: true; + speed: 'fast'; + // Trails must either be length of 2, 4, 6 or >= 9 + trails: + | Tuple + | Tuple + | Tuple + | [...Tuple, ...DCRFrontCard[]]; +} +export interface GroupedTrailsSlowMpu extends GroupedTrailsBase { + injected: true; + speed: 'slow'; + // Trails must either be length of 2, 4, 5, 7 + trails: + | Tuple + | Tuple + | Tuple + | Tuple; +} + export interface DCRTagFrontType { - groupedTrails: GroupedTrails[]; + groupedTrails: Array< + GroupedTrails | GroupedTrailsFastMpu | GroupedTrailsSlowMpu + >; nav: FENavType; tags: FETagType[]; editionId: EditionId; From ff6fb620b7c0692a83bb794f47bc8253ef0f140b Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 27 Jun 2023 11:08:03 +0100 Subject: [PATCH 04/25] WIP: Add support for MPU injection & ads in tag fronts --- .../src/components/TagFrontFastMpu.tsx | 188 +++++++++++++++++ .../src/components/TagFrontSlowMpu.tsx | 190 ++++++++++++++++++ dotcom-rendering/src/layouts/FrontLayout.tsx | 2 +- .../src/layouts/TagFrontLayout.tsx | 115 ++++++++--- .../src/server/index.front.web.ts | 15 +- 5 files changed, 471 insertions(+), 39 deletions(-) create mode 100644 dotcom-rendering/src/components/TagFrontFastMpu.tsx create mode 100644 dotcom-rendering/src/components/TagFrontSlowMpu.tsx diff --git a/dotcom-rendering/src/components/TagFrontFastMpu.tsx b/dotcom-rendering/src/components/TagFrontFastMpu.tsx new file mode 100644 index 00000000000..dd857b6ce92 --- /dev/null +++ b/dotcom-rendering/src/components/TagFrontFastMpu.tsx @@ -0,0 +1,188 @@ +import { Hide } from '@guardian/source-react-components'; +import { Card33Media33, CardDefault } from '../lib/cardWrappers'; +import { isTuple, isTupleOrGreater, type Tuple } from '../lib/tuple'; +import type { DCRFrontCard } from '../types/front'; +import type { GroupedTrailsFastMpu } from '../types/tagFront'; +import { AdSlot } from './AdSlot'; +import { LI } from './Card/components/LI'; +import { UL } from './Card/components/UL'; + +type Props = GroupedTrailsFastMpu & { + adIndex: number; +}; + +export const TwoCard = ({ + trails, + adIndex, +}: { + trails: Tuple; + adIndex: number; +}) => { + return ( +
          +
        • + +
          • +
        • + +
          • +
        • + + + +
          • +
        + ); +}; + +export const FourCard = ({ + trails, + adIndex, +}: { + trails: Tuple; + adIndex: number; +}) => { + return ( +
          +
        • + +
          • +
        • +
            +
          • + +
            • +
          • + +
            • +
          • + +
            • +
          +
          • +
        • + + + +
          • +
        + ); +}; + +export const SixCard = ({ + trails, + adIndex, +}: { + trails: Tuple; + adIndex: number; +}) => { + return ( +
          +
        • +
            +
          • + +
            • +
          • + +
            • +
          • + +
            • +
          +
          • +
        • +
            +
          • + +
            • +
          • + +
            • +
          • + +
            • +
          +
          • +
        • + + + +
          • +
        + ); +}; + +export const NineCard = ({ + trails, + adIndex, +}: { + trails: [...Tuple, ...DCRFrontCard[]]; + adIndex: number; +}) => { + return ( + <> +
          +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        +
          +
        • +
            +
          • + +
            • +
          • + +
            • +
          • + +
            • +
          +
          • +
        • +
            +
          • + +
            • +
          • + +
            • +
          • + +
            • +
          +
          • +
        • + + + +
          • +
        + + ); +}; + +export const TagFrontFastMpu = ({ trails, adIndex }: Props) => { + if (isTuple(trails, 2)) { + return ; + } else if (isTuple(trails, 4)) { + return ; + } else if (isTuple(trails, 6)) { + return ; + } else if (isTupleOrGreater(trails, 9)) { + // In Frontend, we 'limit' the MPU chosen container at 9, if there are more cards, they simply + // aren't shown. + return ; + } else { + // The above if statements cover all the types that trails could be, but ESLint gets upset that "not all code paths return a value" + return <>; + } +}; diff --git a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx new file mode 100644 index 00000000000..6101aef4fd4 --- /dev/null +++ b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx @@ -0,0 +1,190 @@ +import { Hide } from '@guardian/source-react-components'; +import { Card33Media33, CardDefault } from '../lib/cardWrappers'; +import { isTuple, isTupleOrGreater, type Tuple } from '../lib/tuple'; +import type { DCRFrontCard } from '../types/front'; +import type { GroupedTrailsSlowMpu } from '../types/tagFront'; +import { AdSlot } from './AdSlot'; +import { LI } from './Card/components/LI'; +import { UL } from './Card/components/UL'; + +type Props = GroupedTrailsSlowMpu & { + adIndex: number; +}; + +export const TwoCard = ({ + trails, + adIndex, +}: { + trails: Tuple; + adIndex: number; +}) => { + return ( +
          +
        • + +
          • +
        • + +
          • +
        • + + + +
          • +
        + ); +}; + +export const FourCard = ({ + trails, + adIndex, +}: { + trails: Tuple; + adIndex: number; +}) => { + return ( +
          +
        • + +
          • +
        • +
            +
          • + +
            • +
          • + +
            • +
          • + +
            • +
          +
          • +
        • + + + +
          • +
        + ); +}; + +export const SixCard = ({ + trails, + adIndex, +}: { + trails: Tuple; + adIndex: number; +}) => { + return ( +
          +
        • +
            +
          • + +
            • +
          • + +
            • +
          • + +
            • +
          +
          • +
        • +
            +
          • + +
            • +
          • + +
            • +
          • + +
            • +
          +
          • +
        • + + + +
          • +
        + ); +}; + +export const NineCard = ({ + trails, + adIndex, +}: { + trails: [...Tuple, ...DCRFrontCard[]]; + adIndex: number; +}) => { + return ( + <> +
          +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        +
          +
        • +
            +
          • + +
            • +
          • + +
            • +
          • + +
            • +
          +
          • +
        • +
            +
          • + +
            • +
          • + +
            • +
          • + +
            • +
          +
          • +
        • + + + +
          • +
        + + ); +}; + +export const TagFrontSlowMpu = ({ trails, adIndex }: Props) => { + // if (isTuple(trails, 2)) { + // return ; + // } else if (isTuple(trails, 4)) { + // return ; + // } else if (isTuple(trails, 6)) { + // return ; + // } else if (isTupleOrGreater(trails, 9)) { + // // In Frontend, we 'limit' the MPU chosen container at 9, if there are more cards, they simply + // // aren't shown. + // return ; + // } else { + // // The above if statements cover all the types that trails could be, but ESLint gets upset that "not all code paths return a value" + // return <>; + // } + + return <>; +}; diff --git a/dotcom-rendering/src/layouts/FrontLayout.tsx b/dotcom-rendering/src/layouts/FrontLayout.tsx index aee147428ec..be98d08534a 100644 --- a/dotcom-rendering/src/layouts/FrontLayout.tsx +++ b/dotcom-rendering/src/layouts/FrontLayout.tsx @@ -72,7 +72,7 @@ const isToggleable = ( } else return index != 0 && !isNavList(collection); }; -const decideAdSlot = ( +export const decideAdSlot = ( renderAds: boolean, index: number, isNetworkFront: boolean | undefined, diff --git a/dotcom-rendering/src/layouts/TagFrontLayout.tsx b/dotcom-rendering/src/layouts/TagFrontLayout.tsx index 3354d3b7b86..065eb0dac76 100644 --- a/dotcom-rendering/src/layouts/TagFrontLayout.tsx +++ b/dotcom-rendering/src/layouts/TagFrontLayout.tsx @@ -8,6 +8,7 @@ import { news, } from '@guardian/source-foundations'; import { StraightLines } from '@guardian/source-react-components-development-kitchen'; +import { Fragment } from 'react'; import { AdSlot } from '../components/AdSlot'; import { DecideContainerByTrails } from '../components/DecideContainerByTrails'; import { Footer } from '../components/Footer'; @@ -18,13 +19,20 @@ import { Island } from '../components/Island'; import { Nav } from '../components/Nav/Nav'; import { Section } from '../components/Section'; import { SubNav } from '../components/SubNav.importable'; +import { TagFrontFastMpu } from '../components/TagFrontFastMpu'; import { TagFrontHeader } from '../components/TagFrontHeader'; +import { TagFrontSlowMpu } from '../components/TagFrontSlowMpu'; import { TrendingTopics } from '../components/TrendingTopics'; import { canRenderAds } from '../lib/canRenderAds'; import { decidePalette } from '../lib/decidePalette'; import { getEditionFromId } from '../lib/edition'; +import { + getMerchHighPosition, + getMobileAdPositions, +} from '../lib/getAdPositions'; import type { NavType } from '../model/extract-nav'; import type { DCRTagFrontType } from '../types/tagFront'; +import { decideAdSlot } from './FrontLayout'; import { Stuck } from './lib/stickiness'; interface Props { @@ -66,11 +74,20 @@ export const TagFrontLayout = ({ tagFront, NAV }: Props) => { const palette = decidePalette(format); + // const merchHighPosition = getMerchHighPosition( + // tagFront.groupedTrails.length, + // false, + // ); + /** * This property currently only applies to the header and merchandising slots */ const renderAds = canRenderAds(tagFront); + // const mobileAdPositions = renderAds + // ? getMobileAdPositions(front.pressedPage.collections, merchHighPosition) + // : []; + return ( <>
        @@ -192,6 +209,35 @@ export const TagFrontLayout = ({ tagFront, NAV }: Props) => { groupedTrails.day !== undefined, ); + const ContainerComponent = () => { + if ( + 'injected' in groupedTrails && + 'speed' in groupedTrails + ) { + if (groupedTrails.speed === 'fast') { + return ( + + ); + } else { + return ( + + ); + } + } + return ( + + ); + }; + const url = groupedTrails.day !== undefined ? `/${tagFront.pageId}/${groupedTrails.year}/${date @@ -208,37 +254,44 @@ export const TagFrontLayout = ({ tagFront, NAV }: Props) => { : undefined; return ( - - - + + + + + {/* {decideAdSlot( + renderAds, + index, + false, + tagFront.groupedTrails.length, + tagFront.config.isPaidContent, + mobileAdPositions, + hasPageSkin, + )} */} + ); })} diff --git a/dotcom-rendering/src/server/index.front.web.ts b/dotcom-rendering/src/server/index.front.web.ts index ee0cfd2485b..e2d1ad4353f 100644 --- a/dotcom-rendering/src/server/index.front.web.ts +++ b/dotcom-rendering/src/server/index.front.web.ts @@ -50,16 +50,17 @@ const enhanceTagFront = (body: unknown): DCRTagFrontType => { const enhancedCards = enhanceCards(data.contents, {}); const speed = getSpeedFromTrails(data.contents); + const groupedTrails = groupTrailsByDates( + enhancedCards, + speed === 'slow' || data.forceDay, + ); + return { ...data, tags: data.tags.tags, - groupedTrails: injectMpuIntoGroupedTrails( - groupTrailsByDates( - enhancedCards, - speed === 'slow' || data.forceDay, - ), - speed, - ), + groupedTrails: data.isAdFreeUser + ? groupedTrails + : injectMpuIntoGroupedTrails(groupedTrails, speed), speed, // Pagination information comes from the first tag pagination: From 962622eec9116996f97d7eba4dac8b68734dd803 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Wed, 28 Jun 2023 16:35:56 +0100 Subject: [PATCH 05/25] feat: Add support for desktop & mobile ads on tag fronts --- .../src/layouts/TagFrontLayout.tsx | 25 +++++++++------- dotcom-rendering/src/lib/getAdPositions.ts | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/dotcom-rendering/src/layouts/TagFrontLayout.tsx b/dotcom-rendering/src/layouts/TagFrontLayout.tsx index 065eb0dac76..58a413be670 100644 --- a/dotcom-rendering/src/layouts/TagFrontLayout.tsx +++ b/dotcom-rendering/src/layouts/TagFrontLayout.tsx @@ -28,7 +28,7 @@ import { decidePalette } from '../lib/decidePalette'; import { getEditionFromId } from '../lib/edition'; import { getMerchHighPosition, - getMobileAdPositions, + getTagFrontMobileAdPositions, } from '../lib/getAdPositions'; import type { NavType } from '../model/extract-nav'; import type { DCRTagFrontType } from '../types/tagFront'; @@ -74,19 +74,22 @@ export const TagFrontLayout = ({ tagFront, NAV }: Props) => { const palette = decidePalette(format); - // const merchHighPosition = getMerchHighPosition( - // tagFront.groupedTrails.length, - // false, - // ); + const merchHighPosition = getMerchHighPosition( + tagFront.groupedTrails.length, + false, + ); /** * This property currently only applies to the header and merchandising slots */ const renderAds = canRenderAds(tagFront); - // const mobileAdPositions = renderAds - // ? getMobileAdPositions(front.pressedPage.collections, merchHighPosition) - // : []; + const mobileAdPositions = renderAds + ? getTagFrontMobileAdPositions( + tagFront.groupedTrails, + merchHighPosition, + ) + : []; return ( <> @@ -282,15 +285,15 @@ export const TagFrontLayout = ({ tagFront, NAV }: Props) => { > - {/* {decideAdSlot( + {decideAdSlot( renderAds, index, false, tagFront.groupedTrails.length, tagFront.config.isPaidContent, mobileAdPositions, - hasPageSkin, - )} */} + tagFront.config.hasPageSkin, + )} ); })} diff --git a/dotcom-rendering/src/lib/getAdPositions.ts b/dotcom-rendering/src/lib/getAdPositions.ts index 9ddeb895e87..bb3faf48d43 100644 --- a/dotcom-rendering/src/lib/getAdPositions.ts +++ b/dotcom-rendering/src/lib/getAdPositions.ts @@ -1,4 +1,5 @@ import type { DCRCollectionType } from '../types/front'; +import type { GroupedTrailsBase } from '../types/tagFront'; type AdCandidate = Pick; @@ -74,6 +75,34 @@ export const getMobileAdPositions = ( // Should insert no more than 10 ads .slice(0, 10); +/** + * Uses a very similar approach to pressed fronts, except we + * - Don't need to consider thrashers + * - Don't need to consider the 'most viewed' container + * + * The types are also slightly different, as we + */ +export const getTagFrontMobileAdPositions = ( + collections: Array, + merchHighPosition: number, +): number[] => + collections + .filter( + (_, index) => + !hasAdjacentCommercialContainer(index, merchHighPosition), + ) + .filter(isEvenIndex) + .map((collection) => + collections.findIndex( + (c) => + c.day === collection.day && + c.month === collection.month && + c.year === collection.year, + ), + ) + // Should insert no more than 10 ads + .slice(0, 10); + const hasDesktopAd = (collection: AdCandidate) => { return ( collection.collectionType == 'dynamic/slow-mpu' || From d6cac3af58b2990e27c4ff452e5f9ffb13705dcc Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Wed, 28 Jun 2023 16:53:57 +0100 Subject: [PATCH 06/25] Update slow MPU containers to match frontend for tag fronts --- .../src/components/TagFrontSlowMpu.tsx | 141 ++++++++---------- 1 file changed, 59 insertions(+), 82 deletions(-) diff --git a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx index 6101aef4fd4..80d799de8a5 100644 --- a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx +++ b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx @@ -1,6 +1,6 @@ import { Hide } from '@guardian/source-react-components'; -import { Card33Media33, CardDefault } from '../lib/cardWrappers'; -import { isTuple, isTupleOrGreater, type Tuple } from '../lib/tuple'; +import { Card33Media33, Card50Media50, CardDefault } from '../lib/cardWrappers'; +import { isTuple, type Tuple } from '../lib/tuple'; import type { DCRFrontCard } from '../types/front'; import type { GroupedTrailsSlowMpu } from '../types/tagFront'; import { AdSlot } from './AdSlot'; @@ -69,96 +69,77 @@ export const FourCard = ({ ); }; -export const SixCard = ({ +export const FiveCard = ({ trails, adIndex, }: { - trails: Tuple; + trails: Tuple; adIndex: number; }) => { return ( -
          -
        • -
            -
          • - -
            • -
          • - -
            • -
          • - -
            • -
          -
          • -
        • -
            -
          • - -
            • -
          • - -
            • -
          • - -
            • -
          -
          • -
        • - - - -
          • -
        + <> +
          +
        • + +
          • +
        • + +
          • +
        • + +
          • +
        +
          +
        • + +
          • +
        • + +
          • +
        • + + + +
          • +
        + ); }; -export const NineCard = ({ +export const SevenCards = ({ trails, adIndex, }: { - trails: [...Tuple, ...DCRFrontCard[]]; + trails: [...Tuple, ...DCRFrontCard[]]; adIndex: number; }) => { return ( <> +
          +
        • + +
          • +
        • + +
          • +
        • - +
        • - +
        • - +
          -
        • -
            -
          • - -
            • -
          • - -
            • -
          • - -
            • -
          +
        • +
          • -
        • -
            -
          • - -
            • -
          • - -
            • -
          • - -
            • -
          +
        • +
        • @@ -171,20 +152,16 @@ export const NineCard = ({ }; export const TagFrontSlowMpu = ({ trails, adIndex }: Props) => { - // if (isTuple(trails, 2)) { - // return ; - // } else if (isTuple(trails, 4)) { - // return ; - // } else if (isTuple(trails, 6)) { - // return ; - // } else if (isTupleOrGreater(trails, 9)) { - // // In Frontend, we 'limit' the MPU chosen container at 9, if there are more cards, they simply - // // aren't shown. - // return ; - // } else { - // // The above if statements cover all the types that trails could be, but ESLint gets upset that "not all code paths return a value" - // return <>; - // } - - return <>; + if (isTuple(trails, 2)) { + return ; + } else if (isTuple(trails, 4)) { + return ; + } else if (isTuple(trails, 5)) { + return ; + } else if (isTuple(trails, 7)) { + return ; + } else { + // The above if statements cover all the types that trails could be, but ESLint gets upset that "not all code paths return a value" + return <>; + } }; From 1b8e611c7bb422b8195b158b4c1dab640e3eb5fc Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Wed, 28 Jun 2023 16:55:14 +0100 Subject: [PATCH 07/25] use containerId for key on tag fronts map --- dotcom-rendering/src/layouts/TagFrontLayout.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotcom-rendering/src/layouts/TagFrontLayout.tsx b/dotcom-rendering/src/layouts/TagFrontLayout.tsx index 58a413be670..c0c543e3b78 100644 --- a/dotcom-rendering/src/layouts/TagFrontLayout.tsx +++ b/dotcom-rendering/src/layouts/TagFrontLayout.tsx @@ -257,7 +257,7 @@ export const TagFrontLayout = ({ tagFront, NAV }: Props) => { : undefined; return ( - + Date: Wed, 28 Jun 2023 17:13:59 +0100 Subject: [PATCH 08/25] Add stories for mpu containers on tag fronts & fix any bugs --- .../components/TagFrontFastMpu.stories.tsx | 103 ++++++++++++++++++ .../src/components/TagFrontFastMpu.tsx | 10 +- .../components/TagFrontSlowMpu.stories.tsx | 94 ++++++++++++++++ .../src/components/TagFrontSlowMpu.tsx | 16 +-- 4 files changed, 210 insertions(+), 13 deletions(-) create mode 100644 dotcom-rendering/src/components/TagFrontFastMpu.stories.tsx create mode 100644 dotcom-rendering/src/components/TagFrontSlowMpu.stories.tsx diff --git a/dotcom-rendering/src/components/TagFrontFastMpu.stories.tsx b/dotcom-rendering/src/components/TagFrontFastMpu.stories.tsx new file mode 100644 index 00000000000..d9eba85511b --- /dev/null +++ b/dotcom-rendering/src/components/TagFrontFastMpu.stories.tsx @@ -0,0 +1,103 @@ +import { breakpoints } from '@guardian/source-foundations'; +import { trails } from '../../fixtures/manual/trails'; +import { FrontSection } from './FrontSection'; +import { TagFrontFastMpu } from './TagFrontFastMpu'; + +export default { + component: TagFrontFastMpu, + title: 'Components/TagFrontFastMpu', + parameters: { + chromatic: { + viewports: [ + breakpoints.mobile, + breakpoints.tablet, + breakpoints.wide, + ], + }, + }, +}; + +export const WithTwoCards = () => { + return ( + + + + ); +}; +WithTwoCards.storyName = 'With two cards'; + +export const WithFourCards = () => { + return ( + + + + ); +}; +WithFourCards.storyName = 'With four cards'; + +export const WithSixCards = () => { + return ( + + + + ); +}; +WithSixCards.storyName = 'With six cards'; + +export const WithNineCards = () => { + return ( + + + + ); +}; +WithNineCards.storyName = 'With nine cards'; diff --git a/dotcom-rendering/src/components/TagFrontFastMpu.tsx b/dotcom-rendering/src/components/TagFrontFastMpu.tsx index dd857b6ce92..86260fac86c 100644 --- a/dotcom-rendering/src/components/TagFrontFastMpu.tsx +++ b/dotcom-rendering/src/components/TagFrontFastMpu.tsx @@ -79,7 +79,7 @@ export const SixCard = ({ return (
          • -
              +
                • @@ -122,20 +122,20 @@ export const NineCard = ({ }) => { return ( <> -
                  +
                    • -
                  • +
                    • -
                  • +
                  • -
                      +
                        • diff --git a/dotcom-rendering/src/components/TagFrontSlowMpu.stories.tsx b/dotcom-rendering/src/components/TagFrontSlowMpu.stories.tsx new file mode 100644 index 00000000000..f023b9c17c0 --- /dev/null +++ b/dotcom-rendering/src/components/TagFrontSlowMpu.stories.tsx @@ -0,0 +1,94 @@ +import { breakpoints } from '@guardian/source-foundations'; +import { trails } from '../../fixtures/manual/trails'; +import { FrontSection } from './FrontSection'; +import { TagFrontSlowMpu } from './TagFrontSlowMpu'; + +export default { + component: TagFrontSlowMpu, + title: 'Components/TagFrontSlowMpu', + parameters: { + chromatic: { + viewports: [ + breakpoints.mobile, + breakpoints.tablet, + breakpoints.wide, + ], + }, + }, +}; + +export const WithTwoCards = () => { + return ( + + + + ); +}; +WithTwoCards.storyName = 'With two cards'; + +export const WithFourCards = () => { + return ( + + + + ); +}; +WithFourCards.storyName = 'With four cards'; + +export const WithFiveCards = () => { + return ( + + + + ); +}; +WithFiveCards.storyName = 'With five cards'; + +export const WithSevenCards = () => { + return ( + + + + ); +}; +WithSevenCards.storyName = 'With seven cards'; diff --git a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx index 80d799de8a5..c51b19c9fef 100644 --- a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx +++ b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx @@ -78,14 +78,14 @@ export const FiveCard = ({ }) => { return ( <> -
                          +
                            • -
                          • +
                            • -
                          • +
                          @@ -115,22 +115,22 @@ export const SevenCards = ({ }) => { return ( <> -
                            +
                              • -
                            • +
                            -
                              +
                                • -
                              • +
                                • -
                              • +
                              From 944180c27a541a55ed4cb16cca6f9d6c35e51dd2 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Thu, 29 Jun 2023 10:40:16 +0100 Subject: [PATCH 09/25] Add stories for DecideContainerByTrails & fix layout bugs --- .../DecideContainerByTrails.stories.tsx | 188 ++++++++++++ .../components/DecideContainerByTrails.tsx | 270 ++++++++---------- 2 files changed, 304 insertions(+), 154 deletions(-) create mode 100644 dotcom-rendering/src/components/DecideContainerByTrails.stories.tsx diff --git a/dotcom-rendering/src/components/DecideContainerByTrails.stories.tsx b/dotcom-rendering/src/components/DecideContainerByTrails.stories.tsx new file mode 100644 index 00000000000..b0b49b3a477 --- /dev/null +++ b/dotcom-rendering/src/components/DecideContainerByTrails.stories.tsx @@ -0,0 +1,188 @@ +import { breakpoints } from '@guardian/source-foundations'; +import { trails } from '../../fixtures/manual/trails'; +import { DecideContainerByTrails } from './DecideContainerByTrails'; +import { FrontSection } from './FrontSection'; + +export default { + component: DecideContainerByTrails, + title: 'Components/DecideContainerByTrails', + parameters: { + chromatic: { + viewports: [ + breakpoints.mobile, + breakpoints.tablet, + breakpoints.wide, + ], + }, + }, +}; + +export const OneCardFast = () => { + return ( + + + + ); +}; +OneCardFast.storyName = 'Fast - One card'; + +export const TwoCardFast = () => { + return ( + + + + ); +}; +TwoCardFast.storyName = 'Fast - Two cards'; + +export const ThreeCardFast = () => { + return ( + + + + ); +}; +ThreeCardFast.storyName = 'Fast - Three cards'; + +export const FourCardFast = () => { + return ( + + + + ); +}; +FourCardFast.storyName = 'Fast - Four cards'; + +export const FiveCardFast = () => { + return ( + + + + ); +}; +FiveCardFast.storyName = 'Fast - Five cards'; + +export const SixCardFast = () => { + return ( + + + + ); +}; +SixCardFast.storyName = 'Fast - Six cards'; + +export const SevenCardFast = () => { + return ( + + + + ); +}; +SevenCardFast.storyName = 'Fast - Seven cards'; + +export const EightCardFast = () => { + return ( + + + + ); +}; + +EightCardFast.storyName = 'Fast - Eight cards'; + +export const TwelveCardFast = () => { + return ( + + + + ); +}; +TwelveCardFast.storyName = 'Fast - Twelve cards'; + +export const OneCardSlow = () => { + return ( + + + + ); +}; +OneCardSlow.storyName = 'Slow - One card'; + +export const TwoCardSlow = () => { + return ( + + + + ); +}; +TwoCardSlow.storyName = 'Slow - Two cards'; + +export const ThreeCardSlow = () => { + return ( + + + + ); +}; +ThreeCardSlow.storyName = 'Slow - Three cards'; + +export const FourCardSlow = () => { + return ( + + + + ); +}; +FourCardSlow.storyName = 'Slow - Four cards'; + +export const FiveCardSlow = () => { + return ( + + + + ); +}; +FiveCardSlow.storyName = 'Slow - Five cards'; + +export const SixCardSlow = () => { + return ( + + + + ); +}; +SixCardSlow.storyName = 'Slow - Six cards'; + +export const SevenCardSlow = () => { + return ( + + + + ); +}; +SevenCardSlow.storyName = 'Slow - Seven cards'; + +export const EightCardSlow = () => { + return ( + + + + ); +}; + +EightCardSlow.storyName = 'Slow - Eight cards'; + +export const TwelveCardSlow = () => { + return ( + + + + ); +}; +TwelveCardSlow.storyName = 'Slow - Twelve cards'; diff --git a/dotcom-rendering/src/components/DecideContainerByTrails.tsx b/dotcom-rendering/src/components/DecideContainerByTrails.tsx index 5c12db5fabf..0e1ad60bad5 100644 --- a/dotcom-rendering/src/components/DecideContainerByTrails.tsx +++ b/dotcom-rendering/src/components/DecideContainerByTrails.tsx @@ -6,6 +6,8 @@ import { Card50Media50, CardDefault, } from '../lib/cardWrappers'; +import type { Tuple } from '../lib/tuple'; +import { isTuple, isTupleOrGreater } from '../lib/tuple'; import type { DCRFrontCard } from '../types/front'; import { LI } from './Card/components/LI'; import { UL } from './Card/components/UL'; @@ -35,8 +37,7 @@ export const OneCardSlow = ({ trail }: { trail: DCRFrontCard }) => { ); }; -export const TwoCard = ({ trails }: { trails: DCRFrontCard[] }) => { - if (!trails[0] || !trails[1]) return <>; +export const TwoCard = ({ trails }: { trails: Tuple }) => { return (
                              • @@ -49,8 +50,7 @@ export const TwoCard = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const ThreeCard = ({ trails }: { trails: DCRFrontCard[] }) => { - if (!trails[0] || !trails[1] || !trails[2]) return <>; +export const ThreeCard = ({ trails }: { trails: Tuple }) => { return (
                                • @@ -66,8 +66,7 @@ export const ThreeCard = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const FourCard = ({ trails }: { trails: DCRFrontCard[] }) => { - if (!trails[0] || !trails[1] || !trails[2] || !trails[3]) return <>; +export const FourCard = ({ trails }: { trails: Tuple }) => { return (
                                  • @@ -86,9 +85,11 @@ export const FourCard = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const FiveCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { - if (!trails[0] || !trails[1] || !trails[2] || !trails[3] || !trails[4]) - return <>; +export const FiveCardFast = ({ + trails, +}: { + trails: Tuple; +}) => { return (
                                    • @@ -114,12 +115,14 @@ export const FiveCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const FiveCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { - if (!trails[0] || !trails[1] || !trails[2] || !trails[3] || !trails[4]) - return <>; +export const FiveCardSlow = ({ + trails, +}: { + trails: Tuple; +}) => { return ( <> -
                                        +
                                          • @@ -127,14 +130,14 @@ export const FiveCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => {
                                        -
                                          +
                                            • -
                                          • +
                                            • -
                                          • +
                                          @@ -142,17 +145,7 @@ export const FiveCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const SixCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { - if ( - !trails[0] || - !trails[1] || - !trails[2] || - !trails[3] || - !trails[4] || - !trails[5] - ) - return <>; - +export const SixCardFast = ({ trails }: { trails: Tuple }) => { return ( <>
                                            @@ -181,37 +174,28 @@ export const SixCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const SixCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { - if ( - !trails[0] || - !trails[1] || - !trails[2] || - !trails[3] || - !trails[4] || - !trails[5] - ) - return <>; +export const SixCardSlow = ({ trails }: { trails: Tuple }) => { return ( <> -
                                              +
                                                • -
                                              • +
                                                • -
                                              • +
                                              -
                                                +
                                                  • -
                                                • +
                                                  • -
                                                • +
                                                @@ -219,18 +203,11 @@ export const SixCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const SevenCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { - if ( - !trails[0] || - !trails[1] || - !trails[2] || - !trails[3] || - !trails[4] || - !trails[5] || - !trails[6] - ) - return <>; - +export const SevenCardFast = ({ + trails, +}: { + trails: Tuple; +}) => { return ( <>
                                                  @@ -262,41 +239,35 @@ export const SevenCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const SevenCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { - if ( - !trails[0] || - !trails[1] || - !trails[2] || - !trails[3] || - !trails[4] || - !trails[5] || - !trails[6] - ) - return <>; +export const SevenCardSlow = ({ + trails, +}: { + trails: Tuple; +}) => { return ( <> -
                                                    +
                                                      • -
                                                    • +
                                                      • -
                                                    • +
                                                    -
                                                      +
                                                        • -
                                                      • +
                                                        • -
                                                      • +
                                                        • -
                                                      • +
                                                      @@ -304,19 +275,11 @@ export const SevenCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const EightCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { - if ( - !trails[0] || - !trails[1] || - !trails[2] || - !trails[3] || - !trails[4] || - !trails[5] || - !trails[6] || - !trails[7] - ) - return <>; - +export const EightCardFast = ({ + trails, +}: { + trails: Tuple; +}) => { return ( <>
                                                        @@ -351,45 +314,38 @@ export const EightCardFast = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const EightCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { - if ( - !trails[0] || - !trails[1] || - !trails[2] || - !trails[3] || - !trails[4] || - !trails[5] || - !trails[6] || - !trails[7] - ) - return <>; +export const EightCardSlow = ({ + trails, +}: { + trails: Tuple; +}) => { return ( <> -
                                                          +
                                                            • -
                                                          • +
                                                            • -
                                                          • +
                                                            • -
                                                          • +
                                                          -
                                                            +
                                                              • -
                                                            • +
                                                              • -
                                                            • +
                                                              • -
                                                            • +
                                                            @@ -397,12 +353,17 @@ export const EightCardSlow = ({ trails }: { trails: DCRFrontCard[] }) => { ); }; -export const BeyondEightFast = ({ trails }: { trails: DCRFrontCard[] }) => { +export const BeyondEightFast = ({ + trails, +}: { + trails: [...Tuple, ...DCRFrontCard[]]; +}) => { + const firstEight = trails.slice(0, 8) as Tuple; const afterEight = trails.slice(8); return ( <> - ; + ;
                                                              {afterEight.map((trail, index) => (
                                                            • { ); }; -export const BeyondEightSlow = ({ trails }: { trails: DCRFrontCard[] }) => { +export const BeyondEightSlow = ({ + trails, +}: { + trails: [...Tuple, ...DCRFrontCard[]]; +}) => { + const firstEight = trails.slice(0, 8) as Tuple; const afterEight = trails.slice(8); return ( <> - ; + ;
                                                                {afterEight.map((trail, index) => (
                                                              • { }; export const DecideContainerByTrails = ({ trails, speed }: Props) => { - // TODO: Respect MPUs - if (speed === 'fast') { - switch (trails.length) { - case 1: - return trails[0] !== undefined ? ( - - ) : null; - case 2: - return ; - case 3: - return ; - case 4: - return ; - case 5: - return ; - case 6: - return ; - case 7: - return ; - case 8: - return ; - default: - return ; + if (isTuple(trails, 1)) { + return ; + } else if (isTuple(trails, 2)) { + return ; + } else if (isTuple(trails, 3)) { + return ; + } else if (isTuple(trails, 4)) { + return ; + } else if (isTuple(trails, 5)) { + return ; + } else if (isTuple(trails, 6)) { + return ; + } else if (isTuple(trails, 7)) { + return ; + } else if (isTuple(trails, 8)) { + return ; + } else if (isTupleOrGreater(trails, 9)) { + return ; + } else { + return <>; } } else { - switch (trails.length) { - case 1: - return trails[0] !== undefined ? ( - - ) : null; - case 2: - return ; - case 3: - return ; - case 4: - return ; - case 5: - return ; - case 6: - return ; - case 7: - return ; - case 8: - return ; - default: - return ; + if (isTuple(trails, 1)) { + return ; + } else if (isTuple(trails, 2)) { + return ; + } else if (isTuple(trails, 3)) { + return ; + } else if (isTuple(trails, 4)) { + return ; + } else if (isTuple(trails, 5)) { + return ; + } else if (isTuple(trails, 6)) { + return ; + } else if (isTuple(trails, 7)) { + return ; + } else if (isTuple(trails, 8)) { + return ; + } else if (isTupleOrGreater(trails, 9)) { + return ; + } else { + return <>; } } }; From a5221e9360b2ea698763b5180299821d3bb5243f Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 14:19:09 +0100 Subject: [PATCH 10/25] refactor: tidier definitions for tuples Co-Authored-By: Max Duval --- dotcom-rendering/src/types/tagFront.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/dotcom-rendering/src/types/tagFront.ts b/dotcom-rendering/src/types/tagFront.ts index d199f2f181d..74c855d6869 100644 --- a/dotcom-rendering/src/types/tagFront.ts +++ b/dotcom-rendering/src/types/tagFront.ts @@ -41,20 +41,14 @@ export interface GroupedTrailsFastMpu extends GroupedTrailsBase { speed: 'fast'; // Trails must either be length of 2, 4, 6 or >= 9 trails: - | Tuple - | Tuple - | Tuple + | Tuple | [...Tuple, ...DCRFrontCard[]]; } export interface GroupedTrailsSlowMpu extends GroupedTrailsBase { injected: true; speed: 'slow'; // Trails must either be length of 2, 4, 5, 7 - trails: - | Tuple - | Tuple - | Tuple - | Tuple; + trails: Tuple; } export interface DCRTagFrontType { From e4c1af673ae74af1a90ffbfac52b6902d5480479 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 14:29:16 +0100 Subject: [PATCH 11/25] refactor: Use exhaustive checks for tag front MPU components Co-Authored-By: Max Duval --- .../src/components/TagFrontFastMpu.tsx | 26 ++++++++----------- .../src/components/TagFrontSlowMpu.tsx | 20 +++++++------- .../src/model/injectMpuIntoGroupedTrails.ts | 17 +++++++----- dotcom-rendering/src/types/tagFront.ts | 6 ++--- 4 files changed, 33 insertions(+), 36 deletions(-) diff --git a/dotcom-rendering/src/components/TagFrontFastMpu.tsx b/dotcom-rendering/src/components/TagFrontFastMpu.tsx index 86260fac86c..35355e211ea 100644 --- a/dotcom-rendering/src/components/TagFrontFastMpu.tsx +++ b/dotcom-rendering/src/components/TagFrontFastMpu.tsx @@ -1,6 +1,6 @@ import { Hide } from '@guardian/source-react-components'; import { Card33Media33, CardDefault } from '../lib/cardWrappers'; -import { isTuple, isTupleOrGreater, type Tuple } from '../lib/tuple'; +import { type Tuple } from '../lib/tuple'; import type { DCRFrontCard } from '../types/front'; import type { GroupedTrailsFastMpu } from '../types/tagFront'; import { AdSlot } from './AdSlot'; @@ -117,7 +117,7 @@ export const NineCard = ({ trails, adIndex, }: { - trails: [...Tuple, ...DCRFrontCard[]]; + trails: Tuple; adIndex: number; }) => { return ( @@ -171,18 +171,14 @@ export const NineCard = ({ }; export const TagFrontFastMpu = ({ trails, adIndex }: Props) => { - if (isTuple(trails, 2)) { - return ; - } else if (isTuple(trails, 4)) { - return ; - } else if (isTuple(trails, 6)) { - return ; - } else if (isTupleOrGreater(trails, 9)) { - // In Frontend, we 'limit' the MPU chosen container at 9, if there are more cards, they simply - // aren't shown. - return ; - } else { - // The above if statements cover all the types that trails could be, but ESLint gets upset that "not all code paths return a value" - return <>; + switch (trails.length) { + case 2: + return ; + case 4: + return ; + case 6: + return ; + case 9: + return ; } }; diff --git a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx index c51b19c9fef..6efc8175052 100644 --- a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx +++ b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx @@ -152,16 +152,14 @@ export const SevenCards = ({ }; export const TagFrontSlowMpu = ({ trails, adIndex }: Props) => { - if (isTuple(trails, 2)) { - return ; - } else if (isTuple(trails, 4)) { - return ; - } else if (isTuple(trails, 5)) { - return ; - } else if (isTuple(trails, 7)) { - return ; - } else { - // The above if statements cover all the types that trails could be, but ESLint gets upset that "not all code paths return a value" - return <>; + switch (trails.length) { + case 2: + return ; + case 4: + return ; + case 5: + return ; + case 7: + return ; } }; diff --git a/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts b/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts index 819a8d02b3a..585c5ef45a6 100644 --- a/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts +++ b/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts @@ -1,4 +1,4 @@ -import { isTuple, isTupleOrGreater } from '../lib/tuple'; +import { isTuple } from '../lib/tuple'; import type { GroupedTrails, GroupedTrailsFastMpu, @@ -28,18 +28,23 @@ export const injectMpuIntoGroupedTrails = ( } if (speed === 'fast') { + // When we have a container with > 9 trails for fast, + // we 'cap' the number of trails at 9 in order to fit an MPU in. + // Containers that don't get an MPU injected will of course still be + // able to show more than 9 trails. + const fastTrails = grouped.trails.slice(0, 9); if ( - isTuple(grouped.trails, 2) || - isTuple(grouped.trails, 4) || - isTuple(grouped.trails, 6) || - isTupleOrGreater(grouped.trails, 9) + isTuple(fastTrails, 2) || + isTuple(fastTrails, 4) || + isTuple(fastTrails, 6) || + isTuple(fastTrails, 9) ) { injected = true; result.push({ day: grouped.day, month: grouped.month, year: grouped.year, - trails: grouped.trails, + trails: fastTrails, injected: true, speed: 'fast', }); diff --git a/dotcom-rendering/src/types/tagFront.ts b/dotcom-rendering/src/types/tagFront.ts index 74c855d6869..5b177db2949 100644 --- a/dotcom-rendering/src/types/tagFront.ts +++ b/dotcom-rendering/src/types/tagFront.ts @@ -39,10 +39,8 @@ export interface GroupedTrails extends GroupedTrailsBase { export interface GroupedTrailsFastMpu extends GroupedTrailsBase { injected: true; speed: 'fast'; - // Trails must either be length of 2, 4, 6 or >= 9 - trails: - | Tuple - | [...Tuple, ...DCRFrontCard[]]; + // Trails must either be length of 2, 4, 6, 9 + trails: Tuple; } export interface GroupedTrailsSlowMpu extends GroupedTrailsBase { injected: true; From ee4bf30a13f45d861ed80ec328446f9377cc3a40 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 14:34:11 +0100 Subject: [PATCH 12/25] fix: remove unused import --- dotcom-rendering/src/components/TagFrontSlowMpu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx index 6efc8175052..22897a0e45d 100644 --- a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx +++ b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx @@ -1,6 +1,6 @@ import { Hide } from '@guardian/source-react-components'; import { Card33Media33, Card50Media50, CardDefault } from '../lib/cardWrappers'; -import { isTuple, type Tuple } from '../lib/tuple'; +import { type Tuple } from '../lib/tuple'; import type { DCRFrontCard } from '../types/front'; import type { GroupedTrailsSlowMpu } from '../types/tagFront'; import { AdSlot } from './AdSlot'; From b0b5c3c67c75bf02ae98141ff2a683de790ecc24 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 15:38:40 +0100 Subject: [PATCH 13/25] feat: Introduce 'TupleOrLess`and `takeFirst` Co-Authored-By: Max Duval --- dotcom-rendering/src/lib/tuple.ts | 46 +++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/dotcom-rendering/src/lib/tuple.ts b/dotcom-rendering/src/lib/tuple.ts index 2b1caf7fb31..077a5a98701 100644 --- a/dotcom-rendering/src/lib/tuple.ts +++ b/dotcom-rendering/src/lib/tuple.ts @@ -54,3 +54,49 @@ export const isTupleOrGreater = ( arr: Array | ReadonlyArray, count: N, ): arr is [...Tuple, ...Array] => arr.length >= count; + +/** Type where a tuple can have any 'n' number of items or less */ +type TupleOrLess< + T, + N extends 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12, +> = N extends 12 + ? Tuple + : N extends 11 + ? Tuple + : N extends 10 + ? Tuple + : N extends 9 + ? Tuple + : N extends 8 + ? Tuple + : N extends 7 + ? Tuple + : N extends 6 + ? Tuple + : N extends 5 + ? Tuple + : N extends 4 + ? Tuple + : N extends 3 + ? Tuple + : N extends 2 + ? Tuple + : N extends 1 + ? Tuple + : undefined; + +/** + * Takes the first 'n' number of items in an array + * + * By returning `TupleOrLess` you receive a type-safe response + * that can be checked exhaustively. + */ +export const takeFirst = < + T, + N extends 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12, +>( + array: Array | ReadonlyArray, + count: N, +): TupleOrLess => + //@ts-expect-error – we’ve tested this manually and it’s a very helpful method + array.slice(0, count); From bca4962bd4dd2a23d13cecb89b108e1d347ce9b7 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 15:38:44 +0100 Subject: [PATCH 14/25] refactor: Use exhaustive checking for DecideContainerByTrails Co-Authored-By: Max Duval --- .../components/DecideContainerByTrails.tsx | 156 ++++++++---------- 1 file changed, 69 insertions(+), 87 deletions(-) diff --git a/dotcom-rendering/src/components/DecideContainerByTrails.tsx b/dotcom-rendering/src/components/DecideContainerByTrails.tsx index 0e1ad60bad5..0d568f6c27f 100644 --- a/dotcom-rendering/src/components/DecideContainerByTrails.tsx +++ b/dotcom-rendering/src/components/DecideContainerByTrails.tsx @@ -7,7 +7,7 @@ import { CardDefault, } from '../lib/cardWrappers'; import type { Tuple } from '../lib/tuple'; -import { isTuple, isTupleOrGreater } from '../lib/tuple'; +import { takeFirst } from '../lib/tuple'; import type { DCRFrontCard } from '../types/front'; import { LI } from './Card/components/LI'; import { UL } from './Card/components/UL'; @@ -275,11 +275,13 @@ export const SevenCardSlow = ({ ); }; -export const EightCardFast = ({ +export const EightOrMoreFast = ({ trails, }: { - trails: Tuple; + trails: [...Tuple, ...DCRFrontCard[]]; }) => { + const afterEight = trails.slice(8); + return ( <>
                                                                  @@ -310,15 +312,29 @@ export const EightCardFast = ({
                                                                +
                                                                  + {afterEight.map((trail, index) => ( +
                                                                • + +
                                                                  • + ))} +
                                                                ); }; -export const EightCardSlow = ({ +export const EightOrMoreSlow = ({ trails, }: { - trails: Tuple; + trails: [...Tuple, ...DCRFrontCard[]]; }) => { + const afterEight = trails.slice(8); + return ( <>
                                                                  @@ -349,48 +365,6 @@ export const EightCardSlow = ({
                                                                - - ); -}; - -export const BeyondEightFast = ({ - trails, -}: { - trails: [...Tuple, ...DCRFrontCard[]]; -}) => { - const firstEight = trails.slice(0, 8) as Tuple; - const afterEight = trails.slice(8); - - return ( - <> - ; -
                                                                  - {afterEight.map((trail, index) => ( -
                                                                • - -
                                                                  • - ))} -
                                                                - - ); -}; - -export const BeyondEightSlow = ({ - trails, -}: { - trails: [...Tuple, ...DCRFrontCard[]]; -}) => { - const firstEight = trails.slice(0, 8) as Tuple; - const afterEight = trails.slice(8); - - return ( - <> - ;
                                                                  {afterEight.map((trail, index) => (
                                                                • { + const initialTrails = takeFirst(trails, 8); + if (speed === 'fast') { - if (isTuple(trails, 1)) { - return ; - } else if (isTuple(trails, 2)) { - return ; - } else if (isTuple(trails, 3)) { - return ; - } else if (isTuple(trails, 4)) { - return ; - } else if (isTuple(trails, 5)) { - return ; - } else if (isTuple(trails, 6)) { - return ; - } else if (isTuple(trails, 7)) { - return ; - } else if (isTuple(trails, 8)) { - return ; - } else if (isTupleOrGreater(trails, 9)) { - return ; - } else { - return <>; + switch (initialTrails.length) { + case 0: + return <>; + case 1: + return ; + case 2: + return ; + case 3: + return ; + case 4: + return ; + case 5: + return ; + case 6: + return ; + case 7: + return ; + case 8: + return ( + + ); } } else { - if (isTuple(trails, 1)) { - return ; - } else if (isTuple(trails, 2)) { - return ; - } else if (isTuple(trails, 3)) { - return ; - } else if (isTuple(trails, 4)) { - return ; - } else if (isTuple(trails, 5)) { - return ; - } else if (isTuple(trails, 6)) { - return ; - } else if (isTuple(trails, 7)) { - return ; - } else if (isTuple(trails, 8)) { - return ; - } else if (isTupleOrGreater(trails, 9)) { - return ; - } else { - return <>; + switch (initialTrails.length) { + case 0: + return <>; + case 1: + return ; + case 2: + return ; + case 3: + return ; + case 4: + return ; + case 5: + return ; + case 6: + return ; + case 7: + return ; + case 8: + return ( + + ); } } }; From 0ee3334eaf9be67bf926ac913e4da6a639b2ee79 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 15:52:23 +0100 Subject: [PATCH 15/25] fix: dont render empty ULs if theres only 8 cards --- .../components/DecideContainerByTrails.tsx | 60 +++++++++++-------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/dotcom-rendering/src/components/DecideContainerByTrails.tsx b/dotcom-rendering/src/components/DecideContainerByTrails.tsx index 0d568f6c27f..387860a68d7 100644 --- a/dotcom-rendering/src/components/DecideContainerByTrails.tsx +++ b/dotcom-rendering/src/components/DecideContainerByTrails.tsx @@ -298,7 +298,7 @@ export const EightOrMoreFast = ({
                                                                -
                                                                  +
                                                                    0}>
                                                                    • @@ -312,18 +312,22 @@ export const EightOrMoreFast = ({
                                                                  -
                                                                    - {afterEight.map((trail, index) => ( -
                                                                  • - -
                                                                    • - ))} -
                                                                  + {afterEight.length > 0 ? ( +
                                                                    + {afterEight.map((trail, index) => ( +
                                                                  • + +
                                                                    • + ))} +
                                                                  + ) : ( + <> + )} ); }; @@ -351,7 +355,7 @@ export const EightOrMoreSlow = ({
                                                                -
                                                                  +
                                                                    0}>
                                                                    • @@ -365,18 +369,22 @@ export const EightOrMoreSlow = ({
                                                                  -
                                                                    - {afterEight.map((trail, index) => ( -
                                                                  • - -
                                                                    • - ))} -
                                                                  + {afterEight.length > 0 ? ( +
                                                                    + {afterEight.map((trail, index) => ( +
                                                                  • + +
                                                                    • + ))} +
                                                                  + ) : ( + <> + )} ); }; From fc54def6afa7471d4a2ce7163a86a5966eeee90e Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 16:23:40 +0100 Subject: [PATCH 16/25] Add tests for tuples takeFirst --- dotcom-rendering/src/lib/tuple.test.ts | 83 ++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 dotcom-rendering/src/lib/tuple.test.ts diff --git a/dotcom-rendering/src/lib/tuple.test.ts b/dotcom-rendering/src/lib/tuple.test.ts new file mode 100644 index 00000000000..8751a04161c --- /dev/null +++ b/dotcom-rendering/src/lib/tuple.test.ts @@ -0,0 +1,83 @@ +import { takeFirst } from './tuple'; + +describe('takeFirst', () => { + it('Always returns the correct array length when the array is one less, the same as, or one more than n', () => { + const results = [ + // Format, from 1 to 12, cover length n - 1, n & n+1 + takeFirst([0, 1], 1), + takeFirst([0], 1), + takeFirst([], 1), + takeFirst([0, 1, 2], 2), + takeFirst([0, 1], 2), + takeFirst([0], 2), + takeFirst([0, 1, 2, 3], 3), + takeFirst([0, 1, 2], 3), + takeFirst([0, 1], 3), + takeFirst([0, 1, 2, 3, 4], 4), + takeFirst([0, 1, 2, 3], 4), + takeFirst([0, 1, 2], 4), + takeFirst([0, 1, 2, 3, 4, 5], 5), + takeFirst([0, 1, 2, 3, 4], 5), + takeFirst([0, 1, 2, 3], 5), + takeFirst([0, 1, 2, 3, 4, 5, 6], 6), + takeFirst([0, 1, 2, 3, 4, 5], 6), + takeFirst([0, 1, 2, 3, 4], 6), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7], 7), + takeFirst([0, 1, 2, 3, 4, 5, 6], 7), + takeFirst([0, 1, 2, 3, 4, 5], 7), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8], 8), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7], 8), + takeFirst([0, 1, 2, 3, 4, 5, 6], 8), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8, 9], 9), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8], 9), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7], 9), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10], 10), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8, 9], 10), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8], 10), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 11), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10], 11), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8, 9], 11), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12], 12), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 12), + takeFirst([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10], 12), + ] as const; + + // Expected results from n are n, n & n-1 + expect(results[0].length).toEqual(1); + expect(results[1].length).toEqual(1); + expect(results[2].length).toEqual(0); + expect(results[3].length).toEqual(2); + expect(results[4].length).toEqual(2); + expect(results[5].length).toEqual(1); + expect(results[6].length).toEqual(3); + expect(results[7].length).toEqual(3); + expect(results[8].length).toEqual(2); + expect(results[9].length).toEqual(4); + expect(results[10].length).toEqual(4); + expect(results[11].length).toEqual(3); + expect(results[12].length).toEqual(5); + expect(results[13].length).toEqual(5); + expect(results[14].length).toEqual(4); + expect(results[15].length).toEqual(6); + expect(results[16].length).toEqual(6); + expect(results[17].length).toEqual(5); + expect(results[18].length).toEqual(7); + expect(results[19].length).toEqual(7); + expect(results[20].length).toEqual(6); + expect(results[21].length).toEqual(8); + expect(results[22].length).toEqual(8); + expect(results[23].length).toEqual(7); + expect(results[24].length).toEqual(9); + expect(results[25].length).toEqual(9); + expect(results[26].length).toEqual(8); + expect(results[27].length).toEqual(10); + expect(results[28].length).toEqual(10); + expect(results[29].length).toEqual(9); + expect(results[30].length).toEqual(11); + expect(results[31].length).toEqual(11); + expect(results[32].length).toEqual(10); + expect(results[33].length).toEqual(12); + expect(results[34].length).toEqual(12); + expect(results[35].length).toEqual(11); + }); +}); From d7a9adac7a1d243d3568d3ecf1910064ff8f3af5 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 16:24:09 +0100 Subject: [PATCH 17/25] chore: Update comment for takeFirst --- dotcom-rendering/src/lib/tuple.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotcom-rendering/src/lib/tuple.ts b/dotcom-rendering/src/lib/tuple.ts index 077a5a98701..104aa56ff39 100644 --- a/dotcom-rendering/src/lib/tuple.ts +++ b/dotcom-rendering/src/lib/tuple.ts @@ -98,5 +98,5 @@ export const takeFirst = < array: Array | ReadonlyArray, count: N, ): TupleOrLess => - //@ts-expect-error – we’ve tested this manually and it’s a very helpful method + //@ts-expect-error – this output is tested by jest and it’s a very helpful method array.slice(0, count); From 109474675f33bd5c776729e5fb5e20aa81707cef Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 16:25:57 +0100 Subject: [PATCH 18/25] fix: use correct %'s on DecideContainerByTrails --- .../src/components/DecideContainerByTrails.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/dotcom-rendering/src/components/DecideContainerByTrails.tsx b/dotcom-rendering/src/components/DecideContainerByTrails.tsx index 387860a68d7..7ea8691ae52 100644 --- a/dotcom-rendering/src/components/DecideContainerByTrails.tsx +++ b/dotcom-rendering/src/components/DecideContainerByTrails.tsx @@ -258,16 +258,16 @@ export const SevenCardSlow = ({
                                                                  -
                                                                • +
                                                                  • -
                                                                • +
                                                                  • -
                                                                • +
                                                                  • -
                                                                • +
                                                                @@ -356,16 +356,16 @@ export const EightOrMoreSlow = ({
                                                                0}> -
                                                              • +
                                                                • -
                                                              • +
                                                                • -
                                                              • +
                                                                • -
                                                              • +
                                                              From 2edc5d6864ef25c148981f0febcf02ae6e5f1843 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 16:27:09 +0100 Subject: [PATCH 19/25] fix: remove exports from tag front mpu containers as they're not used --- dotcom-rendering/src/components/TagFrontFastMpu.tsx | 8 ++++---- dotcom-rendering/src/components/TagFrontSlowMpu.tsx | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/dotcom-rendering/src/components/TagFrontFastMpu.tsx b/dotcom-rendering/src/components/TagFrontFastMpu.tsx index 35355e211ea..04322cc99f1 100644 --- a/dotcom-rendering/src/components/TagFrontFastMpu.tsx +++ b/dotcom-rendering/src/components/TagFrontFastMpu.tsx @@ -11,7 +11,7 @@ type Props = GroupedTrailsFastMpu & { adIndex: number; }; -export const TwoCard = ({ +const TwoCard = ({ trails, adIndex, }: { @@ -35,7 +35,7 @@ export const TwoCard = ({ ); }; -export const FourCard = ({ +const FourCard = ({ trails, adIndex, }: { @@ -69,7 +69,7 @@ export const FourCard = ({ ); }; -export const SixCard = ({ +const SixCard = ({ trails, adIndex, }: { @@ -113,7 +113,7 @@ export const SixCard = ({ ); }; -export const NineCard = ({ +const NineCard = ({ trails, adIndex, }: { diff --git a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx index 22897a0e45d..1daacad03af 100644 --- a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx +++ b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx @@ -11,7 +11,7 @@ type Props = GroupedTrailsSlowMpu & { adIndex: number; }; -export const TwoCard = ({ +const TwoCard = ({ trails, adIndex, }: { @@ -35,7 +35,7 @@ export const TwoCard = ({ ); }; -export const FourCard = ({ +const FourCard = ({ trails, adIndex, }: { @@ -69,7 +69,7 @@ export const FourCard = ({ ); }; -export const FiveCard = ({ +const FiveCard = ({ trails, adIndex, }: { @@ -106,7 +106,7 @@ export const FiveCard = ({ ); }; -export const SevenCards = ({ +const SevenCards = ({ trails, adIndex, }: { From a4b1ce8ae426725dc006ddfc43f10178649bb794 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 16:27:56 +0100 Subject: [PATCH 20/25] Update dotcom-rendering/src/lib/getAdPositions.ts Co-authored-by: Max Duval --- dotcom-rendering/src/lib/getAdPositions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotcom-rendering/src/lib/getAdPositions.ts b/dotcom-rendering/src/lib/getAdPositions.ts index bb3faf48d43..470af462371 100644 --- a/dotcom-rendering/src/lib/getAdPositions.ts +++ b/dotcom-rendering/src/lib/getAdPositions.ts @@ -77,8 +77,8 @@ export const getMobileAdPositions = ( /** * Uses a very similar approach to pressed fronts, except we - * - Don't need to consider thrashers - * - Don't need to consider the 'most viewed' container + * - Do not need to consider thrashers + * - Do not need to consider the 'most viewed' container * * The types are also slightly different, as we */ From dee5ad3ec05c98bfbc525f817427b7cbe51c0659 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 16:28:15 +0100 Subject: [PATCH 21/25] Update dotcom-rendering/src/lib/getAdPositions.ts Co-authored-by: Max Duval --- dotcom-rendering/src/lib/getAdPositions.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dotcom-rendering/src/lib/getAdPositions.ts b/dotcom-rendering/src/lib/getAdPositions.ts index 470af462371..3f0f49d3b60 100644 --- a/dotcom-rendering/src/lib/getAdPositions.ts +++ b/dotcom-rendering/src/lib/getAdPositions.ts @@ -94,10 +94,10 @@ export const getTagFrontMobileAdPositions = ( .filter(isEvenIndex) .map((collection) => collections.findIndex( - (c) => - c.day === collection.day && - c.month === collection.month && - c.year === collection.year, + ({ day, month, year }) => + day === collection.day && + month === collection.month && + year === collection.year, ), ) // Should insert no more than 10 ads From 3164bba2a9eb4e02b0d5b5e97cac8bc01a2491d5 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 16:28:42 +0100 Subject: [PATCH 22/25] Merge branch 'olly/tag-fronts-containers-and-ads' of github.com:guardian/dotcom-rendering into olly/tag-fronts-containers-and-ads From c2d1f19416b494e7959e831680989957be5c80b7 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 16:28:59 +0100 Subject: [PATCH 23/25] chore: remove isTupleOrGreater as this is no longer used --- dotcom-rendering/src/lib/tuple.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/dotcom-rendering/src/lib/tuple.ts b/dotcom-rendering/src/lib/tuple.ts index 104aa56ff39..88a590c86ea 100644 --- a/dotcom-rendering/src/lib/tuple.ts +++ b/dotcom-rendering/src/lib/tuple.ts @@ -45,16 +45,6 @@ export const isTuple = ( count: N, ): arr is Tuple => arr.length === count; -/** - * Type-guard for whether an array is at least a certain number of elements - * - * Can only guarantee up to 12 elements - */ -export const isTupleOrGreater = ( - arr: Array | ReadonlyArray, - count: N, -): arr is [...Tuple, ...Array] => arr.length >= count; - /** Type where a tuple can have any 'n' number of items or less */ type TupleOrLess< T, From 1468767d6522cf78b68896d8579d36b280f31183 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Tue, 4 Jul 2023 16:34:29 +0100 Subject: [PATCH 24/25] refactor: use takeFirst in injectMpuIntoGroupedTrails --- .../src/model/injectMpuIntoGroupedTrails.ts | 72 ++++++++++--------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts b/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts index 585c5ef45a6..f8753ed2bfe 100644 --- a/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts +++ b/dotcom-rendering/src/model/injectMpuIntoGroupedTrails.ts @@ -1,4 +1,4 @@ -import { isTuple } from '../lib/tuple'; +import { takeFirst } from '../lib/tuple'; import type { GroupedTrails, GroupedTrailsFastMpu, @@ -32,39 +32,47 @@ export const injectMpuIntoGroupedTrails = ( // we 'cap' the number of trails at 9 in order to fit an MPU in. // Containers that don't get an MPU injected will of course still be // able to show more than 9 trails. - const fastTrails = grouped.trails.slice(0, 9); - if ( - isTuple(fastTrails, 2) || - isTuple(fastTrails, 4) || - isTuple(fastTrails, 6) || - isTuple(fastTrails, 9) - ) { - injected = true; - result.push({ - day: grouped.day, - month: grouped.month, - year: grouped.year, - trails: fastTrails, - injected: true, - speed: 'fast', - }); + const fastTrails = takeFirst(grouped.trails, 9); + switch (fastTrails.length) { + case 2: + case 4: + case 6: + case 9: + injected = true; + result.push({ + day: grouped.day, + month: grouped.month, + year: grouped.year, + trails: fastTrails, + injected: true, + speed: 'fast', + }); + break; + default: + break; } } else { - if ( - isTuple(grouped.trails, 2) || - isTuple(grouped.trails, 4) || - isTuple(grouped.trails, 5) || - isTuple(grouped.trails, 7) - ) { - injected = true; - result.push({ - day: grouped.day, - month: grouped.month, - year: grouped.year, - trails: grouped.trails, - injected: true, - speed: 'slow', - }); + // By taking the first 12, we get the benefit of being able to use + // a switch statement here, without 'capping' the number of trails in the + // same way we do when 'fast' + const slowTrails = takeFirst(grouped.trails, 12); + switch (slowTrails.length) { + case 2: + case 4: + case 5: + case 7: + injected = true; + result.push({ + day: grouped.day, + month: grouped.month, + year: grouped.year, + trails: slowTrails, + injected: true, + speed: 'slow', + }); + break; + default: + break; } } }); From c7423453713edd0b831863adbdbf2f9c167c51c2 Mon Sep 17 00:00:00 2001 From: Olly <9575458+OllysCoding@users.noreply.github.com> Date: Wed, 5 Jul 2023 10:39:33 +0100 Subject: [PATCH 25/25] Final tidy-up --- .../src/components/TagFrontFastMpu.tsx | 8 ++++---- .../src/components/TagFrontSlowMpu.tsx | 10 +++++----- dotcom-rendering/src/lib/getAdPositions.ts | 3 ++- dotcom-rendering/src/lib/tuple.ts | 14 +++----------- 4 files changed, 14 insertions(+), 21 deletions(-) diff --git a/dotcom-rendering/src/components/TagFrontFastMpu.tsx b/dotcom-rendering/src/components/TagFrontFastMpu.tsx index 04322cc99f1..261279da0c8 100644 --- a/dotcom-rendering/src/components/TagFrontFastMpu.tsx +++ b/dotcom-rendering/src/components/TagFrontFastMpu.tsx @@ -7,10 +7,6 @@ import { AdSlot } from './AdSlot'; import { LI } from './Card/components/LI'; import { UL } from './Card/components/UL'; -type Props = GroupedTrailsFastMpu & { - adIndex: number; -}; - const TwoCard = ({ trails, adIndex, @@ -170,6 +166,10 @@ const NineCard = ({ ); }; +type Props = GroupedTrailsFastMpu & { + adIndex: number; +}; + export const TagFrontFastMpu = ({ trails, adIndex }: Props) => { switch (trails.length) { case 2: diff --git a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx index 1daacad03af..616818f1763 100644 --- a/dotcom-rendering/src/components/TagFrontSlowMpu.tsx +++ b/dotcom-rendering/src/components/TagFrontSlowMpu.tsx @@ -7,10 +7,6 @@ import { AdSlot } from './AdSlot'; import { LI } from './Card/components/LI'; import { UL } from './Card/components/UL'; -type Props = GroupedTrailsSlowMpu & { - adIndex: number; -}; - const TwoCard = ({ trails, adIndex, @@ -110,7 +106,7 @@ const SevenCards = ({ trails, adIndex, }: { - trails: [...Tuple, ...DCRFrontCard[]]; + trails: Tuple; adIndex: number; }) => { return ( @@ -151,6 +147,10 @@ const SevenCards = ({ ); }; +type Props = GroupedTrailsSlowMpu & { + adIndex: number; +}; + export const TagFrontSlowMpu = ({ trails, adIndex }: Props) => { switch (trails.length) { case 2: diff --git a/dotcom-rendering/src/lib/getAdPositions.ts b/dotcom-rendering/src/lib/getAdPositions.ts index 3f0f49d3b60..f9da1ec6dcb 100644 --- a/dotcom-rendering/src/lib/getAdPositions.ts +++ b/dotcom-rendering/src/lib/getAdPositions.ts @@ -80,7 +80,8 @@ export const getMobileAdPositions = ( * - Do not need to consider thrashers * - Do not need to consider the 'most viewed' container * - * The types are also slightly different, as we + * The types are also slightly different, as we no longer have + * specific container IDs, so we use the date which is unique */ export const getTagFrontMobileAdPositions = ( collections: Array, diff --git a/dotcom-rendering/src/lib/tuple.ts b/dotcom-rendering/src/lib/tuple.ts index 88a590c86ea..a93956eab63 100644 --- a/dotcom-rendering/src/lib/tuple.ts +++ b/dotcom-rendering/src/lib/tuple.ts @@ -1,11 +1,3 @@ -/** - * Type-guard that check whether an array contains at least a single item. - * - * If `true`, the type is narrowed from T[] to the [T, ...T[]] tuple, - * which safely allows accessing the first item. - */ -export const nonEmpty = (arr: T[]): arr is [T, ...T[]] => arr.length >= 1; - /** A tuple of up to 12 items. Larger tuples will not be narrowed */ export type Tuple = N extends 12 ? [T, T, T, T, T, T, T, T, T, T, T, T] @@ -46,7 +38,7 @@ export const isTuple = ( ): arr is Tuple => arr.length === count; /** Type where a tuple can have any 'n' number of items or less */ -type TupleOrLess< +type SlicedTuple< T, N extends 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 12, > = N extends 12 @@ -78,7 +70,7 @@ type TupleOrLess< /** * Takes the first 'n' number of items in an array * - * By returning `TupleOrLess` you receive a type-safe response + * By returning `SlicedTuple` you receive a type-safe response * that can be checked exhaustively. */ export const takeFirst = < @@ -87,6 +79,6 @@ export const takeFirst = < >( array: Array | ReadonlyArray, count: N, -): TupleOrLess => +): SlicedTuple => //@ts-expect-error – this output is tested by jest and it’s a very helpful method array.slice(0, count);