Merge pull request Expensify#11039 from rushatgabhane/rm-setNativePro…

…ps-form Form - Migrate setNativeProps to state.
autosave-app · Oct 7, 2022 · 8c6dab8 · 8c6dab8
2 parents 18b1007 + bfc9345
commit 8c6dab8
Show file tree

Hide file tree

Showing 9 changed files with 103 additions and 67 deletions.
diff --git a/contributingGuides/FORMS.md b/contributingGuides/FORMS.md
@@ -203,11 +203,15 @@ function onSubmit(values) {
 The following prop is available to form inputs:
 
 - inputID: An unique identifier for the input.
+- shouldSaveDraft: Saves a draft of the input value.
+- defaultValue: The initial value of the input.
+- value: The value to show for the input.
+- onValueChange: A callback that is called when the input's value changes.
 
 Form.js will automatically provide the following props to any input with the inputID prop.
 
 - ref: A React ref that must be attached to the input.
-- defaultValue: The input default value.
+- value: The input value.
 - errorText: The translated error text that is returned by validate for that specific input.
 - onBlur: An onBlur handler that calls validate.
 - onInputChange: An onChange handler that saves draft values and calls validate for that input (inputA). Passing an inputID as a second param allows inputA to manipulate the input value of the provided inputID (inputB).
diff --git a/src/components/Form.js b/src/components/Form.js
@@ -66,10 +66,10 @@ class Form extends React.Component {
 
         this.state = {
             errors: {},
+            inputValues: {},
         };
 
         this.inputRefs = {};
-        this.inputValues = {};
         this.touchedInputs = {};
 
         this.setTouchedInput = this.setTouchedInput.bind(this);
@@ -101,12 +101,12 @@ class Form extends React.Component {
         ));
 
         // Validate form and return early if any errors are found
-        if (!_.isEmpty(this.validate(this.inputValues))) {
+        if (!_.isEmpty(this.validate(this.state.inputValues))) {
             return;
         }
 
         // Call submit handler
-        this.props.onSubmit(this.inputValues);
+        this.props.onSubmit(this.state.inputValues);
     }
 
     /**
@@ -159,30 +159,38 @@ class Form extends React.Component {
             const defaultValue = this.props.draftValues[inputID] || child.props.defaultValue;
 
             // We want to initialize the input value if it's undefined
-            if (_.isUndefined(this.inputValues[inputID])) {
-                this.inputValues[inputID] = defaultValue;
+            if (_.isUndefined(this.state.inputValues[inputID])) {
+                this.state.inputValues[inputID] = defaultValue;
+            }
+
+            if (!_.isUndefined(child.props.value)) {
+                this.state.inputValues[inputID] = child.props.value;
             }
 
             return React.cloneElement(child, {
                 ref: node => this.inputRefs[inputID] = node,
-                defaultValue,
+                value: this.state.inputValues[inputID],
                 errorText: this.state.errors[inputID] || '',
                 onBlur: () => {
                     this.setTouchedInput(inputID);
-                    this.validate(this.inputValues);
+                    this.validate(this.state.inputValues);
                 },
                 onInputChange: (value, key) => {
                     const inputKey = key || inputID;
-                    this.inputValues[inputKey] = value;
-                    const inputRef = this.inputRefs[inputKey];
+                    this.setState(prevState => ({
+                        inputValues: {
+                            ...prevState.inputValues,
+                            [inputKey]: value,
+                        },
+                    }), () => this.validate(this.state.inputValues));
 
-                    if (key && inputRef && _.isFunction(inputRef.setNativeProps)) {
-                        inputRef.setNativeProps({value});
-                    }
                     if (child.props.shouldSaveDraft) {
                         FormActions.setDraftValues(this.props.formID, {[inputKey]: value});
                     }
-                    this.validate(this.inputValues);
+
+                    if (child.props.onValueChange) {
+                        child.props.onValueChange(value);
+                    }
                 },
             });
         });

diff --git a/src/components/Picker/BasePicker/index.js b/src/components/Picker/BasePicker/index.js
@@ -10,26 +10,7 @@ class BasePicker extends React.Component {
     constructor(props) {
         super(props);
 
-        this.pickerValue = this.props.defaultValue;
-
-        this.updateSelectedValueAndExecuteOnChange = this.updateSelectedValueAndExecuteOnChange.bind(this);
         this.executeOnCloseAndOnBlur = this.executeOnCloseAndOnBlur.bind(this);
-        this.setNativeProps = this.setNativeProps.bind(this);
-    }
-
-    /**
-     * This method mimicks RN's setNativeProps method. It's exposed to Picker's ref and can be used by other components
-     * to directly manipulate Picker's value when Picker is used as an uncontrolled input.
-     *
-     * @param {*} value
-     */
-    setNativeProps({value}) {
-        this.pickerValue = value;
-    }
-
-    updateSelectedValueAndExecuteOnChange(value) {
-        this.props.onInputChange(value);
-        this.pickerValue = value;
     }
 
     executeOnCloseAndOnBlur() {
@@ -42,13 +23,12 @@ class BasePicker extends React.Component {
         const hasError = !_.isEmpty(this.props.errorText);
         return (
             <RNPickerSelect
-                onValueChange={this.updateSelectedValueAndExecuteOnChange}
+                onValueChange={this.props.onInputChange}
                 items={this.props.items}
                 style={this.props.size === 'normal' ? basePickerStyles(this.props.disabled, hasError, this.props.focused) : styles.pickerSmall}
                 useNativeAndroidPickerStyle={false}
                 placeholder={this.props.placeholder}
-                value={this.props.value || this.pickerValue}
-                key={this.props.value || this.pickerValue}
+                value={this.props.value}
                 Icon={() => this.props.icon(this.props.size)}
                 disabled={this.props.disabled}
                 fixAndroidTouchableBug
@@ -58,15 +38,11 @@ class BasePicker extends React.Component {
                     onFocus: this.props.onOpen,
                     onBlur: this.executeOnCloseAndOnBlur,
                 }}
-                ref={(node) => {
-                    if (!node || !_.isFunction(this.props.innerRef)) {
+                ref={(el) => {
+                    if (!_.isFunction(this.props.innerRef)) {
                         return;
                     }
-
-                    this.props.innerRef(node);
-
-                    // eslint-disable-next-line no-param-reassign
-                    node.setNativeProps = this.setNativeProps;
+                    this.props.innerRef(el);
                 }}
             />
         );

diff --git a/src/components/Picker/index.js b/src/components/Picker/index.js
@@ -29,6 +29,9 @@ const propTypes = {
 
     /** Saves a draft of the input value when used in a form */
     shouldSaveDraft: PropTypes.bool,
+
+    /** A callback method that is called when the value changes and it receives the selected value as an argument */
+    onInputChange: PropTypes.func.isRequired,
 };
 
 const defaultProps = {
@@ -47,6 +50,23 @@ class Picker extends PureComponent {
         this.state = {
             isOpen: false,
         };
+
+        this.onInputChange = this.onInputChange.bind(this);
+    }
+
+    /**
+     * Forms use inputID to set values. But Picker passes an index as the second parameter to onInputChange
+     * We are overriding this behavior to make Picker work with Form
+     * @param {String} value
+     * @param {Number} index
+     */
+    onInputChange(value, index) {
+        if (this.props.inputID) {
+            this.props.onInputChange(value);
+            return;
+        }
+
+        this.props.onInputChange(value, index);
     }
 
     render() {
@@ -72,6 +92,7 @@ class Picker extends PureComponent {
                         value={this.props.value}
                         // eslint-disable-next-line react/jsx-props-no-spreading
                         {...pickerProps}
+                        onInputChange={this.onInputChange}
                     />
                 </View>
                 <InlineErrorText styles={[styles.mh3]}>

diff --git a/src/components/StatePicker.js b/src/components/StatePicker.js
@@ -32,16 +32,12 @@ const propTypes = {
     /** Error text to display */
     errorText: PropTypes.string,
 
-    /** The default value of the state picker */
-    defaultValue: PropTypes.string,
-
     ...withLocalizePropTypes,
 };
 
 const defaultProps = {
     label: '',
     value: undefined,
-    defaultValue: undefined,
     errorText: '',
     shouldSaveDraft: false,
     inputID: undefined,
@@ -56,7 +52,6 @@ const StatePicker = forwardRef((props, ref) => (
         items={STATES}
         onInputChange={props.onInputChange}
         value={props.value}
-        defaultValue={props.defaultValue}
         label={props.label || props.translate('common.state')}
         errorText={props.errorText}
         onBlur={props.onBlur}

diff --git a/src/pages/ReimbursementAccount/CompanyStep.js b/src/pages/ReimbursementAccount/CompanyStep.js
@@ -256,7 +256,7 @@ class CompanyStep extends React.Component {
                             label={this.props.translate('companyStep.companyType')}
                             items={_.map(this.props.translate('companyStep.incorporationTypes'), (label, value) => ({value, label}))}
                             onInputChange={value => this.clearErrorAndSetValue('incorporationType', value)}
-                            defaultValue={this.state.incorporationType}
+                            value={this.state.incorporationType}
                             placeholder={{value: '', label: '-'}}
                             errorText={this.getErrorText('incorporationType')}
                         />
@@ -275,7 +275,7 @@ class CompanyStep extends React.Component {
                         <StatePicker
                             label={this.props.translate('companyStep.incorporationState')}
                             onInputChange={value => this.clearErrorAndSetValue('incorporationState', value)}
-                            defaultValue={this.state.incorporationState}
+                            value={this.state.incorporationState}
                             errorText={this.getErrorText('incorporationState')}
                         />
                     </View>

diff --git a/src/pages/settings/Profile/ProfilePage.js b/src/pages/settings/Profile/ProfilePage.js
@@ -78,6 +78,8 @@ class ProfilePage extends Component {
         this.getLogins = this.getLogins.bind(this);
         this.validate = this.validate.bind(this);
         this.updatePersonalDetails = this.updatePersonalDetails.bind(this);
+        this.setPronouns = this.setPronouns.bind(this);
+        this.setAutomaticTimezone = this.setAutomaticTimezone.bind(this);
     }
 
     componentDidUpdate(prevProps) {
@@ -96,6 +98,36 @@ class ProfilePage extends Component {
         this.setState(stateToUpdate);
     }
 
+    /**
+     * @param {String} pronouns
+     */
+    setPronouns(pronouns) {
+        const hasSelfSelectedPronouns = pronouns === CONST.PRONOUNS.SELF_SELECT;
+        this.pronouns = hasSelfSelectedPronouns ? '' : pronouns;
+
+        if (this.state.hasSelfSelectedPronouns === hasSelfSelectedPronouns) {
+            return;
+        }
+
+        this.setState({hasSelfSelectedPronouns});
+    }
+
+    /**
+     * Update the timezone picker's value to guessed timezone
+     * @param {Boolean} isAutomaticTimezone
+     */
+    setAutomaticTimezone(isAutomaticTimezone) {
+        if (!isAutomaticTimezone) {
+            this.setState({isAutomaticTimezone});
+            return;
+        }
+
+        this.setState({
+            selectedTimezone: moment.tz.guess(),
+            isAutomaticTimezone,
+        });
+    }
+
     /**
      * Get the most validated login of each type
      *
@@ -166,14 +198,6 @@ class ProfilePage extends Component {
             [values.firstName, values.lastName, values.pronouns],
         );
 
-        const hasSelfSelectedPronouns = values.pronouns === CONST.PRONOUNS.SELF_SELECT;
-        this.pronouns = hasSelfSelectedPronouns ? '' : values.pronouns;
-        this.setState({
-            hasSelfSelectedPronouns,
-            isAutomaticTimezone: values.isAutomaticTimezone,
-            selectedTimezone: values.isAutomaticTimezone ? moment.tz.guess() : values.timezone,
-        });
-
         if (hasFirstNameError) {
             errors.firstName = Localize.translateLocal('personalDetails.error.characterLimit', {limit: CONST.FORM_CHARACTER_LIMIT});
         }
@@ -262,6 +286,7 @@ class ProfilePage extends Component {
                                 label: this.props.translate('profilePage.selectYourPronouns'),
                             }}
                             defaultValue={pronounsPickerValue}
+                            onValueChange={this.setPronouns}
                         />
                         {this.state.hasSelfSelectedPronouns && (
                             <View style={styles.mt2}>
@@ -291,14 +316,15 @@ class ProfilePage extends Component {
                             label={this.props.translate('profilePage.timezone')}
                             items={timezones}
                             isDisabled={this.state.isAutomaticTimezone}
-                            defaultValue={this.state.selectedTimezone}
                             value={this.state.selectedTimezone}
+                            onValueChange={selectedTimezone => this.setState({selectedTimezone})}
                         />
                     </View>
                     <CheckboxWithLabel
                         inputID="isAutomaticTimezone"
                         label={this.props.translate('profilePage.setMyTimezoneAutomatically')}
                         defaultValue={this.state.isAutomaticTimezone}
+                        onValueChange={this.setAutomaticTimezone}
                     />
                 </Form>
             </ScreenWrapper>

diff --git a/src/stories/Form.stories.js b/src/stories/Form.stories.js
@@ -34,7 +34,7 @@ const Template = (args) => {
     // Form consumes data from Onyx, so we initialize Onyx with the necessary data here
     NetworkConnection.setOfflineStatus(false);
     FormActions.setIsLoading(args.formID, args.formState.isLoading);
-    FormActions.setErrorMessage(args.formID, args.formState.error);
+    FormActions.setErrors(args.formID, args.formState.error);
     FormActions.setDraftValues(args.formID, args.draftValues);
 
     return (
@@ -132,7 +132,7 @@ const WithNativeEventHandler = (args) => {
     // Form consumes data from Onyx, so we initialize Onyx with the necessary data here
     NetworkConnection.setOfflineStatus(false);
     FormActions.setIsLoading(args.formID, args.formState.isLoading);
-    FormActions.setErrorMessage(args.formID, args.formState.error);
+    FormActions.setErrors(args.formID, args.formState.error);
     FormActions.setDraftValues(args.formID, args.draftValues);
 
     return (