BoxControl: Prevent invalid style values

packages/components/src/box-control/all-input-control.js

@@ -6,7 +6,14 @@ import { noop } from 'lodash';
  * Internal dependencies
  */
 import UnitControl from './unit-control';
-import { LABELS, getAllValue, isValuesMixed, isValuesDefined } from './utils';
+import {
+	ALL_SIDES,
+	LABELS,
+	getAllValue,
+	getAllUnitFallback,
+	isValuesMixed,
+	isValuesDefined,
+} from './utils';
 
 export default function AllInputControl( {
 	onChange = noop,
@@ -15,42 +22,64 @@ export default function AllInputControl( {
 	onHoverOff = noop,
 	values,
 	sides,
+	selectedUnits,
+	setSelectedUnits,
 	...props
 } ) {
-	const allValue = getAllValue( values );
+	const allValue = getAllValue( values, sides );
 	const hasValues = isValuesDefined( values );
-	const isMixed = hasValues && isValuesMixed( values );
-
+	const isMixed = hasValues && isValuesMixed( values, sides );
 	const allPlaceholder = isMixed ? LABELS.mixed : null;
 
+	// Set meaningful unit selection if no allValue and user has previously
+	// selected units without assigning values while controls were unlinked.
+	const allUnitFallback = ! allValue
+		? getAllUnitFallback( selectedUnits )
+		: undefined;
+
 	const handleOnFocus = ( event ) => {
 		onFocus( event, { side: 'all' } );
 	};
 
-	const handleOnChange = ( next ) => {
-		const nextValues = { ...values };
+	// Applies a value to an object representing top, right, bottom and left
+	// sides while taking into account any custom side configuration.
+	const applyValueToSides = ( currentValues, newValue ) => {
+		const newValues = { ...currentValues };
 
 		if ( sides?.length ) {
 			sides.forEach( ( side ) => {
 				if ( side === 'vertical' ) {
-					nextValues.top = next;
-					nextValues.bottom = next;
+					newValues.top = newValue;
+					newValues.bottom = newValue;
 				} else if ( side === 'horizontal' ) {
-					nextValues.left = next;
-					nextValues.right = next;
+					newValues.left = newValue;
+					newValues.right = newValue;
 				} else {
-					nextValues[ side ] = next;
+					newValues[ side ] = newValue;
 				}
 			} );
 		} else {
-			[ 'top', 'right', 'bottom', 'left' ].forEach(
-				( side ) => ( nextValues[ side ] = next )
-			);
+			ALL_SIDES.forEach( ( side ) => ( newValues[ side ] = newValue ) );
 		}
 
+		return newValues;
+	};
+
+	const handleOnChange = ( next ) => {
+		const isNumeric = ! isNaN( parseFloat( next ) );
+		const nextValue = isNumeric ? next : undefined;
+		const nextValues = applyValueToSides( values, nextValue );
+
 		onChange( nextValues );
 	};
 
+	// Set selected unit so it can be used as fallback by unlinked controls
+	// when individual sides do not have a value containing a unit.
+	const handleOnUnitChange = ( unit ) => {
+		const newUnits = applyValueToSides( selectedUnits, unit );
+		setSelectedUnits( newUnits );
+	};
+
 	const handleOnHoverOn = () => {
 		onHoverOn( {
 			top: true,
@@ -75,7 +104,9 @@ export default function AllInputControl( {
 			disableUnits={ isMixed }
 			isOnly
 			value={ allValue }
+			unit={ allUnitFallback }
 			onChange={ handleOnChange }
+			onUnitChange={ handleOnUnitChange }
 			onFocus={ handleOnFocus }
 			onHoverOn={ handleOnHoverOn }
 			onHoverOff={ handleOnHoverOff }

packages/components/src/box-control/axial-input-controls.js

@@ -13,6 +13,8 @@ export default function AxialInputControls( {
 	onHoverOn,
 	onHoverOff,
 	values,
+	selectedUnits,
+	setSelectedUnits,
 	sides,
 	...props
 } ) {
@@ -64,19 +66,38 @@ export default function AxialInputControls( {
 			return;
 		}
 		const nextValues = { ...values };
+		const isNumeric = ! isNaN( parseFloat( next ) );
+		const nextValue = isNumeric ? next : undefined;
 
 		if ( side === 'vertical' ) {
-			nextValues.top = next;
-			nextValues.bottom = next;
+			nextValues.top = nextValue;
+			nextValues.bottom = nextValue;
 		}
+
 		if ( side === 'horizontal' ) {
-			nextValues.left = next;
-			nextValues.right = next;
+			nextValues.left = nextValue;
+			nextValues.right = nextValue;
 		}
 
 		onChange( nextValues );
 	};
 
+	const createHandleOnUnitChange = ( side ) => ( next ) => {
+		const newUnits = { ...selectedUnits };
+
+		if ( side === 'vertical' ) {
+			newUnits.top = next;
+			newUnits.bottom = next;
+		}
+
+		if ( side === 'horizontal' ) {
+			newUnits.left = next;
+			newUnits.right = next;
+		}
+
+		setSelectedUnits( newUnits );
+	};
+
 	// Filter sides if custom configuration provided, maintaining default order.
 	const filteredSides = sides?.length
 		? groupedSides.filter( ( side ) => sides.includes( side ) )
@@ -98,8 +119,14 @@ export default function AxialInputControls( {
 					isFirst={ first === side }
 					isLast={ last === side }
 					isOnly={ only === side }
-					value={ 'vertical' === side ? values.top : values.left }
+					value={ side === 'vertical' ? values.top : values.left }
+					unit={
+						side === 'vertical'
+							? selectedUnits.top
+							: selectedUnits.left
+					}
 					onChange={ createHandleOnChange( side ) }
+					onUnitChange={ createHandleOnUnitChange( side ) }
 					onFocus={ createHandleOnFocus( side ) }
 					onHoverOn={ createHandleOnHoverOn( side ) }
 					onHoverOff={ createHandleOnHoverOff( side ) }

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

@@ -27,6 +27,7 @@ import {
 	Header,
 	HeaderControlWrapper,
 } from './styles/box-control-styles';
+import { parseUnit } from '../unit-control/utils';
 import {
 	DEFAULT_VALUES,
 	DEFAULT_VISUALIZER_VALUES,
@@ -74,6 +75,16 @@ export default function BoxControl( {
 		getInitialSide( isLinked, splitOnAxis )
 	);
 
+	// Tracking selected units via internal state allows filtering of CSS unit
+	// only values from being saved while maintaining preexisting unit selection
+	// behaviour. Filtering CSS only values prevents invalid style values.
+	const [ selectedUnits, setSelectedUnits ] = useState( {
+		top: parseUnit( valuesProp?.top )[ 1 ],
+		right: parseUnit( valuesProp?.right )[ 1 ],
+		bottom: parseUnit( valuesProp?.bottom )[ 1 ],
+		left: parseUnit( valuesProp?.left )[ 1 ],
+	} );
+
 	const id = useUniqueId( idProp );
 	const headingId = `${ id }-heading`;
 
@@ -103,6 +114,7 @@ export default function BoxControl( {
 	const handleOnReset = () => {
 		onChange( resetValues );
 		setValues( resetValues );
+		setSelectedUnits( resetValues );
 		setIsDirty( false );
 	};
 
@@ -114,6 +126,8 @@ export default function BoxControl( {
 		onHoverOff: handleOnHoverOff,
 		isLinked,
 		units,
+		selectedUnits,
+		setSelectedUnits,
 		sides,
 		values: inputValues,
 	};

packages/components/src/box-control/input-controls.js

@@ -7,17 +7,17 @@ import { noop } from 'lodash';
  * Internal dependencies
  */
 import UnitControl from './unit-control';
-import { LABELS } from './utils';
+import { ALL_SIDES, LABELS } from './utils';
 import { LayoutContainer, Layout } from './styles/box-control-styles';
 
-const allSides = [ 'top', 'right', 'bottom', 'left' ];
-
 export default function BoxInputControls( {
 	onChange = noop,
 	onFocus = noop,
 	onHoverOn = noop,
 	onHoverOff = noop,
 	values,
+	selectedUnits,
+	setSelectedUnits,
 	sides,
 	...props
 } ) {
@@ -40,8 +40,10 @@ export default function BoxInputControls( {
 	const createHandleOnChange = ( side ) => ( next, { event } ) => {
 		const { altKey } = event;
 		const nextValues = { ...values };
+		const isNumeric = ! isNaN( parseFloat( next ) );
+		const nextValue = isNumeric ? next : undefined;
 
-		nextValues[ side ] = next;
+		nextValues[ side ] = nextValue;
 
 		/**
 		 * Supports changing pair sides. For example, holding the ALT key
@@ -50,27 +52,33 @@ export default function BoxInputControls( {
 		if ( altKey ) {
 			switch ( side ) {
 				case 'top':
-					nextValues.bottom = next;
+					nextValues.bottom = nextValue;
 					break;
 				case 'bottom':
-					nextValues.top = next;
+					nextValues.top = nextValue;
 					break;
 				case 'left':
-					nextValues.right = next;
+					nextValues.right = nextValue;
 					break;
 				case 'right':
-					nextValues.left = next;
+					nextValues.left = nextValue;
 					break;
 			}
 		}
 
 		handleOnChange( nextValues );
 	};
 
+	const createHandleOnUnitChange = ( side ) => ( next ) => {
+		const newUnits = { ...selectedUnits };
+		newUnits[ side ] = next;
+		setSelectedUnits( newUnits );
+	};
+
 	// Filter sides if custom configuration provided, maintaining default order.
 	const filteredSides = sides?.length
-		? allSides.filter( ( side ) => sides.includes( side ) )
-		: allSides;
+		? ALL_SIDES.filter( ( side ) => sides.includes( side ) )
+		: ALL_SIDES;
 
 	const first = filteredSides[ 0 ];
 	const last = filteredSides[ filteredSides.length - 1 ];
@@ -90,7 +98,11 @@ export default function BoxInputControls( {
 						isLast={ last === side }
 						isOnly={ only === side }
 						value={ values[ side ] }
+						unit={
+							values[ side ] ? undefined : selectedUnits[ side ]
+						}
 						onChange={ createHandleOnChange( side ) }
+						onUnitChange={ createHandleOnUnitChange( side ) }
 						onFocus={ createHandleOnFocus( side ) }
 						onHoverOn={ createHandleOnHoverOn( side ) }
 						onHoverOff={ createHandleOnHoverOff( side ) }

packages/components/src/box-control/test/index.js

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { render, fireEvent } from '@testing-library/react';
+import { render, fireEvent, screen } from '@testing-library/react';
 
 /**
  * WordPress dependencies
@@ -192,4 +192,91 @@ describe( 'BoxControl', () => {
 			} );
 		} );
 	} );
+
+	describe( 'Unit selections', () => {
+		it( 'should update unlinked controls unit selection based on all input control', () => {
+			// Render control
+			render( <BoxControl /> );
+
+			// Make unit selection on all input control
+			const allUnitSelect = screen.getByLabelText( 'Select unit' );
+			allUnitSelect.focus();
+			fireEvent.change( allUnitSelect, { target: { value: 'em' } } );
+
+			// Unlink the controls
+			const unlink = screen.getByLabelText( /Unlink Sides/ );
+			fireEvent.click( unlink );
+
+			// Confirm that each individual control has the selected unit
+			const unlinkedSelects = screen.getAllByDisplayValue( 'em' );
+			expect( unlinkedSelects.length ).toEqual( 4 );
+		} );
+
+		it( 'should use individual side attribute unit when available', () => {
+			// Render control
+			const { rerender } = render( <BoxControl /> );
+
+			// Make unit selection on all input control
+			const allUnitSelect = screen.getByLabelText( 'Select unit' );
+			allUnitSelect.focus();
+			fireEvent.change( allUnitSelect, { target: { value: 'vw' } } );
+
+			// Unlink the controls
+			const unlink = screen.getByLabelText( /Unlink Sides/ );
+			fireEvent.click( unlink );
+
+			// Confirm that each individual control has the selected unit
+			const unlinkedSelects = screen.getAllByDisplayValue( 'vw' );
+			expect( unlinkedSelects.length ).toEqual( 4 );
+
+			// Rerender with individual side value & confirm unit is selected.
+			rerender( <BoxControl values={ { top: '2.5em' } } /> );
+
+			const topSelect = screen.getByDisplayValue( 'em' );
+			const otherSelects = screen.getAllByDisplayValue( 'vw' );
+
+			expect( topSelect ).toBeInTheDocument();
+			expect( otherSelects.length ).toEqual( 3 );
+		} );
+	} );
+
+	describe( 'onChange updates', () => {
+		it( 'should call onChange when values contain more than just CSS units', () => {
+			const setState = jest.fn();
+
+			render( <BoxControl onChange={ setState } /> );
+
+			const input = screen.getByLabelText( 'Box Control', {
+				selector: 'input',
+			} );
+
+			input.focus();
+			fireEvent.change( input, { target: { value: '7.5rem' } } );
+			fireEvent.keyDown( input, { keyCode: ENTER } );
+
+			expect( setState ).toHaveBeenCalledWith( {
+				top: '7.5rem',
+				right: '7.5rem',
+				bottom: '7.5rem',
+				left: '7.5rem',
+			} );
+		} );
+
+		it( 'should not pass invalid CSS unit only values to onChange', () => {
+			const setState = jest.fn();
+
+			render( <BoxControl onChange={ setState } /> );
+
+			const allUnitSelect = screen.getByLabelText( 'Select unit' );
+			allUnitSelect.focus();
+			fireEvent.change( allUnitSelect, { target: { value: 'rem' } } );
+
+			expect( setState ).toHaveBeenCalledWith( {
+				top: undefined,
+				right: undefined,
+				bottom: undefined,
+				left: undefined,
+			} );
+		} );
+	} );
 } );