WIP: Refactor button styling

libs/@guardian/source/src/react-components/button/Button.tsx

@@ -49,6 +49,7 @@ export const Button = ({
 		{...props}
 	>
 		{buttonContents({
+			size,
 			hideLabel,
 			iconSvg,
 			isLoading,

libs/@guardian/source/src/react-components/button/LinkButton.tsx

@@ -4,7 +4,7 @@ import { buttonContents } from './shared';
 import { buttonStyles } from './styles';
 
 export interface LinkButtonProps
-	extends SharedButtonProps,
+	extends Omit<SharedButtonProps, 'isLoading' | 'loadingAnnouncement'>,
 		AnchorHTMLAttributes<HTMLAnchorElement> {}
 
 /**
@@ -42,6 +42,7 @@ export const LinkButton = ({
 		{...props}
 	>
 		{buttonContents({
+			size,
 			hideLabel,
 			iconSvg,
 			children,

libs/@guardian/source/src/react-components/button/shared.tsx

@@ -2,14 +2,33 @@ import { css } from '@emotion/react';
 import type { ReactElement, ReactNode } from 'react';
 import { cloneElement } from 'react';
 import { visuallyHidden } from '../../foundations';
+import type { IconProps, IconSize } from '../@types/Icons';
 import { Spinner } from '../spinner/Spinner';
+import type { Size } from './@types/SharedButtonProps';
+
+const iconSize: Record<Size, IconSize> = {
+	xsmall: 'xsmall',
+	small: 'small',
+	default: 'medium',
+};
+
+/**
+ * Spinners do not use the same icon sizes so we specify custom sizes in pixels
+ */
+const spinnerSize: Record<Size, number> = {
+	xsmall: 16,
+	small: 20,
+	default: 24,
+};
 
 export const buttonContents = ({
+	size = 'default',
 	hideLabel,
 	iconSvg,
 	isLoading,
 	children,
 }: {
+	size?: Size;
 	hideLabel?: boolean;
 	iconSvg?: ReactElement;
 	isLoading?: boolean;
@@ -22,23 +41,26 @@ export const buttonContents = ({
 			contents.push(<div key="space" className="src-button-space" />);
 		}
 		contents.push(
-			cloneElement(
-				<Spinner
-					theme={{
-						background: 'transparent',
-						color: 'currentColor',
-					}}
-				/>,
-				{
-					key: 'svg',
-				},
-			),
+			<Spinner
+				size={spinnerSize[size]}
+				theme={{
+					background: 'transparent',
+					color: 'currentColor',
+				}}
+				key="spinner"
+			/>,
 		);
 	} else if (iconSvg) {
 		if (!hideLabel) {
 			contents.push(<div key="space" className="src-button-space" />);
 		}
-		contents.push(cloneElement(iconSvg, { key: 'svg' }));
+		contents.push(
+			cloneElement(iconSvg as ReactElement<IconProps>, {
+				size: iconSize[size],
+				theme: { fill: 'currentColor' },
+				key: 'icon',
+			}),
+		);
 	}
 	if (hideLabel) {
 		return (

libs/@guardian/source/src/react-components/button/styles.ts

@@ -40,27 +40,13 @@ const button = css`
 	&:focus {
 		${focusHaloSpaced};
 	}
-`;
-
-// Width of the loading spinner in pixels for each button size.
-const loadingSpinnerSizes: Record<Size, number> = {
-	xsmall: 16,
-	small: 20,
-	default: 24,
-};
 
-const applyButtonStylesToLoadingSpinner = (size: Size) => {
-	return css`
-		svg {
-			/*
-		 * The loading spinner width has been specified as 24px in the design
-		 * which falls outside of the icon sizes in foundations, so we
-		 * override the width here.
-		 */
-			width: ${loadingSpinnerSizes[size]}px;
-		}
-	`;
-};
+	svg {
+		flex: 0 0 auto;
+		display: block;
+		position: relative;
+	}
+`;
 
 const primary = (button: ThemeButton): SerializedStyles => css`
 	background-color: ${button.backgroundPrimary};
@@ -119,6 +105,10 @@ const defaultSize = css`
 	padding: 0 ${space[5]}px;
 	border-radius: ${height.ctaMedium}px;
 	padding-bottom: 2px;
+
+	.src-button-space {
+		width: ${space[3]}px;
+	}
 `;
 
 const smallSize = css`
@@ -128,6 +118,10 @@ const smallSize = css`
 	padding: 0 ${space[4]}px;
 	border-radius: ${height.ctaSmall}px;
 	padding-bottom: 2px;
+
+	.src-button-space {
+		width: ${space[2]}px;
+	}
 `;
 
 const xsmallSize = css`
@@ -137,53 +131,16 @@ const xsmallSize = css`
 	padding: 0 ${space[3]}px;
 	border-radius: ${height.ctaXsmall}px;
 	padding-bottom: 1px;
-`;
 
-const iconDefault = css`
-	svg {
-		flex: 0 0 auto;
-		display: block;
-		fill: currentColor;
-		position: relative;
-		width: ${width.iconMedium}px;
-		height: auto;
-	}
-	.src-button-space {
-		width: ${space[3]}px;
-	}
-`;
-
-const iconSmall = css`
-	svg {
-		flex: 0 0 auto;
-		display: block;
-		fill: currentColor;
-		position: relative;
-		width: ${width.iconSmall}px;
-		height: auto;
-	}
-	.src-button-space {
-		width: ${space[2]}px;
-	}
-`;
-
-const iconXsmall = css`
-	svg {
-		flex: 0 0 auto;
-		display: block;
-		fill: currentColor;
-		position: relative;
-		width: ${width.iconXsmall}px;
-		height: auto;
-	}
 	.src-button-space {
 		width: ${space[1]}px;
 	}
 `;
 
-/* TODO: we add some negative margin to icons to account for
- the extra space encoded into the SVG. We should consider removing
- or significantly reducing this space
+/**
+ * TODO: we add some negative margin to icons to account for the extra space
+ * encoded into the SVG. We should consider removing or significantly reducing
+ * this space.
  */
 const pullIconTowardEdge = -space[1];
 
@@ -193,28 +150,25 @@ const iconLeft = css`
 		margin-left: ${pullIconTowardEdge}px;
 	}
 `;
+
 const iconRight = css`
 	svg {
 		margin-right: ${pullIconTowardEdge}px;
 	}
 `;
 
-const iconOnly = css`
-	padding: 0;
-`;
-
 const iconOnlyDefault = css`
-	${iconOnly};
+	padding: 0;
 	width: ${width.ctaMedium}px;
 `;
 
 const iconOnlySmall = css`
-	${iconOnly};
+	padding: 0;
 	width: ${width.ctaSmall}px;
 `;
 
 const iconOnlyXsmall = css`
-	${iconOnly};
+	padding: 0;
 	width: ${width.ctaXsmall}px;
 `;
 
@@ -254,13 +208,7 @@ const sizes: {
 	small: smallSize,
 	xsmall: xsmallSize,
 };
-const iconSizes: {
-	[key in Size]: SerializedStyles;
-} = {
-	default: iconDefault,
-	small: iconSmall,
-	xsmall: iconXsmall,
-};
+
 const iconOnlySizes: {
 	[key in Size]: SerializedStyles;
 } = {
@@ -292,10 +240,8 @@ export const buttonStyles =
 		button,
 		sizes[size],
 		priorities[priority](mergedTheme(providerTheme.button, theme)),
-		iconSvg ?? isLoading ? iconSizes[size] : '',
 		(iconSvg ?? isLoading) && !hideLabel ? iconSides[iconSide] : '',
 		nudgeIcon ? iconNudgeAnimation : '',
 		hideLabel ? iconOnlySizes[size] : '',
-		isLoading ? applyButtonStylesToLoadingSpinner(size) : undefined,
 		cssOverrides,
 	];