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.

Sign up for GitHub

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

Jump to bottom

form: never have value prop for input as undefined #22589

Merged
merged 4 commits into from
Jul 28, 2023
Merged

form: never have value prop for input as undefined #22589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2fa965e
form: never have value prop for input as undefined
hannojg Jul 10, 2023
7e09cc9
fix defaultValue prop usage
hannojg Jul 11, 2023
9eb1f49
remove defaultValue as its controlled by the form
hannojg Jul 11, 2023
a98c3ad
remove default value as its handled by Form.js
hannojg Jul 14, 2023
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
3 changes: 0 additions & 3 deletions src/components/DatePicker/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ class DatePicker extends React.Component {

this.setDate = this.setDate.bind(this);
this.showDatepicker = this.showDatepicker.bind(this);

this.defaultValue = props.defaultValue ? moment(props.defaultValue).format(CONST.DATE.MOMENT_FORMAT_STRING) : '';
Copy link
Contributor

Choose a reason for hiding this comment

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

@hannojg Are we good to remove defaultValue? defaultValue prop is being passed to DatePicker from different components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, i tested it. Its okay, because the DatePicker is only used in Forms. Form.js will copy the datepicker and apply the correct values to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, i'd definitely appreciate a double testing by you of the affected components!

}

componentDidMount() {
Expand Down Expand Up @@ -75,7 +73,6 @@ class DatePicker extends React.Component {
label={this.props.label}
onInputChange={this.setDate}
value={this.props.value}
defaultValue={this.defaultValue}
placeholder={this.props.placeholder}
errorText={this.props.errorText}
containerStyles={this.props.containerStyles}
Expand Down
5 changes: 4 additions & 1 deletion src/components/Form.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ function Form(props) {

// We want to initialize the input value if it's undefined
if (_.isUndefined(inputValues[inputID])) {
inputValues[inputID] = defaultValue;
inputValues[inputID] = defaultValue || '';
}

// We force the form to set the input value from the defaultValue props if there is a saved valid value
Expand Down Expand Up @@ -283,6 +283,9 @@ function Form(props) {
}
},
value: inputValues[inputID],
// As the text input is controlled, we never set the defaultValue prop
// as this is already happening by the value prop.
defaultValue: undefined,
errorText: errors[inputID] || fieldErrorMessage,
onBlur: (event) => {
// We delay the validation in order to prevent Checkbox loss of focus when
Expand Down
6 changes: 0 additions & 6 deletions src/components/NewDatePicker/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,6 @@ class NewDatePicker extends React.Component {
};

this.setDate = this.setDate.bind(this);

// We're using uncontrolled input otherwise it won't be possible to
// raise change events with a date value - each change will produce a date
// and make us reset the text input
this.defaultValue = props.defaultValue ? moment(props.defaultValue).format(CONST.DATE.MOMENT_FORMAT_STRING) : '';
}

/**
Expand All @@ -76,7 +71,6 @@ class NewDatePicker extends React.Component {
icon={Expensicons.Calendar}
label={this.props.label}
value={this.props.value || ''}
defaultValue={this.defaultValue}
placeholder={this.props.placeholder || this.props.translate('common.dateFormat')}
errorText={this.props.errorText}
containerStyles={this.props.containerStyles}
Expand Down
Loading