From 755d36a45930e797d6294958bf657ba25ba63bb3 Mon Sep 17 00:00:00 2001 From: Charlotte Emms <43961396+cemms1@users.noreply.github.com> Date: Wed, 17 Jan 2024 18:03:10 +0000 Subject: [PATCH] Revert "Refactor `YoutubeAtom` so it does not rely on an external `elementId`" --- .../fixtures/generated/articles/Dead.ts | 1 + .../fixtures/generated/articles/Feature.ts | 1 + .../fixtures/generated/articles/Live.ts | 1 + dotcom-rendering/fixtures/manual/trails.ts | 4 +- .../src/components/Card/Card.stories.tsx | 2 +- dotcom-rendering/src/components/Card/Card.tsx | 5 +- .../src/components/Elements.amp.tsx | 2 +- .../src/components/LiveBlock.stories.tsx | 1 + .../YoutubeAtom/YoutubeAtom.stories.tsx | 56 +++++++++---------- .../YoutubeAtom/YoutubeAtom.test.tsx | 18 +++--- .../components/YoutubeAtom/YoutubeAtom.tsx | 12 ++-- .../YoutubeBlockComponent.importable.tsx | 14 +---- .../YoutubeBlockComponent.stories.tsx | 7 +++ .../src/lib/liveblogAdSlots.test.ts | 1 + dotcom-rendering/src/lib/renderElement.tsx | 1 + .../src/model/article-schema.json | 8 ++- dotcom-rendering/src/model/block-schema.json | 4 ++ dotcom-rendering/src/model/enhanceCards.ts | 2 +- dotcom-rendering/src/model/front-schema.json | 4 +- dotcom-rendering/src/types/content.ts | 1 + dotcom-rendering/src/types/mainMedia.ts | 2 +- 21 files changed, 80 insertions(+), 67 deletions(-) diff --git a/dotcom-rendering/fixtures/generated/articles/Dead.ts b/dotcom-rendering/fixtures/generated/articles/Dead.ts index 53b7c9b9c30..6c6ec3a8cc0 100644 --- a/dotcom-rendering/fixtures/generated/articles/Dead.ts +++ b/dotcom-rendering/fixtures/generated/articles/Dead.ts @@ -1974,6 +1974,7 @@ export const Dead: DCRArticle = { ], expired: false, duration: 142, + elementId: '61b9e0cb-6602-4ccb-bda6-a6d6c3f2eae0', }, ], attributes: { diff --git a/dotcom-rendering/fixtures/generated/articles/Feature.ts b/dotcom-rendering/fixtures/generated/articles/Feature.ts index edb492f0322..352d56c22f7 100644 --- a/dotcom-rendering/fixtures/generated/articles/Feature.ts +++ b/dotcom-rendering/fixtures/generated/articles/Feature.ts @@ -1455,6 +1455,7 @@ export const Feature: DCRArticle = { duration: 207, altText: "Press Room - 92nd Academy Awards
epa08208148 Joaquin Phoenix poses in the press room with the Oscar for Best Actor for his performance in 'Joker' during the 92nd annual Academy Awards ceremony at the Dolby Theatre in Hollywood, California, USA, 09 February 2020. The Oscars are presented for outstanding individual or collective efforts in filmmaking in 24 categories. EPA/DAVID SWANSON", + elementId: 'c7aded24-44a0-42fb-bf42-94d2559c0a25', }, ], canonicalUrl: diff --git a/dotcom-rendering/fixtures/generated/articles/Live.ts b/dotcom-rendering/fixtures/generated/articles/Live.ts index 7503e7fd927..8e839b5e80a 100644 --- a/dotcom-rendering/fixtures/generated/articles/Live.ts +++ b/dotcom-rendering/fixtures/generated/articles/Live.ts @@ -1974,6 +1974,7 @@ export const Live: DCRArticle = { ], expired: false, duration: 142, + elementId: '93a2d182-3d98-4319-9a8b-602e4adff560', }, ], attributes: { diff --git a/dotcom-rendering/fixtures/manual/trails.ts b/dotcom-rendering/fixtures/manual/trails.ts index 06c3b19ab7a..919093f4e2e 100644 --- a/dotcom-rendering/fixtures/manual/trails.ts +++ b/dotcom-rendering/fixtures/manual/trails.ts @@ -94,7 +94,7 @@ export const trails: [ showQuotedHeadline: false, mainMedia: { type: 'Video', - id: 'abcdef', + elementId: 'abcdef', videoId: 'abcd', title: 'some title', duration: 378, @@ -527,7 +527,7 @@ export const trails: [ ], mainMedia: { type: 'Video', - id: 'abcdef', + elementId: 'abcdef', videoId: 'abcd', title: 'some title', duration: 378, diff --git a/dotcom-rendering/src/components/Card/Card.stories.tsx b/dotcom-rendering/src/components/Card/Card.stories.tsx index 5509a5ee7f9..083d74b3281 100644 --- a/dotcom-rendering/src/components/Card/Card.stories.tsx +++ b/dotcom-rendering/src/components/Card/Card.stories.tsx @@ -52,7 +52,7 @@ const aBasicLink = { const mainVideo: MainMedia = { type: 'Video', - id: '1234-abcdef-09876-xyz', + elementId: '1234-abcdef-09876-xyz', videoId: '8M_yH-e9cq8', title: '’I care, but I don’t care’: Life after the Queen’s death | Anywhere but Westminster', expired: false, diff --git a/dotcom-rendering/src/components/Card/Card.tsx b/dotcom-rendering/src/components/Card/Card.tsx index ca3e35525af..af4e2f53b65 100644 --- a/dotcom-rendering/src/components/Card/Card.tsx +++ b/dotcom-rendering/src/components/Card/Card.tsx @@ -473,7 +473,10 @@ export const Card = ({ defer={{ until: 'visible' }} > { }, { duration: 142, + elementId: '27eac530-7088-4541-a1c5-3347a3d837fb', expired: false, mediaTitle: 'Nasa launches Perseverance rover in mission to find evidence of life on Mars – video', diff --git a/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.stories.tsx b/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.stories.tsx index 57693d70f29..2907c77e948 100644 --- a/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.stories.tsx +++ b/dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.stories.tsx @@ -55,7 +55,7 @@ export const NoConsent = (): JSX.Element => { return (
{ return (
{
{
{
{
{
Scroll down...
{
Scroll down...
{ return (
{ />
{ return (
{ adTargeting={adTargeting} /> { adTargeting={adTargeting} /> {
Scroll down...
{ return (
{ return (
{ return (
{ return (
{ return (
{ return (
{
{
⬇️
{
⬇️
{ return (
{ />
{ return (
{ abTestParticipations={{}} /> { abTestParticipations={{}} /> { value={{ renderingTarget: 'Web', darkModeAvailable: false }} > { value={{ renderingTarget: 'Web', darkModeAvailable: false }} > { value={{ renderingTarget: 'Web', darkModeAvailable: false }} > { value={{ renderingTarget: 'Web', darkModeAvailable: false }} > { value={{ renderingTarget: 'Web', darkModeAvailable: false }} > { value={{ renderingTarget: 'Web', darkModeAvailable: false }} > { value={{ renderingTarget: 'Web', darkModeAvailable: false }} > { value={{ renderingTarget: 'Web', darkModeAvailable: false }} > { abTestParticipations={{}} /> (false); const [pauseVideo, setPauseVideo] = useState(false); - const uniqueId = `${videoId}-${index ?? 'server'}`; + const uniqueId = `${videoId}-${elementId}`; const enableIma = imaEnabled && !!adTargeting && @@ -145,10 +145,6 @@ export const YoutubeAtom = ({ loadPlayer = false; } - if (isUndefined(index)) { - loadPlayer = false; - } - /** * Create a stable callback as it will be a useEffect dependency in YoutubeAtomPlayer */ diff --git a/dotcom-rendering/src/components/YoutubeBlockComponent.importable.tsx b/dotcom-rendering/src/components/YoutubeBlockComponent.importable.tsx index c96d1fc003f..c3527a6f2b8 100644 --- a/dotcom-rendering/src/components/YoutubeBlockComponent.importable.tsx +++ b/dotcom-rendering/src/components/YoutubeBlockComponent.importable.tsx @@ -13,6 +13,7 @@ import { YoutubeAtom } from './YoutubeAtom/YoutubeAtom'; type Props = { id: string; + elementId: string; mediaTitle?: string; altText?: string; assetId: string; @@ -86,11 +87,9 @@ const getLargestImageSize = ( }[], ) => [...images].sort((a, b) => a.width - b.width).pop(); -/** always undefined on the server */ -let counter: number | undefined; - export const YoutubeBlockComponent = ({ id, + elementId, assetId, mediaTitle, altText, @@ -122,13 +121,6 @@ export const YoutubeBlockComponent = ({ abTestsApi?.isUserInVariant('IntegrateIma', 'variant') ?? false; const abTestParticipations = abTests?.participations ?? {}; - const [index, setIndex] = useState(); - - useEffect(() => { - counter ??= 0; - setIndex(++counter); - }, []); - useEffect(() => { const defineConsentState = async () => { const { onConsentChange } = await import( @@ -208,7 +200,7 @@ export const YoutubeBlockComponent = ({ return (
{ assetId="d2Q5bXvEgMg" mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video" id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28" + elementId="5ab531a2-f6ea-499d-b274-191114c8628c" expired={false} stickyVideos={false} /> @@ -82,6 +83,7 @@ export const Vertical = () => { assetId="d2Q5bXvEgMg" mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video" id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28" + elementId="5ab531a2-f6ea-499d-b274-191114c8628c" expired={false} height={259} width={460} @@ -116,6 +118,7 @@ export const Expired = () => { assetId="d2Q5bXvEgMg" mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video" id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28" + elementId="5ab531a2-f6ea-499d-b274-191114c8628c" expired={true} overrideImage="https://i.guim.co.uk/img/media/49565a29c6586fe6b748926e0be96c5e9c90473c/0_0_4981_2989/500.jpg?quality=85&auto=format&fit=max&s=17c70ec70002ea34886fd6c2605cd81e" height={259} @@ -151,6 +154,7 @@ export const WithOverlayImage = () => { assetId="d2Q5bXvEgMg" mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video" id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28" + elementId="5ab531a2-f6ea-499d-b274-191114c8628c" expired={false} duration={333} overrideImage="https://i.guim.co.uk/img/media/49565a29c6586fe6b748926e0be96c5e9c90473c/0_0_4981_2989/500.jpg?quality=85&auto=format&fit=max&s=17c70ec70002ea34886fd6c2605cd81e" @@ -187,6 +191,7 @@ export const WithPosterImage = () => { assetId="d2Q5bXvEgMg" mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video" id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28" + elementId="5ab531a2-f6ea-499d-b274-191114c8628c" expired={false} duration={333} posterImage={[ @@ -244,6 +249,7 @@ export const WithPosterAndOverlayImage = () => { assetId="d2Q5bXvEgMg" mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video" id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28" + elementId="5ab531a2-f6ea-499d-b274-191114c8628c" expired={false} overrideImage="https://i.guim.co.uk/img/media/49565a29c6586fe6b748926e0be96c5e9c90473c/0_0_4981_2989/500.jpg?quality=85&auto=format&fit=max&s=17c70ec70002ea34886fd6c2605cd81e" duration={333} @@ -302,6 +308,7 @@ export const WithShowMainVideoFlagOff = () => { assetId="d2Q5bXvEgMg" mediaTitle="Prince Harry and Meghan's 'bombshell' plans explained – video" id="c2b8a51c-cb3d-41e7-bb79-1d9a091d0c28" + elementId="5ab531a2-f6ea-499d-b274-191114c8628c" expired={false} overrideImage="https://i.guim.co.uk/img/media/49565a29c6586fe6b748926e0be96c5e9c90473c/0_0_4981_2989/500.jpg?quality=85&auto=format&fit=max&s=17c70ec70002ea34886fd6c2605cd81e" duration={333} diff --git a/dotcom-rendering/src/lib/liveblogAdSlots.test.ts b/dotcom-rendering/src/lib/liveblogAdSlots.test.ts index e86b004753c..28696daa6ac 100644 --- a/dotcom-rendering/src/lib/liveblogAdSlots.test.ts +++ b/dotcom-rendering/src/lib/liveblogAdSlots.test.ts @@ -55,6 +55,7 @@ describe('calculateApproximateBlockHeight', () => { _type: 'model.dotcomrendering.pageElements.YoutubeBlockElement', id: '1', assetId: '', + elementId: '1', expired: false, mediaTitle: '', }, diff --git a/dotcom-rendering/src/lib/renderElement.tsx b/dotcom-rendering/src/lib/renderElement.tsx index 2b14b99afcc..e9c328d3d59 100644 --- a/dotcom-rendering/src/lib/renderElement.tsx +++ b/dotcom-rendering/src/lib/renderElement.tsx @@ -743,6 +743,7 @@ export const renderElement = ({ hideCaption={hideCaption} isMainMedia={isMainMedia} id={element.id} + elementId={element.elementId} assetId={element.assetId} expired={element.expired} overrideImage={element.overrideImage} diff --git a/dotcom-rendering/src/model/article-schema.json b/dotcom-rendering/src/model/article-schema.json index 4eab22c5fdc..153fac988f5 100644 --- a/dotcom-rendering/src/model/article-schema.json +++ b/dotcom-rendering/src/model/article-schema.json @@ -3306,6 +3306,9 @@ "type": "string", "const": "model.dotcomrendering.pageElements.YoutubeBlockElement" }, + "elementId": { + "type": "string" + }, "assetId": { "type": "string" }, @@ -3355,6 +3358,7 @@ "required": [ "_type", "assetId", + "elementId", "expired", "id", "mediaTitle" @@ -4394,7 +4398,7 @@ "type": "string", "const": "Video" }, - "id": { + "elementId": { "type": "string" }, "videoId": { @@ -4439,9 +4443,9 @@ }, "required": [ "duration", + "elementId", "expired", "height", - "id", "images", "origin", "title", diff --git a/dotcom-rendering/src/model/block-schema.json b/dotcom-rendering/src/model/block-schema.json index 88f9707623c..5751e27c6aa 100644 --- a/dotcom-rendering/src/model/block-schema.json +++ b/dotcom-rendering/src/model/block-schema.json @@ -2895,6 +2895,9 @@ "type": "string", "const": "model.dotcomrendering.pageElements.YoutubeBlockElement" }, + "elementId": { + "type": "string" + }, "assetId": { "type": "string" }, @@ -2944,6 +2947,7 @@ "required": [ "_type", "assetId", + "elementId", "expired", "id", "mediaTitle" diff --git a/dotcom-rendering/src/model/enhanceCards.ts b/dotcom-rendering/src/model/enhanceCards.ts index bd3ecfb19ee..a113dadb0bb 100644 --- a/dotcom-rendering/src/model/enhanceCards.ts +++ b/dotcom-rendering/src/model/enhanceCards.ts @@ -225,7 +225,7 @@ const getActiveMediaAtom = (mediaAtom?: FEMediaAtom): MainMedia | undefined => { if (asset?.platform === 'Youtube') { return { type: 'Video', - id: mediaAtom.id, + elementId: mediaAtom.id, videoId: asset.id, duration: mediaAtom.duration ?? 0, title: mediaAtom.title, diff --git a/dotcom-rendering/src/model/front-schema.json b/dotcom-rendering/src/model/front-schema.json index d646e13dc45..9f2dfa8f0af 100644 --- a/dotcom-rendering/src/model/front-schema.json +++ b/dotcom-rendering/src/model/front-schema.json @@ -3577,7 +3577,7 @@ "type": "string", "const": "Video" }, - "id": { + "elementId": { "type": "string" }, "videoId": { @@ -3622,9 +3622,9 @@ }, "required": [ "duration", + "elementId", "expired", "height", - "id", "images", "origin", "title", diff --git a/dotcom-rendering/src/types/content.ts b/dotcom-rendering/src/types/content.ts index c5a9269e30d..8f07bd29e7e 100644 --- a/dotcom-rendering/src/types/content.ts +++ b/dotcom-rendering/src/types/content.ts @@ -552,6 +552,7 @@ export interface VideoYoutubeBlockElement extends ThirdPartyEmbeddedContent { export interface YoutubeBlockElement { _type: 'model.dotcomrendering.pageElements.YoutubeBlockElement'; + elementId: string; assetId: string; mediaTitle: string; id: string; diff --git a/dotcom-rendering/src/types/mainMedia.ts b/dotcom-rendering/src/types/mainMedia.ts index c5ebcb2e7ce..dbb21ca7d38 100644 --- a/dotcom-rendering/src/types/mainMedia.ts +++ b/dotcom-rendering/src/types/mainMedia.ts @@ -5,7 +5,7 @@ type Media = { /** For displaying embedded, playable videos directly in cards */ type Video = Media & { type: 'Video'; - id: string; + elementId: string; videoId: string; height: number; width: number;