Consistent checkbox and radio button spacing (#1603)

* Apply consistent default spacing, removing conditional styles * Add story to show long label on mobile * Update radio spacing to match checkboxes * Add story to show radio with long label on mobile * Add container styles to unlabelled radio buttons * Prevent double container for unlabelled radios * Add changeset
guardian · Aug 8, 2024 · 447e6b6 · 447e6b6
1 parent c26e0ad
commit 447e6b6
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 91 deletions.
diff --git a/.changeset/friendly-parrots-relax.md b/.changeset/friendly-parrots-relax.md
@@ -0,0 +1,6 @@
+---
+'@guardian/source': major
+---
+
+- Applies consistent spacing around checkboxes and radio buttons by removing conditional styles and using padding to ensure minimum height of 44px. The external padding is now consistent regardless of the presence of a label and / or supporting text, and removes any inconsistency when these elements are stacked vertically.
+- Checkboxes and radio buttons are now aligned with the first line of text when labels wrap on to multiple lines.
diff --git a/libs/@guardian/source/src/react-components/checkbox/Checkbox.stories.tsx b/libs/@guardian/source/src/react-components/checkbox/Checkbox.stories.tsx
@@ -1,6 +1,6 @@
 import type { Meta, StoryObj } from '@storybook/react';
 import { useState } from 'react';
-import { palette } from '../../foundations';
+import { breakpoints, palette } from '../../foundations';
 import { Checkbox } from './Checkbox';
 import { themeCheckboxBrand } from './theme';
 
@@ -115,6 +115,21 @@ export const UnlabelledDefaultTheme: Story = {
 	},
 };
 
+export const LongLabelMobileDefaultTheme: Story = {
+	...Template,
+	args: {
+		...DefaultDefaultTheme.args,
+		label:
+			'Circulation / Visitors / Audience (for News media, Magazine and Exhibition requests)',
+	},
+	parameters: {
+		viewport: { defaultViewport: 'mobile' },
+		chromatic: {
+			viewports: [breakpoints.mobile],
+		},
+	},
+};
+
 export const DefaultCustomTheme: Story = {
 	...Template,
 	args: {

diff --git a/libs/@guardian/source/src/react-components/checkbox/Checkbox.tsx b/libs/@guardian/source/src/react-components/checkbox/Checkbox.tsx
@@ -6,15 +6,11 @@ import { mergeThemes } from '../utils/themes';
 import {
 	checkbox,
 	checkboxContainer,
-	checkboxContainerWithSupportingText,
 	errorCheckbox,
 	label,
 	labelText,
-	labelTextWithSupportingText,
 	supportingText,
 	tick,
-	tickWithLabelText,
-	tickWithSupportingText,
 } from './styles';
 import { themeCheckbox as defaultTheme, transformProviderTheme } from './theme';
 import type { ThemeCheckbox } from './theme';
@@ -132,19 +128,12 @@ export const Checkbox = ({
 		);
 	};
 
-	const LabelText = ({
-		hasSupportingText,
-		children,
-	}: {
-		hasSupportingText?: boolean;
-		children: ReactNode;
-	}) => {
+	const LabelText = ({ children }: { children: ReactNode }) => {
 		return (
 			<div
-				css={(providerTheme: Theme) => [
-					labelText(mergedTheme(providerTheme.checkbox)),
-					hasSupportingText ? labelTextWithSupportingText : '',
-				]}
+				css={(providerTheme: Theme) =>
+					labelText(mergedTheme(providerTheme.checkbox))
+				}
 			>
 				{children}
 			</div>
@@ -153,10 +142,9 @@ export const Checkbox = ({
 
 	return (
 		<div
-			css={(providerTheme: Theme) => [
-				checkboxContainer(mergedTheme(providerTheme.checkbox), error),
-				supporting ? checkboxContainerWithSupportingText : '',
-			]}
+			css={(providerTheme: Theme) =>
+				checkboxContainer(mergedTheme(providerTheme.checkbox), error)
+			}
 		>
 			<input
 				id={checkboxId}
@@ -173,17 +161,15 @@ export const Checkbox = ({
 				{...props}
 			/>
 			<span
-				css={(providerTheme: Theme) => [
-					tick(mergedTheme(providerTheme.checkbox)),
-					(labelContent ?? supporting) ? tickWithLabelText : '',
-					supporting ? tickWithSupportingText : '',
-				]}
+				css={(providerTheme: Theme) =>
+					tick(mergedTheme(providerTheme.checkbox))
+				}
 			/>
 
 			<label htmlFor={checkboxId} css={label}>
 				{supporting ? (
 					<div>
-						<LabelText hasSupportingText={true}>{labelContent}</LabelText>
+						<LabelText>{labelContent}</LabelText>
 						<SupportingText>{supporting}</SupportingText>
 					</div>
 				) : (

diff --git a/libs/@guardian/source/src/react-components/checkbox/styles.ts b/libs/@guardian/source/src/react-components/checkbox/styles.ts
@@ -27,8 +27,12 @@ export const checkboxContainer = (
 ): SerializedStyles => css`
 	position: relative;
 	display: flex;
-	align-items: center;
-	min-height: ${height.inputMedium}px;
+	align-items: flex-start;
+	/**
+	 * Ensure minimum height of 44px by applying 10px padding to top and bottom
+	 * of container. This ensures consistent spacing when supporting text present.
+	 */
+	padding: ${(height.inputMedium - height.inputXsmall) / 2}px 0;
 	cursor: pointer;
 
 	&:hover {
@@ -61,11 +65,6 @@ export const label: SerializedStyles = css`
 	cursor: pointer;
 `;
 
-export const checkboxContainerWithSupportingText = css`
-	align-items: flex-start;
-	margin-bottom: ${space[3]}px;
-`;
-
 export const checkbox = (
 	checkbox: ThemeCheckbox,
 	error = false,
@@ -122,10 +121,6 @@ export const labelText = (checkbox: ThemeCheckbox): SerializedStyles => css`
 	${textSans17};
 	color: ${checkbox.textLabel};
 	width: 100%;
-`;
-
-export const labelTextWithSupportingText = css`
-	${textSans17};
 	margin-top: 1px;
 	/* If label text is empty, add additional spacing to align supporting text */
 	&:empty {
@@ -149,16 +144,12 @@ export const tick = (checkbox: ThemeCheckbox): SerializedStyles => css`
 		width: 6px;
 		height: 12px;
 		transform: rotate(45deg);
-		/*
-		these properties are very sensitive and are overridden
-		if the checkbox has a label or supporting text
-	*/
-		top: 14px;
+		top: 15px;
 		left: 9px;
-		/*
-			this prevents simulated click events to the checkbox, eg from Selenium tests
-			from being intercepted by the tick
-		*/
+		/**
+		 * This prevents simulated click events to the checkbox (eg. from Selenium
+		 * tests) being intercepted by the tick
+		 */
 		pointer-events: none;
 
 		/* the checkmark ✓ */
@@ -191,19 +182,6 @@ export const tick = (checkbox: ThemeCheckbox): SerializedStyles => css`
 	}
 `;
 
-export const tickWithLabelText = css`
-	@supports (${appearance}) {
-		top: 15px;
-		left: 9px;
-	}
-`;
-
-export const tickWithSupportingText = css`
-	@supports (${appearance}) {
-		top: 5px;
-	}
-`;
-
 export const errorCheckbox = (checkbox: ThemeCheckbox): SerializedStyles => css`
 	border: 2px solid ${checkbox.borderError};
 	border-radius: 4px;

diff --git a/libs/@guardian/source/src/react-components/radio/Radio.stories.tsx b/libs/@guardian/source/src/react-components/radio/Radio.stories.tsx
@@ -1,5 +1,5 @@
 import type { Meta, StoryObj } from '@storybook/react';
-import { palette } from '../../foundations';
+import { breakpoints, palette } from '../../foundations';
 import { Radio } from './Radio';
 import { themeRadioBrand } from './theme';
 
@@ -88,6 +88,20 @@ export const UnlabelledDefaultTheme: Story = {
 	},
 };
 
+export const LongLabelMobileDefaultTheme: Story = {
+	args: {
+		...DefaultDefaultTheme.args,
+		label:
+			'Circulation / Visitors / Audience (for News media, Magazine and Exhibition requests)',
+	},
+	parameters: {
+		viewport: { defaultViewport: 'mobile' },
+		chromatic: {
+			viewports: [breakpoints.mobile],
+		},
+	},
+};
+
 export const DefaultCustomTheme: Story = {
 	args: {
 		...DefaultDefaultTheme.args,

diff --git a/libs/@guardian/source/src/react-components/radio/Radio.tsx b/libs/@guardian/source/src/react-components/radio/Radio.tsx
@@ -6,8 +6,6 @@ import { mergeThemes } from '../utils/themes';
 import {
 	label,
 	labelText,
-	labelTextWithSupportingText,
-	labelWithSupportingText,
 	radio,
 	radioContainer,
 	supportingText,
@@ -100,19 +98,10 @@ export const Radio = ({
 			transformProviderTheme,
 		);
 
-	const LabelText = ({
-		hasSupportingText,
-		children,
-	}: {
-		hasSupportingText?: boolean;
-		children: ReactNode;
-	}) => {
+	const LabelText = ({ children }: { children: ReactNode }) => {
 		return (
 			<div
-				css={(providerTheme: Theme) => [
-					hasSupportingText ? labelTextWithSupportingText : '',
-					labelText(mergedTheme(providerTheme)),
-				]}
+				css={(providerTheme: Theme) => labelText(mergedTheme(providerTheme))}
 			>
 				{children}
 			</div>
@@ -148,16 +137,13 @@ export const Radio = ({
 
 	const labelledRadioControl = (
 		<div
-			css={(providerTheme: Theme) => [
-				radioContainer(mergedTheme(providerTheme)),
-				supporting ? labelWithSupportingText : '',
-			]}
+			css={(providerTheme: Theme) => radioContainer(mergedTheme(providerTheme))}
 		>
 			{radioControl}
 			<label htmlFor={radioId} css={label}>
 				{supporting ? (
 					<div>
-						<LabelText hasSupportingText={true}>{labelContent}</LabelText>
+						<LabelText>{labelContent}</LabelText>
 						<SupportingText>{supporting}</SupportingText>
 					</div>
 				) : (
@@ -167,7 +153,19 @@ export const Radio = ({
 		</div>
 	);
 
+	const unlabelledRadioControl = (
+		<div
+			css={(providerTheme: Theme) => radioContainer(mergedTheme(providerTheme))}
+		>
+			{radioControl}
+		</div>
+	);
+
 	return (
-		<>{(labelContent ?? supporting) ? labelledRadioControl : radioControl}</>
+		<>
+			{(labelContent ?? supporting)
+				? labelledRadioControl
+				: unlabelledRadioControl}
+		</>
 	);
 };
diff --git a/libs/@guardian/source/src/react-components/radio/styles.ts b/libs/@guardian/source/src/react-components/radio/styles.ts
@@ -8,7 +8,6 @@ import {
 	space,
 	textSans15,
 	textSans17,
-	textSansBold17,
 	transitions,
 	width,
 } from '../../foundations';
@@ -30,8 +29,12 @@ export const fieldset = (radio: ThemeRadioGroup): SerializedStyles => css`
 export const radioContainer = (radio: ThemeRadio): SerializedStyles => css`
 	position: relative;
 	display: flex;
-	align-items: center;
-	min-height: ${height.inputMedium}px;
+	align-items: flex-start;
+	/**
+	 * Ensure minimum height of 44px by applying 10px padding to top and bottom
+	 * of container. This ensures consistent spacing when supporting text present.
+	 */
+	padding: ${(height.inputMedium - height.inputXsmall) / 2}px 0;
 	cursor: pointer;
 
 	&:hover {
@@ -45,11 +48,6 @@ export const label: SerializedStyles = css`
 	cursor: pointer;
 `;
 
-export const labelWithSupportingText = css`
-	align-items: flex-start;
-	margin-bottom: ${space[3]}px;
-`;
-
 export const radio = (radio: ThemeRadio): SerializedStyles => css`
 	flex: 0 0 auto;
 	cursor: pointer;
@@ -112,10 +110,6 @@ export const labelText = (radio: ThemeRadio): SerializedStyles => css`
 	${textSans17};
 	color: ${radio.textLabel};
 	width: 100%;
-`;
-
-export const labelTextWithSupportingText = css`
-	${textSansBold17};
 	margin-top: 1px;
 	/* If label text is empty, add additional spacing to align supporting text */
 	&:empty {