Font Library modal: Try to improve checkbox labelling

packages/components/CHANGELOG.md

@@ -13,6 +13,10 @@
 -   `DropdownMenuV2`: Restore accent color themability ([#58130](https://github.com/WordPress/gutenberg/pull/58130)).
 -   Expand theming support in the `COLORS` variable object ([#58097](https://github.com/WordPress/gutenberg/pull/58097)).
 
+### Enhancements
+
+-   `CheckboxControl`: Remove ability for label prop to be false ([#58339](https://github.com/WordPress/gutenberg/pull/58339)).
+
 ## 25.16.0 (2024-01-24)
 
 ### Enhancements

packages/components/src/checkbox-control/README.md

@@ -69,12 +69,11 @@ const MyCheckboxControl = () => {
 The set of props accepted by the component will be specified below.
 Props not included in this set will be applied to the input element.
 
-#### `label`: `string|false`
+#### `label`: `string`
 
 A label for the input field, that appears at the side of the checkbox.
 The prop will be rendered as content a label element.
 If no prop is passed an empty label is rendered.
-If the prop is set to false no label is rendered.
 
 -   Required: No
 

packages/components/src/checkbox-control/index.tsx

@@ -125,14 +125,12 @@ export function CheckboxControl(
 					/>
 				) : null }
 			</span>
-			{ label && (
-				<label
-					className="components-checkbox-control__label"
-					htmlFor={ id }
-				>
-					{ label }
-				</label>
-			) }
+			<label
+				className="components-checkbox-control__label"
+				htmlFor={ id }
+			>
+				{ label }
+			</label>
 		</BaseControl>
 	);
 }

packages/components/src/checkbox-control/test/__snapshots__/index.tsx.snap

@@ -5,14 +5,14 @@ Snapshot Diff:
 - First value
 + Second value
 
-@@ -8,13 +8,27 @@
+@@ -8,17 +8,31 @@
         <span
           class="components-checkbox-control__input-container"
         >
           <input
             class="components-checkbox-control__input"
--           id="inspector-checkbox-control-6"
-+           id="inspector-checkbox-control-7"
+-           id="inspector-checkbox-control-5"
++           id="inspector-checkbox-control-6"
             type="checkbox"
             value="1"
 +         />
@@ -31,6 +31,11 @@ Snapshot Diff:
             />
 +         </svg>
         </span>
+        <label
+          class="components-checkbox-control__label"
+-         for="inspector-checkbox-control-5"
++         for="inspector-checkbox-control-6"
+        />
       </div>
     </div>
   </div>

packages/components/src/checkbox-control/test/index.tsx

@@ -60,13 +60,6 @@ describe( 'CheckboxControl', () => {
 			expect( label ).toBeInTheDocument();
 		} );
 
-		it( 'should not render label element if label is set to false', () => {
-			render( <CheckboxControl label={ false } /> );
-
-			const label = screen.queryByRole( 'label' );
-			expect( label ).not.toBeInTheDocument();
-		} );
-
 		it( 'should render a checkbox in an indeterminate state', () => {
 			render( <CheckboxControl indeterminate /> );
 			expect( getInput() ).toHaveProperty( 'indeterminate', true );

packages/components/src/checkbox-control/types.ts

@@ -19,10 +19,9 @@ export type CheckboxControlProps = Pick<
 	/**
 	 * A label for the input field, that appears at the side of the checkbox.
 	 * The prop will be rendered as content a label element. If no prop is
-	 * passed an empty label is rendered. If the prop is set to false no label
-	 * is rendered.
+	 * passed an empty label is rendered.
 	 */
-	label?: string | false;
+	label?: string;
 	/**
 	 * If checked is true the checkbox will be checked. If checked is false the
 	 * checkbox will be unchecked. If no value is passed the checkbox will be

packages/edit-site/src/components/global-styles/font-library-modal/collection-font-variant.js

@@ -1,18 +1,13 @@
 /**
  * WordPress dependencies
  */
-import {
-	CheckboxControl,
-	Flex,
-	privateApis as componentsPrivateApis,
-} from '@wordpress/components';
+import { CheckboxControl, Flex } from '@wordpress/components';
 
 /**
  * Internal dependencies
  */
 import { getFontFaceVariantName } from './utils';
 import FontFaceDemo from './font-demo';
-import { unlock } from '../../../lock-unlock';
 
 function CollectionFontVariant( {
 	face,
@@ -29,27 +24,23 @@ function CollectionFontVariant( {
 	};
 
 	const displayName = font.name + ' ' + getFontFaceVariantName( face );
-	const { kebabCase } = unlock( componentsPrivateApis );
-	const checkboxId = kebabCase(
-		`${ font.slug }-${ getFontFaceVariantName( face ) }`
-	);
 
 	return (
-		<label
-			className="font-library-modal__library-font-variant"
-			htmlFor={ checkboxId }
-		>
+		<div className="font-library-modal__library-font-variant">
 			<Flex justify="flex-start" align="center" gap="1rem">
 				<CheckboxControl
 					checked={ selected }
 					onChange={ handleToggleActivation }
 					__nextHasNoMarginBottom={ true }
-					id={ checkboxId }
-					label={ false }
+					label={ displayName }
+				/>
+				<FontFaceDemo
+					fontFace={ face }
+					text={ displayName }
+					onClick={ handleToggleActivation }
 				/>
-				<FontFaceDemo fontFace={ face } text={ displayName } />
 			</Flex>
-		</label>
+		</div>
 	);
 }
 

packages/edit-site/src/components/global-styles/font-library-modal/font-demo.js

@@ -19,7 +19,13 @@ function getPreviewUrl( fontFace ) {
 	}
 }
 
-function FontFaceDemo( { customPreviewUrl, fontFace, text, style = {} } ) {
+function FontFaceDemo( {
+	customPreviewUrl,
+	fontFace,
+	text,
+	onClick,
+	style = {},
+} ) {
 	const ref = useRef( null );
 	const [ isIntersecting, setIsIntersecting ] = useState( false );
 	const [ isAssetLoaded, setIsAssetLoaded ] = useState( false );
@@ -43,6 +49,9 @@ function FontFaceDemo( { customPreviewUrl, fontFace, text, style = {} } ) {
 		height: '23px',
 		width: 'auto',
 	};
+	const containerStyle = {
+		cursor: 'pointer',
+	};
 
 	useEffect( () => {
 		const observer = new window.IntersectionObserver( ( [ entry ] ) => {
@@ -65,7 +74,14 @@ function FontFaceDemo( { customPreviewUrl, fontFace, text, style = {} } ) {
 	}, [ fontFace, isIntersecting, loadFontFaceAsset, isPreviewImage ] );
 
 	return (
-		<div ref={ ref }>
+		<div
+			ref={ ref }
+			onClick={ onClick }
+			onKeyDown={ onClick }
+			tabIndex={ -1 }
+			role="button"
+			style={ containerStyle }
+		>
 			{ isPreviewImage ? (
 				<img
 					src={ previewUrl }

packages/edit-site/src/components/global-styles/font-library-modal/library-font-variant.js

@@ -2,19 +2,14 @@
  * WordPress dependencies
  */
 import { useContext } from '@wordpress/element';
-import {
-	CheckboxControl,
-	Flex,
-	privateApis as componentsPrivateApis,
-} from '@wordpress/components';
+import { CheckboxControl, Flex } from '@wordpress/components';
 
 /**
  * Internal dependencies
  */
 import { getFontFaceVariantName } from './utils';
 import { FontLibraryContext } from './context';
 import FontFaceDemo from './font-demo';
-import { unlock } from '../../../lock-unlock';
 
 function LibraryFontVariant( { face, font } ) {
 	const { isFontActivated, toggleActivateFont } =
@@ -39,27 +34,23 @@ function LibraryFontVariant( { face, font } ) {
 	};
 
 	const displayName = font.name + ' ' + getFontFaceVariantName( face );
-	const { kebabCase } = unlock( componentsPrivateApis );
-	const checkboxId = kebabCase(
-		`${ font.slug }-${ getFontFaceVariantName( face ) }`
-	);
 
 	return (
-		<label
-			className="font-library-modal__library-font-variant"
-			htmlFor={ checkboxId }
-		>
+		<div className="font-library-modal__library-font-variant">
 			<Flex justify="flex-start" align="center" gap="1rem">
 				<CheckboxControl
 					checked={ isInstalled }
 					onChange={ handleToggleActivation }
 					__nextHasNoMarginBottom={ true }
-					id={ checkboxId }
-					label={ false }
+					label={ displayName }
+				/>
+				<FontFaceDemo
+					fontFace={ face }
+					text={ displayName }
+					onClick={ handleToggleActivation }
 				/>
-				<FontFaceDemo fontFace={ face } text={ displayName } />
 			</Flex>
-		</label>
+		</div>
 	);
 }
 

packages/edit-site/src/components/global-styles/font-library-modal/style.scss

@@ -63,6 +63,15 @@
 	border: 1px solid $gray-200;
 	padding: $grid-unit-20;
 	margin-top: -1px; /* To collapse the margin with the previous element */
+
+	// Hide the checkbox label while still allowing screen-readers to read it.
+	// Visual label is part of the `FontFaceDemo` component.
+	label {
+		display: inline-block;
+		width: 0;
+		height: 0;
+		overflow: hidden;
+	}
 }
 
 .font-library-modal__font-variant {

test/e2e/specs/site-editor/font-library.spec.js

@@ -77,7 +77,10 @@ test.describe( 'Font Library', () => {
 					name: /manage fonts/i,
 				} )
 				.click();
-			await page.getByRole( 'button', { name: /system font/i } ).click();
+			await page
+				.getByRole( 'button', { name: /system font/i } )
+				.first()
+				.click();
 			await expect(
 				page.getByRole( 'heading', { name: /system font/i } )
 			).toBeVisible();