Redo MoneyInput (better UX, less error cases) (#175)

* feat(money-input): reduce to hasError and hasWarning

BREAKING CHANGE: dropped support for hasAmountError, hasAmountWarning, hasCurrencyError and hasCurrencyWarning. Use hasError and hasWarning instead.

BREAKING CHANGE: isTouched only returns true when both fields were touched from now on.

* feat(money-field): mark both fields as touched when one of them loses focus

We only mark both as blurred when an element which is not part of the MoneyInput gains focus. This is done to ensure the MoneyInput acts as one unit so that errors can be displayed correctly (as they rely on both touched states and would pop up too early if only one of them is checked).

This also auto-focuses the amount intput after a currency has been selected.

* test(money-input): switch tests to react-testing-library

* docs(money-input): fix leftover areAllTouched

Author: dferber90

examples/form-fields.story.js

@@ -274,6 +274,7 @@ class ProductForm extends React.Component {
           value={this.props.formik.values.inventory}
           onChange={this.props.formik.handleChange}
           onBlur={this.props.formik.handleBlur}
+          isDisabled={this.props.formik.isSubmitting}
           touched={this.props.formik.touched.inventory}
           errors={this.props.formik.errors.inventory}
           renderError={key => {
@@ -293,6 +294,7 @@ class ProductForm extends React.Component {
           value={this.props.formik.values.price}
           onChange={this.props.formik.handleChange}
           onBlur={this.props.formik.handleBlur}
+          isDisabled={this.props.formik.isSubmitting}
           currencies={currencies}
           touched={this.props.formik.touched.price}
           errors={this.props.formik.errors.price}
@@ -312,6 +314,7 @@ class ProductForm extends React.Component {
           value={this.props.formik.values.status}
           onChange={this.props.formik.handleChange}
           onBlur={this.props.formik.handleBlur}
+          isDisabled={this.props.formik.isSubmitting}
           options={[
             { value: 'unpublished', label: 'Unpublished' },
             { value: 'modified', label: 'Modified' },

examples/form-inputs.story.js

@@ -385,6 +385,7 @@ class ProductForm extends React.Component {
             value={this.props.formik.values.description}
             onChange={this.props.formik.handleChange}
             onBlur={this.props.formik.handleBlur}
+            isDisabled={this.props.formik.isSubmitting}
             hasError={
               this.props.formik.touched.description &&
               Boolean(this.props.formik.errors.description)
@@ -435,6 +436,7 @@ class ProductForm extends React.Component {
             value={this.props.formik.values.inventory}
             onChange={this.props.formik.handleChange}
             onBlur={this.props.formik.handleBlur}
+            isDisabled={this.props.formik.isSubmitting}
             hasError={
               this.props.formik.touched.inventory &&
               Boolean(this.props.formik.errors.inventory)
@@ -465,20 +467,17 @@ class ProductForm extends React.Component {
             onBlur={this.props.formik.handleBlur}
             currencies={currencies}
             isDisabled={this.props.formik.isSubmitting}
-            hasAmountError={Boolean(
-              this.props.formik.touched.price &&
-                this.props.formik.touched.price.amount &&
-                this.props.formik.errors.price
-            )}
+            hasError={
+              MoneyInput.isTouched(this.props.formik.touched.price) &&
+              Boolean(this.props.formik.errors.price)
+            }
           />
           {MoneyInput.isTouched(this.props.formik.touched.price) &&
-            this.props.formik.errors.price &&
-            this.props.formik.errors.price.missing && (
+            this.props.formik.errors.price?.missing && (
               <ErrorMessage>Missing price</ErrorMessage>
             )}
           {MoneyInput.isTouched(this.props.formik.touched.price) &&
-            this.props.formik.errors.price &&
-            this.props.formik.errors.price.unsupportedHighPrecision && (
+            this.props.formik.errors.price?.unsupportedHighPrecision && (
               <ErrorMessage>
                 This value is a high precision value. High precision pricing is
                 not supported for products.

src/components/fields/money-field/money-field.form.story.js

@@ -39,7 +39,6 @@ storiesOf('Examples|Forms/Fields', module)
             if (MoneyField.isEmpty(values.pricePerTon))
               errors.pricePerTon.missing = true;
 
-            console.log(values, omitEmpty(errors));
             return omitEmpty(errors);
           }}
           onSubmit={(values, formik) => {

src/components/fields/money-field/money-field.js

@@ -1,8 +1,8 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import has from 'lodash.has';
 import requiredIf from 'react-required-if';
 import { FormattedMessage } from 'react-intl';
+import has from 'lodash.has';
 import Constraints from '../../constraints';
 import Spacings from '../../spacings';
 import FieldLabel from '../../field-label';
@@ -98,21 +98,11 @@ class MoneyField extends React.Component {
   });
 
   render() {
-    // We could determine the errors of amount and currencyCode separately
-    // and forward hasCurrencyError / hasAmountError depending on the error.
-    // This would work for example for the known "missing" error.
-    // Doing so would lead to the correct part of the MoneyField being marked
-    // with a red border instead of the complete field.
-    // This is something we can do later / when somebody asks for it.
-    //
-    // We do not use MoneyField.isTouched() as we want to ensure both fields have been touched.
-    // This avoids showing an error when the user just selected a language but didn't add
-    // an amount yet.
-    const hasAnyErrors =
-      this.props.touched &&
-      this.props.touched.amount &&
-      this.props.touched.currencyCode &&
-      hasErrors(this.props.errors);
+    // MoneyField.isTouched() ensures both fields have been touched.
+    // This avoids showing an error when the user just selected a language but
+    // didn't add an amount yet.
+    const hasError =
+      MoneyInput.isTouched(this.props.touched) && hasErrors(this.props.errors);
     return (
       <Constraints.Horizontal constraint={this.props.horizontalConstraint}>
         <Spacings.Stack scale="xs">
@@ -148,13 +138,12 @@ class MoneyField extends React.Component {
             onBlur={this.props.onBlur}
             isDisabled={this.props.isDisabled}
             onChange={this.props.onChange}
-            hasCurrencyError={hasAnyErrors}
-            hasAmountError={hasAnyErrors}
+            hasError={hasError}
             {...filterDataAttributes(this.props)}
           />
           <FieldErrors
             errors={this.props.errors}
-            isVisible={hasAnyErrors}
+            isVisible={hasError}
             renderError={this.props.renderError}
           />
         </Spacings.Stack>

src/components/fields/money-field/money-field.spec.js

@@ -114,8 +114,7 @@ describe('rendering', () => {
       expect(moneyInput).toHaveProp('onBlur', props.onBlur);
       expect(moneyInput).toHaveProp('isDisabled', props.isDisabled);
       expect(moneyInput).toHaveProp('placeholder', props.placeholder);
-      expect(moneyInput).toHaveProp('hasAmountError', true);
-      expect(moneyInput).toHaveProp('hasCurrencyError', true);
+      expect(moneyInput).toHaveProp('hasError', true);
 
       expect(wrapper).toRender(FieldErrors);
       expect(wrapper.find(FieldErrors)).toHaveProp('errors', props.errors);
@@ -145,7 +144,7 @@ describe('rendering', () => {
       wrapper = shallow(<MoneyField {...props} />);
     });
     it('should mark the MoneyInput as erroneous', () => {
-      expect(wrapper.find(MoneyInput)).toHaveProp('hasAmountError', true);
+      expect(wrapper.find(MoneyInput)).toHaveProp('hasError', true);
     });
     it('should render the known error', () => {
       expect(wrapper).toRender(FieldErrors);
@@ -164,7 +163,7 @@ describe('rendering', () => {
       wrapper = shallow(<MoneyField {...props} />);
     });
     it('should mark the NumberInput as erroneous', () => {
-      expect(wrapper.find(MoneyInput)).toHaveProp('hasAmountError', true);
+      expect(wrapper.find(MoneyInput)).toHaveProp('hasError', true);
     });
     it('should forward the error', () => {
       expect(wrapper.find(FieldErrors)).toHaveProp('errors', props.errors);

src/components/inputs/money-input/README.md

@@ -41,10 +41,8 @@ The amount can have an arbitrary precision. When the precision of the amount exc
 | `isDisabled`           | `bool`                                     |    -     | -                            | `false` | Indicates that the field cannot be used.                                                                                                                      |
 | `onBlur`               | `func`                                     |    -     | -                            | -       | Called when the amount field or the currency code dropdown is blurred.                                                                                        |
 | `onChange`             | `function(event)`                          |    ✳️    | -                            | -       | Called with the event of the input or dropdown when either the currency or the amount have changed. Either `onChange` or `onChangeValue` must be passed.      |
-| `hasCurrencyError`     | `bool`                                     |    -     | -                            | -       | Indicates if the currency field has an error                                                                                                                  |
-| `hasCurrencyWarning`   | `bool`                                     |    -     | -                            | -       | Indicates if the currency field has a warning                                                                                                                 |
-| `hasAmountError`       | `bool`                                     |    -     | -                            | -       | Indicates if the centAmount field has an error                                                                                                                |
-| `hasAmountWarning`     | `bool`                                     |    -     | -                            | -       | Indicates if the centAmount field has a warning                                                                                                               |
+| `hasError`             | `bool`                                     |    -     | -                            | -       | Indicates if the input has an error                                                                                                                           |
+| `hasWarning`           | `bool`                                     |    -     | -                            | -       | Indicates if the input has a warning                                                                                                                          |
 | `horizontalConstraint` | `string`                                   |    -     | `s`, `m`, `l`, `xl`, `scale` | `scale` | Horizontal size limit of the input fields.                                                                                                                    |
 
 ### Static methods
@@ -93,6 +91,19 @@ MoneyInput.isEmpty(); // -> true
 MoneyInput.isEmpty({ amount: '5', currencyCode: 'EUR' }); // -> false
 ```
 
+#### `MoneyInput.isTouched`
+
+The `isTouched` function will return `true` when all input elements were touched (currency dropdown and amount input).
+
+```js
+MoneyInput.isTouched({ amount: true, currencyCode: true }); // -> true
+
+MoneyInput.isTouched({ amount: true }); // -> false
+MoneyInput.isTouched({ currencyCode: true }); // -> false
+MoneyInput.isTouched({ amount: false, currencyCode: false }); // -> false
+MoneyInput.isTouched({}); // -> false
+```
+
 #### `MoneyInput.getCurrencyDropdownId`
 
 ##### `getCurrencyDropdownId(idPrefix)`
@@ -209,7 +220,9 @@ return (
           onBlur={() => setFieldTouched('somePrice')}
           isDisabled={isSubmitting}
           onChange={value => setFieldValue('somePrice', value)}
-          hasAmountError={touched.somePrice && Boolean(errors.somePrice)}
+          hasError={
+            MoneyInput.isTouched(touched.somePrice) && Boolean(errors.somePrice)
+          }
           horizontalConstraint="l"
         />
         {touched.somePrice &&

src/components/inputs/money-input/currency-dropdown.js

@@ -6,24 +6,25 @@ import styles from './money-input.mod.css';
 import Currency from './currency';
 import Option from './option';
 import DropdownChevron from './dropdown-chevron';
+import filterDataAttributes from '../../../utils/filter-data-attributes';
 
 const getCurrencyDropdownSelectStyles = ({
   isDisabled,
-  hasCurrencyError,
-  hasCurrencyWarning,
+  hasError,
+  hasWarning,
   isOpen,
 }) => {
   if (isDisabled) return styles['currency-disabled'];
-  if (hasCurrencyError) return styles['currency-error'];
-  if (hasCurrencyWarning) return styles['currency-warning'];
+  if (hasError) return styles['currency-error'];
+  if (hasWarning) return styles['currency-warning'];
   if (isOpen) return styles['currency-active'];
 
   return styles['currency-default'];
 };
 
 const getCurrencyDropdownOptionsStyles = props => {
-  if (props.hasCurrencyError) return styles['options-wrapper-error'];
-  if (props.hasCurrencyWarning) return styles['options-wrapper-warning'];
+  if (props.hasError) return styles['options-wrapper-error'];
+  if (props.hasWarning) return styles['options-wrapper-warning'];
 
   return styles['options-wrapper-active'];
 };
@@ -34,19 +35,20 @@ const CurrencyDropdown = props => (
       <div
         className={getCurrencyDropdownSelectStyles({
           isDisabled: props.isDisabled,
-          hasCurrencyError: props.hasCurrencyError,
-          hasCurrencyWarning: props.hasCurrencyWarning,
+          hasError: props.hasError,
+          hasWarning: props.hasWarning,
           isOpen,
         })}
+        {...filterDataAttributes(props)}
       >
         <div className={styles.languagesDropdown} onClick={toggleMenu}>
           <Spacings.Inline scale="xs" alignItems="center">
             <Currency
               id={props.id}
               isDropdown={true}
               isDisabled={props.isDisabled}
-              hasError={props.hasCurrencyError}
-              hasWarning={props.hasCurrencyWarning}
+              hasError={props.hasError}
+              hasWarning={props.hasWarning}
               currency={props.currencyCode}
             />
             {props.currencies.length > 0 && (
@@ -58,8 +60,8 @@ const CurrencyDropdown = props => (
           props.currencies.length > 0 && (
             <div
               className={getCurrencyDropdownOptionsStyles({
-                hasCurrencyError: props.hasCurrencyError,
-                hasCurrencyWarning: props.hasCurrencyWarning,
+                hasError: props.hasError,
+                hasWarning: props.hasWarning,
               })}
             >
               {props.currencies.map(currencyCode => (
@@ -90,8 +92,8 @@ CurrencyDropdown.propTypes = {
   currencies: PropTypes.arrayOf(PropTypes.string).isRequired,
   currencyCode: PropTypes.string,
   isDisabled: PropTypes.bool,
-  hasCurrencyError: PropTypes.bool,
-  hasCurrencyWarning: PropTypes.bool,
+  hasError: PropTypes.bool,
+  hasWarning: PropTypes.bool,
   onChange: PropTypes.func,
   onBlur: PropTypes.func,
   name: PropTypes.string,

src/components/inputs/money-input/currency.spec.js

@@ -105,9 +105,7 @@ describe('rendering', () => {
 
         describe('error', () => {
           beforeEach(() => {
-            props = createTestProps({
-              hasCurrencyError: true,
-            });
+            props = createTestProps({ hasError: true });
             downshiftProps = { isOpen: false, toggleMenu: jest.fn() };
             downshiftRenderWrapper = render(props, downshiftProps);
           });
@@ -121,9 +119,7 @@ describe('rendering', () => {
 
         describe('warning', () => {
           beforeEach(() => {
-            props = createTestProps({
-              hasCurrencyWarning: true,
-            });
+            props = createTestProps({ hasWarning: true });
             downshiftProps = { isOpen: false, toggleMenu: jest.fn() };
             downshiftRenderWrapper = render(props, downshiftProps);
           });
@@ -185,9 +181,7 @@ describe('rendering', () => {
 
       describe('error', () => {
         beforeEach(() => {
-          props = createTestProps({
-            hasAmountError: true,
-          });
+          props = createTestProps({ hasError: true });
           wrapper = shallow(<MoneyInput {...props} />);
           centAmountField = wrapper.find('input');
         });
@@ -199,9 +193,7 @@ describe('rendering', () => {
 
       describe('warning', () => {
         beforeEach(() => {
-          props = createTestProps({
-            hasAmountWarning: true,
-          });
+          props = createTestProps({ hasWarning: true });
           wrapper = shallow(<MoneyInput {...props} />);
           centAmountField = wrapper.find('input');
         });

src/components/inputs/money-input/money-input.form.story.js

@@ -51,26 +51,18 @@ storiesOf('Examples|Forms/Inputs', module)
                 value={formik.values.price}
                 onChange={formik.handleChange}
                 onBlur={formik.handleBlur}
-                hasCurrencyError={Boolean(
-                  formik.touched.price &&
-                    formik.touched.price.currencyCode &&
-                    formik.errors.price
-                )}
-                hasAmountError={Boolean(
-                  formik.touched.price &&
-                    formik.touched.price.amount &&
-                    formik.errors.price
-                )}
+                hasError={
+                  MoneyInput.isTouched(formik.touched.price) &&
+                  Boolean(formik.errors.price)
+                }
                 horizontalConstraint="m"
               />
               {MoneyInput.isTouched(formik.touched.price) &&
-                formik.errors.price &&
-                formik.errors.price.missing && (
+                formik.errors.price?.missing && (
                   <ErrorMessage>Missing price</ErrorMessage>
                 )}
               {MoneyInput.isTouched(formik.touched.price) &&
-                formik.errors.price &&
-                formik.errors.price.unsupportedHighPrecision && (
+                formik.errors.price?.unsupportedHighPrecision && (
                   <ErrorMessage>
                     This value is a high precision value. High precision pricing
                     is not supported for products.