Remove decidePalette from CommentCount (#9841)

* Migrate fillCommentCount to the theme palette * Migrate fillCommentCountUntilDesktop to themePalette
guardian · Dec 12, 2023 · 0132c33 · 0132c33
1 parent 9826061
commit 0132c33
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 66 deletions.
diff --git a/dotcom-rendering/src/components/ArticleMeta.tsx b/dotcom-rendering/src/components/ArticleMeta.tsx
@@ -454,7 +454,6 @@ export const ArticleMeta = ({
 										<CommentCount
 											discussionApiUrl={discussionApiUrl}
 											shortUrlId={shortUrlId}
-											format={format}
 										/>
 									</Island>
 								)}

diff --git a/dotcom-rendering/src/components/CommentCount.importable.tsx b/dotcom-rendering/src/components/CommentCount.importable.tsx
@@ -1,29 +1,27 @@
 import { css } from '@emotion/react';
 import { isUndefined } from '@guardian/libs';
 import { between, textSans, until } from '@guardian/source-foundations';
-import { decidePalette } from '../lib/decidePalette';
 import { formatCount } from '../lib/formatCount';
 import { useCommentCount } from '../lib/useCommentCount';
+import { palette as themePalette } from '../palette';
 import CommentIcon from '../static/icons/comment.svg';
-import type { Palette } from '../types/palette';
 
 type Props = {
-	format: ArticleFormat;
 	discussionApiUrl: string;
 	shortUrlId: string;
 };
 
-const containerStyles = (palette: Palette) => css`
+const containerStyles = css`
 	display: flex;
 	align-self: flex-end;
 	flex-direction: column;
 	${textSans.medium()};
 	font-weight: bold;
-	color: ${palette.fill.commentCount};
+	color: ${themePalette('--comment-count-fill')};
 	padding-top: 5px;
 
 	${until.desktop} {
-		color: ${palette.fill.commentCountUntilDesktop};
+		color: ${themePalette('--comment-count-mobile-fill')};
 	}
 `;
 
@@ -38,10 +36,10 @@ const iconContainerStyles = css`
 	}
 `;
 
-const iconStyles = (palette: Palette) => css`
-	fill: ${palette.fill.commentCount};
+const iconStyles = css`
+	fill: ${themePalette('--comment-count-fill')};
 	${until.desktop} {
-		fill: ${palette.fill.commentCountUntilDesktop};
+		fill: ${themePalette('--comment-count-mobile-fill')};
 	}
 `;
 
@@ -83,22 +81,17 @@ const linkStyles = css`
  *
  * [`Count` on Chromatic](https://www.chromatic.com/component?appId=63e251470cfbe61776b0ef19&csfId=components-counts&buildNumber=2967)
  */
-export const CommentCount = ({
-	discussionApiUrl,
-	format,
-	shortUrlId,
-}: Props) => {
+export const CommentCount = ({ discussionApiUrl, shortUrlId }: Props) => {
 	const commentCount = useCommentCount(discussionApiUrl, shortUrlId);
 
 	// If the comment count is 0 we still want to display it
 	if (isUndefined(commentCount)) return null;
 
 	const { short, long } = formatCount(commentCount);
-	const palette = decidePalette(format);
 
 	return (
 		<data
-			css={containerStyles(palette)}
+			css={containerStyles}
 			data-testid="comment-counts"
 			value={`${long} comments on this article`}
 		>
@@ -108,7 +101,7 @@ export const CommentCount = ({
 				aria-label={`${short} Comments`}
 			>
 				<div css={iconContainerStyles}>
-					<CommentIcon css={iconStyles(palette)} />
+					<CommentIcon css={iconStyles} />
 				</div>
 				<div
 					data-testid="long-comment-count"

diff --git a/dotcom-rendering/src/components/Island.test.tsx b/dotcom-rendering/src/components/Island.test.tsx
@@ -135,17 +135,7 @@ describe('Island: server-side rendering', () => {
 
 	test('CommentCount', () => {
 		expect(() =>
-			renderToString(
-				<CommentCount
-					format={{
-						theme: Pillar.News,
-						design: ArticleDesign.Standard,
-						display: ArticleDisplay.Standard,
-					}}
-					discussionApiUrl=""
-					shortUrlId=""
-				/>,
-			),
+			renderToString(<CommentCount discussionApiUrl="" shortUrlId="" />),
 		).not.toThrow();
 	});
 

diff --git a/dotcom-rendering/src/lib/decidePalette.ts b/dotcom-rendering/src/lib/decidePalette.ts
@@ -523,39 +523,6 @@ const backgroundFilterButtonActive = (format: ArticleFormat): string => {
 	}
 };
 
-const fillCommentCount = (format: ArticleFormat): string => {
-	if (
-		format.design === ArticleDesign.LiveBlog ||
-		format.design === ArticleDesign.DeadBlog
-	)
-		return neutral[46];
-	if (format.theme === ArticleSpecial.Labs) return BLACK;
-	if (format.theme === ArticleSpecial.SpecialReport)
-		return specialReport[300];
-
-	if (format.theme === ArticleSpecial.SpecialReportAlt)
-		return palette.specialReportAlt[100];
-
-	if (format.design === ArticleDesign.Analysis) {
-		switch (format.theme) {
-			case Pillar.News:
-				return news[300];
-			default:
-				return pillarPalette[format.theme].main;
-		}
-	}
-	if (format.design === ArticleDesign.Picture) {
-		return palette.neutral[86];
-	}
-	return pillarPalette[format.theme].main;
-};
-
-const fillCommentCountUntilDesktop = (format: ArticleFormat): string => {
-	if (format.design === ArticleDesign.LiveBlog) return WHITE;
-
-	return fillCommentCount(format);
-};
-
 const fillGuardianLogo = (format: ArticleFormat): string => {
 	if (format.design === ArticleDesign.NewsletterSignup) return WHITE;
 
@@ -1090,8 +1057,6 @@ export const decidePalette = (
 				backgroundDynamoSublink(format),
 		},
 		fill: {
-			commentCount: fillCommentCount(format),
-			commentCountUntilDesktop: fillCommentCountUntilDesktop(format),
 			guardianLogo: fillGuardianLogo(format),
 		},
 		border: {

diff --git a/dotcom-rendering/src/palette.ts b/dotcom-rendering/src/palette.ts
@@ -3985,6 +3985,36 @@ const witnessTitleIcon: PaletteFunction = (format) => witnessTitleText(format);
 const witnessTitleAuthor: PaletteFunction = (format) =>
 	witnessTitleText(format);
 
+const commentCountFill: PaletteFunction = ({ design, theme }) => {
+	if (design === ArticleDesign.LiveBlog || design === ArticleDesign.DeadBlog)
+		return sourcePalette.neutral[46];
+	if (theme === ArticleSpecial.Labs) return sourcePalette.neutral[7];
+	if (theme === ArticleSpecial.SpecialReport)
+		return sourcePalette.specialReport[300];
+
+	if (theme === ArticleSpecial.SpecialReportAlt)
+		return sourcePalette.specialReportAlt[100];
+
+	if (design === ArticleDesign.Analysis) {
+		switch (theme) {
+			case Pillar.News:
+				return sourcePalette.news[300];
+			default:
+				pillarPalette(theme, 400);
+		}
+	}
+	if (design === ArticleDesign.Picture) {
+		return sourcePalette.neutral[86];
+	}
+	return pillarPalette(theme, 400);
+};
+
+const mobileCommentCountFill: PaletteFunction = (format) => {
+	if (format.design === ArticleDesign.LiveBlog)
+		return sourcePalette.neutral[100];
+	return commentCountFill(format);
+};
+
 // ----- Palette ----- //
 
 /**
@@ -4616,6 +4646,14 @@ const paletteColours = {
 		light: witnessTitleAuthor,
 		dark: witnessTitleAuthor,
 	},
+	'--comment-count-fill': {
+		light: commentCountFill,
+		dark: commentCountFill,
+	},
+	'--comment-count-mobile-fill': {
+		light: mobileCommentCountFill,
+		dark: mobileCommentCountFill,
+	},
 } satisfies PaletteColours;
 
 /**

diff --git a/dotcom-rendering/src/types/palette.ts b/dotcom-rendering/src/types/palette.ts
@@ -55,8 +55,6 @@ export type Palette = {
 		dynamoSublink: Colour;
 	};
 	fill: {
-		commentCount: Colour;
-		commentCountUntilDesktop: Colour;
 		guardianLogo: Colour;
 	};
 	border: {