From 39f19f49dfabd2d13d96ca1f475273a24fa355ad Mon Sep 17 00:00:00 2001
From: Tetsuaki Hamano <tetsuaki.hamano@gmail.com>
Date: Sat, 24 Feb 2024 10:15:00 +0900
Subject: [PATCH] Font Library: Add headings to user font lists and group them

---
 .../font-library-modal/font-collection.js     |  40 ++++--
 .../font-library-modal/installed-fonts.js     | 135 +++++++++++-------
 .../font-library-modal/style.scss             |  19 ++-
 .../specs/site-editor/font-library.spec.js    |   2 +-
 4 files changed, 128 insertions(+), 68 deletions(-)

diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js b/packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js
index 01f7a90357c8b..2129caac040a5 100644
--- a/packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js
+++ b/packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js
@@ -337,18 +337,34 @@ function FontCollection( { slug } ) {
 						) }
 
 					<div className="font-library-modal__fonts-grid__main">
-						{ items.map( ( font ) => (
-							<FontCard
-								key={ font.font_family_settings.slug }
-								font={ font.font_family_settings }
-								navigatorPath={ '/fontFamily' }
-								onClick={ () => {
-									setSelectedFont(
-										font.font_family_settings
-									);
-								} }
-							/>
-						) ) }
+						{ /*
+						 * Disable reason: The `list` ARIA role is redundant but
+						 * Safari+VoiceOver won't announce the list otherwise.
+						 */
+						/* eslint-disable jsx-a11y/no-redundant-roles */ }
+						<ul
+							role="list"
+							className="font-library-modal__fonts-list"
+						>
+							{ items.map( ( font ) => (
+								<li
+									key={ font.slug }
+									className="font-library-modal__fonts-list-item"
+								>
+									<FontCard
+										key={ font.font_family_settings.slug }
+										font={ font.font_family_settings }
+										navigatorPath={ '/fontFamily' }
+										onClick={ () => {
+											setSelectedFont(
+												font.font_family_settings
+											);
+										} }
+									/>
+								</li>
+							) ) }
+						</ul>
+						{ /* eslint-enable jsx-a11y/no-redundant-roles */ }
 					</div>
 				</NavigatorScreen>
 
diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js b/packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js
index a72b7d59d150c..583c19c7aeb96 100644
--- a/packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js
+++ b/packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js
@@ -97,71 +97,98 @@ function InstalledFonts() {
 		<div className="font-library-modal__tabpanel-layout">
 			{ isResolvingLibrary && (
 				<HStack align="center">
-					<Spacer />
 					<Spinner />
-					<Spacer />
 				</HStack>
 			) }
-
 			<NavigatorProvider initialPath="/">
 				<NavigatorScreen path="/">
-					{ notice && (
-						<>
-							<Spacer margin={ 1 } />
+					<VStack spacing="8">
+						{ notice && (
 							<Notice
 								status={ notice.type }
 								onRemove={ () => setNotice( null ) }
 							>
 								{ notice.message }
 							</Notice>
-							<Spacer margin={ 1 } />
-						</>
-					) }
-					{ baseCustomFonts.length > 0 && (
-						<>
-							<Text className="font-library-modal__subtitle">
-								{ __( 'Installed Fonts' ) }
-							</Text>
-							<Spacer margin={ 2 } />
-							{ baseCustomFonts.map( ( font ) => (
-								<FontCard
-									font={ font }
-									key={ font.slug }
-									navigatorPath={ '/fontFamily' }
-									variantsText={ getFontCardVariantsText(
-										font
-									) }
-									onClick={ () => {
-										handleSetLibraryFontSelected( font );
-									} }
-								/>
-							) ) }
-							<Spacer margin={ 8 } />
-						</>
-					) }
-
-					{ baseThemeFonts.length > 0 && (
-						<>
-							<Text className="font-library-modal__subtitle">
-								{ __( 'Theme Fonts' ) }
-							</Text>
-							<Spacer margin={ 2 } />
-							{ baseThemeFonts.map( ( font ) => (
-								<FontCard
-									font={ font }
-									key={ font.slug }
-									navigatorPath={ '/fontFamily' }
-									variantsText={ getFontCardVariantsText(
-										font
-									) }
-									onClick={ () => {
-										handleSetLibraryFontSelected( font );
-									} }
-								/>
-							) ) }
-						</>
-					) }
-					<Spacer margin={ 16 } />
+						) }
+						{ baseCustomFonts.length > 0 && (
+							<VStack>
+								<h2 className="font-library-modal__fonts-title">
+									{ __( 'Installed Fonts' ) }
+								</h2>
+								{ /*
+								 * Disable reason: The `list` ARIA role is redundant but
+								 * Safari+VoiceOver won't announce the list otherwise.
+								 */
+								/* eslint-disable jsx-a11y/no-redundant-roles */ }
+								<ul
+									role="list"
+									className="font-library-modal__fonts-list"
+								>
+									{ baseCustomFonts.map( ( font ) => (
+										<li
+											key={ font.slug }
+											className="font-library-modal__fonts-list-item"
+										>
+											<FontCard
+												font={ font }
+												key={ font.slug }
+												navigatorPath={ '/fontFamily' }
+												variantsText={ getFontCardVariantsText(
+													font
+												) }
+												onClick={ () => {
+													handleSetLibraryFontSelected(
+														font
+													);
+												} }
+											/>
+										</li>
+									) ) }
+								</ul>
+								{ /* eslint-enable jsx-a11y/no-redundant-roles */ }
+							</VStack>
+						) }
+						{ baseThemeFonts.length > 0 && (
+							<VStack>
+								<h2 className="font-library-modal__fonts-title">
+									{ __( 'Theme Fonts' ) }
+								</h2>
+								{ /*
+								 * Disable reason: The `list` ARIA role is redundant but
+								 * Safari+VoiceOver won't announce the list otherwise.
+								 */
+								/* eslint-disable jsx-a11y/no-redundant-roles */ }
+								<ul
+									role="list"
+									className="font-library-modal__fonts-list"
+								>
+									{ baseThemeFonts.map( ( font ) => (
+										<li
+											key={ font.slug }
+											className="font-library-modal__fonts-list-item"
+										>
+											<FontCard
+												font={ font }
+												key={ font.slug }
+												navigatorPath={ '/fontFamily' }
+												variantsText={ getFontCardVariantsText(
+													font
+												) }
+												onClick={ () => {
+													handleSetLibraryFontSelected(
+														font
+													);
+												} }
+											/>
+										</li>
+									) ) }
+								</ul>
+								{ /* eslint-enable jsx-a11y/no-redundant-roles */ }
+							</VStack>
+						) }
+					</VStack>
+					<Spacer paddingY={ 8 } marginBottom={ 0 } />
 				</NavigatorScreen>
 
 				<NavigatorScreen path="/fontFamily">
diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/style.scss b/packages/edit-site/src/components/global-styles/font-library-modal/style.scss
index cfc42aa63a5c5..5cdc69dbfc1a7 100644
--- a/packages/edit-site/src/components/global-styles/font-library-modal/style.scss
+++ b/packages/edit-site/src/components/global-styles/font-library-modal/style.scss
@@ -48,12 +48,29 @@
 	margin-bottom: 0;
 }
 
+.font-library-modal__fonts-title {
+	text-transform: uppercase;
+	font-size: 11px;
+	font-weight: 600;
+	margin-top: 0;
+	margin-bottom: 0;
+}
+
+.font-library-modal__fonts-list {
+	margin-top: 0;
+	margin-bottom: 0;
+}
+
+.font-library-modal__fonts-list-item {
+	margin-top: -1px; /* To collapse the margin with the previous element */
+	margin-bottom: 0;
+}
+
 .font-library-modal__font-card {
 	border: 1px solid $gray-200;
 	width: 100%;
 	height: auto;
 	padding: $grid-unit-20;
-	margin-top: -1px; /* To collapse the margin with the previous element */
 
 	&:hover {
 		background-color: $gray-100;
diff --git a/test/e2e/specs/site-editor/font-library.spec.js b/test/e2e/specs/site-editor/font-library.spec.js
index 8bc7cfb17ea62..73aac85cdff28 100644
--- a/test/e2e/specs/site-editor/font-library.spec.js
+++ b/test/e2e/specs/site-editor/font-library.spec.js
@@ -61,7 +61,7 @@ test.describe( 'Font Library', () => {
 				.click();
 			await expect( page.getByRole( 'dialog' ) ).toBeVisible();
 			await expect(
-				page.getByRole( 'heading', { name: 'Fonts' } )
+				page.getByRole( 'heading', { name: 'Fonts', exact: true } )
 			).toBeVisible();
 		} );