Skip to content

Commit

Permalink
refactor(YoutubeAtom): no reliance on elementId
Browse files Browse the repository at this point in the history 
the uniqueId only needs to be truly unique on the client-side
which can be simply achieved with an increasing integer

update tests to use regular expression matching
  • Loading branch information
@mxdvl
mxdvl committed Jan 16, 2024
1 parent 7c33275 commit cfbd8a9
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 59 deletions.
3 changes: 0 additions & 3 deletions dotcom-rendering/src/components/Card/Card.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,6 @@ export const Card = ({
>
<YoutubeBlockComponent
id={media.mainMedia.elementId}
elementId={
media.mainMedia.elementId
}
assetId={
media.mainMedia.videoId
}
Expand Down
28 changes: 0 additions & 28 deletions dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export const NoConsent = (): JSX.Element => {
return (
<div style={containerStyle}>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -83,7 +82,6 @@ export const NoOverlay = (): JSX.Element => {
return (
<div style={containerStyle}>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -118,7 +116,6 @@ export const WithOverrideImage = (): JSX.Element => {
<div style={containerStyle}>
<OverlayAutoplayExplainer />
<YoutubeAtom
elementId="xyz"
videoId="3jpXAMwRSu4"
alt="Microscopic image of COVID"
eventEmitters={[
Expand Down Expand Up @@ -150,7 +147,6 @@ export const WithPosterImage = (): JSX.Element => {
<div style={containerStyle}>
<OverlayAutoplayExplainer />
<YoutubeAtom
elementId="xyz"
videoId="N9Cgy-ke5-s"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -185,7 +181,6 @@ export const WithOverlayAndPosterImage = (): JSX.Element => {
<div style={containerStyle}>
<OverlayAutoplayExplainer />
<YoutubeAtom
elementId="xyz"
videoId="N9Cgy-ke5-s"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -224,7 +219,6 @@ export const GiveConsent = (): JSX.Element => {
<button onClick={() => setConsented(true)}>Give consent</button>
<div style={containerStyle}>
<YoutubeAtom
elementId="xyz"
videoId="3jpXAMwRSu4"
alt="Microscopic image of COVID"
eventEmitters={[
Expand Down Expand Up @@ -258,7 +252,6 @@ export const Sticky = (): JSX.Element => {
<div>Scroll down...</div>
<div style={{ height: '1000px' }}></div>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -292,7 +285,6 @@ export const StickyMainMedia = (): JSX.Element => {
<div>Scroll down...</div>
<div style={{ height: '1000px' }}></div>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -328,7 +320,6 @@ export const DuplicateVideos = (): JSX.Element => {
return (
<div style={containerStyleSmall}>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand All @@ -350,7 +341,6 @@ export const DuplicateVideos = (): JSX.Element => {
/>
<br />
<YoutubeAtom
elementId="xyz2"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -388,7 +378,6 @@ export const MultipleStickyVideos = (): JSX.Element => {
return (
<div style={{ width: '500px', height: '5000px' }}>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand All @@ -411,7 +400,6 @@ export const MultipleStickyVideos = (): JSX.Element => {
adTargeting={adTargeting}
/>
<YoutubeAtom
elementId="xyz-2"
videoId="pcMiS6PW8aQ"
alt=""
eventEmitters={[
Expand All @@ -434,7 +422,6 @@ export const MultipleStickyVideos = (): JSX.Element => {
adTargeting={adTargeting}
/>
<YoutubeAtom
elementId="xyu"
videoId="3jpXAMwRSu4"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -469,7 +456,6 @@ export const PausesOffscreen = (): JSX.Element => {
<div>
<div>Scroll down...</div>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -506,7 +492,6 @@ export const NoConsentWithIma = (): JSX.Element => {
return (
<div style={containerStyle}>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -534,7 +519,6 @@ export const AdFreeWithIma = (): JSX.Element => {
return (
<div style={containerStyle}>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -563,7 +547,6 @@ export const NoOverlayWithIma = (): JSX.Element => {
return (
<div style={containerStyle}>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -593,7 +576,6 @@ export const WithOverrideImageWithIma = (): JSX.Element => {
return (
<div style={containerStyle}>
<YoutubeAtom
elementId="xyz"
videoId="3jpXAMwRSu4"
alt="Microscopic image of COVID"
eventEmitters={[
Expand Down Expand Up @@ -622,7 +604,6 @@ export const WithPosterImageWithIma = (): JSX.Element => {
return (
<div style={containerStyle}>
<YoutubeAtom
elementId="xyz"
videoId="N9Cgy-ke5-s"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -653,7 +634,6 @@ export const WithOverlayAndPosterImageWithIma = (): JSX.Element => {
return (
<div style={containerStyle}>
<YoutubeAtom
elementId="xyz"
videoId="N9Cgy-ke5-s"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -688,7 +668,6 @@ export const GiveConsentWithIma = (): JSX.Element => {
<button onClick={() => setConsented(true)}>Give consent</button>
<div style={containerStyle}>
<YoutubeAtom
elementId="xyz"
videoId="3jpXAMwRSu4"
alt="Microscopic image of COVID"
eventEmitters={[
Expand Down Expand Up @@ -722,7 +701,6 @@ export const StickyWithIma = (): JSX.Element => {
<div style={{ fontSize: '36px' }}>⬇️</div>
<div style={{ height: '1000px' }}></div>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -755,7 +733,6 @@ export const StickyMainMediaWithIma = (): JSX.Element => {
<div style={{ fontSize: '36px' }}>⬇️</div>
<div style={{ height: '1000px' }}></div>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -786,7 +763,6 @@ export const DuplicateVideosWithIma = (): JSX.Element => {
return (
<div style={containerStyleSmall}>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand All @@ -808,7 +784,6 @@ export const DuplicateVideosWithIma = (): JSX.Element => {
/>
<br />
<YoutubeAtom
elementId="xyz2"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand Down Expand Up @@ -840,7 +815,6 @@ export const MultipleStickyVideosWithIma = (): JSX.Element => {
return (
<div style={{ width: '500px', height: '5000px' }}>
<YoutubeAtom
elementId="xyz"
videoId="-ZCvZmYlQD8"
alt=""
eventEmitters={[
Expand All @@ -863,7 +837,6 @@ export const MultipleStickyVideosWithIma = (): JSX.Element => {
abTestParticipations={{}}
/>
<YoutubeAtom
elementId="xyz-2"
videoId="pcMiS6PW8aQ"
alt=""
eventEmitters={[
Expand All @@ -886,7 +859,6 @@ export const MultipleStickyVideosWithIma = (): JSX.Element => {
abTestParticipations={{}}
/>
<YoutubeAtom
elementId="xyu"
videoId="3jpXAMwRSu4"
alt=""
eventEmitters={[
Expand Down
9 changes: 0 additions & 9 deletions dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ describe('YoutubeAtom', () => {
value={{ renderingTarget: 'Web', darkModeAvailable: false }}
>
<YoutubeAtom
elementId="123"
title="My Youtube video!"
videoId="ZCvZmYlQD8"
alt=""
Expand Down Expand Up @@ -58,7 +57,6 @@ describe('YoutubeAtom', () => {
value={{ renderingTarget: 'Web', darkModeAvailable: false }}
>
<YoutubeAtom
elementId="123"
title="My Youtube video!"
videoId="ZCvZmYlQD8"
alt=""
Expand Down Expand Up @@ -97,7 +95,6 @@ describe('YoutubeAtom', () => {
value={{ renderingTarget: 'Web', darkModeAvailable: false }}
>
<YoutubeAtom
elementId="123"
title="My Youtube video!"
videoId="ZCvZmYlQD8"
alt=""
Expand Down Expand Up @@ -128,7 +125,6 @@ describe('YoutubeAtom', () => {
value={{ renderingTarget: 'Web', darkModeAvailable: false }}
>
<YoutubeAtom
elementId="123"
title="My Youtube video!"
videoId="ZCvZmYlQD8"
alt=""
Expand Down Expand Up @@ -161,7 +157,6 @@ describe('YoutubeAtom', () => {
value={{ renderingTarget: 'Web', darkModeAvailable: false }}
>
<YoutubeAtom
elementId="123"
title="My Youtube video!"
videoId="ZCvZmYlQD8"
alt=""
Expand Down Expand Up @@ -192,7 +187,6 @@ describe('YoutubeAtom', () => {
value={{ renderingTarget: 'Web', darkModeAvailable: false }}
>
<YoutubeAtom
elementId="123"
title="My Youtube video!"
videoId="ZCvZmYlQD8"
alt=""
Expand Down Expand Up @@ -222,7 +216,6 @@ describe('YoutubeAtom', () => {
value={{ renderingTarget: 'Web', darkModeAvailable: false }}
>
<YoutubeAtom
elementId="123"
title="My Youtube video!"
videoId="ZCvZmYlQD8"
alt=""
Expand Down Expand Up @@ -256,7 +249,6 @@ describe('YoutubeAtom', () => {
value={{ renderingTarget: 'Web', darkModeAvailable: false }}
>
<YoutubeAtom
elementId="123"
title="My Youtube video!"
videoId="ZCvZmYlQD8"
alt=""
Expand All @@ -274,7 +266,6 @@ describe('YoutubeAtom', () => {
abTestParticipations={{}}
/>
<YoutubeAtom
elementId="123"
title="My Youtube video 2!"
videoId="ZCvZmYlQD8"
alt=""
Expand Down
23 changes: 15 additions & 8 deletions dotcom-rendering/src/components/YoutubeAtom/YoutubeAtom.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Participations } from '@guardian/ab-core';
import type { ConsentState } from '@guardian/consent-management-platform/dist/types';
import type { ArticleFormat } from '@guardian/libs';
import { useCallback, useState } from 'react';
import { useCallback, useEffect, useState } from 'react';
import { MaintainAspectRatio } from '../MaintainAspectRatio';
import type { VideoCategory } from './YoutubeAtomOverlay';
import { YoutubeAtomOverlay } from './YoutubeAtomOverlay';
Expand All @@ -21,7 +21,6 @@ export type VideoEventKey =
| 'pause';

type Props = {
elementId: string;
videoId: string;
overrideImage?: string | undefined;
posterImage?: string | undefined;
Expand All @@ -45,8 +44,10 @@ type Props = {
showTextOverlay?: boolean;
};

/** always undefined on the server */
let index: number | undefined = undefined;

export const YoutubeAtom = ({
elementId,
videoId,
overrideImage,
posterImage,
Expand Down Expand Up @@ -74,8 +75,14 @@ export const YoutubeAtom = ({
const [isActive, setIsActive] = useState<boolean>(false);
const [isClosed, setIsClosed] = useState<boolean>(false);
const [pauseVideo, setPauseVideo] = useState<boolean>(false);
const [uniqueId, setUniqueId] = useState<string>();

useEffect(() => {
index ??= 0;
index++;
setUniqueId(`${videoId}-${index}`);
}, [videoId]);

const uniqueId = `${videoId}-${elementId}`;
const enableIma =
imaEnabled &&
!!adTargeting &&
Expand Down Expand Up @@ -152,7 +159,7 @@ export const YoutubeAtom = ({

return (
<YoutubeAtomSticky
uniqueId={uniqueId}
uniqueId={uniqueId ?? videoId}
videoId={videoId}
shouldStick={shouldStick}
isActive={isActive}
Expand All @@ -164,7 +171,7 @@ export const YoutubeAtom = ({
shouldPauseOutOfView={shouldPauseOutOfView}
>
<MaintainAspectRatio height={height} width={width}>
{loadPlayer && consentState && adTargeting && (
{loadPlayer && consentState && adTargeting && !!uniqueId && (
<YoutubeAtomPlayer
videoId={videoId}
uniqueId={uniqueId}
Expand All @@ -191,7 +198,7 @@ export const YoutubeAtom = ({
)}
{showOverlay && (
<YoutubeAtomOverlay
uniqueId={uniqueId}
uniqueId={uniqueId ?? videoId}
overrideImage={overrideImage}
posterImage={posterImage}
height={height}
Expand All @@ -207,7 +214,7 @@ export const YoutubeAtom = ({
/>
)}
{showPlaceholder && (
<YoutubeAtomPlaceholder uniqueId={uniqueId} />
<YoutubeAtomPlaceholder uniqueId={uniqueId ?? videoId} />
)}
</MaintainAspectRatio>
</YoutubeAtomSticky>
Expand Down
Loading

0 comments on commit cfbd8a9

Please sign in to comment.