Skip to content

Commit

Permalink
Make the fonts management modal dialog more discoverable (#62129)
Browse files Browse the repository at this point in the history
* Replace Manage fonts icon button with visible text button.

* Fix after rebase.

* Male buttons use the next 40 pixels height.

Co-authored-by: afercia <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: jameskoster <[email protected]>
Co-authored-by: richtabor <[email protected]>
Co-authored-by: colorful-tones <[email protected]>
Co-authored-by: noisysocks <[email protected]>
Co-authored-by: annezazu <[email protected]>
  • Loading branch information
8 people authored Jun 24, 2024
1 parent 73a9d35 commit 8d99dcf
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 34 deletions.
47 changes: 28 additions & 19 deletions packages/edit-site/src/components/global-styles/font-families.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ import { __ } from '@wordpress/i18n';
import {
__experimentalItemGroup as ItemGroup,
__experimentalVStack as VStack,
__experimentalHStack as HStack,
Button,
} from '@wordpress/components';
import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor';
import { settings } from '@wordpress/icons';
import { useContext } from '@wordpress/element';

/**
Expand Down Expand Up @@ -51,30 +49,41 @@ function FontFamilies() {
) }

<VStack spacing={ 2 }>
<HStack justify="space-between">
<Subtitle level={ 3 }>{ __( 'Fonts' ) }</Subtitle>
<Button
onClick={ () => setModalTabOpen( 'installed-fonts' ) }
label={ __( 'Manage fonts' ) }
icon={ settings }
size="small"
/>
</HStack>
<Subtitle level={ 3 }>{ __( 'Fonts' ) }</Subtitle>
{ hasFonts ? (
<ItemGroup isBordered isSeparated>
{ customFonts.map( ( font ) => (
<FontFamilyItem key={ font.slug } font={ font } />
) ) }
{ themeFonts.map( ( font ) => (
<FontFamilyItem key={ font.slug } font={ font } />
) ) }
</ItemGroup>
<>
<ItemGroup isBordered isSeparated>
{ customFonts.map( ( font ) => (
<FontFamilyItem
key={ font.slug }
font={ font }
/>
) ) }
{ themeFonts.map( ( font ) => (
<FontFamilyItem
key={ font.slug }
font={ font }
/>
) ) }
</ItemGroup>
<Button
className="edit-site-global-styles-font-families__manage-fonts"
variant="secondary"
__next40pxDefaultSize
onClick={ () =>
setModalTabOpen( 'installed-fonts' )
}
>
{ __( 'Manage fonts' ) }
</Button>
</>
) : (
<>
{ __( 'No fonts installed.' ) }
<Button
className="edit-site-global-styles-font-families__add-fonts"
variant="secondary"
__next40pxDefaultSize
onClick={ () => setModalTabOpen( 'upload-fonts' ) }
>
{ __( 'Add fonts' ) }
Expand Down
3 changes: 2 additions & 1 deletion packages/edit-site/src/components/global-styles/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
color: $gray-700;
}

.edit-site-global-styles-font-families__add-fonts {
.edit-site-global-styles-font-families__add-fonts,
.edit-site-global-styles-font-families__manage-fonts {
justify-content: center;
}

Expand Down
29 changes: 15 additions & 14 deletions test/e2e/specs/site-editor/font-library.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ test.describe( 'Font Library', () => {
await editor.canvas.locator( 'body' ).click();
} );

test( 'should display the "Manage Fonts" icon', async ( { page } ) => {
test( 'should display the "Add fonts" button', async ( { page } ) => {
await page.getByRole( 'button', { name: 'Styles' } ).click();
await page
.getByRole( 'button', { name: 'Typography Styles' } )
.click();
const manageFontsIcon = page.getByRole( 'button', {
name: 'Manage Fonts',
const addFontsButton = page.getByRole( 'button', {
name: 'Add fonts',
} );
await expect( manageFontsIcon ).toBeVisible();
await expect( addFontsButton ).toBeVisible();
} );
} );

Expand All @@ -36,18 +36,20 @@ test.describe( 'Font Library', () => {
await editor.canvas.locator( 'body' ).click();
} );

test( 'should display the "Manage Fonts" icon', async ( { page } ) => {
test( 'should display the "Manage fonts" button', async ( {
page,
} ) => {
await page.getByRole( 'button', { name: 'Styles' } ).click();
await page
.getByRole( 'button', { name: 'Typography Styles' } )
.click();
const manageFontsIcon = page.getByRole( 'button', {
name: 'Manage Fonts',
const manageFontsButton = page.getByRole( 'button', {
name: 'Manage fonts',
} );
await expect( manageFontsIcon ).toBeVisible();
await expect( manageFontsButton ).toBeVisible();
} );

test( 'should open the "Manage Fonts" modal when clicking the "Manage Fonts" icon', async ( {
test( 'should open the "Manage fonts" modal when clicking the "Manage fonts" button', async ( {
page,
} ) => {
await page.getByRole( 'button', { name: 'Styles' } ).click();
Expand All @@ -56,7 +58,7 @@ test.describe( 'Font Library', () => {
.click();
await page
.getByRole( 'button', {
name: 'Manage Fonts',
name: 'Manage fonts',
} )
.click();
await expect( page.getByRole( 'dialog' ) ).toBeVisible();
Expand All @@ -74,7 +76,7 @@ test.describe( 'Font Library', () => {
.click();
await page
.getByRole( 'button', {
name: 'Manage Fonts',
name: 'Manage fonts',
} )
.click();
await page.getByRole( 'button', { name: 'System Font' } ).click();
Expand Down Expand Up @@ -120,12 +122,11 @@ test.describe( 'Font Library', () => {
.click();
await page
.getByRole( 'button', {
name: 'Manage Fonts',
name: 'Add fonts',
} )
.click();

// Upload local fonts.
await page.getByRole( 'tab', { name: 'Upload' } ).click();
const fileChooserPromise = page.waitForEvent( 'filechooser' );
await page.getByRole( 'button', { name: 'Upload Font' } ).click();
const fileChooser = await fileChooserPromise;
Expand Down Expand Up @@ -163,7 +164,7 @@ test.describe( 'Font Library', () => {
await page.getByRole( 'button', { name: 'Back' } ).click();
await page
.getByRole( 'button', {
name: 'Manage Fonts',
name: 'Manage fonts',
} )
.click();

Expand Down

0 comments on commit 8d99dcf

Please sign in to comment.