diff --git a/contributingGuides/FORMS.md b/contributingGuides/FORMS.md index 2f4c217ad422..b1dcf2dd675b 100644 --- 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 index b20ddc59c3f6..1899efd8fc66 100644 --- 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 index 083929eb4eba..a34b523f383d 100644 --- 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 ( 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 index 81e80bea6541..4dc676966827 100644 --- 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} /> diff --git a/src/components/StatePicker.js b/src/components/StatePicker.js index a76fd7ba0197..ceff688954a8 100644 --- 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 index 4c806b821f7e..5edcc7c53c6a 100644 --- 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 { this.clearErrorAndSetValue('incorporationState', value)} - defaultValue={this.state.incorporationState} + value={this.state.incorporationState} errorText={this.getErrorText('incorporationState')} /> diff --git a/src/pages/settings/Profile/ProfilePage.js b/src/pages/settings/Profile/ProfilePage.js index f7d60d11a6a3..e6ee4c6fe61e 100755 --- 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 && ( @@ -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})} /> diff --git a/src/stories/Form.stories.js b/src/stories/Form.stories.js index 341b132dd2aa..da684206f204 100644 --- 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 ( diff --git a/src/stories/Picker.stories.js b/src/stories/Picker.stories.js index dd26adebc484..9a7a0ea88c4b 100644 --- a/src/stories/Picker.stories.js +++ b/src/stories/Picker.stories.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useState} from 'react'; import Picker from '../components/Picker'; /** @@ -12,7 +12,17 @@ const story = { }; // eslint-disable-next-line react/jsx-props-no-spreading -const Template = args => ; +const Template = (args) => { + const [value, setValue] = useState(''); + return ( + setValue(e)} + // eslint-disable-next-line react/jsx-props-no-spreading + {...args} + /> + ); +}; // Arguments can be passed to the component by binding // See: https://storybook.js.org/docs/react/writing-stories/introduction#using-args @@ -21,7 +31,6 @@ const Default = Template.bind({}); Default.args = { label: 'Default picker', name: 'Default', - onInputChange: () => {}, items: [ { label: 'Orange', @@ -38,7 +47,6 @@ const PickerWithValue = Template.bind({}); PickerWithValue.args = { label: 'Picker with defined value', name: 'Picker with defined value', - onInputChange: () => {}, value: 'apple', items: [ { @@ -57,7 +65,6 @@ ErrorStory.args = { label: 'Picker with error', name: 'PickerWithError', errorText: 'This field has an error.', - onInputChange: () => {}, items: [ { label: 'Orange', @@ -75,7 +82,6 @@ Disabled.args = { label: 'Picker disabled', name: 'Disabled', isDisabled: true, - onInputChange: () => {}, items: [ { label: 'Orange',