Merge pull request #1007 from woocommerce/fix/1002-1003-1004-verifica…

…tion-code

Fix issues and react warnings for verification code components

js/src/components/app-button/index.js

@@ -49,15 +49,20 @@ const AppButton = ( props ) => {
 		onClick( ...args );
 	};
 
-	const text = [];
 	const classes = [ 'app-button', className ];
+	let text;
 
 	if ( loading ) {
-		text.push( <Spinner /> );
+		text = <Spinner />;
 	}
 
 	if ( passedInText ) {
-		text.push( passedInText );
+		text = (
+			<>
+				{ loading && <Spinner /> }
+				{ passedInText }
+			</>
+		);
 
 		if ( rest.icon ) {
 			classes.push( 'app-button--icon-with-text' );
@@ -71,7 +76,7 @@ const AppButton = ( props ) => {
 		<Button
 			className={ classnames( ...classes ) }
 			disabled={ disabled || loading }
-			text={ text.length ? text : undefined }
+			text={ text }
 			onClick={ handleClick }
 			{ ...rest }
 		/>

js/src/components/contact-information/phone-number-card/verification-code-control.js

@@ -48,6 +48,12 @@ export default function VerificationCodeControl( {
 
 	onCodeChangeRef.current = onCodeChange;
 
+	/**
+	 * Moves focus to the input at given input
+	 * if it exists.
+	 *
+	 * @param {number} targetIdx Index of the node to move the focus to.
+	 */
 	const maybeMoveFocus = ( targetIdx ) => {
 		const node = inputsRef.current[ targetIdx ];
 		if ( node ) {
@@ -56,13 +62,13 @@ export default function VerificationCodeControl( {
 	};
 
 	const handleKeyDown = ( e ) => {
-		const { dataset, selectionStart, value } = e.target;
+		const { dataset, selectionStart, selectionEnd, value } = e.target;
 		const idx = Number( dataset.idx );
 
 		switch ( e.keyCode ) {
 			case KEY_CODE_LEFT:
 			case KEY_CODE_BACKSPACE:
-				if ( selectionStart === 0 ) {
+				if ( selectionStart === 0 && selectionEnd === 0 ) {
 					maybeMoveFocus( idx - 1 );
 				}
 				break;
@@ -75,6 +81,7 @@ export default function VerificationCodeControl( {
 		}
 	};
 
+	// Track the cursor's position.
 	const handleBeforeInput = ( e ) => {
 		cursorRef.current = e.target.selectionStart;
 	};
@@ -103,18 +110,39 @@ export default function VerificationCodeControl( {
 		}
 	};
 
-	useEffect( () => {
-		maybeMoveFocus( 0 );
-	}, [] );
-
+	// Update the inputs' values.
 	useEffect( () => {
 		inputsRef.current.forEach( ( el ) => ( el.value = '' ) );
-		maybeMoveFocus( 0 );
 
 		setDigits( initDigits );
 		onCodeChangeRef.current( toCallbackData( initDigits ) );
 	}, [ resetNeedle ] );
 
+	/**
+	 * Set the focus to the first input if the control's value is (back) at the initial state.
+	 *
+	 * Since the <InputControl> has an internal state management that always controls the actual `value` prop of the <input>,
+	 * the <InputControl> is forced the <input> to be a controlled input.
+	 * When using it, it's always necessary to specify `value` prop from the below <AppInputControl>
+	 * to avoid the warning - A component is changing an uncontrolled input to be controlled.
+	 *
+	 * @see https://github.com/WordPress/gutenberg/blob/%40wordpress/components%4012.0.8/packages/components/src/input-control/input-field.js#L47-L68
+	 * @see https://github.com/WordPress/gutenberg/blob/%40wordpress/components%4012.0.8/packages/components/src/input-control/input-field.js#L115-L118
+	 *
+	 * But after specifying the `value` prop,
+	 * the synchronization of external and internal `value` state will depend on whether the input is focused.
+	 * It'd sync external to internal only if the input is not focused.
+	 * So here we await the `digits` is reset back to `initDigits` by above useEffect and sync to internal value,
+	 * then move the focus calling after the synchronization tick finished.
+	 *
+	 * @see https://github.com/WordPress/gutenberg/blob/%40wordpress/components%4012.0.8/packages/components/src/input-control/input-field.js#L73-L90
+	 */
+	useEffect( () => {
+		if ( digits === initDigits ) {
+			maybeMoveFocus( 0 );
+		}
+	}, [ resetNeedle, digits ] );
+
 	return (
 		<Flex
 			className="gla-verification-code-control"

js/src/components/contact-information/phone-number-card/verify-phone-number-content.js

@@ -14,6 +14,7 @@ import { Notice, Flex } from '@wordpress/components';
 /**
  * Internal dependencies
  */
+import useIsMounted from '.~/hooks/useIsMounted';
 import useCountdown from './useCountdown';
 import Section from '.~/wcdl/section';
 import Subsection from '.~/wcdl/subsection';
@@ -108,6 +109,7 @@ export default function VerifyPhoneNumberContent( {
 	display,
 	onPhoneNumberVerified,
 } ) {
+	const isMounted = useIsMounted();
 	const [ method, setMethod ] = useState( verificationMethod );
 	const { second, callCount, startCountdown } = useCountdown( method );
 	const [ verification, setVerification ] = useState( null );
@@ -134,10 +136,12 @@ export default function VerifyPhoneNumberContent( {
 				verificationIdRef.current[ method ] = id;
 			} )
 			.catch( () => {
-				startCountdown( 0 );
+				if ( isMounted() ) {
+					startCountdown( 0 );
+				}
 				// TODO: [full-contact-info] add error handling.
 			} );
-	}, [ country, number, method, startCountdown ] );
+	}, [ country, number, method, startCountdown, isMounted ] );
 
 	const handleVerifyClick = () => {
 		setError( null );
@@ -150,8 +154,10 @@ export default function VerifyPhoneNumberContent( {
 			} )
 			.catch( ( e ) => {
 				// TODO: [full-contact-info] align to the real error data structure.
-				setError( e );
-				setVerifying( false );
+				if ( isMounted() ) {
+					setError( e );
+					setVerifying( false );
+				}
 			} );
 	};
 

js/src/hooks/useIsMounted.js

@@ -0,0 +1,21 @@
+/**
+ * External dependencies
+ */
+import { useEffect, useCallback, useRef } from '@wordpress/element';
+
+/**
+ * Returns a function to check whether the caller component is mounted or unmounted.
+ * Usually, it's used to avoid the warning - Can't perform a React state update on an unmounted component.
+ *
+ * @return {() => boolean} A function returns a boolean that indicates the caller component is mounted or unmounted.
+ */
+export default function useIsMounted() {
+	const mountedRef = useRef( false );
+	useEffect( () => {
+		mountedRef.current = true;
+		return () => {
+			mountedRef.current = false;
+		};
+	}, [] );
+	return useCallback( () => mountedRef.current, [] );
+}