From c615d83acdc9ef34be4024cdaec620ef29ca0f17 Mon Sep 17 00:00:00 2001
From: Max Duval <max.duval@theguardian.com>
Date: Tue, 27 Sep 2022 14:00:02 +0100
Subject: [PATCH 1/3] refactor(Card): extract image

Settle on a single image type
to avoid parallel branches that
are harder to reason about.

Allow optional parameters not to be set
---
 .../src/web/components/Card/Card.tsx          | 83 ++++++++++---------
 .../components/Card/components/CardLayout.tsx |  4 +-
 2 files changed, 46 insertions(+), 41 deletions(-)

diff --git a/dotcom-rendering/src/web/components/Card/Card.tsx b/dotcom-rendering/src/web/components/Card/Card.tsx
index 8e9b74a4582..4abcbed4158 100644
--- a/dotcom-rendering/src/web/components/Card/Card.tsx
+++ b/dotcom-rendering/src/web/components/Card/Card.tsx
@@ -166,6 +166,18 @@ const CommentFooter = ({
 	);
 };
 
+const getImage = ({
+	imageUrl,
+	avatarUrl,
+}: {
+	imageUrl?: string;
+	avatarUrl?: string;
+}): { type: CardImageType; src: string } | undefined => {
+	if (avatarUrl) return { type: 'avatar', src: avatarUrl };
+	if (imageUrl) return { type: 'mainMedia', src: imageUrl };
+	return undefined;
+};
+
 export const Card = ({
 	linkTo,
 	format,
@@ -280,13 +292,10 @@ export const Card = ({
 		return <Snap snapData={snapData} />;
 	}
 
-	// Decide what type of image to show, main media, avatar or none
-	let imageType: CardImageType | undefined;
-	if (imageUrl && avatarUrl) {
-		imageType = 'avatar';
-	} else if (imageUrl) {
-		imageType = 'mainMedia';
-	}
+	const image = getImage({
+		imageUrl,
+		avatarUrl,
+	});
 
 	return (
 		<CardWrapper
@@ -301,41 +310,37 @@ export const Card = ({
 				dataLinkName={dataLinkName}
 			/>
 			<CardLayout
-				imagePosition={imageUrl !== undefined ? imagePosition : 'top'}
-				imagePositionOnMobile={
-					imageUrl !== undefined ? imagePositionOnMobile : 'top'
-				}
+				imagePosition={image ? imagePosition : 'top'}
+				imagePositionOnMobile={image ? imagePositionOnMobile : 'top'}
 				minWidthInPixels={minWidthInPixels}
-				imageType={imageType}
+				imageType={image?.type}
 			>
-				<ImageWrapper
-					imageSize={imageSize}
-					imageType={imageType}
-					imagePosition={
-						imageUrl !== undefined ? imagePosition : 'top'
-					}
-					imagePositionOnMobile={
-						imageUrl !== undefined ? imagePositionOnMobile : 'top'
-					}
-				>
-					{imageType === 'avatar' && !!avatarUrl ? (
-						<AvatarContainer
-							imageSize={imageSize}
-							imagePosition={imagePosition}
-						>
-							<Avatar
-								imageSrc={avatarUrl}
-								imageAlt={byline ?? ''}
-								containerPalette={containerPalette}
-								format={format}
-							/>
-						</AvatarContainer>
-					) : (
-						<img src={imageUrl} alt="" role="presentation" />
-					)}
-				</ImageWrapper>
+				{image && (
+					<ImageWrapper
+						imageSize={imageSize}
+						imageType={image.type}
+						imagePosition={imagePosition}
+						imagePositionOnMobile={imagePositionOnMobile}
+					>
+						{image.type === 'avatar' ? (
+							<AvatarContainer
+								imageSize={imageSize}
+								imagePosition={imagePosition}
+							>
+								<Avatar
+									imageSrc={image.src}
+									imageAlt={byline ?? ''}
+									containerPalette={containerPalette}
+									format={format}
+								/>
+							</AvatarContainer>
+						) : (
+							<img src={image.src} alt="" role="presentation" />
+						)}
+					</ImageWrapper>
+				)}
 				<ContentWrapper
-					imageType={imageType}
+					imageType={image?.type}
 					imageSize={imageSize}
 					imagePosition={imagePosition}
 				>
diff --git a/dotcom-rendering/src/web/components/Card/components/CardLayout.tsx b/dotcom-rendering/src/web/components/Card/components/CardLayout.tsx
index a458d3066ff..b52d5f38650 100644
--- a/dotcom-rendering/src/web/components/Card/components/CardLayout.tsx
+++ b/dotcom-rendering/src/web/components/Card/components/CardLayout.tsx
@@ -37,8 +37,8 @@ const decideWidth = (minWidthInPixels?: number) => {
 };
 
 const decidePosition = (
-	imagePosition: ImagePositionType,
-	imagePositionOnMobile: ImagePositionType,
+	imagePosition?: ImagePositionType,
+	imagePositionOnMobile?: ImagePositionType,
 	imageType?: CardImageType,
 ) => {
 	if (imageType === 'avatar') {

From b27392f7ff6593bfc479bea0b1c7dfdece1d1d4d Mon Sep 17 00:00:00 2001
From: Max Duval <max.duval@theguardian.com>
Date: Tue, 27 Sep 2022 21:37:21 +0100
Subject: [PATCH 2/3] Update dotcom-rendering/src/web/components/Card/Card.tsx

---
 dotcom-rendering/src/web/components/Card/Card.tsx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dotcom-rendering/src/web/components/Card/Card.tsx b/dotcom-rendering/src/web/components/Card/Card.tsx
index 4abcbed4158..1d92edc3705 100644
--- a/dotcom-rendering/src/web/components/Card/Card.tsx
+++ b/dotcom-rendering/src/web/components/Card/Card.tsx
@@ -310,8 +310,8 @@ export const Card = ({
 				dataLinkName={dataLinkName}
 			/>
 			<CardLayout
-				imagePosition={image ? imagePosition : 'top'}
-				imagePositionOnMobile={image ? imagePositionOnMobile : 'top'}
+				imagePosition={imagePosition}
+				imagePositionOnMobile={imagePositionOnMobile}
 				minWidthInPixels={minWidthInPixels}
 				imageType={image?.type}
 			>

From 6f7655db4b8f72e8a6c60790f852c3ddaeb8463d Mon Sep 17 00:00:00 2001
From: Ioanna Kokkini <ioannakok@hotmail.com>
Date: Thu, 29 Sep 2022 17:46:56 +0100
Subject: [PATCH 3/3] imagePosition and mobileImagePosition are non-optional

Co-authored-by: Max Duval <max.duval@guardian.co.uk>
---
 .../src/web/components/Card/components/CardLayout.tsx     | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/dotcom-rendering/src/web/components/Card/components/CardLayout.tsx b/dotcom-rendering/src/web/components/Card/components/CardLayout.tsx
index b52d5f38650..301636367ab 100644
--- a/dotcom-rendering/src/web/components/Card/components/CardLayout.tsx
+++ b/dotcom-rendering/src/web/components/Card/components/CardLayout.tsx
@@ -9,7 +9,7 @@ type Props = {
 	minWidthInPixels?: number;
 };
 
-const decideDirection = (imagePosition?: ImagePositionType) => {
+const decideDirection = (imagePosition: ImagePositionType) => {
 	switch (imagePosition) {
 		case 'top':
 			return 'column';
@@ -20,7 +20,7 @@ const decideDirection = (imagePosition?: ImagePositionType) => {
 		case 'right':
 			return 'row-reverse';
 		// If there's no image (so no imagePosition) default to top down
-		default:
+		case 'none':
 			return 'column';
 	}
 };
@@ -37,8 +37,8 @@ const decideWidth = (minWidthInPixels?: number) => {
 };
 
 const decidePosition = (
-	imagePosition?: ImagePositionType,
-	imagePositionOnMobile?: ImagePositionType,
+	imagePosition: ImagePositionType,
+	imagePositionOnMobile: ImagePositionType,
 	imageType?: CardImageType,
 ) => {
 	if (imageType === 'avatar') {