Simplify controlled/uncontrolled logic

WordPress · Jun 28, 2024 · fbf1458 · fbf1458
1 parent 58b60be
commit fbf1458
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 85 deletions.
diff --git a/packages/components/src/date-time/time-input/index.tsx b/packages/components/src/date-time/time-input/index.tsx
@@ -7,7 +7,6 @@ import clsx from 'clsx';
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
-import { useState, useEffect, useRef, useMemo } from '@wordpress/element';
 
 /**
  * Internal dependencies
@@ -29,64 +28,48 @@ import {
 } from '../utils';
 import type { TimeInputProps } from '../types';
 import type { InputChangeCallback } from '../../input-control/types';
+import { useControlledValue } from '../../utils';
 
 export function TimeInput( {
-	value: entryValue,
+	value: valueProp,
+	defaultValue,
 	is12Hour,
 	minutesProps,
 	onChange,
 }: TimeInputProps ) {
-	const value = useMemo(
-		() =>
-			entryValue || {
-				hours: new Date().getHours(),
-				minutes: new Date().getMinutes(),
-			},
-		[ entryValue ]
-	);
-	const [ hours, setHours ] = useState( value.hours );
-	const [ hours12Format, setHours12Format ] = useState(
-		from24hTo12h( value.hours )
-	);
-	const [ minutes, setMinutes ] = useState( value.minutes );
-	const [ dayPeriod, setDayPeriod ] = useState(
-		parseDayPeriod( value.hours )
-	);
-
-	const prevValues = useRef( value );
+	const [
+		value = {
+			hours: new Date().getHours(),
+			minutes: new Date().getMinutes(),
+		},
+		setValue,
+	] = useControlledValue( {
+		value: valueProp,
+		onChange,
+		defaultValue,
+	} );
+	const dayPeriod = parseDayPeriod( value.hours );
+	const hours12Format = from24hTo12h( value.hours );
 
 	const buildNumberControlChangeCallback = (
 		method: 'hours' | 'minutes'
-	) => {
-		const callback: InputChangeCallback = ( _value, { event } ) => {
+	): InputChangeCallback => {
+		return ( _value, { event } ) => {
 			if ( ! validateInputElementTarget( event ) ) {
 				return;
 			}
 
 			// We can safely assume value is a number if target is valid.
 			const numberValue = Number( _value );
 
-			switch ( method ) {
-				case 'hours':
-					if ( is12Hour ) {
-						setHours(
-							from12hTo24h( numberValue, dayPeriod === 'PM' )
-						);
-						setHours12Format( numberValue );
-					} else {
-						setHours( numberValue );
-						setHours12Format( from24hTo12h( numberValue ) );
-						setDayPeriod( parseDayPeriod( numberValue ) );
-					}
-					break;
-
-				case 'minutes':
-					setMinutes( numberValue );
-					break;
-			}
+			setValue( {
+				...value,
+				[ method ]:
+					method === 'hours' && is12Hour
+						? from12hTo24h( numberValue, dayPeriod === 'PM' )
+						: numberValue,
+			} );
 		};
-
-		return callback;
 	};
 
 	const buildAmPmChangeCallback = ( _value: 'AM' | 'PM' ) => {
@@ -95,36 +78,17 @@ export function TimeInput( {
 				return;
 			}
 
-			setDayPeriod( _value );
-			setHours( from12hTo24h( hours12Format, _value === 'PM' ) );
+			setValue( {
+				...value,
+				hours: from12hTo24h( hours12Format, _value === 'PM' ),
+			} );
 		};
 	};
 
 	function parseDayPeriod( _hours: number ) {
 		return _hours < 12 ? 'AM' : 'PM';
 	}
 
-	useEffect( () => {
-		setHours( value.hours );
-		setMinutes( value.minutes );
-
-		setDayPeriod( parseDayPeriod( value.hours ) );
-		setHours12Format( from24hTo12h( value.hours ) );
-	}, [ value ] );
-
-	useEffect( () => {
-		if (
-			( prevValues.current.hours !== hours ||
-				prevValues.current.minutes !== minutes ) &&
-			( entryValue?.hours !== hours || entryValue?.minutes !== minutes )
-		) {
-			onChange?.( { hours, minutes } );
-		}
-
-		prevValues.current.hours = hours;
-		prevValues.current.minutes = minutes;
-	}, [ onChange, entryValue, hours, minutes ] );
-
 	return (
 		<HStack alignment="left">
 			<TimeWrapper
@@ -135,10 +99,9 @@ export function TimeInput( {
 					label={ __( 'Hours' ) }
 					hideLabelFromVision
 					__next40pxDefaultSize
-					value={ String( is12Hour ? hours12Format : hours ).padStart(
-						2,
-						'0'
-					) }
+					value={ String(
+						is12Hour ? hours12Format : value.hours
+					).padStart( 2, '0' ) }
 					step={ 1 }
 					min={ is12Hour ? 1 : 0 }
 					max={ is12Hour ? 12 : 23 }
@@ -164,7 +127,7 @@ export function TimeInput( {
 					label={ __( 'Minutes' ) }
 					hideLabelFromVision
 					__next40pxDefaultSize
-					value={ String( minutes ).padStart( 2, '0' ) }
+					value={ String( value.minutes ).padStart( 2, '0' ) }
 					step={ 1 }
 					min={ 0 }
 					max={ 59 }

diff --git a/packages/components/src/date-time/time-input/test/index.tsx b/packages/components/src/date-time/time-input/test/index.tsx
@@ -17,7 +17,10 @@ describe( 'TimePicker', () => {
 		const onChangeSpy = jest.fn();
 
 		render(
-			<TimeInput value={ timeInputValue } onChange={ onChangeSpy } />
+			<TimeInput
+				defaultValue={ timeInputValue }
+				onChange={ onChangeSpy }
+			/>
 		);
 
 		const hoursInput = screen.getByRole( 'spinbutton', { name: 'Hours' } );
@@ -68,7 +71,7 @@ describe( 'TimePicker', () => {
 		render(
 			<TimeInput
 				is12Hour
-				value={ timeInputValue }
+				defaultValue={ timeInputValue }
 				onChange={ onChangeSpy }
 			/>
 		);
@@ -115,7 +118,7 @@ describe( 'TimePicker', () => {
 
 		render(
 			<TimeInput
-				value={ timeInputValue }
+				defaultValue={ timeInputValue }
 				minutesProps={ { step: 5 } }
 				onChange={ onChangeSpy }
 			/>
@@ -151,4 +154,18 @@ describe( 'TimePicker', () => {
 
 		expect( minutesInput ).toHaveValue( 50 );
 	} );
+
+	it( 'should reflect changes to the value prop', () => {
+		const { rerender } = render(
+			<TimeInput value={ { hours: 0, minutes: 0 } } />
+		);
+		rerender( <TimeInput value={ { hours: 1, minutes: 2 } } /> );
+
+		expect(
+			screen.getByRole( 'spinbutton', { name: 'Hours' } )
+		).toHaveValue( 1 );
+		expect(
+			screen.getByRole( 'spinbutton', { name: 'Minutes' } )
+		).toHaveValue( 2 );
+	} );
 } );
diff --git a/packages/components/src/date-time/time/index.tsx b/packages/components/src/date-time/time/index.tsx
@@ -15,7 +15,7 @@ import { __ } from '@wordpress/i18n';
 import BaseControl from '../../base-control';
 import SelectControl from '../../select-control';
 import TimeZone from './timezone';
-import type { TimeInputChangeArgs, TimePickerProps } from '../types';
+import type { TimeInputValue, TimePickerProps } from '../types';
 import {
 	Wrapper,
 	Fieldset,
@@ -105,7 +105,7 @@ export function TimePicker( {
 	const onTimeInputChangeCallback = ( {
 		hours: newHours,
 		minutes: newMinutes,
-	}: TimeInputChangeArgs ) => {
+	}: TimeInputValue ) => {
 		const newDate = set( date, {
 			hours: newHours,
 			minutes: newMinutes,

diff --git a/packages/components/src/date-time/types.ts b/packages/components/src/date-time/types.ts
@@ -23,9 +23,9 @@ export type TimePickerProps = {
 	onChange?: ( time: string ) => void;
 };
 
-export type TimeInputChangeArgs = {
+export type TimeInputValue = {
 	/**
-	 * The hours value.
+	 * The hours value in 24-hour format.
 	 */
 	hours: number;
 
@@ -34,6 +34,7 @@ export type TimeInputChangeArgs = {
 	 */
 	minutes: number;
 };
+
 export type TimeInputProps = {
 	/**
 	 * Whether we use a 12-hour clock. With a 12-hour clock, an AM/PM widget is
@@ -42,14 +43,18 @@ export type TimeInputProps = {
 	is12Hour?: boolean;
 
 	/**
-	 * The time value with hours and minutes props
-	 * - hours: 24-hours format value
-	 * - minutes: minutes value
+	 * The time input object with hours and minutes values.
+	 *
+	 * - hours: number (24-hour format)
+	 * - minutes: number
 	 */
-	value?: {
-		hours: number;
-		minutes: number;
-	};
+	value?: TimeInputValue;
+
+	/**
+	 * An optional default value for the control when used in uncontrolled mode.
+	 * If left `undefined`, the current time will be used.
+	 */
+	defaultValue?: TimeInputValue;
 
 	/**
 	 * The props to pass down to the minutes input.
@@ -60,7 +65,7 @@ export type TimeInputProps = {
 	 * The function is called when a new time has been selected.
 	 * Passing hours and minutes as an object properties.
 	 */
-	onChange?: ( time: TimeInputChangeArgs ) => void;
+	onChange?: ( time: TimeInputValue ) => void;
 };
 
 export type DatePickerEvent = {