Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Convert TextInput and ValidatedTextInput to TypeScript #4226

Merged
merged 10 commits into from
May 19, 2021
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
/**
* External dependencies
*/
import { forwardRef } from 'react';
import PropTypes from 'prop-types';
import { forwardRef, InputHTMLAttributes } from 'react';
import classnames from 'classnames';
import { useState } from '@wordpress/element';
import { Label } from '@woocommerce/blocks-checkout';
Expand All @@ -12,7 +11,24 @@ import { Label } from '@woocommerce/blocks-checkout';
*/
import './style.scss';

const TextInput = forwardRef(
interface TextInputProps
extends Omit<
InputHTMLAttributes< HTMLInputElement >,
'onChange' | 'onBlur'
> {
id: string;
ariaLabel?: string;
label?: string;
ariaDescribedBy?: string;
screenReaderLabel?: string;
help?: string;
feedback?: boolean | JSX.Element;
autoComplete?: string;
onChange: ( newValue: string ) => void;
onBlur?: ( newValue: string ) => void;
}

const TextInput = forwardRef< HTMLInputElement, TextInputProps >(
(
{
className,
Expand All @@ -29,7 +45,9 @@ const TextInput = forwardRef(
value = '',
onChange,
required = false,
onBlur = () => {},
onBlur = () => {
/* Do nothing */
},
feedback,
},
ref
Expand Down Expand Up @@ -57,8 +75,8 @@ const TextInput = forwardRef(
onChange( event.target.value );
} }
onFocus={ () => setIsActive( true ) }
onBlur={ () => {
onBlur();
onBlur={ ( event ) => {
onBlur( event.target.value );
setIsActive( false );
} }
aria-label={ ariaLabel || label }
Expand Down Expand Up @@ -93,19 +111,4 @@ const TextInput = forwardRef(
}
);

TextInput.propTypes = {
id: PropTypes.string.isRequired,
onChange: PropTypes.func.isRequired,
value: PropTypes.string,
ariaLabel: PropTypes.string,
ariaDescribedBy: PropTypes.string,
label: PropTypes.string,
screenReaderLabel: PropTypes.string,
disabled: PropTypes.bool,
help: PropTypes.string,
autoCapitalize: PropTypes.string,
autoComplete: PropTypes.string,
required: PropTypes.bool,
};

export default TextInput;
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import { __ } from '@wordpress/i18n';
import { useCallback, useRef, useEffect, useState } from 'react';
import PropTypes from 'prop-types';
import classnames from 'classnames';
import {
ValidationInputError,
Expand All @@ -17,6 +16,29 @@ import { withInstanceId } from '@woocommerce/base-hocs/with-instance-id';
import TextInput from './text-input';
import './style.scss';

interface ValidatedTextInputPropsWithId {
instanceId?: string;
id: string;
}

interface ValidatedTextInputPropsWithInstanceId {
instanceId: string;
id?: string;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Essentially this means: if id is passed, instanceId is optional, and vice/versa.

type ValidatedTextInputProps = (
| ValidatedTextInputPropsWithId
| ValidatedTextInputPropsWithInstanceId
) & {
className?: string;
ariaDescribedBy?: string;
errorId?: string;
validateOnMount?: boolean;
focusOnMount?: boolean;
showError?: boolean;
onChange: ( newValue: string ) => void;
};

const ValidatedTextInput = ( {
className,
instanceId,
Expand All @@ -28,9 +50,9 @@ const ValidatedTextInput = ( {
onChange,
showError = true,
...rest
} ) => {
}: ValidatedTextInputProps ) => {
const [ isPristine, setIsPristine ] = useState( true );
const inputRef = useRef();
const inputRef = useRef< HTMLInputElement >( null );
const {
getValidationError,
hideValidationError,
Expand All @@ -39,8 +61,8 @@ const ValidatedTextInput = ( {
getValidationErrorId,
} = useValidationContext();

const textInputId = id || 'textinput-' + instanceId;
errorId = errorId || textInputId;
const textInputId = typeof id !== 'undefined' ? id : 'textinput-' + instanceId;
const errorIdString = errorId !== undefined ? errorId : textInputId;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change was made because TS still couldn't properly infer that after this line, errorId was a string. It still thought it would be string | undefined.


const validateInput = useCallback(
( errorsHidden = true ) => {
Expand All @@ -52,10 +74,10 @@ const ValidatedTextInput = ( {
inputObject.value = inputObject.value.trim();
const inputIsValid = inputObject.checkValidity();
if ( inputIsValid ) {
clearValidationError( errorId );
clearValidationError( errorIdString );
} else {
setValidationErrors( {
[ errorId ]: {
[ errorIdString ]: {
message:
inputObject.validationMessage ||
__(
Expand All @@ -67,13 +89,13 @@ const ValidatedTextInput = ( {
} );
}
},
[ clearValidationError, errorId, setValidationErrors ]
[ clearValidationError, errorIdString, setValidationErrors ]
);

useEffect( () => {
if ( isPristine ) {
if ( focusOnMount ) {
inputRef.current.focus();
inputRef.current?.focus();
}
setIsPristine( false );
}
Expand All @@ -91,15 +113,19 @@ const ValidatedTextInput = ( {
// Remove validation errors when unmounted.
useEffect( () => {
return () => {
clearValidationError( errorId );
clearValidationError( errorIdString );
};
}, [ clearValidationError, errorId ] );
}, [ clearValidationError, errorIdString ] );

const errorMessage = getValidationError( errorId ) || {};
// TODO - When useValidationContext is converted to TypeScript, remove this cast and use the correct type.
opr marked this conversation as resolved.
Show resolved Hide resolved
opr marked this conversation as resolved.
Show resolved Hide resolved
const errorMessage = ( getValidationError( errorIdString ) || {} ) as {
message?: string;
hidden?: boolean;
};
const hasError = errorMessage.message && ! errorMessage.hidden;
const describedBy =
showError && hasError && getValidationErrorId( errorId )
? getValidationErrorId( errorId )
showError && hasError && getValidationErrorId( errorIdString )
? getValidationErrorId( errorIdString )
: ariaDescribedBy;

return (
Expand All @@ -112,11 +138,13 @@ const ValidatedTextInput = ( {
validateInput( false );
} }
feedback={
showError && <ValidationInputError propertyName={ errorId } />
showError && (
<ValidationInputError propertyName={ errorIdString } />
)
}
ref={ inputRef }
onChange={ ( val ) => {
hideValidationError( errorId );
hideValidationError( errorIdString );
onChange( val );
} }
ariaDescribedBy={ describedBy }
Expand All @@ -125,15 +153,4 @@ const ValidatedTextInput = ( {
);
};

ValidatedTextInput.propTypes = {
onChange: PropTypes.func.isRequired,
id: PropTypes.string,
value: PropTypes.string,
ariaDescribedBy: PropTypes.string,
errorId: PropTypes.string,
validateOnMount: PropTypes.bool,
focusOnMount: PropTypes.bool,
showError: PropTypes.bool,
};

export default withInstanceId( ValidatedTextInput );
6 changes: 3 additions & 3 deletions packages/checkout/label/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
*/
import { Fragment } from '@wordpress/element';
import classNames from 'classnames';
import type { ReactElement, HTMLAttributes } from 'react';
import type { ReactElement, HTMLProps } from 'react';

interface LabelProps {
interface LabelProps extends HTMLProps< HTMLElement > {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This lets us pass extra props to Label which are not defined in the LabelProps interface. The specific prop I needed to add this for was htmlFor but it makes sense to allow all valid props through.

label?: string;
screenReaderLabel?: string;
wrapperElement?: string;
wrapperProps?: HTMLAttributes< HTMLElement >;
wrapperProps?: HTMLProps< HTMLElement >;
}

/**
Expand Down