Skip to content

Conversation

@nkuoch
Copy link
Contributor

@nkuoch nkuoch commented Feb 18, 2022

Details

When there was an error at field level, the field didn't get red, and the corresponding error didn't show up.
It's because getErrors was always sending the original props, not the updated ones that were received after an Onyx change.
Now, I'm passing the right props.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/197281

Tests / QA Steps

Try to transfer your wallet balance.
When asked for your personal additional details, submit the form with empty fields.
Required fields should get red and with a corresponding error message.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

image

Web

@nkuoch nkuoch requested a review from a team as a code owner February 18, 2022 09:57
@nkuoch nkuoch self-assigned this Feb 18, 2022
@MelvinBot MelvinBot requested review from marcochavezf and removed request for a team February 18, 2022 09:57
@nkuoch nkuoch requested review from a team and marcaaron and removed request for marcochavezf February 18, 2022 09:57
@MelvinBot MelvinBot requested review from marcochavezf and removed request for a team February 18, 2022 09:58
Copy link
Contributor

@marcaaron marcaaron left a comment

Choose a reason for hiding this comment

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

Thanks for finding this! 🙇 Just left a couple style comments.


this.getErrors = () => formHelper.getErrors(props);
this.clearError = path => formHelper.clearError(props, path);
getErrors() {
Copy link
Contributor

Choose a reason for hiding this comment

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

style - needs a doc with @returns {Object}

return `${this.props.translate(this.fieldNameTranslationKeys[fieldName])} ${this.props.translate('common.isRequiredField')}.`;
}

clearError(path) {
Copy link
Contributor

Choose a reason for hiding this comment

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

style - needs a doc with @param {String} path

}

clearError(path) {
return this.formHelper.clearError(this.props, path);
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably can skip the return since clearError() returns void

@nkuoch
Copy link
Contributor Author

nkuoch commented Feb 18, 2022

Updated

@nkuoch nkuoch removed the request for review from marcochavezf February 18, 2022 21:12
@marcaaron marcaaron merged commit 2bc5c84 into main Feb 22, 2022
@marcaaron marcaaron deleted the nat-p2pkycfielderr branch February 22, 2022 17:39
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @marcaaron in version: 1.1.40-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@mvtglobally
Copy link

@nkuoch @marcaaron
We do not have any account with balance to check this PR. IF account has $0.00 balance - "Transfer balance" button do not appear at all. Can you please validate this internally?

@nkuoch
Copy link
Contributor Author

nkuoch commented Feb 24, 2022

I just checked it off thanks

@OSBotify
Copy link
Contributor

OSBotify commented Mar 1, 2022

🚀 Deployed to production by @yuwenmemon in version: 1.1.40-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants