LineHeightControl: Migrate internals to NumberControl (WordPress#37160

)

* Add story for LineHeightControl

* Keep legacy styling

* Add deprecation warning

* Update docs

* Update changelog

* Update style deprecation to match new guidelines

* Update usage in Typography Panel

* Update usage in Global Styles

* Update docs

* Tweak stories

* Update story for ToolsPanel

* Add deprecation note to prop readme

* LineHeightControl: Migrate internals to `NumberControl` (Part 2: Behavior) (WordPress#37164)

* Maintain previous behavior via state reducer

* Add tests

* Add comment about strange reducer behavior

* Fix tests

* Remove temporary code

packages/block-editor/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### New Features
+
+-   `LineHeightControl`: Changes internal implementation to use `NumberControl`, which allows enhanced interactions such as dragging to change the value. To improve consistency with other control components, the bottom margin styles on the component has been deprecated, and will be removed in a future version. To opt into this simplified margin style, set the `__nextHasNoMarginBottom` prop to `true`.
+
 ## 8.1.1 (2022-02-10)
 
 ### Bug Fix

packages/block-editor/src/components/line-height-control/README.md

@@ -18,9 +18,13 @@ _Note:_ It is worth noting that the line height setting option is an opt-in feat
 Renders the markup for the line height setting option in the block inspector.
 
 ```jsx
-import { KeyboardShortcuts } from '@wordpress/block-editor';
+import { LineHeightControl } from '@wordpress/block-editor';
 const MyLineHeightControl = () => (
-	<LineHeightControl value={ lineHeight } onChange={ onChange } />
+	<LineHeightControl
+		value={ lineHeight }
+		onChange={ onChange }
+		__nextHasNoMarginBottom={ true }
+	/>
 );
 ```
 
@@ -38,6 +42,13 @@ The value of the line height.
 
 A callback function that handles the application of the line height value.
 
+#### `__nextHasNoMarginBottom`
+
+-   **Type:** `boolean`
+-   **Default:** `false`
+
+Start opting into the new margin-free styles that will become the default in a future version, currently scheduled to be WordPress 6.4. (The prop can be safely removed once this happens.)
+
 ## Related components
 
 Block Editor components are components that can be used to compose the UI of your block editor. Thus, they can only be used under a [`BlockEditorProvider`](https://github.com/WordPress/gutenberg/blob/HEAD/packages/block-editor/src/components/provider/README.md) in the components tree.

packages/block-editor/src/components/line-height-control/index.js

@@ -1,9 +1,9 @@
 /**
  * WordPress dependencies
  */
+import deprecated from '@wordpress/deprecated';
 import { __ } from '@wordpress/i18n';
-import { TextControl } from '@wordpress/components';
-import { ZERO } from '@wordpress/keycodes';
+import { __experimentalNumberControl as NumberControl } from '@wordpress/components';
 
 /**
  * Internal dependencies
@@ -15,64 +15,87 @@ import {
 	isLineHeightDefined,
 } from './utils';
 
-export default function LineHeightControl( { value: lineHeight, onChange } ) {
+export default function LineHeightControl( {
+	value: lineHeight,
+	onChange,
+	/** Start opting into the new margin-free styles that will become the default in a future version. */
+	__nextHasNoMarginBottom = false,
+	__unstableInputWidth = '60px',
+} ) {
 	const isDefined = isLineHeightDefined( lineHeight );
 
-	const handleOnKeyDown = ( event ) => {
-		const { keyCode } = event;
-
-		if ( keyCode === ZERO && ! isDefined ) {
-			/**
-			 * Prevents the onChange callback from firing, which prevents
-			 * the logic from assuming the change was triggered from
-			 * an input arrow CLICK.
-			 */
-			event.preventDefault();
-			onChange( '0' );
-		}
-	};
-
-	const handleOnChange = ( nextValue ) => {
+	const adjustNextValue = ( nextValue, wasTypedOrPasted ) => {
 		// Set the next value without modification if lineHeight has been defined
-		if ( isDefined ) {
-			onChange( nextValue );
-			return;
-		}
+		if ( isDefined ) return nextValue;
 
-		// Otherwise...
 		/**
-		 * The following logic handles the initial up/down arrow CLICK of the
-		 * input element. This is so that the next values (from an undefined value state)
-		 * are more better suited for line-height rendering.
+		 * The following logic handles the initial step up/down action
+		 * (from an undefined value state) so that the next values are better suited for
+		 * line-height rendering. For example, the first step up should immediately
+		 * go to 1.6, rather than the normally expected 0.1.
+		 *
+		 * Step up/down actions can be triggered by keydowns of the up/down arrow keys,
+		 * or by clicking the spin buttons.
 		 */
-		let adjustedNextValue = nextValue;
-
 		switch ( nextValue ) {
 			case `${ STEP }`:
 				// Increment by step value
-				adjustedNextValue = BASE_DEFAULT_VALUE + STEP;
-				break;
-			case '0':
+				return BASE_DEFAULT_VALUE + STEP;
+			case '0': {
+				// This means the user explicitly input '0', rather than stepped down
+				// from an undefined value state
+				if ( wasTypedOrPasted ) return nextValue;
 				// Decrement by step value
-				adjustedNextValue = BASE_DEFAULT_VALUE - STEP;
-				break;
+				return BASE_DEFAULT_VALUE - STEP;
+			}
+			case '':
+				return BASE_DEFAULT_VALUE;
+			default:
+				return nextValue;
 		}
+	};
 
-		onChange( adjustedNextValue );
+	const stateReducer = ( state, action ) => {
+		// Be careful when changing this — cross-browser behavior of the
+		// `inputType` field in `input` events are inconsistent.
+		// For example, Firefox emits an input event with inputType="insertReplacementText"
+		// on spin button clicks, while other browsers do not even emit an input event.
+		const wasTypedOrPasted = [ 'insertText', 'insertFromPaste' ].includes(
+			action.payload.event.nativeEvent.inputType
+		);
+		state.value = adjustNextValue( state.value, wasTypedOrPasted );
+		return state;
 	};
 
 	const value = isDefined ? lineHeight : RESET_VALUE;
 
+	if ( ! __nextHasNoMarginBottom ) {
+		deprecated(
+			'Bottom margin styles for wp.blockEditor.LineHeightControl',
+			{
+				since: '6.0',
+				version: '6.4',
+				hint:
+					'Set the `__nextHasNoMarginBottom` prop to true to start opting into the new styles, which will become the default in a future version',
+			}
+		);
+	}
+	const deprecatedStyles = __nextHasNoMarginBottom
+		? undefined
+		: { marginBottom: 24 };
+
 	return (
-		<div className="block-editor-line-height-control">
-			<TextControl
-				autoComplete="off"
-				onKeyDown={ handleOnKeyDown }
-				onChange={ handleOnChange }
+		<div
+			className="block-editor-line-height-control"
+			style={ deprecatedStyles }
+		>
+			<NumberControl
+				__unstableInputWidth={ __unstableInputWidth }
+				__unstableStateReducer={ stateReducer }
+				onChange={ onChange }
 				label={ __( 'Line height' ) }
 				placeholder={ BASE_DEFAULT_VALUE }
 				step={ STEP }
-				type="number"
 				value={ value }
 				min={ 0 }
 			/>

packages/block-editor/src/components/line-height-control/stories/index.js

@@ -0,0 +1,33 @@
+/**
+ * WordPress dependencies
+ */
+import { useState } from '@wordpress/element';
+
+/**
+ * Internal dependencies
+ */
+import LineHeightControl from '../';
+
+export default {
+	component: LineHeightControl,
+	title: 'BlockEditor/LineHeightControl',
+};
+
+const Template = ( props ) => {
+	const [ value, setValue ] = useState();
+	return (
+		<LineHeightControl onChange={ setValue } value={ value } { ...props } />
+	);
+};
+
+export const Default = Template.bind( {} );
+Default.args = {
+	__nextHasNoMarginBottom: true,
+	__unstableInputWidth: '60px',
+};
+
+export const UnconstrainedWidth = Template.bind( {} );
+UnconstrainedWidth.args = {
+	...Default.args,
+	__unstableInputWidth: '100%',
+};

packages/block-editor/src/components/line-height-control/test/index.js

@@ -0,0 +1,61 @@
+/**
+ * External dependencies
+ */
+import { fireEvent, render, screen } from '@testing-library/react';
+
+/**
+ * WordPress dependencies
+ */
+import { useState } from '@wordpress/element';
+import { UP, DOWN } from '@wordpress/keycodes';
+
+/**
+ * Internal dependencies
+ */
+import LineHeightControl from '../';
+import { BASE_DEFAULT_VALUE, STEP } from '../utils';
+
+const ControlledLineHeightControl = () => {
+	const [ value, setValue ] = useState();
+	return (
+		<LineHeightControl
+			value={ value }
+			onChange={ setValue }
+			__nextHasNoMarginBottom={ true }
+		/>
+	);
+};
+
+describe( 'LineHeightControl', () => {
+	it( 'should immediately step up from the default value if up-arrowed from an unset state', () => {
+		render( <ControlledLineHeightControl /> );
+		const input = screen.getByRole( 'spinbutton' );
+		input.focus();
+		fireEvent.keyDown( input, { keyCode: UP } );
+		expect( input ).toHaveValue( BASE_DEFAULT_VALUE + STEP );
+	} );
+
+	it( 'should immediately step down from the default value if down-arrowed from an unset state', () => {
+		render( <ControlledLineHeightControl /> );
+		const input = screen.getByRole( 'spinbutton' );
+		input.focus();
+		fireEvent.keyDown( input, { keyCode: DOWN } );
+		expect( input ).toHaveValue( BASE_DEFAULT_VALUE - STEP );
+	} );
+
+	it( 'should immediately step up from the default value if spin button up was clicked from an unset state', () => {
+		render( <ControlledLineHeightControl /> );
+		const input = screen.getByRole( 'spinbutton' );
+		input.focus();
+		fireEvent.change( input, { target: { value: 0.1 } } ); // simulates click on spin button up
+		expect( input ).toHaveValue( BASE_DEFAULT_VALUE + STEP );
+	} );
+
+	it( 'should immediately step down from the default value if spin button down was clicked from an unset state', () => {
+		render( <ControlledLineHeightControl /> );
+		const input = screen.getByRole( 'spinbutton' );
+		input.focus();
+		fireEvent.change( input, { target: { value: 0 } } ); // simulates click on spin button down
+		expect( input ).toHaveValue( BASE_DEFAULT_VALUE - STEP );
+	} );
+} );

packages/block-editor/src/hooks/line-height.js

@@ -38,6 +38,8 @@ export function LineHeightEdit( props ) {
 	};
 	return (
 		<LineHeightControl
+			__unstableInputWidth="100%"
+			__nextHasNoMarginBottom={ true }
 			value={ style?.typography?.lineHeight }
 			onChange={ onChange }
 		/>

packages/block-editor/src/hooks/typography.scss

@@ -6,10 +6,6 @@
 		margin-bottom: 0;
 	}
 
-	.block-editor-line-height-control input {
-		max-width: 100%;
-	}
-
 	.single-column {
 		grid-column: span 1;
 	}

packages/block-editor/src/style.scss

@@ -36,7 +36,6 @@
 @import "./components/inner-blocks/style.scss";
 @import "./components/inserter-list-item/style.scss";
 @import "./components/justify-content-control/style.scss";
-@import "./components/line-height-control/style.scss";
 @import "./components/link-control/style.scss";
 @import "./components/list-view/style.scss";
 @import "./components/media-replace-flow/style.scss";

packages/components/src/input-control/reducer/reducer.ts

@@ -50,6 +50,9 @@ export const composeStateReducers = (
 ): StateReducer => {
 	return ( ...args ) => {
 		return fns.reduceRight( ( state, fn ) => {
+			// TODO: Assess whether this can be replaced with a more standard `compose` implementation
+			// like wp.data.compose() (aka lodash flowRight) or Redux compose().
+			// The current implementation only works by functions mutating the original state object.
 			const fnState = fn( ...args );
 			return isEmpty( fnState ) ? state : { ...state, ...fnState };
 		}, {} as InputState );

packages/components/src/tools-panel/stories/typography-panel.js

@@ -161,6 +161,8 @@ export const TypographyPanel = () => {
 							<LineHeightControl
 								value={ lineHeight }
 								onChange={ setLineHeight }
+								__unstableInputWidth="100%"
+								__nextHasNoMarginBottom={ true }
 							/>
 						</ToolsPanelItem>
 

packages/edit-site/src/components/global-styles/typography-panel.js

@@ -7,7 +7,11 @@ import {
 	__experimentalFontAppearanceControl as FontAppearanceControl,
 	__experimentalLetterSpacingControl as LetterSpacingControl,
 } from '@wordpress/block-editor';
-import { PanelBody, FontSizePicker } from '@wordpress/components';
+import {
+	PanelBody,
+	FontSizePicker,
+	__experimentalSpacer as Spacer,
+} from '@wordpress/components';
 
 /**
  * Internal dependencies
@@ -143,10 +147,13 @@ export default function TypographyPanel( { name, element } ) {
 				/>
 			) }
 			{ hasLineHeightEnabled && (
-				<LineHeightControl
-					value={ lineHeight }
-					onChange={ setLineHeight }
-				/>
+				<Spacer marginBottom={ 6 }>
+					<LineHeightControl
+						__nextHasNoMarginBottom={ true }
+						value={ lineHeight }
+						onChange={ setLineHeight }
+					/>
+				</Spacer>
 			) }
 			{ hasAppearanceControl && (
 				<FontAppearanceControl