diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 6917427ef15716..2cff4f26c20b96 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -11,6 +11,7 @@ ### Bug Fixes +- `FontSizePicker`: Remove non translatable additional info from font size picker visual label and improve labeling. ([#69011](https://github.com/WordPress/gutenberg/pull/69011)). - `Notice`: Fix text contrast for dark mode ([#69226](https://github.com/WordPress/gutenberg/pull/69226)). ## 29.4.0 (2025-02-12) diff --git a/packages/components/src/font-size-picker/font-size-picker-select.tsx b/packages/components/src/font-size-picker/font-size-picker-select.tsx index fcc80355ddd19a..0d3681f39d5a97 100644 --- a/packages/components/src/font-size-picker/font-size-picker-select.tsx +++ b/packages/components/src/font-size-picker/font-size-picker-select.tsx @@ -7,12 +7,11 @@ import { __, sprintf } from '@wordpress/i18n'; * Internal dependencies */ import CustomSelectControl from '../custom-select-control'; -import { parseQuantityAndUnitFromRawValue } from '../unit-control'; import type { FontSizePickerSelectProps, FontSizePickerSelectOption, } from './types'; -import { getCommonSizeUnit, isSimpleCssValue } from './utils'; +import { isSimpleCssValue } from './utils'; const DEFAULT_OPTION: FontSizePickerSelectOption = { key: 'default', @@ -23,20 +22,11 @@ const DEFAULT_OPTION: FontSizePickerSelectOption = { const FontSizePickerSelect = ( props: FontSizePickerSelectProps ) => { const { __next40pxDefaultSize, fontSizes, value, size, onChange } = props; - const areAllSizesSameUnit = !! getCommonSizeUnit( fontSizes ); - const options: FontSizePickerSelectOption[] = [ DEFAULT_OPTION, ...fontSizes.map( ( fontSize ) => { let hint; - if ( areAllSizesSameUnit ) { - const [ quantity ] = parseQuantityAndUnitFromRawValue( - fontSize.size - ); - if ( quantity !== undefined ) { - hint = String( quantity ); - } - } else if ( isSimpleCssValue( fontSize.size ) ) { + if ( isSimpleCssValue( fontSize.size ) ) { hint = String( fontSize.size ); } return { diff --git a/packages/components/src/font-size-picker/index.tsx b/packages/components/src/font-size-picker/index.tsx index a47812640f1a29..6bc2bd206ebe0a 100644 --- a/packages/components/src/font-size-picker/index.tsx +++ b/packages/components/src/font-size-picker/index.tsx @@ -8,7 +8,8 @@ import type { ForwardedRef } from 'react'; */ import { __ } from '@wordpress/i18n'; import { settings } from '@wordpress/icons'; -import { useState, useMemo, forwardRef } from '@wordpress/element'; +import { useState, forwardRef } from '@wordpress/element'; +import { useInstanceId } from '@wordpress/compose'; /** * Internal dependencies @@ -21,20 +22,11 @@ import { parseQuantityAndUnitFromRawValue, useCustomUnits, } from '../unit-control'; -import { VisuallyHidden } from '../visually-hidden'; -import { getCommonSizeUnit } from './utils'; import type { FontSizePickerProps } from './types'; -import { - Container, - Header, - HeaderHint, - HeaderLabel, - HeaderToggle, -} from './styles'; +import { Container, Header, HeaderLabel, HeaderToggle } from './styles'; import { Spacer } from '../spacer'; import FontSizePickerSelect from './font-size-picker-select'; import FontSizePickerToggleGroup from './font-size-picker-toggle-group'; -import { T_SHIRT_NAMES } from './constants'; import { maybeWarnDeprecated36pxSize } from '../utils/deprecated-36px-size'; const DEFAULT_UNITS = [ 'px', 'em', 'rem', 'vw', 'vh' ]; @@ -58,6 +50,11 @@ const UnforwardedFontSizePicker = ( withReset = true, } = props; + const labelId = useInstanceId( + UnforwardedFontSizePicker, + 'font-size-picker-label' + ); + const units = useCustomUnits( { availableUnits: unitsProp, } ); @@ -83,29 +80,6 @@ const UnforwardedFontSizePicker = ( : ( 'togglegroup' as const ); } - const headerHint = useMemo( () => { - switch ( currentPickerType ) { - case 'custom': - return __( 'Custom' ); - case 'togglegroup': - if ( selectedFontSize ) { - return ( - selectedFontSize.name || - T_SHIRT_NAMES[ fontSizes.indexOf( selectedFontSize ) ] - ); - } - break; - case 'select': - const commonUnit = getCommonSizeUnit( fontSizes ); - if ( commonUnit ) { - return `(${ commonUnit })`; - } - break; - } - - return ''; - }, [ currentPickerType, selectedFontSize, fontSizes ] ); - if ( fontSizes.length === 0 && disableCustomFontSizes ) { return null; } @@ -131,19 +105,16 @@ const UnforwardedFontSizePicker = ( } ); return ( - - { __( 'Font size' ) } +
- - { __( 'Size' ) } - { headerHint && ( - - { headerHint } - - ) } + + { __( 'Font size' ) } { ! disableCustomFontSizes && ( { await render( ); - const input = screen.getByLabelText( 'Custom' ); + const input = screen.getByRole( 'spinbutton', { + name: 'Font size', + } ); await user.clear( input ); await user.type( input, '80' ); expect( onChange ).toHaveBeenCalledTimes( 3 ); // Once for the clear, then once per keystroke. @@ -79,7 +81,9 @@ describe( 'FontSizePicker', () => { await user.click( screen.getByRole( 'button', { name: 'Set custom size' } ) ); - const input = screen.getByLabelText( 'Custom' ); + const input = screen.getByRole( 'spinbutton', { + name: 'Font size', + } ); await user.type( input, '80' ); expect( onChange ).toHaveBeenCalledTimes( 2 ); // Once per keystroke. expect( onChange ).toHaveBeenCalledWith( expectedValue ); @@ -129,28 +133,14 @@ describe( 'FontSizePicker', () => { const options = screen.getAllByRole( 'option' ); expect( options ).toHaveLength( 7 ); expect( options[ 0 ] ).toHaveAccessibleName( 'Default' ); - expect( options[ 1 ] ).toHaveAccessibleName( 'Tiny 8' ); - expect( options[ 2 ] ).toHaveAccessibleName( 'Small 12' ); - expect( options[ 3 ] ).toHaveAccessibleName( 'Medium 16' ); - expect( options[ 4 ] ).toHaveAccessibleName( 'Large 20' ); - expect( options[ 5 ] ).toHaveAccessibleName( 'Extra Large 30' ); - expect( options[ 6 ] ).toHaveAccessibleName( 'xx-large 40' ); + expect( options[ 1 ] ).toHaveAccessibleName( 'Tiny 8px' ); + expect( options[ 2 ] ).toHaveAccessibleName( 'Small 12px' ); + expect( options[ 3 ] ).toHaveAccessibleName( 'Medium 16px' ); + expect( options[ 4 ] ).toHaveAccessibleName( 'Large 20px' ); + expect( options[ 5 ] ).toHaveAccessibleName( 'Extra Large 30px' ); + expect( options[ 6 ] ).toHaveAccessibleName( 'xx-large 40px' ); } ); - test.each( [ - { value: undefined, expectedLabel: 'Size (px)' }, - { value: '8px', expectedLabel: 'Size (px)' }, - { value: '3px', expectedLabel: 'Size Custom' }, - ] )( - 'displays $expectedLabel as label when value is $value', - async ( { value, expectedLabel } ) => { - await render( - - ); - expect( screen.getByLabelText( expectedLabel ) ).toBeVisible(); - } - ); - test.each( [ { option: 'Default', @@ -158,7 +148,7 @@ describe( 'FontSizePicker', () => { expectedArguments: [ undefined ], }, { - option: 'Tiny 8', + option: 'Tiny 8px', value: undefined, expectedArguments: [ '8px', fontSizes[ 0 ] ], }, @@ -255,23 +245,6 @@ describe( 'FontSizePicker', () => { } ); - test.each( [ - { value: undefined, expectedLabel: 'Size' }, - { value: '8px', expectedLabel: 'Size' }, - { value: '1em', expectedLabel: 'Size' }, - { value: '2rem', expectedLabel: 'Size' }, - { value: 'clamp(1.75rem, 3vw, 2.25rem)', expectedLabel: 'Size' }, - { value: '3px', expectedLabel: 'Size Custom' }, - ] )( - 'displays $expectedLabel as label when value is $value', - async ( { value, expectedLabel } ) => { - await render( - - ); - expect( screen.getByLabelText( expectedLabel ) ).toBeVisible(); - } - ); - test.each( [ { option: 'Default', @@ -372,20 +345,6 @@ describe( 'FontSizePicker', () => { expect( options[ 4 ] ).toHaveAccessibleName( 'Gigantosaurus' ); } ); - test.each( [ - { value: undefined, expectedLabel: 'Size' }, - { value: '12px', expectedLabel: 'Size Small' }, - { value: '40px', expectedLabel: 'Size Gigantosaurus' }, - ] )( - 'displays $expectedLabel as label when value is $value', - async ( { value, expectedLabel } ) => { - await render( - - ); - expect( screen.getByLabelText( expectedLabel ) ).toBeVisible(); - } - ); - it( 'calls onChange when a font size is selected', async () => { const user = userEvent.setup(); const onChange = jest.fn(); @@ -439,25 +398,6 @@ describe( 'FontSizePicker', () => { expect( options[ 3 ] ).toHaveAccessibleName( 'Extra Large' ); } ); - test.each( [ - { value: undefined, expectedLabel: 'Size' }, - { value: '12px', expectedLabel: 'Size Small' }, - { value: '1em', expectedLabel: 'Size Medium' }, - { value: '2rem', expectedLabel: 'Size Large' }, - { - value: 'clamp(1.75rem, 3vw, 2.25rem)', - expectedLabel: 'Size Extra Large', - }, - ] )( - 'displays $expectedLabel as label when value is $value', - async ( { value, expectedLabel } ) => { - await render( - - ); - expect( screen.getByLabelText( expectedLabel ) ).toBeVisible(); - } - ); - test.each( [ { radio: 'Small', expectedArguments: [ '12px', fontSizes[ 0 ] ] }, { radio: 'Medium', expectedArguments: [ '1em', fontSizes[ 1 ] ] }, @@ -524,14 +464,18 @@ describe( 'FontSizePicker', () => { await render( ); - expect( screen.getByLabelText( 'Custom' ) ).toBeVisible(); + expect( + screen.getByRole( 'spinbutton', { name: 'Font size' } ) + ).toBeVisible(); } ); it( 'hides custom input when disableCustomFontSizes is set to `true` with a custom font size', async () => { const { rerender } = await render( ); - expect( screen.getByLabelText( 'Custom' ) ).toBeVisible(); + expect( + screen.getByRole( 'spinbutton', { name: 'Font size' } ) + ).toBeVisible(); rerender( { const { rerender } = await render( ); - expect( screen.getByLabelText( 'Custom' ) ).toBeVisible(); + expect( + screen.getByRole( 'spinbutton', { name: 'Font size' } ) + ).toBeVisible(); rerender( { value={ fontSizes[ 0 ].size } /> ); - expect( screen.getByLabelText( 'Custom' ) ).toBeVisible(); + expect( + screen.getByRole( 'spinbutton', { name: 'Font size' } ) + ).toBeVisible(); } ); it( 'allows custom values by default', async () => { @@ -569,7 +517,10 @@ describe( 'FontSizePicker', () => { await user.click( screen.getByRole( 'button', { name: 'Set custom size' } ) ); - await user.type( screen.getByLabelText( 'Custom' ), '80' ); + await user.type( + screen.getByRole( 'spinbutton', { name: 'Font size' } ), + '80' + ); expect( onChange ).toHaveBeenCalledTimes( 2 ); // Once per keystroke. expect( onChange ).toHaveBeenCalledWith( '80px' ); } ); @@ -585,7 +536,10 @@ describe( 'FontSizePicker', () => { screen.getByRole( 'button', { name: 'Set custom size' } ) ); - await user.type( screen.getByLabelText( 'Custom' ), '80' ); + await user.type( + screen.getByRole( 'spinbutton', { name: 'Font size' } ), + '80' + ); await user.click( screen.getByRole( 'button', { name: 'Use size preset' } ) @@ -632,7 +586,9 @@ describe( 'FontSizePicker', () => { await user.click( screen.getByRole( 'button', { name: 'Set custom size' } ) ); - const sliderInput = screen.getByLabelText( 'Custom Size' ); + const sliderInput = screen.getByRole( 'slider', { + name: 'Font size', + } ); fireEvent.change( sliderInput, { target: { value: 80 }, } ); diff --git a/packages/components/src/font-size-picker/test/utils.ts b/packages/components/src/font-size-picker/test/utils.ts index 584820ae27a71a..78cd538c2e1f24 100644 --- a/packages/components/src/font-size-picker/test/utils.ts +++ b/packages/components/src/font-size-picker/test/utils.ts @@ -1,7 +1,7 @@ /** * Internal dependencies */ -import { isSimpleCssValue, getCommonSizeUnit } from '../utils'; +import { isSimpleCssValue } from '../utils'; describe( 'isSimpleCssValue', () => { test.each( [ @@ -31,39 +31,3 @@ describe( 'isSimpleCssValue', () => { expect( isSimpleCssValue( cssValue ) ).toBe( result ); } ); } ); - -describe( 'getCommonSizeUnit', () => { - it( 'returns null when fontSizes is empty', () => { - expect( getCommonSizeUnit( [] ) ).toBe( null ); - } ); - - it( 'returns px when all sizes are px', () => { - expect( - getCommonSizeUnit( [ - { slug: 'small', size: '10px' }, - { slug: 'medium', size: '20px' }, - { slug: 'large', size: '30px' }, - ] ) - ).toBe( 'px' ); - } ); - - it( 'returns em when all sizes are em', () => { - expect( - getCommonSizeUnit( [ - { slug: 'small', size: '1em' }, - { slug: 'medium', size: '2em' }, - { slug: 'large', size: '3em' }, - ] ) - ).toBe( 'em' ); - } ); - - it( 'returns null when sizes are heterogeneous', () => { - expect( - getCommonSizeUnit( [ - { slug: 'small', size: '10px' }, - { slug: 'medium', size: '2em' }, - { slug: 'large', size: '3rem' }, - ] ) - ).toBe( null ); - } ); -} ); diff --git a/packages/components/src/font-size-picker/utils.ts b/packages/components/src/font-size-picker/utils.ts index 64816f12255bac..4d256496e63006 100644 --- a/packages/components/src/font-size-picker/utils.ts +++ b/packages/components/src/font-size-picker/utils.ts @@ -1,8 +1,7 @@ /** * Internal dependencies */ -import type { FontSizePickerProps, FontSize } from './types'; -import { parseQuantityAndUnitFromRawValue } from '../unit-control'; +import type { FontSizePickerProps } from './types'; /** * Some themes use css vars for their font sizes, so until we @@ -18,25 +17,3 @@ export function isSimpleCssValue( /^[\d\.]+(px|em|rem|vw|vh|%|svw|lvw|dvw|svh|lvh|dvh|vi|svi|lvi|dvi|vb|svb|lvb|dvb|vmin|svmin|lvmin|dvmin|vmax|svmax|lvmax|dvmax)?$/i; return sizeRegex.test( String( value ) ); } - -/** - * If all of the given font sizes have the same unit (e.g. 'px'), return that - * unit. Otherwise return null. - * - * @param fontSizes List of font sizes. - * @return The common unit, or null. - */ -export function getCommonSizeUnit( fontSizes: FontSize[] ) { - const [ firstFontSize, ...otherFontSizes ] = fontSizes; - if ( ! firstFontSize ) { - return null; - } - const [ , firstUnit ] = parseQuantityAndUnitFromRawValue( - firstFontSize.size - ); - const areAllSizesSameUnit = otherFontSizes.every( ( fontSize ) => { - const [ , unit ] = parseQuantityAndUnitFromRawValue( fontSize.size ); - return unit === firstUnit; - } ); - return areAllSizesSameUnit ? firstUnit : null; -} diff --git a/test/e2e/specs/editor/various/font-size-picker.spec.js b/test/e2e/specs/editor/various/font-size-picker.spec.js index 597b458d682e84..d3957e7cb65d8d 100644 --- a/test/e2e/specs/editor/various/font-size-picker.spec.js +++ b/test/e2e/specs/editor/various/font-size-picker.spec.js @@ -31,7 +31,7 @@ test.describe( 'Font Size Picker', () => { await page.click( 'role=region[name="Editor settings"i] >> role=button[name="Set custom size"i]' ); - await page.click( 'role=spinbutton[name="Custom"i]' ); + await page.click( 'role=spinbutton[name="Font size"i]' ); await page.keyboard.type( '23' ); @@ -54,7 +54,7 @@ test.describe( 'Font Size Picker', () => { await page.click( 'role=region[name="Editor settings"i] >> role=button[name="Set custom size"i]' ); - await page.click( 'role=spinbutton[name="Custom"i]' ); + await page.click( 'role=spinbutton[name="Font size"i]' ); await page.keyboard.type( '23' ); await expect.poll( editor.getEditedPostContent ) @@ -214,7 +214,7 @@ test.describe( 'Font Size Picker', () => { await page.click( 'role=region[name="Editor settings"i] >> role=button[name="Set custom size"i]' ); - await page.click( 'role=spinbutton[name="Custom"i]' ); + await page.click( 'role=spinbutton[name="Font size"i]' ); await pageUtils.pressKeys( 'primary+A' ); await page.keyboard.press( 'Backspace' ); @@ -299,7 +299,7 @@ test.describe( 'Font Size Picker', () => { await page.click( 'role=region[name="Editor settings"i] >> role=button[name="Set custom size"i]' ); - await page.click( 'role=spinbutton[name="Custom"i]' ); + await page.click( 'role=spinbutton[name="Font size"i]' ); await pageUtils.pressKeys( 'primary+A' ); await page.keyboard.press( 'Backspace' ); diff --git a/test/e2e/specs/site-editor/style-variations.spec.js b/test/e2e/specs/site-editor/style-variations.spec.js index 8ec92ff657391b..53de08717226a4 100644 --- a/test/e2e/specs/site-editor/style-variations.spec.js +++ b/test/e2e/specs/site-editor/style-variations.spec.js @@ -97,8 +97,8 @@ test.describe( 'Global styles variations', () => { await page.click( 'role=button[name="Text"i]' ); await expect( - page.locator( 'css=.components-font-size-picker__header__hint' ) - ).toHaveText( 'Medium' ); + page.locator( 'role=radio[name="Medium"i]' ) + ).toHaveAttribute( 'aria-checked', 'true' ); } ); test( 'should apply custom colors and font sizes in a variation', async ( { @@ -132,14 +132,8 @@ test.describe( 'Global styles variations', () => { await page.click( 'role=button[name="Typography"i]' ); await page.click( 'role=button[name="Text"i]' ); - // TODO: to avoid use classnames to locate these elements, - // we could provide accessible attributes to the source code in packages/components/src/font-size-picker/index.js. await expect( - page.locator( 'css=.components-font-size-picker__header__hint' ) - ).toHaveText( 'Custom' ); - - await expect( - page.locator( 'role=spinbutton[name="Custom"i]' ) + page.locator( 'role=spinbutton[name="Font size"i]' ) ).toHaveValue( '15' ); } );