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.

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

Form refactor bankaccountstep #10900

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c09bf34
bank account step form refactored
jayeshmangwani Sep 5, 2022
3f8c116
Merge branch 'main' into form_refactor_bankaccountstep
jayeshmangwani Sep 6, 2022
8808c9c
fixed console warnings & changed inputID defaultValue from undefined …
jayeshmangwani Sep 6, 2022
9bf7312
added shouldSaveDraft to inputs and checkbox
jayeshmangwani Sep 8, 2022
7ecc6c2
Merge branch 'main' into form_refactor_bankaccountstep
jayeshmangwani Sep 8, 2022
590d0df
resolved conflicts & replaced form key with REIMBURSEMENT_ACCOUNT key
jayeshmangwani Sep 15, 2022
f4134db
Merge branch 'main' into form_refactor_bankaccountstep
jayeshmangwani Sep 20, 2022
191739a
added isSubmitButtonVisible form prop
jayeshmangwani Sep 21, 2022
90057a2
Merge branch 'main' into form_refactor_bankaccountstep
jayeshmangwani Sep 21, 2022
2d99e38
skipped validation for plaid
jayeshmangwani Sep 23, 2022
cdb883a
removed inputID from AddPlaidBankAccount
jayeshmangwani Sep 23, 2022
2ef74ca
Merge branch 'main' into form_refactor_bankaccountstep
jayeshmangwani Sep 23, 2022
0e2b26f
added validate function to PLAID Form
jayeshmangwani Sep 24, 2022
4bdc2d7
Merge branch 'main' into form_refactor_bankaccountstep
jayeshmangwani Sep 24, 2022
5f08c30
updated onyx key name for bankaccount form
jayeshmangwani Sep 26, 2022
df9b2c0
merged & resolved conflicts, accepted incoming changes
jayeshmangwani Sep 26, 2022
84011f1
added form component to BankAccountManualStep & BankAccountPlaidStep
jayeshmangwani Sep 26, 2022
e19f42f
updated formID for plaid
jayeshmangwani Sep 26, 2022
e8c0496
Merge branch 'main' into form_refactor_bankaccountstep
jayeshmangwani Sep 26, 2022
58198e2
merged and resolved conflicts
jayeshmangwani Sep 27, 2022
27dddb3
updated ONYX key name from BANK_FORM to REIMBURSEMENT_ACCOUNT_FORM
jayeshmangwani Sep 28, 2022
ebcc540
Merge branch 'main' into form_refactor_bankaccountstep
jayeshmangwani Sep 28, 2022
44cc59d
resolved conflicts
jayeshmangwani Sep 29, 2022
fbd3ca4
Merge branch 'main' into form_refactor_bankaccountstep
jayeshmangwani Sep 30, 2022
bd2e691
merged code from Form and FormActions
jayeshmangwani Sep 30, 2022
08753f3
fixed newline lint issue
jayeshmangwani Sep 30, 2022
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
28 changes: 17 additions & 11 deletions src/components/Form.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ const propTypes = {
// eslint-disable-next-line react/forbid-prop-types
draftValues: PropTypes.object,

/** Should we show the form button */
isSubmitButtonVisible: PropTypes.bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be moved below submitButtonText, as now it sits under the /* Onyx Props */ comment but it's not an Onyx prop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change has been done


...withLocalizePropTypes,
};

Expand All @@ -49,6 +52,7 @@ const defaultProps = {
error: '',
},
draftValues: {},
isSubmitButtonVisible: true,
};

class Form extends React.Component {
Expand Down Expand Up @@ -184,17 +188,19 @@ class Form extends React.Component {
>
<View style={[this.props.style]}>
{this.childrenWrapperWithProps(this.props.children)}
<FormAlertWithSubmitButton
buttonText={this.props.submitButtonText}
isAlertVisible={_.size(this.state.errors) > 0 || Boolean(this.props.formState.error)}
isLoading={this.props.formState.isLoading}
message={this.props.formState.error}
onSubmit={this.submit}
onFixTheErrorsLinkPressed={() => {
this.inputRefs[_.first(_.keys(this.state.errors))].focus();
}}
containerStyles={[styles.mh0, styles.mt5]}
/>
{this.props.isSubmitButtonVisible && (
<FormAlertWithSubmitButton
buttonText={this.props.submitButtonText}
isAlertVisible={_.size(this.state.errors) > 0 || Boolean(this.props.formState.error)}
isLoading={this.props.formState.isLoading}
message={this.props.formState.error}
onSubmit={this.submit}
onFixTheErrorsLinkPressed={() => {
this.inputRefs[_.first(_.keys(this.state.errors))].focus();
}}
containerStyles={[styles.mh0, styles.mt5]}
/>
)}
</View>
</ScrollView>
</>
Expand Down
26 changes: 18 additions & 8 deletions src/pages/ReimbursementAccount/BankAccountStep.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ class BankAccountStep extends React.Component {
this.addPlaidAccount = this.addPlaidAccount.bind(this);
this.validate = this.validate.bind(this);
this.validatePlaidAccount = this.validatePlaidAccount.bind(this);
this.state = {
selectedPlaidBankAccount: undefined,
};
}

/**
Expand All @@ -83,9 +86,10 @@ class BankAccountStep extends React.Component {
return errors;
}

validatePlaidAccount(values) {
validatePlaidAccount() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can skip validating Plaid account, as we don't allow the user to continue unless they choose an account. I don't see a way of triggering the validation error if it exists. @luacmartins what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I think we can skip this, removed validatePlaidAccount for now

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense since we can't display that error and the user can't continue without selecting an account.

const selectedPlaidBankAccount = this.state.selectedPlaidBankAccount;
const errors = {};
if (_.isUndefined(values.selectedPlaidBankAccount)) {
if (_.isUndefined(selectedPlaidBankAccount)) {
errors.selectedPlaidBankAccount = this.props.translate('bankAccount.error.noBankAccountSelected');
}

Expand All @@ -110,9 +114,11 @@ class BankAccountStep extends React.Component {
* Add the Bank account retrieved via Plaid in db
* @param {Object} values - form input values passed by the Form component
*/
addPlaidAccount(values) {
const selectedPlaidBankAccount = values.selectedPlaidBankAccount;

addPlaidAccount() {
const selectedPlaidBankAccount = this.state.selectedPlaidBankAccount;
if (!this.state.selectedPlaidBankAccount) {
return;
}
BankAccounts.setupWithdrawalAccount({
acceptTerms: true,
setupType: CONST.BANK_ACCOUNT.SETUP_TYPE.PLAID,
Expand Down Expand Up @@ -235,11 +241,15 @@ class BankAccountStep extends React.Component {
validate={this.validatePlaidAccount}
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing this completely will cause an error, maybe we can have it return an empty object validate={() => ({})} to satisfy Form. I still think we don't need that validation method because it will only run when selectedPlaidBankAccount is already defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added empty object to validate in PLAID Form

onSubmit={this.addPlaidAccount}
submitButtonText={this.props.translate('common.saveAndContinue')}
style={[styles.mh5, styles.mb5, styles.flex1]}
style={[styles.mh5, styles.flexGrow1]}
isSubmitButtonVisible={!_.isUndefined(this.state.selectedPlaidBankAccount)}
>
<AddPlaidBankAccount
inputID="selectedPlaidBankAccount"
text={this.props.translate('bankAccount.plaidBodyCopy')}
onSelect={(params) => {
this.setState({
selectedPlaidBankAccount: params.selectedPlaidBankAccount,
});
}}
onExitPlaid={() => BankAccounts.setBankAccountSubStep(null)}
receivedRedirectURI={this.props.receivedRedirectURI}
plaidLinkOAuthToken={this.props.plaidLinkOAuthToken}
Expand Down