From cb9c6e3b1071d0e2474732f19458778d0bf7944c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ismael=20Mart=C3=ADn=20Alabarce?= Date: Fri, 2 Jun 2023 14:35:08 +0200 Subject: [PATCH] Validate PO text fields on change only after blur (#6430) --- .../update-6407-po-form-validation-on-blur | 5 + .../components/phone-number-control/index.tsx | 7 +- client/onboarding-prototype/form.tsx | 10 +- client/onboarding-prototype/test/form.tsx | 102 +++++++++++++----- 4 files changed, 93 insertions(+), 31 deletions(-) create mode 100644 changelog/update-6407-po-form-validation-on-blur diff --git a/changelog/update-6407-po-form-validation-on-blur b/changelog/update-6407-po-form-validation-on-blur new file mode 100644 index 00000000000..a1440a5f9dd --- /dev/null +++ b/changelog/update-6407-po-form-validation-on-blur @@ -0,0 +1,5 @@ +Significance: patch +Type: update +Comment: Minor update behind PO feature flag + + diff --git a/client/components/phone-number-control/index.tsx b/client/components/phone-number-control/index.tsx index 23eb68d8974..a40227dcc03 100644 --- a/client/components/phone-number-control/index.tsx +++ b/client/components/phone-number-control/index.tsx @@ -26,6 +26,7 @@ const countryCodes = window.intlTelInputGlobals interface Props { value: string; onChange: ( value: string, country: string ) => void; + onBlur?: () => void; country?: string; className?: string; label?: string; @@ -36,6 +37,7 @@ const PhoneNumberControl: React.FC< Props > = ( { value, country, onChange, + onBlur, ...rest } ) => { const [ focused, setFocused ] = useState( false ); @@ -107,7 +109,10 @@ const PhoneNumberControl: React.FC< Props > = ( { value={ phoneNumber } onChange={ handleInput } onFocus={ () => setFocused( true ) } - onBlur={ () => setFocused( false ) } + onBlur={ () => { + setFocused( false ); + onBlur?.(); + } } style={ { paddingLeft: spanWidth + 8, marginLeft: -spanWidth, diff --git a/client/onboarding-prototype/form.tsx b/client/onboarding-prototype/form.tsx index 3171edda647..780c16b1035 100644 --- a/client/onboarding-prototype/form.tsx +++ b/client/onboarding-prototype/form.tsx @@ -62,7 +62,7 @@ export const OnboardingTextField: React.FC< OnboardingTextFieldProps > = ( { name, ...rest } ) => { - const { data, setData } = useOnboardingContext(); + const { data, setData, touched } = useOnboardingContext(); const { validate, error } = useValidation( name ); return ( @@ -71,8 +71,9 @@ export const OnboardingTextField: React.FC< OnboardingTextFieldProps > = ( { value={ data[ name ] || '' } onChange={ ( value: string ) => { setData( { [ name ]: value } ); - validate( value ); + if ( touched[ name ] ) validate( value ); } } + onBlur={ () => validate() } error={ error() } { ...rest } /> @@ -88,7 +89,7 @@ export const OnboardingPhoneNumberField: React.FC< OnboardingPhoneNumberFieldPro name, ...rest } ) => { - const { data, setData, temp, setTemp } = useOnboardingContext(); + const { data, setData, temp, setTemp, touched } = useOnboardingContext(); const { validate, error } = useValidation( name ); return ( @@ -99,8 +100,9 @@ export const OnboardingPhoneNumberField: React.FC< OnboardingPhoneNumberFieldPro onChange={ ( value: string, phoneCountryCode: string ) => { setTemp( { phoneCountryCode } ); setData( { [ name ]: value } ); - validate( value ); + if ( touched[ name ] ) validate( value ); } } + onBlur={ () => validate() } error={ error() } { ...rest } /> diff --git a/client/onboarding-prototype/test/form.tsx b/client/onboarding-prototype/test/form.tsx index 4e2a1d1db68..3bfe4fbaa90 100644 --- a/client/onboarding-prototype/test/form.tsx +++ b/client/onboarding-prototype/test/form.tsx @@ -24,6 +24,7 @@ declare const global: { let nextStep = jest.fn(); let data = {}; let errors = {}; +let touched = {}; let temp = {}; let setData = jest.fn(); @@ -36,6 +37,7 @@ jest.mock( '../context', () => ( { useOnboardingContext: jest.fn( () => ( { data, errors, + touched, temp, setData, setTouched, @@ -61,6 +63,7 @@ describe( 'Progressive Onboarding Prototype Form', () => { nextStep = jest.fn(); data = {}; errors = {}; + touched = {}; temp = {}; setData = jest.fn(); setTouched = jest.fn(); @@ -123,7 +126,7 @@ describe( 'Progressive Onboarding Prototype Form', () => { expect( errorMessage ).toBeInTheDocument(); } ); - it( 'calls setData and validate on change', () => { + it( 'calls setData on change', () => { render( ); const textField = screen.getByLabelText( 'First name' ); @@ -132,8 +135,30 @@ describe( 'Progressive Onboarding Prototype Form', () => { expect( setData ).toHaveBeenCalledWith( { 'individual.first_name': 'John', } ); + + expect( validate ).not.toHaveBeenCalled(); + } ); + + it( 'only calls validate on change if touched', () => { + touched = { 'individual.first_name': true }; + render( ); + + const textField = screen.getByLabelText( 'First name' ); + userEvent.type( textField, 'John' ); + expect( validate ).toHaveBeenCalledWith( 'John' ); } ); + + it( 'calls validate on blur', () => { + render( ); + + const textField = screen.getByLabelText( 'First name' ); + userEvent.type( textField, 'John' ); + userEvent.tab(); + fireEvent.focusOut( textField ); // Workaround for onFocus event not firing with jsdom <16.3.0 + + expect( validate ).toHaveBeenCalledWith(); + } ); } ); describe( 'OnboardingSelectField', () => { @@ -172,40 +197,65 @@ describe( 'Progressive Onboarding Prototype Form', () => { } ); expect( validate ).toHaveBeenCalledWith( 'individual' ); } ); + } ); + + describe( 'OnboardingPhoneNumberField', () => { + it( 'renders component with provided props ', () => { + data = { phone: '+123' }; + error.mockReturnValue( 'error message' ); + + render( ); + + const textField = screen.getByLabelText( + 'What’s your mobile phone number?' + ); + const errorMessage = screen.getByText( 'error message' ); + + expect( textField ).toHaveValue( '23' ); + expect( errorMessage ).toBeInTheDocument(); + } ); - describe( 'OnboardingPhoneNumberField', () => { - it( 'renders component with provided props ', () => { - data = { phone: '+123' }; - error.mockReturnValue( 'error message' ); + it( 'calls setTemp and setData on change', () => { + render( ); - render( ); + const textField = screen.getByLabelText( + 'What’s your mobile phone number?' + ); + userEvent.type( textField, '23' ); - const textField = screen.getByLabelText( - 'What’s your mobile phone number?' - ); - const errorMessage = screen.getByText( 'error message' ); + expect( setTemp ).toHaveBeenCalledWith( { + phoneCountryCode: 'US', + } ); - expect( textField ).toHaveValue( '23' ); - expect( errorMessage ).toBeInTheDocument(); + expect( setData ).toHaveBeenCalledWith( { + phone: '+123', } ); + expect( validate ).not.toHaveBeenCalledWith(); + } ); - it( 'calls setTemp, setData and validate on change', () => { - render( ); + it( 'only calls validate on change if touched', () => { + touched = { phone: true }; + render( ); - const textField = screen.getByLabelText( - 'What’s your mobile phone number?' - ); - userEvent.type( textField, '23' ); + const textField = screen.getByLabelText( + 'What’s your mobile phone number?' + ); + userEvent.type( textField, '23' ); - expect( setTemp ).toHaveBeenCalledWith( { - phoneCountryCode: 'US', - } ); + expect( validate ).toHaveBeenCalledWith( '+123' ); + } ); - expect( setData ).toHaveBeenCalledWith( { - phone: '+123', - } ); - expect( validate ).toHaveBeenCalledWith( '+123' ); - } ); + it( 'calls validate on blur', () => { + render( ); + + const textField = screen.getByLabelText( + 'What’s your mobile phone number?' + ); + userEvent.type( textField, '23' ); + userEvent.tab(); + fireEvent.focusOut( textField ); // Workaround for onFocus event not firing with jsdom <16.3.0 + + expect( validate ).toHaveBeenCalledWith(); } ); } ); } );