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

Handle errorText in TextInputWithLabel #4741

Merged
merged 7 commits into from
Aug 20, 2021
Merged

Handle errorText in TextInputWithLabel #4741

merged 7 commits into from
Aug 20, 2021

Conversation

aldo-expensify
Copy link
Contributor

@aldo-expensify aldo-expensify commented Aug 18, 2021 •
edited
Loading

@pecanoro @jasperhuangg

Details

Add support for errorText prop in TextInputWithLabel component.

Currently the error is showing like this:

Screen Shot 2021-08-19 at 9 50 20 AM

Is the styling correct?

Fixed Issues

$ #4730

Tests/QA

  1. Navigate to <baseURL>/bank-account/company.
  2. Input invalid data for the following fields and try to submit the form. Verify that red outlines AND growl messages occur.
    • Empty password
    • Invalid address street
    • Invalid address zip code
    • Invalid website URL
    • Invalid company tax ID
    • Invalid industry code
    • Invalid incorporation date

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@aldo-expensify aldo-expensify requested a review from a team as a code owner August 18, 2021 18:27
@MelvinBot MelvinBot requested review from nkuoch and removed request for a team August 18, 2021 18:28
@jasperhuangg
Copy link
Contributor

jasperhuangg commented Aug 18, 2021 •
edited
Loading

Is the styling correct?

Styling looks good! @nkuoch all yours :)

@aldo-expensify aldo-expensify requested a review from a team August 19, 2021 02:49
@MelvinBot MelvinBot requested review from alex-mechler and removed request for a team August 19, 2021 02:49
shawnborton
shawnborton reviewed Aug 19, 2021
View reviewed changes
src/components/ExpensiTextInput/BaseExpensiTextInput.js Outdated
</TouchableWithoutFeedback>
</View>
{errorText !== '' && (
<Text style={[styles.formError]}>{errorText}</Text>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also put a small margin on top of the error text, like mt1? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Solved.

@jasperhuangg jasperhuangg self-requested a review August 19, 2021 16:37
jasperhuangg
jasperhuangg requested changes Aug 19, 2021
View reviewed changes
Copy link
Contributor

@jasperhuangg jasperhuangg left a comment

Choose a reason for hiding this comment

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

Updated my review, I think we should update this component to have propTypes, defaultProps, and a displayName.

jasperhuangg
jasperhuangg previously approved these changes Aug 19, 2021
View reviewed changes
Copy link
Contributor

@jasperhuangg jasperhuangg left a comment

Choose a reason for hiding this comment

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

Lookin good 👍

@nkuoch nkuoch merged commit cc87bc5 into main Aug 20, 2021
@nkuoch nkuoch deleted the aldo_fix-input-error-messages branch August 20, 2021 08:28
@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.

@isagoico isagoico mentioned this pull request Aug 20, 2021
Closed
This was referenced Aug 23, 2021
Closed
Closed
@github-actions
Copy link
Contributor

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

github-actions bot pushed a commit that referenced this pull request Aug 23, 2021
@nkuoch
Merge pull request #4741 from Expensify/aldo_fix-input-error-messages
a36974f 
Handle errorText in TextInputWithLabel

(cherry picked from commit cc87bc5)
@OSBotify OSBotify mentioned this pull request Aug 23, 2021
Merged
@OSBotify
Copy link
Contributor

🚀 Cherry-picked to staging by @AndrewGable in version: 1.0.86-8 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@Expensify Expensify deleted a comment from OSBotify Aug 24, 2021
@Expensify Expensify deleted a comment from OSBotify Aug 24, 2021
@Julesssss
Copy link
Contributor

Please ignore the above, CI skipped these jobs and simply added a message here.

@OSBotify OSBotify mentioned this pull request Aug 25, 2021
Closed
@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.0.86-11 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.0.88-2 🚀

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

This was referenced Sep 1, 2021
Closed
Closed
Closed
@OSBotify OSBotify mentioned this pull request Aug 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shawnborton shawnborton shawnborton left review comments

@nkuoch nkuoch nkuoch approved these changes

@alex-mechler alex-mechler alex-mechler approved these changes

@jasperhuangg jasperhuangg jasperhuangg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@aldo-expensify @jasperhuangg @OSBotify @Julesssss @shawnborton @nkuoch @alex-mechler