Change font-size-picker markup to use select (#16148)

* Change font-size-picker markup to use select * Improve naming. * Fix e2e tests. * Fix more tests. * Address PR feedback. * Address PR feedback.
WordPress · Jul 9, 2019 · 4912471 · 4912471
1 parent 105cb47
commit 4912471
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 121 deletions.
diff --git a/packages/block-editor/src/components/font-sizes/with-font-sizes.js b/packages/block-editor/src/components/font-sizes/with-font-sizes.js
@@ -64,7 +64,7 @@ export default ( ...fontSizeNames ) => {
 
 					createSetFontSize( fontSizeAttributeName, customFontSizeAttributeName ) {
 						return ( fontSizeValue ) => {
-							const fontSizeObject = find( this.props.fontSizes, { size: fontSizeValue } );
+							const fontSizeObject = find( this.props.fontSizes, { size: Number( fontSizeValue ) } );
 							this.props.setAttributes( {
 								[ fontSizeAttributeName ]: fontSizeObject && fontSizeObject.slug ? fontSizeObject.slug : undefined,
 								[ customFontSizeAttributeName ]: fontSizeObject && fontSizeObject.slug ? undefined : fontSizeValue,

diff --git a/packages/components/src/base-control/index.js b/packages/components/src/base-control/index.js
@@ -3,11 +3,14 @@
  */
 import classnames from 'classnames';
 
-function BaseControl( { id, label, help, className, children } ) {
+function BaseControl( { id, label, hideLabelFromVision, help, className, children } ) {
 	return (
 		<div className={ classnames( 'components-base-control', className ) }>
 			<div className="components-base-control__field">
-				{ label && id && <label className="components-base-control__label" htmlFor={ id }>{ label }</label> }
+				{ label && id && <label
+					className={ classnames( 'components-base-control__label', { 'screen-reader-text': hideLabelFromVision } ) }
+					htmlFor={ id }>{ label }
+				</label> }
 				{ label && ! id && <BaseControl.VisualLabel>{ label }</BaseControl.VisualLabel> }
 				{ children }
 			</div>

diff --git a/packages/components/src/font-size-picker/index.js b/packages/components/src/font-size-picker/index.js
@@ -1,22 +1,31 @@
-/**
- * External dependencies
- */
-import { map } from 'lodash';
 
 /**
  * WordPress dependencies
  */
-import { __, _x, sprintf } from '@wordpress/i18n';
+import { useState } from '@wordpress/element';
+import { __ } from '@wordpress/i18n';
 
 /**
  * Internal dependencies
  */
-import Dashicon from '../dashicon';
-import BaseControl from '../base-control';
 import Button from '../button';
-import Dropdown from '../dropdown';
 import RangeControl from '../range-control';
-import { NavigableMenu } from '../navigable-container';
+import SelectControl from '../select-control';
+
+function getSelectValueFromFontSize( fontSizes, value ) {
+	if ( value ) {
+		const fontSizeValue = fontSizes.find( ( font ) => font.size === value );
+		return fontSizeValue ? fontSizeValue.slug : 'custom';
+	}
+	return 'normal';
+}
+
+function getSelectOptions( optionsArray ) {
+	return [
+		...optionsArray.map( ( option ) => ( { value: option.slug, label: option.name } ) ),
+		{ value: 'custom', label: __( 'Custom' ) },
+	];
+}
 
 function FontSizePicker( {
 	fallbackFontSize,
@@ -26,84 +35,64 @@ function FontSizePicker( {
 	value,
 	withSlider = false,
 } ) {
+	// eslint-disable-next-line @wordpress/no-unused-vars-before-return
+	const [ currentSelectValue, setCurrentSelectValue ] = useState( getSelectValueFromFontSize( fontSizes, value ) );
+
 	if ( disableCustomFontSizes && ! fontSizes.length ) {
 		return null;
 	}
+
 	const onChangeValue = ( event ) => {
 		const newValue = event.target.value;
+		setCurrentSelectValue( getSelectValueFromFontSize( fontSizes, Number( newValue ) ) );
 		if ( newValue === '' ) {
 			onChange( undefined );
 			return;
 		}
 		onChange( Number( newValue ) );
 	};
 
-	const currentFont = fontSizes.find( ( font ) => font.size === value );
-	const currentFontSizeName = ( currentFont && currentFont.name ) || ( ! value && _x( 'Normal', 'font size name' ) ) || _x( 'Custom', 'font size name' );
+	const onSelectChangeValue = ( eventValue ) => {
+		setCurrentSelectValue( eventValue );
+		const selectedFont = fontSizes.find( ( font ) => font.slug === eventValue );
+		if ( selectedFont ) {
+			onChange( selectedFont.size );
+		}
+	};
 
 	return (
-		<BaseControl>
-			<BaseControl.VisualLabel>
+		<fieldset>
+			<legend>
 				{ __( 'Font Size' ) }
-			</BaseControl.VisualLabel>
-			<div className="components-font-size-picker__buttons">
+			</legend>
+			<div className="components-font-size-picker__controls">
 				{ ( fontSizes.length > 0 ) &&
-					<Dropdown
-						className="components-font-size-picker__dropdown"
-						contentClassName="components-font-size-picker__dropdown-content"
-						position="bottom"
-						renderToggle={ ( { isOpen, onToggle } ) => (
-							<Button
-								className="components-font-size-picker__selector"
-								isLarge
-								onClick={ onToggle }
-								aria-expanded={ isOpen }
-								aria-label={ sprintf(
-									/* translators: %s: font size name */
-									__( 'Font size: %s' ), currentFontSizeName
-								) }
-							>
-								{ currentFontSizeName }
-							</Button>
-						) }
-						renderContent={ () => (
-							<NavigableMenu>
-								{ map( fontSizes, ( { name, size, slug } ) => {
-									const isSelected = ( value === size || ( ! value && slug === 'normal' ) );
-
-									return (
-										<Button
-											key={ slug }
-											onClick={ () => onChange( slug === 'normal' ? undefined : size ) }
-											className={ `is-font-${ slug }` }
-											role="menuitemradio"
-											aria-checked={ isSelected }
-										>
-											{ isSelected && <Dashicon icon="saved" /> }
-											<span className="components-font-size-picker__dropdown-text-size" style={ { fontSize: size } }>
-												{ name }
-											</span>
-										</Button>
-									);
-								} ) }
-							</NavigableMenu>
-						) }
+					<SelectControl
+						className={ 'components-font-size-picker__select' }
+						label={ 'Choose preset' }
+						hideLabelFromVision={ true }
+						value={ currentSelectValue }
+						onChange={ onSelectChangeValue }
+						options={ getSelectOptions( fontSizes ) }
 					/>
 				}
 				{ ( ! withSlider && ! disableCustomFontSizes ) &&
 					<input
 						className="components-range-control__number"
 						type="number"
 						onChange={ onChangeValue }
-						aria-label={ __( 'Custom font size' ) }
+						aria-label={ __( 'Custom' ) }
 						value={ value || '' }
 					/>
 				}
 				<Button
 					className="components-color-palette__clear"
 					type="button"
 					disabled={ value === undefined }
-					onClick={ () => onChange( undefined ) }
+					onClick={ () => {
+						onChange( undefined );
+						setCurrentSelectValue( getSelectValueFromFontSize( fontSizes, undefined ) );
+					} }
 					isSmall
 					isDefault
 				>
@@ -123,7 +112,7 @@ function FontSizePicker( {
 					afterIcon="editor-textcolor"
 				/>
 			}
-		</BaseControl>
+		</fieldset>
 	);
 }
 

diff --git a/packages/components/src/font-size-picker/style.scss b/packages/components/src/font-size-picker/style.scss
@@ -1,8 +1,9 @@
-.components-font-size-picker__buttons {
+.components-font-size-picker__controls {
 	max-width: $sidebar-width - ( 2 * $panel-padding );
 	display: flex;
 	justify-content: space-between;
 	align-items: center;
+	margin-bottom: $grid-size * 3;
 
 	// Apply the same height as the isSmall Reset button.
 	.components-range-control__number {
@@ -18,51 +19,16 @@
 	}
 }
 
+// needed to override CSS set in https://github.com/WordPress/gutenberg/blob/9c438f93a7215d50d1efc0492c308e4cbaa59c52/packages/edit-post/src/components/sidebar/settings-sidebar/style.scss#L6
+.components-font-size-picker__select.components-font-size-picker__select.components-font-size-picker__select.components-font-size-picker__select,
+.components-font-size-picker__select .components-base-control__field {
+	margin-bottom: 0;
+}
+
 .components-font-size-picker__custom-input {
 	.components-range-control__slider + .dashicon {
 		width: 30px;
 		height: 30px;
 	}
 }
 
-.components-font-size-picker__dropdown-content .components-button {
-	display: block;
-	position: relative;
-	padding: 10px 20px 10px 40px;
-	width: 100%;
-	text-align: left;
-
-	.dashicon {
-		position: absolute;
-		top: calc(50% - 10px);
-		left: 10px;
-	}
-
-	&:hover {
-		@include menu-style__hover;
-	}
-
-	&:focus {
-		@include menu-style__focus;
-	}
-}
-
-.components-font-size-picker__buttons .components-font-size-picker__selector {
-	border: 1px solid;
-	background: none;
-	position: relative;
-	width: 110px;
-
-	@include input-style__neutral();
-
-	&:focus {
-		@include input-style__focus();
-	}
-
-	&::after {
-		@include dropdown-arrow();
-		right: 8px;
-		top: 12px;
-		position: absolute;
-	}
-}
diff --git a/packages/components/src/select-control/index.js b/packages/components/src/select-control/index.js
@@ -21,6 +21,7 @@ function SelectControl( {
 	onChange,
 	options = [],
 	className,
+	hideLabelFromVision,
 	...props
 } ) {
 	const id = `inspector-select-control-${ instanceId }`;
@@ -38,7 +39,7 @@ function SelectControl( {
 
 	/* eslint-disable jsx-a11y/no-onchange */
 	return ! isEmpty( options ) && (
-		<BaseControl label={ label } id={ id } help={ help } className={ className }>
+		<BaseControl label={ label } hideLabelFromVision={ hideLabelFromVision } id={ id } help={ help } className={ className }>
 			<select
 				id={ id }
 				className="components-select-control__input"

diff --git a/packages/e2e-tests/specs/editor-modes.test.js b/packages/e2e-tests/specs/editor-modes.test.js
@@ -56,7 +56,7 @@ describe( 'Editing modes (visual/HTML)', () => {
 
 		// The font size picker for the paragraph block should appear, even in
 		// HTML editing mode.
-		const fontSizePicker = await page.$$( '.edit-post-sidebar .components-font-size-picker__buttons' );
+		const fontSizePicker = await page.$$( '.edit-post-sidebar .components-font-size-picker__controls' );
 		expect( fontSizePicker ).toHaveLength( 1 );
 	} );
 
@@ -74,9 +74,7 @@ describe( 'Editing modes (visual/HTML)', () => {
 		expect( htmlBlockContent ).toEqual( '<p>Hello world!</p>' );
 
 		// Change the font size using the sidebar.
-		await page.click( '.components-font-size-picker__selector' );
-		const changeSizeButton = await page.waitForSelector( '.components-button.is-font-large' );
-		await changeSizeButton.click();
+		await page.select( '.components-font-size-picker__select .components-select-control__input', 'large' );
 
 		// Make sure the HTML content updated.
 		htmlBlockContent = await page.$eval( '.block-editor-block-list__layout .block-editor-block-list__block .block-editor-block-list__block-html-textarea', ( node ) => node.textContent );

diff --git a/packages/e2e-tests/specs/font-size-picker.test.js b/packages/e2e-tests/specs/font-size-picker.test.js
@@ -17,9 +17,7 @@ describe( 'Font Size Picker', () => {
 		// Create a paragraph block with some content.
 		await clickBlockAppender();
 		await page.keyboard.type( 'Paragraph to be made "large"' );
-		await page.click( '.components-font-size-picker__selector' );
-		const changeSizeButton = await page.waitForSelector( '.components-button.is-font-large' );
-		await changeSizeButton.click();
+		await page.select( '.components-font-size-picker__select .components-select-control__input', 'large' );
 
 		// Ensure content matches snapshot.
 		const content = await getEditedPostContent();
@@ -58,14 +56,9 @@ describe( 'Font Size Picker', () => {
 		await clickBlockAppender();
 		await page.keyboard.type( 'Paragraph with font size reset using button' );
 
-		await page.click( '.blocks-font-size .components-range-control__number' );
-		// This should be the default font-size of the current theme.
-		await page.keyboard.type( '22' );
-
-		// Blur the range control
-		await page.click( '.components-base-control__label' );
+		await page.select( '.components-font-size-picker__select .components-select-control__input', 'normal' );
 
-		const resetButton = ( await page.$x( '//*[contains(concat(" ", @class, " "), " components-font-size-picker__buttons ")]//*[text()=\'Reset\']' ) )[ 0 ];
+		const resetButton = ( await page.$x( '//*[contains(concat(" ", @class, " "), " components-font-size-picker__controls ")]//*[text()=\'Reset\']' ) )[ 0 ];
 		await resetButton.click();
 
 		// Ensure content matches snapshot.
@@ -78,9 +71,7 @@ describe( 'Font Size Picker', () => {
 		await clickBlockAppender();
 		await page.keyboard.type( 'Paragraph with font size reset using input field' );
 
-		await page.click( '.components-font-size-picker__selector' );
-		const changeSizeButton = await page.waitForSelector( '.components-button.is-font-large' );
-		await changeSizeButton.click();
+		await page.select( '.components-font-size-picker__select .components-select-control__input', 'large' );
 
 		// Clear the custom font size input.
 		await page.click( '.blocks-font-size .components-range-control__number' );