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

[BD-46] feat: added FormControl input mask #2626

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 47 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"react-colorful": "^5.6.1",
"react-dropzone": "^14.2.1",
"react-focus-on": "^3.5.4",
"react-imask": "^7.1.3",
"react-loading-skeleton": "^3.1.0",
"react-popper": "^2.2.5",
"react-proptype-conditional-require": "^1.0.4",
Expand Down
8 changes: 7 additions & 1 deletion src/Form/FormControl.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useCallback, useEffect } from 'react';
import PropTypes from 'prop-types';
import classNames from 'classnames';
import RBFormControl from 'react-bootstrap/FormControl';
import { IMaskInput } from 'react-imask';
import { useFormGroupContext } from './FormGroupContext';
import FormControlFeedback from './FormControlFeedback';
import FormControlDecoratorGroup from './FormControlDecoratorGroup';
Expand All @@ -17,6 +18,7 @@ const FormControl = React.forwardRef(({
floatingLabel,
autoResize,
onChange,
inputMask,
...props
}, ref) => {
const {
Expand Down Expand Up @@ -71,7 +73,7 @@ const FormControl = React.forwardRef(({
className={className}
>
<RBFormControl
as={as}
as={inputMask ? IMaskInput : as}
ref={resolvedRef}
size={size}
isInvalid={isInvalid}
Expand All @@ -80,6 +82,7 @@ const FormControl = React.forwardRef(({
'has-value': hasValue,
})}
onChange={handleOnChange}
mask={inputMask}
{...controlProps}
/>
</FormControlDecoratorGroup>
Expand Down Expand Up @@ -122,6 +125,8 @@ FormControl.propTypes = {
isInvalid: PropTypes.bool,
/** Only for `as="textarea"`. Specifies whether the input can be resized according to the height of content. */
autoResize: PropTypes.bool,
/** Specifies what format to use for the input mask. */
inputMask: PropTypes.string,
};

FormControl.defaultProps = {
Expand All @@ -140,6 +145,7 @@ FormControl.defaultProps = {
isValid: undefined,
isInvalid: undefined,
autoResize: false,
inputMask: undefined,
};

export default FormControl;
155 changes: 154 additions & 1 deletion src/Form/form-control.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ or [select attributes](https://developer.mozilla.org/en-US/docs/Web/HTML/Element
}
```


## Input types

```jsx live
Expand Down Expand Up @@ -163,6 +162,160 @@ or [select attributes](https://developer.mozilla.org/en-US/docs/Web/HTML/Element
}
```

## Input masks
Paragon uses the [react-imask](https://www.npmjs.com/package/react-imask) library,
which allows you to add masks of different types for inputs.
To create your own mask, you need to pass the required mask pattern (`+{1}(000)000-00-00`) to the `inputMask` property. <br />
Copy link
Member

Choose a reason for hiding this comment

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

The US-based example phone number mask should likely be +{1} (000) 000-0000 so that phone numbers are formatted like +1 (555) 555-5555, which is typical for a US-based phone number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected, thanks

See [react-imask](https://imask.js.org) for documentation on available props.

```jsx live
() => {
{/* start example state */}
const [maskType, setMaskType] = useState('phone');
{/* end example state */}

const inputsWithMask = {
phone: (
<>
<h3>Phone</h3>
<Form.Group>
<Form.Control
inputMask="+{1}(000)000-00-00"
Copy link
Member

Choose a reason for hiding this comment

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

The US-based example phone number mask should likely be +{1} (000) 000-0000 so that phone numbers are formatted like +1 (555) 555-5555, which is typical for a US-based phone number.

value={value}
onChange={handleChange}
leadingElement={<Icon src={FavoriteBorder} />}
floatingLabel="What is your phone number?"
/>
</Form.Group>
</>
),
creditCard: (<>
<h3>Credit card</h3>
<Form.Group>
<Form.Control
inputMask="0000 0000 0000 0000"
value={value}
onChange={handleChange}
leadingElement={<Icon src={FavoriteBorder} />}
floatingLabel="What is your credit card number?"
/>
</Form.Group>
</>),
date: (<>
<h3>Date</h3>
<Form.Group>
<Form.Control
inputMask={Date}
Copy link
Member

Choose a reason for hiding this comment

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

A bit curious about this one; it appears it doesn't let users input months without leading zeros? For example, when trying to type in June 10, I type 6 for the month, but it doesn't accept any input until I add a leading zero first.

Beyond this, I'm also wondering if we should keep the date input mask example of these examples given the Form.Control can be used specifically with a date picker input type.

Copy link
Contributor Author

@PKulkoRaccoonGang PKulkoRaccoonGang Nov 22, 2023

Choose a reason for hiding this comment

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

It seems to me that this is a good solution if the mask requires entering a zero before the month.
But this example seems redundant since we have a better use case (Form.Control).
Removed this example

value={value}
onChange={handleChange}
leadingElement={<Icon src={FavoriteBorder} />}
floatingLabel="What is your date of birth?"
/>
</Form.Group>
</>),
securePassword: (<>
<h3>Secure password</h3>
<Form.Group>
<Form.Control
inputMask="XXX-XX-0000"
value={value}
definitions={{
X: {
mask: '0',
displayChar: 'X',
placeholderChar: '#',
},
}}
onChange={handleChange}
leadingElement={<Icon src={FavoriteBorder} />}
floatingLabel="What is your password?"
/>
</Form.Group>
</>),
OTPpassword: (<>
<h3>OTP password</h3>
<Form.Group>
<Form.Control
inputMask="G-00000"
value={value}
onChange={handleChange}
leadingElement={<Icon src={FavoriteBorder} />}
floatingLabel="What is your OPT password?"
/>
</Form.Group>
</>),
price: (
<>
<h3>Course prise</h3>
Copy link
Member

Choose a reason for hiding this comment

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

nit: "Price"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected

<Form.Group>
<Form.Control
inputMask="$num"
blocks={{
num: {
// nested masks are available!
mask: Number,
thousandsSeparator: ' '
}
}}
value={value}
onChange={handleChange}
leadingElement={<Icon src={FavoriteBorder} />}
floatingLabel="What is the price of this course?"
/>
</Form.Group>
</>
),
};

const [value, setValue] = useState('');

const handleChange = (e) => setValue(e.target.value);

return (
<>
{/* start example form block */}
<ExamplePropsForm
inputs={[
{ value: maskType, setValue: setMaskType, options: Object.keys(inputsWithMask), name: 'Mask variants' },
]}
/>
{/* end example form block */}

{inputsWithMask[maskType]}
</>
);
}
```

## Input masks with clear value
To get a value without a mask, you need to use `onChange` instead of `onAccept` to handle changes.

```jsx live
() => {
const [value, setValue] = useState('');

return (
<>
<Form.Group>
<Form.Control
inputMask="+{1}(000)000-00-00"
Copy link
Member

Choose a reason for hiding this comment

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

The US-based example phone number mask should likely be +{1} (000) 000-0000 so that phone numbers are formatted like +1 (555) 555-5555, which is typical for a US-based phone number.

leadingElement={<Icon src={FavoriteBorder} />}
trailingElement={<Icon src={Verified} />}
floatingLabel="What is your phone number?"
value={value}
// depending on prop above first argument is
// `value` if `unmask=false`,
// `unmaskedValue` if `unmask=true`,
// `typedValue` if `unmask='typed'`
onAccept={(_, mask) => setValue(mask._unmaskedValue)}
/>
</Form.Group>
Unmasked value: {JSON.stringify(value)}
</>
);
}
```

## Textarea autoResize

`autoResize` prop allows input to be resized according to the content height.
Expand Down
39 changes: 37 additions & 2 deletions src/Form/tests/FormControl.test.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import React, { useState } from 'react';
import {
render, screen, act, fireEvent,
} from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import FormControl from '../FormControl';
Expand All @@ -8,6 +10,22 @@ const ref = {
current: null,
};

// eslint-disable-next-line react/prop-types
function Component({ isClearValue }) {
const onChangeFunc = jest.fn();
const [inputValue, setInputValue] = useState('');

return (
<FormControl
inputMask="+{1}(000)000-00-00"
value={inputValue}
onChange={isClearValue ? onChangeFunc : (e) => setInputValue(e.target.value)}
onAccept={isClearValue ? onChangeFunc : (value) => setInputValue(value)}
data-testid="form-control-with-mask"
/>
);
}

describe('FormControl', () => {
it('textarea changes its height with autoResize prop', async () => {
const useReferenceSpy = jest.spyOn(React, 'useRef').mockReturnValue(ref);
Expand All @@ -31,4 +49,21 @@ describe('FormControl', () => {
expect(onChangeFunc).toHaveBeenCalledTimes(inputText.length);
expect(ref.current.style.height).toEqual(`${ref.current.scrollHeight + ref.current.offsetHeight}px`);
});

it('should apply and accept input mask for phone numbers', () => {
render(<Component />);

act(() => {
const input = screen.getByTestId('form-control-with-mask');
userEvent.type(input, '1234567890');
expect(input.value).toBe('+1(234)567-89-0');
Copy link
Member

Choose a reason for hiding this comment

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

[curious] generally, userEvent.* functions are called outside of act since the userEvent.* functions are already wrapped with act (example source).

Copy link
Contributor Author

@PKulkoRaccoonGang PKulkoRaccoonGang Nov 22, 2023

Choose a reason for hiding this comment

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

@adamstankiewicz thank you for bringing this issue to our attention 👍

At the moment we have a lot of similar examples of wrapping in act userEvent in Paragon. I've looked into several interesting sources (including the one you shared), this is really an unnecessary solution, here are some confirmations:

  1. When should I use act() in react-testing-library?
  2. Internal implementation
  3. Is wrapping with act now required?

My solution is based on Is wrapping with act now required?. As it turned out, we have differences in the @testing-library/dom subdependency in the versions of the @testing-library/[email protected] and @testing-library/[email protected] libraries. A dependency conflict negatively affects the performance of the userEvent and we see a warning after running tests.

image

We have several options for further action:

  1. Apply now the solution with an override of the @testing-library/dom package (with version 9.3.3). This way we will get rid of the problem of displaying warnings in the console and our tests will be cleaner. This solution can be developed in a separate task, where we remove the wrapping in act userEvent. I have verified this in the Tabs component and in my current case (FormControl).
  "overrides": {
    "@testing-library/dom": "9.3.3"
  }
image
  1. Make these changes and “synchronize” the @testing-library/dom subdependency in @testing-library/react and @testing-library/user-event in the pull request for updating to React 18. So It is quite likely that we will not have to do this since we will update the versions of @testing-library/react and @testing-library/user-event. But we cannot predict whether @testing-library/dom versions will change in the future. Therefore, the override option seems workable.

An article about dependency overriding, in which we can make sure that our intentions are correct:

There should only be a single copy of a given dependency in our tree, but npm's dependency resolution will result in a duplicate due to version conflicts.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for digging into, this @PKulkoRaccoonGang! TIL that @testing-library/dom has to be synchronized :) I agree with your synopsis/options; interesting that I'm not sure this has come up before in MFEs that use RTL. But I agree the overrides is sufficient to hold over until the React 18 upgrade.

});
});
it('should be cleared from the mask elements value', () => {
render(<Component isClearValue />);

const input = screen.getByTestId('form-control-with-mask');
fireEvent.change(input, { target: { value: '1234567890' } });
expect(input.value).toBe('1234567890');
Copy link
Member

Choose a reason for hiding this comment

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

[curious] Why use userEvent.type but fireEvent.change here? Ideally, we could rely on userEvent to more closely mimic real user behavior.

});
});
Loading