From 9821c59e115a119e3d0ec5e8decdb0e01acd4ffb Mon Sep 17 00:00:00 2001 From: George Marshall Date: Thu, 10 Nov 2022 11:13:15 -0800 Subject: [PATCH] `TextField` component updates (#16424) * Updating showClearButton prop name and making component dumber * Docs update * Adding missing proptype * Fixing casing on tests * Replacing clear button placeholder with ButtonIcon and docs update * Fixing linting issues * Adding note about controlled only for showClearButton to work and fixing some tests * Updating test to include controlled testing setup function for clearButton tests --- .../component-library/text-field/README.mdx | 68 +++--- .../text-field/text-field.js | 121 +++++------ .../text-field/text-field.stories.js | 83 +++----- .../text-field/text-field.test.js | 196 ++++++++---------- 4 files changed, 210 insertions(+), 258 deletions(-) diff --git a/ui/components/component-library/text-field/README.mdx b/ui/components/component-library/text-field/README.mdx index 7a9bc8cd6..617111a7b 100644 --- a/ui/components/component-library/text-field/README.mdx +++ b/ui/components/component-library/text-field/README.mdx @@ -16,58 +16,78 @@ The `TextField` accepts all props below as well as all [Box](/docs/ui-components -### Show Clear +### Show Clear Button -Use the `showClear` prop to display a clear button when `TextField` has a value. Clicking the button will clear the value. -You can also attach an `onClear` handler to the `TextField` to perform additional actions when the clear button is clicked. +Use the `showClearButton` prop to display a clear button when `TextField` has a value. Use the `clearButtonOnClick` prop to pass an `onClick` event handler to clear the value of the input. + +The clear button uses [ButtonIcon](/docs/ui-components-component-library-button-icon-button-icon-stories-js--default-story) and accepts all props from that component. + +**NOTE: The `showClearButton` only works with a controlled input.** - + ```jsx import { TextField } from '../../ui/component-library/text-field'; -; +const [value, setValue] = useState('show clear'); + +const handleOnChange = (e) => { + setValue(e.target.value); +}; + +const handleOnClear = () => { + setValue(''); +}; + +; ``` -### On Clear +### Clear Button Props -Use the `onClear` prop to perform additional actions when the clear button is clicked. +Use the `clearButtonProps` to access other props of the clear button. - - - -```jsx -import { TextField } from '../../ui/component-library/text-field'; - - console.log('cleared input')} />; -``` - -### Clear Button Props and Clear Button Icon Props - -Use the `clearButtonProps` and `clearButtonIconProps` props to pass props to the clear button and clear button icon respectively. - - - + ```jsx +import React, { useState } from 'react'; import { SIZES, COLORS, BORDER_RADIUS, } from '../../../helpers/constants/design-system'; + import { TextField } from '../../ui/component-library/text-field'; +const [value, setValue] = useState('show clear'); + +const handleOnChange = (e) => { + setValue(e.target.value); +}; + +const handleOnClear = () => { + setValue(''); +}; + ; ``` diff --git a/ui/components/component-library/text-field/text-field.js b/ui/components/component-library/text-field/text-field.js index ddf351776..7d26ff49f 100644 --- a/ui/components/component-library/text-field/text-field.js +++ b/ui/components/component-library/text-field/text-field.js @@ -1,86 +1,65 @@ -import React, { useState } from 'react'; +import React from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; -import { - SIZES, - DISPLAY, - JUSTIFY_CONTENT, - ALIGN_ITEMS, - COLORS, -} from '../../../helpers/constants/design-system'; +import { SIZES } from '../../../helpers/constants/design-system'; import Box from '../../ui/box'; -import { Icon, ICON_NAMES } from '../icon'; +import { ICON_NAMES } from '../icon'; +import { ButtonIcon } from '../button-icon'; import { TextFieldBase } from '../text-field-base'; export const TextField = ({ className, - showClear, - clearButtonIconProps, + showClearButton, // only works with a controlled input + clearButtonOnClick, clearButtonProps, rightAccessory, - value: valueProp, - onChange, - onClear, inputProps, + value, + onChange, ...props -}) => { - const [value, setValue] = useState(valueProp || ''); - const handleOnChange = (e) => { - setValue(e.target.value); - onChange?.(e); - }; - const handleClear = (e) => { - setValue(''); - clearButtonProps?.onClick?.(e); - onClear?.(e); - }; - return ( - - {/* replace with ButtonIcon */} - - - - {rightAccessory} - - ) : ( - rightAccessory - ) - } - inputProps={{ - marginRight: showClear ? 6 : 0, - ...inputProps, - }} - {...props} - /> - ); -}; +}) => ( + + + {rightAccessory} + + ) : ( + rightAccessory + ) + } + inputProps={{ + marginRight: showClearButton ? 6 : 0, + ...inputProps, + }} + {...props} + /> +); TextField.propTypes = { + /** + * The value af the TextField + */ + value: TextFieldBase.propTypes.value.isRequired, + /** + * The onChange handler af the TextField + */ + onChange: TextFieldBase.propTypes.onChange.isRequired, /** * An additional className to apply to the text-field */ @@ -88,19 +67,15 @@ TextField.propTypes = { /** * Show a clear button to clear the input */ - showClear: PropTypes.bool, + showClearButton: PropTypes.bool, /** - * The event handler for when the clear button is clicked + * The onClick handler for the clear button */ - onClear: PropTypes.func, + clearButtonOnClick: PropTypes.func, /** * The props to pass to the clear button */ clearButtonProps: PropTypes.shape(Box.PropTypes), - /** - * The props to pass to the icon inside of the close button - */ - clearButtonIconProps: PropTypes.shape(Icon.PropTypes), /** * TextField accepts all the props from TextFieldBase and Box */ diff --git a/ui/components/component-library/text-field/text-field.stories.js b/ui/components/component-library/text-field/text-field.stories.js index 70832ed2b..ca3d1c0a0 100644 --- a/ui/components/component-library/text-field/text-field.stories.js +++ b/ui/components/component-library/text-field/text-field.stories.js @@ -1,4 +1,5 @@ -import React, { useState } from 'react'; +import React from 'react'; +import { useArgs } from '@storybook/client-api'; import { SIZES, @@ -6,8 +7,6 @@ import { BORDER_RADIUS, } from '../../../helpers/constants/design-system'; -import { Text } from '../text'; - import { TEXT_FIELD_SIZES, TEXT_FIELD_TYPES } from './text-field.constants'; import { TextField } from './text-field'; @@ -41,21 +40,17 @@ export default { }, }, argTypes: { - showClear: { - control: 'boolean', - }, value: { control: 'text', }, onChange: { action: 'onChange', - table: { category: 'text field base props' }, }, - onClear: { - action: 'onClear', + showClearButton: { + control: 'boolean', }, - clearButtonIconProps: { - control: 'object', + clearButtonOnClick: { + action: 'clearButtonOnClick', }, clearButtonProps: { control: 'object', @@ -172,7 +167,7 @@ export default { }, }, args: { - showClear: false, + showClearButton: false, placeholder: 'Placeholder...', autoFocus: false, disabled: false, @@ -186,62 +181,48 @@ export default { }, }; -const Template = (args) => ; - -export const DefaultStory = Template.bind({}); -DefaultStory.storyName = 'Default'; - -export const ShowClear = (args) => { - const [value, setValue] = useState('show clear'); +const Template = (args) => { + const [{ value }, updateArgs] = useArgs(); const handleOnChange = (e) => { - setValue(e.target.value); + updateArgs({ value: e.target.value }); + }; + const handleOnClear = () => { + updateArgs({ value: '' }); }; return ( ); }; -export const OnClear = (args) => { - const [value, setValue] = useState('onClear example'); - const [showOnClearMessage, setShowOnClearMessage] = useState(false); - const handleOnChange = (e) => { - setValue(e.target.value); - showOnClearMessage && setShowOnClearMessage(false); - }; - const handleOnClear = () => { - setShowOnClearMessage(true); - }; - return ( - <> - - {showOnClearMessage && onClear called} - - ); +export const DefaultStory = Template.bind({}); +DefaultStory.storyName = 'Default'; + +export const ShowClearButton = Template.bind({}); + +ShowClearButton.args = { + placeholder: 'Enter text to show clear', + showClearButton: true, }; -export const ClearButtonPropsClearButtonIconProps = Template.bind({}); -ClearButtonPropsClearButtonIconProps.args = { +export const ClearButtonOnClick = Template.bind({}); + +ShowClearButton.args = { + placeholder: 'Enter text to show clear', + showClearButton: true, +}; + +export const ClearButtonProps = Template.bind({}); +ClearButtonProps.args = { value: 'clear button props', size: SIZES.LG, - showClear: true, + showClearButton: true, clearButtonProps: { backgroundColor: COLORS.BACKGROUND_ALTERNATIVE, borderRadius: BORDER_RADIUS.XS, }, - clearButtonIconProps: { - size: SIZES.MD, - }, }; diff --git a/ui/components/component-library/text-field/text-field.test.js b/ui/components/component-library/text-field/text-field.test.js index f2d7819be..7dfa930f9 100644 --- a/ui/components/component-library/text-field/text-field.test.js +++ b/ui/components/component-library/text-field/text-field.test.js @@ -1,25 +1,45 @@ /* eslint-disable jest/require-top-level-describe */ -import React from 'react'; +import React, { useState } from 'react'; import { fireEvent, render } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { TextField } from './text-field'; +// userEvent setup function as per testing-library docs +// https://testing-library.com/docs/user-event/intr +function setup(jsx) { + return { + user: userEvent.setup(), + ...render(jsx), + }; +} + +// Custom userEvent setup function that renders the component in a controlled environment. +// This is used for the showClearButton and related props as the clearButton will only show in a controlled environment. +function setupControlled(FormComponent, props) { + const ControlledWrapper = () => { + const [value, setValue] = useState(''); + return ( + setValue(e.target.value)} + {...props} + /> + ); + }; + return { user: userEvent.setup(), ...render() }; +} + describe('TextField', () => { it('should render correctly', () => { const { getByRole } = render(); expect(getByRole('textbox')).toBeDefined(); }); - it('should render and be able to input text', () => { - const { getByTestId } = render( - , - ); - const textField = getByTestId('text-field'); - - expect(textField.value).toBe(''); // initial value is empty string - fireEvent.change(textField, { target: { value: 'text value' } }); - expect(textField.value).toBe('text value'); - fireEvent.change(textField, { target: { value: '' } }); // reset value - expect(textField.value).toBe(''); // value is empty string after reset + it('should render and be able to input text', async () => { + const { user, getByRole } = setup(); + const textField = getByRole('textbox'); + await user.type(textField, 'text value'); + expect(textField).toHaveValue('text value'); }); it('should render and fire onFocus and onBlur events', () => { const onFocus = jest.fn(); @@ -38,46 +58,27 @@ describe('TextField', () => { fireEvent.blur(textField); expect(onBlur).toHaveBeenCalledTimes(1); }); - it('should render and fire onChange event', () => { + it('should render and fire onChange event', async () => { const onChange = jest.fn(); - const { getByTestId } = render( + const { user, getByRole } = setup( , ); - const textField = getByTestId('text-field'); - - fireEvent.change(textField, { target: { value: 'text value' } }); - expect(onChange).toHaveBeenCalledTimes(1); - }); - it('should render and fire onClick event', () => { - const onClick = jest.fn(); - const { getByTestId } = render( - , - ); - const textField = getByTestId('text-field'); - - fireEvent.click(textField); - expect(onClick).toHaveBeenCalledTimes(1); - }); - it('should render showClear button when showClear is true and value exists', () => { - const { getByRole, getByTestId } = render( - , - ); const textField = getByRole('textbox'); - expect(textField.value).toBe(''); // initial value is empty string - fireEvent.change(textField, { target: { value: 'text value' } }); - expect(textField.value).toBe('text value'); - expect(getByTestId('clear-button')).toBeDefined(); - expect(getByTestId('clear-button-icon')).toBeDefined(); + + await user.type(textField, '123'); + expect(textField).toHaveValue('123'); + expect(onChange).toHaveBeenCalledTimes(3); + }); + it('should render and fire onClick event', async () => { + const onClick = jest.fn(); + const { user, getByTestId } = setup( + , + ); + await user.click(getByTestId('text-field')); + expect(onClick).toHaveBeenCalledTimes(1); }); it('should render with the rightAccessory', () => { const { getByText } = render( @@ -85,77 +86,52 @@ describe('TextField', () => { ); expect(getByText('right-accessory')).toBeDefined(); }); - it('should still render with the rightAccessory when showClear is true', () => { - const { getByRole, getByTestId, getByText } = render( - right-accessory} - showClear - />, - ); - const textField = getByRole('textbox'); - expect(textField.value).toBe(''); // initial value is empty string - fireEvent.change(textField, { target: { value: 'text value' } }); - expect(textField.value).toBe('text value'); - expect(getByTestId('clear-button')).toBeDefined(); - expect(getByTestId('clear-button-icon')).toBeDefined(); + it('should render showClearButton button when showClearButton is true and value exists', async () => { + // As showClearButton is intended to be used with a controlled input we need to use setupControlled + const { user, getByRole } = setupControlled(TextField, { + showClearButton: true, + }); + await user.type(getByRole('textbox'), 'test value'); + expect(getByRole('textbox')).toHaveValue('test value'); + expect(getByRole('button', { name: /Clear/u })).toBeDefined(); + }); + it('should still render with the rightAccessory when showClearButton is true', async () => { + // As showClearButton is intended to be used with a controlled input we need to use setupControlled + const { user, getByRole, getByText } = setupControlled(TextField, { + showClearButton: true, + rightAccessory:
right-accessory
, + }); + await user.type(getByRole('textbox'), 'test value'); + expect(getByRole('textbox')).toHaveValue('test value'); + expect(getByRole('button', { name: /Clear/u })).toBeDefined(); expect(getByText('right-accessory')).toBeDefined(); }); - it('should clear text when clear button is clicked', () => { - const { getByRole, getByTestId } = render( - right-accessory} - showClear - />, - ); - const textField = getByRole('textbox'); - fireEvent.change(textField, { target: { value: 'text value' } }); - expect(textField.value).toBe('text value'); - fireEvent.click(getByTestId('clear-button')); - expect(textField.value).toBe(''); + it('should fire onClick event when passed to clearButtonOnClick when clear button is clicked', async () => { + // As showClearButton is intended to be used with a controlled input we need to use setupControlled + const fn = jest.fn(); + const { user, getByRole } = setupControlled(TextField, { + showClearButton: true, + clearButtonOnClick: fn, + }); + await user.type(getByRole('textbox'), 'test value'); + await user.click(getByRole('button', { name: /Clear/u })); + expect(fn).toHaveBeenCalledTimes(1); }); - it('should fire onClear event when passed to onClear prop', () => { - const onClear = jest.fn(); - const { getByRole, getByTestId } = render( - , - ); - const textField = getByRole('textbox'); - fireEvent.change(textField, { target: { value: 'text value' } }); - expect(textField.value).toBe('text value'); - fireEvent.click(getByTestId('clear-button')); - expect(onClear).toHaveBeenCalledTimes(1); - }); - it('should fire clearButtonProps.onClick event when passed to clearButtonProps.onClick prop', () => { - const onClear = jest.fn(); - const onClick = jest.fn(); - const { getByRole, getByTestId } = render( - , - ); - const textField = getByRole('textbox'); - fireEvent.change(textField, { target: { value: 'text value' } }); - expect(textField.value).toBe('text value'); - fireEvent.click(getByTestId('clear-button')); - expect(onClear).toHaveBeenCalledTimes(1); - expect(onClick).toHaveBeenCalledTimes(1); + it('should fire onClick event when passed to clearButtonProps.onClick prop', async () => { + // As showClearButton is intended to be used with a controlled input we need to use setupControlled + const fn = jest.fn(); + const { user, getByRole } = setupControlled(TextField, { + showClearButton: true, + clearButtonProps: { onClick: fn }, + }); + await user.type(getByRole('textbox'), 'test value'); + await user.click(getByRole('button', { name: /Clear/u })); + expect(fn).toHaveBeenCalledTimes(1); }); it('should be able to accept inputProps', () => { - const { getByRole } = render( + const { getByTestId } = render( , ); - const textField = getByRole('textbox'); - expect(textField).toBeDefined(); + expect(getByTestId('text-field')).toBeDefined(); }); });