Fix: input control drag and box control change

packages/components/src/input-control/input-field.js

@@ -7,7 +7,7 @@ import { useDrag } from 'react-use-gesture';
 /**
  * WordPress dependencies
  */
-import { forwardRef } from '@wordpress/element';
+import { forwardRef, useRef } from '@wordpress/element';
 import { UP, DOWN, ENTER } from '@wordpress/keycodes';
 /**
  * Internal dependencies
@@ -66,22 +66,26 @@ function InputField(
 	} );
 
 	const { _event, value, isDragging, isDirty } = state;
+	const wasDirtyOnBlur = useRef( false );
 
 	const dragCursor = useDragCursor( isDragging, dragDirection );
 
 	/*
-	 * Syncs value state using the focus state to determine the direction.
-	 * Without focus it updates the value from the props. With focus it
-	 * propagates the value and event through onChange.
+	 * Handles syncronization of external and internal value state.
+	 * If not focused and did not hold a dirty value[1] on blur
+	 * updates the value from the props. Otherwise if not holding
+	 * a dirty value[1] propagates the value and event through onChange.
+	 * [1] value is only made dirty if isPressEnterToChange is true
 	 */
 	useUpdateEffect( () => {
 		if ( valueProp === value ) {
 			return;
 		}
-		if ( ! isFocused ) {
+		if ( ! isFocused && ! wasDirtyOnBlur.current ) {
 			update( valueProp );
 		} else if ( ! isDirty ) {
 			onChange( value, { event: _event } );
+			wasDirtyOnBlur.current = false;
 		}
 	}, [ value, isDirty, isFocused, valueProp ] );
 
@@ -94,27 +98,15 @@ function InputField(
 		 * the onChange callback.
 		 */
 		if ( isPressEnterToChange && isDirty ) {
+			wasDirtyOnBlur.current = true;
 			if ( ! isValueEmpty( value ) ) {
-				handleOnCommit( { target: { value } }, event );
+				handleOnCommit( event );
 			} else {
 				reset( valueProp );
 			}
 		}
 	};
 
-	/*
-	 * Works around the odd UA (e.g. Firefox) that does not focus inputs of
-	 * type=number when their spinner arrows are pressed.
-	 */
-	let handleOnMouseDown;
-	if ( type === 'number' ) {
-		handleOnMouseDown = ( event ) => {
-			if ( event.target !== event.target.ownerDocument.activeElement ) {
-				event.target.focus();
-			}
-		};
-	}
-
 	const handleOnFocus = ( event ) => {
 		onFocus( event );
 		setIsFocused( true );
@@ -129,10 +121,10 @@ function InputField(
 		const nextValue = event.target.value;
 
 		try {
-			onValidate( nextValue, { event } );
+			onValidate( nextValue, event );
 			commit( nextValue, event );
 		} catch ( err ) {
-			invalidate( err, { event } );
+			invalidate( err, event );
 		}
 	};
 
@@ -164,7 +156,6 @@ function InputField(
 		( dragProps ) => {
 			const { distance, dragging, event } = dragProps;
 
-			if ( ! isDragEnabled ) return;
 			if ( ! distance ) return;
 			event.stopPropagation();
 
@@ -192,10 +183,29 @@ function InputField(
 		}
 	);
 
+	const { onMouseDown, onTouchStart } = isDragEnabled
+		? dragGestureProps()
+		: {};
+	let handleOnMouseDown = onMouseDown;
+
+	/*
+	 * Works around the odd UA (e.g. Firefox) that does not focus inputs of
+	 * type=number when their spinner arrows are pressed.
+	 */
+	if ( type === 'number' ) {
+		handleOnMouseDown = ( event ) => {
+			if ( event.target !== event.target.ownerDocument.activeElement ) {
+				event.target.focus();
+			}
+			if ( isDragEnabled ) {
+				onMouseDown( event );
+			}
+		};
+	}
+
 	return (
 		<Input
 			{ ...props }
-			{ ...dragGestureProps() }
 			className="components-input-control__input"
 			disabled={ disabled }
 			dragCursor={ dragCursor }
@@ -206,6 +216,7 @@ function InputField(
 			onFocus={ handleOnFocus }
 			onKeyDown={ handleOnKeyDown }
 			onMouseDown={ handleOnMouseDown }
+			onTouchStart={ onTouchStart }
 			ref={ ref }
 			size={ size }
 			value={ value }

packages/components/src/input-control/state.js

@@ -131,10 +131,8 @@ function inputControlStateReducer( composedStateReducers ) {
 				break;
 
 			case actionTypes.UPDATE:
-				if ( payload.value !== state.value ) {
-					nextState.value = payload.value;
-					nextState.isDirty = false;
-				}
+				nextState.value = payload.value;
+				nextState.isDirty = false;
 				break;
 
 			/**
@@ -218,7 +216,7 @@ export function useInputControlStateReducer(
 	 * Actions for the reducer
 	 */
 	const change = createChangeEvent( actionTypes.CHANGE );
-	const inValidate = createChangeEvent( actionTypes.INVALIDATE );
+	const invalidate = createChangeEvent( actionTypes.INVALIDATE );
 	const reset = createChangeEvent( actionTypes.RESET );
 	const commit = createChangeEvent( actionTypes.COMMIT );
 	const update = createChangeEvent( actionTypes.UPDATE );
@@ -238,7 +236,7 @@ export function useInputControlStateReducer(
 		drag,
 		dragEnd,
 		dragStart,
-		inValidate,
+		invalidate,
 		pressDown,
 		pressEnter,
 		pressUp,

packages/components/src/number-control/index.js

@@ -150,11 +150,11 @@ export function NumberControl(
 		}
 
 		/**
-		 * Handles ENTER key press and submit
+		 * Handles commit (ENTER key press or on blur if isPressEnterToChange)
 		 */
 		if (
 			type === inputControlActionTypes.PRESS_ENTER ||
-			type === inputControlActionTypes.SUBMIT
+			type === inputControlActionTypes.COMMIT
 		) {
 			state.value = roundClamp( currentValue, min, max );
 		}

packages/components/src/unit-control/index.js

@@ -8,6 +8,7 @@ import classnames from 'classnames';
  * WordPress dependencies
  */
 import { forwardRef, useRef } from '@wordpress/element';
+import { ENTER } from '@wordpress/keycodes';
 
 /**
  * Internal dependencies
@@ -48,11 +49,8 @@ function UnitControl(
 		initial: initialUnit,
 	} );
 
-	/*
-	 * Storing parsed unit changes to be applied during the onChange callback
-	 * cycle.
-	 */
-	const nextParsedUnitRef = useRef();
+	// Stores parsed value for hand-off in state reducer
+	const refParsedValue = useRef( null );
 
 	const classes = classnames( 'components-unit-control', className );
 
@@ -66,30 +64,9 @@ function UnitControl(
 		 * Customizing the onChange callback.
 		 * This allows as to broadcast a combined value+unit to onChange.
 		 */
-		const [ parsedValue, parsedUnit ] = getValidParsedUnit(
-			next,
-			units,
-			value,
-			unit
-		);
-		const nextParsedUnit = nextParsedUnitRef.current;
+		next = getValidParsedUnit( next, units, value, unit ).join( '' );
 
-		/*
-		 * If we've noticed a (parsed) unit change, which would have been
-		 * stored in nextParsedUnitRef, we'll update our local unit set,
-		 * as well as fire the onUnitChange callback.
-		 */
-		if ( nextParsedUnit ) {
-			onUnitChange( nextParsedUnit, changeProps );
-			setUnit( nextParsedUnit );
-			// We have to reset this ref value to properly track new changes.
-			nextParsedUnitRef.current = null;
-		}
-
-		const nextUnit = nextParsedUnit || parsedUnit;
-		const nextValue = `${ parsedValue }${ nextUnit }`;
-
-		onChange( nextValue, changeProps );
+		onChange( next, changeProps );
 	};
 
 	const handleOnUnitChange = ( next, changeProps ) => {
@@ -107,6 +84,42 @@ function UnitControl(
 		setUnit( next );
 	};
 
+	const mayUpdateUnit = ( event ) => {
+		if ( ! isNaN( event.target.value ) ) {
+			refParsedValue.current = null;
+			return;
+		}
+		const [ parsedValue, parsedUnit ] = getValidParsedUnit(
+			event.target.value,
+			units,
+			value,
+			unit
+		);
+
+		refParsedValue.current = parsedValue;
+
+		if ( isPressEnterToChange && parsedUnit !== unit ) {
+			const data = units.find(
+				( option ) => option.value === parsedUnit
+			);
+			const changeProps = { event, data };
+
+			onChange( `${ parsedValue }${ parsedUnit }`, changeProps );
+			onUnitChange( parsedUnit, changeProps );
+
+			setUnit( parsedUnit );
+		}
+	};
+
+	const handleOnBlur = mayUpdateUnit;
+
+	const handleOnKeyDown = ( event ) => {
+		const { keyCode } = event;
+		if ( keyCode === ENTER ) {
+			mayUpdateUnit( event );
+		}
+	};
+
 	/**
 	 * "Middleware" function that intercepts updates from InputControl.
 	 * This allows us to tap into actions to transform the (next) state for
@@ -117,39 +130,15 @@ function UnitControl(
 	 * @return {Object} The updated state to apply to InputControl
 	 */
 	const unitControlStateReducer = ( state, action ) => {
-		const { type, payload } = action;
-		const event = payload?.event;
-
 		/*
-		 * Customizes the commit interaction.
-		 *
-		 * This occurs when pressing ENTER to fire a change.
-		 * By intercepting the state change, we can parse the incoming
-		 * value to determine if the unit needs to be updated.
+		 * On commits (when pressing ENTER and on blur if
+		 * isPressEnterToChange is true), if a parse has been performed
+		 * then use that result to update the state.
 		 */
-		if ( type === inputControlActionTypes.COMMIT ) {
-			const valueToParse = event?.target?.value;
-
-			const [ parsedValue, parsedUnit ] = getValidParsedUnit(
-				valueToParse,
-				units,
-				value,
-				unit
-			);
-
-			state.value = parsedValue;
-
-			// Update unit if the incoming parsed unit is different.
-			if ( unit !== parsedUnit ) {
-				/*
-				 * We start by storing the next parsedUnit value in our
-				 * nextParsedUnitRef. We can't set the unit during this
-				 * stateReducer callback as it cause state update
-				 * conflicts within React's render cycle.
-				 *
-				 * https://github.com/facebook/react/issues/18178
-				 */
-				nextParsedUnitRef.current = parsedUnit;
+		if ( action.type === inputControlActionTypes.COMMIT ) {
+			if ( refParsedValue.current !== null ) {
+				state.value = refParsedValue.current;
+				refParsedValue.current = null;
 			}
 		}
 
@@ -179,6 +168,8 @@ function UnitControl(
 				disableUnits={ disableUnits }
 				isPressEnterToChange={ isPressEnterToChange }
 				label={ label }
+				onBlur={ handleOnBlur }
+				onKeyDown={ handleOnKeyDown }
 				onChange={ handleOnChange }
 				ref={ ref }
 				size={ size }