Fix: Page Attributes: Order field does not allow negative numbers (#9803

)
WordPress · Sep 18, 2018 · bd5910b · bd5910b
1 parent f9ea5be
commit bd5910b
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 32 deletions.
diff --git a/packages/editor/src/components/page-attributes/order.js b/packages/editor/src/components/page-attributes/order.js
@@ -1,41 +1,52 @@
+/**
+ * External dependencies
+ */
+import { invoke } from 'lodash';
+
 /**
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
-import { Fragment } from '@wordpress/element';
+import { TextControl } from '@wordpress/components';
 import { withSelect, withDispatch } from '@wordpress/data';
-import { compose, withInstanceId } from '@wordpress/compose';
+import { compose, withState } from '@wordpress/compose';
 
 /**
  * Internal dependencies
  */
 import PostTypeSupportCheck from '../post-type-support-check';
 
-export function PageAttributesOrder( { onUpdateOrder, instanceId, order } ) {
-	const setUpdatedOrder = ( event ) => {
-		const newOrder = Number( event.target.value );
-		if ( newOrder >= 0 ) {
-			onUpdateOrder( newOrder );
-		}
-	};
-	// Create unique identifier for inputs
-	const inputId = `editor-page-attributes__order-${ instanceId }`;
-
-	return (
-		<Fragment>
-			<label htmlFor={ inputId }>
-				{ __( 'Order' ) }
-			</label>
-			<input
-				type="text"
-				value={ order || 0 }
+export const PageAttributesOrder = withState( {
+	orderInput: null,
+} )(
+	( { onUpdateOrder, order = 0, orderInput, setState } ) => {
+		const setUpdatedOrder = ( value ) => {
+			setState( {
+				orderInput: value,
+			} );
+			const newOrder = Number( value );
+			if ( Number.isInteger( newOrder ) && invoke( value, [ 'trim' ] ) !== '' ) {
+				onUpdateOrder( Number( value ) );
+			}
+		};
+		const value = orderInput === null ? order : orderInput;
+		return (
+			<TextControl
+				className="editor-page-attributes__order"
+				type="number"
+				label={ __( 'Order' ) }
+				value={ value }
 				onChange={ setUpdatedOrder }
-				id={ inputId }
 				size={ 6 }
+				onBlur={ () => {
+					setState( {
+						orderInput: null,
+					} );
+				} }
 			/>
-		</Fragment>
-	);
-}
+		);
+	}
+);
 
 function PageAttributesOrderWithChecks( props ) {
 	return (
@@ -58,5 +69,4 @@ export default compose( [
 			} );
 		},
 	} ) ),
-	withInstanceId,
 ] )( PageAttributesOrderWithChecks );
diff --git a/packages/editor/src/components/page-attributes/style.scss b/packages/editor/src/components/page-attributes/style.scss
@@ -5,3 +5,16 @@
 	}
 	margin-bottom: 10px;
 }
+
+.editor-page-attributes__order {
+	width: 100%;
+	.components-base-control__field {
+		display: flex;
+		justify-content: space-between;
+		align-items: center;
+	}
+
+	input {
+		width: 66px;
+	}
+}
diff --git a/packages/editor/src/components/page-attributes/test/order.js b/packages/editor/src/components/page-attributes/test/order.js
@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { shallow } from 'enzyme';
+import { mount } from 'enzyme';
 
 /**
  * Internal dependencies
@@ -11,37 +11,91 @@ import { PageAttributesOrder } from '../order';
 describe( 'PageAttributesOrder', () => {
 	it( 'should reject invalid input', () => {
 		const onUpdateOrder = jest.fn();
-		const wrapper = shallow(
+		const wrapper = mount(
 			<PageAttributesOrder onUpdateOrder={ onUpdateOrder } />
 		);
 
 		wrapper.find( 'input' ).simulate( 'change', {
 			target: {
-				value: -1,
+				value: 'bad',
 			},
 		} );
 
 		wrapper.find( 'input' ).simulate( 'change', {
 			target: {
-				value: 'bad',
+				value: '----+++',
+			},
+		} );
+
+		wrapper.find( 'input' ).simulate( 'change', {
+			target: {
+				value: '-',
+			},
+		} );
+
+		wrapper.find( 'input' ).simulate( 'change', {
+			target: {
+				value: '+',
+			},
+		} );
+
+		wrapper.find( 'input' ).simulate( 'change', {
+			target: {
+				value: '',
+			},
+		} );
+
+		wrapper.find( 'input' ).simulate( 'change', {
+			target: {
+				value: ' ',
 			},
 		} );
 
 		expect( onUpdateOrder ).not.toHaveBeenCalled();
 	} );
 
-	it( 'should update with valid input', () => {
+	it( 'should update with zero input', () => {
 		const onUpdateOrder = jest.fn();
-		const wrapper = shallow(
+		const wrapper = mount(
 			<PageAttributesOrder onUpdateOrder={ onUpdateOrder } />
 		);
 
 		wrapper.find( 'input' ).simulate( 'change', {
 			target: {
-				value: 4,
+				value: 0,
+			},
+		} );
+
+		expect( onUpdateOrder ).toHaveBeenCalledWith( 0 );
+	} );
+
+	it( 'should update with valid positive input', () => {
+		const onUpdateOrder = jest.fn();
+		const wrapper = mount(
+			<PageAttributesOrder onUpdateOrder={ onUpdateOrder } />
+		);
+
+		wrapper.find( 'input' ).simulate( 'change', {
+			target: {
+				value: '4',
 			},
 		} );
 
 		expect( onUpdateOrder ).toHaveBeenCalledWith( 4 );
 	} );
+
+	it( 'should update with valid negative input', () => {
+		const onUpdateOrder = jest.fn();
+		const wrapper = mount(
+			<PageAttributesOrder onUpdateOrder={ onUpdateOrder } />
+		);
+
+		wrapper.find( 'input' ).simulate( 'change', {
+			target: {
+				value: '-1',
+			},
+		} );
+
+		expect( onUpdateOrder ).toHaveBeenCalledWith( -1 );
+	} );
 } );