[InputBase] Fix inconsistent filled state (#16526)

* [FomControl] Add useFormControl hook * Reintroduce onFilled/onEmpty callbacks * [InputBase] Fix onEmpty being fired too often * Remove internal onEmpty/onFilled * [InputBase] Fix out-of-sync filled state
mui · Jul 9, 2019 · 2cceffb · 2cceffb
1 parent 1388c2a
commit 2cceffb
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 107 deletions.
diff --git a/packages/material-ui/src/InputBase/InputBase.d.ts b/packages/material-ui/src/InputBase/InputBase.d.ts
@@ -38,7 +38,6 @@ export interface InputBaseProps
   startAdornment?: React.ReactNode;
   type?: string;
   value?: unknown;
-  onFilled?: () => void;
   /**
    * `onChange`, `onKeyUp` + `onKeyDown` are applied to the inner `InputComponent`,
    * which by default is an input or textarea. Since these handlers differ from the

diff --git a/packages/material-ui/src/InputBase/InputBase.js b/packages/material-ui/src/InputBase/InputBase.js
@@ -143,6 +143,8 @@ export const styles = theme => {
   };
 };
 
+const useEnhancedEffect = typeof window === 'undefined' ? React.useEffect : React.useLayoutEffect;
+
 /**
  * `InputBase` contains as few styles as possible.
  * It aims to be a simple building block for creating an input.
@@ -170,8 +172,6 @@ const InputBase = React.forwardRef(function InputBase(props, ref) {
     onBlur,
     onChange,
     onClick,
-    onEmpty,
-    onFilled,
     onFocus,
     onKeyDown,
     onKeyUp,
@@ -231,34 +231,19 @@ const InputBase = React.forwardRef(function InputBase(props, ref) {
         if (muiFormControl && muiFormControl.onFilled) {
           muiFormControl.onFilled();
         }
-        if (onFilled) {
-          onFilled();
-        }
-        return;
-      }
-
-      if (muiFormControl && muiFormControl.onEmpty) {
+      } else if (muiFormControl && muiFormControl.onEmpty) {
         muiFormControl.onEmpty();
       }
-      if (onEmpty) {
-        onEmpty();
-      }
     },
-    [muiFormControl, onEmpty, onFilled],
+    [muiFormControl],
   );
 
-  React.useEffect(() => {
+  useEnhancedEffect(() => {
     if (isControlled) {
       checkDirty({ value });
     }
   }, [value, checkDirty, isControlled]);
 
-  React.useEffect(() => {
-    if (!isControlled) {
-      checkDirty(inputRef.current);
-    }
-  }, [checkDirty, isControlled]);
-
   const handleFocus = event => {
     // Fix a bug with IE 11 where the focus/blur events are triggered
     // while the input is disabled.
@@ -505,14 +490,6 @@ InputBase.propTypes = {
    * @ignore
    */
   onClick: PropTypes.func,
-  /**
-   * @ignore
-   */
-  onEmpty: PropTypes.func,
-  /**
-   * @ignore
-   */
-  onFilled: PropTypes.func,
   /**
    * @ignore
    */

diff --git a/packages/material-ui/src/InputBase/InputBase.test.js b/packages/material-ui/src/InputBase/InputBase.test.js
@@ -159,41 +159,6 @@ describe('<InputBase />', () => {
       fireEvent.change(input, { target: { value: 'do not work' } });
       expect(input).to.have.property('value', '');
     });
-
-    ['', 0].forEach(value => {
-      describe(`${typeof value} value`, () => {
-        let wrapper;
-        let handleFilled;
-        let handleEmpty;
-
-        before(() => {
-          handleEmpty = spy();
-          handleFilled = spy();
-          wrapper = render(
-            <InputBase value={value} onFilled={handleFilled} onEmpty={handleEmpty} />,
-          );
-        });
-
-        // don't test number because zero is a empty state, whereas '' is not
-        if (typeof value !== 'number') {
-          it('should have called the handleEmpty callback', () => {
-            expect(handleEmpty.callCount).to.equal(1);
-          });
-
-          it('should fire the onFilled callback when dirtied', () => {
-            expect(handleFilled.callCount).to.equal(0);
-            wrapper.setProps({ value: typeof value === 'number' ? 2 : 'hello' });
-            expect(handleFilled.callCount).to.equal(1);
-          });
-
-          it('should fire the onEmpty callback when dirtied', () => {
-            expect(handleEmpty.callCount).to.equal(1);
-            wrapper.setProps({ value });
-            expect(handleEmpty.callCount).to.equal(2);
-          });
-        }
-      });
-    });
   });
 
   describe('prop: inputComponent', () => {
@@ -222,28 +187,6 @@ describe('<InputBase />', () => {
     });
   });
 
-  // Note the initial callback when
-  // uncontrolled only fires for a full mount
-  describe('uncontrolled', () => {
-    it('should fire the onFilled callback when dirtied', () => {
-      const handleFilled = spy();
-      const { container } = render(<InputBase onFilled={handleFilled} defaultValue="hell" />);
-      expect(handleFilled.callCount, 1);
-
-      fireEvent.change(container.querySelector('input'), { target: { value: 'heaven' } });
-      expect(handleFilled.callCount, 2);
-    });
-
-    it('should fire the onEmpty callback when cleaned', () => {
-      const handleEmpty = spy();
-      const { container } = render(<InputBase onEmpty={handleEmpty} defaultValue="hell" />);
-      expect(handleEmpty.callCount, 0);
-
-      fireEvent.change(container.querySelector('input'), { target: { value: '' } });
-      expect(handleEmpty.callCount, 1);
-    });
-  });
-
   describe('with FormControl', () => {
     it('should have the formControl class', () => {
       const { getByTestId } = render(
@@ -255,27 +198,6 @@ describe('<InputBase />', () => {
     });
 
     describe('callbacks', () => {
-      it('should fire the onFilled props callback when dirtied', () => {
-        const handleFilled = spy();
-        const { container } = render(<InputBase onFilled={handleFilled} />);
-
-        fireEvent.change(container.querySelector('input'), { target: { value: 'hello' } });
-        expect(handleFilled.callCount).to.equal(1);
-      });
-
-      it('should fire the and props callback when cleaned', () => {
-        const handleEmpty = spy();
-        const { container } = render(<InputBase onEmpty={handleEmpty} />);
-
-        // Set value to be cleared
-        fireEvent.change(container.querySelector('input'), { target: { value: 'test' } });
-        expect(handleEmpty.callCount, 0);
-
-        // Clear value
-        fireEvent.change(container.querySelector('input'), { target: { value: '' } });
-        expect(handleEmpty.callCount).to.equal(2);
-      });
-
       it('should fire the onClick prop', () => {
         const handleClick = spy();
         const handleFocus = spy();