Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support paste verification #1106

Closed
wants to merge 10 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,8 @@ export default function VerificationCodeControl( {
} ) {
const inputsRef = useRef( [] );
const cursorRef = useRef( 0 );
const onCodeChangeRef = useRef();
const [ digits, setDigits ] = useState( initDigits );

onCodeChangeRef.current = onCodeChange;

/**
* Moves focus to the input at given input
* if it exists.
Expand All @@ -61,6 +58,27 @@ export default function VerificationCodeControl( {
}
};

const getEventData = ( e ) => {
const { value, dataset } = e.target;
const idx = Number( dataset.idx );

return {
idx,
value: e.clipboardData?.getData( 'text/plain' ) ?? value,
};
};

const updateInputRefs = ( nextDigits ) => {
inputsRef.current.forEach(
( el, idx ) => ( el.value = nextDigits[ idx ] )
);
};

const updateState = ( nextDigits ) => {
setDigits( nextDigits );
onCodeChange( toCallbackData( nextDigits ) );
};

const handleKeyDown = ( e ) => {
const { dataset, selectionStart, selectionEnd, value } = e.target;
const idx = Number( dataset.idx );
Expand All @@ -86,36 +104,57 @@ export default function VerificationCodeControl( {
cursorRef.current = e.target.selectionStart;
};

const handleUpdate = ( e ) => {
e.preventDefault();

const { nextDigits, nextFocusIdx } = e.clipboardData
? handlePaste( e )
: handleInput( e );

maybeMoveFocus( nextFocusIdx );
updateState( nextDigits );
eason9487 marked this conversation as resolved.
Show resolved Hide resolved
};

const handleInput = ( e ) => {
const { value, dataset } = e.target;
const idx = Number( dataset.idx );
const { value, idx } = getEventData( e );

// Only keep the first entered char from the starting position of key cursor.
// If that char is not a digit, then clear the input to empty.
const digit = value.substr( cursorRef.current, 1 ).replace( /\D/, '' );

// If that char is not a digit, then clear the input to empty.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


The meaning of this code comment is different from the if ( digit !== value ) condition and its processing below.

if ( digit !== value ) {
e.target.value = digit;
}

if ( digit ) {
maybeMoveFocus( idx + 1 );
}
const nextDigits = [ ...digits ];
nextDigits[ idx ] = digit;

// always increase focus index by one except for digit deletions
return { nextDigits, nextFocusIdx: digit ? idx + 1 : idx };
};

if ( digit !== digits[ idx ] ) {
const nextDigits = [ ...digits ];
nextDigits[ idx ] = digit;
setDigits( nextDigits );
const handlePaste = ( e ) => {
const { idx, value } = getEventData( e );

onCodeChange( toCallbackData( nextDigits ) );
}
// only allow n digits, from the current idx position until the end
const newDigits = [
...value.replace( /\D/g, '' ).substr( 0, DIGIT_LENGTH - idx ),
];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


We don't need to handle content other than plain text. Not sure why it needs to use onPaste event? The same newDigits value could be got in the original onInput handler by

const handleInput = ( e ) => {
	const { value, dataset } = e.target;
	const newDigits = value
		.replace( /\D/g, '' )
		.substr( cursorRef.current, DIGIT_LENGTH - idx );

	// ...omitted
};

I think this could simplify the processing of getting content from onPaste, and even further simplify other changes by using the original handleInput to handle a 1 to DIGIT_LENGTH length array of digits.


const nextDigits = [ ...digits ];

newDigits.forEach(
( digit, i ) => ( nextDigits[ i + idx ] = newDigits[ i ] )
);

// set next focus index to the last inserted digit
return { nextDigits, nextFocusIdx: newDigits.length + idx - 1 };
};

// Update the inputs' values.
// Reset the inputs' refs and state when resetNeedle changes.
useEffect( () => {
inputsRef.current.forEach( ( el ) => ( el.value = '' ) );

setDigits( initDigits );
onCodeChangeRef.current( toCallbackData( initDigits ) );
updateInputRefs( initDigits );
updateState( initDigits );
puntope marked this conversation as resolved.
Show resolved Hide resolved
}, [ resetNeedle ] );

/**
Expand All @@ -135,13 +174,17 @@ export default function VerificationCodeControl( {
* So here we await the `digits` is reset back to `initDigits` by above useEffect and sync to internal value,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💅
The code comments are outdated after changing the position and implementation of the original useEffect.

* then move the focus calling after the synchronization tick finished.
*
* Note the above also impacts in the state updates for the focused element...
*
* @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 );
} else {
updateInputRefs( digits );
}
}, [ resetNeedle, digits ] );
}, [ digits ] );
eason9487 marked this conversation as resolved.
Show resolved Hide resolved

return (
<Flex
Expand All @@ -158,7 +201,8 @@ export default function VerificationCodeControl( {
value={ value }
onKeyDown={ handleKeyDown }
onBeforeInput={ handleBeforeInput }
onInput={ handleInput }
onInput={ handleUpdate }
onPaste={ handleUpdate }
autoComplete="off"
/>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { fireEvent, render, screen } from '@testing-library/react';
import { createEvent, fireEvent, render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom/extend-expect';

Expand Down Expand Up @@ -127,4 +127,116 @@ describe( 'VerificationCodeControl component', () => {

expect( onSubmit ).toHaveBeenCalled();
} );

test( 'it is possible to paste multiple digits', async () => {
render( <VerificationCodeControl onCodeChange={ () => {} } /> );
const inputs = screen.getAllByRole( 'textbox' );

const paste = createEvent.paste( inputs[ 0 ], {
clipboardData: {
getData: () => '222222',
},
} );

fireEvent( inputs[ 0 ], paste );

inputs.forEach( ( el ) => expect( el.value ).toBe( '2' ) );
} );

test( 'it pastes a maximum of {DIGIT_LENGTH} digits', async () => {
render( <VerificationCodeControl onCodeChange={ () => {} } /> );
const inputs = screen.getAllByRole( 'textbox' );

const paste = createEvent.paste( inputs[ 0 ], {
clipboardData: {
getData: () => '22222233333333',
},
} );

fireEvent( inputs[ 0 ], paste );

inputs.forEach( ( el ) => expect( el.value ).toBe( '2' ) );
} );

test( 'it pastes with other indexes and number of digits', async () => {
render( <VerificationCodeControl onCodeChange={ () => {} } /> );
const inputs = screen.getAllByRole( 'textbox' );

const paste = createEvent.paste( inputs[ 1 ], {
clipboardData: {
getData: () => '22',
},
} );

fireEvent( inputs[ 1 ], paste );

inputs.forEach( ( el, idx ) => {
const expectedValue = idx > 0 && idx < 3 ? '2' : '';
expect( el.value ).toBe( expectedValue );
} );
} );

test( 'updates validation code prop on paste', async () => {
const onCodeChange = jest
.fn( ( data ) => data )
.mockName( 'onCodeChange' );

render( <VerificationCodeControl onCodeChange={ onCodeChange } /> );
const inputs = screen.getAllByRole( 'textbox' );

expect( onCodeChange.mock.results[ 0 ].value ).toStrictEqual( {
code: '',
isFilled: false,
} );

let paste = createEvent.paste( inputs[ 0 ], {
clipboardData: {
getData: () => '222',
},
} );

fireEvent( inputs[ 0 ], paste );

expect( onCodeChange.mock.results[ 1 ].value ).toStrictEqual( {
code: '222',
isFilled: false,
} );

paste = createEvent.paste( inputs[ 0 ], {
clipboardData: {
getData: () => '333333',
},
} );

fireEvent( inputs[ 0 ], paste );

expect( onCodeChange.mock.results[ 2 ].value ).toStrictEqual( {
code: '333333',
isFilled: true,
} );
} );
test( 'updates validation code prop on input', async () => {
const onCodeChange = jest
.fn( ( data ) => data )
.mockName( 'onCodeChange' );

render( <VerificationCodeControl onCodeChange={ onCodeChange } /> );
const inputs = screen.getAllByRole( 'textbox' );

expect( onCodeChange.mock.results[ 0 ].value ).toStrictEqual( {
code: '',
isFilled: false,
} );

let currentCode = '';
for ( const input of inputs ) {
const i = inputs.indexOf( input ) + 1;
await userEvent.type( input, '1' );
currentCode += '1';
expect( onCodeChange.mock.results[ i ].value ).toStrictEqual( {
code: currentCode,
isFilled: i === 6,
} );
}
} );
} );